Merge pull request #255 from dvsekhvalnov/issue_248-aes-gcm-strict

Issue 248 aes gcm strict
dvsekhvalnov · Nov 6, 2024 · 145c06a · 145c06a
2 parents 33f66b1 + 87a8217
commit 145c06a
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 4 deletions.
diff --git a/README.md b/README.md
@@ -9,7 +9,7 @@ JWE JSON Serialization cross-tested with [JWCrypto](https://github.com/latchset/
 Library is fully FIPS compliant since v2.1
 
 ## Which version?
-- v5.1 support for experimental algorithms RSA-OAEP-384, RSA-OAEP-512
+- v5.1 support for experimental algorithms RSA-OAEP-384, RSA-OAEP-512 and forced strict AES-GCM to avoid trancated tags (see https://github.com/dotnet/runtime/issues/71366)
 
 - v5.0 brings Linux, OSX and FreeBSD compatibility for [ECDH encryption](#ecdh-es-and-ecdh-es-with-aes-key-wrap-key-management-family-of-algorithms) as long as managed `ECDsa` keys support. Fixes cross compatibility issues with encryption over NIST P-384, P-521 curves. And introduces new [security fixes and controls](#customizing-compression).
 

diff --git a/UnitTests/SecurityVulnerabilitiesTest.cs b/UnitTests/SecurityVulnerabilitiesTest.cs
@@ -22,6 +22,8 @@ public class SecurityVulnerabilitiesTest
     {
         private readonly TestConsole Console;
 
+        private static readonly byte[] aes128Key = new byte[] { 194, 164, 235, 6, 138, 248, 171, 239, 24, 216, 11, 22, 137, 199, 215, 133 };
+
         public SecurityVulnerabilitiesTest(ITestOutputHelper output)
         {
             this.Console = new TestConsole(output);
@@ -205,5 +207,25 @@ public void DeflateBomb()
                 Console.Out.WriteLine(e.ToString());
             }
         }
+
+        [Fact]
+        public void TruncatedGcmAuthTag()
+        {
+            // given 
+            string token = "eyJhbGciOiJkaXIiLCJlbmMiOiJBMTI4R0NNIn0..PEXf1goWOF0SZRe_.Zp3CHYq4ZqM3_opMIy25O50gmQzw_p-nCOiW2ROuQSv80-aD-78n8m103kgPRPCsOt7qrckDRGSDACOBZGr2WovzSC-dxIcW3EsPqtibueyh0p3FY43h-bcbhPzXBdjQPaNTCY0o26wcEV_4FzPYdE9_ngRFIUe_7Kby-E2CWYLFc5D9RO9TLGN5dpHL6l4SOGbNz8M0o4aQuyJv3BV1wj_KswqyVcKBHjm0eh6RmFhoERxWjvt5yeo83bzxTfReVWAxXw.AVLr7JE1r1uiUSLj";
+
+            try
+            {
+                // when decrypt token with trunated AES GCM tag, it should fail
+                Jose.JWT.Decode(token, aes128Key);  
+                Assert.True(false, "Should fail with IntegrityException");
+
+            }
+            catch (ArgumentException e)
+            {
+                Console.Out.WriteLine(e.ToString());
+            }
+        }
+
     }
 }
diff --git a/UnitTestsNet40/SecurityVulnerabilitiesTest.cs b/UnitTestsNet40/SecurityVulnerabilitiesTest.cs
@@ -16,6 +16,8 @@ namespace UnitTests
     [TestFixture]
     public class SecurityVulnerabilitiesTest
     {
+        private static readonly byte[] aes128Key = new byte[] { 194, 164, 235, 6, 138, 248, 171, 239, 24, 216, 11, 22, 137, 199, 215, 133 };
+
         [Test]
         public void UnboundedPBKDF2Attack()
         {
@@ -155,6 +157,26 @@ public void DeflateBomb()
             {
                 Console.Out.WriteLine(e.ToString());
             }
+        }
+
+        [Test]
+        public void TruncatedGcmAuthTag()
+        {
+            // given 
+            string token = "eyJhbGciOiJkaXIiLCJlbmMiOiJBMTI4R0NNIn0..PEXf1goWOF0SZRe_.Zp3CHYq4ZqM3_opMIy25O50gmQzw_p-nCOiW2ROuQSv80-aD-78n8m103kgPRPCsOt7qrckDRGSDACOBZGr2WovzSC-dxIcW3EsPqtibueyh0p3FY43h-bcbhPzXBdjQPaNTCY0o26wcEV_4FzPYdE9_ngRFIUe_7Kby-E2CWYLFc5D9RO9TLGN5dpHL6l4SOGbNz8M0o4aQuyJv3BV1wj_KswqyVcKBHjm0eh6RmFhoERxWjvt5yeo83bzxTfReVWAxXw.AVLr7JE1r1uiUSLj";
+
+            try
+            {
+                // when decrypt token with trunated AES GCM tag, it should fail
+                Jose.JWT.Decode(token, aes128Key);
+                Assert.Fail("Should fail with IntegrityException");
+
+            }
+            catch (ArgumentException e)
+            {
+                Console.Out.WriteLine(e.ToString());
+            }
         }
+
     }
 }
