Do not OCSP staple invalid OCSP responses
authorKevin Jones <kevin@vcsjones.com>
Mon, 14 Aug 2023 22:00:32 +0000 (18:00 -0400)
committerGitHub <noreply@github.com>
Mon, 14 Aug 2023 22:00:32 +0000 (18:00 -0400)
src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/RevocationResponder.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs [new file with mode: 0644]
src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/AiaTests.cs

index b08655e..afed3b9 100644 (file)
@@ -16,6 +16,9 @@ namespace System.Security.Cryptography.X509Certificates.Tests.Common
         private static readonly bool s_traceEnabled =
             Environment.GetEnvironmentVariable("TRACE_REVOCATION_RESPONSE") != null;
 
+        private static readonly byte[] s_invalidResponse =
+            "<html><marquee>The server is down for maintenence.</marquee></html>"u8.ToArray();
+
         private readonly HttpListener _listener;
 
         private readonly Dictionary<string, CertificateAuthority> _aiaPaths =
@@ -29,7 +32,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests.Common
 
         public string UriPrefix { get; }
 
-        public bool RespondEmpty { get; set; }
+        public RespondKind RespondKind { get; set; }
         public AiaResponseKind AiaResponseKind { get; set; }
 
         public TimeSpan ResponseDelay { get; set; }
@@ -183,7 +186,12 @@ namespace System.Security.Cryptography.X509Certificates.Tests.Common
                     Thread.Sleep(ResponseDelay);
                 }
 
-                byte[] certData = RespondEmpty ? Array.Empty<byte>() : GetCertDataForAiaResponseKind(AiaResponseKind, authority);
+                byte[] certData = RespondKind switch
+                {
+                    RespondKind.Empty => Array.Empty<byte>(),
+                    RespondKind.Invalid => s_invalidResponse,
+                    _ => GetCertDataForAiaResponseKind(AiaResponseKind, authority),
+                };
 
                 responded = true;
                 context.Response.StatusCode = 200;
@@ -201,7 +209,12 @@ namespace System.Security.Cryptography.X509Certificates.Tests.Common
                     Thread.Sleep(ResponseDelay);
                 }
 
-                byte[] crl = RespondEmpty ? Array.Empty<byte>() : authority.GetCrl();
+                byte[] crl = RespondKind switch
+                {
+                    RespondKind.Empty => Array.Empty<byte>(),
+                    RespondKind.Invalid => s_invalidResponse,
+                    _ => authority.GetCrl(),
+                };
 
                 responded = true;
                 context.Response.StatusCode = 200;
@@ -236,7 +249,12 @@ namespace System.Security.Cryptography.X509Certificates.Tests.Common
                             return;
                         }
 
-                        byte[] ocspResponse = RespondEmpty ? Array.Empty<byte>() : authority.BuildOcspResponse(certId, nonce);
+                        byte[] ocspResponse = RespondKind switch
+                        {
+                            RespondKind.Empty => Array.Empty<byte>(),
+                            RespondKind.Invalid => s_invalidResponse,
+                            _ => authority.BuildOcspResponse(certId, nonce),
+                        };
 
                         if (DelayedActions.HasFlag(DelayedActionsFlag.Ocsp))
                         {
@@ -468,4 +486,11 @@ namespace System.Security.Cryptography.X509Certificates.Tests.Common
         Cert = 0,
         Pkcs12 = 1,
     }
+
+    public enum RespondKind
+    {
+        Normal = 0,
+        Empty = 1,
+        Invalid = 2,
+    }
 }
index 98380b0..cc80cc6 100644 (file)
@@ -229,6 +229,7 @@ namespace System.Net.Security
                     {
                         if (!Interop.Crypto.X509DecodeOcspToExpiration(ret, ocspRequest, subject, issuer, out DateTimeOffset expiration))
                         {
+                            ret = null;
                             continue;
                         }
 
diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs
new file mode 100644 (file)
index 0000000..a3fd09c
--- /dev/null
@@ -0,0 +1,64 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Reflection;
+using System.Security.Cryptography.X509Certificates;
+using System.Security.Cryptography.X509Certificates.Tests.Common;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace System.Net.Security.Tests
+{
+    public static class SslStreamCertificateContextTests
+    {
+        [Fact]
+        [OuterLoop("Subject to resource contention and load.")]
+        [PlatformSpecific(TestPlatforms.Linux)]
+        public static async Task Create_OcspDoesNotReturnOrCacheInvalidStapleData()
+        {
+            string serverName = $"{nameof(Create_OcspDoesNotReturnOrCacheInvalidStapleData)}.example";
+
+            CertificateAuthority.BuildPrivatePki(
+                PkiOptions.EndEntityRevocationViaOcsp | PkiOptions.CrlEverywhere,
+                out RevocationResponder responder,
+                out CertificateAuthority rootAuthority,
+                out CertificateAuthority[] intermediateAuthorities,
+                out X509Certificate2 serverCert,
+                intermediateAuthorityCount: 1,
+                subjectName: serverName,
+                keySize: 2048,
+                extensions: TestHelper.BuildTlsServerCertExtensions(serverName));
+
+            using (responder)
+            using (rootAuthority)
+            using (CertificateAuthority intermediateAuthority = intermediateAuthorities[0])
+            using (serverCert)
+            using (X509Certificate2 rootCert = rootAuthority.CloneIssuerCert())
+            using (X509Certificate2 issuerCert = intermediateAuthority.CloneIssuerCert())
+            {
+                responder.RespondKind = RespondKind.Invalid;
+
+                SslStreamCertificateContext context = SslStreamCertificateContext.Create(
+                    serverCert,
+                    additionalCertificates: new X509Certificate2Collection { issuerCert },
+                    offline: false);
+
+                MethodInfo fetchOcspAsyncMethod = typeof(SslStreamCertificateContext).GetMethod(
+                    "DownloadOcspAsync",
+                    BindingFlags.Instance | BindingFlags.NonPublic);
+                FieldInfo ocspResponseField = typeof(SslStreamCertificateContext).GetField(
+                    "_ocspResponse",
+                    BindingFlags.Instance | BindingFlags.NonPublic);
+
+                Assert.NotNull(fetchOcspAsyncMethod);
+                Assert.NotNull(ocspResponseField);
+
+                byte[] ocspFetch = await (ValueTask<byte[]>)fetchOcspAsyncMethod.Invoke(context, Array.Empty<object>());
+                Assert.Null(ocspFetch);
+
+                byte[] ocspResponseValue = (byte[])ocspResponseField.GetValue(context);
+                Assert.Null(ocspResponseValue);
+            }
+        }
+    }
+}
index b72ebc8..341ba3b 100644 (file)
@@ -29,6 +29,7 @@
     <Compile Include="ServerAsyncAuthenticateTest.cs" />
     <Compile Include="ServerNoEncryptionTest.cs" />
     <Compile Include="ServerRequireEncryptionTest.cs" />
+    <Compile Include="SslStreamCertificateContextTests.cs" />
     <Compile Include="SslStreamConformanceTests.cs" />
     <Compile Include="SslStreamStreamToStreamTest.cs" />
     <Compile Include="SslStreamNetworkStreamTest.cs" />
index 6aada45..a1ce84c 100644 (file)
@@ -33,7 +33,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests.RevocationTests
             using (endEntity)
             using (X509Certificate2 intermediate2Cert = intermediate2.CloneIssuerCert())
             {
-                responder.RespondEmpty = true;
+                responder.RespondKind = RespondKind.Empty;
 
                 RetryHelper.Execute(() => {
                     using (ChainHolder holder = new ChainHolder())