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 5d2b8c02684732551fe9cc05c0f59cfed4baf09a..97b29e1ccb8e27ba9a966ab3cdbf36d0a8eafc73 100644 (file)
--- 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 2a72ac6a16105bd035944eb6c6131cbff5d54abf..e039fff2c50a19ab4b7ff7b60dfb4f09e10e0c26 100644 (file)
--- 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 1f25d6f102bdd734ed0bce7bfaed2e254442bd99..06b63d17f25fbafe27901c896d0fdbb6343ffeee 100644 (file)
--- 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 454e83c367dccaa3ccf264b7503ebad0c710d72c..b554f406f4ed994835bdeccfab4dfb448257a78e 100644 (file)
--- a/src/libstrongswan/plugins/revocation/revocation_validator.c
+++ b/src/libstrongswan/plugins/revocation/revocation_validator.c
@@ -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)