dreezey / argon2-password-hash-provider

Argon2 Password Hash Provider for Keycloak
Apache License 2.0
25 stars 16 forks source link

Index 1 out of bounds for length 1 #3

Closed RobertHeim closed 3 years ago

RobertHeim commented 3 years ago

Hi and thanks for the integration!

For existing users, re-hasing their existing passwords on login with another hash algo fails.

I can consistently reproduce it:

There exists a testuser with a hashed password using {"hashIterations":27500,"algorithm":"pbkdf2-sha256"}. This is the default and there was no policy set. Then, I changed the password policy to argon2. Creating a new user is fine. However, when I try to login with the existing testuser I get this error:

keycloak_1  | 15:51:39,472 ERROR [org.keycloak.services.error.KeycloakErrorHandler] (default task-10) Uncaught server error: be.cronos.keycloak.exceptions.Argon2RuntimeException: Index 1 out of bounds for length 1
keycloak_1  |   at deployment.argon2-password-hash-provider.jar//be.cronos.keycloak.utils.Argon2EncodingUtils.extractArgon2ParametersFromEncodedPassword(Argon2EncodingUtils.java:46)
keycloak_1  |   at deployment.argon2-password-hash-provider.jar//be.cronos.keycloak.credential.hash.Argon2PasswordHashProvider.policyCheck(Argon2PasswordHashProvider.java:31)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.credential.PasswordCredentialProvider.isValid(PasswordCredentialProvider.java:265)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.credential.UserCredentialStoreManager.validate(UserCredentialStoreManager.java:187)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.credential.UserCredentialStoreManager.isValid(UserCredentialStoreManager.java:177)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.credential.UserCredentialStoreManager.isValid(UserCredentialStoreManager.java:112)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.authentication.authenticators.directgrant.ValidatePassword.authenticate(ValidatePassword.java:47)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.authentication.DefaultAuthenticationFlow.processSingleFlowExecutionModel(DefaultAuthenticationFlow.java:443)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.authentication.DefaultAuthenticationFlow.processFlow(DefaultAuthenticationFlow.java:252)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.authentication.AuthenticationProcessor.authenticateOnly(AuthenticationProcessor.java:978)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.protocol.oidc.endpoints.TokenEndpoint.resourceOwnerPasswordCredentialsGrant(TokenEndpoint.java:635)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.protocol.oidc.endpoints.TokenEndpoint.processGrantRequest(TokenEndpoint.java:220)
keycloak_1  |   at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
keycloak_1  |   at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
keycloak_1  |   at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
keycloak_1  |   at java.base/java.lang.reflect.Method.invoke(Method.java:566)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:138)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.internalInvokeOnTarget(ResourceMethodInvoker.java:543)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTargetAfterFilter(ResourceMethodInvoker.java:432)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.lambda$invokeOnTarget$0(ResourceMethodInvoker.java:393)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.interception.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:358)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:395)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:364)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:150)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:110)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:141)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:104)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:440)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:229)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:135)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.interception.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:358)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:138)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:215)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.plugins.server.servlet.ServletContainerDispatcher.service(ServletContainerDispatcher.java:245)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher.service(HttpServletDispatcher.java:61)
keycloak_1  |   at org.jboss.resteasy.resteasy-jaxrs@3.12.1.Final//org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher.service(HttpServletDispatcher.java:56)
keycloak_1  |   at javax.servlet.api@2.0.0.Final//javax.servlet.http.HttpServlet.service(HttpServlet.java:590)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:74)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:129)
keycloak_1  |   at org.keycloak.keycloak-wildfly-extensions@11.0.1//org.keycloak.provider.wildfly.WildFlyRequestFilter.lambda$doFilter$0(WildFlyRequestFilter.java:41)
keycloak_1  |   at org.keycloak.keycloak-services@11.0.1//org.keycloak.services.filters.AbstractRequestFilter.filter(AbstractRequestFilter.java:43)
keycloak_1  |   at org.keycloak.keycloak-wildfly-extensions@11.0.1//org.keycloak.provider.wildfly.WildFlyRequestFilter.doFilter(WildFlyRequestFilter.java:39)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.core.ManagedFilter.doFilter(ManagedFilter.java:61)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:131)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.FilterHandler.handleRequest(FilterHandler.java:84)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:62)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletChain$1.handleRequest(ServletChain.java:68)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.security.SecurityContextAssociationHandler.handleRequest(SecurityContextAssociationHandler.java:78)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.RedirectDirHandler.handleRequest(RedirectDirHandler.java:68)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.security.SSLInformationAssociationHandler.handleRequest(SSLInformationAssociationHandler.java:132)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.security.ServletAuthenticationCallHandler.handleRequest(ServletAuthenticationCallHandler.java:57)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.security.handlers.AbstractConfidentialityHandler.handleRequest(AbstractConfidentialityHandler.java:46)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.security.ServletConfidentialityConstraintHandler.handleRequest(ServletConfidentialityConstraintHandler.java:64)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.security.handlers.AuthenticationMechanismsHandler.handleRequest(AuthenticationMechanismsHandler.java:60)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.security.CachedAuthenticatedSessionHandler.handleRequest(CachedAuthenticatedSessionHandler.java:77)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.security.handlers.NotificationReceiverHandler.handleRequest(NotificationReceiverHandler.java:50)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.security.handlers.AbstractSecurityContextAssociationHandler.handleRequest(AbstractSecurityContextAssociationHandler.java:43)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.security.jacc.JACCContextIdHandler.handleRequest(JACCContextIdHandler.java:61)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.deployment.GlobalRequestControllerHandler.handleRequest(GlobalRequestControllerHandler.java:68)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler.handleFirstRequest(ServletInitialHandler.java:269)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler.access$100(ServletInitialHandler.java:78)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:133)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:130)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.core.ServletRequestContextThreadSetupAction$1.call(ServletRequestContextThreadSetupAction.java:48)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.core.ContextClassLoaderSetupAction$1.call(ContextClassLoaderSetupAction.java:43)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.security.SecurityContextThreadSetupAction.lambda$create$0(SecurityContextThreadSetupAction.java:105)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1530)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1530)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1530)
keycloak_1  |   at org.wildfly.extension.undertow@20.0.1.Final//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1530)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:249)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler.access$000(ServletInitialHandler.java:78)
keycloak_1  |   at io.undertow.servlet@2.1.3.Final//io.undertow.servlet.handlers.ServletInitialHandler$1.handleRequest(ServletInitialHandler.java:99)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.Connectors.executeRootHandler(Connectors.java:370)
keycloak_1  |   at io.undertow.core@2.1.3.Final//io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830)
keycloak_1  |   at org.jboss.threads@2.3.3.Final//org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
keycloak_1  |   at org.jboss.threads@2.3.3.Final//org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:1982)
keycloak_1  |   at org.jboss.threads@2.3.3.Final//org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1486)
keycloak_1  |   at org.jboss.threads@2.3.3.Final//org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1377)
keycloak_1  |   at java.base/java.lang.Thread.run(Thread.java:834)

