From 4c8abb84e399f964d6b70f24c6324f261fd4565a Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Thu, 23 Apr 2020 15:10:24 +0300 Subject: gRPC: RST_STREAM(NO_ERROR) handling (ticket #1792). As per https://tools.ietf.org/html/rfc7540#section-8.1, : A server can send a complete response prior to the client : sending an entire request if the response does not depend on : any portion of the request that has not been sent and : received. When this is true, a server MAY request that the : client abort transmission of a request without error by : sending a RST_STREAM with an error code of NO_ERROR after : sending a complete response (i.e., a frame with the : END_STREAM flag). Clients MUST NOT discard responses as a : result of receiving such a RST_STREAM, though clients can : always discard responses at their discretion for other : reasons. Previously, RST_STREAM(NO_ERROR) received from upstream after a frame with the END_STREAM flag was incorrectly treated as an error. Now, a single RST_STREAM(NO_ERROR) is properly handled. This fixes problems observed with modern grpc-c [1], as well as with the Go gRPC module. [1] https://github.com/grpc/grpc/pull/1661 --- src/http/modules/ngx_http_grpc_module.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) (limited to 'src/http') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index d4af66dbf..9e62d8e2a 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -120,6 +120,7 @@ typedef struct { unsigned end_stream:1; unsigned done:1; unsigned status:1; + unsigned rst:1; ngx_http_request_t *request; @@ -1205,6 +1206,7 @@ ngx_http_grpc_reinit_request(ngx_http_request_t *r) ctx->end_stream = 0; ctx->done = 0; ctx->status = 0; + ctx->rst = 0; ctx->connection = NULL; return NGX_OK; @@ -2088,7 +2090,9 @@ ngx_http_grpc_filter(void *data, ssize_t bytes) return NGX_ERROR; } - if (ctx->stream_id && ctx->done) { + if (ctx->stream_id && ctx->done + && ctx->type != NGX_HTTP_V2_RST_STREAM_FRAME) + { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "upstream sent frame for closed stream %ui", ctx->stream_id); @@ -2131,11 +2135,21 @@ ngx_http_grpc_filter(void *data, ssize_t bytes) return NGX_ERROR; } - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream rejected request with error %ui", - ctx->error); + if (ctx->error || !ctx->done) { + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, + "upstream rejected request with error %ui", + ctx->error); + return NGX_ERROR; + } - return NGX_ERROR; + if (ctx->rst) { + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, + "upstream sent frame for closed stream %ui", + ctx->stream_id); + return NGX_ERROR; + } + + ctx->rst = 1; } if (ctx->type == NGX_HTTP_V2_GOAWAY_FRAME) { -- cgit From ee9c61b89bcb891575c809e388affd2bb04e9e60 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Thu, 23 Apr 2020 15:10:26 +0300 Subject: gRPC: WINDOW_UPDATE after END_STREAM handling (ticket #1797). As per https://tools.ietf.org/html/rfc7540#section-6.9, WINDOW_UPDATE received after a frame with the END_STREAM flag should be handled and not treated as an error. --- src/http/modules/ngx_http_grpc_module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/http') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 9e62d8e2a..992211e73 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -2091,7 +2091,8 @@ ngx_http_grpc_filter(void *data, ssize_t bytes) } if (ctx->stream_id && ctx->done - && ctx->type != NGX_HTTP_V2_RST_STREAM_FRAME) + && ctx->type != NGX_HTTP_V2_RST_STREAM_FRAME + && ctx->type != NGX_HTTP_V2_WINDOW_UPDATE_FRAME) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "upstream sent frame for closed stream %ui", -- cgit From 41ecd45a5bb78b2214c4515768a51aff0c57eead Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Fri, 8 May 2020 19:19:16 +0300 Subject: Variables: fixed buffer over-read when evaluating "$arg_". --- src/http/ngx_http_variables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/http') diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c index e067cf0c2..c2113c843 100644 --- a/src/http/ngx_http_variables.c +++ b/src/http/ngx_http_variables.c @@ -1075,7 +1075,7 @@ ngx_http_variable_argument(ngx_http_request_t *r, ngx_http_variable_value_t *v, len = name->len - (sizeof("arg_") - 1); arg = name->data + sizeof("arg_") - 1; - if (ngx_http_arg(r, arg, len, &value) != NGX_OK) { + if (len == 0 || ngx_http_arg(r, arg, len, &value) != NGX_OK) { v->not_found = 1; return NGX_OK; } -- cgit From b47c1f35e2e421bc0e5be67afb897276324c57a4 Mon Sep 17 00:00:00 2001 From: Jinhua Tan <312841925@qq.com> Date: Wed, 13 May 2020 22:02:47 +0800 Subject: Upstream: jump out of loop after matching the status code. --- src/http/ngx_http_upstream.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/http') diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c index 89e1319f9..be96be47a 100644 --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -2502,6 +2502,8 @@ ngx_http_upstream_test_next(ngx_http_request_t *r, ngx_http_upstream_t *u) } #endif + + break; } #if (NGX_HTTP_CACHE) -- cgit From 60438ae395d83b0f8b21bf667a1e260d60c3f46a Mon Sep 17 00:00:00 2001 From: Roman Arutyunyan Date: Fri, 22 May 2020 17:30:12 +0300 Subject: SSL: client certificate validation with OCSP (ticket #1534). OCSP validation for client certificates is enabled by the "ssl_ocsp" directive. OCSP responder can be optionally specified by "ssl_ocsp_responder". When session is reused, peer chain is not available for validation. If the verified chain contains certificates from the peer chain not available at the server, validation will fail. --- src/http/modules/ngx_http_ssl_module.c | 64 +++++++++++++++++++++++++++++++--- src/http/modules/ngx_http_ssl_module.h | 3 ++ src/http/ngx_http_request.c | 12 +++++++ 3 files changed, 74 insertions(+), 5 deletions(-) (limited to 'src/http') diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c index 495e628d3..1c9accdd3 100644 --- a/src/http/modules/ngx_http_ssl_module.c +++ b/src/http/modules/ngx_http_ssl_module.c @@ -74,6 +74,14 @@ static ngx_conf_enum_t ngx_http_ssl_verify[] = { }; +static ngx_conf_enum_t ngx_http_ssl_ocsp[] = { + { ngx_string("off"), 0 }, + { ngx_string("on"), 1 }, + { ngx_string("leaf"), 2 }, + { ngx_null_string, 0 } +}; + + static ngx_conf_deprecated_t ngx_http_ssl_deprecated = { ngx_conf_deprecated, "ssl", "listen ... ssl" }; @@ -214,6 +222,20 @@ static ngx_command_t ngx_http_ssl_commands[] = { offsetof(ngx_http_ssl_srv_conf_t, crl), NULL }, + { ngx_string("ssl_ocsp"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_conf_set_enum_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_ssl_srv_conf_t, ocsp), + &ngx_http_ssl_ocsp }, + + { ngx_string("ssl_ocsp_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, ocsp_responder), + NULL }, + { ngx_string("ssl_stapling"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, ngx_conf_set_flag_slot, @@ -561,6 +583,7 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t *cf) * sscf->crl = { 0, NULL }; * sscf->ciphers = { 0, NULL }; * sscf->shm_zone = NULL; + * sscf->ocsp_responder = { 0, NULL }; * sscf->stapling_file = { 0, NULL }; * sscf->stapling_responder = { 0, NULL }; */ @@ -578,6 +601,7 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t *cf) sscf->session_timeout = NGX_CONF_UNSET; sscf->session_tickets = NGX_CONF_UNSET; sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; + sscf->ocsp = NGX_CONF_UNSET_UINT; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; @@ -641,6 +665,9 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_str_value(conf->ciphers, prev->ciphers, NGX_DEFAULT_CIPHERS); + ngx_conf_merge_uint_value(conf->ocsp, prev->ocsp, 0); + ngx_conf_merge_str_value(conf->ocsp_responder, prev->ocsp_responder, ""); + 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, ""); @@ -802,6 +829,22 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) return NGX_CONF_ERROR; } + if (conf->ocsp) { + + if (conf->verify == 3) { + ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + "\"ssl_ocsp\" is incompatible with " + "\"ssl_verify_client optional_no_ca\""); + return NGX_CONF_ERROR; + } + + if (ngx_ssl_ocsp(cf, &conf->ssl, &conf->ocsp_responder, conf->ocsp) + != NGX_OK) + { + return NGX_CONF_ERROR; + } + } + if (ngx_ssl_dhparam(cf, &conf->ssl, &conf->dhparam) != NGX_OK) { return NGX_CONF_ERROR; } @@ -1118,17 +1161,28 @@ ngx_http_ssl_init(ngx_conf_t *cf) sscf = cscfp[s]->ctx->srv_conf[ngx_http_ssl_module.ctx_index]; - if (sscf->ssl.ctx == NULL || !sscf->stapling) { + if (sscf->ssl.ctx == NULL) { continue; } clcf = cscfp[s]->ctx->loc_conf[ngx_http_core_module.ctx_index]; - if (ngx_ssl_stapling_resolver(cf, &sscf->ssl, clcf->resolver, + if (sscf->stapling) { + if (ngx_ssl_stapling_resolver(cf, &sscf->ssl, clcf->resolver, + clcf->resolver_timeout) + != NGX_OK) + { + return NGX_ERROR; + } + } + + if (sscf->ocsp) { + if (ngx_ssl_ocsp_resolver(cf, &sscf->ssl, clcf->resolver, clcf->resolver_timeout) - != NGX_OK) - { - return NGX_ERROR; + != NGX_OK) + { + return NGX_ERROR; + } } } diff --git a/src/http/modules/ngx_http_ssl_module.h b/src/http/modules/ngx_http_ssl_module.h index 26fdccfe4..92d459f60 100644 --- a/src/http/modules/ngx_http_ssl_module.h +++ b/src/http/modules/ngx_http_ssl_module.h @@ -54,6 +54,9 @@ typedef struct { ngx_flag_t session_tickets; ngx_array_t *session_ticket_keys; + ngx_uint_t ocsp; + ngx_str_t ocsp_responder; + ngx_flag_t stapling; ngx_flag_t stapling_verify; ngx_str_t stapling_file; diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index eb53996b1..6feb6cc31 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -1993,6 +1993,7 @@ ngx_http_process_request(ngx_http_request_t *r) if (r->http_connection->ssl) { long rc; X509 *cert; + const char *s; ngx_http_ssl_srv_conf_t *sscf; if (c->ssl == NULL) { @@ -2037,6 +2038,17 @@ ngx_http_process_request(ngx_http_request_t *r) X509_free(cert); } + + if (ngx_ssl_ocsp_get_status(c, &s) != NGX_OK) { + ngx_log_error(NGX_LOG_INFO, c->log, 0, + "client SSL certificate verify error: %s", s); + + ngx_ssl_remove_cached_session(c->ssl->session_ctx, + (SSL_get0_session(c->ssl->connection))); + + ngx_http_finalize_request(r, NGX_HTTPS_CERT_ERROR); + return; + } } } -- cgit From 5727f9a1e0cca082eb1f3e599e0453a7a9cfe319 Mon Sep 17 00:00:00 2001 From: Roman Arutyunyan Date: Fri, 22 May 2020 17:25:27 +0300 Subject: OCSP: certificate status cache. When enabled, certificate status is stored in cache and is used to validate the certificate in future requests. New directive ssl_ocsp_cache is added to configure the cache. --- src/http/modules/ngx_http_ssl_module.c | 94 +++++++++++++++++++++++++++++++++- src/http/modules/ngx_http_ssl_module.h | 1 + 2 files changed, 94 insertions(+), 1 deletion(-) (limited to 'src/http') diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c index 1c9accdd3..d7072a626 100644 --- a/src/http/modules/ngx_http_ssl_module.c +++ b/src/http/modules/ngx_http_ssl_module.c @@ -50,6 +50,8 @@ static char *ngx_http_ssl_password_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); static char *ngx_http_ssl_session_cache(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_http_ssl_ocsp_cache(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf); static ngx_int_t ngx_http_ssl_init(ngx_conf_t *cf); @@ -236,6 +238,13 @@ static ngx_command_t ngx_http_ssl_commands[] = { offsetof(ngx_http_ssl_srv_conf_t, ocsp_responder), NULL }, + { ngx_string("ssl_ocsp_cache"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1, + ngx_http_ssl_ocsp_cache, + NGX_HTTP_SRV_CONF_OFFSET, + 0, + NULL }, + { ngx_string("ssl_stapling"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, ngx_conf_set_flag_slot, @@ -602,6 +611,7 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t *cf) sscf->session_tickets = NGX_CONF_UNSET; sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; sscf->ocsp = NGX_CONF_UNSET_UINT; + sscf->ocsp_cache_zone = NGX_CONF_UNSET_PTR; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; @@ -667,6 +677,8 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_uint_value(conf->ocsp, prev->ocsp, 0); ngx_conf_merge_str_value(conf->ocsp_responder, prev->ocsp_responder, ""); + ngx_conf_merge_ptr_value(conf->ocsp_cache_zone, + prev->ocsp_cache_zone, NULL); ngx_conf_merge_value(conf->stapling, prev->stapling, 0); ngx_conf_merge_value(conf->stapling_verify, prev->stapling_verify, 0); @@ -838,7 +850,8 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) return NGX_CONF_ERROR; } - if (ngx_ssl_ocsp(cf, &conf->ssl, &conf->ocsp_responder, conf->ocsp) + if (ngx_ssl_ocsp(cf, &conf->ssl, &conf->ocsp_responder, conf->ocsp, + conf->ocsp_cache_zone) != NGX_OK) { return NGX_CONF_ERROR; @@ -1143,6 +1156,85 @@ invalid: } +static char * +ngx_http_ssl_ocsp_cache(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + ngx_http_ssl_srv_conf_t *sscf = conf; + + size_t len; + ngx_int_t n; + ngx_str_t *value, name, size; + ngx_uint_t j; + + if (sscf->ocsp_cache_zone != NGX_CONF_UNSET_PTR) { + return "is duplicate"; + } + + value = cf->args->elts; + + if (ngx_strcmp(value[1].data, "off") == 0) { + sscf->ocsp_cache_zone = NULL; + return NGX_CONF_OK; + } + + if (value[1].len <= sizeof("shared:") - 1 + || ngx_strncmp(value[1].data, "shared:", sizeof("shared:") - 1) != 0) + { + goto invalid; + } + + len = 0; + + for (j = sizeof("shared:") - 1; j < value[1].len; j++) { + if (value[1].data[j] == ':') { + break; + } + + len++; + } + + if (len == 0) { + goto invalid; + } + + name.len = len; + name.data = value[1].data + sizeof("shared:") - 1; + + size.len = value[1].len - j - 1; + size.data = name.data + len + 1; + + n = ngx_parse_size(&size); + + if (n == NGX_ERROR) { + goto invalid; + } + + if (n < (ngx_int_t) (8 * ngx_pagesize)) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "OCSP cache \"%V\" is too small", &value[1]); + + return NGX_CONF_ERROR; + } + + sscf->ocsp_cache_zone = ngx_shared_memory_add(cf, &name, n, + &ngx_http_ssl_module_ctx); + if (sscf->ocsp_cache_zone == NULL) { + return NGX_CONF_ERROR; + } + + sscf->ocsp_cache_zone->init = ngx_ssl_ocsp_cache_init; + + return NGX_CONF_OK; + +invalid: + + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "invalid OCSP cache \"%V\"", &value[1]); + + return NGX_CONF_ERROR; +} + + static ngx_int_t ngx_http_ssl_init(ngx_conf_t *cf) { diff --git a/src/http/modules/ngx_http_ssl_module.h b/src/http/modules/ngx_http_ssl_module.h index 92d459f60..98aa1be40 100644 --- a/src/http/modules/ngx_http_ssl_module.h +++ b/src/http/modules/ngx_http_ssl_module.h @@ -56,6 +56,7 @@ typedef struct { ngx_uint_t ocsp; ngx_str_t ocsp_responder; + ngx_shm_zone_t *ocsp_cache_zone; ngx_flag_t stapling; ngx_flag_t stapling_verify; -- cgit From 4056ce4f4ee4d80d41c56af8c1b4fd41b76144c8 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 25 May 2020 18:33:42 +0300 Subject: HTTP/2: invalid connection preface logging (ticket #1981). Previously, invalid connection preface errors were only logged at debug level, providing no visible feedback, in particular, when a plain text HTTP/2 listening socket is erroneously used for HTTP/1.x connections. Now these are explicitly logged at the info level, much like other client-related errors. --- src/http/v2/ngx_http_v2.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'src/http') diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 8b0fc53b0..08d66c97b 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -731,9 +731,8 @@ ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c, u_char *pos, } if (ngx_memcmp(pos, preface, sizeof(preface) - 1) != 0) { - ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "invalid http2 connection preface \"%*s\"", - sizeof(preface) - 1, pos); + ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "invalid connection preface"); return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR); } @@ -754,9 +753,8 @@ ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c, u_char *pos, } if (ngx_memcmp(pos, preface, sizeof(preface) - 1) != 0) { - ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "invalid http2 connection preface \"%*s\"", - sizeof(preface) - 1, pos); + ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "invalid connection preface"); return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR); } -- cgit