From 5b103338f5b5c7a8ab9a2e789abbc15467b66993 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Thu, 30 Jan 2020 20:25:49 -0800 Subject: [PATCH] improve handling of kSecTrustResultRecoverableTrustFailure (#2135) * improve handling of kSecTrustResultRecoverableTrustFailure * feedback from review --- .../pal_ssl.c | 21 +++++++++++++-- .../CertificateValidationRemoteServer.cs | 30 +++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ssl.c index 28f5075..127a846 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ssl.c @@ -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) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs index 931c665..862c4ad 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs @@ -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); -- 2.7.4