Skip to content

Commit

Permalink
Fix SslStreamCertificateContext OCSP test failures (dotnet#100644)
Browse files Browse the repository at this point in the history 
* Fix SslStreamCertificateContext OCSP test failures

* Fix code style

* Remove unwanted change

* fixup! Fix code style

* fixup! Remove unwanted change
  • Loading branch information
@rzikm @matouskozak
rzikm authored and matouskozak committed Apr 30, 2024
1 parent b90cec4 commit d76742d
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 61 deletions.
146 changes: 99 additions & 47 deletions ...ibraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using System.Net;
using System.Threading.Tasks;
using Microsoft.Win32.SafeHandles;

Expand Down Expand Up @@ -115,13 +116,21 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran

if (task.IsCompletedSuccessfully)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"Got OCSP response.");
}
return task.Result;
}
}
catch
{
}

if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, "No OCSP response available.");
}
return null;
}

Expand All @@ -136,11 +145,19 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran

if (now > _ocspExpiration)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, "Cached OCSP response expired, fetching fresh staple.");
}
return DownloadOcspAsync();
}

if (now > _nextDownload)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, "Starting async refresh of OCSP staple");
}
// Calling DownloadOcsp will activate a Task to initiate
// in the background. Further calls will attach to the
// same Task if it's still running.
Expand Down Expand Up @@ -171,6 +188,10 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran

if (pending is not null && !pending.IsFaulted)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"Pending download task exists.");
}
return new ValueTask<byte[]?>(pending);
}

Expand Down Expand Up @@ -208,14 +229,18 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran

if (pending is null || pending.IsFaulted)
{
_pendingDownload = pending = FetchOcspAsync();
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"Starting new OCSP download task.");
}
pending = FetchOcspAsync();
}
}

return new ValueTask<byte[]?>(pending);
}

private async Task<byte[]?> FetchOcspAsync()
private Task<byte[]?> FetchOcspAsync()
{
Debug.Assert(_rootCertificate != null);
X509Certificate2? caCert = _privateIntermediateCertificates.Length > 0 ? _privateIntermediateCertificates[0] : _rootCertificate;
Expand All @@ -235,7 +260,7 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
if (subject == 0 || issuer == 0)
{
_staplingForbidden = true;
return null;
return Task.FromResult<byte[]?>(null);
}

IntPtr[] issuerHandles = ArrayPool<IntPtr>.Shared.Rent(_privateIntermediateCertificates.Length + 1);
Expand All @@ -245,64 +270,91 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
}
issuerHandles[_privateIntermediateCertificates.Length] = _rootCertificate.Handle;

using (SafeOcspRequestHandle ocspRequest = Interop.Crypto.X509BuildOcspRequest(subject, issuer))
{
byte[] rentedBytes = ArrayPool<byte>.Shared.Rent(Interop.Crypto.GetOcspRequestDerSize(ocspRequest));
int encodingSize = Interop.Crypto.EncodeOcspRequest(ocspRequest, rentedBytes);
ArraySegment<byte> encoded = new ArraySegment<byte>(rentedBytes, 0, encodingSize);
TaskCompletionSource<byte[]?> completionSource = new TaskCompletionSource<byte[]?>();

ArraySegment<char> rentedChars = UrlBase64Encoding.RentEncode(encoded);
byte[]? ret = null;
_pendingDownload = completionSource.Task;
FetchOcspAsyncCore(completionSource);
return completionSource.Task;

for (int i = 0; i < _ocspUrls.Count; i++)
async void FetchOcspAsyncCore(TaskCompletionSource<byte[]?> completionSource)
{
try
{
string url = MakeUrl(_ocspUrls[i], rentedChars);
ret = await System.Net.Http.X509ResourceClient.DownloadAssetAsync(url, TimeSpan.MaxValue).ConfigureAwait(false);
using SafeOcspRequestHandle ocspRequest = Interop.Crypto.X509BuildOcspRequest(subject, issuer);
byte[] rentedBytes = ArrayPool<byte>.Shared.Rent(Interop.Crypto.GetOcspRequestDerSize(ocspRequest));
int encodingSize = Interop.Crypto.EncodeOcspRequest(ocspRequest, rentedBytes);
ArraySegment<byte> encoded = new ArraySegment<byte>(rentedBytes, 0, encodingSize);

if (ret is not null)
ArraySegment<char> rentedChars = UrlBase64Encoding.RentEncode(encoded);
byte[]? ret = null;

for (int i = 0; i < _ocspUrls.Count; i++)
{
if (!Interop.Crypto.X509DecodeOcspToExpiration(ret, ocspRequest, subject, issuerHandles.AsSpan(0, _privateIntermediateCertificates.Length + 1), out DateTimeOffset expiration))
{
ret = null;
continue;
}
string url = MakeUrl(_ocspUrls[i], rentedChars);
ret = await System.Net.Http.X509ResourceClient.DownloadAssetAsync(url, TimeSpan.MaxValue).ConfigureAwait(false);

// Swap the working URL in as the first one we'll try next time.
if (i != 0)
if (ret is not null)
{
string tmp = _ocspUrls[0];
_ocspUrls[0] = _ocspUrls[i];
_ocspUrls[i] = tmp;
if (!Interop.Crypto.X509DecodeOcspToExpiration(ret, ocspRequest, subject, issuerHandles.AsSpan(0, _privateIntermediateCertificates.Length + 1), out DateTimeOffset expiration))
{
ret = null;
continue;
}

// Swap the working URL in as the first one we'll try next time.
if (i != 0)
{
string tmp = _ocspUrls[0];
_ocspUrls[0] = _ocspUrls[i];
_ocspUrls[i] = tmp;
}

DateTimeOffset nextCheckA = DateTimeOffset.UtcNow.Add(DefaultOcspRefreshInterval);
DateTimeOffset nextCheckB = expiration.Subtract(MinRefreshBeforeExpirationInterval);

_ocspResponse = ret;
_ocspExpiration = expiration;
_nextDownload = nextCheckA < nextCheckB ? nextCheckA : nextCheckB;
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"Received {ret.Length} B OCSP response, Expiration: {_ocspExpiration}, Next refresh: {_nextDownload}");
}
break;
}
}

DateTimeOffset nextCheckA = DateTimeOffset.UtcNow.Add(DefaultOcspRefreshInterval);
DateTimeOffset nextCheckB = expiration.Subtract(MinRefreshBeforeExpirationInterval);
issuerHandles.AsSpan().Clear();
ArrayPool<IntPtr>.Shared.Return(issuerHandles);
ArrayPool<byte>.Shared.Return(rentedBytes);
ArrayPool<char>.Shared.Return(rentedChars.Array!);
GC.KeepAlive(TargetCertificate);
GC.KeepAlive(_privateIntermediateCertificates);
GC.KeepAlive(_rootCertificate);
GC.KeepAlive(caCert);

_ocspResponse = ret;
_ocspExpiration = expiration;
_nextDownload = nextCheckA < nextCheckB ? nextCheckA : nextCheckB;
break;
if (ret == null)
{
// All download attempts failed, don't try again for 5 seconds.
// This backoff will be applied only if the OCSP staple is not expired.
// If it is expired, we will force-refresh it during next GetOcspResponseAsync call.
_nextDownload = DateTimeOffset.UtcNow.Add(RefreshAfterFailureBackOffInterval);
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"OCSP response fetch failed, backing off, Next refresh = {_nextDownload}");
}
}
}

