From: Tobias Brunner Date: Fri, 22 Jul 2022 12:50:41 +0000 (+0200) Subject: revocation: Enforce a (configurable) timeout when fetching OCSP/CRL X-Git-Tag: 5.9.8~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19686155906ed2e4f113c78d69803d7d0af0e48e;p=thirdparty%2Fstrongswan.git 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). --- diff --git a/conf/plugins/revocation.opt b/conf/plugins/revocation.opt index 5d2b8c0268..97b29e1ccb 100644 --- a/conf/plugins/revocation.opt +++ b/conf/plugins/revocation.opt @@ -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. diff --git a/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_fetcher.c b/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_fetcher.c index 2a72ac6a16..e039fff2c5 100644 --- a/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_fetcher.c +++ b/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_fetcher.c @@ -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; diff --git a/src/libstrongswan/plugins/files/files_fetcher.c b/src/libstrongswan/plugins/files/files_fetcher.c index 1f25d6f102..06b63d17f2 100644 --- a/src/libstrongswan/plugins/files/files_fetcher.c +++ b/src/libstrongswan/plugins/files/files_fetcher.c @@ -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; diff --git a/src/libstrongswan/plugins/revocation/revocation_validator.c b/src/libstrongswan/plugins/revocation/revocation_validator.c index 454e83c367..b554f406f4 100644 --- a/src/libstrongswan/plugins/revocation/revocation_validator.c +++ b/src/libstrongswan/plugins/revocation/revocation_validator.c @@ -29,6 +29,11 @@ #include #include +/** + * 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)