From: Remi Tricot-Le Breton Date: Fri, 13 Dec 2024 15:08:56 +0000 (+0100) Subject: MINOR: ssl/ocsp: Add extra details in error logs when possible X-Git-Tag: v3.2-dev2~35 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=93f2c7342370c15ec1d82ad60f01add8491abcb8;p=thirdparty%2Fhaproxy.git MINOR: ssl/ocsp: Add extra details in error logs when possible When the ocsp response auto update process fails during insertion or while validating the received ocsp response, we call ssl_sock_update_ocsp_response or ssl_ocsp_check_response respectively and both these functions take an 'err' parameter in which detailed error messages can be written. Until now, those error messages were discarded and the only information given to the user was a generic error (ERR_CHECK or ERR_INSERT) which does not help much. We now keep a pointer to the last error message in the certificate_ocsp structure and dump its content in the update logs as well as in the "show ssl ocsp-updates" cli command. This issue was raised in GitHub #2817. --- diff --git a/doc/configuration.txt b/doc/configuration.txt index 69d1061e92..b79b028c99 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -5520,6 +5520,9 @@ ocsp-update [ off | on ] section. In case of "OCSP response check failure" error, you might want to check that the issuer certificate that you provided is valid. + A more precise error message might also be displayed between parenthesis + after the "generic" error message. It can happen for "OCSP response check + failure" or "Error during insertion" errors. 4. Proxies ---------- diff --git a/include/haproxy/ssl_ocsp-t.h b/include/haproxy/ssl_ocsp-t.h index f9fef4d812..aa7086d52e 100644 --- a/include/haproxy/ssl_ocsp-t.h +++ b/include/haproxy/ssl_ocsp-t.h @@ -58,6 +58,7 @@ struct certificate_ocsp { /* OCSP update stats */ u64 last_update; /* Time of last successful update */ + char *last_update_error; /* Error message filled in case of update issue */ 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 */ diff --git a/src/ssl_ocsp.c b/src/ssl_ocsp.c index 323c32e19d..741911074f 100644 --- a/src/ssl_ocsp.c +++ b/src/ssl_ocsp.c @@ -376,6 +376,7 @@ static void ssl_sock_free_ocsp_data(struct certificate_ocsp *ocsp) ha_free(&ocsp->uri->area); ha_free(&ocsp->uri); } + ha_free(&ocsp->last_update_error); free(ocsp); } @@ -1099,6 +1100,8 @@ static void ssl_ocsp_send_log() int status_str_len = 0; char *status_str = NULL; struct certificate_ocsp *ocsp = ssl_ocsp_task_ctx.cur_ocsp; + char *last_error = NULL; + struct buffer *tmpbuf = get_trash_chunk(); if (!httpclient_ocsp_update_px) return; @@ -1106,15 +1109,20 @@ static void ssl_ocsp_send_log() if (ocsp && ssl_ocsp_task_ctx.update_status < OCSP_UPDT_ERR_LAST) { status_str_len = istlen(ocsp_update_errors[ssl_ocsp_task_ctx.update_status]); status_str = istptr(ocsp_update_errors[ssl_ocsp_task_ctx.update_status]); + last_error = ocsp->last_update_error; } - send_log(httpclient_ocsp_update_px, LOG_NOTICE, "%s %s %u \"%.*s\" %u %u", - httpclient_ocsp_update_px->id, - ocsp->path, - ssl_ocsp_task_ctx.update_status, - status_str_len, status_str, - ocsp ? ocsp->num_failure : 0, - ocsp ? ocsp->num_success : 0); + chunk_printf(tmpbuf, "%s %s %u \"%.*s", httpclient_ocsp_update_px->id, + ocsp->path, ssl_ocsp_task_ctx.update_status, + status_str_len, status_str); + + if (last_error) + chunk_appendf(tmpbuf, " (%s)", last_error); + + chunk_appendf(tmpbuf, "\" %u %u", ocsp ? ocsp->num_failure : 0, + ocsp ? ocsp->num_success : 0); + + send_log(httpclient_ocsp_update_px, LOG_NOTICE, "%.*s", (int)b_data(tmpbuf), b_orig(tmpbuf)); } /* @@ -1147,6 +1155,7 @@ static struct task *ssl_ocsp_update_responses(struct task *task, void *context, struct buffer *req_body = NULL; OCSP_CERTID *certid = NULL; struct ssl_ocsp_task_ctx *ctx = &ssl_ocsp_task_ctx; + char *err = NULL; if (ctx->cur_ocsp) { /* An update is in process */ @@ -1189,12 +1198,12 @@ static struct task *ssl_ocsp_update_responses(struct task *task, void *context, /* Process the body that must be complete since * HC_F_RES_END is set. */ if (ctx->flags & HC_F_RES_BODY) { - if (ssl_ocsp_check_response(ocsp->chain, ocsp->issuer, &hc->res.buf, NULL)) { + if (ssl_ocsp_check_response(ocsp->chain, ocsp->issuer, &hc->res.buf, &err)) { ctx->update_status = OCSP_UPDT_ERR_CHECK; goto http_error; } - if (ssl_sock_update_ocsp_response(&hc->res.buf, NULL) != 0) { + if (ssl_sock_update_ocsp_response(&hc->res.buf, &err) != 0) { ctx->update_status = OCSP_UPDT_ERR_INSERT; goto http_error; } @@ -1208,6 +1217,7 @@ static struct task *ssl_ocsp_update_responses(struct task *task, void *context, ocsp->last_update = date.tv_sec; ctx->update_status = OCSP_UPDT_OK; ocsp->last_update_status = ctx->update_status; + ha_free(&ocsp->last_update_error); ssl_ocsp_send_log(); @@ -1352,13 +1362,15 @@ wait: return task; http_error: - ssl_ocsp_send_log(); /* Reinsert certificate into update list so that it can be updated later */ if (ocsp) { ++ocsp->num_failure; ocsp->last_update_status = ctx->update_status; + ha_free(&ocsp->last_update_error); + ocsp->last_update_error = err; ssl_ocsp_update_insert_after_error(ocsp); } + ssl_ocsp_send_log(); if (hc) httpclient_stop_and_destroy(hc); @@ -1806,8 +1818,11 @@ static int dump_ocsp_update_info(struct certificate_ocsp *ocsp, struct buffer *o /* Last update status str */ if (ocsp->last_update_status >= OCSP_UPDT_ERR_LAST) chunk_appendf(out, "-"); - else + else { chunk_appendf(out, "%s", istptr(ocsp_update_errors[ocsp->last_update_status])); + if (ocsp->last_update_error) + chunk_appendf(out, " (%s)", ocsp->last_update_error); + } chunk_appendf(out, "\n");