[PATCH] OCSP stapling: better handling of successful OCSP responses.

Piotr Sikora piotr at cloudflare.com
Wed May 22 00:17:52 UTC 2013


Hey Maxim,

> You may consider adding a control for this

Updated changeset attached below.

> e.g., it looks like Apache has something similar called
> SSLStaplingReturnResponderErrors:
>
> https://httpd.apache.org/docs/trunk/mod/mod_ssl.html#sslstaplingreturnrespondererrors

I didn't test it, but both: directive's name and description suggest
that this is different behavior from the one that I suggested, i.e.
with this turned on Apache would return all OCSP responses, regardless
of _their_ status (malformed responses, expired responses, etc).

Best regards,
Piotr Sikora


# HG changeset patch
# User Piotr Sikora <piotr at cloudflare.com>
# Date 1369181290 25200
# Node ID 7f52714a0b6a4229ab2fa9e640edbf80060eaf34
# Parent  1d68b502088c9d6e6603e9699354e36d03d77f9c
OCSP stapling: add ssl_stapling_strict directive.

When turned on (default), nginx will cache only OCSP responeses
with "good" certificate status.

When turned off, nginx will cache all successful OCSP responseses,
regardless of the certificate status.

While there, log certificate's common name and revocation reason,
because certificate status alone isn't very useful information.

Signed-off-by: Piotr Sikora <piotr at cloudflare.com>

diff -r 1d68b502088c -r 7f52714a0b6a src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h     Tue May 21 21:47:50 2013 +0400
+++ b/src/event/ngx_event_openssl.h     Tue May 21 17:08:10 2013 -0700
@@ -106,7 +106,8 @@
     ngx_str_t *cert, ngx_int_t depth);
 ngx_int_t ngx_ssl_crl(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *crl);
 ngx_int_t ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl,
-    ngx_str_t *file, ngx_str_t *responder, ngx_uint_t verify);
+    ngx_str_t *file, ngx_str_t *responder, ngx_uint_t verify,
+    ngx_uint_t strict);
 ngx_int_t ngx_ssl_stapling_resolver(ngx_conf_t *cf, ngx_ssl_t *ssl,
     ngx_resolver_t *resolver, ngx_msec_t resolver_timeout);
 RSA *ngx_ssl_rsa512_key_callback(SSL *ssl, int is_export, int key_length);
diff -r 1d68b502088c -r 7f52714a0b6a src/event/ngx_event_openssl_stapling.c
--- a/src/event/ngx_event_openssl_stapling.c    Tue May 21 21:47:50 2013 +0400
+++ b/src/event/ngx_event_openssl_stapling.c    Tue May 21 17:08:10 2013 -0700
@@ -34,6 +34,7 @@
     time_t                       valid;

     unsigned                     verify:1;
+    unsigned                     strict:1;
     unsigned                     loading:1;
 } ngx_ssl_stapling_t;

@@ -116,7 +117,7 @@

 ngx_int_t
 ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file,
