Added functionality to enable CRL checking for CURL on linux; added t…

…ests for this new functionality. (#3923)

# Added functionality to enable CRL checking for CURL on linux.

This one is somewhat unpleasant and much larger than expected.

This pull request enables two pieces of functionality:
1. The ability to specify a known root certificate to the CURL HTTP transport (instead of a certificate file).
2. The ability to enable CRL validation (normally this is disabled in libCURL).

Enabling CRL validation ended up pulling in a significant chunk of code from azure-c-shared-util which handled retrieving CRLs (I was unable to find code in libCURL to do this). Native LibCURL support for CRL validation is limited to the schannel SSL backend (Windows Only).

This change also adds logic to the CURL transport to enable the ability to ignore CRL retrieval errors (there doesn't seem to be a comparable way of doing this for WinHTTP so it is a CURL transport only option).

To verify the root certificate logic, an extremely simple client for the SDK Test Proxy was written and is used to "record" a request to the C++ SDK HTTP server.

eng/pipelines/templates/jobs/archetype-sdk-client.yml

@@ -33,10 +33,10 @@ parameters:
     type: object
     default: []
   - name: PreTestSteps
-    type: object
+    type: stepList
     default: []
   - name: PostTestSteps
-    type: object
+    type: stepList
     default: []
 
 jobs:

eng/pipelines/templates/jobs/ci.tests.yml

@@ -47,10 +47,10 @@ parameters:
     type: boolean
     default: false
   - name: PreTestSteps
-    type: object
+    type: stepList
     default: []
   - name: PostTestSteps
-    type: object
+    type: stepList
     default: []
 
 
@@ -145,7 +145,7 @@ jobs:
           Env: "$(CmakeEnvArg)"
 
       - ${{ parameters.PreTestSteps }}
-
+      
       - pwsh: |
           ctest `
             -C Debug `
@@ -156,7 +156,7 @@ jobs:
             -T Test
         workingDirectory: build
         displayName: Test
-
+        
       - ${{ parameters.PostTestSteps }}
 
       - task: PublishTestResults@2

eng/pipelines/templates/jobs/live.tests.yml

@@ -37,10 +37,10 @@ parameters:
   type: boolean
   default: false
 - name: PreTestSteps
-  type: object
+  type: stepList
   default: []
 - name: PostTestSteps
-  type: object
+  type: stepList
   default: []
 
 jobs:

eng/pipelines/templates/stages/archetype-sdk-client.yml

@@ -68,10 +68,10 @@ parameters:
   type: string
   default: ''
 - name: PreTestSteps
-  type: object
+  type: stepList
   default: []
 - name: PostTestSteps
-  type: object
+  type: stepList
   default: []
 
 

eng/pipelines/templates/stages/archetype-sdk-tests.yml

@@ -30,10 +30,10 @@ parameters:
   type: string
   default: ''
 - name: PreTestSteps
-  type: object
+  type: stepList
   default: []
 - name: PostTestSteps
-  type: object
+  type: stepList
   default: []
 
 stages:

sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp

@@ -38,10 +38,31 @@ namespace Azure { namespace Core { namespace Http {
      *
      * @remark Libcurl does revocation list check by default for SSL backends that supports this
      * feature. However, the Azure SDK overrides libcurl's behavior and disables the revocation list
-     * check by default.
-     *
+     * check by default. This ensures that the LibCURL behavior matches the WinHTTP behavior.
      */
     bool EnableCertificateRevocationListCheck = false;
+
+    /**
+     * @brief This option allows SSL connections to proceed even if there is an error retrieving the
+     * Certificate Revocation List.
+     *
+     * @remark Note that this only works when LibCURL is configured to use openssl as its TLS
+     * provider. That functionally limits this check to Linux only, and then only when openssl is
+     * configured (the default).
+     */
+    bool AllowFailedCrlRetrieval = false;
+
+    /**
+     * @brief A set of PEM encoded X.509 certificates and CRLs describing the certificates used to
+     * validate the server.
+     *
+     * @remark The Azure SDK will not directly validate these certificates.
+     *
+     * @remark More about this option:
+     * https://curl.haxx.se/libcurl/c/CURLOPT_CAINFO_BLOB.html
+     *
+     */
+    std::string PemEncodedExpectedRootCertificates;
   };
 
   /**
@@ -86,7 +107,8 @@ namespace Azure { namespace Core { namespace Http {
      */
     std::string ProxyPassword;
     /**
-     * @brief The string for the certificate authenticator is sent to libcurl handle directly.
+     * @brief Path to a PEM encoded file containing the certificate authorities sent to libcurl
+     * handle directly.
      *
      * @remark The Azure SDK will not check if the path is valid or not.
      *
@@ -95,6 +117,7 @@ namespace Azure { namespace Core { namespace Http {
      *
      */
     std::string CAInfo;
+
     /**
      * @brief All HTTP requests will keep the connection channel open to the service.
      *
@@ -106,6 +129,7 @@ namespace Azure { namespace Core { namespace Http {
      * handle. It is `true` by default.
      */
     bool HttpKeepAlive = true;
+
     /**
      * @brief This option determines whether libcurl verifies the authenticity of the peer's
      * certificate.

sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp

@@ -144,17 +144,35 @@ namespace Azure { namespace Core { namespace Http { namespace Policies {
      *
      * @remark The URL for the proxy server to use for this connection.
      */
-    Azure::Nullable<std::string> HttpProxy;
+    Azure::Nullable<std::string> HttpProxy{};
 
     /**
      * @brief The username to use when authenticating with the proxy server.
      */
-    std::string ProxyUserName;
+    std::string ProxyUserName{};
 
     /**
      * @brief The password to use when authenticating with the proxy server.
      */
-    std::string ProxyPassword;
+    std::string ProxyPassword{};
+
+    /**
+     * @brief Enable TLS Certificate validation against a certificate revocation list.
+     *
+     * @remark Note that by default CRL validation is *disabled*.
+     */
+    bool EnableCertificateRevocationListCheck{false};
+
+    /**
+     * @brief Base64 encoded DER representation of an X.509 certificate expected in the certificate
+     * chain used in TLS connections.
+     *
+     * @remark Note that with the schannel and sectransp crypto backends, settting the
+     * expected root certificate disables access to the system certificate store.
+     * This means that the expected root certificate is the only certificate that will be trusted.
+     */
+    std::string ExpectedTlsRootCertificate{};
+
 #endif // defined(CURL_ADAPTER) || defined(WINHTTP_ADAPTER)
 #endif // !defined(BUILD_TRANSPORT_CUSTOM_ADAPTER)
 

sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp

@@ -4,6 +4,7 @@
 /**
  * @file
  * @brief #Azure::Core::Http::HttpTransport implementation via WinHTTP.
+ * cspell:words HCERTIFICATECHAIN PCCERT CCERT HCERTCHAINENGINE HCERTSTORE
  */
 
 #pragma once
@@ -22,11 +23,13 @@
 #define NOMINMAX
 #endif
 #include <windows.h>
+
 #endif
 
 #include <memory>
 #include <type_traits>
 #include <vector>
+#include <wincrypt.h>
 #include <winhttp.h>
 
 namespace Azure { namespace Core { namespace Http {
@@ -49,6 +52,61 @@ namespace Azure { namespace Core { namespace Http {
     };
     using unique_HINTERNET = std::unique_ptr<void, HINTERNET_deleter>;
 
+    // unique_ptr class wrapping a PCCERT_CHAIN_CONTEXT
+    struct HCERTIFICATECHAIN_deleter
+    {
+      void operator()(PCCERT_CHAIN_CONTEXT handle)
+      {
+        // unique_ptr class wrapping an HINTERNET handle
+        {
+          CertFreeCertificateChain(handle);
+        }
+      }
+    };
+    using unique_CCERT_CHAIN_CONTEXT
+        = std::unique_ptr<const CERT_CHAIN_CONTEXT, HCERTIFICATECHAIN_deleter>;
+
+    // unique_ptr class wrapping an HCERTCHAINENGINE handle
+    struct HCERTCHAINENGINE_deleter
+    {
+      void operator()(HCERTCHAINENGINE handle) noexcept
+      {
+        if (handle != nullptr)
+        {
+          CertFreeCertificateChainEngine(handle);
+        }
+      }
+    };
+    using unique_HCERTCHAINENGINE = std::unique_ptr<void, HCERTCHAINENGINE_deleter>;
+
+    // unique_ptr class wrapping an HCERTSTORE handle
+    struct HCERTSTORE_deleter
+    {
+    public:
+      void operator()(HCERTSTORE handle) noexcept
+      {
+        if (handle != nullptr)
+        {
+          CertCloseStore(handle, 0);
+        }
+      }
+    };
+    using unique_HCERTSTORE = std::unique_ptr<void, HCERTSTORE_deleter>;
+
+    // unique_ptr class wrapping a PCCERT_CONTEXT
+    struct CERTCONTEXT_deleter
+    {
+    public:
+      void operator()(PCCERT_CONTEXT handle) noexcept
+      {
+        if (handle != nullptr)
+        {
+          CertFreeCertificateContext(handle);
+        }
+      }
+    };
+    using unique_PCCERT_CONTEXT = std::unique_ptr<CERT_CONTEXT const, CERTCONTEXT_deleter>;
+
     class WinHttpStream final : public Azure::Core::IO::BodyStream {
     private:
       _detail::unique_HINTERNET m_requestHandle;
@@ -122,6 +180,11 @@ namespace Azure { namespace Core { namespace Http {
      */
     bool EnableSystemDefaultProxy{false};
 
+    /**
+     * @brief If True, enables checks for certificate revocation.
+     */
+    bool EnableCertificateRevocationListCheck{false};
+
     /**
      * @brief Proxy information.
      *
@@ -142,6 +205,13 @@ namespace Azure { namespace Core { namespace Http {
      * @brief Password for proxy authentication.
      */
     std::string ProxyPassword;
+
+    /**
+     * @brief Array of Base64 encoded DER encoded X.509 certificate.  These certificates should form
+     * a chain of certificates which will be used to validate the server certificate sent by the
+     * server.
+     */
+    std::vector<std::string> ExpectedTlsRootCertificates;
   };
 
   /**
@@ -155,6 +225,7 @@ namespace Azure { namespace Core { namespace Http {
     // This should remain immutable and not be modified after calling the ctor, to avoid threading
     // issues.
     _detail::unique_HINTERNET m_sessionHandle;
+    bool m_requestHandleClosed{false};
 
     _detail::unique_HINTERNET CreateSessionHandle();
     _detail::unique_HINTERNET CreateConnectionHandle(
@@ -183,6 +254,34 @@ namespace Azure { namespace Core { namespace Http {
         _detail::unique_HINTERNET& requestHandle,
         HttpMethod requestMethod);
 
+    /*
+     * Callback from WinHTTP called after the TLS certificates are received when the caller sets
+     * expected TLS root certificates.
+     */
+    static void CALLBACK StatusCallback(
+        HINTERNET hInternet,
+        DWORD_PTR dwContext,
+        DWORD dwInternetStatus,
+        LPVOID lpvStatusInformation,
+        DWORD dwStatusInformationLength) noexcept;
+    /*
+     * Callback from WinHTTP called after the TLS certificates are received when the caller sets
+     * expected TLS root certificates.
+     */
+    void OnHttpStatusOperation(HINTERNET hInternet, DWORD dwInternetStatus);
+    /*
+     * Adds the specified trusted certificates to the specified certificate store.
+     */
+    bool AddCertificatesToStore(
+        std::vector<std::string> const& trustedCertificates,
+        _detail::unique_HCERTSTORE const& hCertStore);
+    /*
+     * Verifies that the certificate context is in the trustedCertificates set of certificates.
+     */
+    bool VerifyCertificatesInChain(
+        std::vector<std::string> const& trustedCertificates,
+        _detail::unique_PCCERT_CONTEXT const& serverCertificate);
+
     // Callback to allow a derived transport to extract the request handle. Used for WebSocket
     // transports.
   protected:

sdk/core/azure-core/inc/azure/core/platform.hpp

@@ -47,4 +47,10 @@
 #define AZ_PLATFORM_WINDOWS
 #elif defined(__unix__) || defined(__unix) || (defined(__APPLE__) && defined(__MACH__))
 #define AZ_PLATFORM_POSIX
+#if defined(__APPLE__) || defined(__MACH__)
+#define AZ_PLATFORM_MAC
+#else
+#define AZ_PLATFORM_LINUX
+#endif
+
 #endif