Any idea?

RobertHeim commented 3 years ago

I think that in this line

https://github.com/dreezey/argon2-password-hash-provider/blob/67d91193af519c317a7618aa81afe6fb67573d05/src/main/java/be/cronos/keycloak/credential/hash/Argon2PasswordHashProvider.java#L31

it is implicitly assumed that the hash is argon2, then the extraction on its own assumes its argon 2 and would need to contain a $ but it does not and hence this fails with the index out of bound for hash algorithms that do not contain a $ sign in the hash: https://github.com/dreezey/argon2-password-hash-provider/blob/67d91193af519c317a7618aa81afe6fb67573d05/src/main/java/be/cronos/keycloak/utils/Argon2EncodingUtils.java#L38

RobertHeim commented 3 years ago

I have sent a pull request to fix it. Would be happy to get a 2.0.1 release for it.

dreezey commented 3 years ago

Hi @RobertHeim , thanks for your interest in the module.

This is something I have noticed previously but did not manage to implement a fix for this, so thanks for investigating this in detail and submitting PR, I will attempt to get merge done this week.

dreezey commented 3 years ago

Hi @RobertHeim , I released the new version along with an updated JUnit dependency.

Thanks for the PR!

RobertHeim commented 3 years ago

That is great @dreezey ! Thanks!