From cc346678dce96a3cdb70ccebf229ac215251bd7f Mon Sep 17 00:00:00 2001 From: Remi Tricot-Le Breton Date: Tue, 20 Dec 2022 11:11:08 +0100 Subject: [PATCH] MEDIUM: ssl: Add ocsp_certid in ckch structure and discard ocsp buffer early The ocsp_response member of the cert_key_and_chain structure is only used temporarily. During a standard init process where an ocsp response is provided, this ocsp file is first copied into the ocsp_response buffer without any ocsp-related parsing (see ssl_sock_load_ocsp_response_from_file), and then the contents are actually interpreted and inserted into the actual ocsp tree (cert_ocsp_tree) later in the process (see ssl_sock_load_ocsp). If the response was deemed valid, it is then copied into the actual ocsp_response structure's 'response' field (see ssl_sock_load_ocsp_response). From this point, the ocsp_response field of the cert_key_and_chain object could be discarded since actual ocsp operations will be based of the certificate_ocsp object. The only remaining runtime use of the ckch's ocsp_response field was in the CLI, and more precisely in the 'show ssl cert' mechanism. This constraint could be removed by adding an OCSP_CERTID directly in the ckch because the buffer was only used to get this id. This patch then adds the OCSP_CERTID pointer in the ckch, it clears the ocsp_response buffer early and simplifies the ckch_store_build_certid function. --- include/haproxy/ssl_ckch-t.h | 1 + src/ssl_ckch.c | 38 ++++++++++++------------------------ src/ssl_sock.c | 26 +++++++++++++----------- 3 files changed, 28 insertions(+), 37 deletions(-) diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h index 973d73b236..eba0b1a368 100644 --- a/include/haproxy/ssl_ckch-t.h +++ b/include/haproxy/ssl_ckch-t.h @@ -54,6 +54,7 @@ struct ckch_data { struct buffer *sctl; struct buffer *ocsp_response; X509 *ocsp_issuer; + OCSP_CERTID *ocsp_cid; }; /* diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index 7a6168a019..f8e63edc33 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -719,6 +719,9 @@ void ssl_sock_free_cert_key_and_chain_contents(struct ckch_data *data) if (data->ocsp_issuer) X509_free(data->ocsp_issuer); data->ocsp_issuer = NULL; + + OCSP_CERTID_free(data->ocsp_cid); + data->ocsp_cid = NULL; } /* @@ -788,6 +791,8 @@ struct ckch_data *ssl_sock_copy_cert_key_and_chain(struct ckch_data *src, dst->ocsp_issuer = src->ocsp_issuer; } + dst->ocsp_cid = OCSP_CERTID_dup(src->ocsp_cid); + return dst; error: @@ -1760,44 +1765,25 @@ end: * Build the OCSP tree entry's key for a given ckch_store. * Returns a negative value in case of error. */ -static int ckch_store_build_certid(struct ckch_store *ckch_store, unsigned char certid[128], unsigned int *key_length) +static int ckch_store_build_certid(struct ckch_store *ckch_store, unsigned char certid[OCSP_MAX_CERTID_ASN1_LENGTH], unsigned int *key_length) { - OCSP_RESPONSE *resp; - OCSP_BASICRESP *bs = NULL; - OCSP_SINGLERESP *sr; - OCSP_CERTID *id; unsigned char *p = NULL; + int i; if (!key_length) return -1; *key_length = 0; - if (!ckch_store->data->ocsp_response) + if (!ckch_store->data->ocsp_cid) return 0; - p = (unsigned char *) ckch_store->data->ocsp_response->area; - - resp = d2i_OCSP_RESPONSE(NULL, (const unsigned char **)&p, - ckch_store->data->ocsp_response->data); - if (!resp) { - goto end; - } - - bs = OCSP_response_get1_basic(resp); - if (!bs) { - goto end; - } - - sr = OCSP_resp_get0(bs, 0); - if (!sr) { - goto end; - } - - id = (OCSP_CERTID*)OCSP_SINGLERESP_get0_id(sr); + i = i2d_OCSP_CERTID(ckch_store->data->ocsp_cid, NULL); + if (!i || (i > OCSP_MAX_CERTID_ASN1_LENGTH)) + return 0; p = certid; - *key_length = i2d_OCSP_CERTID(id, &p); + *key_length = i2d_OCSP_CERTID(ckch_store->data->ocsp_cid, &p); end: return *key_length > 0; diff --git a/src/ssl_sock.c b/src/ssl_sock.c index d7c2430b72..aed823c736 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1650,10 +1650,9 @@ static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp) * Returns 1 if no ".ocsp" file found, 0 if OCSP status extension is * successfully enabled, or -1 in other error case. */ -static int ssl_sock_load_ocsp(SSL_CTX *ctx, const struct ckch_data *data, STACK_OF(X509) *chain) +static int ssl_sock_load_ocsp(SSL_CTX *ctx, struct ckch_data *data, STACK_OF(X509) *chain) { X509 *x, *issuer; - OCSP_CERTID *cid = NULL; int i, ret = -1; struct certificate_ocsp *ocsp = NULL, *iocsp; char *warn = NULL; @@ -1684,11 +1683,11 @@ static int ssl_sock_load_ocsp(SSL_CTX *ctx, const struct ckch_data *data, STACK_ if (!issuer) goto out; - cid = OCSP_cert_to_id(0, x, issuer); - if (!cid) + data->ocsp_cid = OCSP_cert_to_id(0, x, issuer); + if (!data->ocsp_cid) goto out; - i = i2d_OCSP_CERTID(cid, NULL); + i = i2d_OCSP_CERTID(data->ocsp_cid, NULL); if (!i || (i > OCSP_MAX_CERTID_ASN1_LENGTH)) goto out; @@ -1697,7 +1696,7 @@ static int ssl_sock_load_ocsp(SSL_CTX *ctx, const struct ckch_data *data, STACK_ goto out; p = ocsp->key_data; - ocsp->key_length = i2d_OCSP_CERTID(cid, &p); + ocsp->key_length = i2d_OCSP_CERTID(data->ocsp_cid, &p); HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); iocsp = (struct certificate_ocsp *)ebmb_insert(&cert_ocsp_tree, &ocsp->key, OCSP_MAX_CERTID_ASN1_LENGTH); @@ -1768,14 +1767,19 @@ static int ssl_sock_load_ocsp(SSL_CTX *ctx, const struct ckch_data *data, STACK_ ret = 0; warn = NULL; - if (ssl_sock_load_ocsp_response(data->ocsp_response, iocsp, cid, &warn)) { + if (ssl_sock_load_ocsp_response(data->ocsp_response, iocsp, data->ocsp_cid, &warn)) { memprintf(&warn, "Loading: %s. Content will be ignored", warn ? warn : "failure"); ha_warning("%s.\n", warn); } out: - if (cid) - OCSP_CERTID_free(cid); + if (ret && data->ocsp_cid) + OCSP_CERTID_free(data->ocsp_cid); + + if (!ret && data->ocsp_response) { + ha_free(&data->ocsp_response->area); + ha_free(&data->ocsp_response); + } if (ocsp) ssl_sock_free_ocsp(ocsp); @@ -1788,7 +1792,7 @@ out: #endif #ifdef OPENSSL_IS_BORINGSSL -static int ssl_sock_load_ocsp(SSL_CTX *ctx, const struct ckch_data *data, STACK_OF(X509) *chain) +static int ssl_sock_load_ocsp(SSL_CTX *ctx, struct ckch_data *data, STACK_OF(X509) *chain) { return SSL_CTX_set_ocsp_response(ctx, (const uint8_t *)ckch->ocsp_response->area, ckch->ocsp_response->data); } @@ -3909,7 +3913,7 @@ end: * The value 0 means there is no error nor warning and * the operation succeed. */ -static int ssl_sock_put_ckch_into_ctx(const char *path, const struct ckch_data *data, SSL_CTX *ctx, char **err) +static int ssl_sock_put_ckch_into_ctx(const char *path, struct ckch_data *data, SSL_CTX *ctx, char **err) { int errcode = 0; STACK_OF(X509) *find_chain = NULL; -- 2.39.5