diff --git a/include/haproxy/ssl_ocsp-t.h b/include/haproxy/ssl_ocsp-t.h index 52aeb08a5..028d6fa09 100644 --- a/include/haproxy/ssl_ocsp-t.h +++ b/include/haproxy/ssl_ocsp-t.h @@ -60,7 +60,8 @@ struct certificate_ocsp { unsigned int last_update_status;/* Status of the last OCSP update */ unsigned int num_success; /* Number of successful updates */ unsigned int num_failure; /* Number of failed updates */ - unsigned int fail_count; /* Number of successive failures */ + unsigned int fail_count:31; /* Number of successive failures */ + unsigned int update_once:1; /* Set if an entry should not be reinserted into te tree after update */ char path[VAR_ARRAY]; }; diff --git a/reg-tests/ssl/ocsp_auto_update.vtc b/reg-tests/ssl/ocsp_auto_update.vtc index f6d3d788b..a1d9a3c8d 100644 --- a/reg-tests/ssl/ocsp_auto_update.vtc +++ b/reg-tests/ssl/ocsp_auto_update.vtc @@ -288,6 +288,7 @@ haproxy h4 -conf { mode http option httplog bind "127.0.0.1:12345" + http-response set-var(proc.processed) int(1) server s1 "127.0.0.1:12346" } -start @@ -319,10 +320,17 @@ shell { # Store the current "Produced At" in order to ensure that after the update # the OCSP response actually changed produced_at=$(echo "show ssl ocsp-response 303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021015" | socat "${tmpdir}/h4/stats" - | grep "Produced At") - # We should receive the OCSP response's details on the standard output when calling - # 'update ssl ocsp-response' - ocsp_response=$(echo "update ssl ocsp-response ${testdir}/ocsp_update/multicert/server_ocsp.pem.rsa" | socat "${tmpdir}/h4/stats" -) + echo "update ssl ocsp-response ${testdir}/ocsp_update/multicert/server_ocsp.pem.rsa" | socat "${tmpdir}/h4/stats" - + while ! echo "get var proc.processed" | socat "${tmpdir}/h4/stats" - | grep 'proc.processed: type=sint value=<1>' + do + echo "get var proc.processed" | socat "${tmpdir}/h4/stats" - >> /tmp/toto + sleep 0.5 + done + + echo "experimental-mode on;set var proc.processed int(0)" | socat "${tmpdir}/h4/stats" - + + ocsp_response=$(echo "show ssl ocsp-response 303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021015" | socat "${tmpdir}/h4/stats" -) new_produced_at=$(echo "$ocsp_response" | grep "Produced At") echo "$ocsp_response" | grep -q "Serial Number: 1015" && \ @@ -335,10 +343,17 @@ shell { # Store the current "Produced At" in order to ensure that after the update # the OCSP response actually changed produced_at=$(echo "show ssl ocsp-response 303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016" | socat "${tmpdir}/h4/stats" - | grep "Produced At") - # We should receive the OCSP response's details on the standard output when calling - # 'update ssl ocsp-response' - ocsp_response=$(echo "update ssl ocsp-response ${testdir}/ocsp_update/multicert/server_ocsp_ecdsa.pem" | socat "${tmpdir}/h4/stats" -) + echo "update ssl ocsp-response ${testdir}/ocsp_update/multicert/server_ocsp_ecdsa.pem" | socat "${tmpdir}/h4/stats" - + while ! echo "get var proc.processed" | socat "${tmpdir}/h4/stats" - | grep 'proc.processed: type=sint value=<1>' + do + echo "get var proc.processed" | socat "${tmpdir}/h4/stats" - >> /tmp/toto + sleep 0.5 + done + + echo "experimental-mode on;set var proc.processed int(0)" | socat "${tmpdir}/h4/stats" - + + ocsp_response=$(echo "show ssl ocsp-response 303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016" | socat "${tmpdir}/h4/stats" -) new_produced_at=$(echo "$ocsp_response" | grep "Produced At") echo "$ocsp_response" | grep -q "Serial Number: 1016" && \ @@ -482,7 +497,7 @@ haproxy h6 -conf { timeout server "${HAPROXY_TEST_TIMEOUT-5s}" frontend ssl-fe - bind "${tmpdir}/ssl.sock" ssl crt-list ${testdir}/simple.crt-list ca-file ${testdir}/set_cafile_rootCA.crt verify none crt-ignore-err all + bind "${tmpdir}/ssl9.sock" ssl crt-list ${testdir}/simple.crt-list ca-file ${testdir}/set_cafile_rootCA.crt verify none crt-ignore-err all http-request return status 200 listen http_rebound_lst diff --git a/src/ssl_ocsp.c b/src/ssl_ocsp.c index 445463ae5..c34868102 100644 --- a/src/ssl_ocsp.c +++ b/src/ssl_ocsp.c @@ -936,6 +936,12 @@ static inline void ssl_ocsp_set_next_update(struct certificate_ocsp *ocsp) */ int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) { + /* This entry was only supposed to be updated once, it does not need to + * be reinserted into the update tree. + */ + if (ocsp->update_once) + return 0; + /* Set next_update based on current time and the various OCSP * minimum/maximum update times. */ @@ -960,6 +966,13 @@ int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) int ssl_ocsp_update_insert_after_error(struct certificate_ocsp *ocsp) { int replay_delay = 0; + + /* This entry was only supposed to be updated once, it does not need to + * be reinserted into the update tree. + */ + if (ocsp->update_once) + return 0; + /* * Set next_update based on current time and the various OCSP * minimum/maximum update times. @@ -1325,99 +1338,21 @@ static int ssl_ocsp_update_precheck() REGISTER_PRE_CHECK(ssl_ocsp_update_precheck); - -struct ocsp_cli_ctx { - struct httpclient *hc; - struct ckch_data *ckch_data; - X509 *ocsp_issuer; - uint flags; - uint do_update; -}; - - -void cli_ocsp_res_stline_cb(struct httpclient *hc) -{ - struct appctx *appctx = hc->caller; - struct ocsp_cli_ctx *ctx; - - if (!appctx) - return; - - ctx = appctx->svcctx; - ctx->flags |= HC_F_RES_STLINE; - appctx_wakeup(appctx); -} - -void cli_ocsp_res_headers_cb(struct httpclient *hc) -{ - struct appctx *appctx = hc->caller; - struct ocsp_cli_ctx *ctx; - - if (!appctx) - return; - - ctx = appctx->svcctx; - ctx->flags |= HC_F_RES_HDR; - appctx_wakeup(appctx); -} - -void cli_ocsp_res_body_cb(struct httpclient *hc) -{ - struct appctx *appctx = hc->caller; - struct ocsp_cli_ctx *ctx; - - if (!appctx) - return; - - ctx = appctx->svcctx; - ctx->flags |= HC_F_RES_BODY; - appctx_wakeup(appctx); -} - -void cli_ocsp_res_end_cb(struct httpclient *hc) -{ - struct appctx *appctx = hc->caller; - struct ocsp_cli_ctx *ctx; - - if (!appctx) - return; - - ctx = appctx->svcctx; - ctx->flags |= HC_F_RES_END; - appctx_wakeup(appctx); -} - static int cli_parse_update_ocsp_response(char **args, char *payload, struct appctx *appctx, void *private) { int errcode = 0; char *err = NULL; struct ckch_store *ckch_store = NULL; - X509 *cert = NULL; - struct ocsp_cli_ctx *ctx = applet_reserve_svcctx(appctx, sizeof(*ctx)); - struct httpclient *hc = NULL; - struct buffer *req_url = NULL; - struct buffer *req_body = NULL; - OCSP_CERTID *certid = NULL; + struct certificate_ocsp *ocsp = NULL; + int update_once = 0; + unsigned char key[OCSP_MAX_CERTID_ASN1_LENGTH] = {}; + unsigned char *p; if (!*args[3]) { memprintf(&err, "'update ssl ocsp-response' expects a filename\n"); return cli_dynerr(appctx, err); } - req_url = alloc_trash_chunk(); - if (!req_url) { - memprintf(&err, "%sCan't allocate memory\n", err ? err : ""); - errcode |= ERR_ALERT | ERR_FATAL; - goto end; - } - - req_body = alloc_trash_chunk(); - if (!req_body) { - memprintf(&err, "%sCan't allocate memory\n", err ? err : ""); - errcode |= ERR_ALERT | ERR_FATAL; - goto end; - } - /* The operations on the CKCH architecture are locked so we can * manipulate ckch_store and ckch_inst */ if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock)) { @@ -1435,195 +1370,45 @@ static int cli_parse_update_ocsp_response(char **args, char *payload, struct app goto end; } - ctx->ckch_data = ckch_store->data; + p = key; + i2d_OCSP_CERTID(ckch_store->data->ocsp_cid, &p); - cert = ckch_store->data->cert; - - if (ssl_ocsp_get_uri_from_cert(cert, req_url, &err)) { - errcode |= ERR_ALERT | ERR_FATAL; - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - goto end; - } - - /* Look for the ocsp issuer in the ckch_data or in the certificate - * chain, the same way it is done in ssl_sock_load_ocsp. */ - ctx->ocsp_issuer = ctx->ckch_data->ocsp_issuer; - - /* take issuer from chain over ocsp_issuer, is what is done historicaly */ - if (ctx->ckch_data->chain) { - int i = 0; - /* check if one of the certificate of the chain is the issuer */ - for (i = 0; i < sk_X509_num(ctx->ckch_data->chain); i++) { - X509 *ti = sk_X509_value(ctx->ckch_data->chain, i); - if (X509_check_issued(ti, cert) == X509_V_OK) { - ctx->ocsp_issuer = ti; - break; - } - } - } - - if (!ctx->ocsp_issuer) { - memprintf(&err, "%sOCSP issuer not found\n", err ? err : ""); - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - goto end; - } - - X509_up_ref(ctx->ocsp_issuer); - - certid = OCSP_cert_to_id(NULL, cert, ctx->ocsp_issuer); - if (certid == NULL) { - memprintf(&err, "%sOCSP_cert_to_id() error\n", err ? err : ""); - HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - goto end; - } - - /* From here on the lock is not needed anymore. */ HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); - /* Create ocsp request */ - if (ssl_ocsp_create_request_details(certid, req_url, req_body, &err) != 0) { - memprintf(&err, "%sCreate ocsp request error\n", err ? err : ""); + + HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); + ocsp = (struct certificate_ocsp *)ebmb_lookup(&cert_ocsp_tree, key, OCSP_MAX_CERTID_ASN1_LENGTH); + if (!ocsp) { + memprintf(&err, "%s'update ssl ocsp-response' only works on certificates that already have a known OCSP response.\n", err ? err : ""); + errcode |= ERR_ALERT | ERR_FATAL; + HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); goto end; } - hc = httpclient_new_from_proxy(httpclient_ocsp_update_px, appctx, - b_data(req_body) ? HTTP_METH_POST : HTTP_METH_GET, - ist2(b_orig(req_url), b_data(req_url))); - if (!hc) { - memprintf(&err, "%sCan't allocate httpclient\n", err ? err : ""); - goto end; - } + update_once = (ocsp->next_update.node.leaf_p == NULL); + eb64_delete(&ocsp->next_update); - if (httpclient_req_gen(hc, hc->req.url, hc->req.meth, b_data(req_body) ? ocsp_request_hdrs : NULL, - ist2(b_orig(req_body), b_data(req_body))) != ERR_NONE) { - memprintf(&err, "%shttpclient_req_gen() error\n", err ? err : ""); - goto end; - } + /* Insert the entry at the beginning of the update tree. */ + ocsp->next_update.key = 0; + eb64_insert(&ocsp_update_tree, &ocsp->next_update); + ocsp->update_once = update_once; - hc->ops.res_stline = cli_ocsp_res_stline_cb; - hc->ops.res_headers = cli_ocsp_res_headers_cb; - hc->ops.res_payload = cli_ocsp_res_body_cb; - hc->ops.res_end = cli_ocsp_res_end_cb; + HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); - ctx->hc = hc; /* store the httpclient ptr in the applet */ - ctx->flags = 0; + if (!ocsp_update_task) + ssl_create_ocsp_update_task(&err); - if (!httpclient_start(hc)) { - memprintf(&err, "%shttpclient_start() error\n", err ? err : ""); - goto end; - } - - free_trash_chunk(req_url); - free_trash_chunk(req_body); + task_wakeup(ocsp_update_task, TASK_WOKEN_MSG); return 0; end: - free_trash_chunk(req_url); - free_trash_chunk(req_body); - if (errcode & ERR_CODE) { return cli_dynerr(appctx, memprintf(&err, "%sCan't send ocsp request for %s!\n", err ? err : "", args[3])); } return cli_dynmsg(appctx, LOG_NOTICE, err); } -static int cli_io_handler_update_ocsp_response(struct appctx *appctx) -{ - struct ocsp_cli_ctx *ctx = appctx->svcctx; - struct httpclient *hc = ctx->hc; - - if (ctx->flags & HC_F_RES_STLINE) { - if (hc->res.status != 200) { - chunk_printf(&trash, "OCSP response error (status %d)\n", hc->res.status); - if (applet_putchk(appctx, &trash) == -1) - goto more; - goto end; - } - ctx->flags &= ~HC_F_RES_STLINE; - } - - if (ctx->flags & HC_F_RES_HDR) { - struct http_hdr *hdr; - int found = 0; - /* Look for "Content-Type" header which should have - * "application/ocsp-response" value. */ - for (hdr = hc->res.hdrs; isttest(hdr->v); hdr++) { - if (isteqi(hdr->n, ist("Content-Type")) && - isteqi(hdr->v, ist("application/ocsp-response"))) { - found = 1; - break; - } - } - if (!found) { - chunk_printf(&trash, "Missing 'Content-Type: application/ocsp-response' header\n"); - if (applet_putchk(appctx, &trash) == -1) - goto more; - goto end; - } - ctx->flags &= ~HC_F_RES_HDR; - } - - if (ctx->flags & HC_F_RES_BODY) { - /* Wait until the full body is received and HC_F_RES_END flag is - * set. */ - } - - /* we must close only if F_END is the last flag */ - if (ctx->flags & HC_F_RES_END) { - char *err = NULL; - - if (ssl_ocsp_check_response(ctx->ckch_data->chain, ctx->ocsp_issuer, &hc->res.buf, &err)) { - chunk_printf(&trash, "%s", err); - free(err); - if (applet_putchk(appctx, &trash) == -1) - goto more; - goto end; - } - - if (ssl_sock_update_ocsp_response(&hc->res.buf, &err) != 0) { - chunk_printf(&trash, "%s", err); - free(err); - if (applet_putchk(appctx, &trash) == -1) - goto more; - goto end; - } - - free(err); - chunk_reset(&trash); - - if (ssl_ocsp_response_print(&hc->res.buf, &trash)) - goto end; - - if (applet_putchk(appctx, &trash) == -1) - goto more; - ctx->flags &= ~HC_F_RES_BODY; - ctx->flags &= ~HC_F_RES_END; - goto end; - } - -more: - if (!ctx->flags) - applet_have_no_more_data(appctx); - return 0; -end: - return 1; -} - -static void cli_release_update_ocsp_response(struct appctx *appctx) -{ - struct ocsp_cli_ctx *ctx = appctx->svcctx; - struct httpclient *hc = ctx->hc; - - X509_free(ctx->ocsp_issuer); - - /* Everything possible was printed on the CLI, we can destroy the client */ - httpclient_stop_and_destroy(hc); - - return; -} - - #endif /* !defined OPENSSL_IS_BORINGSSL */ @@ -2082,7 +1867,7 @@ static struct cli_kw_list cli_kws = {{ },{ { { "show", "ssl", "ocsp-response", NULL },"show ssl ocsp-response [[text|base64] id] : display the IDs of the OCSP responses used in memory, or the details of a single OCSP response (in text or base64 format)", cli_parse_show_ocspresponse, cli_io_handler_show_ocspresponse, NULL }, { { "show", "ssl", "ocsp-updates", NULL }, "show ssl ocsp-updates : display information about the next 'nb' ocsp responses that will be updated automatically", cli_parse_show_ocsp_updates, cli_io_handler_show_ocsp_updates, cli_release_show_ocsp_updates }, #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) && !defined OPENSSL_IS_BORINGSSL) - { { "update", "ssl", "ocsp-response", NULL }, "update ssl ocsp-response : send ocsp request and update stored ocsp response", cli_parse_update_ocsp_response, cli_io_handler_update_ocsp_response, cli_release_update_ocsp_response }, + { { "update", "ssl", "ocsp-response", NULL }, "update ssl ocsp-response : send ocsp request and update stored ocsp response", cli_parse_update_ocsp_response, NULL, NULL }, #endif { { NULL }, NULL, NULL, NULL } }}; diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 5ebc72f67..97d2c3bb4 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1116,18 +1116,17 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * tlsextStatusCb callback; #endif struct buffer *ocsp_uri = get_trash_chunk(); + char *err = NULL; x = data->cert; if (!x) goto out; - if (data->ocsp_update_mode == SSL_SOCK_OCSP_UPDATE_ON) { - ssl_ocsp_get_uri_from_cert(x, ocsp_uri, NULL); - /* We should have an "OCSP URI" field in order for auto update to work. */ - if (b_data(ocsp_uri) == 0) - goto out; - } + ssl_ocsp_get_uri_from_cert(x, ocsp_uri, &err); + /* We should have an "OCSP URI" field in order for auto update to work. */ + if (data->ocsp_update_mode == SSL_SOCK_OCSP_UPDATE_ON && b_data(ocsp_uri) == 0) + goto out; /* In case of ocsp update mode set to 'on', this function might be * called with no known ocsp response. If no ocsp uri can be found in @@ -1244,29 +1243,29 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * ha_warning("%s.\n", warn); } - if (data->ocsp_update_mode == SSL_SOCK_OCSP_UPDATE_ON) { - /* Do not insert the same certificate_ocsp structure in the - * update tree more than once. */ - if (!ocsp) { - /* Issuer certificate is not included in the certificate - * chain, it will have to be treated separately during - * ocsp response validation. */ - if (issuer == data->ocsp_issuer) { - iocsp->issuer = issuer; - X509_up_ref(issuer); - } - if (data->chain) - iocsp->chain = X509_chain_up_ref(data->chain); + /* Do not insert the same certificate_ocsp structure in the + * update tree more than once. */ + if (!ocsp) { + /* Issuer certificate is not included in the certificate + * chain, it will have to be treated separately during + * ocsp response validation. */ + if (issuer == data->ocsp_issuer) { + iocsp->issuer = issuer; + X509_up_ref(issuer); + } + if (data->chain) + iocsp->chain = X509_chain_up_ref(data->chain); - iocsp->uri = calloc(1, sizeof(*iocsp->uri)); - if (!chunk_dup(iocsp->uri, ocsp_uri)) { - ha_free(&iocsp->uri); - goto out; - } + iocsp->uri = calloc(1, sizeof(*iocsp->uri)); + if (!chunk_dup(iocsp->uri, ocsp_uri)) { + ha_free(&iocsp->uri); + goto out; + } - strcpy(iocsp->path, path); + strcpy(iocsp->path, path); + if (data->ocsp_update_mode == SSL_SOCK_OCSP_UPDATE_ON) { ssl_ocsp_update_insert(iocsp); /* If we are during init the update task is not * scheduled yet so a wakeup won't do anything.