Skip to content

SNOW-2684090 Validate NextUpdate for freshly downloaded CRLs #1277

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
merged 2 commits into from
Nov 13, 2025
+124 −5
Merged

SNOW-2684090 Validate NextUpdate for freshly downloaded CRLs #1277

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc24a7d
SNOW-2684090 Validate NextUpdate for freshly downloaded CRLs
sfc-gh-mgemra Nov 7, 2025
1884cc7
Merge branch 'master' into mgemra/crl-fresh-crl-is-expired
sfc-gh-mgemra Nov 13, 2025
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#### For the official .NET Release Notes please refer to https://docs.snowflake.com/en/release-notes/clients-drivers/dotnet

# Changelog
- v5.1.1
- Fixed CRL validation to reject newly downloaded CRLs if their NextUpdate has already expired.
- v5.1.0
- Added `APPLICATION_PATH` to `CLIENT_ENVIRONMENT` sent during authentication to identify the application connecting to Snowflake.
- Renew idle sessions in the pool if keep alive is enabled.
Expand Down
115 changes: 110 additions & 5 deletions Snowflake.Data.Tests/UnitTests/Revocation/CertificateRevocationVerifierTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ public void TestVerifyCertificateAsUnrevoked()
MockByteResponseForGet(restRequester, DigiCertCrlUrl2, crlBytes);
var crlRepository = new CrlRepository(config.EnableCRLInMemoryCaching, config.EnableCRLDiskCaching);
var environmentOperation = new Mock<EnvironmentOperations>();
var verifier = new CertificateRevocationVerifier(config, TimeProvider.Instance, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);
var timeProvider = new Mock<TimeProvider>();
var testTime = DateTimeOffset.Parse(DigiCertThisUpdateString).UtcDateTime.AddHours(1);
timeProvider.Setup(tp => tp.UtcNow()).Returns(testTime);
var verifier = new CertificateRevocationVerifier(config, timeProvider.Object, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);

// act
var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);
Expand All @@ -81,7 +84,10 @@ public void TestVerifyCertificateAsErrorWhenCouldNotDownloadCrl()
MockErrorResponseForGet(restRequester, DigiCertCrlUrl2, NotFoundHttpExceptionProvider);
var crlRepository = new CrlRepository(config.EnableCRLInMemoryCaching, config.EnableCRLDiskCaching);
var environmentOperation = new Mock<EnvironmentOperations>();
var verifier = new CertificateRevocationVerifier(config, TimeProvider.Instance, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);
var timeProvider = new Mock<TimeProvider>();
var testTime = DateTimeOffset.Parse(DigiCertThisUpdateString).UtcDateTime.AddHours(1);
timeProvider.Setup(tp => tp.UtcNow()).Returns(testTime);
var verifier = new CertificateRevocationVerifier(config, timeProvider.Object, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);

// act
var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);
Expand All @@ -105,7 +111,10 @@ public void TestVerifyCertificateAsErrorWhenOneOfCrlsIsNotParsable()
MockByteResponseForGet(restRequester, DigiCertCrlUrl1, notParsableCrlBytes);
var crlRepository = new CrlRepository(config.EnableCRLInMemoryCaching, config.EnableCRLDiskCaching);
var environmentOperation = new Mock<EnvironmentOperations>();
var verifier = new CertificateRevocationVerifier(config, TimeProvider.Instance, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);
var timeProvider = new Mock<TimeProvider>();
var testTime = DateTimeOffset.Parse(DigiCertThisUpdateString).UtcDateTime.AddHours(1);
timeProvider.Setup(tp => tp.UtcNow()).Returns(testTime);
var verifier = new CertificateRevocationVerifier(config, timeProvider.Object, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);

// act
var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);
Expand Down Expand Up @@ -141,7 +150,10 @@ public void TestVerifyCertificateAsErrorWhenCrlExceedsMaxSize()

var crlRepository = new CrlRepository(config.EnableCRLInMemoryCaching, config.EnableCRLDiskCaching);
var environmentOperation = new Mock<EnvironmentOperations>();
var verifier = new CertificateRevocationVerifier(config, TimeProvider.Instance, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);
var timeProvider = new Mock<TimeProvider>();
var testTime = DateTimeOffset.Parse(DigiCertThisUpdateString).UtcDateTime.AddHours(1);
timeProvider.Setup(tp => tp.UtcNow()).Returns(testTime);
var verifier = new CertificateRevocationVerifier(config, timeProvider.Object, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);

// act
var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);
Expand Down Expand Up @@ -339,6 +351,99 @@ private static void MockErrorResponseForGet(Mock<IRestRequester> restRequester,
.Throws(exceptionProvider);
}

