improve handling of kSecTrustResultRecoverableTrustFailure (#2135)
authorTomas Weinfurt <tweinfurt@yahoo.com>
Fri, 31 Jan 2020 04:25:49 +0000 (20:25 -0800)
committerGitHub <noreply@github.com>
Fri, 31 Jan 2020 04:25:49 +0000 (20:25 -0800)
* improve handling of kSecTrustResultRecoverableTrustFailure

* feedback from review

src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ssl.c
src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs

index 28f5075..127a846 100644 (file)
@@ -396,9 +396,9 @@ int32_t AppleCryptoNative_SslIsHostnameMatch(SSLContextRef sslContext, CFStringR
         return -6;
     }
 
-    CFIndex count = SecTrustGetCertificateCount(existingTrust);
+    CFIndex certificateCount = SecTrustGetCertificateCount(existingTrust);
 
-    for (CFIndex i = 0; i < count; i++)
+    for (CFIndex i = 0; i < certificateCount; i++)
     {
         SecCertificateRef item = SecTrustGetCertificateAtIndex(existingTrust, i);
         CFArrayAppendValue(certs, item);
@@ -433,6 +433,23 @@ int32_t AppleCryptoNative_SslIsHostnameMatch(SSLContextRef sslContext, CFStringR
 #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)
index 931c665..862c4ad 100644 (file)
@@ -6,7 +6,7 @@ using System.Net.Sockets;
 using System.Net.Test.Common;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading.Tasks;
-
+using Microsoft.DotNet.XUnitExtensions;
 using Xunit;
 
 namespace System.Net.Security.Tests
@@ -36,6 +36,34 @@ namespace System.Net.Security.Tests
             }
         }
 
+        // MacOS has has special validation rules for apple.com and icloud.com
+        [ConditionalTheory]
+        [OuterLoop("Uses external servers")]
+        [InlineData("www.apple.com")]
+        [InlineData("www.icloud.com")]
+        [PlatformSpecific(TestPlatforms.OSX)]
+        public async Task CertificateValidationApple_EndToEnd_Ok(string host)
+        {
+            using (var client = new TcpClient())
+            {
+                try
+                {
+                    await client.ConnectAsync(host, 443);
+                }
+                catch (Exception ex)
+                {
+                    // if we cannot connect skip the test instead of failing.
+                    new SkipTestException($"Unable to connect to '{host}': {ex.Message}");
+                }
+
+
+                using (SslStream sslStream = new SslStream(client.GetStream(), false, RemoteHttpsCertValidation, null))
+                {
+                    await sslStream.AuthenticateAsClientAsync(host);
+                }
+            }
+        }
+
         private bool RemoteHttpsCertValidation(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
         {
             Assert.Equal(SslPolicyErrors.None, sslPolicyErrors);