Skip to content

Use RetryHelper with AIA tests #55230

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.

Sign up for GitHub

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

Merged
stephentoub merged 2 commits into from
Jul 9, 2021
Merged

Use RetryHelper with AIA tests #55230

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f96e434
Use RetryHelper with AIA tests.
vcsjones Jul 6, 2021
edc029c
Narrow the use of RetryHelper
vcsjones Jul 7, 2021
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
Next Next commit
Use RetryHelper with AIA tests. 
The AIA tests depend on many external factors including the underlying
operation system, network, and the resources available on the system.
To accomodate this we will re-run the tests to improve reliability.
  • Loading branch information
@vcsjones
vcsjones authored Jul 6, 2021
commit f96e4342d6be27dff8a3a0753a134ea851e56dce
196 changes: 100 additions & 96 deletions ...libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,124 +14,128 @@ public static class AiaTests
[Fact]
public static void EmptyAiaResponseIsIgnored()
{
CertificateAuthority.BuildPrivatePki(
PkiOptions.AllRevocation,
out RevocationResponder responder,
out CertificateAuthority root,
out CertificateAuthority[] intermediates,
out X509Certificate2 endEntity,
intermediateAuthorityCount: 2,
pkiOptionsInSubject: false,
testName: nameof(EmptyAiaResponseIsIgnored));

using (responder)
using (root)
using (CertificateAuthority intermediate1 = intermediates[0])
using (CertificateAuthority intermediate2 = intermediates[1])
using (endEntity)
using (ChainHolder holder = new ChainHolder())
using (X509Certificate2 intermediate2Cert = intermediate2.CloneIssuerCert())
{
responder.RespondEmpty = true;

X509Chain chain = holder.Chain;
chain.ChainPolicy.ExtraStore.Add(intermediate2Cert);
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
chain.ChainPolicy.VerificationTime = endEntity.NotBefore.AddMinutes(1);
chain.ChainPolicy.UrlRetrievalTimeout = DynamicRevocationTests.s_urlRetrievalLimit;
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;

Assert.False(chain.Build(endEntity));
X509ChainStatusFlags chainFlags = chain.AllStatusFlags();
Assert.True(chainFlags.HasFlag(X509ChainStatusFlags.PartialChain), $"expected partial chain flags, got {chainFlags}");
Assert.Equal(2, chain.ChainElements.Count);
}
RetryHelper.Execute(() => {
CertificateAuthority.BuildPrivatePki(
Copy link
Member

@bartonjs bartonjs Jul 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the one hand, not capturing closures is nice. On the other, recreating the PKI pieces and listener and such feels like it could be hiding bugs.. so we probably only want to retry the inner portion of the usings (though moving ChainHolder inside the retry so we're not introducing a "works on the second call to Build" bug)

Copy link
Member Author

@vcsjones vcsjones Jul 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, I'm wary of making that even more non-deterministic based on my experience last time from the AIA tests. But those tests for sensitive to execution time, so maybe I am applying the wrong logic. I suppose it can't hurt to try the more narrow way first.

PkiOptions.AllRevocation,
out RevocationResponder responder,
out CertificateAuthority root,
out CertificateAuthority[] intermediates,
out X509Certificate2 endEntity,
intermediateAuthorityCount: 2,
pkiOptionsInSubject: false,
testName: nameof(EmptyAiaResponseIsIgnored));

using (responder)
using (root)
using (CertificateAuthority intermediate1 = intermediates[0])
using (CertificateAuthority intermediate2 = intermediates[1])
using (endEntity)
using (ChainHolder holder = new ChainHolder())
using (X509Certificate2 intermediate2Cert = intermediate2.CloneIssuerCert())
{
responder.RespondEmpty = true;

X509Chain chain = holder.Chain;
chain.ChainPolicy.ExtraStore.Add(intermediate2Cert);
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
chain.ChainPolicy.VerificationTime = endEntity.NotBefore.AddMinutes(1);
chain.ChainPolicy.UrlRetrievalTimeout = DynamicRevocationTests.s_urlRetrievalLimit;
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;

Assert.False(chain.Build(endEntity));
X509ChainStatusFlags chainFlags = chain.AllStatusFlags();
Assert.True(chainFlags.HasFlag(X509ChainStatusFlags.PartialChain), $"expected partial chain flags, got {chainFlags}");
Assert.Equal(2, chain.ChainElements.Count);
}
});
}

[Fact]
[SkipOnPlatform(PlatformSupport.MobileAppleCrypto, "CA store is not available")]
public static void DisableAiaOptionWorks()
{
CertificateAuthority.BuildPrivatePki(
PkiOptions.AllRevocation,
out RevocationResponder responder,
out CertificateAuthority root,
out CertificateAuthority intermediate,
out X509Certificate2 endEntity,
pkiOptionsInSubject: false,
testName: nameof(DisableAiaOptionWorks));

using (responder)
using (root)
using (intermediate)
using (endEntity)
using (ChainHolder holder = new ChainHolder())
using (X509Certificate2 rootCert = root.CloneIssuerCert())
using (X509Certificate2 intermediateCert = intermediate.CloneIssuerCert())
using (var cuCaStore = new X509Store(StoreName.CertificateAuthority, StoreLocation.CurrentUser))
{
cuCaStore.Open(OpenFlags.ReadWrite);

X509Chain chain = holder.Chain;

// macOS combines revocation and AIA fetching in to a single flag. Both need to be disabled
// to prevent AIA fetches.
if (PlatformDetection.IsOSX)
RetryHelper.Execute(() => {
CertificateAuthority.BuildPrivatePki(
PkiOptions.AllRevocation,
out RevocationResponder responder,
out CertificateAuthority root,
out CertificateAuthority intermediate,
out X509Certificate2 endEntity,
pkiOptionsInSubject: false,
testName: nameof(DisableAiaOptionWorks));

using (responder)
using (root)
using (intermediate)
using (endEntity)
using (ChainHolder holder = new ChainHolder())
using (X509Certificate2 rootCert = root.CloneIssuerCert())
using (X509Certificate2 intermediateCert = intermediate.CloneIssuerCert())
using (var cuCaStore = new X509Store(StoreName.CertificateAuthority, StoreLocation.CurrentUser))
{
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
}
cuCaStore.Open(OpenFlags.ReadWrite);

chain.ChainPolicy.DisableCertificateDownloads = true;
chain.ChainPolicy.CustomTrustStore.Add(rootCert);
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
chain.ChainPolicy.VerificationTime = endEntity.NotBefore.AddMinutes(1);
chain.ChainPolicy.UrlRetrievalTimeout = DynamicRevocationTests.s_urlRetrievalLimit;
X509Chain chain = holder.Chain;

Assert.False(chain.Build(endEntity), "Chain build with no intermediate, AIA disabled");
// macOS combines revocation and AIA fetching in to a single flag. Both need to be disabled
// to prevent AIA fetches.
if (PlatformDetection.IsOSX)
{
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
}

// If a previous run of this test leaves contamination in the CU\CA store on Windows
// the Windows chain engine will match the bad issuer and report NotSignatureValid instead
// of PartialChain.
X509ChainStatusFlags chainFlags = chain.AllStatusFlags();
chain.ChainPolicy.DisableCertificateDownloads = true;
chain.ChainPolicy.CustomTrustStore.Add(rootCert);
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
chain.ChainPolicy.VerificationTime = endEntity.NotBefore.AddMinutes(1);
chain.ChainPolicy.UrlRetrievalTimeout = DynamicRevocationTests.s_urlRetrievalLimit;

if (chainFlags.HasFlag(X509ChainStatusFlags.NotSignatureValid))
{
Assert.Equal(3, chain.ChainElements.Count);
Assert.False(chain.Build(endEntity), "Chain build with no intermediate, AIA disabled");

foreach (X509Certificate2 storeCert in cuCaStore.Certificates)
// If a previous run of this test leaves contamination in the CU\CA store on Windows
// the Windows chain engine will match the bad issuer and report NotSignatureValid instead
// of PartialChain.
X509ChainStatusFlags chainFlags = chain.AllStatusFlags();

if (chainFlags.HasFlag(X509ChainStatusFlags.NotSignatureValid))
{
if (storeCert.Subject.Equals(intermediateCert.Subject))
Assert.Equal(3, chain.ChainElements.Count);

foreach (X509Certificate2 storeCert in cuCaStore.Certificates)
{
cuCaStore.Remove(storeCert);
if (storeCert.Subject.Equals(intermediateCert.Subject))
{
cuCaStore.Remove(storeCert);
}

storeCert.Dispose();
}

storeCert.Dispose();
holder.DisposeChainElements();

// Try again, with no caching side effect.
Assert.False(chain.Build(endEntity), "Chain build 2 with no intermediate, AIA disabled");
}

Assert.Equal(1, chain.ChainElements.Count);
Assert.Contains(X509ChainStatusFlags.PartialChain, chain.ChainStatus.Select(s => s.Status));
holder.DisposeChainElements();

// Try again, with no caching side effect.
Assert.False(chain.Build(endEntity), "Chain build 2 with no intermediate, AIA disabled");
}

Assert.Equal(1, chain.ChainElements.Count);
Assert.Contains(X509ChainStatusFlags.PartialChain, chain.ChainStatus.Select(s => s.Status));
holder.DisposeChainElements();

chain.ChainPolicy.ExtraStore.Add(intermediateCert);
Assert.True(chain.Build(endEntity), "Chain build with intermediate, AIA disabled");
Assert.Equal(3, chain.ChainElements.Count);
Assert.Equal(X509ChainStatusFlags.NoError, chain.AllStatusFlags());
holder.DisposeChainElements();
chain.ChainPolicy.ExtraStore.Add(intermediateCert);
Assert.True(chain.Build(endEntity), "Chain build with intermediate, AIA disabled");
Assert.Equal(3, chain.ChainElements.Count);
Assert.Equal(X509ChainStatusFlags.NoError, chain.AllStatusFlags());
holder.DisposeChainElements();

chain.ChainPolicy.DisableCertificateDownloads = false;
chain.ChainPolicy.ExtraStore.Clear();
Assert.True(chain.Build(endEntity), "Chain build with no intermediate, AIA enabled");
Assert.Equal(3, chain.ChainElements.Count);
Assert.Equal(X509ChainStatusFlags.NoError, chain.AllStatusFlags());
chain.ChainPolicy.DisableCertificateDownloads = false;
chain.ChainPolicy.ExtraStore.Clear();
Assert.True(chain.Build(endEntity), "Chain build with no intermediate, AIA enabled");
Assert.Equal(3, chain.ChainElements.Count);
Assert.Equal(X509ChainStatusFlags.NoError, chain.AllStatusFlags());

cuCaStore.Remove(intermediateCert);
}
cuCaStore.Remove(intermediateCert);
}
});
}
}
}