From 0635d8a348f6d68a047270a0c4ed8318dfe7f5c4 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-hx Date: Fri, 20 Oct 2023 20:23:19 -0700 Subject: [PATCH 1/3] skip OCSP retry on host reached timeout for previous certificate entry --- deps/curl-8.1.2/lib/vtls/sf_ocsp.c | 95 ++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 17 deletions(-) diff --git a/deps/curl-8.1.2/lib/vtls/sf_ocsp.c b/deps/curl-8.1.2/lib/vtls/sf_ocsp.c index f81ad8278e..80703c5d20 100644 --- a/deps/curl-8.1.2/lib/vtls/sf_ocsp.c +++ b/deps/curl-8.1.2/lib/vtls/sf_ocsp.c @@ -71,6 +71,11 @@ typedef pthread_mutex_t SF_MUTEX_HANDLE; #include "memdebug.h" #include "sf_ocsp_telemetry_data.h" +#ifdef _WIN32 +#include +#define strcasecmp _stricmp +#endif + #define DEFAULT_OCSP_RESPONSE_CACHE_HOST "http://ocsp.snowflakecomputing.com" #define OCSP_RESPONSE_CACHE_JSON "ocsp_response_cache.json" #define OCSP_RESPONSE_CACHE_URL "%s/%s" @@ -151,15 +156,17 @@ static OCSP_RESPONSE * findOCSPRespInMem(OCSP_CERTID *certid, struct Curl_easy *data, SF_OTD *ocsp_log_data); static OCSP_RESPONSE * getOCSPResponse(X509 *cert, X509 *issuer, - struct connectdata *conn, struct Curl_easy *data, - SF_FAILOPEN_STATUS ocsp_fail_open, SF_OTD *ocsp_log); + struct connectdata *conn, struct Curl_easy *data, + SF_FAILOPEN_STATUS ocsp_fail_open, SF_OTD *ocsp_log, + char *last_timeout_host); static CURLcode checkOneCert(X509 *cert, X509 *issuer, STACK_OF(X509) *ch, X509_STORE *st, SF_FAILOPEN_STATUS ocsp_fail_open, struct connectdata *conn, - struct Curl_easy *data, + struct Curl_easy *data, SF_OTD *ocsp_log_data, - bool oob_enable); + bool oob_enable, + char *last_timeout_host); static char* ensureCacheDir(char* cache_dir, struct Curl_easy* data); static char* mkdirIfNotExists(char* dir, struct Curl_easy* data); static void writeOCSPCacheFile(struct Curl_easy* data); @@ -167,10 +174,12 @@ static void readOCSPCacheFile(struct Curl_easy* data, SF_OTD *ocsp_log_data); static OCSP_RESPONSE * queryResponderUsingCurl(char *url, OCSP_CERTID *certid, char *hostname, OCSP_REQUEST *req, struct Curl_easy *data, - SF_FAILOPEN_STATUS ocsp_fail_open, - SF_OTD *ocsp_log_data); + SF_FAILOPEN_STATUS ocsp_fail_open, + SF_OTD *ocsp_log_data, + char *last_timeout_host +); static void initOCSPCacheServer(struct Curl_easy *data); -static void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data); +static void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data, char *last_timeout_host); static char* encodeOCSPCertIDToBase64(OCSP_CERTID *certid, struct Curl_easy *data); static char* encodeOCSPRequestToBase64(OCSP_REQUEST *reqp, struct Curl_easy *data); static char* encodeOCSPResponseToBase64(OCSP_RESPONSE* resp, struct Curl_easy *data); @@ -632,9 +641,10 @@ static char * getOCSPPostReqData(const char *hname, } static OCSP_RESPONSE * queryResponderUsingCurl(char *url, OCSP_CERTID *certid, char *hostname, - OCSP_REQUEST *req, struct Curl_easy *data, - SF_FAILOPEN_STATUS ocsp_fail_open, - SF_OTD *ocsp_log_data) + OCSP_REQUEST *req, struct Curl_easy *data, + SF_FAILOPEN_STATUS ocsp_fail_open, + SF_OTD *ocsp_log_data, + char *last_timeout_host) { unsigned char *ocsp_req_der = NULL; int len_ocsp_req_der = 0; @@ -753,6 +763,21 @@ static OCSP_RESPONSE * queryResponderUsingCurl(char *url, OCSP_CERTID *certid, c infof(data, "OCSP fetch URL: %s", urlbuf); + int url_parse_result; + + url_parse_result = OCSP_parse_url( + urlbuf, &host, &port, &path, &use_ssl); + if (!url_parse_result) + { + failf(data, "Invalid OCSP fetch URL: %s", urlbuf); + goto end; + } + if (strcasecmp(host, last_timeout_host) == 0) + { + // skip if we got timeout on the same host for previous certificate entry + goto end; + } + /* allocate another curl handle for ocsp checking */ ocsp_curl = curl_easy_init(); @@ -805,6 +830,11 @@ static OCSP_RESPONSE * queryResponderUsingCurl(char *url, OCSP_CERTID *certid, c curl_easy_strerror(res)); if (ocsp_retry_cnt == max_retry -1) { + if (CURLE_OPERATION_TIMEDOUT == res) + { + failf(data, "Timeout reached when fetching OCSP response."); + strcpy(last_timeout_host, host); + } snprintf(error_msg, OCSP_TELEMETRY_ERROR_MSG_MAX_LEN, "OCSP checking curl_easy_perform() failed: %s\n", curl_easy_strerror(res)); @@ -863,6 +893,9 @@ static OCSP_RESPONSE * queryResponderUsingCurl(char *url, OCSP_CERTID *certid, c if (cert_id_b64) curl_free(cert_id_b64); if (ocsp_req_der) OPENSSL_free(ocsp_req_der); if (ocsp_curl) curl_easy_cleanup(ocsp_curl); + if (host) OPENSSL_free(host); + if (port) OPENSSL_free(port); + if (path) OPENSSL_free(path); free(ocsp_response_raw.memory_ptr); @@ -1403,7 +1436,7 @@ void updateCacheWithBulkEntries(cJSON* tmp_cache, struct Curl_easy *data) * Download a OCSP cache content from OCSP cache server. * @param data curl handle */ -void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data) +void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data, char *last_timeout_host) { struct curl_memory_write ocsp_response_cache_json_mem; CURL *curlh = NULL; @@ -1417,6 +1450,22 @@ void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data) ocsp_response_cache_json_mem.memory_ptr = malloc(1); /* will be grown as needed by the realloc above */ ocsp_response_cache_json_mem.size = 0; + int use_ssl; + int url_parse_result; + char *host = NULL, *port = NULL, *path = NULL; + + url_parse_result = OCSP_parse_url( + ocsp_cache_server_url, &host, &port, &path, &use_ssl); + if (!url_parse_result) + { + failf(data, "Invalid OCSP cache server URL: %s", ocsp_cache_server_url); + goto end; + } + if (strcasecmp(host, last_timeout_host) == 0) + { + // skip if we got timeout on the same host for previous certificate entry + goto end; + } /* allocate another curl handle for ocsp checking */ curlh = curl_easy_init(); @@ -1483,6 +1532,11 @@ void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data) else { failf(data, "CURL Response code is not CURLE_OK."); + if (CURLE_OPERATION_TIMEDOUT == res) + { + failf(data, "Timeout reached when downloading OCSP cache."); + strcpy(last_timeout_host, host); + } sf_otd_set_event_sub_type(OCSP_RESPONSE_CURL_FAILURE, ocsp_log_data); goto end; } @@ -1504,6 +1558,9 @@ void downloadOCSPCache(struct Curl_easy *data, SF_OTD *ocsp_log_data) if (tmp_cache) cJSON_Delete(tmp_cache); if (curlh) curl_easy_cleanup(curlh); if (headers) curl_slist_free_all(headers); + if (host) OPENSSL_free(host); + if (port) OPENSSL_free(port); + if (path) OPENSSL_free(path); free(ocsp_response_cache_json_mem.memory_ptr); } @@ -1521,7 +1578,8 @@ OCSP_RESPONSE * getOCSPResponse(X509 *cert, X509 *issuer, struct connectdata *conn, struct Curl_easy *data, SF_FAILOPEN_STATUS ocsp_fail_open, - SF_OTD *ocsp_log_data) + SF_OTD *ocsp_log_data, + char *last_timeout_host) { int i; bool ocsp_url_missing = true; @@ -1546,7 +1604,7 @@ OCSP_RESPONSE * getOCSPResponse(X509 *cert, X509 *issuer, { sf_otd_set_cache_enabled(1, ocsp_log_data); /* download OCSP Cache from the server*/ - downloadOCSPCache(data, ocsp_log_data); + downloadOCSPCache(data, ocsp_log_data, last_timeout_host); /* try hitting cache again */ resp = findOCSPRespInMem(certid, data, ocsp_log_data); @@ -1603,7 +1661,7 @@ OCSP_RESPONSE * getOCSPResponse(X509 *cert, X509 *issuer, } ocsp_url_invalid = false; - resp = queryResponderUsingCurl(ocsp_url, certid, conn->host.name, req, data, ocsp_fail_open, ocsp_log_data); + resp = queryResponderUsingCurl(ocsp_url, certid, conn->host.name, req, data, ocsp_fail_open, ocsp_log_data, last_timeout_host); /* update local cache */ OPENSSL_free(host); OPENSSL_free(path); @@ -1690,7 +1748,8 @@ CURLcode checkOneCert(X509 *cert, X509 *issuer, struct connectdata *conn, struct Curl_easy *data, SF_OTD *ocsp_log_data, - bool oob_enable) + bool oob_enable, + char *last_timeout_host) { CURLcode result; SF_CERT_STATUS sf_cert_status = CERT_STATUS_INVALID; @@ -1707,7 +1766,7 @@ CURLcode checkOneCert(X509 *cert, X509 *issuer, { OCSP_CERTID *certid = NULL; int ocsp_status = 0; - resp = getOCSPResponse(cert, issuer, conn, data, ocsp_fail_open, ocsp_log_data); + resp = getOCSPResponse(cert, issuer, conn, data, ocsp_fail_open, ocsp_log_data, last_timeout_host); infof(data, "Starting checking cert for %s...", X509_NAME_oneline(X509_get_subject_name(cert), X509_cert_name, MAX_CERT_NAME_LEN)); @@ -2298,6 +2357,8 @@ SF_PUBLIC(CURLcode) checkCertOCSP(struct connectdata *conn, CURLcode rs = CURLE_OK; char *ocsp_fail_open_env = getenv("SF_OCSP_FAIL_OPEN"); // Testing Only SF_FAILOPEN_STATUS ocsp_fail_open = ENABLED; + char last_timeout_host[MAX_BUFFER_LENGTH]; + last_timeout_host[0] = '\0'; // These end points are Out of band telemetry end points. @@ -2351,7 +2412,7 @@ SF_PUBLIC(CURLcode) checkCertOCSP(struct connectdata *conn, X509* cert = sk_X509_value(ch, i); X509* issuer = sk_X509_value(ch, i+1); - rs = checkOneCert(cert, issuer, ch, st, ocsp_fail_open, conn, data, &ocsp_log_data, oob_enable); + rs = checkOneCert(cert, issuer, ch, st, ocsp_fail_open, conn, data, &ocsp_log_data, oob_enable, last_timeout_host); if (rs != CURLE_OK) { goto end; From 43b92ac1ee3922e36b1a2ccdefde13d066dde3cf Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-hx Date: Mon, 23 Oct 2023 10:27:41 -0700 Subject: [PATCH 2/3] add test case --- tests/unit_test_ocsp/test_ocsp.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/unit_test_ocsp/test_ocsp.c b/tests/unit_test_ocsp/test_ocsp.c index 092408e877..2845959ff0 100644 --- a/tests/unit_test_ocsp/test_ocsp.c +++ b/tests/unit_test_ocsp/test_ocsp.c @@ -317,12 +317,25 @@ int main(int argc, char **argv) unlink(cache_file); checkCertificateRevocationStatus(host, port, cacert, NULL, NULL, 1, 1); - printf("===> Case 10: Delete file cache with invalid cache server URL to test OOB disabled\n"); + printf("===> Case 10: Delete file cache with invalid cache server URL to test delay on failure and OOB disabled\n"); setenv("SF_OCSP_RESPONSE_CACHE_SERVER_ENABLED", "false", 1); // use random IP address so it will get connection timeout setenv("SF_OCSP_RESPONSE_CACHE_SERVER_URL", "http://10.24.123.89/ocsp_response_cache.json", 1); unlink(cache_file); + + time_t start_time = time(NULL); checkCertificateRevocationStatus(host, port, cacert, NULL, NULL, 0, 1); + time_t end_time = time(NULL); + // should be around 5 seconds but no longer than 10. + if ((end_time - start_time) > 10) + { + fprintf(stderr, "Delay check FAILED! Delayed %d seconds\n", end_time - start_time); + exit(1); + } + else + { + fprintf(stderr, "Delay check OK\n"); + } unsetenv("SF_OCSP_RESPONSE_CACHE_SERVER_ENABLED"); unsetenv("SF_OCSP_RESPONSE_CACHE_SERVER_URL"); From fed93f6e96dea003d162f9f69943e7bbb9f7d311 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-hx Date: Mon, 23 Oct 2023 10:30:08 -0700 Subject: [PATCH 3/3] bump up curl build version number for the fix --- scripts/build_curl.bat | 2 +- scripts/build_curl.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/build_curl.bat b/scripts/build_curl.bat index 20fac89347..7988f1b811 100644 --- a/scripts/build_curl.bat +++ b/scripts/build_curl.bat @@ -11,7 +11,7 @@ @echo off set CURL_SRC_VERSION=8.1.2 -set CURL_BUILD_VERSION=3 +set CURL_BUILD_VERSION=4 set CURL_VERSION=%CURL_SRC_VERSION%.%CURL_BUILD_VERSION% call %* goto :EOF diff --git a/scripts/build_curl.sh b/scripts/build_curl.sh index 008cd89db8..f07535b217 100755 --- a/scripts/build_curl.sh +++ b/scripts/build_curl.sh @@ -13,7 +13,7 @@ function usage() { set -o pipefail CURL_SRC_VERSION=8.1.2 -CURL_BUILD_VERSION=3 +CURL_BUILD_VERSION=4 CURL_DIR=$CURL_SRC_VERSION CURL_VERSION=${CURL_DIR}.${CURL_BUILD_VERSION}