SNOW-2450415 Add crl download max size limitation (#1263)

Author: sfc-gh-mgemra

CHANGELOG.md

@@ -4,6 +4,7 @@
 - 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.
+    - Added `CRLDOWNLOADMAXSIZE` connection parameter to limit the maximum size of CRL files downloaded during certificate revocation checks.
     - AWS WIF will now also check the application config and AWS profile credential store when determining the current AWS region
 - v5.0.0
     - Disabled CRL checks by default.

Snowflake.Data.Tests/UnitTests/Revocation/CertificateRevocationVerifierTest.cs

@@ -114,6 +114,78 @@ public void TestVerifyCertificateAsErrorWhenOneOfCrlsIsNotParsable()
             Assert.AreEqual(CertRevocationCheckResult.CertError, result);
         }
 
+        [Test]
+        public void TestVerifyCertificateAsErrorWhenCrlExceedsMaxSize()
+        {
+            // arrange
+            var expectedCrlUrls = new[] { DigiCertCrlUrl1 };
+            var certificate = CertificateGenerator.LoadFromFile(s_digiCertCertificatePath);
+            var parentCertificate = CertificateGenerator.LoadFromFile(s_digiCertParentCertificatePath);
+            var crlBytes = File.ReadAllBytes(s_digiCertCrlPath);
+
+            var maxSize = crlBytes.Length - 1;
+            var config = GetHttpConfig(CertRevocationCheckMode.Enabled, maxSize);
+
+            var restRequester = new Mock<IRestRequester>();
+            var mockResponse = new HttpResponseMessage(HttpStatusCode.OK)
+            {
+                Content = new StreamContent(new MemoryStream(crlBytes))
+            };
+            mockResponse.Content.Headers.ContentLength = null; // Remove Content-Length to bypass early check
+
+            restRequester
+                .Setup(requester => requester.Get(
+                    It.Is<RestRequestWrapper>(wrapper =>
+                        wrapper.ToRequestMessage(HttpMethod.Get).RequestUri.AbsoluteUri == DigiCertCrlUrl1)))
+                .Returns(mockResponse);
+
+            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);
+
+            // act
+            var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);
+
+            // assert
+            Assert.AreEqual(CertRevocationCheckResult.CertError, result);
+        }
+
+        [Test]
+        public void TestVerifyCertificateAsErrorWhenContentLengthHeaderExceedsMaxSize()
+        {
+            // arrange
+            var expectedCrlUrls = new[] { DigiCertCrlUrl1 };
+            var certificate = CertificateGenerator.LoadFromFile(s_digiCertCertificatePath);
+            var parentCertificate = CertificateGenerator.LoadFromFile(s_digiCertParentCertificatePath);
+
+            var maxSize = 1000;
+            var contentLengthTooLarge = maxSize + 1;
+            var config = GetHttpConfig(CertRevocationCheckMode.Enabled, maxSize);
+
+            var restRequester = new Mock<IRestRequester>();
+            var mockResponse = new HttpResponseMessage(HttpStatusCode.OK)
+            {
+                Content = new ByteArrayContent(Array.Empty<byte>())
+            };
+            mockResponse.Content.Headers.ContentLength = contentLengthTooLarge;
+
+            restRequester
+                .Setup(requester => requester.Get(
+                    It.Is<RestRequestWrapper>(wrapper =>
+                        wrapper.ToRequestMessage(HttpMethod.Get).RequestUri.AbsoluteUri == DigiCertCrlUrl1)))
+                .Returns(mockResponse);
+
+            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);
+
+            // act
+            var result = verifier.CheckCertRevocation(certificate, expectedCrlUrls, parentCertificate);
+
+            // assert
+            Assert.AreEqual(CertRevocationCheckResult.CertError, result);
+        }
+
         [Test]
         public void TestFailWhenCrlSignatureNotMatchingParentKey()
         {
@@ -267,7 +339,7 @@ private static void MockErrorResponseForGet(Mock<IRestRequester> restRequester,
                 .Throws(exceptionProvider);
         }
 
-        private HttpClientConfig GetHttpConfig(CertRevocationCheckMode checkMode = CertRevocationCheckMode.Enabled) =>
+        private HttpClientConfig GetHttpConfig(CertRevocationCheckMode checkMode = CertRevocationCheckMode.Enabled, long crlDownloadMaxSize = 209715200) =>
             new HttpClientConfig(
                 null,
                 null,
@@ -282,6 +354,8 @@ private HttpClientConfig GetHttpConfig(CertRevocationCheckMode checkMode = CertR
                 checkMode.ToString(),
                 false,
                 false,
-                false);
+                false,
+                10,
+                crlDownloadMaxSize);
     }
 }

Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs

