Merge pull request #321 from donoghuc/pick-crl

(maint) Force full cert chain in curl when validating CRLs

Author: mcdonaldseanp

curl/inc/leatherman/curl/client.hpp

@@ -350,6 +350,13 @@ namespace leatherman { namespace curl {
          */
         void set_client_cert(std::string const& client_cert, std::string const& client_key);
 
+        /**
+         * Set client SSL certificate revocation list.
+         * @param client_crl The path to the client's CRL file.
+         *        (see more: https://curl.haxx.se/libcurl/c/CURLOPT_CRLFILE.html)
+         */
+        void set_client_crl(std::string const& client_crl);
+
         /**
          * Set proxy information.
          * @param proxy String with following components [scheme]://[hostname]:[port].
@@ -394,6 +401,7 @@ namespace leatherman { namespace curl {
         std::string _ca_cert;
         std::string _client_cert;
         std::string _client_key;
+        std::string _client_crl;
         std::string _proxy;
         long _client_protocols = CURLPROTO_ALL;
 
@@ -414,6 +422,7 @@ namespace leatherman { namespace curl {
         LEATHERMAN_CURL_NO_EXPORT void set_write_callbacks(context& ctx, FILE* fp);
         LEATHERMAN_CURL_NO_EXPORT void set_client_info(context &ctx);
         LEATHERMAN_CURL_NO_EXPORT void set_ca_info(context& ctx);
+        LEATHERMAN_CURL_NO_EXPORT void set_crl_info(context& ctx);
         LEATHERMAN_CURL_NO_EXPORT void set_client_protocols(context& ctx);
         LEATHERMAN_CURL_NO_EXPORT void set_proxy_info(context& ctx);
 

curl/src/client.cc

@@ -229,6 +229,7 @@ namespace leatherman { namespace curl {
         set_timeouts(ctx);
         set_write_callbacks(ctx);
         set_ca_info(ctx);
+        set_crl_info(ctx);
         set_client_info(ctx);
         set_client_protocols(ctx);
         set_proxy_info(ctx);
@@ -274,6 +275,7 @@ namespace leatherman { namespace curl {
         set_timeouts(ctx);
         set_write_callbacks(ctx, temp_file.get_fp());
         set_ca_info(ctx);
+        set_crl_info(ctx);
         set_client_info(ctx);
         set_client_protocols(ctx);
         set_proxy_info(ctx);
@@ -307,6 +309,11 @@ namespace leatherman { namespace curl {
         _ca_cert = cert_file;
     }
 
+    void client::set_client_crl(string const& client_crl)
+    {
+        _client_crl = client_crl;
+    }
+
     void client::set_proxy(string const& proxy)
     {
         _proxy = proxy;
@@ -437,6 +444,25 @@ namespace leatherman { namespace curl {
         curl_easy_setopt_maybe(ctx, CURLOPT_SSLKEY, _client_key.c_str());
     }
 
+    void client::set_crl_info(context& ctx){
+        if (_client_crl == "") {
+            return;
+        }
+
+        curl_easy_setopt_maybe(ctx, CURLOPT_CRLFILE, _client_crl.c_str());
+
+        #ifdef CURLSSLOPT_NO_PARTIALCHAIN
+        // Curl 7.68 has a bug where it defaults to passing
+        // X509_V_FLAG_PARTIAL_CHAIN to openssl. This breaks CRL
+        // chains, since the crl logic passes
+        // X509_V_FLAG_CRL_CHECK_ALL, which requires a full chain.
+        //
+        // We disable partial chains explicitly here to work around
+        // this.
+        curl_easy_setopt_maybe(ctx, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_PARTIALCHAIN);
+        #endif
+    }
+
     void client::set_proxy_info(context &ctx) {
         if (_proxy == "") {
           return;

curl/tests/client_test.cc

@@ -251,6 +251,21 @@ TEST_CASE("curl::client CA bundle and SSL setup") {
         REQUIRE(test_impl->proxy == "proxy");
     }
 
+    SECTION("CRL should be unspecified by default") {
+        auto resp = test_client.get(test_request);
+        CURL* const& handle = test_client.get_handle();
+        auto test_impl = reinterpret_cast<curl_impl* const>(handle);
+        REQUIRE(test_impl->client_crl == "");
+    }
+
+    SECTION("cURL should receive the CRL specified in the request") {
+        test_client.set_client_crl("/tmp/foo.pem");
+        auto resp = test_client.get(test_request);
+        CURL* const& handle = test_client.get_handle();
+        auto test_impl = reinterpret_cast<curl_impl* const>(handle);
+        REQUIRE(test_impl->client_crl == "/tmp/foo.pem");
+    }
+
     SECTION("Client cert name should be unspecified by default") {
         auto resp = test_client.get(test_request);
         CURL* const& handle = test_client.get_handle();

curl/tests/mock_curl.cc

@@ -217,6 +217,14 @@ CURLcode curl_easy_setopt(CURL *handle, CURLoption option, ...)
             }
             h->client_cert = va_arg(vl, char*);
             break;
+        case CURLOPT_CRLFILE:
+            // Set the mock curl SSL CRL to that which was passed in the request.
+            if (h->test_failure_mode == curl_impl::error_mode::client_crl_error) {
+                va_end(vl);
+                return CURLE_OUT_OF_MEMORY;
+            }
+            h->client_crl = va_arg(vl, char*);
+            break;
         case CURLOPT_PROXY:
             // Set the mock curl proxy to that which was passed in the request.
             if (h->test_failure_mode == curl_impl::error_mode::proxy_error) {

curl/tests/mock_curl.hpp

@@ -40,7 +40,8 @@ struct curl_impl
         ssl_cert_error,
         ssl_key_error,
         protocol_error,
-        proxy_error
+        proxy_error,
+        client_crl_error
     };
 
     error_mode test_failure_mode = error_mode::success;
@@ -57,7 +58,7 @@ struct curl_impl
     std::function<size_t(char*, size_t, size_t, void*)> read_function;
     void* read_data; // Where to read the request body from
 
-    std::string request_url, cookie, cacert, client_cert, client_key, proxy;
+    std::string request_url, cookie, cacert, client_cert, client_key, client_crl, proxy;
     long protocols;
     long connect_timeout;
     http_method method = http_method::get;