From 8d7921e980b3fdab7c54ebd4cb3cb320b371e6bb Mon Sep 17 00:00:00 2001 From: Remi Tricot-Le Breton Date: Mon, 15 Sep 2025 15:32:40 +0200 Subject: [PATCH] BUG/MINOR: ocsp: Crash when updating CA during ocsp updates If an ocsp response is set to be updated automatically and some certificate or CA updates are performed on the CLI, if the CLI update happens while the OCSP response is being updated and is then detached from the udapte tree, it might be wrongly inserted into the update tree in 'ssl_sock_load_ocsp', and then reinserted when the update finishes. The update tree then gets corrupted and we could end up crashing when accessing other nodes in the ocsp response update tree. This patch must be backported up to 2.8. This patch fixes GitHub #3100. (cherry picked from commit 257df69fbdcff5c1feb3b24a4cf6141f86984447) Signed-off-by: Christopher Faulet (cherry picked from commit f885845e2b2c50996d3373eb5d2e84d3e7fcfbb8) Signed-off-by: Christopher Faulet (cherry picked from commit 7a2c66c5e90e2f69ba59dbdc6818e1f2dba27f41) Signed-off-by: Christopher Faulet --- include/haproxy/ssl_ocsp.h | 1 + src/ssl_ocsp.c | 15 +++++++++++++-- src/ssl_sock.c | 28 +++++++++++++++++++--------- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/include/haproxy/ssl_ocsp.h b/include/haproxy/ssl_ocsp.h index f6a72b9..2de9f6c 100644 --- a/include/haproxy/ssl_ocsp.h +++ b/include/haproxy/ssl_ocsp.h @@ -54,6 +54,7 @@ int ssl_create_ocsp_update_task(char **err); void ssl_destroy_ocsp_update_task(void); int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp); +int __ssl_ocsp_update_insert_unlocked(struct certificate_ocsp *ocsp); int ocsp_update_init(void *value, char *buf, struct ckch_data *d, int cli, char **err); diff --git a/src/ssl_ocsp.c b/src/ssl_ocsp.c index 9f79491..68aa423 100644 --- a/src/ssl_ocsp.c +++ b/src/ssl_ocsp.c @@ -976,8 +976,9 @@ static inline void ssl_ocsp_set_next_update(struct certificate_ocsp *ocsp) * the system too much (in theory). Likewise, a minimum 5 minutes interval is * defined in order to avoid updating too often responses that have a really * short expire time or even no 'Next Update' at all. + * The ocsp_update_tree lock must be taken by the caller. */ -int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) +int __ssl_ocsp_update_insert_unlocked(struct certificate_ocsp *ocsp) { /* Set next_update based on current time and the various OCSP * minimum/maximum update times. @@ -986,13 +987,23 @@ 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); + + return 0; +} + +/* + * Insert a certificate_ocsp structure into the ocsp_update_tree tree. + */ +int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) +{ + HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); + __ssl_ocsp_update_insert_unlocked(ocsp); HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); return 0; diff --git a/src/ssl_sock.c b/src/ssl_sock.c index d4c78cf..0db09b2 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1302,18 +1302,28 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_store * prior to the activation of the ocsp auto update and in such a * case we must "force" insertion in the auto update tree. */ + HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); if (iocsp->next_update.node.leaf_p == NULL) { - ssl_ocsp_update_insert(iocsp); - /* If we are during init the update task is not - * scheduled yet so a wakeup won't do anything. - * Otherwise, if the OCSP was added through the CLI, we - * wake the task up to manage the case of a new entry - * that needs to be updated before the previous first - * entry. + /* We might be facing an entry that is currently being + * updated, which can take some time (especially if the + * ocsp responder is unreachable). + * The entry will be reinserted by the update task, it + * mustn't be reinserted here. */ - if (ocsp_update_task) - task_wakeup(ocsp_update_task, TASK_WOKEN_MSG); + if (!iocsp->updating) { + __ssl_ocsp_update_insert_unlocked(iocsp); + /* If we are during init the update task is not + * scheduled yet so a wakeup won't do anything. + * Otherwise, if the OCSP was added through the CLI, we + * wake the task up to manage the case of a new entry + * that needs to be updated before the previous first + * entry. + */ + if (ocsp_update_task) + task_wakeup(ocsp_update_task, TASK_WOKEN_MSG); + } } + HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); } out: -- 1.7.10.4