Opened 5 years ago

Closed 5 years ago

#769 closed defect (fixed)

nginx 1.9.2 breaks ssl_stapling_file

Reported by: Faidon Liambotis Owned by: Maxim Dounin
Priority: major Milestone:
Component: nginx-core Version: 1.9.x
Keywords: Cc:
uname -a:
nginx -V: nginx version: nginx/1.9.2
built with OpenSSL 1.0.1k 8 Jan 2015
TLS SNI support enabled

Description

changeset 6181:6893a1007a7c ("OCSP stapling: avoid sending expired responses (ticket #425)"), included in 1.9.2, broke stapling with ssl_stapling_file (= no OCSP response is being stapled, despite that configuration directive being present).

The problems seems to lie here:
1.24 - if (staple->staple.len) {
1.25 + if (staple->staple.len
1.26 + && staple->valid >= ngx_time())
1.27 + {
That's from ngx_ssl_certificate_status_callback().

However, staple->valid is only set by ngx_ssl_stapling_ocsp_handler(), which is clearly only called when online stapling is being used.

Change History (5)

comment:1 by Faidon Liambotis, 5 years ago

Fix, from Brandon Black, a colleague at Wikimedia:
https://git.wikimedia.org/blob/operations%2Fsoftware%2Fnginx.git/b15a432da142c40a668f77ab85bdd37d6b53393f/debian%2Fpatches%2F2001-Stapling-File-Bugfix.patch

diff --git a/src/event/ngx_event_openssl_stapling.c b/src/event/ngx_event_openssl_stapling.c
index 03ff540..f487534 100644
--- a/src/event/ngx_event_openssl_stapling.c
+++ b/src/event/ngx_event_openssl_stapling.c
@@ -466,7 +466,7 @@ ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn, void *data)
     rc = SSL_TLSEXT_ERR_NOACK;

     if (staple->staple.len
-        && staple->valid >= ngx_time())
+        && (staple->host.len == 0 || staple->valid >= ngx_time()))
     {
         /* we have to copy ocsp response as OpenSSL will free it by itself */

comment:2 by maxim, 5 years ago

Owner: set to Maxim Dounin
Status: newassigned

comment:3 by Maxim Dounin, 5 years ago

Thanks, here is slightly different patch to fix this:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1436197779 -10800
#      Mon Jul 06 18:49:39 2015 +0300
# Node ID 12aa8e9ded17e1f037948864c914acb290670377
# Parent  33e62c32b9d602700a3cd48955067668fe9a86ab
OCSP stapling: fixed ssl_stapling_file (ticket #769).

Broken by 6893a1007a7c (1.9.2) during introduction of strict OCSP response
validity checks.  As stapling file is expected to be returned unconditionally,
fix is to set its validity to the maximum supported time.

Reported by Faidon Liambotis.

diff --git a/src/event/ngx_event_openssl_stapling.c b/src/event/ngx_event_openssl_stapling.c
--- a/src/event/ngx_event_openssl_stapling.c
+++ b/src/event/ngx_event_openssl_stapling.c
@@ -245,6 +245,7 @@ ngx_ssl_stapling_file(ngx_conf_t *cf, ng
 
     staple->staple.data = buf;
     staple->staple.len = len;
+    staple->valid = NGX_MAX_TIME_T_VALUE;
 
     return NGX_OK;
 

comment:4 by Maxim Dounin <mdounin@…>, 5 years ago

In dcae651b2a0cbd3de2f1fd5cf5b8c72627db94fd/nginx:

OCSP stapling: fixed ssl_stapling_file (ticket #769).

Broken by 6893a1007a7c (1.9.2) during introduction of strict OCSP response
validity checks. As stapling file is expected to be returned unconditionally,
fix is to set its validity to the maximum supported time.

Reported by Faidon Liambotis.

comment:5 by Maxim Dounin, 5 years ago

Resolution: fixed
Status: assignedclosed

Fix committed, thanks.

Note: See TracTickets for help on using tickets.