-    ngx_str_t *responder, ngx_uint_t verify)
+    ngx_str_t *responder, ngx_uint_t verify, ngx_uint_t strict)
 {
     ngx_int_t                  rc;
     ngx_pool_cleanup_t        *cln;
@@ -146,6 +147,7 @@
     staple->ssl_ctx = ssl->ctx;
     staple->timeout = 60000;
     staple->verify = verify;
+    staple->strict = strict;

     if (file->len) {
         /* use OCSP response from the file */
@@ -529,7 +531,7 @@
     const
 #endif
     u_char                *p;
-    int                    n;
+    int                    n, r, idx;
     size_t                 len;
     ngx_str_t              response;
     X509_STORE            *store;
@@ -539,6 +541,10 @@
     OCSP_BASICRESP        *basic;
     ngx_ssl_stapling_t    *staple;
     ASN1_GENERALIZEDTIME  *thisupdate, *nextupdate;
+    X509_NAME             *name;
+    X509_NAME_ENTRY       *entry;
+    ASN1_STRING           *str;
+    ngx_str_t              s;

     staple = ctx->data;
     ocsp = NULL;
@@ -606,7 +612,7 @@
         goto error;
     }

-    if (OCSP_resp_find_status(basic, id, &n, NULL, NULL,
+    if (OCSP_resp_find_status(basic, id, &n, &r, NULL,
                               &thisupdate, &nextupdate)
         != 1)
     {
@@ -615,19 +621,47 @@
         goto error;
     }

-    if (n != V_OCSP_CERTSTATUS_GOOD) {
-        ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
-                      "certificate status \"%s\" in the OCSP response",
-                      OCSP_cert_status_str(n));
-        goto error;
-    }
-
     if (OCSP_check_validity(thisupdate, nextupdate, 300, -1) != 1) {
         ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
                       "OCSP_check_validity() failed");
         goto error;
     }

+    if (n != V_OCSP_CERTSTATUS_GOOD) {
+        ngx_str_set(&s, "unknown");
+
+        if (ctx->cert) {
+            name = X509_get_subject_name(ctx->cert);
+            if (name) {
+                idx = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
+                if (idx != -1) {
+                    entry = X509_NAME_get_entry(name, idx);
+                    if (entry) {
+                        str = X509_NAME_ENTRY_get_data(entry);
+                        s.data = ASN1_STRING_data(str);
+                        s.len = ASN1_STRING_length(str);
+                    }
+                }
+            }
+        }
+
+        if (n == V_OCSP_CERTSTATUS_REVOKED && r != -1) {
+            ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+                          "certificate status \"%s\" (reason: \"%s\") in the "
+                          "OCSP response for \"%V\"",
+                          OCSP_cert_status_str(n), OCSP_crl_reason_str(r), &s);
+
+        } else {
+            ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+                          "certificate status \"%s\" in the OCSP response "
+                          "for \"%V\"", OCSP_cert_status_str(n), &s);
+        }
+
+        if (staple->strict) {
+            goto error;
+        }
+    }
+
     OCSP_CERTID_free(id);
     OCSP_BASICRESP_free(basic);
     OCSP_RESPONSE_free(ocsp);
@@ -1729,7 +1763,7 @@

 ngx_int_t
 ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file,
-    ngx_str_t *responder, ngx_uint_t verify)
+    ngx_str_t *responder, ngx_uint_t verify, ngx_uint_t strict)
 {
     ngx_log_error(NGX_LOG_WARN, ssl->log, 0,
                   "\"ssl_stapling\" ignored, not supported");
diff -r 1d68b502088c -r 7f52714a0b6a src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c    Tue May 21 21:47:50 2013 +0400
+++ b/src/http/modules/ngx_http_ssl_module.c    Tue May 21 17:08:10 2013 -0700
@@ -195,6 +195,13 @@
       offsetof(ngx_http_ssl_srv_conf_t, stapling_verify),
       NULL },

+    { ngx_string("ssl_stapling_strict"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_SRV_CONF_OFFSET,
+      offsetof(ngx_http_ssl_srv_conf_t, stapling_strict),
+      NULL },
+
       ngx_null_command
 };

@@ -423,6 +430,7 @@
     sscf->session_timeout = NGX_CONF_UNSET;
     sscf->stapling = NGX_CONF_UNSET;
     sscf->stapling_verify = NGX_CONF_UNSET;
+    sscf->stapling_strict = NGX_CONF_UNSET;

     return sscf;
 }
@@ -478,6 +486,7 @@

     ngx_conf_merge_value(conf->stapling, prev->stapling, 0);
     ngx_conf_merge_value(conf->stapling_verify, prev->stapling_verify, 0);
+    ngx_conf_merge_value(conf->stapling_strict, prev->stapling_strict, 1);
     ngx_conf_merge_str_value(conf->stapling_file, prev->stapling_file, "");
     ngx_conf_merge_str_value(conf->stapling_responder,
                          prev->stapling_responder, "");
@@ -624,8 +633,9 @@

     if (conf->stapling) {

-        if (ngx_ssl_stapling(cf, &conf->ssl, &conf->stapling_file,
-                             &conf->stapling_responder, conf->stapling_verify)
+        if (ngx_ssl_stapling(cf, &conf->ssl,&conf->stapling_file,
+                             &conf->stapling_responder, conf->stapling_verify,
+                             conf->stapling_strict)
             != NGX_OK)
         {
             return NGX_CONF_ERROR;
diff -r 1d68b502088c -r 7f52714a0b6a src/http/modules/ngx_http_ssl_module.h
--- a/src/http/modules/ngx_http_ssl_module.h    Tue May 21 21:47:50 2013 +0400
+++ b/src/http/modules/ngx_http_ssl_module.h    Tue May 21 17:08:10 2013 -0700
@@ -44,6 +44,7 @@

     ngx_flag_t                      stapling;
     ngx_flag_t                      stapling_verify;
+    ngx_flag_t                      stapling_strict;
     ngx_str_t                       stapling_file;
     ngx_str_t                       stapling_responder;



More information about the nginx-devel mailing list