From 4d2a61a0277d34250b07c3248a429edc3587018b Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Tue, 2 May 2023 20:18:33 -0600 Subject: [PATCH 1/6] Add a setting to make OCSP request by GET method This adds proxy.config.ssl.ocsp.request_mode. Co-authored-by: Jeff Elsloo --- doc/admin-guide/files/records.yaml.en.rst | 8 ++++++++ doc/admin-guide/security/index.en.rst | 1 + iocore/net/P_SSLConfig.h | 1 + iocore/net/SSLConfig.cc | 2 ++ src/records/RecordsConfig.cc | 3 +++ 5 files changed, 15 insertions(+) diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 0b06b3eb6aa..9d0eb087e75 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -4254,6 +4254,14 @@ OCSP Stapling Configuration Number of seconds before an OCSP response expires in the stapling cache. +.. ts:cv:: CONFIG proxy.config.ssl.ocsp.request_mode INT 0 + + Set the request method to prefer when querying OCSP responders. The default + is zero, or POST, and a value of 1 will cause ATS to attempt a GET request. + Because the length of the encoded request must be less than 255 bytes per RFC + 6960, Appendix A, ATS will fall back to the POST request method when the + encoded size exceeds this limit. + .. ts:cv:: CONFIG proxy.config.ssl.ocsp.request_timeout INT 10 :units: seconds diff --git a/doc/admin-guide/security/index.en.rst b/doc/admin-guide/security/index.en.rst index e4e1d396d7f..e4e7f382660 100644 --- a/doc/admin-guide/security/index.en.rst +++ b/doc/admin-guide/security/index.en.rst @@ -360,6 +360,7 @@ in :file:`records.yaml` file: * :ts:cv:`proxy.config.ssl.ocsp.enabled` * :ts:cv:`proxy.config.ssl.ocsp.cache_timeout` +* :ts:cv:`proxy.config.ssl.ocsp.request_mode` * :ts:cv:`proxy.config.ssl.ocsp.request_timeout` * :ts:cv:`proxy.config.ssl.ocsp.update_period` * :ts:cv:`proxy.config.ssl.ocsp.response.path` diff --git a/iocore/net/P_SSLConfig.h b/iocore/net/P_SSLConfig.h index aa9c1f9e7fa..924072341c1 100644 --- a/iocore/net/P_SSLConfig.h +++ b/iocore/net/P_SSLConfig.h @@ -130,6 +130,7 @@ struct SSLConfigParams : public ConfigInfo { static bool ssl_ocsp_enabled; static int ssl_ocsp_cache_timeout; + static bool ssl_ocsp_request_mode; static int ssl_ocsp_request_timeout; static int ssl_ocsp_update_period; static int ssl_handshake_timeout_in; diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 796432cc72f..1353a193662 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -63,6 +63,7 @@ int SSLConfigParams::ssl_misc_max_iobuffer_size_index = 8; bool SSLConfigParams::ssl_allow_client_renegotiation = false; bool SSLConfigParams::ssl_ocsp_enabled = false; int SSLConfigParams::ssl_ocsp_cache_timeout = 3600; +bool SSLConfigParams::ssl_ocsp_request_mode = false; int SSLConfigParams::ssl_ocsp_request_timeout = 10; int SSLConfigParams::ssl_ocsp_update_period = 60; char *SSLConfigParams::ssl_ocsp_user_agent = nullptr; @@ -469,6 +470,7 @@ SSLConfigParams::initialize() // SSL OCSP Stapling configurations REC_ReadConfigInt32(ssl_ocsp_enabled, "proxy.config.ssl.ocsp.enabled"); REC_EstablishStaticConfigInt32(ssl_ocsp_cache_timeout, "proxy.config.ssl.ocsp.cache_timeout"); + REC_ReadConfigInt32(ssl_ocsp_request_mode, "proxy.config.ssl.ocsp.request_mode"); REC_EstablishStaticConfigInt32(ssl_ocsp_request_timeout, "proxy.config.ssl.ocsp.request_timeout"); REC_EstablishStaticConfigInt32(ssl_ocsp_update_period, "proxy.config.ssl.ocsp.update_period"); REC_ReadConfigStringAlloc(ssl_ocsp_response_path, "proxy.config.ssl.ocsp.response.path"); diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index 182e4f6c177..d1ace70b38f 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -1207,6 +1207,9 @@ static const RecordElement RecordsConfig[] = // # Number of seconds before an OCSP response expires in the stapling cache. 3600s (1 hour) by default. {RECT_CONFIG, "proxy.config.ssl.ocsp.cache_timeout", RECD_INT, "3600", RECU_DYNAMIC, RR_NULL, RECC_NULL, "^[0-9]+$", RECA_NULL} , + // # Request method "mode" for queries to OCSP responders; 0 is POST, 1 is "prefer GET." + {RECT_CONFIG, "proxy.config.ssl.ocsp.request_mode", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} + , // # Timeout for queries to OCSP responders. 10s by default. {RECT_CONFIG, "proxy.config.ssl.ocsp.request_timeout", RECD_INT, "10", RECU_DYNAMIC, RR_NULL, RECC_NULL, "^[0-9]+$", RECA_NULL} , From f2cc4af29b586ece3504b94c1efea99362d4c018 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Wed, 3 May 2023 09:41:37 -0600 Subject: [PATCH 2/6] Implement OCSP request by GET method Co-authored-by: Jeff Elsloo --- iocore/net/OCSPStapling.cc | 156 +++++++++++++++++++++++++++++++++---- 1 file changed, 140 insertions(+), 16 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 579e730a4a9..7818ba3df97 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -27,6 +27,8 @@ #include #include "tscore/ink_memory.h" +#include "tscore/Encoding.h" +#include "tscore/ink_base64.h" #include "P_Net.h" #include "P_SSLConfig.h" #include "P_SSLUtils.h" @@ -41,6 +43,10 @@ // so 10K should be more than enough. #define MAX_STAPLING_DER 10240 +// maximum length allowed for the encoded OCSP GET request; per RFC 6960, Appendix A, the encoded request must be less than 255 +// bytes +#define MAX_OCSP_GET_ENCODED_LENGTH 255 // maximum of 254 bytes + \0 + extern ClassAllocator FetchSMAllocator; // clang-format off @@ -299,7 +305,7 @@ class HTTPRequest : public Continuation } void - set_request_line(bool use_post, const char *uri) + set_request_line(bool use_get, const char *uri) { struct sockaddr_in sin; memset(&sin, 0, sizeof(sin)); @@ -308,10 +314,10 @@ class HTTPRequest : public Continuation sin.sin_port = 65535; this->_fsm = FetchSMAllocator.alloc(); - if (use_post) { - this->_fsm->ext_init(this, "POST", uri, "HTTP/1.1", reinterpret_cast(&sin), TS_FETCH_FLAGS_SKIP_REMAP); - } else { + if (use_get) { this->_fsm->ext_init(this, "GET", uri, "HTTP/1.1", reinterpret_cast(&sin), TS_FETCH_FLAGS_SKIP_REMAP); + } else { + this->_fsm->ext_init(this, "POST", uri, "HTTP/1.1", reinterpret_cast(&sin), TS_FETCH_FLAGS_SKIP_REMAP); } } @@ -996,7 +1002,7 @@ stapling_check_response(certinfo *cinf, TS_OCSP_RESPONSE *rsp) } static TS_OCSP_RESPONSE * -query_responder(const char *uri, const char *user_agent, TS_OCSP_REQUEST *req, int req_timeout) +query_responder(const char *uri, const char *user_agent, TS_OCSP_REQUEST *req, int req_timeout, bool use_get) { ink_hrtime start, end; TS_OCSP_RESPONSE *resp = nullptr; @@ -1005,9 +1011,8 @@ query_responder(const char *uri, const char *user_agent, TS_OCSP_REQUEST *req, i end = ink_hrtime_add(start, ink_hrtime_from_sec(req_timeout)); HTTPRequest httpreq; - bool use_post = true; - httpreq.set_request_line(use_post, uri); + httpreq.set_request_line(use_get, uri); // Host header const char *host = strstr(uri, "://") + 3; @@ -1024,7 +1029,7 @@ query_responder(const char *uri, const char *user_agent, TS_OCSP_REQUEST *req, i } // Content-Type, Content-Length, Request Body - if (use_post) { + if (!use_get) { if (httpreq.set_body("application/ocsp-request", ASN1_ITEM_rptr(TS_OCSP_REQUEST), (const ASN1_VALUE *)req) != 1) { Error("failed to make a request for OCSP server; uri=%s", uri); return nullptr; @@ -1073,18 +1078,119 @@ query_responder(const char *uri, const char *user_agent, TS_OCSP_REQUEST *req, i return nullptr; } +// The default encoding table, per the RFC, does not encode any of the following: ,+/= +static const unsigned char encoding_map[32] = { + 0xFF, 0xFF, 0xFF, + 0xFF, // control + 0xB4, // space " # % + 0x19, // , + / + 0x00, // + 0x0E, // < > = + 0x00, 0x00, // + 0x00, // + 0x1E, 0x80, // [ \ ] ^ ` + 0x00, 0x00, // + 0x1F, // { | } ~ DEL + 0x00, 0x00, 0x00, + 0x00, // all non-ascii characters unmodified + 0x00, 0x00, 0x00, + 0x00, // . + 0x00, 0x00, 0x00, + 0x00, // . + 0x00, 0x00, 0x00, + 0x00 // . +}; + +static IOBufferBlock * +make_url_for_get(TS_OCSP_REQUEST *req, const char *base_url) +{ + unsigned char *ocsp_der = nullptr; + int ocsp_der_len = -1; + char ocsp_encoded_der[MAX_OCSP_GET_ENCODED_LENGTH]; // Stores base64 encoded data + size_t ocsp_encoded_der_len = 0; + char ocsp_escaped[MAX_OCSP_GET_ENCODED_LENGTH]; + int ocsp_escaped_len = -1; + IOBufferBlock *url = nullptr; + + ocsp_der_len = i2d_TS_OCSP_REQUEST(req, &ocsp_der); + + // ats_base64_encode does not insert newlines, which would need to be removed otherwise. + // When ats_base64_encode is false, the encoded length exceeds the length of our buffer, + // which is set to MAX_OCSP_GET_ENCODED_LENGTH; 255 bytes per RFC6960, Appendix A. + if (ocsp_der_len <= 0 || ocsp_der == nullptr) { + Error("stapling_refresh_response: unable to convert OCSP request to DER; falling back to POST; url=%s", base_url); + return nullptr; + } + Debug("ssl_ocsp", "converted OCSP request to DER; length=%d", ocsp_der_len); + + if (ats_base64_encode(ocsp_der, ocsp_der_len, ocsp_encoded_der, MAX_OCSP_GET_ENCODED_LENGTH, &ocsp_encoded_der_len) == false || + ocsp_encoded_der_len == 0) { + Error("stapling_refresh_response: unable to base64 encode OCSP DER; falling back to POST; url=%s", base_url); + OPENSSL_free(ocsp_der); + return nullptr; + } + OPENSSL_free(ocsp_der); + Debug("ssl_ocsp", "encoded DER with base64: %s", ocsp_encoded_der); + + if (nullptr == Encoding::pure_escapify_url(nullptr, ocsp_encoded_der, ocsp_encoded_der_len, &ocsp_escaped_len, ocsp_escaped, + MAX_OCSP_GET_ENCODED_LENGTH, encoding_map)) { + Error("stapling_refresh_response: unable to escapify encoded url; falling back to POST; url=%s", base_url); + return nullptr; + } + Debug("ssl_ocsp", "escaped encoded path; %d bytes, %s", ocsp_escaped_len, ocsp_escaped); + + size_t total_url_len = + sizeof(char) * (strlen(base_url) + 1 + ocsp_escaped_len + 1); // + / + + \0 + static uint buffer_idx = DEFAULT_SMALL_BUFFER_SIZE; // idx 2, aka BUFFER_SIZE_INDEX_512 should be enough in most cases + static uint buffer_size = BUFFER_SIZE_FOR_INDEX(buffer_idx); + + // increase buffer index as necessary to fit the largest observed encoded_path_len + while (buffer_size < total_url_len && buffer_idx < MAX_BUFFER_SIZE_INDEX) { + buffer_size = BUFFER_SIZE_FOR_INDEX(++buffer_idx); + Debug("ssl_ocsp", "increased buffer index to %d", buffer_idx); + } + + if (buffer_size < total_url_len) { + // this case should never occur; the largest index, 14, is 2MB + Error("Unable to identify a buffer index large enough to fit %zu bytes for the the request url; maximum index %d with size %d", + total_url_len, buffer_idx, buffer_size); + return nullptr; + } + + Debug("ssl_ocsp", "creating new buffer block with index %d, size %d, to store %zu encoded bytes", buffer_idx, buffer_size, + total_url_len); + url = new_IOBufferBlock(); + url->alloc(buffer_idx); + + int written = strlcpy(url->end(), base_url, url->write_avail()); + url->fill(written); + + // Append '/' if base_url does not end with it + if (url->buf()[url->size() - 1] != '/') { + strncat(url->end(), "/", 1); + url->fill(1); + } + + written = strlcat(url->end(), ocsp_escaped, url->write_avail()); + url->fill(written); + Debug("ssl_ocsp", "appended encoded data to path: %s", url->buf()); + + return url; +} + static bool stapling_refresh_response(certinfo *cinf, TS_OCSP_RESPONSE **prsp) { - bool rv = true; - TS_OCSP_REQUEST *req = nullptr; - TS_OCSP_CERTID *id = nullptr; - int response_status = 0; + bool rv = true; + TS_OCSP_REQUEST *req = nullptr; + TS_OCSP_CERTID *id = nullptr; + int response_status = 0; + IOBufferBlock *url_for_get = nullptr; + const char *url = nullptr; // Final URL to use + bool use_get = false; *prsp = nullptr; - Debug("ssl_ocsp", "stapling_refresh_response: querying responder; uri=%s", cinf->uri); - req = TS_OCSP_REQUEST_new(); if (!req) { goto err; @@ -1097,7 +1203,21 @@ stapling_refresh_response(certinfo *cinf, TS_OCSP_RESPONSE **prsp) goto err; } - *prsp = query_responder(cinf->uri, cinf->user_agent, req, SSLConfigParams::ssl_ocsp_request_timeout); + if (SSLConfigParams::ssl_ocsp_request_mode) { // True: GET, False: POST + url_for_get = make_url_for_get(req, cinf->uri); + if (url_for_get != nullptr) { + url = url_for_get->buf(); + use_get = true; + } + } + if (url == nullptr) { + // GET request is disabled or the request is too large for GET request + url = cinf->uri; + } + + Debug("ssl_ocsp", "stapling_refresh_response: querying responder; method=%s uri=%s", use_get ? "GET" : "POST", cinf->uri); + + *prsp = query_responder(url, cinf->user_agent, req, SSLConfigParams::ssl_ocsp_request_timeout, use_get); if (*prsp == nullptr) { goto err; } @@ -1107,7 +1227,8 @@ stapling_refresh_response(certinfo *cinf, TS_OCSP_RESPONSE **prsp) Debug("ssl_ocsp", "stapling_refresh_response: query response received"); stapling_check_response(cinf, *prsp); } else { - Error("stapling_refresh_response: responder response error; uri=%s response_status=%d", cinf->uri, response_status); + Error("stapling_refresh_response: responder response error; uri=%s method=%s response_status=%d", cinf->uri, + use_get ? "GET" : "POST", response_status); } if (!stapling_cache_response(*prsp, cinf)) { @@ -1128,6 +1249,9 @@ stapling_refresh_response(certinfo *cinf, TS_OCSP_RESPONSE **prsp) if (*prsp) { TS_OCSP_RESPONSE_free(*prsp); } + if (url_for_get) { + url_for_get->free(); + } return rv; } From 2a858ad512c2cb3eb2ccde5b64cf43287c43aa4a Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Tue, 29 Aug 2023 14:58:39 -0600 Subject: [PATCH 3/6] Include cstring header file --- iocore/net/OCSPStapling.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 7818ba3df97..be5787f55b3 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include "tscore/ink_memory.h" #include "tscore/Encoding.h" From 4fbdb24d11755442e4979e0826f57107def75d94 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Wed, 30 Aug 2023 15:37:32 -0600 Subject: [PATCH 4/6] Use ink_string instead --- iocore/net/OCSPStapling.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index be5787f55b3..d3f0e00bcfc 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -25,11 +25,11 @@ #include #include #include -#include #include "tscore/ink_memory.h" #include "tscore/Encoding.h" #include "tscore/ink_base64.h" +#include "tscore/ink_string.h" #include "P_Net.h" #include "P_SSLConfig.h" #include "P_SSLUtils.h" @@ -1163,7 +1163,7 @@ make_url_for_get(TS_OCSP_REQUEST *req, const char *base_url) url = new_IOBufferBlock(); url->alloc(buffer_idx); - int written = strlcpy(url->end(), base_url, url->write_avail()); + int written = ink_strlcpy(url->end(), base_url, url->write_avail()); url->fill(written); // Append '/' if base_url does not end with it @@ -1172,7 +1172,7 @@ make_url_for_get(TS_OCSP_REQUEST *req, const char *base_url) url->fill(1); } - written = strlcat(url->end(), ocsp_escaped, url->write_avail()); + written = ink_strlcat(url->end(), ocsp_escaped, url->write_avail()); url->fill(written); Debug("ssl_ocsp", "appended encoded data to path: %s", url->buf()); From 309adbbb81aafc5d3a54c1ca2c951456b3ddfb4c Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Thu, 31 Aug 2023 10:12:50 -0600 Subject: [PATCH 5/6] Remove unnecessary static --- iocore/net/OCSPStapling.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index d3f0e00bcfc..9cb81b45716 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -1142,8 +1142,8 @@ make_url_for_get(TS_OCSP_REQUEST *req, const char *base_url) size_t total_url_len = sizeof(char) * (strlen(base_url) + 1 + ocsp_escaped_len + 1); // + / + + \0 - static uint buffer_idx = DEFAULT_SMALL_BUFFER_SIZE; // idx 2, aka BUFFER_SIZE_INDEX_512 should be enough in most cases - static uint buffer_size = BUFFER_SIZE_FOR_INDEX(buffer_idx); + unsigned int buffer_idx = DEFAULT_SMALL_BUFFER_SIZE; // idx 2, aka BUFFER_SIZE_INDEX_512 should be enough in most cases + unsigned int buffer_size = BUFFER_SIZE_FOR_INDEX(buffer_idx); // increase buffer index as necessary to fit the largest observed encoded_path_len while (buffer_size < total_url_len && buffer_idx < MAX_BUFFER_SIZE_INDEX) { From fd2ecc990f0bb0762fc91a7a9f6a58c4010f4af8 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 8 Sep 2023 17:15:09 -0600 Subject: [PATCH 6/6] Remove a repeat --- iocore/net/OCSPStapling.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 9cb81b45716..375716b6c2f 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -315,11 +315,8 @@ class HTTPRequest : public Continuation sin.sin_port = 65535; this->_fsm = FetchSMAllocator.alloc(); - if (use_get) { - this->_fsm->ext_init(this, "GET", uri, "HTTP/1.1", reinterpret_cast(&sin), TS_FETCH_FLAGS_SKIP_REMAP); - } else { - this->_fsm->ext_init(this, "POST", uri, "HTTP/1.1", reinterpret_cast(&sin), TS_FETCH_FLAGS_SKIP_REMAP); - } + this->_fsm->ext_init(this, use_get ? "GET" : "POST", uri, "HTTP/1.1", reinterpret_cast(&sin), + TS_FETCH_FLAGS_SKIP_REMAP); } int