[PATCH] OCSP stapling: never return an expired OCSP response (ticket #425)

[Andrew Ayer]

# HG changeset patch
# User Andrew Ayer <agwa at andrewayer.name>
# Date 1433458802 25200
# Node ID f231e66871bcac1aa6db81e6153f818b417f8338
# Parent  d811f22033ad2eb5c70dc4bbdda31c949a127ddb
OCSP stapling: never return an expired OCSP response (ticket #425)

Previously, once an OCSP response was cached, it would be sent to
clients until a new response replaced it, even if the response expired.
This is bad, since Firefox displays an interstitial SSL error when it
receives an expired stapled OCSP response, and Chrome is planning to do
the same as well.

With this change, nginx now caches the expiration time of the OCSP
response, and re-checks it before sending the response to the client.
If the response is expired, it's immediately cleared from the cache,
which prevents it from being returned, and triggers an immediate update
from the OCSP responder.

diff -r d811f22033ad -r f231e66871bc src/event/ngx_event_openssl_stapling.c
--- a/src/event/ngx_event_openssl_stapling.c	Wed Jun 03 19:12:26 2015 +0300
+++ b/src/event/ngx_event_openssl_stapling.c	Thu Jun 04 16:00:02 2015 -0700
@@ -18,6 +18,9 @@
     ngx_str_t                    staple;
     ngx_msec_t                   timeout;
 
+    ASN1_GENERALIZEDTIME        *thisupdate;
+    ASN1_GENERALIZEDTIME        *nextupdate;
+
     ngx_resolver_t              *resolver;
     ngx_msec_t                   resolver_timeout;
 
@@ -462,6 +465,27 @@
     staple = data;
     rc = SSL_TLSEXT_ERR_NOACK;
 
+    if (staple->thisupdate && staple->nextupdate
+        && OCSP_check_validity(staple->thisupdate,
+                               staple->nextupdate, 300, -1) != 1)
+    {
+        ngx_ssl_error(NGX_LOG_ERR, c->log, 0, "OCSP_check_validity() failed");
+        /*
+         * free the cached response so that we immediately try to retrieve
+         * a new one instead of waiting until the cache expires
+         */
+        if (staple->staple.data) {
+            ngx_free(staple->staple.data);
+            staple->staple.data = NULL;
+        }
+        staple->staple.len = 0;
+        staple->valid = 0;
+        ASN1_GENERALIZEDTIME_free(staple->thisupdate);
+        staple->thisupdate = NULL;
+        ASN1_GENERALIZEDTIME_free(staple->nextupdate);
+        staple->nextupdate = NULL;
+    }
+
     if (staple->staple.len) {
         /* we have to copy ocsp response as OpenSSL will free it by itself */
 
@@ -629,6 +653,26 @@
         goto error;
     }
 
+    if (staple->thisupdate) {
+        ASN1_GENERALIZEDTIME_free(staple->thisupdate);
+    }
+    staple->thisupdate = M_ASN1_GENERALIZEDTIME_dup(thisupdate);
+    if (staple->thisupdate == NULL) {
+        ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
+                      "M_ASN1_GENERALIZEDTIME_dup() failed");
+        goto error;
+    }
+
+    if (staple->nextupdate) {
+        ASN1_GENERALIZEDTIME_free(staple->nextupdate);
+    }
+    staple->nextupdate = M_ASN1_GENERALIZEDTIME_dup(nextupdate);
+    if (staple->nextupdate == NULL) {
+        ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
+                      "M_ASN1_GENERALIZEDTIME_dup() failed");
+        goto error;
+    }
+
     OCSP_CERTID_free(id);
     OCSP_BASICRESP_free(basic);
     OCSP_RESPONSE_free(ocsp);
@@ -692,6 +736,13 @@
         X509_free(staple->issuer);
     }
 
+    if (staple->thisupdate) {
+        ASN1_GENERALIZEDTIME_free(staple->thisupdate);
+    }
+    if (staple->nextupdate) {
+        ASN1_GENERALIZEDTIME_free(staple->nextupdate);
+    }
+
     if (staple->staple.data) {
         ngx_free(staple->staple.data);
     }