Uploaded image for project: 'Keycloak'
  1. Keycloak
  2. KEYCLOAK-18914

Password credential decoding from DB may fail in rare cases - No login possible

    XMLWordPrintable

Details

    • Bug
    • Status: Open (View Workflow)
    • Major
    • Resolution: Unresolved
    • 12.0.0, 13.0.0, 14.0.0, 15.0.0
    • 17.0.0
    • Core
    • None

    Description

      Hi,

      there is a small possibility (that actually occurred in production) where a user can't login or do any action anymore with his account (e.g. Password reset). This is triggered by an invalid password salt decoding when fetching a password from the DB.

       

      Detailed explanation:

      1. When a user changes or creates a new password and the Pbkdf2PasswordHashProvider is used a random salt is generated and stored in the DB.

      (see https://github.com/keycloak/keycloak/blob/cd342ad5714f15db1cc8b0cd55b788e6543c6dc8/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java#L118)

      2. When the user later tries to login (or do any other action like pw reset) the stored user password credential is fetched from the DB. During this process the PasswordSecretData (stored as JSON string) is decoded. The salt value is Base64 encoded and will be decoded.

      (see https://github.com/keycloak/keycloak/blob/cd342ad5714f15db1cc8b0cd55b788e6543c6dc8/server-spi/src/main/java/org/keycloak/models/credential/dto/PasswordSecretData.java#L39)

      3. Here is a custom Base64 Keycloak implementation used. This implementation does not only do a base64decode, but also tries a gzip unzip by default.

      (see https://github.com/keycloak/keycloak/blob/cd342ad5714f15db1cc8b0cd55b788e6543c6dc8/common/src/main/java/org/keycloak/common/util/Base64.java#L1260)

       

      4. This is problematic in cases where the value is random or anything unexpected matching a certain pattern. This may cause an IOException in the best case. It may also result in an invalid result if possibly can be decoded by coincedence.

       

      5. If an IOException by Java GZIP is thrown it is caught and checked in https://github.com/keycloak/keycloak/blob/cd342ad5714f15db1cc8b0cd55b788e6543c6dc8/common/src/main/java/org/keycloak/common/util/Base64.java#L1285 .
      The Problem here again is the line:

      if (e.getMessage().equals("Unsupported compression method")) {
      

      This may lead to an NullPointerException as in most cases, but not all, a message is set.

       

      6. If the random salt the user created matches certain conditions (GZIP magic header bytes + compression method check + FNAME or FDESCRIPTION + no Terminating 0 byte)
      An Exception without message is thrown.

      (see here https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/19fb8f93c59dfd791f62d41f332db9e306bc1422/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L187 and in combination here https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/19fb8f93c59dfd791f62d41f332db9e306bc1422/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L269)

       

      This causes an unhandled NullPointerException as mentioned above. This actually occurred in production on a real customer account. Here is a stack trace:

      2021-07-28 17:05:39,618 WARN  [org.keycloak.services] (default task-530) KC-SERVICES0013: Failed authentication: java.lang.RuntimeException: com.fasterxml.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `org.keycloak.models.credential.dto.PasswordSecretData`, problem: `java.lang.NullPointerException`
       at [Source: (String)"{"value":"[removed]","salt":"H4sIy[..removed part..]A=="}"; line: 1, column: 134]
              at org.keycloak.keycloak-server-spi@12.0.4//org.keycloak.models.credential.PasswordCredentialModel.createFromCredentialModel(PasswordCredentialModel.java:59)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.PasswordCredentialProvider.getPassword(PasswordCredentialProvider.java:72)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.PasswordCredentialProvider.isValid(PasswordCredentialProvider.java:242)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.UserCredentialStoreManager.lambda$validate$3(UserCredentialStoreManager.java:167)
              at java.base/java.util.Collection.removeIf(Collection.java:544)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.UserCredentialStoreManager.validate(UserCredentialStoreManager.java:167)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.UserCredentialStoreManager.lambda$isValid$2(UserCredentialStoreManager.java:161)
              at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
              at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
              at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
              at java.base/java.util.LinkedList$LLSpliterator.forEachRemaining(LinkedList.java:1239)
              at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
              at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
              at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
              at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
              at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
              at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.UserCredentialStoreManager.isValid(UserCredentialStoreManager.java:161)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.credential.UserCredentialStoreManager.isValid(UserCredentialStoreManager.java:110)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator.validatePassword(AbstractUsernameFormAuthenticator.java:218)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator.validatePassword(AbstractUsernameFormAuthenticator.java:207)
              at org.keycloak.keycloak-services@12.0.4//org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator.validateUserAndPassword(AbstractUsernameFormAuthenticator.java:147)
      
      [.....]
      Caused by: com.fasterxml.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `org.keycloak.models.credential.dto.PasswordSecretData`, problem: `java.lang.NullPointerException`
       at [Source: (String)"{"value":"[removed]","salt":"H4sIy[..removed part..]A=="}"; line: 1, column: 134]
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.exc.ValueInstantiationException.from(ValueInstantiationException.java:47)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.DeserializationContext.instantiationException(DeserializationContext.java:1735)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.wrapAsJsonMappingException(StdValueInstantiator.java:491)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.rewrapCtorProblem(StdValueInstantiator.java:514)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createFromObjectWith(StdValueInstantiator.java:285)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:229)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:198)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:488)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1292)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:326)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4218)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3214)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3182)
              at org.keycloak.keycloak-core@12.0.4//org.keycloak.util.JsonSerialization.readValue(JsonSerialization.java:75)
              at org.keycloak.keycloak-server-spi@12.0.4//org.keycloak.models.credential.PasswordCredentialModel.createFromCredentialModel(PasswordCredentialModel.java:47)
              ... 103 more
      Caused by: java.lang.NullPointerException
              at org.keycloak.keycloak-common@12.0.4//org.keycloak.common.util.Base64.decode(Base64.java:1285)
              at org.keycloak.keycloak-common@12.0.4//org.keycloak.common.util.Base64.decode(Base64.java:1224)
              at org.keycloak.keycloak-server-spi@12.0.4//org.keycloak.models.credential.dto.PasswordSecretData.<init>(PasswordSecretData.java:39)
              at jdk.internal.reflect.GeneratedConstructorAccessor271.newInstance(Unknown Source)
              at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
              at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.introspect.AnnotatedConstructor.call(AnnotatedConstructor.java:124)
              at com.fasterxml.jackson.core.jackson-databind@2.10.5//com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createFromObjectWith(StdValueInstantiator.java:283)
              ... 114 more

       

      Solution Proposals:

      1. The obvious adjustment at the exception check:

      https://github.com/keycloak/keycloak/blob/cd342ad5714f15db1cc8b0cd55b788e6543c6dc8/common/src/main/java/org/keycloak/common/util/Base64.java#L1285 .

      if (e.getMessage() != null && e.getMessage().equals("Unsupported compression method")) {
      

       

      This will result in stack trace printing else case and returning the original value.

      2. However, this approach (alone) isn't very clean as it still logs invalid errors and leaves a hole for rare cases where an unencoded random value passes the decoding process. This is a conceptual flaw as an unzipped values is tried to be unzipped. I would recommend here to carefully check where a zipped value is expected and such a case is tolerable and only unzip there. Therefore, I would propose to invert the non default option DONT_GUNZIP to DO_GUNZIP (or TRY_GUNZIP) and enable it explicitly only where needed / expected.

      If you don't agree please consider at least disabling this feature at least for the known random salt decoding in https://github.com/keycloak/keycloak/blob/05dfed721a62a4117d1c8e5dcfdc7b6e26e466dd/server-spi/src/main/java/org/keycloak/models/credential/dto/PasswordSecretData.java#L39 .

      I hope this issue can be resolved by the information I provided.

      Best, Hans-Christian Halfbrodt

      Attachments

        Activity

          People

            Unassigned Unassigned
            hc.halfbrodt Hans-Christian Halfbrodt (Inactive)
            Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated: