pedroigor · Jun 22, 2023 · Jun 20, 2023
@@ -30,11 +30,11 @@ public static boolean validOTP(String token, OTPCredentialModel credentialModel,
             TimeBasedOTP validator = new TimeBasedOTP(credentialModel.getOTPCredentialData().getAlgorithm(),
                     credentialModel.getOTPCredentialData().getDigits(), credentialModel.getOTPCredentialData().getPeriod(),
                     lookAheadWindow);
-            return validator.validateTOTP(token, credentialModel.getOTPSecretData().getValue().getBytes());
+            return validator.validateTOTP(token, credentialModel.getDecodedSecret());
         } else {
             HmacOTP validator = new HmacOTP(credentialModel.getOTPCredentialData().getDigits(),
                     credentialModel.getOTPCredentialData().getAlgorithm(), lookAheadWindow);
-            int c = validator.validateHOTP(token, credentialModel.getOTPSecretData().getValue(),
+            int c = validator.validateHOTP(token, credentialModel.getDecodedSecret(),
                     credentialModel.getOTPCredentialData().getCounter());
             return c > -1;
         }

@@ -230,7 +230,7 @@ private static void convertDeprecatedCredentialsFormat(UserRepresentation user)
                             cred.setSecretData("{\"value\":\"" + cred.getHashedSaltedValue() + "\",\"salt\":\"" + cred.getSalt() + "\"}");
                             cred.setPriority(10);
                         } 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());
                             cred.setCredentialData(JsonSerialization.writeValueAsString(credentialData));
                             cred.setSecretData(JsonSerialization.writeValueAsString(secretData));

@@ -54,7 +54,7 @@ public TimeBasedOTP(String algorithm, int numberDigits, int timeIntervalInSecond
      *
      * @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();
 
         String steps = Long.toHexString(T).toUpperCase();
@@ -67,6 +67,10 @@ public String generateTOTP(String secretKey) {
         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>
      *
@@ -88,7 +92,7 @@ public boolean validateTOTP(String token, byte[] secret) {
                 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)) {
                 return true;

@@ -18,6 +18,9 @@
 
 import org.junit.Assert;
 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 java.nio.charset.StandardCharsets;
@@ -55,4 +58,32 @@ public void testTotpLookAround() {
             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()));
+    }
 }
@@ -6,22 +6,34 @@
 import org.keycloak.models.credential.dto.OTPSecretData;
 import org.keycloak.models.OTPPolicy;
 import org.keycloak.models.RealmModel;
+import org.keycloak.models.utils.Base32;
 import org.keycloak.util.JsonSerialization;
 
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 
 public class OTPCredentialModel extends CredentialModel {
 
     public static final String TYPE = "otp";
-
     public static final String TOTP = "totp";
     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 OTPSecretData secretData;
 
     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);
     }
 
@@ -31,7 +43,11 @@ private OTPCredentialModel(OTPCredentialData credentialData, OTPSecretData secre
     }
 
     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();
         return credentialModel;
     }
@@ -92,6 +108,24 @@ public OTPSecretData getOTPSecretData() {
         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(){
         try {
             setCredentialData(JsonSerialization.writeValueAsString(credentialData));

@@ -10,17 +10,21 @@ public class OTPCredentialData {
     private final int period;
     private final String algorithm;
 
+    private final String secretEncoding;
+
     @JsonCreator
     public OTPCredentialData(@JsonProperty("subType") String subType,
                              @JsonProperty("digits") int digits,
                              @JsonProperty("counter") int counter,
                              @JsonProperty("period") int period,
-                             @JsonProperty("algorithm") String algorithm) {
+                             @JsonProperty("algorithm") String algorithm,
+                             @JsonProperty("secretEncoding") String secretEncoding) {
         this.subType = subType;
         this.digits = digits;
         this.counter = counter;
         this.period = period;
         this.algorithm = algorithm;
+        this.secretEncoding = secretEncoding;
     }
 
     public String getSubType() {
@@ -46,4 +50,8 @@ public int getPeriod() {
     public String getAlgorithm() {
         return algorithm;
     }
+
+    public String getSecretEncoding() {
+        return secretEncoding;
+    }
 }
@@ -55,7 +55,7 @@ public static String generateSecret(int length) {
         return sb.toString();
     }
 
-    public String generateHOTP(String key, int counter) {
+    public String generateHOTP(byte[] key, int counter) {
         String steps = Integer.toHexString(counter).toUpperCase();
 
         // Just get a 16 digit string
@@ -66,14 +66,18 @@ public String generateHOTP(String key, int counter) {
 
     }
 
+    public String generateHOTP(String key, int counter) {
+        return generateHOTP(key.getBytes(), counter);
+    }
+
     /**
      *
      * @param token
      * @param key
      * @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
      */
-    public int validateHOTP(String token, String key, int counter) {
+    public int validateHOTP(String token, byte[] key, int counter) {
 
         int newCounter = counter;
         for (newCounter = counter; newCounter <= counter + lookAheadWindow; newCounter++) {
@@ -86,6 +90,10 @@ public int validateHOTP(String token, String key, int counter) {
         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.
      *
@@ -97,7 +105,7 @@ public int validateHOTP(String token, String key, int counter) {
      * @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;
         byte[] hash;
 
@@ -112,9 +120,8 @@ public String generateOTP(String key, String counter, int returnDigits, String c
 
         // Adding one byte to get the right conversion
         // 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
         int offset = hash[hash.length - 1] & 0xf;

@@ -31,8 +31,6 @@
 import org.keycloak.models.utils.HmacOTP;
 import org.keycloak.models.utils.TimeBasedOTP;
 
-import java.nio.charset.StandardCharsets;
-
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
@@ -103,7 +101,7 @@ public boolean isValid(RealmModel realm, UserModel user, CredentialInput credent
 
         if (OTPCredentialModel.HOTP.equals(credentialData.getSubType())) {
             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) {
                 return false;
             }
@@ -112,7 +110,7 @@ public boolean isValid(RealmModel realm, UserModel user, CredentialInput credent
             return true;
         } else if (OTPCredentialModel.TOTP.equals(credentialData.getSubType())) {
             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 (policy.isCodeReusable()) return true;

@@ -22,10 +22,16 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.OAuth2Constants;
+import org.keycloak.admin.client.resource.UserResource;
 import org.keycloak.events.Details;
 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.representations.idm.CredentialRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.AssertEvents;
@@ -40,15 +46,17 @@
 import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.util.RealmRepUtil;
 import org.keycloak.testsuite.util.UserBuilder;
-import org.keycloak.testsuite.util.WaitUtils;
 
 import jakarta.ws.rs.client.Client;
 import jakarta.ws.rs.client.Entity;
 import jakarta.ws.rs.client.WebTarget;
 import jakarta.ws.rs.core.Form;
 import jakarta.ws.rs.core.Response;
+import org.keycloak.util.JsonSerialization;
+
 import java.io.IOException;
 import java.net.MalformedURLException;
+import java.util.List;
 
 import static org.keycloak.testsuite.auth.page.AuthRealm.TEST;
 
@@ -233,4 +241,46 @@ public void loginWithTotp_getToken_checkCompatibilityCLI() throws IOException {
             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());
+    }
 }