diff --git a/UnitTestsNet46/SecurityVulnerabilitiesTest.cs b/UnitTestsNet46/SecurityVulnerabilitiesTest.cs
@@ -238,5 +238,24 @@ public void DeflateBomb()
                 Console.Out.WriteLine(e.ToString());
             }
         }
+
+        [Fact]
+        public void TruncatedGcmAuthTag()
+        {
+            // given 
+            string token = "eyJhbGciOiJkaXIiLCJlbmMiOiJBMTI4R0NNIn0..PEXf1goWOF0SZRe_.Zp3CHYq4ZqM3_opMIy25O50gmQzw_p-nCOiW2ROuQSv80-aD-78n8m103kgPRPCsOt7qrckDRGSDACOBZGr2WovzSC-dxIcW3EsPqtibueyh0p3FY43h-bcbhPzXBdjQPaNTCY0o26wcEV_4FzPYdE9_ngRFIUe_7Kby-E2CWYLFc5D9RO9TLGN5dpHL6l4SOGbNz8M0o4aQuyJv3BV1wj_KswqyVcKBHjm0eh6RmFhoERxWjvt5yeo83bzxTfReVWAxXw.AVLr7JE1r1uiUSLj";
+
+            try
+            {
+                // when decrypt token with trunated AES GCM tag, it should fail
+                Jose.JWT.Decode(token, aes128Key);  
+                Assert.True(false, "Should fail with IntegrityException");
+
+            }
+            catch (ArgumentException e)
+            {
+                Console.Out.WriteLine(e.ToString());
+            }
+        }
     }
 }
diff --git a/jose-jwt/crypto/AesGcm.cs b/jose-jwt/crypto/AesGcm.cs
@@ -8,7 +8,7 @@
 namespace Jose
 {
     public static class AesGcm
-    {
+    {       
         /// <summary>
         /// Performs AES encryption in GCM chaining mode over plain text
         /// </summary>
@@ -71,11 +71,14 @@ public static byte[] Decrypt(byte[] key, byte[] iv, byte[] aad, byte[] cipherTex
             IntPtr hKey, keyDataBuffer = ImportKey(hAlg, key, out hKey);
 
             byte[] plainText;
+            int expectedTagSize = MaxAuthTagSize(hAlg);
+
+            Ensure.ByteSize(authTag, expectedTagSize, "Expected auth tag of length: {0} bytes, but got: {1} bytes", expectedTagSize, authTag.Length);
 
             var authInfo = new BCrypt.BCRYPT_AUTHENTICATED_CIPHER_MODE_INFO(iv, aad, authTag);
             using (authInfo)
             {
-                byte[] ivData = new byte[MaxAuthTagSize(hAlg)];
+                byte[] ivData = new byte[expectedTagSize];
 
                 int plainTextSize = 0;
 

diff --git a/jose-jwt/crypto/AesGcmNetCore.cs b/jose-jwt/crypto/AesGcmNetCore.cs
@@ -37,7 +37,9 @@ public static byte[][] Encrypt(byte[] key, byte[] iv, byte[] aad, byte[] plainTe
         /// <exception cref="CryptographicException">if decryption failed by any reason</exception>
         public static byte[] Decrypt(byte[] key, byte[] iv, byte[] aad, byte[] cipherText, byte[] authTag)
         {
-            using var gcm = new System.Security.Cryptography.AesGcm(key);
+            Ensure.ByteSize(authTag, System.Security.Cryptography.AesGcm.TagByteSizes.MaxSize, "Expected auth tag of length: {0} bytes, but got: {1} bytes", System.Security.Cryptography.AesGcm.TagByteSizes.MaxSize, authTag.Length);
+
+	    using var gcm = new System.Security.Cryptography.AesGcm(key);
 
             var plaintext = new byte[cipherText.Length];
 

diff --git a/jose-jwt/util/Ensure.cs b/jose-jwt/util/Ensure.cs
@@ -31,6 +31,12 @@ public static void BitSize(byte[] array, long expectedSize, string msg, params o
             if(expectedSize != array.Length * 8L)
                 throw new ArgumentException(string.Format(msg,args));
         }
+
+        public static void ByteSize(byte[] array, long expectedSize, string msg, params object[] args)
+        {
+            if(expectedSize != array.Length)
+                throw new ArgumentException(string.Format(msg,args));
+        }
 
         public static void BitSize(int actualSize, int expectedSize, string msg)
         {