revocation: Enforce a (configurable) timeout when fetching OCSP/CRL

Malicious servers could otherwise block the fetching thread indefinitely
after the initial TCP handshake (which has a default timeout of 10s
in the curl and winhttp plugins, the soup plugin actually has a default
overall timeout of 10s).
This commit is contained in:
Tobias Brunner 2022-07-22 14:50:41 +02:00
parent b1e926148a
commit 1968615590
4 changed files with 56 additions and 23 deletions

View File

@ -4,4 +4,5 @@ charon.plugins.revocation.enable_ocsp = yes
charon.plugins.revocation.enable_crl = yes
Whether CRL validation should be enabled.
charon.plugins.revocation.timeout = 10s
Timeout used when fetching OCSP/CRL.

View File

@ -134,6 +134,11 @@ METHOD(fetcher_t, set_option, bool,
this->request_type = strdup(va_arg(args, char*));
break;
}
case FETCH_TIMEOUT:
{
/* we already enforce a 10s timeout */
break;
}
default:
supported = FALSE;
break;

View File

@ -82,6 +82,9 @@ METHOD(fetcher_t, set_option, bool,
this->cb = va_arg(args, fetcher_callback_t);
break;
}
case FETCH_TIMEOUT:
/* irrelevant, but fine if requested */
break;
default:
supported = FALSE;
break;

View File

