<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 4, 2017, at 7:28 AM, Maxim Dounin <<a href="mailto:mdounin@mdounin.ru" class="">mdounin@mdounin.ru</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hello!</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Sat, Jul 01, 2017 at 06:21:03PM -0700, Peter Linss wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""># HG changeset patch<br class=""># User Peter Linss <<a href="mailto:peter@linss.com" class="">peter@linss.com</a>><br class=""># Date 1498957095 25200<br class="">#      Sat Jul 01 17:58:15 2017 -0700<br class=""># Node ID 0580b76366e8540973e0ed884d0cec9fc4a7e488<br class=""># Parent  a1c6685e80cba59284fc5e500818ea3b871403eb<br class="">Enable multiple ssl_stapling_file configuration directives<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">There should be "SSL: " prefix and a dot after the sentence.  For<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">more examples, consider looking at "hg log -v”.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>Ok, will do.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class="">When using OCSP stapling files with multiple certificates,<br class="">each certificate must have its own ssl_stapling_file.<br class="">Changes ssl_stapling_file to store an array, and sets individual<br class="">staples per certificate.<br class="">Generates an error if ssl_stapling_file is specified but not<br class="">the same number of times as certificates.<br class=""><br class="">diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h<br class="">--- a/src/event/ngx_event_openssl.h<br class="">+++ b/src/event/ngx_event_openssl.h<br class="">@@ -154,17 +154,17 @@ ngx_int_t ngx_ssl_certificate(ngx_conf_t<br class="">ngx_int_t ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *ciphers,<br class="">    ngx_uint_t prefer_server_ciphers);<br class="">ngx_int_t ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,<br class="">    ngx_str_t *cert, ngx_int_t depth);<br class="">ngx_int_t ngx_ssl_trusted_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,<br class="">    ngx_str_t *cert, ngx_int_t depth);<br class="">ngx_int_t ngx_ssl_crl(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *crl);<br class="">ngx_int_t ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl,<br class="">-    ngx_str_t *file, ngx_str_t *responder, ngx_uint_t verify);<br class="">+    ngx_array_t *files, ngx_str_t *responder, ngx_uint_t verify);<br class="">ngx_int_t ngx_ssl_stapling_resolver(ngx_conf_t *cf, ngx_ssl_t *ssl,<br class="">    ngx_resolver_t *resolver, ngx_msec_t resolver_timeout);<br class="">RSA *ngx_ssl_rsa512_key_callback(ngx_ssl_conn_t *ssl_conn, int is_export,<br class="">    int key_length);<br class="">ngx_array_t *ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file);<br class="">ngx_int_t ngx_ssl_dhparam(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file);<br class="">ngx_int_t ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *name);<br class="">ngx_int_t ngx_ssl_session_cache(ngx_ssl_t *ssl, ngx_str_t *sess_ctx,<br class="">diff --git a/src/event/ngx_event_openssl_stapling.c b/src/event/ngx_event_openssl_stapling.c<br class="">--- a/src/event/ngx_event_openssl_stapling.c<br class="">+++ b/src/event/ngx_event_openssl_stapling.c<br class="">@@ -120,29 +120,47 @@ static ngx_int_t ngx_ssl_ocsp_parse_stat<br class="">static ngx_int_t ngx_ssl_ocsp_process_headers(ngx_ssl_ocsp_ctx_t *ctx);<br class="">static ngx_int_t ngx_ssl_ocsp_parse_header_line(ngx_ssl_ocsp_ctx_t *ctx);<br class="">static ngx_int_t ngx_ssl_ocsp_process_body(ngx_ssl_ocsp_ctx_t *ctx);<br class=""><br class="">static u_char *ngx_ssl_ocsp_log_error(ngx_log_t *log, u_char *buf, size_t len);<br class=""><br class=""><br class="">ngx_int_t<br class="">-ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file,<br class="">+ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *files,<br class="">    ngx_str_t *responder, ngx_uint_t verify)<br class="">{<br class="">-    X509  *cert;<br class="">+    X509        *cert;<br class="">+    ngx_str_t   *file;<br class="">+    ngx_uint_t   i;<br class=""><br class="">-    for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index);<br class="">-         cert;<br class="">-         cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index))<br class="">+    if (files == NULL)<br class="">    {<br class="">-        if (ngx_ssl_stapling_certificate(cf, ssl, cert, file, responder, verify)<br class="">-            != NGX_OK)<br class="">+        for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index);<br class="">+             cert;<br class="">+             cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index))<br class="">        {<br class="">-            return NGX_ERROR;<br class="">+            if (ngx_ssl_stapling_certificate(cf, ssl, cert, NULL, responder, verify)<br class="">+                != NGX_OK)<br class="">+            {<br class="">+                return NGX_ERROR;<br class="">+            }<br class="">+        }<br class="">+    } else {<br class="">+        file = files->elts;<br class="">+<br class="">+        for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index), i = files->nelts - 1;<br class="">+             cert;<br class="">+             cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index), i--)<br class="">+        {<br class="">+            if (ngx_ssl_stapling_certificate(cf, ssl, cert, &file[i], responder, verify)<br class="">+                != NGX_OK)<br class="">+            {<br class="">+                return NGX_ERROR;<br class="">+            }<br class="">        }<br class="">    }<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">This looks overcomplicated and should be rewritten.  Instead,<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">consider preserving a single loop over certificates available, and<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">providing appropriate parameters to ngx_ssl_stapling_certificate()<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">inside the loop.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div>Will do. I wasn’t happy this this myself, but was trying to match existing code style as much as possible in the use of the files array.</div><div><br class=""></div><div>Doing this in a single loop my default approach would be to use a conditional operator to handle the file parameter, but given that files can be NULL it gets a bit ugly without the additional setup. Do you prefer this approach? (untested at this point, but will test before resubmitting, just checking style)</div><div><br class=""></div><div><div><div>    ngx_int_t</div><div>    ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *files,</div><div>        ngx_str_t *responder, ngx_uint_t verify)</div><div>    {</div><div>        X509        *cert;</div><div>        ngx_uint_t   i;</div><div><br class=""></div><div>        for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index, i = 1);</div><div>             cert;</div><div>             cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index), i++)</div><div>        {</div><div>            if (ngx_ssl_stapling_certificate(cf, ssl, cert, files ? &(files->elts[files->nelts - i]) : NULL,</div><div>                                             responder, verify)</div><div>                != NGX_OK)</div><div>            {</div><div>                return NGX_ERROR;</div><div>            }</div><div>        }</div><div><br class=""></div><div>        SSL_CTX_set_tlsext_status_cb(ssl->ctx, ngx_ssl_certificate_status_callback);</div><div><br class=""></div><div>        return NGX_OK;</div><div>    }</div><div class=""><br class=""></div><div class="">Also, matching the staple file to the certificate based on the certificate order feels a bit fragile to me. Have a suggestion for a more robust approach? Or is the certificate order stable enough? (Nothing obvious jumped out at me without changing a bunch of other setup logic and I wanted to keep the impact of this change minimal.)</div></div></div><div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class="">    SSL_CTX_set_tlsext_status_cb(ssl->ctx, ngx_ssl_certificate_status_callback);<br class=""><br class="">    return NGX_OK;<br class="">}<br class=""><br class="">@@ -175,17 +193,17 @@ ngx_ssl_stapling_certificate(ngx_conf_t<span class="Apple-converted-space"> </span><br class=""><br class="">    staple->ssl_ctx = ssl->ctx;<br class="">    staple->timeout = 60000;<br class="">    staple->verify = verify;<br class="">    staple->cert = cert;<br class="">    staple->name = X509_get_ex_data(staple->cert,<br class="">                                    ngx_ssl_certificate_name_index);<br class=""><br class="">-    if (file->len) {<br class="">+    if (file && file->len) {<br class="">        /* use OCSP response from the file */<br class=""><br class="">        if (ngx_ssl_stapling_file(cf, ssl, staple, file) != NGX_OK) {<br class="">            return NGX_ERROR;<br class="">        }<br class=""><br class="">        return NGX_OK;<br class="">    }<br class="">@@ -1866,17 +1884,17 @@ ngx_ssl_ocsp_log_error(ngx_log_t *log, u<br class="">    return p;<br class="">}<br class=""><br class=""><br class="">#else<br class=""><br class=""><br class="">ngx_int_t<br class="">-ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file,<br class="">+ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *files,<br class="">    ngx_str_t *responder, ngx_uint_t verify)<br class="">{<br class="">    ngx_log_error(NGX_LOG_WARN, ssl->log, 0,<br class="">                  "\"ssl_stapling\" ignored, not supported");<br class=""><br class="">    return NGX_OK;<br class="">}<br class=""><br class="">diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c<br class="">--- a/src/http/modules/ngx_http_ssl_module.c<br class="">+++ b/src/http/modules/ngx_http_ssl_module.c<br class="">@@ -210,19 +210,19 @@ static ngx_command_t  ngx_http_ssl_comma<br class="">      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,<br class="">      ngx_conf_set_flag_slot,<br class="">      NGX_HTTP_SRV_CONF_OFFSET,<br class="">      offsetof(ngx_http_ssl_srv_conf_t, stapling),<br class="">      NULL },<br class=""><br class="">    { ngx_string("ssl_stapling_file"),<br class="">      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,<br class="">-      ngx_conf_set_str_slot,<br class="">+      ngx_conf_set_str_array_slot,<br class="">      NGX_HTTP_SRV_CONF_OFFSET,<br class="">-      offsetof(ngx_http_ssl_srv_conf_t, stapling_file),<br class="">+      offsetof(ngx_http_ssl_srv_conf_t, stapling_files),<br class="">      NULL },<br class=""><br class="">    { ngx_string("ssl_stapling_responder"),<br class="">      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,<br class="">      ngx_conf_set_str_slot,<br class="">      NGX_HTTP_SRV_CONF_OFFSET,<br class="">      offsetof(ngx_http_ssl_srv_conf_t, stapling_responder),<br class="">      NULL },<br class="">@@ -532,33 +532,33 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t<span class="Apple-converted-space"> </span><br class="">     *     sscf->protocols = 0;<br class="">     *     sscf->dhparam = { 0, NULL };<br class="">     *     sscf->ecdh_curve = { 0, NULL };<br class="">     *     sscf->client_certificate = { 0, NULL };<br class="">     *     sscf->trusted_certificate = { 0, NULL };<br class="">     *     sscf->crl = { 0, NULL };<br class="">     *     sscf->ciphers = { 0, NULL };<br class="">     *     sscf->shm_zone = NULL;<br class="">-     *     sscf->stapling_file = { 0, NULL };<br class="">     *     sscf->stapling_responder = { 0, NULL };<br class="">     */<br class=""><br class="">    sscf->enable = NGX_CONF_UNSET;<br class="">    sscf->prefer_server_ciphers = NGX_CONF_UNSET;<br class="">    sscf->buffer_size = NGX_CONF_UNSET_SIZE;<br class="">    sscf->verify = NGX_CONF_UNSET_UINT;<br class="">    sscf->verify_depth = NGX_CONF_UNSET_UINT;<br class="">    sscf->certificates = NGX_CONF_UNSET_PTR;<br class="">    sscf->certificate_keys = NGX_CONF_UNSET_PTR;<br class="">    sscf->passwords = NGX_CONF_UNSET_PTR;<br class="">    sscf->builtin_session_cache = NGX_CONF_UNSET;<br class="">    sscf->session_timeout = NGX_CONF_UNSET;<br class="">    sscf->session_tickets = NGX_CONF_UNSET;<br class="">    sscf->session_ticket_keys = NGX_CONF_UNSET_PTR;<br class="">    sscf->stapling = NGX_CONF_UNSET;<br class="">+    sscf->stapling_files = NGX_CONF_UNSET_PTR;<br class="">    sscf->stapling_verify = NGX_CONF_UNSET;<br class=""><br class="">    return sscf;<br class="">}<br class=""><br class=""><br class="">static char *<br class="">ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child)<br class="">@@ -611,17 +611,17 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *<br class=""><br class="">    ngx_conf_merge_str_value(conf->ecdh_curve, prev->ecdh_curve,<br class="">                         NGX_DEFAULT_ECDH_CURVE);<br class=""><br class="">    ngx_conf_merge_str_value(conf->ciphers, prev->ciphers, NGX_DEFAULT_CIPHERS);<br class=""><br class="">    ngx_conf_merge_value(conf->stapling, prev->stapling, 0);<br class="">    ngx_conf_merge_value(conf->stapling_verify, prev->stapling_verify, 0);<br class="">-    ngx_conf_merge_str_value(conf->stapling_file, prev->stapling_file, "");<br class="">+    ngx_conf_merge_ptr_value(conf->stapling_files, prev->stapling_files, NULL);<br class="">    ngx_conf_merge_str_value(conf->stapling_responder,<br class="">                         prev->stapling_responder, "");<br class=""><br class="">    conf->ssl.log = cf->log;<br class=""><br class="">    if (conf->enable) {<br class=""><br class="">        if (conf->certificates == NULL) {<br class="">@@ -662,16 +662,28 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *<br class="">        {<br class="">            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br class="">                          "no \"ssl_certificate_key\" is defined "<br class="">                          "for certificate \"%V\"",<br class="">                          ((ngx_str_t *) conf->certificates->elts)<br class="">                          + conf->certificates->nelts - 1);<br class="">            return NGX_CONF_ERROR;<br class="">        }<br class="">+<br class="">+        if ((conf->stapling_files) &&<br class="">+            (conf->stapling_files->nelts != conf->certificates->nelts))<br class="">+        {<br class="">+            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br class="">+                          "no \"ssl_stapling_file\" is defined "<br class="">+                          "for certificate \"%V\"",<br class="">+                          ((ngx_str_t *) conf->certificates->elts)<br class="">+                          + conf->certificates->nelts - 1);<br class="">+            return NGX_CONF_ERROR;<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Certainly an error here is too restrictive, I would rather<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">recommend preserving the current behaviour (or may be emitting a<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">warning instead).</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>The current behavior is to serve the wrong OCSP staple for one (or more) of the certificates. I don’t think this is desirable as it can cause clients that validate OCSP staples to reject the TLS connection (especially if the certificate has Must-Staple). That seemed like a significant issue to me, but I can make it a warning if you prefer. As a server admin, I personally prefer the error if my ssl config can lead to client side issues, like not being able to reach my site, and it may not be something I see in testing my server if I don’t test against all clients (this stuff is hard enough to get right in the first place).</div><div><br class=""></div><div>(FWIW, trying to set multiple stapling files currently gives an error due to a duplicate ssl_stapling_file directive.)</div><div><br class=""></div><div>It was also unclear to me if the check needed to be repeated in the if (conf->enable) block as well as the else block, guidance appreciated as I don’t really get the point of that logic.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+        }<br class="">+<br class="">    }<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Style, there should be no empty line here.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div>Will fix all the above and resubmit shortly. Thanks for the review!</div><div><br class=""></div><div>(also, I’m currently running the original patch on a production server with dual RSA/ECDSA certificates and a bot keeping the stapling files current at <a href="https://elemental.software/" class="">https://elemental.software/</a> if you want to test against a running instance)</div><div><br class=""></div><div>Peter</div><div><br class=""></div><div> <br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class="">    if (ngx_ssl_create(&conf->ssl, conf->protocols, conf) != NGX_OK) {<br class="">        return NGX_CONF_ERROR;<br class="">    }<br class=""><br class="">#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME<br class=""><br class="">@@ -786,17 +798,17 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *<br class="">    if (ngx_ssl_session_ticket_keys(cf, &conf->ssl, conf->session_ticket_keys)<br class="">        != NGX_OK)<br class="">    {<br class="">        return NGX_CONF_ERROR;<br class="">    }<br class=""><br class="">    if (conf->stapling) {<br class=""><br class="">-        if (ngx_ssl_stapling(cf, &conf->ssl, &conf->stapling_file,<br class="">+        if (ngx_ssl_stapling(cf, &conf->ssl, conf->stapling_files,<br class="">                             &conf->stapling_responder, conf->stapling_verify)<br class="">            != NGX_OK)<br class="">        {<br class="">            return NGX_CONF_ERROR;<br class="">        }<br class=""><br class="">    }<br class=""><br class="">diff --git a/src/http/modules/ngx_http_ssl_module.h b/src/http/modules/ngx_http_ssl_module.h<br class="">--- a/src/http/modules/ngx_http_ssl_module.h<br class="">+++ b/src/http/modules/ngx_http_ssl_module.h<br class="">@@ -47,17 +47,17 @@ typedef struct {<br class=""><br class="">    ngx_shm_zone_t                 *shm_zone;<br class=""><br class="">    ngx_flag_t                      session_tickets;<br class="">    ngx_array_t                    *session_ticket_keys;<br class=""><br class="">    ngx_flag_t                      stapling;<br class="">    ngx_flag_t                      stapling_verify;<br class="">-    ngx_str_t                       stapling_file;<br class="">+    ngx_array_t                    *stapling_files;<br class="">    ngx_str_t                       stapling_responder;<br class=""><br class="">    u_char                         *file;<br class="">    ngx_uint_t                      line;<br class="">} ngx_http_ssl_srv_conf_t;<br class=""><br class=""><br class="">extern ngx_module_t  ngx_http_ssl_module;<br class="">_______________________________________________<br class="">nginx-devel mailing list<br class=""><a href="mailto:nginx-devel@nginx.org" class="">nginx-devel@nginx.org</a><br class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Maxim Dounin</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://nginx.org/" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">http://nginx.org/</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">nginx-devel mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:nginx-devel@nginx.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">nginx-devel@nginx.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a></div></blockquote></div><br class=""></body></html>