Merge branch 'master' into SNOW-1629635-fix-connection-pool-allocation

README.md

@@ -114,6 +114,10 @@ Using stage files within PUT/GET commands:
 Logging description and configuration:
 [Logging and Easy Logging](doc/Logging.md)
 
+## Certificate validation
+Method of validating the connection's certificates in the .NET driver differs from the rest of the Snowflake drivers.
+Read more in [certificate validation](doc/CertficateValidation.md) docs.
+
 ---------------
 
 ## Notice

Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs

@@ -9,13 +9,55 @@ namespace Snowflake.Data.Tests.UnitTests
     using NUnit.Framework;
     using Snowflake.Data.Core;
     using RichardSzalay.MockHttp;
+    using System.Threading;
     using System.Threading.Tasks;
     using System.Net;
     using System;
+    using System.Security.Authentication;
+    using Moq;
+    using Moq.Protected;
 
     [TestFixture]
     class HttpUtilTest
     {
+        [Test]
+        public async Task TestNonRetryableHttpExceptionThrowsError()
+        {
+            var request = new HttpRequestMessage(HttpMethod.Post, new Uri("https://authenticationexceptiontest.com/"));
+            // Disable warning as this is the way to be compliant with netstandard2.0
+            // API reference: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httprequestmessage?view=netstandard-2.0
+#pragma warning disable CS0618 // Type or member is obsolete
+            request.Properties[BaseRestRequest.HTTP_REQUEST_TIMEOUT_KEY] = Timeout.InfiniteTimeSpan;
+            request.Properties[BaseRestRequest.REST_REQUEST_TIMEOUT_KEY] = Timeout.InfiniteTimeSpan;
+#pragma warning restore CS0618 // Type or member is obsolete
+
+            var handler = new Mock<DelegatingHandler>();
+            handler.Protected()
+              .Setup<Task<HttpResponseMessage>>(
+                "SendAsync",
+                ItExpr.Is<HttpRequestMessage>(req => req.RequestUri.ToString().Contains("https://authenticationexceptiontest.com/")),
+                ItExpr.IsAny<CancellationToken>())
+            .ThrowsAsync(new HttpRequestException("", new AuthenticationException()));
+
+            var httpClient = HttpUtil.Instance.GetHttpClient(
+                new HttpClientConfig(false, "fakeHost", "fakePort", "user", "password", "fakeProxyList", false, false, 7),
+                handler.Object);
+
+            try
+            {
+                await httpClient.SendAsync(request, CancellationToken.None).ConfigureAwait(false);
+                Assert.Fail();
+            }
+            catch (HttpRequestException e)
+            {
+                Assert.IsInstanceOf<AuthenticationException>(e.InnerException);
+            }
+            catch (Exception unexpected)
+            {
+                Assert.Fail($"Unexpected {unexpected.GetType()} exception occurred");
+            }
+        }
+
         [Test]
         // Parameters: status code, force retry on 404, expected retryable value
         [TestCase(HttpStatusCode.OK, false, false)]

Snowflake.Data/Core/HttpUtil.cs

@@ -100,24 +100,24 @@ private HttpUtil()
 
         private Dictionary<string, HttpClient> _HttpClients = new Dictionary<string, HttpClient>();
 
-        internal HttpClient GetHttpClient(HttpClientConfig config)
+        internal HttpClient GetHttpClient(HttpClientConfig config, DelegatingHandler customHandler = null)
         {
             lock (httpClientProviderLock)
             {
-                return RegisterNewHttpClientIfNecessary(config);
+                return RegisterNewHttpClientIfNecessary(config, customHandler);
             }
         }
 
 
-        private HttpClient RegisterNewHttpClientIfNecessary(HttpClientConfig config)
+        private HttpClient RegisterNewHttpClientIfNecessary(HttpClientConfig config, DelegatingHandler customHandler = null)
         {
             string name = config.ConfKey;
             if (!_HttpClients.ContainsKey(name))
             {
                 logger.Debug("Http client not registered. Adding.");
 
                 var httpClient = new HttpClient(
-                    new RetryHandler(SetupCustomHttpHandler(config), config.DisableRetry, config.ForceRetryOn404, config.MaxHttpRetries, config.IncludeRetryReason))
+                    new RetryHandler(SetupCustomHttpHandler(config, customHandler), config.DisableRetry, config.ForceRetryOn404, config.MaxHttpRetries, config.IncludeRetryReason))
                 {
                     Timeout = Timeout.InfiniteTimeSpan
                 };
@@ -129,8 +129,13 @@ private HttpClient RegisterNewHttpClientIfNecessary(HttpClientConfig config)
             return _HttpClients[name];
         }
 
-        internal HttpMessageHandler SetupCustomHttpHandler(HttpClientConfig config)
+        internal HttpMessageHandler SetupCustomHttpHandler(HttpClientConfig config, DelegatingHandler customHandler = null)
         {
+            if (customHandler != null)
+            {
+                return customHandler;
+            }
+
             HttpMessageHandler httpHandler;
             try
             {
@@ -352,6 +357,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                 bool isOktaSSORequest = IsOktaSSORequest(requestMessage.RequestUri.Host, absolutePath);
                 int backOffInSec = s_baseBackOffTime;
                 int totalRetryTime = 0;
+                Exception lastException = null;
 
                 ServicePoint p = ServicePointManager.FindServicePoint(requestMessage.RequestUri);
                 p.Expect100Continue = false; // Saves about 100 ms per request
@@ -392,6 +398,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                     }
                     catch (Exception e)
                     {
+                        lastException = e;
                         if (cancellationToken.IsCancellationRequested)
                         {
                             logger.Debug("SF rest request timeout or explicit cancel called.");
@@ -404,8 +411,18 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                         }
                         else
                         {
-                            //TODO: Should probably check to see if the error is recoverable or transient.
-                            logger.Warn("Error occurred during request, retrying...", e);
+                            Exception innermostException = GetInnerMostException(e);
+
+                            if (innermostException is AuthenticationException)
+                            {
+                                logger.Error("Non-retryable error encountered: ", e);
+                                throw;
+                            }
+                            else
+                            {
+                                //TODO: Should probably check to see if the error is recoverable or transient.
+                                logger.Warn("Error occurred during request, retrying...", e);
+                            }
                         }
                     }
 
@@ -455,7 +472,10 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                         {
                             return response;
                         }
-                        throw new OperationCanceledException($"http request failed and max retry {maxRetryCount} reached");
+                        var errorMessage = $"http request failed and max retry {maxRetryCount} reached.\n";
+                        errorMessage += $"Last exception encountered: {lastException}";
+                        logger.Error(errorMessage);
+                        throw new OperationCanceledException(errorMessage);
                     }
 
                     // Disposing of the response if not null now that we don't need it anymore
@@ -495,6 +515,14 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             }
         }
 
