diff --git a/include/haproxy/ssl_ocsp.h b/include/haproxy/ssl_ocsp.h index e7828b6f0..c7526bf11 100644 --- a/include/haproxy/ssl_ocsp.h +++ b/include/haproxy/ssl_ocsp.h @@ -53,7 +53,7 @@ int ssl_ocsp_check_response(STACK_OF(X509) *chain, X509 *issuer, 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(struct certificate_ocsp *ocsp, int needs_locking); int ocsp_update_init(void *value, char *buf, struct ckch_data *d, int cli, const char *filename, int linenum, char **err); diff --git a/src/ssl_ocsp.c b/src/ssl_ocsp.c index c2fe8d105..52a245a65 100644 --- a/src/ssl_ocsp.c +++ b/src/ssl_ocsp.c @@ -1048,7 +1048,7 @@ static inline void ssl_ocsp_set_next_update(struct certificate_ocsp *ocsp) * defined in order to avoid updating too often responses that have a really * short expire time or even no 'Next Update' at all. */ -int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) +int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp, int needs_locking) { /* Set next_update based on current time and the various OCSP * minimum/maximum update times. @@ -1057,14 +1057,16 @@ int ssl_ocsp_update_insert(struct certificate_ocsp *ocsp) ocsp->fail_count = 0; - HA_SPIN_LOCK(OCSP_LOCK, &ocsp_tree_lock); + if (needs_locking) + 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); - HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); + if (needs_locking) + HA_SPIN_UNLOCK(OCSP_LOCK, &ocsp_tree_lock); return 0; } @@ -1292,7 +1294,7 @@ static struct task *ssl_ocsp_update_responses(struct task *task, void *context, ssl_ocsp_send_log(); /* Reinsert the entry into the update list so that it can be updated later */ - ssl_ocsp_update_insert(ocsp); + ssl_ocsp_update_insert(ocsp, 0); /* Release the reference kept on the updated ocsp response. */ ssl_sock_free_ocsp_instance(ctx->cur_ocsp); ctx->cur_ocsp = NULL; diff --git a/src/ssl_sock.c b/src/ssl_sock.c index ecaf24ee9..6dce3cb98 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1499,7 +1499,7 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_store memcpy(iocsp->path, path, path_len + 1); if (enable_auto_update) { - ssl_ocsp_update_insert(iocsp); + ssl_ocsp_update_insert(iocsp, 1); /* 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 @@ -1517,18 +1517,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(iocsp, 0); + /* 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: