Support reading base32 encoded OTP secret

Closes #9434
Closes #11561
This commit is contained in:
Pedro Igor 2023-06-20 17:47:10 -03:00
parent 0ecdebc000
commit eb5edb3a9b
9 changed files with 152 additions and 20 deletions

View file

@ -30,11 +30,11 @@ public class CredentialValidation {
TimeBasedOTP validator = new TimeBasedOTP(credentialModel.getOTPCredentialData().getAlgorithm(), TimeBasedOTP validator = new TimeBasedOTP(credentialModel.getOTPCredentialData().getAlgorithm(),
credentialModel.getOTPCredentialData().getDigits(), credentialModel.getOTPCredentialData().getPeriod(), credentialModel.getOTPCredentialData().getDigits(), credentialModel.getOTPCredentialData().getPeriod(),
lookAheadWindow); lookAheadWindow);
return validator.validateTOTP(token, credentialModel.getOTPSecretData().getValue().getBytes()); return validator.validateTOTP(token, credentialModel.getDecodedSecret());
} else { } else {
HmacOTP validator = new HmacOTP(credentialModel.getOTPCredentialData().getDigits(), HmacOTP validator = new HmacOTP(credentialModel.getOTPCredentialData().getDigits(),
credentialModel.getOTPCredentialData().getAlgorithm(), lookAheadWindow); credentialModel.getOTPCredentialData().getAlgorithm(), lookAheadWindow);
int c = validator.validateHOTP(token, credentialModel.getOTPSecretData().getValue(), int c = validator.validateHOTP(token, credentialModel.getDecodedSecret(),
credentialModel.getOTPCredentialData().getCounter()); credentialModel.getOTPCredentialData().getCounter());
return c > -1; return c > -1;
} }

View file

