Reducing chances of polluting SSL error queue

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,24 @@ 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);
+
+            if (!ret)
+            {
+                // OpenSSL DSA signature processing requires a DER encode and decode, so
+                // the error queue may have been contaminated.
+                ErrClearError();
+            }
+
+            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

@@ -92,6 +92,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,24 @@ 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)
+            {
+                // OpenSSL ECDSA signature processing requires a DER encode and decode, so
+                // the error queue may have been contaminated.
+                ErrClearError();
+            }
+
+            return ret;
+        }
 
         /*-
          * returns
@@ -31,6 +47,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

@@ -94,6 +94,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));
                 }
 
@@ -144,7 +145,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)
@@ -225,7 +229,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;
                     }
@@ -282,7 +286,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();
                 }
             }
 
@@ -364,6 +368,12 @@ private static void QueryUniqueChannelBinding(SafeSslHandle context, SafeChannel
                 throw CreateSslException(SR.net_ssl_get_channel_binding_token_failed);
             }
 
+            if (!sessionReused)
+            {
+                // The operation may have left errors on the queue
+                Crypto.ErrClearError();
+            }
+
             bindingHandle.SetCertHashLength(certHashLength);
         }
 
@@ -520,7 +530,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);
         }

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

@@ -96,8 +96,24 @@ internal static bool RsaSign(int type, ReadOnlySpan<byte> m, int m_len, Span<byt
         [return: MarshalAs(UnmanagedType.Bool)]
         private static extern bool RsaSign(int type, ref byte m, int m_len, ref byte sigret, out int siglen, SafeRsaHandle rsa);
 
-        internal static bool RsaVerify(int type, ReadOnlySpan<byte> m, int m_len, ReadOnlySpan<byte> sigbuf, int siglen, SafeRsaHandle rsa) =>
-            RsaVerify(type, ref MemoryMarshal.GetReference(m), m_len, ref MemoryMarshal.GetReference(sigbuf), siglen, rsa);
+        internal static bool RsaVerify(int type, ReadOnlySpan<byte> m, ReadOnlySpan<byte> sigbuf, SafeRsaHandle rsa)
+        {
+            bool ret = RsaVerify(
+                type,
+                ref MemoryMarshal.GetReference(m),
+                m.Length,
+                ref MemoryMarshal.GetReference(sigbuf),
+                sigbuf.Length,
+                rsa);
+
+            if (!ret)
+            {
+                ErrClearError();
+            }
+
+            return ret;
+        }
+
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaVerify")]
         [return: MarshalAs(UnmanagedType.Bool)]
@@ -171,7 +187,8 @@ private static extern bool GetRsaParameters(
             out IntPtr iqmp);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetRsaParameters")]
-        internal static extern void SetRsaParameters(
+        [return: MarshalAs(UnmanagedType.Bool)]
+        internal static extern bool SetRsaParameters(
             SafeRsaHandle key,
             byte[] n,
             int nLength,

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

@@ -49,7 +49,8 @@ internal static partial class Ssl
         private static extern IntPtr SslGetVersion(SafeSslHandle ssl);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetTlsExtHostName")]
-        internal static extern int SslSetTlsExtHostName(SafeSslHandle ssl, string host);
+        [return: MarshalAs(UnmanagedType.Bool)]
+        internal static extern bool SslSetTlsExtHostName(SafeSslHandle ssl, string host);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGet0AlpnSelected")]
         internal static extern void SslGetAlpnSelected(SafeSslHandle ssl, out IntPtr protocol, out int len);
@@ -128,6 +129,7 @@ internal static extern bool GetSslConnectionInfo(
         internal static extern int SslGetFinished(SafeSslHandle ssl, IntPtr buf, int count);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionReused")]
+        [return: MarshalAs(UnmanagedType.Bool)]
         internal static extern bool SslSessionReused(SafeSslHandle ssl);
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddExtraChainCert")]
@@ -161,6 +163,7 @@ internal static bool AddExtraChainCertificates(SafeSslHandle sslContext, X509Cha
                 Crypto.CheckValidOpenSslHandle(dupCertHandle);
                 if (!SslAddExtraChainCert(sslContext, dupCertHandle))
                 {
+                    Crypto.ErrClearError();
                     dupCertHandle.Dispose(); // we still own the safe handle; clean it up
                     return false;
                 }

src/Common/src/System/Security/Cryptography/DSAOpenSsl.cs

@@ -282,7 +282,7 @@ public override bool VerifySignature(ReadOnlySpan<byte> hash, ReadOnlySpan<byte>
 
                 byte[] openSslFormat = AsymmetricAlgorithmHelpers.ConvertIeee1363ToDer(signature);
 
-                return Interop.Crypto.DsaVerify(key, hash, hash.Length, openSslFormat, openSslFormat.Length);
+                return Interop.Crypto.DsaVerify(key, hash, openSslFormat);
             }
 
             private void SetKey(SafeDsaHandle newKey)

src/Common/src/System/Security/Cryptography/ECDsaOpenSsl.cs

@@ -149,7 +149,7 @@ public override bool VerifyHash(ReadOnlySpan<byte> hash, ReadOnlySpan<byte> sign
                 byte[] openSslFormat = AsymmetricAlgorithmHelpers.ConvertIeee1363ToDer(signature);
 
                 SafeEcKeyHandle key = _key.Value;
-                int verifyResult = Interop.Crypto.EcDsaVerify(hash, hash.Length, openSslFormat, openSslFormat.Length, key);
+                int verifyResult = Interop.Crypto.EcDsaVerify(hash, openSslFormat, key);
                 return verifyResult == 1;
             }
 

src/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs

@@ -42,6 +42,10 @@ public int ImportParameters(ECParameters parameters)
                 throw Interop.Crypto.CreateOpenSslCryptographicException();
             }
 
+            // The Import* methods above may have polluted the error queue even if in the end they succeeded.
+            // Clean up the error queue.
+            Interop.Crypto.ErrClearError();
+
             FreeKey();
             _key = new Lazy<SafeEcKeyHandle>(key);
             return KeySize;

src/Common/src/System/Security/Cryptography/RSAOpenSsl.cs

@@ -355,7 +355,7 @@ public override void ImportParameters(RSAParameters parameters)
 
             try
             {
-                Interop.Crypto.SetRsaParameters(
+                if (!Interop.Crypto.SetRsaParameters(
                     key,
                     parameters.Modulus,
                     parameters.Modulus != null ? parameters.Modulus.Length : 0,
@@ -372,7 +372,10 @@ public override void ImportParameters(RSAParameters parameters)
                     parameters.DQ, 
                     parameters.DQ != null ? parameters.DQ.Length : 0,
                     parameters.InverseQ,
-                    parameters.InverseQ != null ? parameters.InverseQ.Length : 0);
+                    parameters.InverseQ != null ? parameters.InverseQ.Length : 0))
+                {
+                    throw Interop.Crypto.CreateOpenSslCryptographicException();
+                }
 
                 imported = true;
             }
@@ -694,7 +697,7 @@ public override bool VerifyHash(ReadOnlySpan<byte> hash, ReadOnlySpan<byte> sign
             {
                 int algorithmNid = GetAlgorithmNid(hashAlgorithm);
                 SafeRsaHandle rsa = _key.Value;
-                return Interop.Crypto.RsaVerify(algorithmNid, hash, hash.Length, signature, signature.Length, rsa);
+                return Interop.Crypto.RsaVerify(algorithmNid, hash, signature, rsa);
             }
             else if (padding == RSASignaturePadding.Pss)
             {
@@ -748,6 +751,7 @@ private static int GetAlgorithmNid(HashAlgorithmName hashAlgorithmName)
 
             if (nid == Interop.Crypto.NID_undef)
             {
+                Interop.Crypto.ErrClearError();
                 throw new CryptographicException(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithmName.Name);
             }