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 69d1061e929f4f72eae681ec7fb825e56b8b1747..b79b028c99e2f1a9548f5417cb28024e2335f85c 100644 (file)
--- 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 f9fef4d8125c96a91a067c213ee0648d1e75d2ce..aa7086d52e510d9e9dc158b9dc6a00c722de8159 100644 (file)
--- 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 323c32e19df586978f417de684b4abf867b61439..741911074f1a949bc88232b23fe27781a8c4e9c8 100644 (file)
--- 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");