Port "Keep SSL error queue clean" to 2.1 (#29438)

* Reducing chances of polluting SSL error queue (#29351)

Author: Paulo Janotti

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ASN1.Print.cs

@@ -26,6 +26,7 @@ internal static string DerStringToManagedString(byte[] derString)
 
             if (asn1String.IsInvalid)
             {
+                Interop.Crypto.ErrClearError();
                 return null;
             }
 
@@ -54,6 +55,8 @@ private static string Asn1StringToManagedString<THandle>(
 
             using (SafeBioHandle bio = CreateMemoryBio())
             {
+                CheckValidOpenSslHandle(bio);
+                
                 int len = asn1StringPrintEx(bio, asn1String, Asn1StringPrintFlags.ASN1_STRFLGS_UTF8_CONVERT);
 
                 if (len < 0)

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ASN1.cs

@@ -20,7 +20,17 @@ internal static partial class Crypto
         private static extern unsafe int ObjObj2Txt(byte* buf, int buf_len, IntPtr a);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetObjectDefinitionByName", CharSet = CharSet.Ansi)]
-        internal static extern IntPtr GetObjectDefinitionByName(string friendlyName);
+        private static extern IntPtr CryptoNative_GetObjectDefinitionByName(string friendlyName);
+        internal static IntPtr GetObjectDefinitionByName(string friendlyName)
+        {
+            IntPtr ret = CryptoNative_GetObjectDefinitionByName(friendlyName);
+            if (ret == IntPtr.Zero)
+            {
+                ErrClearError();
+            }
+
+            return ret;
+        }
 
         // Returns shared pointers, should not be tracked as a SafeHandle.
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ObjNid2Obj")]

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Bignum.cs

@@ -29,7 +29,14 @@ private static IntPtr CreateBignumPtr(byte[] bigEndianValue)
                 return IntPtr.Zero;
             }
 
-            return BigNumFromBinary(bigEndianValue, bigEndianValue.Length);
+            IntPtr ret = BigNumFromBinary(bigEndianValue, bigEndianValue.Length);
+
+            if (ret == IntPtr.Zero)
+            {
+                throw CreateOpenSslCryptographicException();
+            }
+
+            return ret;
         }
 
         internal static SafeBignumHandle CreateBignum(byte[] bigEndianValue)

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Crypto.cs

@@ -16,7 +16,18 @@ internal static partial class Crypto
         private delegate int NegativeSizeReadMethod<in THandle>(THandle handle, byte[] buf, int cBuf);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioTell")]
-        internal static extern int BioTell(SafeBioHandle bio);
+        internal static extern int CryptoNative_BioTell(SafeBioHandle bio);
+
+        internal static int BioTell(SafeBioHandle bio)
+        {
+            int ret = CryptoNative_BioTell(bio);
+            if (ret < 0)
+            {
+                throw CreateOpenSslCryptographicException();
+            }
+
+            return ret;
+        }
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioSeek")]
         internal static extern int BioSeek(SafeBioHandle bio, int pos);

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Dsa.cs

@@ -73,8 +73,19 @@ internal static bool DsaSign(SafeDsaHandle dsa, ReadOnlySpan<byte> hash, int has
         [return: MarshalAs(UnmanagedType.Bool)]
         private static extern bool DsaSign(SafeDsaHandle dsa, ref byte hash, int hashLength, ref byte refSignature, out int outSignatureLength);
 
-        internal static bool DsaVerify(SafeDsaHandle dsa, ReadOnlySpan<byte> hash, int hashLength, ReadOnlySpan<byte> signature, int signatureLength) =>
-            DsaVerify(dsa, ref MemoryMarshal.GetReference(hash), hashLength, ref MemoryMarshal.GetReference(signature), signatureLength);
+        internal static bool DsaVerify(SafeDsaHandle dsa, ReadOnlySpan<byte> hash, ReadOnlySpan<byte> signature)
+        {
+            bool ret = DsaVerify(
+                dsa,
+                ref MemoryMarshal.GetReference(hash),
+                hash.Length,
+                ref MemoryMarshal.GetReference(signature),
+                signature.Length);
+
+            // Error queue already cleaned on the native function.
+
+            return ret;
+        }
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_DsaVerify")]
         [return: MarshalAs(UnmanagedType.Bool)]

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs

@@ -33,8 +33,9 @@ internal static SafeEcKeyHandle EcKeyCreateByKeyParameters(
             int rc = EcKeyCreateByKeyParameters(out key, oid, qx, qxLength, qy, qyLength, d, dLength);
             if (rc == -1)
             {
-                if (key != null)
-                    key.Dispose();
+                key?.Dispose();
+                Interop.Crypto.ErrClearError();
+                
                 throw new PlatformNotSupportedException(string.Format(SR.Cryptography_CurveNotSupported, oid));
             }
             return key;
@@ -92,6 +93,10 @@ internal static SafeEcKeyHandle EcKeyCreateByExplicitCurve(ECCurve curve)
                 throw Interop.Crypto.CreateOpenSslCryptographicException();
             }
 
+            // EcKeyCreateByExplicitParameters may have polluted the error queue, but key was good in the end.
+            // Clean up the error queue.
+            Interop.Crypto.ErrClearError();
+
             return key;
         }
 

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.cs

@@ -17,8 +17,22 @@ internal static bool EcDsaSign(ReadOnlySpan<byte> dgst, int dlen, Span<byte> sig
         [return: MarshalAs(UnmanagedType.Bool)]
         private static extern bool EcDsaSign(ref byte dgst, int dlen, ref byte sig, [In, Out] ref int siglen, SafeEcKeyHandle ecKey);
 
-        internal static unsafe int EcDsaVerify(ReadOnlySpan<byte> dgst, int dgst_len, ReadOnlySpan<byte> sigbuf, int sig_len, SafeEcKeyHandle ecKey) =>
-            EcDsaVerify(ref MemoryMarshal.GetReference(dgst), dgst_len, ref MemoryMarshal.GetReference(sigbuf), sig_len, ecKey);
+        internal static int EcDsaVerify(ReadOnlySpan<byte> dgst, ReadOnlySpan<byte> sigbuf, SafeEcKeyHandle ecKey)
+        {
+            int ret = EcDsaVerify(
+                ref MemoryMarshal.GetReference(dgst),
+                dgst.Length,
+                ref MemoryMarshal.GetReference(sigbuf),
+                sigbuf.Length,
+                ecKey);
+
+            if (ret < 0)
+            {
+                ErrClearError();
+            }
+
+            return ret;
+        }
 
         /*-
          * returns
@@ -31,6 +45,18 @@ internal static unsafe int EcDsaVerify(ReadOnlySpan<byte> dgst, int dgst_len, Re
 
         // returns the maximum length of a DER encoded ECDSA signature created with this key.
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EcDsaSize")]
-        internal static extern int EcDsaSize(SafeEcKeyHandle ecKey);
+        private static extern int CryptoNative_EcDsaSize(SafeEcKeyHandle ecKey);
+
+        internal static int EcDsaSize(SafeEcKeyHandle ecKey)
+        {
+            int ret = CryptoNative_EcDsaSize(ecKey);
+
+            if (ret == 0)
+            {
+                throw CreateOpenSslCryptographicException();
+            }
+
+            return ret;
+        }
     }
 }

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcKey.cs

@@ -12,7 +12,17 @@ internal static partial class Interop
     internal static partial class Crypto
     {
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EcKeyCreateByOid")]
-        internal static extern SafeEcKeyHandle EcKeyCreateByOid(string oid);
+        private static extern SafeEcKeyHandle CryptoNative_EcKeyCreateByOid(string oid);
+        internal static SafeEcKeyHandle EcKeyCreateByOid(string oid)
+        {
+            SafeEcKeyHandle handle = CryptoNative_EcKeyCreateByOid(oid);
+            if (handle == null || handle.IsInvalid)
+            {
+                ErrClearError();
+            }
+
+            return handle;
+        }
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EcKeyDestroy")]
         internal static extern void EcKeyDestroy(IntPtr a);

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Encode.cs

@@ -28,8 +28,18 @@ internal static byte[] OpenSslEncode<THandle>(GetEncodedSizeFunc<THandle> getSiz
             byte[] data = new byte[size];
 
             int size2 = encode(handle, data);
-            Debug.Assert(size == size2);
+            if (size2 < 1)
+            {
+                Debug.Fail(
+                    $"{nameof(OpenSslEncode)}: {nameof(getSize)} succeeded ({size}) and {nameof(encode)} failed ({size2})");
 
+                // If it ever happens, ensure the error queue gets cleared.
+                // And since it didn't write the data, reporting an exception is good too.
+                throw CreateOpenSslCryptographicException();
+            }
+
+            Debug.Assert(size == size2);
+            
             return data;
         }
     }

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs

@@ -79,6 +79,7 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50
 
                 if (!Ssl.SetEncryptionPolicy(innerContext, policy))
                 {
+                    Crypto.ErrClearError();
                     throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, policy));
                 }
 
@@ -129,7 +130,10 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50
                         string punyCode = s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost);
 
                         // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors.
-                        Ssl.SslSetTlsExtHostName(context, punyCode);
+                        if (!Ssl.SslSetTlsExtHostName(context, punyCode))
+                        {
+                            Crypto.ErrClearError();
+                        }
                     }
 
                     if (hasCertificateAndKey)
@@ -210,7 +214,7 @@ internal static bool DoSslHandshake(SafeSslHandle context, byte[] recvBuf, int r
                     if (sendCount <= 0)
                     {
                         // Make sure we clear out the error that is stored in the queue
-                        Crypto.ErrGetError();
+                        Crypto.ErrClearError();
                         sendBuf = null;
                         sendCount = 0;
                     }
@@ -227,6 +231,10 @@ internal static bool DoSslHandshake(SafeSslHandle context, byte[] recvBuf, int r
 
         internal static int Encrypt(SafeSslHandle context, ReadOnlyMemory<byte> input, ref byte[] output, out Ssl.SslErrorCode errorCode)
         {
+#if DEBUG
+            ulong assertNoError = Crypto.ErrPeekError();
+            Debug.Assert(assertNoError == 0, "OpenSsl error queue is not empty, run: 'openssl errstr " + assertNoError.ToString("X") + "' for original error.");
+#endif
             errorCode = Ssl.SslErrorCode.SSL_ERROR_NONE;
 
             int retVal;
@@ -267,7 +275,7 @@ internal static int Encrypt(SafeSslHandle context, ReadOnlyMemory<byte> input, r
                 if (retVal <= 0)
                 {
                     // Make sure we clear out the error that is stored in the queue
-                    Crypto.ErrGetError();
+                    Crypto.ErrClearError();
                 }
             }
 
@@ -276,6 +284,10 @@ internal static int Encrypt(SafeSslHandle context, ReadOnlyMemory<byte> input, r
 
         internal static int Decrypt(SafeSslHandle context, byte[] outBuffer, int offset, int count, out Ssl.SslErrorCode errorCode)
         {
+#if DEBUG
+            ulong assertNoError = Crypto.ErrPeekError();
+            Debug.Assert(assertNoError == 0, "OpenSsl error queue is not empty, run: 'openssl errstr " + assertNoError.ToString("X") + "' for original error.");
+#endif
             errorCode = Ssl.SslErrorCode.SSL_ERROR_NONE;
 
             int retVal = BioWrite(context.InputBio, outBuffer, offset, count);
@@ -505,7 +517,9 @@ private static void SetSslCertificate(SafeSslContextHandle contextPtr, SafeX509H
 
         internal static SslException CreateSslException(string message)
         {
-            ulong errorVal = Crypto.ErrGetError();
+            // Capture last error to be consistent with CreateOpenSslCryptographicException
+            ulong errorVal = Crypto.ErrPeekLastError();
+            Crypto.ErrClearError();
             string msg = SR.Format(message, Marshal.PtrToStringAnsi(Crypto.ErrReasonErrorString(errorVal)));
             return new SslException(msg, (int)errorVal);
         }