Skip to content

Commit a6671bb

Browse files
committed
OPENAM-6768 FR-880
1 parent eab5e96 commit a6671bb

File tree

2 files changed

+49
-13
lines changed
  • openam-authentication
    • openam-auth-fr-oath/src/main/java/org/forgerock/openam/authentication/modules/fr/oath
    • openam-auth-oath/src/main/java/org/forgerock/openam/authentication/modules/oath

2 files changed

+49
-13
lines changed

openam-authentication/openam-auth-fr-oath/src/main/java/org/forgerock/openam/authentication/modules/fr/oath/AuthenticatorOATH.java

+23-5
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.sun.identity.shared.debug.Debug;
4141
import com.sun.identity.sm.SMSException;
4242
import java.io.IOException;
43+
import java.security.MessageDigest;
4344
import java.util.ArrayList;
4445
import java.util.Arrays;
4546
import java.util.Collections;
@@ -599,7 +600,7 @@ private boolean checkOTP(String otp, AMIdentity id, OathDeviceSettings settings)
599600
for (int i = 0; i <= windowSize; i++) {
600601
otpGen = HOTPAlgorithm.generateOTP(secretKeyBytes, counter + i, passLen, checksum,
601602
truncationOffset);
602-
if (otpGen.equals(otp)) {
603+
if (isEqual(otpGen, otp)) {
603604
//OTP is correct set the counter value to counter+i (+1 for having been successful)
604605
setCounterAttr(id, counter + i + 1, settings);
605606
return true;
@@ -633,6 +634,11 @@ private boolean checkOTP(String otp, AMIdentity id, OathDeviceSettings settings)
633634
//get Time Step
634635
long localTime = (time / totpTimeStep) + (settings.getClockDriftSeconds() / totpTimeStep);
635636

637+
if(lastLoginTimeStep == localTime){
638+
debug.error("OATH.checkOTP(): Login failed attempting to use the same OTP in same Time Step: " + localTime);
639+
throw new InvalidPasswordException(amAuthOATH, "authFailed", null, userName, null);
640+
}
641+
636642
boolean sameWindow = false;
637643

638644
//check if we are in the time window to prevent 2 logins within the window using the same OTP
@@ -648,7 +654,7 @@ private boolean checkOTP(String otp, AMIdentity id, OathDeviceSettings settings)
648654
String passLenStr = Integer.toString(passLen);
649655
otpGen = TOTPAlgorithm.generateTOTP(secretKey, Long.toHexString(localTime), passLenStr);
650656

651-
if (otpGen.equals(otp)) {
657+
if (isEqual(otpGen, otp)) {
652658
setLoginTime(id, localTime, settings);
653659
return true;
654660
}
@@ -660,19 +666,19 @@ private boolean checkOTP(String otp, AMIdentity id, OathDeviceSettings settings)
660666
//check time step after current time
661667
otpGen = TOTPAlgorithm.generateTOTP(secretKey, Long.toHexString(time1), passLenStr);
662668

663-
if (otpGen.equals(otp)) {
669+
if (isEqual(otpGen, otp)) {
664670
setLoginTime(id, time1, settings);
665671
return true;
666672
}
667673

668674
//check time step before current time
669675
otpGen = TOTPAlgorithm.generateTOTP(secretKey, Long.toHexString(time2), passLenStr);
670676

671-
if (otpGen.equals(otp) && sameWindow) {
677+
if (isEqual(otpGen, otp) && sameWindow) {
672678
debug.error("OATH.checkOTP() : Logging in in the same window with a OTP that is older " +
673679
"than the current times OTP");
674680
return false;
675-
} else if (otpGen.equals(otp) && !sameWindow) {
681+
} else if (isEqual(otpGen, otp) && !sameWindow) {
676682
setLoginTime(id, time2, settings);
677683
return true;
678684
}
@@ -790,4 +796,16 @@ private void setLoginTime(AMIdentity id, long time, OathDeviceSettings settings)
790796
devicesDao.saveDeviceProfiles(id.getName(), id.getRealm(),
791797
Collections.singletonList(JsonConversionUtils.toJsonValue(settings)));
792798
}
799+
800+
/**
801+
* Perform time constant equality check.
802+
* Both values should not be null.
803+
*
804+
* @param str1 first value
805+
* @param str2 second vale
806+
* @return true if values are equal
807+
*/
808+
private boolean isEqual(String str1, String str2) {
809+
return MessageDigest.isEqual(str1.getBytes(), str2.getBytes());
810+
}
793811
}

openam-authentication/openam-auth-oath/src/main/java/org/forgerock/openam/authentication/modules/oath/OATH.java

+26-8
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@
5050
import java.util.Collections;
5151
import java.util.ResourceBundle;
5252

53+
import java.security.MessageDigest;
54+
5355
import javax.security.auth.Subject;
5456
import javax.security.auth.callback.Callback;
5557
import javax.security.auth.callback.PasswordCallback;
56-
import javax.xml.bind.DatatypeConverter;
5758

5859
import org.forgerock.openam.authentication.modules.oath.plugins.DefaultSharedSecretProvider;
5960
import org.forgerock.openam.authentication.modules.oath.plugins.SharedSecretProvider;
@@ -419,7 +420,7 @@ private boolean checkOTP(String otp) throws AuthLoginException {
419420
passLen,
420421
checksum,
421422
truncationOffset);
422-
if (otpGen.equals(otp)) {
423+
if (isEqual(otpGen, otp)) {
423424
//OTP is correct set the counter value to counter+i
424425
setCounterAttr(id, counter + i);
425426
return true;
@@ -478,8 +479,13 @@ private boolean checkOTP(String otp) throws AuthLoginException {
478479
long lastLoginTimeStep = lastLoginTimeInSeconds / totpTimeStep;
479480
// get the current time step based on arrival time of OTP
480481
long currentTimeStep = (timeInSeconds / totpTimeStep) + (lastClockDriftInSeconds / totpTimeStep);
481-
boolean sameWindow = false;
482482

483+
if(lastLoginTimeStep == currentTimeStep){
484+
debug.error("OATH.checkOTP(): Login failed attempting to use the same OTP in same Time Step: " + currentTimeStep);
485+
throw new InvalidPasswordException(amAuthOATH, "authFailed", null, userName, null);
486+
}
487+
488+
boolean sameWindow = false;
483489
// check if we are in the time window to prevent 2
484490
// logins within the window using the same OTP
485491
if (lastLoginTimeStep >= (currentTimeStep - totpStepsInWindow)
@@ -499,7 +505,7 @@ private boolean checkOTP(String otp) throws AuthLoginException {
499505

500506
String passLenStr = Integer.toString(passLen);
501507
otpGen = TOTPAlgorithm.generateTOTP(secretKeyBytes, Long.toHexString(currentTimeStep), passLenStr);
502-
if (otpGen.equals(otp)) {
508+
if (isEqual(otpGen, otp)) {
503509
setLoginTime(id, currentTimeStep);
504510
return true;
505511
}
@@ -510,18 +516,18 @@ private boolean checkOTP(String otp) throws AuthLoginException {
510516

511517
// check time step after current time
512518
otpGen = TOTPAlgorithm.generateTOTP(secretKeyBytes, Long.toHexString(timeInFutureStep), passLenStr);
513-
if (otpGen.equals(otp)) {
519+
if (isEqual(otpGen, otp)) {
514520
setLoginTime(id, timeInFutureStep);
515521
return true;
516522
}
517523

518524
// check time step before current time
519525
otpGen = TOTPAlgorithm.generateTOTP(secretKeyBytes, Long.toHexString(timeInPastStep), passLenStr);
520-
if (otpGen.equals(otp) && sameWindow) {
526+
if (isEqual(otpGen, otp) && sameWindow) {
521527
debug.error("OATH.checkOTP(): "
522-
+ "Logging in in the same window with a OTP that is older than the current OTP");
528+
+ "Login the same window with a OTP that is older than the current OTP");
523529
return false;
524-
} else if (otpGen.equals(otp) && !sameWindow) {
530+
} else if (isEqual(otpGen, otp) && !sameWindow) {
525531
setLoginTime(id, timeInPastStep);
526532
return true;
527533
}
@@ -749,4 +755,16 @@ private void validateTOTPParameters() throws AuthLoginException {
749755
throw new AuthLoginException(amAuthOATH, "authFailed", null);
750756
}
751757
}
758+
759+
/**
760+
* Perform time constant equality check.
761+
* Both values should not be null.
762+
*
763+
* @param str1 first value
764+
* @param str2 second vale
765+
* @return true if values are equal
766+
*/
767+
private boolean isEqual(String str1, String str2) {
768+
return MessageDigest.isEqual(str1.getBytes(), str2.getBytes());
769+
}
752770
}

0 commit comments

Comments
 (0)