vertx-auth icon indicating copy to clipboard operation
vertx-auth copied to clipboard

Access token JWT being validated and failing token flow

Open narras-oss opened this issue 2 years ago • 2 comments

Version: Seen in latest 4.2.6

Context

The library fails if access token is in JWT format with invalid JWT signature. However if the access token is not in valid JWT format, it ignores the error and moves on. OAuth2 spec does not require access tokens to be in any format , they should be treated as opaque string. hence we should ignore errors if JWT decoding and signature validation of access token fails.

This issue was encountered when we tries to perform simple oAuth2 flow with Azure as IDP. Azure generated access token which is in JWT format but has invalid signature

Do you have a reproducer?

No

Steps to reproduce

If the access token returned in token call is passed in as a random string things works fine but when it is passed in JWT format and the signature is not valid, the call fails with "java.lang.RuntimeException: Signature verification failed" Stack trace is below java.lang.RuntimeException: Signature verification failed at io.vertx.ext.auth.impl.jose.JWT.decode(JWT.java:312) ~[vertx-auth-common-4.2.6.jar:4.2.6] at io.vertx.ext.auth.impl.jose.JWT.decode(JWT.java:177) ~[vertx-auth-common-4.2.6.jar:4.2.6] at io.vertx.ext.auth.oauth2.impl.OAuth2AuthProviderImpl.createUser(OAuth2AuthProviderImpl.java:592) ~[vertx-auth-oauth2-4.2.6.jar:4.2.6] at io.vertx.ext.auth.oauth2.impl.OAuth2AuthProviderImpl.lambda$authenticate$4(OAuth2AuthProviderImpl.java:464) ~[vertx-auth-oauth2-4.2.6.jar:4.2.6] at io.vertx.ext.auth.oauth2.impl.OAuth2API.lambda$token$1(OAuth2API.java:264) ~[vertx-auth-oauth2-4.2.6.jar:4.2.6] at io.vertx.ext.auth.oauth2.impl.OAuth2API.lambda$null$6(OAuth2API.java:656) ~[vertx-auth-oauth2-4.2.6.jar:4.2.6] at io.vertx.core.impl.future.FutureImpl$3.onSuccess(FutureImpl.java:141) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.future.FutureBase.lambda$emitSuccess$0(FutureBase.java:54) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.EventLoopContext.execute(EventLoopContext.java:81) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.ContextImpl.execute(ContextImpl.java:260) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.EventLoopContext.execute(EventLoopContext.java:22) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.future.FutureBase.emitSuccess(FutureBase.java:51) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.future.FutureImpl.tryComplete(FutureImpl.java:211) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.future.PromiseImpl.tryComplete(PromiseImpl.java:23) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.HttpEventHandler.handleEnd(HttpEventHandler.java:79) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.HttpClientResponseImpl.handleEnd(HttpClientResponseImpl.java:250) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.Http1xClientConnection$StreamImpl.lambda$new$0(Http1xClientConnection.java:383) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.streams.impl.InboundBuffer.handleEvent(InboundBuffer.java:240) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.streams.impl.InboundBuffer.write(InboundBuffer.java:130) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.Http1xClientConnection$StreamImpl.handleEnd(Http1xClientConnection.java:610) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.EventLoopContext.execute(EventLoopContext.java:71) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.impl.ContextImpl.execute(ContextImpl.java:267) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.Http1xClientConnection.handleResponseEnd(Http1xClientConnection.java:844) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.Http1xClientConnection.handleHttpMessage(Http1xClientConnection.java:716) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.http.impl.Http1xClientConnection.handleMessage(Http1xClientConnection.java:680) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.net.impl.ConnectionBase.read(ConnectionBase.java:155) ~[vertx-core-4.2.6.jar:4.2.6] at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:153) ~[vertx-core-4.2.6.jar:4.2.6] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:327) ~[netty-codec-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:299) ~[netty-codec-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1372) ~[netty-handler-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1235) ~[netty-handler-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1284) ~[netty-handler-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:510) ~[netty-codec-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:449) ~[netty-codec-4.1.75.Final.jar:4.1.75.Final] at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:279) ~[netty-codec-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:722) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:658) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:584) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:496) ~[netty-transport-4.1.75.Final.jar:4.1.75.Final] at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986) ~[netty-common-4.1.75.Final.jar:4.1.75.Final] at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[netty-common-4.1.75.Final.jar:4.1.75.Final] at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[netty-common-4.1.75.Final.jar:4.1.75.Final] at java.lang.Thread.run(Thread.java:748) ~[?:1.8.0_211]

narras-oss avatar Apr 13 '22 15:04 narras-oss

Hi, i see no activity on this issue. Ideally, the fix would be to treat access token as opaque string (rather than JWT) when JWT signature fails instead of failing the authenticate call entirely. Does this fix sound reasonable?

narras-oss avatar Jul 18 '22 19:07 narras-oss

Can you please provide a simple reproducer?

pendula95 avatar Jul 19 '22 11:07 pendula95

There is a lot of reading to be done and mess they have made with Azure AD.

The idea that OAuth2 spec does not require access tokens to be in any format , they should be treated as opaque string. is correct. But in case the token is in JWT format and it contains a signature then absolutely the signature must be validate. If signature is not valid this token should be rejected.

After reading multiple long threads: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/609 https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/877

There are couple of take always: Tokens you are trying to use are not meant to be used by external apps and is used by Azure AD Graph for internal communication. Token contains a nonce filed in header of JWT which is some ad-hook implementation of Microsoft for added security to it. There is some logic that needs to be applied to the header in order to validate the signature correctly.

Any attempts to avoid signature validation should not be implemented. #609

pendula95 avatar Jan 24 '23 18:01 pendula95

If we at least have an option to treat Access token as opaque in OAuth2Options, that will solve the issue we are facing with Azure Access tokens. We only rely on ID token.

narras-oss avatar Jan 24 '23 23:01 narras-oss

If we at least have an option to treat Access token as opaque in OAuth2Options, that will solve the issue we are facing with Azure Access tokens. We only rely on ID token.

Maybe this will help, https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/609#issuecomment-383877585 together with this https://learn.microsoft.com/en-gb/azure/active-directory/develop/v2-oauth2-client-creds-grant-flow#get-a-token

pendula95 avatar Jan 25 '23 10:01 pendula95