Unify certificate validation code on OSX between SslStream and QuicConnection (#117611)

* Unify certificate validation code on OSX between SslStream and QuicConnection

* Fix build

* Remove spaces in test case

Author: rzikm

src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs

@@ -16,8 +16,6 @@ internal static partial class Interop
 {
     internal static partial class AppleCrypto
     {
-        private static readonly IdnMapping s_idnMapping = new IdnMapping();
-
         // Read data from connection (or an instance delegate captured context) and write it to data
         // dataLength comes in as the capacity of data, goes out as bytes written.
         // Note: the true type of dataLength is `size_t*`, but on macOS that's most equal to `void**`
@@ -152,13 +150,6 @@ internal static unsafe partial int SslSetIoCallbacks(
         [LibraryImport(Interop.Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_SslRead")]
         internal static unsafe partial PAL_TlsIo SslRead(SafeSslHandle sslHandle, byte* writeFrom, int count, out int bytesWritten);
 
-        [LibraryImport(Interop.Libraries.AppleCryptoNative)]
-        private static partial int AppleCryptoNative_SslIsHostnameMatch(
-            SafeSslHandle handle,
-            SafeCreateHandle cfHostname,
-            SafeCFDateHandle cfValidTime,
-            out int pOSStatus);
-
         [LibraryImport(Interop.Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_SslShutdown")]
         internal static partial int SslShutdown(SafeSslHandle sslHandle);
 
@@ -462,40 +453,6 @@ internal static unsafe int SslCtxSetAlpnProtocol(SafeSslHandle ctx, SslApplicati
                 protocol.Dispose();
             }
         }
-
-        public static bool SslCheckHostnameMatch(SafeSslHandle handle, string hostName, DateTime notBefore, out int osStatus)
-        {
-            int result;
-            // The IdnMapping converts Unicode input into the IDNA punycode sequence.
-            // It also does host case normalization.  The bypass logic would be something
-            // like "all characters being within [a-z0-9.-]+"
-            //
-            // The SSL Policy (SecPolicyCreateSSL) has been verified as not inherently supporting
-            // IDNA as of macOS 10.12.1 (Sierra).  If it supports low-level IDNA at a later date,
-            // this code could be removed.
-            //
-            // It was verified as supporting case invariant match as of 10.12.1 (Sierra).
-            string matchName = string.IsNullOrEmpty(hostName) ? string.Empty : s_idnMapping.GetAscii(hostName);
-
-            using (SafeCFDateHandle cfNotBefore = CoreFoundation.CFDateCreate(notBefore))
-            using (SafeCreateHandle cfHostname = CoreFoundation.CFStringCreateWithCString(matchName))
-            {
-                result = AppleCryptoNative_SslIsHostnameMatch(handle, cfHostname, cfNotBefore, out osStatus);
-            }
-
-            switch (result)
-            {
-                case 0:
-                    return false;
-                case 1:
-                    return true;
-                default:
-                    if (NetEventSource.Log.IsEnabled())
-                        NetEventSource.Error(null, $"AppleCryptoNative_SslIsHostnameMatch returned '{result}' for '{hostName}'");
-                    Debug.Fail($"AppleCryptoNative_SslIsHostnameMatch returned {result}");
-                    throw new SslException();
-            }
-        }
     }
 }
 

src/libraries/System.Net.Security/src/System.Net.Security.csproj

@@ -442,6 +442,8 @@
              Link="Common\Interop\OSX\System.Security.Cryptography.Native.Apple\Interop.X509Chain.cs" />
     <Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeCreateHandle.OSX.cs"
              Link="Common\Microsoft\Win32\SafeHandles\SafeCreateHandle.OSX.cs" />
+    <Compile Include="$(CommonPath)System\Net\Security\CertificateValidation.OSX.cs"
+             Link="Common\System\Net\Security\CertificateValidation.OSX.cs" />
     <Compile Include="System\Net\CertificateValidationPal.OSX.cs" />
     <Compile Include="System\Net\Security\Pal.Managed\SslProtocolsValidation.cs" />
     <Compile Include="System\Net\Security\Pal.OSX\SafeDeleteSslContext.cs" />

src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs

@@ -10,41 +10,14 @@ namespace System.Net
     internal static partial class CertificateValidationPal
     {
         internal static SslPolicyErrors VerifyCertificateProperties(
-            SafeDeleteContext securityContext,
+            SafeDeleteContext? _ /*securityContext*/,
             X509Chain chain,
-            X509Certificate2? remoteCertificate,
+            X509Certificate2 remoteCertificate,
             bool checkCertName,
             bool isServer,
             string? hostName)
         {
-            SslPolicyErrors errors = SslPolicyErrors.None;
-
-            if (remoteCertificate == null)
-            {
-                errors |= SslPolicyErrors.RemoteCertificateNotAvailable;
-            }
-            else
-            {
-                if (!chain.Build(remoteCertificate))
-                {
-                    errors |= SslPolicyErrors.RemoteCertificateChainErrors;
-                }
-
-                if (!isServer && checkCertName)
-                {
-                    SafeDeleteSslContext sslContext = (SafeDeleteSslContext)securityContext;
-
-                    if (!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext, hostName!, remoteCertificate.NotBefore, out int osStatus))
-                    {
-                        errors |= SslPolicyErrors.RemoteCertificateNameMismatch;
-
-                        if (NetEventSource.Log.IsEnabled())
-                            NetEventSource.Error(sslContext, $"Cert name validation for '{hostName}' failed with status '{osStatus}'");
-                    }
-                }
-            }
-
-            return errors;
+            return CertificateValidation.BuildChainAndVerifyProperties(chain, remoteCertificate, checkCertName, isServer, hostName, Span<byte>.Empty);
         }
 
         private static X509Certificate2? GetRemoteCertificate(

src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamSniTest.cs

@@ -378,7 +378,7 @@ public static IEnumerable<object[]> HostNameData()
             yield return new object[] { "test" };
             // max allowed hostname length is 63
             yield return new object[] { new string('a', 63) };
-            yield return new object[] { "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D" };
+            yield return new object[] { "\u017C\u00F3\u0142\u0107g\u0119\u015Bl\u0105ja\u017A\u0144.\u7EA2\u70E7.\u7167\u308A\u713C\u304D" };
         }
     }
 }

src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c

@@ -62,7 +62,6 @@ static const Entry s_cryptoAppleNative[] =
     DllImportEntry(AppleCryptoNative_SetKeychainNeverLock)
     DllImportEntry(AppleCryptoNative_SslCopyCADistinguishedNames)
     DllImportEntry(AppleCryptoNative_SslCopyCertChain)
-    DllImportEntry(AppleCryptoNative_SslIsHostnameMatch)
     DllImportEntry(AppleCryptoNative_SslRead)
     DllImportEntry(AppleCryptoNative_SslSetBreakOnCertRequested)
     DllImportEntry(AppleCryptoNative_SslSetBreakOnClientAuth)

src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c

@@ -416,181 +416,6 @@ PAL_TlsIo AppleCryptoNative_SslRead(SSLContextRef sslContext, uint8_t* buf, uint
     return OSStatusToPAL_TlsIo(status);
 }
 
-int32_t AppleCryptoNative_SslIsHostnameMatch(SSLContextRef sslContext, CFStringRef cfHostname, CFDateRef notBefore, int32_t* pOSStatus)
-{
-    if (pOSStatus != NULL)
-        *pOSStatus = noErr;
-
-    if (sslContext == NULL || notBefore == NULL || pOSStatus == NULL)
-        return -1;
-
-    if (cfHostname == NULL)
-        return -2;
-
-    SecPolicyRef sslPolicy = SecPolicyCreateSSL(true, cfHostname);
-
-    if (sslPolicy == NULL)
-        return -3;
-
-    CFMutableArrayRef certs = CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks);
-
-    if (certs == NULL)
-        return -4;
-
-    SecTrustRef existingTrust = NULL;
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    OSStatus osStatus = SSLCopyPeerTrust(sslContext, &existingTrust);
-#pragma clang diagnostic pop
-
-    if (osStatus != noErr)
-    {
-        CFRelease(certs);
-        *pOSStatus = osStatus;
-        return -5;
-    }
-
-    CFMutableArrayRef anchors = CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks);
-
-    if (anchors == NULL)
-    {
-        CFRelease(certs);
-        CFRelease(existingTrust);
-        return -6;
-    }
-
-    CFIndex certificateCount = SecTrustGetCertificateCount(existingTrust);
-
-    for (CFIndex i = 0; i < certificateCount; i++)
-    {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-        SecCertificateRef item = SecTrustGetCertificateAtIndex(existingTrust, i);
-#pragma clang diagnostic pop
-
-        CFArrayAppendValue(certs, item);
-
-        // Copy the EE cert into the anchors set, this will make the chain part
-        // always return true.
-        if (i == 0)
-        {
-            CFArrayAppendValue(anchors, item);
-        }
-    }
-
-    SecTrustRef trust = NULL;
-    osStatus = SecTrustCreateWithCertificates(certs, sslPolicy, &trust);
-    int32_t ret = INT_MIN;
-
-    if (osStatus == noErr)
-    {
-        osStatus = SecTrustSetAnchorCertificates(trust, anchors);
-    }
-
-    if (osStatus == noErr)
-    {
-        osStatus = SecTrustSetVerifyDate(trust, notBefore);
-    }
-
-    if (osStatus == noErr)
-    {
-        SecTrustResultType trustResult;
-        memset(&trustResult, 0, sizeof(SecTrustResultType));
-
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-        osStatus = SecTrustEvaluate(trust, &trustResult);
-
-        if (trustResult == kSecTrustResultRecoverableTrustFailure && osStatus == noErr && certificateCount > 1)
-        {
-            // If we get recoverable failure, let's try it again with full anchor list.
-            // We already stored just the first certificate into anchors; now we store the rest.
-            for (CFIndex i = 1; i < certificateCount; i++)
-            {
-                CFArrayAppendValue(anchors, SecTrustGetCertificateAtIndex(existingTrust, i));
-            }
-
-            osStatus = SecTrustSetAnchorCertificates(trust, anchors);
-            if (osStatus == noErr)
-            {
-                memset(&trustResult, 0, sizeof(SecTrustResultType));
-                osStatus = SecTrustEvaluate(trust, &trustResult);
-            }
-        }
-#pragma clang diagnostic pop
-
-        if (osStatus == noErr && trustResult != kSecTrustResultUnspecified && trustResult != kSecTrustResultProceed)
-        {
-            // If evaluation succeeded but result is not trusted try to get details.
-            CFDictionaryRef detailsAndStuff = SecTrustCopyResult(trust);
-
-            if (detailsAndStuff != NULL)
-            {
-                CFArrayRef details = CFDictionaryGetValue(detailsAndStuff, CFSTR("TrustResultDetails"));
-
-                if (details != NULL && CFArrayGetCount(details) > 0)
-                {
-                    CFArrayRef statusCodes = CFDictionaryGetValue(CFArrayGetValueAtIndex(details,0), CFSTR("StatusCodes"));
-
-                    if (statusCodes != NULL)
-                    {
-                        OSStatus status = 0;
-                        // look for first failure to keep it simple. Normally, there will be exactly one.
-                        for (int i = 0; i < CFArrayGetCount(statusCodes); i++)
-                        {
-                            CFNumberGetValue(CFArrayGetValueAtIndex(statusCodes, i), kCFNumberSInt32Type, &status);
-                            if (status != noErr)
-                            {
-                                *pOSStatus = status;
-                                break;
-                            }
-                        }
-                    }
-                }
-
-                CFRelease(detailsAndStuff);
-            }
-        }
-
-        if (osStatus != noErr)
-        {
-            ret = -7;
-            *pOSStatus = osStatus;
-        }
-        else if (trustResult == kSecTrustResultUnspecified || trustResult == kSecTrustResultProceed)
-        {
-            ret = 1;
-        }
-        else if (trustResult == kSecTrustResultDeny || trustResult == kSecTrustResultRecoverableTrustFailure)
-        {
-            ret = 0;
-        }
-        else
-        {
-            ret = -8;
-        }
-    }
-    else
-    {
-        *pOSStatus = osStatus;
-    }
-
-    if (trust != NULL)
-        CFRelease(trust);
-
-    if (certs != NULL)
-        CFRelease(certs);
-
-    if (anchors != NULL)
-        CFRelease(anchors);
-
-    if (existingTrust != NULL)
-        CFRelease(existingTrust);
-
-    CFRelease(sslPolicy);
-    return ret;
-}
-
 int32_t AppleCryptoNative_SslShutdown(SSLContextRef sslContext)
 {
 #pragma clang diagnostic push

src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.h

@@ -226,17 +226,6 @@ written: Receives the number of bytes written into buf
 PALEXPORT PAL_TlsIo
 AppleCryptoNative_SslRead(SSLContextRef sslContext, uint8_t* buf, uint32_t bufLen, uint32_t* written);
 
-/*
-Check to see if the server identity certificate for this connection matches the requested hostname.
-
-notBefore: Specify the EE/leaf certificate's notBefore value to prevent a false negative due to
-the certificate being expired (or not yet valid).
-
-Returns 1 on match, 0 on mismatch, any other value indicates an invalid state.
-*/
-PALEXPORT int32_t
-AppleCryptoNative_SslIsHostnameMatch(SSLContextRef sslContext, CFStringRef cfHostname, CFDateRef notBefore, int32_t* pOSStatus);
-
 /*
 Generate a TLS Close alert to terminate the session.