search

dvsekhvalnov / jose-jwt

Ultimate Javascript Object Signing and Encryption (JOSE), JSON Web Token (JWT) and Json Web Keys (JWK) Implementation for .NET and .NET Core
MIT License
921 stars 183 forks source link

Add .NET 8.0 TFM and use new AesGcm constructor #248

Open zzyzy opened 2 weeks ago

zzyzy commented 2 weeks ago

A penetration testing reported indicated that decrypting JWE with truncated authentication tag still works without throwing exception.

After researching further, found that the issue is due to how .NET AesGcm class works (from System.Security.Cryptography namespace) prior to .NET 8.0. (see https://github.com/dotnet/runtime/issues/71366).

To remediate the findings:

  1. Add .NET 8.0 TFM
  2. Update AesGcmNetCore.cs to use new System.Security.Cryptography.AesGcm constructor by specifying the tag size
  3. Add unit test to validate the new changes

Example JWE: eyJhbGciOiJFQ0RILUVTK0EyNTZLVyIsImVwayI6eyJrdHkiOiJFQyIsIngiOiJ4ZUNycEg3MS1RbnBpT0htVVVEaWh1bGRyN3BqSGtPVVRuUXZTUzRpUEc4IiwieSI6InYtbHVhNldrS3ZFYThzQm4xRTFHNjdfRUZEaTRHVkZNWE13YjZZdXl4LWMiLCJjcnYiOiJQLTI1NiJ9LCJlbmMiOiJBMjU2R0NNIn0.V1Fv0OIeukGSMYIwFwfdnqllTzW-e-KgXTj0w0MYmbr14PLx8NhSvg.4YK1emgVFKcdsS55.2ol-hLJuZNSSgyb3JuyFaGKAjJfHw4fGit8gkqFVQCVM1T8EhsEwezH9MtL6BwKUJCbrEglNeloMG2ScYT_GeTUI_-8mh_s6rdv8gioSRoBpe-PGkXax0FaMskGasMWR3tFWJ9rZ4KzZ_Wi5BvpVsIe5fEoWUluW3QbGn5IudNb26iaCXestMURCo_nxLs52quKqN_Xz_U5PDRMGF1Zcjmpmk-uIRBILan11oNC39rDzqxwZ9OBB16KuXXjRgDYzfczg1I6B1-FwNwdeLOViQWSH2i682AZU2rsqq3RokwNlmZ4_kMeecXnLuKJiVFAX-M1-dWfEpv7r9w_y0RXBwdssFJ1pAyxG_2zbgaWEWiThkNQDPggDxQUub_sa1_sZimIVaWTVtYuJVEOvtsAh61crcSxvfVpieOt6CJGr4Bp5sBpyvkCvcERVkdhiS4f0fwMIRPE32kC7GXo2xc38SPHyDeAd4RCfOvqBJ1WLM0kwb17rODJUV4Ycb5TkAYcb3NGJmwGFQo_XpzKhHCwYPnBWXInyIl4MjiCZ062brYRho20EuTDzaBKQPhsuXavOsRSxrBCS9qOMvO9E.dyLFkX5Rb-cGg3STAC-7Bg

Truncated JWE: eyJhbGciOiJFQ0RILUVTK0EyNTZLVyIsImVwayI6eyJrdHkiOiJFQyIsIngiOiJ4ZUNycEg3MS1RbnBpT0htVVVEaWh1bGRyN3BqSGtPVVRuUXZTUzRpUEc4IiwieSI6InYtbHVhNldrS3ZFYThzQm4xRTFHNjdfRUZEaTRHVkZNWE13YjZZdXl4LWMiLCJjcnYiOiJQLTI1NiJ9LCJlbmMiOiJBMjU2R0NNIn0.V1Fv0OIeukGSMYIwFwfdnqllTzW-e-KgXTj0w0MYmbr14PLx8NhSvg.4YK1emgVFKcdsS55.2ol-hLJuZNSSgyb3JuyFaGKAjJfHw4fGit8gkqFVQCVM1T8EhsEwezH9MtL6BwKUJCbrEglNeloMG2ScYT_GeTUI_-8mh_s6rdv8gioSRoBpe-PGkXax0FaMskGasMWR3tFWJ9rZ4KzZ_Wi5BvpVsIe5fEoWUluW3QbGn5IudNb26iaCXestMURCo_nxLs52quKqN_Xz_U5PDRMGF1Zcjmpmk-uIRBILan11oNC39rDzqxwZ9OBB16KuXXjRgDYzfczg1I6B1-FwNwdeLOViQWSH2i682AZU2rsqq3RokwNlmZ4_kMeecXnLuKJiVFAX-M1-dWfEpv7r9w_y0RXBwdssFJ1pAyxG_2zbgaWEWiThkNQDPggDxQUub_sa1_sZimIVaWTVtYuJVEOvtsAh61crcSxvfVpieOt6CJGr4Bp5sBpyvkCvcERVkdhiS4f0fwMIRPE32kC7GXo2xc38SPHyDeAd4RCfOvqBJ1WLM0kwb17rODJUV4Ycb5TkAYcb3NGJmwGFQo_XpzKhHCwYPnBWXInyIl4MjiCZ062brYRho20EuTDzaBKQPhsuXavOsRSxrBCS9qOMvO9E.dyLFkX5Rb-cGg3STAC-7 (removed Bg)

Both of the above JWE will be decrypted successfully. The second JWE should throw exception.

dvsekhvalnov commented 14 hours ago

Hi @zzyzy ,

thanks for reporting, interesting one. Give me sometime to to review PR and vulnerability in general.

dvsekhvalnov commented 14 hours ago

I see there is wrapper proposed in a thread you linked: https://github.com/sdrapkin/SecurityDriven.AesGcmStrict that just checks byte length of tag.

Sounds like can be better solution to replicate it in code for all supported .net versions then having fix for .net 8+ and leave others open to potential vulnerability?