[PATCH] Enable multiple ssl_stapling_file configuration directives
Maxim Dounin
mdounin at mdounin.ru
Tue Jul 4 14:28:25 UTC 2017
Hello!
On Sat, Jul 01, 2017 at 06:21:03PM -0700, Peter Linss wrote:
> # HG changeset patch
> # User Peter Linss <peter at linss.com>
> # Date 1498957095 25200
> # Sat Jul 01 17:58:15 2017 -0700
> # Node ID 0580b76366e8540973e0ed884d0cec9fc4a7e488
> # Parent a1c6685e80cba59284fc5e500818ea3b871403eb
> Enable multiple ssl_stapling_file configuration directives
There should be "SSL: " prefix and a dot after the sentence. For
more examples, consider looking at "hg log -v".
>
> When using OCSP stapling files with multiple certificates,
> each certificate must have its own ssl_stapling_file.
> Changes ssl_stapling_file to store an array, and sets individual
> staples per certificate.
> Generates an error if ssl_stapling_file is specified but not
> the same number of times as certificates.
>
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -154,17 +154,17 @@ ngx_int_t ngx_ssl_certificate(ngx_conf_t
> ngx_int_t ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *ciphers,
> ngx_uint_t prefer_server_ciphers);
> ngx_int_t ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,
> ngx_str_t *cert, ngx_int_t depth);
> ngx_int_t ngx_ssl_trusted_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,
> 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_array_t *files, ngx_str_t *responder, ngx_uint_t verify);
> 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(ngx_ssl_conn_t *ssl_conn, int is_export,
> int key_length);
> ngx_array_t *ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file);
> ngx_int_t ngx_ssl_dhparam(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file);
> ngx_int_t ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *name);
> ngx_int_t ngx_ssl_session_cache(ngx_ssl_t *ssl, ngx_str_t *sess_ctx,
> 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
> @@ -120,29 +120,47 @@ static ngx_int_t ngx_ssl_ocsp_parse_stat
> static ngx_int_t ngx_ssl_ocsp_process_headers(ngx_ssl_ocsp_ctx_t *ctx);
> static ngx_int_t ngx_ssl_ocsp_parse_header_line(ngx_ssl_ocsp_ctx_t *ctx);
> static ngx_int_t ngx_ssl_ocsp_process_body(ngx_ssl_ocsp_ctx_t *ctx);
>
> static u_char *ngx_ssl_ocsp_log_error(ngx_log_t *log, u_char *buf, size_t len);
>
>
> ngx_int_t
> -ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file,
> +ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *files,
> ngx_str_t *responder, ngx_uint_t verify)
> {
> - X509 *cert;
> + X509 *cert;
> + ngx_str_t *file;
> + ngx_uint_t i;
>
> - for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index);
> - cert;
> - cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index))
> + if (files == NULL)
> {
> - if (ngx_ssl_stapling_certificate(cf, ssl, cert, file, responder, verify)
> - != NGX_OK)
> + for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index);
> + cert;
> + cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index))
> {
> - return NGX_ERROR;
> + if (ngx_ssl_stapling_certificate(cf, ssl, cert, NULL, responder, verify)
> + != NGX_OK)
> + {
> + return NGX_ERROR;
> + }
> + }
> + } else {
> + file = files->elts;
> +
> + for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index), i = files->nelts - 1;
> + cert;
> + cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index), i--)
> + {
> + if (ngx_ssl_stapling_certificate(cf, ssl, cert, &file[i], responder, verify)
> + != NGX_OK)
> + {
> + return NGX_ERROR;
> + }
> }
> }
This looks overcomplicated and should be rewritten. Instead,
consider preserving a single loop over certificates available, and
providing appropriate parameters to ngx_ssl_stapling_certificate()
inside the loop.
>
> SSL_CTX_set_tlsext_status_cb(ssl->ctx, ngx_ssl_certificate_status_callback);
>
> return NGX_OK;
> }
>
> @@ -175,17 +193,17 @@ ngx_ssl_stapling_certificate(ngx_conf_t
>
> staple->ssl_ctx = ssl->ctx;
> staple->timeout = 60000;
> staple->verify = verify;
> staple->cert = cert;
> staple->name = X509_get_ex_data(staple->cert,
> ngx_ssl_certificate_name_index);
>
> - if (file->len) {
> + if (file && file->len) {
> /* use OCSP response from the file */
>
> if (ngx_ssl_stapling_file(cf, ssl, staple, file) != NGX_OK) {
> return NGX_ERROR;
> }
>
> return NGX_OK;
> }
> @@ -1866,17 +1884,17 @@ ngx_ssl_ocsp_log_error(ngx_log_t *log, u
> return p;
> }
>
>
> #else
>
>
> ngx_int_t
> -ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file,
> +ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *files,
> ngx_str_t *responder, ngx_uint_t verify)
> {
> ngx_log_error(NGX_LOG_WARN, ssl->log, 0,
> "\"ssl_stapling\" ignored, not supported");
>
> return NGX_OK;
> }
>
> diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
> --- a/src/http/modules/ngx_http_ssl_module.c
> +++ b/src/http/modules/ngx_http_ssl_module.c
> @@ -210,19 +210,19 @@ static ngx_command_t ngx_http_ssl_comma
> 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),
> NULL },
>
> { ngx_string("ssl_stapling_file"),
> NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
> - ngx_conf_set_str_slot,
> + ngx_conf_set_str_array_slot,
> NGX_HTTP_SRV_CONF_OFFSET,
> - offsetof(ngx_http_ssl_srv_conf_t, stapling_file),
> + offsetof(ngx_http_ssl_srv_conf_t, stapling_files),
> NULL },
>
> { ngx_string("ssl_stapling_responder"),
> NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
> ngx_conf_set_str_slot,
> NGX_HTTP_SRV_CONF_OFFSET,
> offsetof(ngx_http_ssl_srv_conf_t, stapling_responder),
> NULL },
> @@ -532,33 +532,33 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t
> * sscf->protocols = 0;
> * sscf->dhparam = { 0, NULL };
> * sscf->ecdh_curve = { 0, NULL };
> * sscf->client_certificate = { 0, NULL };
> * sscf->trusted_certificate = { 0, NULL };
> * sscf->crl = { 0, NULL };
> * sscf->ciphers = { 0, NULL };
> * sscf->shm_zone = NULL;
> - * sscf->stapling_file = { 0, NULL };
> * sscf->stapling_responder = { 0, NULL };
> */
>
> sscf->enable = NGX_CONF_UNSET;
> sscf->prefer_server_ciphers = NGX_CONF_UNSET;
> sscf->buffer_size = NGX_CONF_UNSET_SIZE;
> sscf->verify = NGX_CONF_UNSET_UINT;
> sscf->verify_depth = NGX_CONF_UNSET_UINT;
> sscf->certificates = NGX_CONF_UNSET_PTR;
> sscf->certificate_keys = NGX_CONF_UNSET_PTR;
> sscf->passwords = NGX_CONF_UNSET_PTR;
> sscf->builtin_session_cache = NGX_CONF_UNSET;
> sscf->session_timeout = NGX_CONF_UNSET;
> sscf->session_tickets = NGX_CONF_UNSET;
> sscf->session_ticket_keys = NGX_CONF_UNSET_PTR;
> sscf->stapling = NGX_CONF_UNSET;
> + sscf->stapling_files = NGX_CONF_UNSET_PTR;
> sscf->stapling_verify = NGX_CONF_UNSET;
>
> return sscf;
> }
>
>
> static char *
> ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child)
> @@ -611,17 +611,17 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
>
> ngx_conf_merge_str_value(conf->ecdh_curve, prev->ecdh_curve,
> NGX_DEFAULT_ECDH_CURVE);
>
> ngx_conf_merge_str_value(conf->ciphers, prev->ciphers, NGX_DEFAULT_CIPHERS);
>
> ngx_conf_merge_value(conf->stapling, prev->stapling, 0);
> ngx_conf_merge_value(conf->stapling_verify, prev->stapling_verify, 0);
> - ngx_conf_merge_str_value(conf->stapling_file, prev->stapling_file, "");
> + ngx_conf_merge_ptr_value(conf->stapling_files, prev->stapling_files, NULL);
> ngx_conf_merge_str_value(conf->stapling_responder,
> prev->stapling_responder, "");
>
> conf->ssl.log = cf->log;
>
> if (conf->enable) {
>
> if (conf->certificates == NULL) {
> @@ -662,16 +662,28 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
> {
> ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
> "no \"ssl_certificate_key\" is defined "
> "for certificate \"%V\"",
> ((ngx_str_t *) conf->certificates->elts)
> + conf->certificates->nelts - 1);
> return NGX_CONF_ERROR;
> }
> +
> + if ((conf->stapling_files) &&
> + (conf->stapling_files->nelts != conf->certificates->nelts))
> + {
> + ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
> + "no \"ssl_stapling_file\" is defined "
> + "for certificate \"%V\"",
> + ((ngx_str_t *) conf->certificates->elts)
> + + conf->certificates->nelts - 1);
> + return NGX_CONF_ERROR;
Certainly an error here is too restrictive, I would rather
recommend preserving the current behaviour (or may be emitting a
warning instead).
> + }
> +
> }
Style, there should be no empty line here.
>
> if (ngx_ssl_create(&conf->ssl, conf->protocols, conf) != NGX_OK) {
> return NGX_CONF_ERROR;
> }
>
> #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
>
> @@ -786,17 +798,17 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
> if (ngx_ssl_session_ticket_keys(cf, &conf->ssl, conf->session_ticket_keys)
> != NGX_OK)
> {
> return NGX_CONF_ERROR;
> }
>
> if (conf->stapling) {
>
> - if (ngx_ssl_stapling(cf, &conf->ssl, &conf->stapling_file,
> + if (ngx_ssl_stapling(cf, &conf->ssl, conf->stapling_files,
> &conf->stapling_responder, conf->stapling_verify)
> != NGX_OK)
> {
> return NGX_CONF_ERROR;
> }
>
> }
>
> diff --git a/src/http/modules/ngx_http_ssl_module.h b/src/http/modules/ngx_http_ssl_module.h
> --- a/src/http/modules/ngx_http_ssl_module.h
> +++ b/src/http/modules/ngx_http_ssl_module.h
> @@ -47,17 +47,17 @@ typedef struct {
>
> ngx_shm_zone_t *shm_zone;
>
> ngx_flag_t session_tickets;
> ngx_array_t *session_ticket_keys;
>
> ngx_flag_t stapling;
> ngx_flag_t stapling_verify;
> - ngx_str_t stapling_file;
> + ngx_array_t *stapling_files;
> ngx_str_t stapling_responder;
>
> u_char *file;
> ngx_uint_t line;
> } ngx_http_ssl_srv_conf_t;
>
>
> extern ngx_module_t ngx_http_ssl_module;
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Maxim Dounin
http://nginx.org/
More information about the nginx-devel
mailing list