@ -230,7 +230,7 @@ public class RepresentationToModel {
cred.setSecretData("{\"value\":\"" + cred.getHashedSaltedValue() + "\",\"salt\":\"" + cred.getSalt() + "\"}"); cred.setSecretData("{\"value\":\"" + cred.getHashedSaltedValue() + "\",\"salt\":\"" + cred.getSalt() + "\"}");
cred.setPriority(10); cred.setPriority(10);
} else if (OTPCredentialModel.TOTP.equals(cred.getType()) || OTPCredentialModel.HOTP.equals(cred.getType())) { } else if (OTPCredentialModel.TOTP.equals(cred.getType()) || OTPCredentialModel.HOTP.equals(cred.getType())) {
OTPCredentialData credentialData = new OTPCredentialData(cred.getType(), cred.getDigits(), cred.getCounter(), cred.getPeriod(), cred.getAlgorithm()); OTPCredentialData credentialData = new OTPCredentialData(cred.getType(), cred.getDigits(), cred.getCounter(), cred.getPeriod(), cred.getAlgorithm(), null);
OTPSecretData secretData = new OTPSecretData(cred.getHashedSaltedValue()); OTPSecretData secretData = new OTPSecretData(cred.getHashedSaltedValue());
cred.setCredentialData(JsonSerialization.writeValueAsString(credentialData)); cred.setCredentialData(JsonSerialization.writeValueAsString(credentialData));
cred.setSecretData(JsonSerialization.writeValueAsString(secretData)); cred.setSecretData(JsonSerialization.writeValueAsString(secretData));

View file

@ -54,7 +54,7 @@ public class TimeBasedOTP extends HmacOTP {
* *
* @param secretKey the secret key to derive the token from. * @param secretKey the secret key to derive the token from.
*/ */
public String generateTOTP(String secretKey) { public String generateTOTP(byte[] secretKey) {
long T = this.clock.getCurrentInterval(); long T = this.clock.getCurrentInterval();
String steps = Long.toHexString(T).toUpperCase(); String steps = Long.toHexString(T).toUpperCase();
@ -67,6 +67,10 @@ public class TimeBasedOTP extends HmacOTP {
return generateOTP(secretKey, steps, this.numberDigits, this.algorithm); return generateOTP(secretKey, steps, this.numberDigits, this.algorithm);
} }
public String generateTOTP(String secretKey) {
return generateTOTP(secretKey.getBytes());
}
/** /**
* <p>Validates a token using a secret key.</p> * <p>Validates a token using a secret key.</p>
* *
@ -88,7 +92,7 @@ public class TimeBasedOTP extends HmacOTP {
steps = "0" + steps; steps = "0" + steps;
} }
String candidate = generateOTP(new String(secret), steps, this.numberDigits, this.algorithm); String candidate = generateOTP(secret, steps, this.numberDigits, this.algorithm);
if (candidate.equals(token)) { if (candidate.equals(token)) {
return true; return true;

View file

@ -18,6 +18,9 @@ package org.keycloak.models;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Test; import org.junit.Test;
import org.keycloak.models.credential.OTPCredentialModel;
import org.keycloak.models.credential.OTPCredentialModel.SecretEncoding;
import org.keycloak.models.utils.Base32;
import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.models.utils.TimeBasedOTP;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
@ -55,4 +58,32 @@ public class TotpTest {
Assert.assertTrue("Should accept code with skew offset " + i,totp.validateTOTP(otp, secret.getBytes(StandardCharsets.UTF_8))); Assert.assertTrue("Should accept code with skew offset " + i,totp.validateTOTP(otp, secret.getBytes(StandardCharsets.UTF_8)));
} }
} }
@Test
public void testBase32EncodedSecret() {
TimeBasedOTP totp = new TimeBasedOTP("HmacSHA1", 8, 60, 1);
String rawSecret = "JNSVMMTEKZCUGSKJIVGHMNSQOZBDA5JT";
String otp = totp.generateTOTP(Base32.decode(rawSecret));
OTPCredentialModel credentialModel = OTPCredentialModel.createTOTP(rawSecret, 8, 30, "HmacSHA1");
Assert.assertFalse(totp.validateTOTP(otp, credentialModel.getDecodedSecret()));
OTPCredentialModel encodedCredential = OTPCredentialModel.createTOTP(rawSecret, 8, 30, "HmacSHA1", SecretEncoding.BASE32.name());
Assert.assertTrue(totp.validateTOTP(otp, encodedCredential.getDecodedSecret()));
}
@Test
public void testBase32BinaryEncodedSecret() {
TimeBasedOTP totp = new TimeBasedOTP("HmacSHA1", 8, 60, 1);
String rawSecret = "CDLYAYRJ73ORTU4PUWWATWSYQCP4H2QL";
String otp = totp.generateTOTP(Base32.decode(rawSecret));
OTPCredentialModel credentialModel = OTPCredentialModel.createTOTP(rawSecret, 8, 30, "HmacSHA1");
Assert.assertFalse(totp.validateTOTP(otp, credentialModel.getDecodedSecret()));
OTPCredentialModel encodedCredential = OTPCredentialModel.createTOTP(rawSecret, 8, 30, "HmacSHA1", SecretEncoding.BASE32.name());
Assert.assertTrue(totp.validateTOTP(otp, encodedCredential.getDecodedSecret()));
}
} }

View file

@ -6,22 +6,34 @@ import org.keycloak.models.credential.dto.OTPCredentialData;
import org.keycloak.models.credential.dto.OTPSecretData; import org.keycloak.models.credential.dto.OTPSecretData;
import org.keycloak.models.OTPPolicy; import org.keycloak.models.OTPPolicy;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.utils.Base32;
import org.keycloak.util.JsonSerialization; import org.keycloak.util.JsonSerialization;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.StandardCharsets;
public class OTPCredentialModel extends CredentialModel { public class OTPCredentialModel extends CredentialModel {
public static final String TYPE = "otp"; public static final String TYPE = "otp";
public static final String TOTP = "totp"; public static final String TOTP = "totp";
public static final String HOTP = "hotp"; public static final String HOTP = "hotp";
/**
* The supported encodings when reading the raw secret from the storage
*/
public enum SecretEncoding {
BASE32
}
private final OTPCredentialData credentialData; private final OTPCredentialData credentialData;
private final OTPSecretData secretData; private final OTPSecretData secretData;
private OTPCredentialModel(String secretValue, String subType, int digits, int counter, int period, String algorithm) { private OTPCredentialModel(String secretValue, String subType, int digits, int counter, int period, String algorithm) {
credentialData = new OTPCredentialData(subType, digits, counter, period, algorithm); this(secretValue, subType, digits, counter, period, algorithm, null);
}
private OTPCredentialModel(String secretValue, String subType, int digits, int counter, int period, String algorithm, String secretEncoding) {
credentialData = new OTPCredentialData(subType, digits, counter, period, algorithm, secretEncoding);
secretData = new OTPSecretData(secretValue); secretData = new OTPSecretData(secretValue);
} }
@ -31,7 +43,11 @@ public class OTPCredentialModel extends CredentialModel {
} }
public static OTPCredentialModel createTOTP(String secretValue, int digits, int period, String algorithm){ public static OTPCredentialModel createTOTP(String secretValue, int digits, int period, String algorithm){
OTPCredentialModel credentialModel = new OTPCredentialModel(secretValue, TOTP, digits, 0, period, algorithm); return createTOTP(secretValue, digits, period, algorithm, null);
}
public static OTPCredentialModel createTOTP(String secretValue, int digits, int period, String algorithm, String encoding){
OTPCredentialModel credentialModel = new OTPCredentialModel(secretValue, TOTP, digits, 0, period, algorithm, encoding);
credentialModel.fillCredentialModelFields(); credentialModel.fillCredentialModelFields();
return credentialModel; return credentialModel;
} }
@ -92,6 +108,24 @@ public class OTPCredentialModel extends CredentialModel {
return secretData; return secretData;
} }
public byte[] getDecodedSecret() {
String encoding = credentialData.getSecretEncoding();
if (encoding == null) {
return secretData.getValue().getBytes(StandardCharsets.UTF_8);
}
try {
if (SecretEncoding.BASE32.equals(SecretEncoding.valueOf(encoding.toUpperCase()))) {
return Base32.decode(secretData.getValue());
}
throw new RuntimeException("Unsupported secret encoding: " + encoding);
} catch (Exception cause) {
throw new RuntimeException("Failed to decode otp secret using encoding [" + encoding + "]", cause);
}
}
private void fillCredentialModelFields(){ private void fillCredentialModelFields(){
try { try {
setCredentialData(JsonSerialization.writeValueAsString(credentialData)); setCredentialData(JsonSerialization.writeValueAsString(credentialData));

View file

@ -10,17 +10,21 @@ public class OTPCredentialData {
private final int period; private final int period;
private final String algorithm; private final String algorithm;
private final String secretEncoding;
@JsonCreator @JsonCreator
public OTPCredentialData(@JsonProperty("subType") String subType, public OTPCredentialData(@JsonProperty("subType") String subType,
@JsonProperty("digits") int digits, @JsonProperty("digits") int digits,
@JsonProperty("counter") int counter, @JsonProperty("counter") int counter,
@JsonProperty("period") int period, @JsonProperty("period") int period,
@JsonProperty("algorithm") String algorithm) { @JsonProperty("algorithm") String algorithm,
@JsonProperty("secretEncoding") String secretEncoding) {
this.subType = subType; this.subType = subType;
this.digits = digits; this.digits = digits;
this.counter = counter; this.counter = counter;
this.period = period; this.period = period;
this.algorithm = algorithm; this.algorithm = algorithm;
this.secretEncoding = secretEncoding;
} }
public String getSubType() { public String getSubType() {
@ -46,4 +50,8 @@ public class OTPCredentialData {
public String getAlgorithm() { public String getAlgorithm() {
return algorithm; return algorithm;
} }
public String getSecretEncoding() {
return secretEncoding;
}
} }

View file

@ -55,7 +55,7 @@ public class HmacOTP {
return sb.toString(); return sb.toString();
} }
public String generateHOTP(String key, int counter) { public String generateHOTP(byte[] key, int counter) {
String steps = Integer.toHexString(counter).toUpperCase(); String steps = Integer.toHexString(counter).toUpperCase();
// Just get a 16 digit string // Just get a 16 digit string
@ -66,6 +66,10 @@ public class HmacOTP {
} }
public String generateHOTP(String key, int counter) {
return generateHOTP(key.getBytes(), counter);
}
/** /**
* *
* @param token * @param token
@ -73,7 +77,7 @@ public class HmacOTP {
* @param counter * @param counter
* @return -1 if not a match. A positive number means successful validation. This positive number is also the new value of the counter * @return -1 if not a match. A positive number means successful validation. This positive number is also the new value of the counter
*/ */
public int validateHOTP(String token, String key, int counter) { public int validateHOTP(String token, byte[] key, int counter) {
int newCounter = counter; int newCounter = counter;
for (newCounter = counter; newCounter <= counter + lookAheadWindow; newCounter++) { for (newCounter = counter; newCounter <= counter + lookAheadWindow; newCounter++) {
@ -86,6 +90,10 @@ public class HmacOTP {
return -1; return -1;
} }
public int validateHOTP(String token, String key, int counter) {
return validateHOTP(token, key.getBytes(), counter);
}
/** /**
* This method generates an OTP value for the given set of parameters. * This method generates an OTP value for the given set of parameters.
* *
@ -97,7 +105,7 @@ public class HmacOTP {
* @throws java.security.GeneralSecurityException * @throws java.security.GeneralSecurityException
* *
*/ */
public String generateOTP(String key, String counter, int returnDigits, String crypto) { public String generateOTP(byte[] key, String counter, int returnDigits, String crypto) {
String result = null; String result = null;
byte[] hash; byte[] hash;
@ -112,9 +120,8 @@ public class HmacOTP {
// Adding one byte to get the right conversion // Adding one byte to get the right conversion
// byte[] k = hexStr2Bytes(key); // byte[] k = hexStr2Bytes(key);
byte[] k = key.getBytes();
hash = hmac_sha1(crypto, k, msg); hash = hmac_sha1(crypto, key, msg);
// put selected bytes into result int // put selected bytes into result int
int offset = hash[hash.length - 1] & 0xf; int offset = hash[hash.length - 1] & 0xf;

View file

@ -31,8 +31,6 @@ import org.keycloak.models.credential.dto.OTPSecretData;
import org.keycloak.models.utils.HmacOTP; import org.keycloak.models.utils.HmacOTP;
import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.models.utils.TimeBasedOTP;
import java.nio.charset.StandardCharsets;
/** /**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a> * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $ * @version $Revision: 1 $
@ -103,7 +101,7 @@ public class OTPCredentialProvider implements CredentialProvider<OTPCredentialMo
if (OTPCredentialModel.HOTP.equals(credentialData.getSubType())) { if (OTPCredentialModel.HOTP.equals(credentialData.getSubType())) {
HmacOTP validator = new HmacOTP(credentialData.getDigits(), credentialData.getAlgorithm(), policy.getLookAheadWindow()); HmacOTP validator = new HmacOTP(credentialData.getDigits(), credentialData.getAlgorithm(), policy.getLookAheadWindow());
int counter = validator.validateHOTP(challengeResponse, secretData.getValue(), credentialData.getCounter()); int counter = validator.validateHOTP(challengeResponse, otpCredentialModel.getDecodedSecret(), credentialData.getCounter());
if (counter < 0) { if (counter < 0) {
return false; return false;
} }
@ -112,7 +110,7 @@ public class OTPCredentialProvider implements CredentialProvider<OTPCredentialMo
return true; return true;
} else if (OTPCredentialModel.TOTP.equals(credentialData.getSubType())) { } else if (OTPCredentialModel.TOTP.equals(credentialData.getSubType())) {
TimeBasedOTP validator = new TimeBasedOTP(credentialData.getAlgorithm(), credentialData.getDigits(), credentialData.getPeriod(), policy.getLookAheadWindow()); TimeBasedOTP validator = new TimeBasedOTP(credentialData.getAlgorithm(), credentialData.getDigits(), credentialData.getPeriod(), policy.getLookAheadWindow());
final boolean isValid = validator.validateTOTP(challengeResponse, secretData.getValue().getBytes(StandardCharsets.UTF_8)); final boolean isValid = validator.validateTOTP(challengeResponse, otpCredentialModel.getDecodedSecret());
if (isValid) { if (isValid) {
if (policy.isCodeReusable()) return true; if (policy.isCodeReusable()) return true;

View file

@ -22,10 +22,16 @@ import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.keycloak.OAuth2Constants; import org.keycloak.OAuth2Constants;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.models.Constants; import org.keycloak.models.Constants;
import org.keycloak.models.OTPPolicy; import org.keycloak.models.credential.OTPCredentialModel;
import org.keycloak.models.credential.OTPCredentialModel.SecretEncoding;
import org.keycloak.models.credential.dto.OTPCredentialData;
import org.keycloak.models.credential.dto.OTPSecretData;
import org.keycloak.models.utils.Base32;
import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.AssertEvents;
@ -40,15 +46,17 @@ import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.RealmRepUtil; import org.keycloak.testsuite.util.RealmRepUtil;
import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.UserBuilder;
import org.keycloak.testsuite.util.WaitUtils;
import jakarta.ws.rs.client.Client; import jakarta.ws.rs.client.Client;
import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.client.Entity;
import jakarta.ws.rs.client.WebTarget; import jakarta.ws.rs.client.WebTarget;
import jakarta.ws.rs.core.Form; import jakarta.ws.rs.core.Form;
import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response;
import org.keycloak.util.JsonSerialization;
import java.io.IOException; import java.io.IOException;
import java.net.MalformedURLException; import java.net.MalformedURLException;
import java.util.List;
import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; import static org.keycloak.testsuite.auth.page.AuthRealm.TEST;
@ -233,4 +241,46 @@ public class LoginTotpTest extends AbstractTestRealmKeycloakTest {
httpClient.close(); httpClient.close();
} }
} }
@Test
public void testBase32EncodedSecret() throws IOException {
UserRepresentation userRep = testRealm().users().search("test-user@localhost").get(0);
UserResource user = testRealm().users().get(userRep.getId());
List<CredentialRepresentation> credentials = user.credentials();
CredentialRepresentation otpCredential = credentials.stream()
.filter(c -> OTPCredentialModel.TYPE.equals(c.getType()))
.findAny().orElse(null);
Assert.assertNotNull(otpCredential);
OTPCredentialData credentialData = JsonSerialization.readValue(otpCredential.getCredentialData(), OTPCredentialData.class);
OTPCredentialData newCredentialData = new OTPCredentialData(credentialData.getSubType(), credentialData.getDigits(), credentialData.getCounter(), credentialData.getPeriod(), credentialData.getAlgorithm(),
SecretEncoding.BASE32.name());
UserRepresentation newUser = UserBuilder.create().username("test-otp-user@localhost").password("password").enabled(true).build();
CredentialRepresentation credential = new CredentialRepresentation();
credential.setType(otpCredential.getType());
credential.setTemporary(false);
credential.setUserLabel("my-otp");
credential.setCredentialData(JsonSerialization.writeValueAsString(newCredentialData));
String rawSecret = "JXGDDKNLXTBKGTA2KV6QJGAF4SS4R75X";
credential.setSecretData(JsonSerialization.writeValueAsString(new OTPSecretData(rawSecret)));
newUser.getCredentials().add(credential);
testRealm().users().create(newUser).close();
loginPage.open();
loginPage.login(newUser.getUsername(), "password");
Assert.assertTrue(loginTotpPage.isCurrent());
setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp);
loginTotpPage.login(totp.generateTOTP(Base32.decode(rawSecret)));
Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
}
} }