Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Linux fetch timeout behavior for chain building #40676

Merged
merged 8 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ private readonly Dictionary<string, CertificateAuthority> _crlPaths

public bool RespondEmpty { get; set; }

public TimeSpan ResponseDelay { get; set; }
public DelayedActionsFlag DelayedActions { get; set; }

private RevocationResponder(HttpListener listener, string uriPrefix)
{
_listener = listener;
Expand Down Expand Up @@ -160,6 +163,12 @@ private void HandleRequest(HttpListenerContext context, ref bool responded)

if (_aiaPaths.TryGetValue(url, out authority))
{
if (DelayedActions.HasFlag(DelayedActionsFlag.Aia))
{
Trace($"Delaying response by {ResponseDelay}.");
Thread.Sleep(ResponseDelay);
}

byte[] certData = RespondEmpty ? Array.Empty<byte>() : authority.GetCertData();

responded = true;
Expand All @@ -172,6 +181,12 @@ private void HandleRequest(HttpListenerContext context, ref bool responded)

if (_crlPaths.TryGetValue(url, out authority))
{
if (DelayedActions.HasFlag(DelayedActionsFlag.Crl))
{
Trace($"Delaying response by {ResponseDelay}.");
Thread.Sleep(ResponseDelay);
}

byte[] crl = RespondEmpty ? Array.Empty<byte>() : authority.GetCrl();

responded = true;
Expand Down Expand Up @@ -211,6 +226,12 @@ private void HandleRequest(HttpListenerContext context, ref bool responded)

byte[] ocspResponse = RespondEmpty ? Array.Empty<byte>() : authority.BuildOcspResponse(certId, nonce);

if (DelayedActions.HasFlag(DelayedActionsFlag.Ocsp))
{
Trace($"Delaying response by {ResponseDelay}.");
Thread.Sleep(ResponseDelay);
}

responded = true;
context.Response.StatusCode = 200;
context.Response.StatusDescription = "OK";
Expand Down Expand Up @@ -352,5 +373,14 @@ private static void Trace(string trace)
Console.WriteLine(trace);
}
}

internal enum DelayedActionsFlag : byte
{
None = 0,
Ocsp = 0b1,
Crl = 0b10,
Aia = 0b100,
All = 0b11111111
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ internal static class CertificateAssetDownloader
{
private static readonly Func<string, CancellationToken, byte[]>? s_downloadBytes = CreateDownloadBytesFunc();

internal static X509Certificate2? DownloadCertificate(string uri, ref TimeSpan remainingDownloadTime)
internal static X509Certificate2? DownloadCertificate(string uri, TimeSpan downloadTimeout)
{
byte[]? data = DownloadAsset(uri, ref remainingDownloadTime);
byte[]? data = DownloadAsset(uri, downloadTimeout);

if (data == null || data.Length == 0)
{
Expand All @@ -39,9 +39,9 @@ internal static class CertificateAssetDownloader
}
}

internal static SafeX509CrlHandle? DownloadCrl(string uri, ref TimeSpan remainingDownloadTime)
internal static SafeX509CrlHandle? DownloadCrl(string uri, TimeSpan downloadTimeout)
{
byte[]? data = DownloadAsset(uri, ref remainingDownloadTime);
byte[]? data = DownloadAsset(uri, downloadTimeout);

if (data == null)
{
Expand Down Expand Up @@ -77,9 +77,9 @@ internal static class CertificateAssetDownloader
return null;
}

internal static SafeOcspResponseHandle? DownloadOcspGet(string uri, ref TimeSpan remainingDownloadTime)
internal static SafeOcspResponseHandle? DownloadOcspGet(string uri, TimeSpan downloadTimeout)
{
byte[]? data = DownloadAsset(uri, ref remainingDownloadTime);
byte[]? data = DownloadAsset(uri, downloadTimeout);

if (data == null)
{
Expand All @@ -100,12 +100,11 @@ internal static class CertificateAssetDownloader
return resp;
}

private static byte[]? DownloadAsset(string uri, ref TimeSpan remainingDownloadTime)
private static byte[]? DownloadAsset(string uri, TimeSpan downloadTimeout)
{
if (s_downloadBytes != null && remainingDownloadTime > TimeSpan.Zero)
if (s_downloadBytes != null && downloadTimeout > TimeSpan.Zero)
{
long totalMillis = (long)remainingDownloadTime.TotalMilliseconds;
Stopwatch stopwatch = Stopwatch.StartNew();
long totalMillis = (long)downloadTimeout.TotalMilliseconds;
CancellationTokenSource? cts = totalMillis > int.MaxValue ? null : new CancellationTokenSource((int)totalMillis);

try
Expand All @@ -115,8 +114,6 @@ internal static class CertificateAssetDownloader
catch { }
finally
{
// TimeSpan.Zero isn't a worrisome value on the subtraction, it only means "no limit" on the original input.
remainingDownloadTime -= stopwatch.Elapsed;
cts?.Dispose();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ public static IChainPal BuildChain(
TimeSpan timeout,
bool disableAia)
{
// An input value of 0 on the timeout is "take all the time you need".
// An input value of 0 on the timeout is treated as 15 seconds, to match Windows.
if (timeout == TimeSpan.Zero)
{
timeout = TimeSpan.MaxValue;
timeout = TimeSpan.FromSeconds(15);
}

bartonjs marked this conversation as resolved.
Show resolved Hide resolved
// Let Unspecified mean Local, so only convert if the source was UTC.
Expand All @@ -55,14 +55,14 @@ public static IChainPal BuildChain(
{
}

TimeSpan remainingDownloadTime = timeout;
TimeSpan downloadTimeout = timeout;

OpenSslX509ChainProcessor chainPal = OpenSslX509ChainProcessor.InitiateChain(
((OpenSslX509CertificateReader)cert).SafeHandle,
customTrustStore,
trustMode,
verificationTime,
remainingDownloadTime);
downloadTimeout);

Interop.Crypto.X509VerifyStatusCode status = chainPal.FindFirstChain(extraStore);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public static void AddCrlForCertificate(
SafeX509StoreHandle store,
X509RevocationMode revocationMode,
DateTime verificationTime,
ref TimeSpan remainingDownloadTime)
TimeSpan downloadTimeout)
{
// In Offline mode, accept any cached CRL we have.
// "CRL is Expired" is a better match for Offline than "Could not find CRL"
Expand All @@ -59,14 +59,13 @@ public static void AddCrlForCertificate(
return;
}

// Don't do any work if we're over limit or prohibited from fetching new CRLs
if (remainingDownloadTime <= TimeSpan.Zero ||
revocationMode != X509RevocationMode.Online)
// Don't do any work if we're prohibited from fetching new CRLs
if (revocationMode != X509RevocationMode.Online)
{
return;
}

DownloadAndAddCrl(url, crlFileName, store, ref remainingDownloadTime);
DownloadAndAddCrl(url, crlFileName, store, downloadTimeout);
}

private static bool AddCachedCrl(string crlFileName, SafeX509StoreHandle store, DateTime verificationTime)
Expand Down Expand Up @@ -134,11 +133,11 @@ private static void DownloadAndAddCrl(
string url,
string crlFileName,
SafeX509StoreHandle store,
ref TimeSpan remainingDownloadTime)
TimeSpan downloadTimeout)
{
// X509_STORE_add_crl will increase the refcount on the CRL object, so we should still
// dispose our copy.
using (SafeX509CrlHandle? crl = CertificateAssetDownloader.DownloadCrl(url, ref remainingDownloadTime))
using (SafeX509CrlHandle? crl = CertificateAssetDownloader.DownloadCrl(url, downloadTimeout))
{
// null is a valid return (e.g. no remainingDownloadTime)
if (crl != null && !crl.IsInvalid)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal sealed class OpenSslX509ChainProcessor : IChainPal
private readonly SafeX509StackHandle _untrustedLookup;
private readonly SafeX509StoreCtxHandle _storeCtx;
private readonly DateTime _verificationTime;
private TimeSpan _remainingDownloadTime;
private readonly TimeSpan _downloadTimeout;
private WorkingChain? _workingChain;

private OpenSslX509ChainProcessor(
Expand All @@ -52,14 +52,14 @@ private OpenSslX509ChainProcessor(
SafeX509StackHandle untrusted,
SafeX509StoreCtxHandle storeCtx,
DateTime verificationTime,
TimeSpan remainingDownloadTime)
TimeSpan downloadTimeout)
{
_leafHandle = leafHandle;
_store = store;
_untrustedLookup = untrusted;
_storeCtx = storeCtx;
_verificationTime = verificationTime;
_remainingDownloadTime = remainingDownloadTime;
_downloadTimeout = downloadTimeout;
}

public void Dispose()
Expand Down Expand Up @@ -236,7 +236,7 @@ internal Interop.Crypto.X509VerifyStatusCode FindChainViaAia(

X509Certificate2? downloaded = DownloadCertificate(
authorityInformationAccess,
ref _remainingDownloadTime);
_downloadTimeout);

// The AIA record is contained in a public structure, so no need to clear it.
CryptoPool.Return(authorityInformationAccess.Array!, clearSize: 0);
Expand Down Expand Up @@ -366,7 +366,7 @@ internal void ProcessRevocation(
_store,
revocationMode,
_verificationTime,
ref _remainingDownloadTime);
_downloadTimeout);
}
}
}
Expand Down Expand Up @@ -655,7 +655,7 @@ private Interop.Crypto.X509VerifyStatusCode CheckOcsp(
return status;
}

if (revocationMode != X509RevocationMode.Online || _remainingDownloadTime <= TimeSpan.Zero)
if (revocationMode != X509RevocationMode.Online)
{
return Interop.Crypto.X509VerifyStatusCode.X509_V_ERR_UNABLE_TO_GET_CRL;
}
Expand Down Expand Up @@ -690,7 +690,7 @@ private Interop.Crypto.X509VerifyStatusCode CheckOcsp(
//
// So, for now, only try GET.
SafeOcspResponseHandle? resp =
CertificateAssetDownloader.DownloadOcspGet(requestUrl, ref _remainingDownloadTime);
CertificateAssetDownloader.DownloadOcspGet(requestUrl, _downloadTimeout);

using (resp)
{
Expand Down Expand Up @@ -1072,22 +1072,16 @@ private static X509ChainStatusFlags MapVerifyErrorToChainStatus(Interop.Crypto.X

private static X509Certificate2? DownloadCertificate(
ReadOnlyMemory<byte> authorityInformationAccess,
ref TimeSpan remainingDownloadTime)
TimeSpan downloadTimeout)
{
// Don't do any work if we're over limit.
if (remainingDownloadTime <= TimeSpan.Zero)
{
return null;
}

string? uri = FindHttpAiaRecord(authorityInformationAccess, Oids.CertificateAuthorityIssuers);

if (uri == null)
{
return null;
}

return CertificateAssetDownloader.DownloadCertificate(uri, ref remainingDownloadTime);
return CertificateAssetDownloader.DownloadCertificate(uri, downloadTimeout);
}

private static string? GetOcspEndpoint(SafeX509Handle cert)
Expand Down
Loading