wip: revocation: Optionally require nonces in OCSP responses

To prevent potential replay attacks it might be preferable to actually
require that nonces are contained in OCSP responses.  Otherwise, if the
OCSP server replies to requests with and without nonces, and depending on
the lifetime of the responses, attackers could send cached responses they
requested without nonce as reply to our requests with nonces.

wip: As mentioned above, this is only a problem if servers reply to requests
with and without nonce. So if this is a concern, it might be an option to
configure the server to not reply to requests that don't contain a nonce (if
possible, the OpenSSL OCSP server does not seem to have an option to do so).

diff --git a/conf/plugins/revocation.opt b/conf/plugins/revocation.opt
index 5d2b8c02684732551fe9cc05c0f59cfed4baf09a..8956f5e9576cbb25b2b21064c255fd8b0a3a676e 100644 (file)
--- a/conf/plugins/revocation.opt
+++ b/conf/plugins/revocation.opt
@@ -4,4 +4,11 @@ charon.plugins.revocation.enable_ocsp = yes
 charon.plugins.revocation.enable_crl = yes
        Whether CRL validation should be enabled.
 
+charon.plugins.revocation.require_ocsp_nonce = no
+       Whether nonces are required in OCSP responses to prevent replay attacks.
 
+       Whether nonces are required in OCSP responses to prevent replay attacks.
+       Many commercial CAs don't support replying with nonces (most likely because
+       they use cached OCSP responses). Nonces are always sent in the requests, but
+       if this option is disabled, strongSwan accepts valid replies without nonce
+       extension.

diff --git a/src/libstrongswan/plugins/revocation/revocation_validator.c b/src/libstrongswan/plugins/revocation/revocation_validator.c
index 2e90fb50d32880058fa220f82d11dadaa24c6418..533e78c906a8d2d3c7d316e3c1c4b75047272b17 100644 (file)
--- a/src/libstrongswan/plugins/revocation/revocation_validator.c
+++ b/src/libstrongswan/plugins/revocation/revocation_validator.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2018 Tobias Brunner
+ * Copyright (C) 2015-2020 Tobias Brunner
  * Copyright (C) 2010 Martin Willi
  * Copyright (C) 2010 revosec AG
  * Copyright (C) 2009 Andreas Steffen
@@ -46,6 +46,11 @@ struct private_revocation_validator_t {
         */
        bool enable_ocsp;
 
+       /**
+        * Require nonces in OCSP replies
+        */
+       bool require_nonce;
+
        /**
         * Enable CRL validation
         */
@@ -61,7 +66,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, bool require_nonce)
 {
        certificate_t *request, *response;
        ocsp_request_t *ocsp_request;
@@ -112,11 +117,19 @@ static certificate_t *fetch_ocsp(char *url, certificate_t *subject,
        }
        ocsp_request = (ocsp_request_t*)request;
        ocsp_response = (ocsp_response_t*)response;
-       if (ocsp_response->get_nonce(ocsp_response).len &&
-               !chunk_equals_const(ocsp_request->get_nonce(ocsp_request),
-                                                       ocsp_response->get_nonce(ocsp_response)))
+       if (ocsp_response->get_nonce(ocsp_response).len)
+       {
+               if (!chunk_equals_const(ocsp_request->get_nonce(ocsp_request),
+                                                               ocsp_response->get_nonce(ocsp_response)))
+               {
+                       DBG1(DBG_CFG, "nonce in ocsp response doesn't match");
+                       request->destroy(request);
+                       return NULL;
+               }
+       }
+       else if (require_nonce)
        {
-               DBG1(DBG_CFG, "nonce in ocsp response doesn't match");
+               DBG1(DBG_CFG, "nonce in ocsp response missing");
                request->destroy(request);
                return NULL;
        }
@@ -295,7 +308,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, bool require_nonce)
 {
        enumerator_t *enumerator;
        cert_validation_t valid = VALIDATION_SKIPPED;
@@ -334,7 +347,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,
+                                                                require_nonce);
                        if (current)
                        {
                                best = get_better_ocsp(current, best, subject, issuer,
@@ -356,7 +370,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,
+                                                                require_nonce);
                        if (current)
                        {
                                best = get_better_ocsp(current, best, subject, issuer,
@@ -814,10 +829,11 @@ METHOD(cert_validator_t, validate, bool,
        certificate_t *issuer, bool online, u_int pathlen, bool anchor,
        auth_cfg_t *auth)
 {
-       bool enable_ocsp, enable_crl;
+       bool enable_ocsp, require_nonce, enable_crl;
 
        this->lock->lock(this->lock);
        enable_ocsp = this->enable_ocsp;
+       require_nonce = this->require_nonce;
        enable_crl = this->enable_crl;
        this->lock->unlock(this->lock);
 
@@ -830,7 +846,8 @@ 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,
+                                                          require_nonce))
                        {
                                case VALIDATION_GOOD:
                                        DBG1(DBG_CFG, "certificate status is good");
@@ -894,15 +911,19 @@ METHOD(cert_validator_t, validate, bool,
 METHOD(revocation_validator_t, reload, void,
        private_revocation_validator_t *this)
 {
-       bool enable_ocsp, enable_crl;
+       bool enable_ocsp, require_nonce, enable_crl;
 
        enable_ocsp = lib->settings->get_bool(lib->settings,
                                                        "%s.plugins.revocation.enable_ocsp", TRUE, lib->ns);
+       require_nonce = lib->settings->get_bool(lib->settings,
+                                                       "%s.plugins.revocation.require_ocsp_nonce", FALSE,
+                                                       lib->ns);
        enable_crl  = lib->settings->get_bool(lib->settings,
                                                        "%s.plugins.revocation.enable_crl",  TRUE, lib->ns);
 
        this->lock->lock(this->lock);
        this->enable_ocsp = enable_ocsp;
+       this->require_nonce = require_nonce;
        this->enable_crl = enable_crl;
        this->lock->unlock(this->lock);
 
@@ -910,6 +931,11 @@ METHOD(revocation_validator_t, reload, void,
        {
                DBG1(DBG_LIB, "all OCSP validation disabled");
        }
+       else
+       {
+               DBG1(DBG_LIB, "nonces are %srequired in OCSP replies",
+                        require_nonce ? "" : "not ");
+       }
        if (!enable_crl)
        {
                DBG1(DBG_LIB, "all CRL validation disabled");