Make OCSP stapling tests more stable (#97979)

* WIP * Make Server OCSP tests more stable * uncomment * Last fix * Move attribute to class instead
dotnet · Feb 6, 2024 · e13a23d · e13a23d
1 parent 31a816a
commit e13a23d
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 18 deletions.
diff --git a/...ibraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/...ibraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
@@ -154,6 +154,17 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
             return ValueTask.FromResult(_ocspResponse);
         }
 
+        internal ValueTask<byte[]?> WaitForPendingOcspFetchAsync()
+        {
+            Task<byte[]?>? pending = _pendingDownload;
+            if (pending is not null && !pending.IsFaulted)
+            {
+                return new ValueTask<byte[]?>(pending);
+            }
+
+            return ValueTask.FromResult(DateTimeOffset.UtcNow <= _ocspExpiration ? _ocspResponse : null);
+        }
+
         private ValueTask<byte[]?> DownloadOcspAsync()
         {
             Task<byte[]?>? pending = _pendingDownload;

diff --git a/...ibraries/System.Net.Security/tests/UnitTests/SslStreamCertificateContextOcspLinuxTests.cs b/...ibraries/System.Net.Security/tests/UnitTests/SslStreamCertificateContextOcspLinuxTests.cs
@@ -15,6 +15,7 @@
 
 namespace System.Net.Security.Tests;
 
+[PlatformSpecific(TestPlatforms.Linux)]
 public class SslStreamCertificateContextOcspLinuxTests
 {
     [Fact]
@@ -101,7 +102,8 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
             intermediate.RevocationExpiration = DateTimeOffset.UtcNow.Add(SslStreamCertificateContext.MinRefreshBeforeExpirationInterval);
 
             SslStreamCertificateContext ctx = ctxFactory(false);
-            byte[] ocsp = await ctx.GetOcspResponseAsync();
+            byte[] ocsp = await ctx.WaitForPendingOcspFetchAsync();
+
             Assert.NotNull(ocsp);
 
             intermediate.RevocationExpiration = DateTimeOffset.UtcNow.AddDays(1);
@@ -111,12 +113,10 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
             byte[] ocsp2 = ctx.GetOcspResponseNoWaiting();
             Assert.Equal(ocsp, ocsp2);
 
-            await RetryHelper.ExecuteAsync(async () =>
-            {
-                byte[] ocsp3 = await ctx.GetOcspResponseAsync();
-                Assert.NotNull(ocsp3);
-                Assert.NotEqual(ocsp, ocsp3);
-            }, maxAttempts: 5, backoffFunc: i => (i + 1) * 200 /* ms */);
+            // The download should succeed
+            byte[] ocsp3 = await ctx.WaitForPendingOcspFetchAsync();
+            Assert.NotNull(ocsp3);
+            Assert.NotEqual(ocsp, ocsp3);
         });
     }
 
@@ -128,7 +128,10 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
             intermediate.RevocationExpiration = DateTimeOffset.UtcNow.AddSeconds(1);
 
             SslStreamCertificateContext ctx = ctxFactory(false);
+            // Make sure the inner OCSP fetch finished
+            await ctx.WaitForPendingOcspFetchAsync();
 
+            // wait until the cached OCSP response expires
             await Task.Delay(2000);
 
             intermediate.RevocationExpiration = DateTimeOffset.UtcNow.AddDays(1);
@@ -137,8 +140,8 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
             byte[] ocsp = ctx.GetOcspResponseNoWaiting();
             Assert.Null(ocsp);
 
-            // subsequent call will return the new response
-            byte[] ocsp2 = await ctx.GetOcspResponseAsync();
+            // The download should succeed
+            byte[] ocsp2 = await ctx.WaitForPendingOcspFetchAsync();
             Assert.NotNull(ocsp2);
         });
     }
@@ -155,25 +158,34 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
             intermediate.RevocationExpiration = DateTimeOffset.UtcNow.Add(SslStreamCertificateContext.MinRefreshBeforeExpirationInterval);
 
             SslStreamCertificateContext ctx = ctxFactory(false);
-            byte[] ocsp = await ctx.GetOcspResponseAsync();
+            // Make sure the inner OCSP fetch finished
+            byte[] ocsp = await ctx.WaitForPendingOcspFetchAsync();
             Assert.NotNull(ocsp);
 
             responder.RespondKind = RespondKind.Invalid;
-            for (int i = 0; i < 3; i++)
+            for (int i = 0; i < 2; i++)
             {
-                await Task.Delay(SslStreamCertificateContext.RefreshAfterFailureBackOffInterval);
                 byte[] ocsp2 = await ctx.GetOcspResponseAsync();
+                await ctx.WaitForPendingOcspFetchAsync();
                 Assert.Equal(ocsp, ocsp2);
+                await Task.Delay(SslStreamCertificateContext.RefreshAfterFailureBackOffInterval.Add(TimeSpan.FromSeconds(1)));
             }
 
+            // make sure we try again only after backoff expires
+            await ctx.WaitForPendingOcspFetchAsync();
+            await Task.Delay(SslStreamCertificateContext.RefreshAfterFailureBackOffInterval.Add(TimeSpan.FromSeconds(1)));
+
             // after responder comes back online, the staple is eventually refreshed
+            intermediate.RevocationExpiration = DateTimeOffset.UtcNow.AddDays(1);
             responder.RespondKind = RespondKind.Normal;
-            await RetryHelper.ExecuteAsync(async () =>
-            {
-                byte[] ocsp3 = await ctx.GetOcspResponseAsync();
-                Assert.NotNull(ocsp3);
-                Assert.NotEqual(ocsp, ocsp3);
-            }, maxAttempts: 5, backoffFunc: i => (i + 1) * 200 /* ms */);
+
+            // dispatch background refresh (first call still returns the old cached value)
+            await ctx.GetOcspResponseAsync();
+
+            // after refresh we should have a new staple
+            byte[] ocsp3 = await ctx.WaitForPendingOcspFetchAsync();
+            Assert.NotNull(ocsp3);
+            Assert.NotEqual(ocsp, ocsp3);
         });
     }