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.

diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h
index 973d73b23616ff8715d73a692445e310656cadee..eba0b1a368ab88c8598833d584132d5e28459533 100644 (file)
--- 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 7a6168a0196a633ea718658a9238802e67e67728..f8e63edc33d4f9a16572ff4f0b885c88a1025e33 100644 (file)
--- 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 d7c2430b726d126f4aa8fdf5b21b8073caa1a109..aed823c73619b912b9fbdda8f984d13e4375cf99 100644 (file)
--- 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;