[Test]
public void TestDownloadedCrlIsExpiredAndNoneValidExists()
{
// arrange
var now = DateTime.UtcNow;
var timeProvider = new Mock<TimeProvider>();
timeProvider.Setup(tp => tp.UtcNow()).Returns(now);
var expectedCrlUrls = new[] { "http://test.crl" };

var certKeys = CertificateGenerator.GenerateKeysForCertAndItsParent();
var certSubject = "CN=Test Cert CN, O=Snowflake, OU=Drivers, L=Warsaw, ST=Masovian, C=Poland";
var rootSubject = "CN=Test Root CA, O=Snowflake, OU=Drivers, L=Warsaw, ST=Masovian, C=Poland";
var certificate = CertificateGenerator.GenerateCertificate(certSubject, rootSubject, DateTimeOffset.Now.AddDays(-1), DateTimeOffset.Now.AddDays(300), new[] { expectedCrlUrls }, certKeys[0]);
var parentCertificate = CertificateGenerator.GenerateCertificate(rootSubject, rootSubject, DateTimeOffset.Now.AddDays(-1), DateTimeOffset.Now.AddDays(300), null, certKeys[1]);
var config = GetHttpConfig();

var expiredCrl = CertificateGenerator.GenerateCrl(
CertificateGenerator.SHA256WithRsaAlgorithm,
certKeys[1].Private,
rootSubject,
now.AddHours(-2),
now.AddHours(-1),
now.AddHours(-2)
);
var crlBytes = expiredCrl.GetEncoded();

var restRequester = new Mock<IRestRequester>();
MockByteResponseForGet(restRequester, expectedCrlUrls[0], crlBytes);
var crlRepository = new CrlRepository(config.EnableCRLInMemoryCaching, config.EnableCRLDiskCaching);
var environmentOperation = new Mock<EnvironmentOperations>();
var verifier = new CertificateRevocationVerifier(config, timeProvider.Object, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, new CrlParser(environmentOperation.Object), crlRepository);

// act
var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);

// assert
Assert.AreEqual(CertRevocationCheckResult.CertError, result);
}

[Test]
public void TestDownloadedCrlIsExpiredButTheValidExists()
{
// arrange
var now = DateTime.UtcNow;
var timeProvider = new Mock<TimeProvider>();
timeProvider.Setup(tp => tp.UtcNow()).Returns(now);
var expectedCrlUrls = new[] { "http://test.crl" };

var certKeys = CertificateGenerator.GenerateKeysForCertAndItsParent();
var certSubject = "CN=Test Cert CN, O=Snowflake, OU=Drivers, L=Warsaw, ST=Masovian, C=Poland";
var rootSubject = "CN=Test Root CA, O=Snowflake, OU=Drivers, L=Warsaw, ST=Masovian, C=Poland";
var certificate = CertificateGenerator.GenerateCertificate(certSubject, rootSubject, DateTimeOffset.Now.AddDays(-1), DateTimeOffset.Now.AddDays(300), new[] { expectedCrlUrls }, certKeys[0]);
var parentCertificate = CertificateGenerator.GenerateCertificate(rootSubject, rootSubject, DateTimeOffset.Now.AddDays(-1), DateTimeOffset.Now.AddDays(300), null, certKeys[1]);
var config = GetHttpConfig();

var validCrl = CertificateGenerator.GenerateCrl(
CertificateGenerator.SHA256WithRsaAlgorithm,
certKeys[1].Private,
rootSubject,
now.AddDays(-2),
now.AddHours(2), // NextUpdate in future
now.AddDays(-2)
);

var expiredCrl = CertificateGenerator.GenerateCrl(
CertificateGenerator.SHA256WithRsaAlgorithm,
certKeys[1].Private,
rootSubject,
now.AddHours(-2),
now.AddHours(-1),
now.AddHours(-2)
);
var expiredCrlBytes = expiredCrl.GetEncoded();

var restRequester = new Mock<IRestRequester>();
MockByteResponseForGet(restRequester, expectedCrlUrls[0], expiredCrlBytes);
var crlRepository = new CrlRepository(config.EnableCRLInMemoryCaching, config.EnableCRLDiskCaching);
var environmentOperation = new Mock<EnvironmentOperations>();
var crlParser = new CrlParser(environmentOperation.Object);

// Pre-populate cache with valid CRL
var cachedCrlObject = crlParser.Create(validCrl, now.AddHours(-25));
crlRepository.Set(expectedCrlUrls[0], cachedCrlObject);

var verifier = new CertificateRevocationVerifier(config, timeProvider.Object, restRequester.Object, CertificateCrlDistributionPointsExtractor.Instance, crlParser, crlRepository);

// act
var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);

// assert
Assert.AreEqual(CertRevocationCheckResult.CertUnrevoked, result);
}

private HttpClientConfig GetHttpConfig(CertRevocationCheckMode checkMode = CertRevocationCheckMode.Enabled, long crlDownloadMaxSize = 209715200) =>
new HttpClientConfig(
null,
Expand All @@ -353,7 +458,7 @@ private HttpClientConfig GetHttpConfig(CertRevocationCheckMode checkMode = CertR
true,
checkMode.ToString(),
false,
false,
true,
false,
10,
crlDownloadMaxSize);
Expand Down
12 changes: 12 additions & 0 deletions Snowflake.Data/Core/Revocation/CertificateRevocationVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,18 @@ private CertRevocationCheckResult CheckCertRevocationForOneCrlUrl(X509Certificat
if (needsFreshCrl)
{
var newCrl = FetchCrl(crlUrl);

if (newCrl != null && newCrl.NextUpdate.HasValue && newCrl.NextUpdate.Value < now)
{
s_logger.Warn($"Downloaded CRL from '{crlUrl}' is already expired (next update: {newCrl.NextUpdate.Value})");
newCrl = null;
if (cachedCrl == null)
{
s_logger.Error($"Unable to fetch a valid CRL from '{crlUrl}' for certificate: '{certificate.Subject}'. Downloaded CRL is expired and no fallback available.");
return CertRevocationCheckResult.CertError;
}
}

shouldUpdateCrl = newCrl != null && (cachedCrl == null || newCrl.ThisUpdate > cachedCrl.ThisUpdate);
if (shouldUpdateCrl)
{
Expand Down
Loading