From: Martin Willi Date: Wed, 29 Oct 2025 08:48:31 +0000 (+0100) Subject: revocation: Synchronize CRL fetches of multiple threads to the same URL X-Git-Tag: 6.0.4rc1~5^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=330a7d1963032d03b8eae32e86346b99dc826b26;p=thirdparty%2Fstrongswan.git revocation: Synchronize CRL fetches of multiple threads to the same URL When handling many connection attempts from peers using the same CA, a slow or non-responsive CRL distribution point can lead to concurrent fetches of the same CRL by multiple threads. This is not only inefficient, but results in all threads blocking for the full fetch timeout, potentially blocking all threads in the pool. As a first step, synchronize CRL fetches using a global mutex and a per-URL condvar, so threads can wait for the CRL if another is already fetching it. This reduces the number of useless concurrent CRL fetches, and allows threads joining the party late to get blocked only until the first fetch completes or times out. The URL entry is preserved in the hashtable after completing the fetch. This will allow subsequent optimizations to store the last fetch result and act accordingly. The CRL itself is not, as CRLs can be rather large and caching them can be done using existing mechanisms controlled via corresponding options. --- diff --git a/src/libstrongswan/plugins/revocation/revocation_fetcher.c b/src/libstrongswan/plugins/revocation/revocation_fetcher.c index 42620d35fd..8d0cd602bc 100644 --- a/src/libstrongswan/plugins/revocation/revocation_fetcher.c +++ b/src/libstrongswan/plugins/revocation/revocation_fetcher.c @@ -19,6 +19,9 @@ #include "revocation_fetcher.h" #include +#include +#include +#include #include #include #include @@ -34,10 +37,51 @@ struct private_revocation_fetcher_t { * Public revocation_fetcher_t interface. */ revocation_fetcher_t public; + + /** + * Mutex to synchronize CRL fetches + */ + mutex_t *mutex; + + /** + * Active/completed/failed CRL fetches, crl_fetch_t. + */ + hashtable_t *crls; }; -METHOD(revocation_fetcher_t, fetch_crl, certificate_t *, - private_revocation_fetcher_t *this, char *url, u_int timeout) +typedef struct crl_fetch_t crl_fetch_t; + +/** + * Represents an active/completed/failed CRL fetch. + */ +struct crl_fetch_t { + + /** + * URL of the CRL. + */ + char *url; + + /** + * Condition variable to signal completion of the fetch. + */ + condvar_t *condvar; + + /** + * Number of threads fetching this CRL. + */ + u_int fetchers; + + /** + * CRL received in the currently active fetch. + */ + certificate_t *crl; +}; + +/** + * Perform the actual CRL fetch from the given URL. + */ +static certificate_t *do_crl_fetch(private_revocation_fetcher_t *this, + char *url, u_int timeout) { certificate_t *crl; chunk_t chunk = chunk_empty; @@ -62,6 +106,82 @@ METHOD(revocation_fetcher_t, fetch_crl, certificate_t *, return crl; } +/** + * Start a new CRL fetch and signal completion to waiting threads. + */ +static certificate_t *start_crl_fetch(private_revocation_fetcher_t *this, + crl_fetch_t *fetch, u_int timeout) +{ + certificate_t *crl; + + fetch->fetchers++; + this->mutex->unlock(this->mutex); + crl = do_crl_fetch(this, fetch->url, timeout); + this->mutex->lock(this->mutex); + fetch->crl = crl; + while (fetch->fetchers > 1) + { + fetch->condvar->signal(fetch->condvar); + fetch->condvar->wait(fetch->condvar, this->mutex); + } + fetch->fetchers--; + fetch->crl = NULL; + return crl; +} + +/** + * Wait for a CRL fetch performed by another thread to complete. + */ +static certificate_t *wait_for_crl(private_revocation_fetcher_t *this, + crl_fetch_t *fetch) +{ + certificate_t *crl = NULL; + + DBG1(DBG_CFG, " waiting for crl fetch from '%s' ...", fetch->url); + if (fetch->crl) + { + /* fetch is already complete, no need to wait */ + return fetch->crl->get_ref(fetch->crl); + } + fetch->fetchers++; + fetch->condvar->wait(fetch->condvar, this->mutex); + fetch->fetchers--; + if (fetch->crl) + { + crl = fetch->crl->get_ref(fetch->crl); + } + fetch->condvar->signal(fetch->condvar); + return crl; +} + +METHOD(revocation_fetcher_t, fetch_crl, certificate_t*, + private_revocation_fetcher_t *this, char *url, u_int timeout) +{ + certificate_t *crl; + crl_fetch_t *fetch; + + this->mutex->lock(this->mutex); + fetch = this->crls->get(this->crls, url); + if (!fetch) + { + INIT(fetch, + .url = strdup(url), + .condvar = condvar_create(CONDVAR_TYPE_DEFAULT), + ); + this->crls->put(this->crls, fetch->url, fetch); + } + if (fetch->fetchers) + { + crl = wait_for_crl(this, fetch); + } + else + { + crl = start_crl_fetch(this, fetch, timeout); + } + this->mutex->unlock(this->mutex); + return crl; +} + METHOD(revocation_fetcher_t, fetch_ocsp, certificate_t*, private_revocation_fetcher_t *this, char *url, certificate_t *subject, certificate_t *issuer, u_int timeout) @@ -134,9 +254,18 @@ METHOD(revocation_fetcher_t, fetch_ocsp, certificate_t*, return response; } +CALLBACK(crl_fetch_destroy, void, crl_fetch_t *fetch, const void *key) +{ + fetch->condvar->destroy(fetch->condvar); + free(fetch->url); + free(fetch); +} + METHOD(revocation_fetcher_t, destroy, void, private_revocation_fetcher_t *this) { + this->crls->destroy_function(this->crls, crl_fetch_destroy); + this->mutex->destroy(this->mutex); free(this); } @@ -153,6 +282,8 @@ revocation_fetcher_t *revocation_fetcher_create() .fetch_ocsp = _fetch_ocsp, .destroy = _destroy, }, + .mutex = mutex_create(MUTEX_TYPE_DEFAULT), + .crls = hashtable_create(hashtable_hash_str, hashtable_equals_str, 8), ); return &this->public;