Make X509_store_add_crl throw on unexpected errors (#28966)

* X509_store_add_crl throws on unexpected errors

Fixes #3063 and it is part 2 of #25676: clear error queue on X509_store_add_crl failures.

* PR feedback

Author: Paulo Janotti

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

@@ -18,11 +18,14 @@ internal static partial class Crypto
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrGetError")]
         internal static extern ulong ErrGetError();
 
+        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrGetErrorAlloc")]
+        private static extern ulong ErrGetErrorAlloc([MarshalAs(UnmanagedType.Bool)] out bool isAllocFailure);
+
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrPeekError")]
         internal static extern ulong ErrPeekError();
 
-        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrGetErrorAlloc")]
-        private static extern ulong ErrGetErrorAlloc([MarshalAs(UnmanagedType.Bool)] out bool isAllocFailure);
+        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrPeekLastError")]
+        internal static extern ulong ErrPeekLastError();
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrReasonErrorString")]
         internal static extern IntPtr ErrReasonErrorString(ulong error);

src/Native/Unix/System.Security.Cryptography.Native/pal_err.cpp

@@ -15,10 +15,6 @@ extern "C" uint64_t CryptoNative_ErrGetError()
     return ERR_get_error();
 }
 
-extern "C" uint64_t CryptoNative_ErrPeekError()
-{
-    return ERR_peek_error();
-}
 extern "C" uint64_t CryptoNative_ErrGetErrorAlloc(int32_t* isAllocFailure)
 {
     unsigned long err = ERR_get_error();
@@ -31,6 +27,16 @@ extern "C" uint64_t CryptoNative_ErrGetErrorAlloc(int32_t* isAllocFailure)
     return err;
 }
 
+extern "C" uint64_t CryptoNative_ErrPeekError()
+{
+    return ERR_peek_error();
+}
+
+extern "C" uint64_t CryptoNative_ErrPeekLastError()
+{
+    return ERR_peek_last_error();
+}
+
 extern "C" const char* CryptoNative_ErrReasonErrorString(uint64_t error)
 {
     return ERR_reason_error_string(static_cast<unsigned long>(error));

src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CrlCache.cs

@@ -13,6 +13,8 @@ namespace Internal.Cryptography.Pal
 {
     internal static class CrlCache
     {
+        private const ulong X509_R_CERT_ALREADY_IN_HASH_TABLE = 0x0B07D065;
+
         public static void AddCrlForCertificate(
             X509Certificate2 cert,
             SafeX509StoreHandle store,
@@ -83,9 +85,18 @@ private static bool AddCachedCrl(X509Certificate2 cert, SafeX509StoreHandle stor
                         return false;
                     }
 
-                    // TODO (#3063): Check the return value of X509_STORE_add_crl, and throw on any error other
-                    // than X509_R_CERT_ALREADY_IN_HASH_TABLE
-                    Interop.Crypto.X509StoreAddCrl(store, crl);
+                    if (!Interop.Crypto.X509StoreAddCrl(store, crl))
+                    {
+                        // Ignore error "cert already in store", throw on anything else. In any case the error queue will be cleared.
+                        if (X509_R_CERT_ALREADY_IN_HASH_TABLE == Interop.Crypto.ErrPeekLastError())
+                        {
+                            Interop.Crypto.ErrClearError();
+                        }
+                        else
+                        {
+                            throw Interop.Crypto.CreateOpenSslCryptographicException();
+                        }
+                    }
 
                     return true;
                 }
@@ -111,9 +122,18 @@ private static void DownloadAndAddCrl(
                 // null is a valid return (e.g. no remainingDownloadTime)
                 if (crl != null && !crl.IsInvalid)
                 {
-                    // TODO (#3063): Check the return value of X509_STORE_add_crl, and throw on any error other
-                    // than X509_R_CERT_ALREADY_IN_HASH_TABLE
-                    Interop.Crypto.X509StoreAddCrl(store, crl);
+                    if (!Interop.Crypto.X509StoreAddCrl(store, crl))
+                    {
+                        // Ignore error "cert already in store", throw on anything else. In any case the error queue will be cleared.
+                        if (X509_R_CERT_ALREADY_IN_HASH_TABLE == Interop.Crypto.ErrPeekLastError())
+                        {
+                            Interop.Crypto.ErrClearError();
+                        }
+                        else
+                        {
+                            throw Interop.Crypto.CreateOpenSslCryptographicException();
+                        }
+                    }
 
                     // Saving the CRL to the disk is just a performance optimization for later requests to not
                     // need to use the network again, so failure to save shouldn't throw an exception or mark