fix decoding of ecdsa signature (#1257)

- incorrect decoding caused flaky builds, verification error only if keys have trailing zeros
OPCFoundation · Jan 27, 2021 · 29fc3f2 · 29fc3f2
1 parent 4a14df9
commit 29fc3f2
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 10 deletions.
diff --git a/Libraries/Opc.Ua.Security.Certificates/X509Crl/X509Signature.cs b/Libraries/Opc.Ua.Security.Certificates/X509Crl/X509Signature.cs
@@ -208,7 +208,7 @@ private bool VerifyForECDsa(X509Certificate2 certificate)
         {
             using (ECDsa key = certificate.GetECDsaPublicKey())
             {
-                var decodedSignature = DecodeECDsa(Signature);
+                var decodedSignature = DecodeECDsa(Signature, key.KeySize);
                 return key.VerifyData(Tbs, decodedSignature, Name);
             }
         }
@@ -258,25 +258,29 @@ private static byte[] EncodeECDsa(byte[] signature)
         /// Decode a ECDSA signature from ASN.1.
         /// </summary>
         /// <param name="signature">The signature to decode from ASN.1</param>
-        private static byte[] DecodeECDsa(ReadOnlyMemory<byte> signature)
+        /// <param name="keySize">The keySize in bits.</param>
+        private static byte[] DecodeECDsa(ReadOnlyMemory<byte> signature, int keySize)
         {
             AsnReader reader = new AsnReader(signature, AsnEncodingRules.DER);
             var seqReader = reader.ReadSequence();
             reader.ThrowIfNotEmpty();
             var r = seqReader.ReadIntegerBytes();
             var s = seqReader.ReadIntegerBytes();
             seqReader.ThrowIfNotEmpty();
-            if (r.Span[0] == 0)
+            keySize >>= 3;
+            if (r.Span[0] == 0 && r.Length > keySize)
             {
                 r = r.Slice(1);
             }
-            if (s.Span[0] == 0)
+            if (s.Span[0] == 0 && s.Length > keySize)
             {
                 s = s.Slice(1);
             }
-            var result = new byte[r.Length + s.Length];
-            r.CopyTo(new Memory<byte>(result, 0, r.Length));
-            s.CopyTo(new Memory<byte>(result, r.Length, s.Length));
+            var result = new byte[2 * keySize];
+            int offset = keySize - r.Length;
+            r.CopyTo(new Memory<byte>(result, offset, r.Length));
+            offset = 2 * keySize - s.Length;
+            s.CopyTo(new Memory<byte>(result, offset, s.Length));
             return result;
         }
     }

diff --git a/Tests/Opc.Ua.Security.Certificates.Tests/CertificateTestsForECDsa.cs b/Tests/Opc.Ua.Security.Certificates.Tests/CertificateTestsForECDsa.cs
@@ -112,7 +112,7 @@ public void VerifyOneSelfSignedAppCertForAll()
         /// <summary>
         /// Create the default RSA certificate.
         /// </summary>
-        [Theory]
+        [Theory, Repeat(10)]
         public void CreateSelfSignedForECDsaDefaultTest(ECCurveHashPair eccurveHashPair)
         {
             // default cert
@@ -145,7 +145,7 @@ public void CreateSelfSignedForECDsaDefaultTest(ECCurveHashPair eccurveHashPair)
             Assert.True(X509Utils.VerifySelfSigned(cert), "Verify self signed.");
         }
 
-        [Theory]
+        [Theory, Repeat(10)]
         public void CreateSelfSignedForECDsaAllFields(
             ECCurveHashPair ecCurveHashPair
             )
@@ -182,7 +182,7 @@ ECCurveHashPair ecCurveHashPair
             Assert.True(X509Utils.VerifySelfSigned(cert));
         }
 
-        [Theory]
+        [Theory, Repeat(10)]
         public void CreateCACertForECDsa(
             ECCurveHashPair ecCurveHashPair
             )