+        static private Exception GetInnerMostException(Exception exception)
+        {
+            var innermostException = exception;
+            while (innermostException.InnerException != null && innermostException != innermostException.InnerException)
+                innermostException = innermostException.InnerException;
+            return innermostException;
+        }
+
         /// <summary>
         /// Check whether or not the error is retryable or not.
         /// </summary>

doc/CertficateValidation.md

@@ -0,0 +1,105 @@
+## Validating certificates
+
+### OCSP and CRL
+At the moment of writing (autumn 2024) Snowflake .NET driver still uses a method to verify the validity
+of certificates upon connection, which is different from all the other Snowflake drivers.
+* .NET driver - uses Certificate Revocation List  (CRL) ([wikipage](https://en.wikipedia.org/wiki/Certificate_revocation_list))
+* all other Snowflake drivers (JDBC, ODBC, Python, etc.) - use Online Certificate Status Protocol (OCSP) ([Snowflake documentation for OCSP](https://docs.snowflake.com/en/user-guide/ocsp))
+
+Therefore all network reachability requirements we documents for OCSP endpoints, also apply here, only difference that instead of the OCSP endpoints exposed by the certificate authorities,
+their CRL endpoints must be reachable.
+
+Both methods operate on port 80, unencrypted HTTP. Click [here](https://community.snowflake.com/s/article/Why-do-OCSP-use-Port-80-and-not-Port-443) if you wish to read more about the reasons.
+The certificate authorities operating their CRL/OCSP services over unencrypted HTTP does not introduce any security risk in itself, because no customer data is actually transmitted unencrypted.
+Only the data necessary for verifying certificates, which is publicly available information.
+
+Without the CRL endpoints being properly reachable for the .NET driver, it won't be able to verify certificate validity, and attempting to connect to Snowflake-related endpoints might suffer unnecessary delays, and eventually time out or break with an error:
+```c#
+error:System.Security.Authentication.AuthenticationException:
+The remote certificate is invalid because of errors in the certificate chain: RevocationStatusUnknown, OfflineRevocation
+```
+
+:warning: Above error message can be also a result of a proxy or other network appliance performing SSL inspection on the connection made to Snowflake. If such an appliance injects its own CA certificate into the chain of trust, the driver won’t be able to properly verify it using the public CRL endpoints.
+
+### What are the relevant CRL endpoints
+These endpoints are not managed or operated by Snowflake - they are part of the public key infrastructure and are provided and managed by the various certificate authorities.
+They can be extracted from the particular certificate which is part of the chain upon connecting to the endpoint (Snowflake, cloud storage).
+
+Many methods are available to read and parse certificates and extract various information from them using the different programming languages.
+We'll demonstrate a simple shell script method using the toolkit generally available on Linux and Mac hosts.
+For Windows, the exact same tools are also available, but possibly you'll need to download them first if you decide to use this same method for verification - mostly `openssl` and `curl` or `wget`.
+
+1. gather relevant Snowflake endpoints, for which you'll collect the CRL endpoints
+   Issue `SELECT SYSTEM$ALLOWLIST()` (or `SYSTEM$ALLOWLIST_PRIVATELINK()` in a privatelink setup), and take a note of hosts for
+* `SNOWFLAKE_DEPLOYMENT`
+* `STAGE`
+
+2. using the hostnames from 1., retrieve the certificate chain associated with those hostnames, and the relevant CRL endpoints for those server and intermediate certificates
+   Again, this is just merely an example using shell scripting and `openssl`.
+```shell
+export hostname="myaccount.eu-central-1.snowflakecomputing.com"
+echo | openssl s_client -showcerts -connect "$hostname":443 -servername "$hostname" 2>/dev/null | awk '/BEGIN/,/END/{ if(/BEGIN/){a++}; out="cert"a".pem"; print >out}'; for cert in cert*.pem; do echo "--> $cert"; openssl x509 -text -in $cert | grep -A4 "X509v3 CRL Distribution Points" ; echo; done
+```
+
+Example output:
+```shell
+--> cert1.pem
+            X509v3 CRL Distribution Points:
+
+                Full Name:
+                  URI:http://www.microsoft.com/pkiops/crl/Microsoft%20Azure%20RSA%20TLS%20Issuing%20CA%2007.crl
+
+
+--> cert2.pem
+            X509v3 CRL Distribution Points:
+
+                Full Name:
+                  URI:http://crl3.digicert.com/DigiCertGlobalRootG2.crl
+
+
+--> cert3.pem
+            X509v3 CRL Distribution Points:
+
+                Full Name:
+                  URI:http://crl.pki.goog/gsr1/gsr1.crl
+```
+
+Please repeat the same for all the Snowflake-related endpoints from step 1.
+
+3. **Ensure (or work with your systems / network / cloud team to ensure) the CRL endpoints from step 2 are reachable from the _same host/network, over port 80_, on which host/network your application is running, which application is using the Snowflake .NET driver**
+
+If your network includes any proxies through which the connection is sent, do make sure those proxies allow the connectivity to the CRL endpoints over port 80.
+If those proxies also employ **SSL inspection technologies**, you'll need to configure them to **bypass the Snowflake-related endpoints** and not use SSL inspection, as [it is not supported in Snowflake](https://docs.snowflake.com/en/user-guide/client-connectivity-troubleshooting/common-issues#firewall-or-proxy-ssl-inspection-issues).
+
+### Verifying connectivity to CRL endpoints
+After retrieving the relevant CRL endpoints and configuring your environment to allow the revocation lists to be retrieved over port 80,
+you can verify the reachability by simply trying to download the CRL from them.
+**It is important to conduct the tests from the same host/network where the .NET driver will be used**.
+
+Using the last CRL as an example from the above output:
+
+-using `curl`:
+```shell
+curl -O http://crl.pki.goog/gsr1/gsr1.crl
+# same with using a proxy:
+curl -x "http://@my.pro.xy:8080" http://crl.pki.goog/gsr1/gsr1.crl
+```
+-using `wget`:
+```shell
+wget http://crl.pki.goog/gsr1/gsr1.crl
+# same with using a proxy:
+wget -e http_proxy=my.pro.xy:8080 http://crl.pki.goog/gsr1/gsr1.crl
+```
+
+Expected result is for the file to be successfully downloaded to the local filesystem.
+Please make sure to **use the same proxy settings** as you will be using in your environment with the .NET driver.
+
+### (break-glass) turning off CRL validation temporarily
+**Snowflake does not recommend using it as a permanent configuration**, but for short periods of testing or perhaps getting out of a blocker situation, one can configure the .NET driver to not use CRL verification.
+The steps are outlined in the [How To: Turn Off OCSP Checking in Snowflake Client Drivers](https://community.snowflake.com/s/article/How-to-turn-off-OCSP-checking-in-Snowflake-client-drivers) article, and basically is adding `insecuremode=true` to your connection string.
+
+### DigiCert Global Root G2 certificate authority (CA) TLS certificate updates
+This might or might not affect your installation. Since the .NET driver doesn't come with its own truststore, it depends on the system's own truststore,
+which (hopefully) already includes all the root certificates needing to verify the chain of trust for connecting to Snowflake services.
+If your installation is very old, this might not be the case. Please give the [FAQ: DigiCert Global Root G2 certificate authority (CA) TLS certificate updates](https://community.snowflake.com/s/article/check-impact-from-digicert-g2-certificate-update) article a read
+on the background and possibly necessary steps.