issuerHandles.AsSpan().Clear();
ArrayPool<IntPtr>.Shared.Return(issuerHandles);
ArrayPool<byte>.Shared.Return(rentedBytes);
ArrayPool<char>.Shared.Return(rentedChars.Array!);
GC.KeepAlive(TargetCertificate);
GC.KeepAlive(_privateIntermediateCertificates);
GC.KeepAlive(_rootCertificate);
GC.KeepAlive(caCert);

_pendingDownload = null;
if (ret == null)
_pendingDownload = null;
completionSource.SetResult(ret);
}
catch (Exception ex)
{
// All download attempts failed, don't try again for 5 seconds.
// This backoff will be applied only if the OCSP staple is not expired.
// If it is expired, we will force-refresh it during next GetOcspResponseAsync call.
_nextDownload = DateTimeOffset.UtcNow.Add(RefreshAfterFailureBackOffInterval);
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Error(this, $"OCSP refresh failed: {ex}");
}
completionSource.SetException(ex);
}
return ret;
}
}

Expand Down
26 changes: 12 additions & 14 deletions ...ibraries/System.Net.Security/tests/UnitTests/SslStreamCertificateContextOcspLinuxTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/97836")]
public async Task FetchOcspResponse_FirstInvalidThenValid()
{
await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity, ctxFactory, responder) =>
Expand All @@ -95,7 +94,6 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/97779")]
public async Task RefreshOcspResponse_BeforeExpiration()
{
await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity, ctxFactory, responder) =>
Expand All @@ -110,20 +108,19 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity

intermediate.RevocationExpiration = DateTimeOffset.UtcNow.AddDays(1);

// first call will dispatch a download and return the cached response, the first call after
// the pending download finishes will return the updated response
byte[] ocsp2 = ctx.GetOcspResponseNoWaiting();
Assert.Equal(ocsp, ocsp2);
// First call will dispatch a download. It most likely will return the
// previous cached response, but if the current thread gets delayed
// it may actually return the fresh OCSP staple so we won't check the result
ctx.GetOcspResponseNoWaiting();

// The download should succeed
// The pending download should eventually succeed
byte[] ocsp3 = await ctx.WaitForPendingOcspFetchAsync();
Assert.NotNull(ocsp3);
Assert.NotEqual(ocsp, ocsp3);
});
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/97779")]
public async Task RefreshOcspResponse_AfterExpiration()
{
await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity, ctxFactory, responder) =>
Expand All @@ -139,13 +136,14 @@ await SimpleTest(PkiOptions.OcspEverywhere, async (root, intermediate, endEntity

intermediate.RevocationExpiration = DateTimeOffset.UtcNow.AddDays(1);

// The cached OCSP is expired, so the first call will dispatch a download and return the cached response,
byte[] ocsp = ctx.GetOcspResponseNoWaiting();
Assert.Null(ocsp);
// The cached OCSP is expired, so the first call will dispatch a download.
// It most likely will return null, but if the current thread gets delayed
// it may actually return the fresh OCSP staple so we won't check the result
ctx.GetOcspResponseNoWaiting();

// The download should succeed
byte[] ocsp2 = await ctx.WaitForPendingOcspFetchAsync();
Assert.NotNull(ocsp2);
// The pending download should eventually succeed
byte[] ocsp = await ctx.WaitForPendingOcspFetchAsync();
Assert.NotNull(ocsp);
});
}

Expand Down

0 comments on commit d76742d

Please sign in to comment.