@ -29,6 +29,11 @@
#include <selectors/traffic_selector.h>
#include <threading/spinlock.h>
/**
* Default timeout in seconds when fetching OCSP/CRL.
*/
#define DEFAULT_TIMEOUT 10
typedef struct private_revocation_validator_t private_revocation_validator_t;
/**
@ -51,6 +56,11 @@ struct private_revocation_validator_t {
*/
bool enable_crl;
/**
* Timeout in seconds when fetching
*/
u_int timeout;
/**
* Lock to access flags
*/
@ -61,7 +71,7 @@ struct private_revocation_validator_t {
* Do an OCSP request
*/
static certificate_t *fetch_ocsp(char *url, certificate_t *subject,
certificate_t *issuer)
certificate_t *issuer, u_int timeout)
{
certificate_t *request, *response;
ocsp_request_t *ocsp_request;
@ -90,6 +100,7 @@ static certificate_t *fetch_ocsp(char *url, certificate_t *subject,
if (lib->fetcher->fetch(lib->fetcher, url, &receive,
FETCH_REQUEST_DATA, send,
FETCH_REQUEST_TYPE, "application/ocsp-request",
FETCH_TIMEOUT, timeout,
FETCH_END) != SUCCESS)
{
DBG1(DBG_CFG, "ocsp request to %s failed", url);
@ -295,7 +306,7 @@ static certificate_t *get_better_ocsp(certificate_t *cand, certificate_t *best,
* validate a x509 certificate using OCSP
*/
static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
auth_cfg_t *auth)
auth_cfg_t *auth, u_int timeout)
{
enumerator_t *enumerator;
cert_validation_t valid = VALIDATION_SKIPPED;
@ -334,7 +345,8 @@ static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
CERT_X509_OCSP_RESPONSE, keyid);
while (enumerator->enumerate(enumerator, &uri))
{
current = fetch_ocsp(uri, &subject->interface, &issuer->interface);
current = fetch_ocsp(uri, &subject->interface, &issuer->interface,
timeout);
if (current)
{
best = get_better_ocsp(current, best, subject, issuer,
@ -356,7 +368,8 @@ static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
enumerator = subject->create_ocsp_uri_enumerator(subject);
while (enumerator->enumerate(enumerator, &uri))
{
current = fetch_ocsp(uri, &subject->interface, &issuer->interface);
current = fetch_ocsp(uri, &subject->interface, &issuer->interface,
timeout);
if (current)
{
best = get_better_ocsp(current, best, subject, issuer,
@ -386,13 +399,15 @@ static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
/**
* fetch a CRL from an URL
*/
static certificate_t* fetch_crl(char *url)
static certificate_t* fetch_crl(char *url, u_int timeout)
{
certificate_t *crl;
chunk_t chunk = chunk_empty;
DBG1(DBG_CFG, " fetching crl from '%s' ...", url);
if (lib->fetcher->fetch(lib->fetcher, url, &chunk, FETCH_END) != SUCCESS)
if (lib->fetcher->fetch(lib->fetcher, url, &chunk,
FETCH_TIMEOUT, timeout,
FETCH_END) != SUCCESS)
{
DBG1(DBG_CFG, "crl fetching failed");
chunk_free(&chunk);
@ -571,7 +586,7 @@ static certificate_t *get_better_crl(certificate_t *cand, certificate_t *best,
*/
static cert_validation_t find_crl(x509_t *subject, identification_t *issuer,
crl_t *base, certificate_t **best,
bool *uri_found)
bool *uri_found, u_int timeout)
{
cert_validation_t valid = VALIDATION_SKIPPED;
enumerator_t *enumerator;
@ -601,7 +616,7 @@ static cert_validation_t find_crl(x509_t *subject, identification_t *issuer,
while (enumerator->enumerate(enumerator, &uri))
{
*uri_found = TRUE;
current = fetch_crl(uri);
current = fetch_crl(uri, timeout);
if (current)
{
if (!current->has_issuer(current, issuer))
@ -653,7 +668,8 @@ static bool check_issuer(certificate_t *crl, x509_t *issuer, x509_cdp_t *cdp)
* Look for a delta CRL for a given base CRL
*/
static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
crl_t *base, cert_validation_t base_valid)
crl_t *base, cert_validation_t base_valid,
u_int timeout)
{
cert_validation_t valid = VALIDATION_SKIPPED;
certificate_t *best = NULL, *current, *cissuer = (certificate_t*)issuer;
@ -668,7 +684,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
if (chunk.len)
{
id = identification_create_from_encoding(ID_KEY_ID, chunk);
valid = find_crl(subject, id, base, &best, &uri);
valid = find_crl(subject, id, base, &best, &uri, timeout);
id->destroy(id);
}
@ -679,7 +695,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
{
if (cdp->issuer)
{
valid = find_crl(subject, cdp->issuer, base, &best, &uri);
valid = find_crl(subject, cdp->issuer, base, &best, &uri, timeout);
}
}
enumerator->destroy(enumerator);
@ -689,7 +705,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
while (valid != VALIDATION_GOOD && valid != VALIDATION_REVOKED &&
enumerator->enumerate(enumerator, &cdp))
{
current = fetch_crl(cdp->uri);
current = fetch_crl(cdp->uri, timeout);
if (current)
{
if (!check_issuer(current, issuer, cdp))
@ -722,7 +738,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
* validate a x509 certificate using CRL
*/
static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
auth_cfg_t *auth)
auth_cfg_t *auth, u_int timeout)
{
cert_validation_t valid = VALIDATION_SKIPPED;
certificate_t *best = NULL, *cissuer = (certificate_t*)issuer;
@ -738,7 +754,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
if (chunk.len)
{
id = identification_create_from_encoding(ID_KEY_ID, chunk);
valid = find_crl(subject, id, NULL, &best, &uri_found);
valid = find_crl(subject, id, NULL, &best, &uri_found, timeout);
id->destroy(id);
}
@ -749,7 +765,8 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
{
if (cdp->issuer)
{
valid = find_crl(subject, cdp->issuer, NULL, &best, &uri_found);
valid = find_crl(subject, cdp->issuer, NULL, &best, &uri_found,
timeout);
}
}
enumerator->destroy(enumerator);
@ -761,7 +778,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
while (enumerator->enumerate(enumerator, &cdp))
{
uri_found = TRUE;
current = fetch_crl(cdp->uri);
current = fetch_crl(cdp->uri, timeout);
if (current)
{
if (!check_issuer(current, issuer, cdp))
@ -787,7 +804,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
/* look for delta CRLs */
if (best && (valid == VALIDATION_GOOD || valid == VALIDATION_STALE))
{
valid = check_delta_crl(subject, issuer, (crl_t*)best, valid);
valid = check_delta_crl(subject, issuer, (crl_t*)best, valid, timeout);
}
/* an uri was found, but no result. switch validation state to failed */
@ -815,10 +832,12 @@ METHOD(cert_validator_t, validate, bool,
auth_cfg_t *auth)
{
bool enable_ocsp, enable_crl;
u_int timeout;
this->lock->lock(this->lock);
enable_ocsp = this->enable_ocsp;
enable_crl = this->enable_crl;
timeout = this->timeout;
this->lock->unlock(this->lock);
if (online && (enable_ocsp || enable_crl) &&
@ -830,7 +849,7 @@ METHOD(cert_validator_t, validate, bool,
if (enable_ocsp)
{
switch (check_ocsp((x509_t*)subject, (x509_t*)issuer, auth))
switch (check_ocsp((x509_t*)subject, (x509_t*)issuer, auth, timeout))
{
case VALIDATION_GOOD:
DBG1(DBG_CFG, "certificate status is good");
@ -859,7 +878,7 @@ METHOD(cert_validator_t, validate, bool,
if (enable_crl)
{
switch (check_crl((x509_t*)subject, (x509_t*)issuer, auth))
switch (check_crl((x509_t*)subject, (x509_t*)issuer, auth, timeout))
{
case VALIDATION_GOOD:
DBG1(DBG_CFG, "certificate status is good");
@ -895,15 +914,20 @@ METHOD(revocation_validator_t, reload, void,
private_revocation_validator_t *this)
{
bool enable_ocsp, enable_crl;
u_int timeout;
enable_ocsp = lib->settings->get_bool(lib->settings,
"%s.plugins.revocation.enable_ocsp", TRUE, lib->ns);
enable_crl = lib->settings->get_bool(lib->settings,
"%s.plugins.revocation.enable_crl", TRUE, lib->ns);
"%s.plugins.revocation.enable_ocsp", TRUE, lib->ns);
enable_crl = lib->settings->get_bool(lib->settings,
"%s.plugins.revocation.enable_crl", TRUE, lib->ns);
timeout = lib->settings->get_time(lib->settings,
"%s.plugins.revocation.timeout", DEFAULT_TIMEOUT,
lib->ns);
this->lock->lock(this->lock);
this->enable_ocsp = enable_ocsp;
this->enable_crl = enable_crl;
this->timeout = timeout;
this->lock->unlock(this->lock);
if (!enable_ocsp)