AzureAD / azure-activedirectory-identitymodel-extensions-for-dotnet

IdentityModel extensions for .Net
MIT License
1.06k stars 400 forks source link

JwtSecurityTokenHandler ReadToken(XmlReader reader) closes the undelying reader making the WCF stack crash with internal exception #150

Closed popcatalin81 closed 8 years ago

popcatalin81 commented 9 years ago

When JwtSecurityTokenHandler is used with WCF & WIF and it receives a BinarySecurityToken containing a JWT Token it crashes the WCF stack because it closes the underlying XmlReader.

The code at fault is the using statement in the ReadToken method:

https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/master/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs#L585

brentschmaltz commented 9 years ago

@popcatalin81 is this the 3.x release?

popcatalin81 commented 9 years ago

@brentschmaltz this is from the System.IdentityModel.Tokens.Jwt Nuget package last version, when used in a WCF scenario.

polita commented 8 years ago

Please close if this is fixed in 4.x

leastprivilege commented 8 years ago

Is it fixed in 4.x?

polita commented 8 years ago

Looks like no. Since @leastprivilege is still interested in seeing it fixed, can you take a look, Yan?

leastprivilege commented 8 years ago

Well - there are def. migration scenarios where legacy WCF services need to consume JWT tokens. I ran into that bug myself and had to wrap the JWT in a SAML token as a workaround.

https://leastprivilege.com/2015/07/02/give-your-wcf-security-architecture-a-makeover-with-identityserver3/

polita commented 8 years ago

@ChenYan98 Please confirm whether this is an issue in 4.x first.

brentschmaltz commented 8 years ago

@polita we are delaying releasing 4.0.3 to fix this. Shouldn't be long. \cc: @leastprivilege @popcatalin81

ChenYan98 commented 8 years ago

@popcatalin81 I cannot repro this in 4.0.3. It didn't crash when I close the XML reader explicitly before passing to the method.

brentschmaltz commented 8 years ago

@popcatalin81 @ChenYan98 what is happening is the WCF sees the message as malformed, since it can't read the expected EndElement.

brentschmaltz commented 8 years ago

@popcatalin81 @leastprivilege we pushed new packages where we don't close the reader. We are hoping WCF scenario should be fixed.

Feeds are on myget: https://www.myget.org/feed/Packages/azureadwebstackrelease

packages: https://www.myget.org/feed/azureadwebstackrelease/package/nuget/Microsoft.IdentityModel.Protocol.Extensions/1.0.3.308261200

https://www.myget.org/feed/azureadwebstackrelease/package/nuget/System.IdentityModel.Tokens.Jwt/4.0.3.308261200

polita commented 8 years ago

@popcatalin81 @leastprivilege Can you try them out and let us know?

ChenYan98 commented 8 years ago

Fixed in 4.0.3