MINOR: ssl: Use ocsp update task for "update ssl ocsp-response" command
Instead of having a dedicated httpclient instance and its own code
decorrelated from the actual auto update one, the "update ssl
ocsp-response" will now use the update task in order to perform updates.
Since the cli command allows to update responses that were never
included in the auto update tree, a new flag was added to the
certificate_ocsp structure so that the said entry can be inserted into
the tree "by hand" and it won't be reinserted back into the tree after
the update process is performed. The 'update_once' flag "stole" a bit
from the 'fail_count' counter since it is the one less likely to reach
UINT_MAX among the ocsp counters of the certificate_ocsp structure.
This new logic required that every certificate_ocsp entry contained all
the ocsp-related information at all time since entries that are not
supposed to be configured automatically can still be updated through the
cli. The logic of the ssl_sock_load_ocsp was changed accordingly.
diff --git a/include/haproxy/ssl_ocsp-t.h b/include/haproxy/ssl_ocsp-t.h
index 52aeb08..028d6fa 100644
--- a/include/haproxy/ssl_ocsp-t.h
+++ b/include/haproxy/ssl_ocsp-t.h
@@ -60,7 +60,8 @@
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 f6d3d78..a1d9a3c 100644
--- a/reg-tests/ssl/ocsp_auto_update.vtc
+++ b/reg-tests/ssl/ocsp_auto_update.vtc
@@ -288,6 +288,7 @@
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 @@
# 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 @@
# 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 @@
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 445463a..c348681 100644
--- a/src/ssl_ocsp.c
+++ b/src/ssl_ocsp.c
@@ -936,6 +936,12 @@
*/
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_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.
@@ -1324,100 +1337,22 @@
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 @@
goto end;
}
- ctx->ckch_data = ckch_store->data;
-
- 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;
- }
+ p = key;
+ i2d_OCSP_CERTID(ckch_store->data->ocsp_cid, &p);
- /* 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 : "");
- 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 : "");
+ 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;
}
- 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;
- }
+ update_once = (ocsp->next_update.node.leaf_p == NULL);
+ eb64_delete(&ocsp->next_update);
- 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;
+ /* 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;
- ctx->hc = hc; /* store the httpclient ptr in the applet */
- ctx->flags = 0;
+ HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock);
- if (!httpclient_start(hc)) {
- memprintf(&err, "%shttpclient_start() error\n", err ? err : "");
- goto end;
- }
+ if (!ocsp_update_task)
+ ssl_create_ocsp_update_task(&err);
- 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 @@
{ { "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 <certfile> : 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 <certfile> : 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 5ebc72f..97d2c3b 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1116,18 +1116,17 @@
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 @@
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.