@@ -601,6 +601,11 @@ public void TestParseCrlCheckParameters(
         [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadTimeout=abc;", "Parameter CRLDOWNLOADTIMEOUT should have an integer value.")]
         [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadTimeout=0;", "Parameter CRLDOWNLOADTIMEOUT should be greater than 0.")]
         [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadTimeout=-5;", "Parameter CRLDOWNLOADTIMEOUT should be greater than 0.")]
+        [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadMaxSize=abc;", "Parameter CRLDOWNLOADMAXSIZE should have a long value.")]
+        [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadMaxSize=1.5;", "Parameter CRLDOWNLOADMAXSIZE should have a long value.")]
+        [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadMaxSize=9223372036854775808;", "Parameter CRLDOWNLOADMAXSIZE should have a long value.")]
+        [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadMaxSize=0;", "Parameter CRLDOWNLOADMAXSIZE should be greater than 0.")]
+        [TestCase("ACCOUNT=test;USER=testUser;password=testPassword;crlDownloadMaxSize=-100;", "Parameter CRLDOWNLOADMAXSIZE should be greater than 0.")]
         public void TestFailOnInvalidCrlParameters(string connectionString, string expectedErrorMessage)
         {
             // act

Snowflake.Data.Tests/UnitTests/Session/SFHttpClientPropertiesTest.cs

@@ -133,6 +133,23 @@ public void TestThrowsExceptionWhenSettingConnectionLimitPropertyToNonIntegerVal
             Assert.IsTrue(thrown.Message.Contains(expectedErrorMessage));
         }
 
+        [Test]
+        [TestCase(100)]
+        [TestCase(209715200)]
+        public void TestValidCrlDownloadMaxSize(long validMaxSize)
+        {
+            // arrange
+            var connectionString = $"ACCOUNT=account;USER=test;PASSWORD=test;CRLDOWNLOADMAXSIZE={validMaxSize}";
+            var properties = SFSessionProperties.ParseConnectionString(connectionString, new SessionPropertiesContext());
+
+            // act
+            var extractedProperties = SFSessionHttpClientProperties.ExtractAndValidate(properties);
+            var config = extractedProperties.BuildHttpClientConfig();
+
+            // assert
+            Assert.AreEqual(validMaxSize, config.CrlDownloadMaxSize);
+        }
+
         [Test]
         public void TestBuildHttpClientConfig()
         {

Snowflake.Data/Core/HttpUtil.cs

@@ -34,6 +34,7 @@ public HttpClientConfig(
             bool enableCRLInMemoryCaching = true,
             bool allowCertificatesWithoutCrlUrl = true,
             int crlDownloadTimeout = 10,
+            long crlDownloadMaxSize = 209715200,
             string minTlsProtocol = "TLS12",
             string maxTlsProtocol = "TLS13"
             )
@@ -53,6 +54,7 @@ public HttpClientConfig(
             EnableCRLInMemoryCaching = enableCRLInMemoryCaching;
             AllowCertificatesWithoutCrlUrl = allowCertificatesWithoutCrlUrl;
             CrlDownloadTimeout = crlDownloadTimeout;
+            CrlDownloadMaxSize = crlDownloadMaxSize;
             MinTlsProtocol = minTlsProtocol != null ? SslProtocolsExtensions.FromString(minTlsProtocol) : SslProtocols.None;
             MaxTlsProtocol = minTlsProtocol != null ? SslProtocolsExtensions.FromString(maxTlsProtocol) : SslProtocols.None;
 
@@ -73,6 +75,7 @@ public HttpClientConfig(
                     enableCRLInMemoryCaching.ToString(),
                     allowCertificatesWithoutCrlUrl.ToString(),
                     crlDownloadTimeout.ToString(),
+                    crlDownloadMaxSize.ToString(),
                     minTlsProtocol,
                     maxTlsProtocol
                 });
@@ -93,6 +96,7 @@ public HttpClientConfig(
         internal readonly bool EnableCRLInMemoryCaching;
         internal readonly bool AllowCertificatesWithoutCrlUrl;
         internal readonly int CrlDownloadTimeout;
+        internal readonly long CrlDownloadMaxSize;
         internal readonly SslProtocols MinTlsProtocol;
         internal readonly SslProtocols MaxTlsProtocol;
 

Snowflake.Data/Core/Revocation/CertificateRevocationVerifier.cs

@@ -25,6 +25,7 @@ internal class CertificateRevocationVerifier
         private readonly CrlParser _crlParser;
         private readonly CrlRepository _crlRepository;
         private readonly TimeSpan _httpTimeout;
+        private readonly long _crlDownloadMaxSize;
 
         private static readonly ConcurrentDictionary<string, object> s_locksForCrlUrls = new ConcurrentDictionary<string, object>();
         private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger<CertificateRevocationVerifier>();
@@ -40,6 +41,7 @@ public CertificateRevocationVerifier(
             _certRevocationCheckMode = config.CertRevocationCheckMode;
             _allowCertificatesWithoutCrlUrl = config.AllowCertificatesWithoutCrlUrl;
             _httpTimeout = TimeSpan.FromSeconds(config.CrlDownloadTimeout);
+            _crlDownloadMaxSize = config.CrlDownloadMaxSize;
             _timeProvider = timeProvider;
             _restRequester = restRequester;
             _crlExtractor = crlExtractor;
@@ -324,18 +326,43 @@ private Crl FetchCrl(string crlUrl)
             {
                 var response = _restRequester.Get(new RestRequestWrapper(request));
                 now = _timeProvider.UtcNow();
-                crlBytes = response.Content?.ReadAsByteArrayAsync().Result;
+
+                if (response?.Content == null)
+                {
+                    s_logger.Error($"Downloading crl from {crlUrl} failed: response or content is null");
+                    return null;
+                }
+
+                if (response.Content.Headers?.ContentLength.HasValue == true)
+                {
+                    var contentLength = response.Content.Headers.ContentLength.Value;
+                    if (contentLength > _crlDownloadMaxSize)
+                    {
+                        s_logger.Error($"CRL from {crlUrl} exceeds maximum allowed size. Content-Length: {contentLength} bytes, Max allowed: {_crlDownloadMaxSize} bytes");
+                        return null;
+                    }
+                }
+
+                response.Content.LoadIntoBufferAsync(_crlDownloadMaxSize).GetAwaiter().GetResult();
+                crlBytes = response.Content.ReadAsByteArrayAsync().Result;
+            }
+            catch (HttpRequestException ex)
+            {
+                s_logger.Error($"Error reading response from {crlUrl}: Content was too large or a network error occurred. Max allowed: {_crlDownloadMaxSize} bytes. Error: {ex.Message}");
+                return null;
             }
             catch (Exception exception)
             {
                 s_logger.Error($"Downloading crl from {crlUrl} failed", exception);
                 return null;
             }
+
             if (crlBytes == null || crlBytes.Length == 0)
             {
                 s_logger.Error($"Downloading crl from {crlUrl} failed");
                 return null;
             }
+
             try
             {
                 var crl = _crlParser.Parse(crlBytes, now);

Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

@@ -49,6 +49,7 @@ internal class SFSessionHttpClientProperties
         internal bool _enableCrlInMemoryCaching;
         internal bool _allowCertificatesWithoutCrlUrl;
         private int _crlDownloadTimeout;
+        private long _crlDownloadMaxSize;
         internal string _minTlsProtocol;
         internal string _maxTlsProtocol;
 
@@ -219,6 +220,7 @@ public HttpClientConfig BuildHttpClientConfig()
                 _enableCrlInMemoryCaching,
                 _allowCertificatesWithoutCrlUrl,
                 _crlDownloadTimeout,
+                _crlDownloadMaxSize,
                 _minTlsProtocol,
                 _maxTlsProtocol
                 );
@@ -287,6 +289,7 @@ public SFSessionHttpClientProperties ExtractProperties(SFSessionProperties prope
                     _enableCrlInMemoryCaching = Boolean.Parse(propertiesDictionary[SFSessionProperty.ENABLECRLINMEMORYCACHING]),
                     _allowCertificatesWithoutCrlUrl = Boolean.Parse(propertiesDictionary[SFSessionProperty.ALLOWCERTIFICATESWITHOUTCRLURL]),
                     _crlDownloadTimeout = int.Parse(propertiesDictionary[SFSessionProperty.CRLDOWNLOADTIMEOUT]),
+                    _crlDownloadMaxSize = long.Parse(propertiesDictionary[SFSessionProperty.CRLDOWNLOADMAXSIZE]),
                     _minTlsProtocol = propertiesDictionary[SFSessionProperty.MINTLS],
                     _maxTlsProtocol = propertiesDictionary[SFSessionProperty.MAXTLS]
                 };

Snowflake.Data/Core/Session/SFSessionProperty.cs

@@ -144,6 +144,8 @@ internal enum SFSessionProperty
         ALLOWCERTIFICATESWITHOUTCRLURL,
         [SFSessionPropertyAttr(required = false, defaultValue = "10")]
         CRLDOWNLOADTIMEOUT,
+        [SFSessionPropertyAttr(required = false, defaultValue = "209715200")]
+        CRLDOWNLOADMAXSIZE,
         [SFSessionPropertyAttr(required = false, defaultValue = "tls12")]
         MINTLS,
         [SFSessionPropertyAttr(required = false, defaultValue = "tls13")]
@@ -344,6 +346,7 @@ private static void ValidateCrlParameters(SFSessionProperties properties)
             ValidateBooleanParameter(SFSessionProperty.ENABLECRLINMEMORYCACHING, properties);
             ValidateBooleanParameter(SFSessionProperty.ALLOWCERTIFICATESWITHOUTCRLURL, properties);
             ValidatePositiveIntegerParameter(SFSessionProperty.CRLDOWNLOADTIMEOUT, properties);
+            ValidatePositiveLongParameter(SFSessionProperty.CRLDOWNLOADMAXSIZE, properties);
         }
 
         private static void ValidateTlsParameters(SFSessionProperties properties)
@@ -413,6 +416,24 @@ private static int ValidatePositiveIntegerParameter(SFSessionProperty property,
             return result;
         }
 
+        private static long ValidatePositiveLongParameter(SFSessionProperty property, SFSessionProperties properties)
+        {
+            var propertyString = properties[property];
+            if (!long.TryParse(propertyString, out var result))
+            {
+                var exception = new SnowflakeDbException(SFError.INVALID_CONNECTION_STRING, $"Parameter {property.ToString()} should have a long value.");
+                logger.Error(exception.Message, exception);
+                throw exception;
+            }
+            if (result <= 0)
+            {
+                var exception = new SnowflakeDbException(SFError.INVALID_CONNECTION_STRING, $"Parameter {property.ToString()} should be greater than 0.");
+                logger.Error(exception.Message, exception);
+                throw exception;
+            }
+            return result;
+        }
+
         private static void ValidateSchemeHostPort(SFSessionProperties properties)
         {
             var uriBuilder = new UriBuilder();