diff options
| -rw-r--r-- | CHANGELOG | 4 | ||||
| -rw-r--r-- | SUBVERS | 2 | ||||
| -rw-r--r-- | VERDATE | 4 | ||||
| -rw-r--r-- | VERSION | 2 | ||||
| -rw-r--r-- | addons/promex/service-prometheus.c | 42 | ||||
| -rw-r--r-- | doc/configuration.txt | 2 | ||||
| -rw-r--r-- | include/haproxy/ssl_ocsp-t.h | 6 | ||||
| -rw-r--r-- | include/haproxy/ssl_ocsp.h | 1 | ||||
| -rw-r--r-- | reg-tests/ssl/ocsp_auto_update.vtc | 183 | ||||
| -rw-r--r-- | reg-tests/ssl/ocsp_update/multicert_both_certs.crt-list | 2 | ||||
| -rw-r--r-- | src/ssl_ckch.c | 23 | ||||
| -rw-r--r-- | src/ssl_ocsp.c | 94 | ||||
| -rw-r--r-- | src/ssl_sock.c | 12 |
13 files changed, 78 insertions, 299 deletions
@@ -1,6 +1,10 @@ ChangeLog : =========== +2024/02/26 : 2.9.6 + - BUG/MAJOR: promex: fix crash on deleted server + - BUG/MAJOR: ssl/ocsp: crash with ocsp when old process exit or using ocsp CLI + 2024/02/15 : 2.9.5 - BUG/MINOR: diag: always show the version before dumping a diag warning - BUG/MINOR: diag: run the final diags before quitting when using -c @@ -1,2 +1,2 @@ --260dbb8 +-9eafce5 @@ -1,2 +1,2 @@ -2024-02-15 14:53:05 +0100 -2024/02/15 +2024-02-26 18:42:42 +0100 +2024/02/26 @@ -1 +1 @@ -2.9.5 +2.9.6 diff --git a/addons/promex/service-prometheus.c b/addons/promex/service-prometheus.c index 6885d20..e9ad44e 100644 --- a/addons/promex/service-prometheus.c +++ b/addons/promex/service-prometheus.c @@ -412,6 +412,15 @@ enum promex_srv_state promex_srv_status(struct server *sv) return state; } +/* Store <sv> in <ctx> safely by using refcount to prevent server deletion. */ +static void promex_set_ctx_sv(struct promex_ctx *ctx, struct server *sv) +{ + srv_drop(ctx->sv); + ctx->sv = sv; + if (ctx->sv) + srv_take(ctx->sv); +} + /* Convert a field to its string representation and write it in <out>, followed * by a newline, if there is enough space. non-numeric value are converted in * "NaN" because Prometheus only support numerical values (but it is unexepceted @@ -1125,16 +1134,16 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx) &val, labels, &out, max)) goto full; next_sv: - ctx->sv = sv->next; + promex_set_ctx_sv(ctx, sv->next); } next_px: ctx->px = px->next; - ctx->sv = (ctx->px ? ctx->px->srv : NULL); + promex_set_ctx_sv(ctx, ctx->px ? ctx->px->srv : NULL); } ctx->flags |= PROMEX_FL_METRIC_HDR; ctx->px = proxies_list; - ctx->sv = (ctx->px ? ctx->px->srv : NULL); + promex_set_ctx_sv(ctx, ctx->px ? ctx->px->srv : NULL); } @@ -1229,7 +1238,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_INFO_METRIC); ctx->obj_state = 0; ctx->field_num = INF_NAME; @@ -1249,7 +1258,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~PROMEX_FL_INFO_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_FRONT_METRIC); ctx->obj_state = 0; @@ -1270,7 +1279,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = LIST_NEXT(&proxies_list->conf.listeners, struct listener *, by_fe); - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~PROMEX_FL_FRONT_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_LI_METRIC); ctx->obj_state = 0; @@ -1291,7 +1300,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~PROMEX_FL_LI_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_BACK_METRIC); ctx->obj_state = 0; @@ -1312,7 +1321,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = NULL; - ctx->sv = ctx->px ? ctx->px->srv : NULL; + promex_set_ctx_sv(ctx, ctx->px ? ctx->px->srv : NULL); ctx->flags &= ~PROMEX_FL_BACK_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_SRV_METRIC); ctx->obj_state = 0; @@ -1333,7 +1342,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = stktables_list; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~(PROMEX_FL_METRIC_HDR|PROMEX_FL_SRV_METRIC); ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_STICKTABLE_METRIC); ctx->field_num = STICKTABLE_SIZE; @@ -1353,7 +1362,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~(PROMEX_FL_METRIC_HDR|PROMEX_FL_STICKTABLE_METRIC); ctx->field_num = 0; appctx->st1 = PROMEX_DUMPER_DONE; @@ -1374,7 +1383,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags = 0; ctx->field_num = 0; appctx->st1 = PROMEX_DUMPER_DONE; @@ -1529,6 +1538,16 @@ static int promex_appctx_init(struct appctx *appctx) return 0; } +/* Callback function that releases a promex applet. This happens when the + * connection with the agent is closed. */ +static void promex_appctx_release(struct appctx *appctx) +{ + struct promex_ctx *ctx = appctx->svcctx; + + if (appctx->st1 == PROMEX_DUMPER_SRV) + srv_drop(ctx->sv); +} + /* The main I/O handler for the promex applet. */ static void promex_appctx_handle_io(struct appctx *appctx) { @@ -1621,6 +1640,7 @@ struct applet promex_applet = { .name = "<PROMEX>", /* used for logging */ .init = promex_appctx_init, .fct = promex_appctx_handle_io, + .release = promex_appctx_release, }; static enum act_parse_ret service_parse_prometheus_exporter(const char **args, int *cur_arg, struct proxy *px, diff --git a/doc/configuration.txt b/doc/configuration.txt index a1f15fc..978d655 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -3,7 +3,7 @@ Configuration Manual ---------------------- version 2.9 - 2024/02/15 + 2024/02/26 This document covers the configuration language as implemented in the version diff --git a/include/haproxy/ssl_ocsp-t.h b/include/haproxy/ssl_ocsp-t.h index fc2750b..028d6fa 100644 --- a/include/haproxy/ssl_ocsp-t.h +++ b/include/haproxy/ssl_ocsp-t.h @@ -47,8 +47,7 @@ struct certificate_ocsp { struct ebmb_node key; unsigned char key_data[OCSP_MAX_CERTID_ASN1_LENGTH]; unsigned int key_length; - int refcount_store; /* Number of ckch_store that reference this certificate_ocsp */ - int refcount_instance; /* Number of ckch_inst that reference this certificate_ocsp */ + int refcount; struct buffer response; long expire; X509 *issuer; @@ -61,9 +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:30; /* 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 */ - unsigned int updating:1; /* Set if an entry is already being updated */ char path[VAR_ARRAY]; }; diff --git a/include/haproxy/ssl_ocsp.h b/include/haproxy/ssl_ocsp.h index 8a4197c..54a1b88 100644 --- a/include/haproxy/ssl_ocsp.h +++ b/include/haproxy/ssl_ocsp.h @@ -36,7 +36,6 @@ int ssl_sock_get_ocsp_arg_kt_index(int evp_keytype); int ssl_sock_ocsp_stapling_cbk(SSL *ssl, void *arg); void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp); -void ssl_sock_free_ocsp_instance(struct certificate_ocsp *ocsp); int ssl_sock_load_ocsp_response(struct buffer *ocsp_response, struct certificate_ocsp *ocsp, diff --git a/reg-tests/ssl/ocsp_auto_update.vtc b/reg-tests/ssl/ocsp_auto_update.vtc index 2ab4a4a..a1d9a3c 100644 --- a/reg-tests/ssl/ocsp_auto_update.vtc +++ b/reg-tests/ssl/ocsp_auto_update.vtc @@ -533,186 +533,3 @@ haproxy h6 -cli { send "show ssl ocsp-updates" expect ~ "303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016 .*| 1 | 0 | 1 | Update successful" } - -haproxy h6 -wait -process p6 -wait - - -###################### -# # -# SEVENTH TEST CASE # -# # -###################### - -# -# Check that removing crt-list instances does not remove the OCSP responses -# from the tree but that they will not be auto updated anymore if the last -# instance is removed (via del ssl crt-list). -# - -haproxy h7 -conf { - global - tune.ssl.default-dh-param 2048 - tune.ssl.capture-buffer-size 1 - stats socket "${tmpdir}/h7/stats" level admin - crt-base ${testdir}/ocsp_update - - defaults - mode http - option httplog - log stderr local0 debug err - option logasap - timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" - timeout client "${HAPROXY_TEST_TIMEOUT-5s}" - timeout server "${HAPROXY_TEST_TIMEOUT-5s}" - - frontend ssl-fe - bind "${tmpdir}/ssl-h7.sock" ssl crt-list ${testdir}/ocsp_update/multicert_both_certs.crt-list ca-file ${testdir}/set_cafile_rootCA.crt verify none crt-ignore-err all - http-request return status 200 - - listen http_rebound_lst - mode http - bind "127.0.0.1:12345" - server s1 "127.0.0.1:12346" -} -start - -# Check that the two certificates are taken into account in the auto update process -haproxy h7 -cli { - send "show ssl ocsp-updates" - expect ~ "303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021015 .*" - - send "show ssl ocsp-updates" - expect ~ "303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016 .*" -} - -# Remove the second line from the crt-list and check that the corresponding -# ocsp response was removed from the auto update list but is still present in the -# system -haproxy h7 -cli { - send "del ssl crt-list ${testdir}/ocsp_update/multicert_both_certs.crt-list ${testdir}/ocsp_update/multicert/server_ocsp.pem.ecdsa" - expect ~ "Entry.*deleted in crtlist" - - send "show ssl ocsp-updates" - expect !~ "303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016 .*" - - send "show ssl ocsp-response" - expect ~ "Certificate ID key : 303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016" - - send "show ssl ocsp-response ${testdir}/ocsp_update/multicert/server_ocsp.pem.ecdsa" - expect ~ ".* Cert Status: good.*" -} - -# Add the previously removed crt-list line with auto-update enabled and check that -# the ocsp response appears in the auto update list -shell { - printf "add ssl crt-list ${testdir}/ocsp_update/multicert_both_certs.crt-list <<\nmulticert/server_ocsp.pem.ecdsa [ocsp-update on] foo.bar\n\n" | socat "${tmpdir}/h7/stats" - | grep "Inserting certificate.*in crt-list" -} - -haproxy h7 -cli { - send "show ssl ocsp-updates" - expect ~ "303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021016 .*" -} - -# Check that the auto update option consistency check work even when crt-list -# lines are added through the cli -shell { - printf "add ssl crt-list ${testdir}/ocsp_update/multicert_both_certs.crt-list <<\nmulticert/server_ocsp.pem.ecdsa foo.foo\n\n" | socat "${tmpdir}/h7/stats" - | grep "Incompatibilities found in OCSP update mode for certificate" -} - -haproxy h7 -wait - -#################### -# # -# EIGTH TEST CASE # -# # -#################### - -# -# Check that a certificate created through the CLI and which does not have ocsp -# update enabled can be updated via "update ssl ocsp-response" command. -# - -process p8 "openssl ocsp -index ${testdir}/ocsp_update/index.txt -rsigner ${testdir}/ocsp_update/ocsp.haproxy.com.pem -CA ${testdir}/ocsp_update/ocsp_update_rootca.crt -nrequest 1 -ndays 1 -port 12346 -timeout 5" -start - -barrier b8 cond 2 -cyclic - -syslog Syslog_h8 -level info { - recv - expect ~ "GET /MEMwQTA%2FMD0wOzAJBgUrDgMCGgUABBSKg%2BAGD6%2F3Ccp%2Bm5VSKi6BY1%2FaCgQU9lKw5DXV6pI4UVCPCtvpLYXeAHoCAhAV HTTP/1.1" - - barrier b8 sync -} -start - - -haproxy h8 -conf { - global - tune.ssl.default-dh-param 2048 - tune.ssl.capture-buffer-size 1 - stats socket "${tmpdir}/h8/stats" level admin - crt-base ${testdir}/ocsp_update - - defaults - mode http - option httplog - log stderr local0 debug err - option logasap - timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" - timeout client "${HAPROXY_TEST_TIMEOUT-5s}" - timeout server "${HAPROXY_TEST_TIMEOUT-5s}" - - frontend ssl-fe - bind "${tmpdir}/ssl-h8.sock" ssl crt-list ${testdir}/ocsp_update/multicert_ecdsa_no_update.crt-list ca-file ${testdir}/set_cafile_rootCA.crt verify none crt-ignore-err all - http-request return status 200 - - listen http_rebound_lst - mode http - option httplog - log ${Syslog_h8_addr}:${Syslog_h8_port} local0 - bind "127.0.0.1:12345" - server s1 "127.0.0.1:12346" -} -start - -# We need to "enable" the cli with a first cli call before using it only through socats -haproxy h8 -cli { - send "show ssl cert" - expect ~ "" -} - -# Create a new certificate and add it in the crt-list with ocsp auto-update enabled -shell { - echo "new ssl cert ${testdir}/ocsp_update/rsa.pem" | socat "${tmpdir}/h8/stats" - - printf "set ssl cert ${testdir}/ocsp_update/rsa.pem <<\n$(cat ${testdir}/ocsp_update/multicert/server_ocsp.pem.rsa)\n\n" | socat "${tmpdir}/h8/stats" - - printf "set ssl cert ${testdir}/ocsp_update/rsa.pem.issuer <<\n$(cat ${testdir}/ocsp_update/ocsp_update_rootca.crt)\n\n" | socat "${tmpdir}/h8/stats" - - printf "set ssl cert ${testdir}/ocsp_update/rsa.pem.ocsp <<\n$(base64 -w 1000 ${testdir}/ocsp_update/multicert/server_ocsp.pem.rsa.ocsp)\n\n" | socat "${tmpdir}/h8/stats" - - echo "commit ssl cert ${testdir}/ocsp_update/rsa.pem" | socat "${tmpdir}/h8/stats" - - - printf "add ssl crt-list ${testdir}/ocsp_update/multicert_ecdsa_no_update.crt-list <<\nrsa.pem [ocsp-update off] foo.bar\n\n" | socat "${tmpdir}/h8/stats" - -} - -# Check that the line is in the crt-list -haproxy h8 -cli { - send "show ssl crt-list ${testdir}/ocsp_update/multicert_ecdsa_no_update.crt-list" - expect ~ "${testdir}/ocsp_update/rsa.pem .* foo.bar" -} - -# Check that the new certificate is NOT in the auto update list -haproxy h8 -cli { - send "show ssl ocsp-updates" - expect !~ "303b300906052b0e03021a050004148a83e0060faff709ca7e9b95522a2e81635fda0a0414f652b0e435d5ea923851508f0adbe92d85de007a02021015.*" -} - -shell { - echo "update ssl ocsp-response ${testdir}/ocsp_update/rsa.pem" | socat "${tmpdir}/h8/stats" - -} - -shell "sleep 1" - -barrier b8 sync - -haproxy h8 -cli { - send "show ssl ocsp-response ${testdir}/ocsp_update/rsa.pem" - expect ~ ".* Cert Status: revoked.*" -} - -haproxy h8 -wait -process p8 -wait diff --git a/reg-tests/ssl/ocsp_update/multicert_both_certs.crt-list b/reg-tests/ssl/ocsp_update/multicert_both_certs.crt-list deleted file mode 100644 index 0ec641f..0000000 --- a/reg-tests/ssl/ocsp_update/multicert_both_certs.crt-list +++ /dev/null @@ -1,2 +0,0 @@ -multicert/server_ocsp.pem.rsa [ocsp-update on ssl-min-ver TLSv1.2] * -multicert/server_ocsp.pem.ecdsa [ocsp-update on ssl-min-ver TLSv1.2] * diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index ab39755..afe6ff6 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -721,27 +721,8 @@ void ssl_sock_free_cert_key_and_chain_contents(struct ckch_data *data) X509_free(data->ocsp_issuer); data->ocsp_issuer = NULL; - - /* We need to properly remove the reference to the corresponding - * certificate_ocsp structure if it exists (which it should). - */ -#if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) && !defined OPENSSL_IS_BORINGSSL) - if (data->ocsp_cid) { - struct certificate_ocsp *ocsp = NULL; - unsigned char certid[OCSP_MAX_CERTID_ASN1_LENGTH] = {}; - unsigned int certid_length = 0; - - if (ssl_ocsp_build_response_key(data->ocsp_cid, (unsigned char*)certid, &certid_length) >= 0) { - HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); - ocsp = (struct certificate_ocsp *)ebmb_lookup(&cert_ocsp_tree, certid, OCSP_MAX_CERTID_ASN1_LENGTH); - HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); - ssl_sock_free_ocsp(ocsp); - } - - OCSP_CERTID_free(data->ocsp_cid); - data->ocsp_cid = NULL; - } -#endif + OCSP_CERTID_free(data->ocsp_cid); + data->ocsp_cid = NULL; } /* diff --git a/src/ssl_ocsp.c b/src/ssl_ocsp.c index 1adddc4..3e7408a 100644 --- a/src/ssl_ocsp.c +++ b/src/ssl_ocsp.c @@ -392,9 +392,8 @@ void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp) return; HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); - ocsp->refcount_store--; - if (ocsp->refcount_store <= 0) { - BUG_ON(ocsp->refcount_instance > 0); + ocsp->refcount--; + if (ocsp->refcount <= 0) { ebmb_delete(&ocsp->key); eb64_delete(&ocsp->next_update); X509_free(ocsp->issuer); @@ -412,19 +411,6 @@ void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp) HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); } -void ssl_sock_free_ocsp_instance(struct certificate_ocsp *ocsp) -{ - if (!ocsp) - return; - - HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); - ocsp->refcount_instance--; - if (ocsp->refcount_instance <= 0) { - eb64_delete(&ocsp->next_update); - } - HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); -} - /* * This function dumps the details of an OCSP_CERTID. It is based on @@ -640,13 +626,13 @@ void ssl_sock_ocsp_free_func(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int id ocsp_arg = ptr; if (ocsp_arg->is_single) { - ssl_sock_free_ocsp_instance(ocsp_arg->s_ocsp); + ssl_sock_free_ocsp(ocsp_arg->s_ocsp); ocsp_arg->s_ocsp = NULL; } else { int i; for (i = 0; i < SSL_SOCK_NUM_KEYTYPES; i++) { - ssl_sock_free_ocsp_instance(ocsp_arg->m_ocsp[i]); + ssl_sock_free_ocsp(ocsp_arg->m_ocsp[i]); ocsp_arg->m_ocsp[i] = NULL; } } @@ -981,6 +967,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. */ @@ -989,12 +981,7 @@ int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) ocsp->fail_count = 0; HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); - ocsp->updating = 0; - /* An entry with update_once set to 1 was only supposed to be updated - * once, it does not need to be reinserted into the update tree. - */ - if (!ocsp->update_once) - eb64_insert(&ocsp_update_tree, &ocsp->next_update); + eb64_insert(&ocsp_update_tree, &ocsp->next_update); HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); return 0; @@ -1011,6 +998,12 @@ 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. @@ -1033,12 +1026,7 @@ int ssl_ocsp_update_insert_after_error(struct certificate_ocsp *ocsp) ocsp->next_update.key = date.tv_sec + replay_delay; HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); - ocsp->updating = 0; - /* An entry with update_once set to 1 was only supposed to be updated - * once, it does not need to be reinserted into the update tree. - */ - if (!ocsp->update_once) - eb64_insert(&ocsp_update_tree, &ocsp->next_update); + eb64_insert(&ocsp_update_tree, &ocsp->next_update); HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); return 0; @@ -1201,7 +1189,7 @@ static struct task *ssl_ocsp_update_responses(struct task *task, void *context, /* Reinsert the entry into the update list so that it can be updated later */ ssl_ocsp_update_insert(ocsp); /* Release the reference kept on the updated ocsp response. */ - ssl_sock_free_ocsp_instance(ctx->cur_ocsp); + ssl_sock_free_ocsp(ctx->cur_ocsp); ctx->cur_ocsp = NULL; HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); @@ -1244,8 +1232,7 @@ static struct task *ssl_ocsp_update_responses(struct task *task, void *context, * reinserted after the response is processed. */ eb64_delete(&ocsp->next_update); - ocsp->updating = 1; - ocsp->refcount_instance++; + ++ocsp->refcount; ctx->cur_ocsp = ocsp; ocsp->last_update_status = OCSP_UPDT_UNKNOWN; @@ -1312,7 +1299,7 @@ leave: ++ctx->cur_ocsp->num_failure; ssl_ocsp_update_insert_after_error(ctx->cur_ocsp); /* Release the reference kept on the updated ocsp response. */ - ssl_sock_free_ocsp_instance(ctx->cur_ocsp); + ssl_sock_free_ocsp(ctx->cur_ocsp); ctx->cur_ocsp = NULL; } if (hc) @@ -1341,7 +1328,7 @@ http_error: if (hc) httpclient_stop_and_destroy(hc); /* Release the reference kept on the updated ocsp response. */ - ssl_sock_free_ocsp_instance(ctx->cur_ocsp); + ssl_sock_free_ocsp(ctx->cur_ocsp); HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); /* Set next_wakeup to the new first entry of the tree */ eb = eb64_first(&ocsp_update_tree); @@ -1426,24 +1413,13 @@ static int cli_parse_update_ocsp_response(char **args, char *payload, struct app goto end; } - /* No need to try to update this response, it is already being updated. */ - if (!ocsp->updating) { - update_once = (ocsp->next_update.node.leaf_p == NULL); - eb64_delete(&ocsp->next_update); + update_once = (ocsp->next_update.node.leaf_p == NULL); + eb64_delete(&ocsp->next_update); - /* Insert the entry at the beginning of the update tree. - * We don't need to increase the reference counter on the - * certificate_ocsp structure because we would not have a way to - * decrease it afterwards since this update operation is asynchronous. - * If the corresponding entry were to be destroyed before the update can - * be performed, which is pretty unlikely, it would not be such a - * problem because that would mean that the OCSP response is not - * actually used. - */ - ocsp->next_update.key = 0; - eb64_insert(&ocsp_update_tree, &ocsp->next_update); - ocsp->update_once = update_once; - } + /* 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; HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); @@ -1569,7 +1545,7 @@ static int cli_parse_show_ocspresponse(char **args, char *payload, struct appctx HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); return cli_err(appctx, "Certificate ID or path does not match any certificate.\n"); } - ocsp->refcount_instance++; + ++ocsp->refcount; HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); ctx->ocsp = ocsp; @@ -1670,7 +1646,7 @@ yield: free_trash_chunk(tmp); BIO_free(bio); - ocsp->refcount_instance++; + ++ocsp->refcount; ctx->ocsp = ocsp; HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); return 0; @@ -1679,14 +1655,6 @@ yield: #endif } -static void cli_release_show_ocspresponse(struct appctx *appctx) -{ - struct show_ocspresp_cli_ctx *ctx = appctx->svcctx; - - if (ctx) - ssl_sock_free_ocsp(ctx->ocsp); -} - /* Check if the ckch_store and the entry does have the same configuration */ int ocsp_update_check_cfg_consistency(struct ckch_store *store, struct crtlist_entry *entry, char *crt_path, char **err) { @@ -1947,7 +1915,7 @@ smp_fetch_ssl_ocsp_success_cnt(const struct arg *args, struct sample *smp, const static struct cli_kw_list cli_kws = {{ },{ { { "set", "ssl", "ocsp-response", NULL }, "set ssl ocsp-response <resp|payload> : update a certificate's OCSP Response from a base64-encode DER", cli_parse_set_ocspresponse, NULL }, - { { "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, cli_release_show_ocspresponse }, + { { "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, NULL, NULL }, diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 6fbabb4..c7403b8 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1128,7 +1128,6 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * struct buffer *ocsp_uri = get_trash_chunk(); char *err = NULL; size_t path_len; - int inc_refcount_store = 0; x = data->cert; if (!x) @@ -1164,10 +1163,8 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * if (!issuer) goto out; - if (!data->ocsp_cid) { + if (!data->ocsp_cid) data->ocsp_cid = OCSP_cert_to_id(0, x, issuer); - inc_refcount_store = 1; - } if (!data->ocsp_cid) goto out; @@ -1194,9 +1191,6 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * #endif SSL_CTX_get_tlsext_status_cb(ctx, &callback); - if (inc_refcount_store) - iocsp->refcount_store++; - if (!callback) { struct ocsp_cbk_arg *cb_arg; EVP_PKEY *pkey; @@ -1207,7 +1201,7 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * cb_arg->is_single = 1; cb_arg->s_ocsp = iocsp; - iocsp->refcount_instance++; + iocsp->refcount++; pkey = X509_get_pubkey(x); cb_arg->single_kt = EVP_PKEY_base_id(pkey); @@ -1247,7 +1241,7 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data * index = ssl_sock_get_ocsp_arg_kt_index(key_type); if (index >= 0 && !cb_arg->m_ocsp[index]) { cb_arg->m_ocsp[index] = iocsp; - iocsp->refcount_instance++; + iocsp->refcount++; } } HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); |