Hello! On Tue, Aug 20, 2013 at 03:33:43PM +0300, Aviram Cohen wrote:
> Hello! > > Nginx's reverse proxy doesn't verify the SSL certificate of the remote > server (see http://trac.nginx.org/nginx/ticket/13). > > The following is a suggested patch for v1.4.1 that adds this feature. It is > partially inspired by the patch for v1.1.3 that has been suggested in this > list and in the ticket above, with some improvements (i.e. no need to add > the "verification_failed" field to ngx_ssl_connection_t). > > Note that a directory of CA's should be provided as a configuration > parameter ("CApath"), and that this patch is missing a Certificate > Revocation List file feature. It's probably good idea to line up the implementation with ssl_verify_client. It might be also a good idea to reuse ssl_trusted_certificate file as a source of trusted CA certs, not sure though. In any case naming should be consistent (that is, proxy_ssl_ca_certificate is a bad name). See below for some more comments. > > Feedback would be welcome. > > Best regards, > Aviram > > > diff -Nrpu nginx-1.4.1/src/event/ngx_event_openssl.c > nginx-1.4.1-proxy-ssl-verify/src/event/ngx_event_openssl.c > --- nginx-1.4.1/src/event/ngx_event_openssl.c 2013-05-06 13:26:50.000000000 > +0300 > +++ nginx-1.4.1-proxy-ssl-verify/src/event/ngx_event_openssl.c 2013-08-20 > 14:53:31.465251759 +0300 > @@ -337,6 +337,31 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ > > > ngx_int_t > +ngx_ssl_set_verify_options(ngx_ssl_t *ssl, ngx_str_t *cert, > + ngx_int_t depth) > +{ > + if (cert->len == 0) { > + return NGX_OK; > + } > + > + SSL_CTX_set_verify(ssl->ctx, SSL_VERIFY_PEER, > ngx_http_ssl_verify_callback); Just a side note: your mail client corrupts patches. > + > + SSL_CTX_set_verify_depth(ssl->ctx, depth); > + > + if (SSL_CTX_load_verify_locations(ssl->ctx, NULL, (char *) cert->data) > + == 0) > + { > + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, > + "SSL_CTX_load_verify_locations(\"%s\") failed", > + cert->data); > + return NGX_ERROR; > + } > + > + return NGX_OK; > +} > + > + > +ngx_int_t > ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, > ngx_int_t depth) > { > @@ -710,6 +735,17 @@ ngx_ssl_set_session(ngx_connection_t *c, > return NGX_OK; > } > > + > +ngx_int_t > +ngx_ssl_verify_result(ngx_connection_t *c) > +{ > + if (SSL_get_verify_result(c->ssl->connection) != X509_V_OK) { > + ngx_ssl_error(NGX_LOG_EMERG, c->log, 0, "SSL_get_verify_result > failed"); > + return NGX_ERROR; > + } > + return NGX_OK; > +} > + > > ngx_int_t > ngx_ssl_handshake(ngx_connection_t *c) > diff -Nrpu nginx-1.4.1/src/event/ngx_event_openssl.h > nginx-1.4.1-proxy-ssl-verify/src/event/ngx_event_openssl.h > --- nginx-1.4.1/src/event/ngx_event_openssl.h 2013-05-06 13:26:50.000000000 > +0300 > +++ nginx-1.4.1-proxy-ssl-verify/src/event/ngx_event_openssl.h 2013-08-20 > 14:54:37.933252402 +0300 > @@ -100,6 +100,8 @@ ngx_int_t ngx_ssl_init(ngx_log_t *log); > ngx_int_t ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_t protocols, void *data); > ngx_int_t ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, > ngx_str_t *cert, ngx_str_t *key); > +ngx_int_t ngx_ssl_set_verify_options(ngx_ssl_t *ssl, ngx_str_t *cert, > + ngx_int_t depth); > 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, > @@ -155,6 +157,7 @@ ngx_int_t ngx_ssl_get_client_verify(ngx_ > ngx_str_t *s); > > > +ngx_int_t ngx_ssl_verify_result(ngx_connection_t *c); > ngx_int_t ngx_ssl_handshake(ngx_connection_t *c); > ssize_t ngx_ssl_recv(ngx_connection_t *c, u_char *buf, size_t size); > ssize_t ngx_ssl_write(ngx_connection_t *c, u_char *data, size_t size); > diff -Nrpu nginx-1.4.1/src/http/modules/ngx_http_proxy_module.c > nginx-1.4.1-proxy-ssl-verify/src/http/modules/ngx_http_proxy_module.c > --- nginx-1.4.1/src/http/modules/ngx_http_proxy_module.c 2013-05-06 > 13:26:50.000000000 +0300 > +++ nginx-1.4.1-proxy-ssl-verify/src/http/modules/ngx_http_proxy_module.c > 2013-08-20 > 14:56:24.001251235 +0300 > @@ -511,6 +511,26 @@ static ngx_command_t ngx_http_proxy_com > offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_session_reuse), > NULL }, > > + { ngx_string("proxy_ssl_verify_peer"), > + > NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, > + ngx_conf_set_flag_slot, > + NGX_HTTP_LOC_CONF_OFFSET, > + offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_verify_peer), > + NULL }, Just "proxy_ssl_verify" is probably enough. > + > + { ngx_string("proxy_ssl_verify_depth"), > + > NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, > + ngx_conf_set_num_slot, > + NGX_HTTP_LOC_CONF_OFFSET, > + offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_verify_depth), > + NULL }, > + > + { ngx_string("proxy_ssl_ca_certificate"), > + > NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, > + ngx_conf_set_str_slot, > + NGX_HTTP_LOC_CONF_OFFSET, > + offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_ca_certificate), > + NULL }, > #endif See above. > > ngx_null_command > @@ -2421,6 +2441,8 @@ ngx_http_proxy_create_loc_conf(ngx_conf_ > conf->upstream.intercept_errors = NGX_CONF_UNSET; > #if (NGX_HTTP_SSL) > conf->upstream.ssl_session_reuse = NGX_CONF_UNSET; > + conf->upstream.ssl_verify_peer = NGX_CONF_UNSET; > + conf->upstream.ssl_verify_depth = NGX_CONF_UNSET_UINT; > #endif > > /* "proxy_cyclic_temp_file" is disabled */ > @@ -2697,6 +2719,22 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t > #if (NGX_HTTP_SSL) > ngx_conf_merge_value(conf->upstream.ssl_session_reuse, > prev->upstream.ssl_session_reuse, 1); > + ngx_conf_merge_value(conf->upstream.ssl_verify_peer, > + prev->upstream.ssl_verify_peer, 0); > + ngx_conf_merge_uint_value(conf->upstream.ssl_verify_depth, > + prev->upstream.ssl_verify_depth, 1); > + ngx_conf_merge_str_value(conf->upstream.ssl_ca_certificate, > + prev->upstream.ssl_ca_certificate, ""); > + > + if (conf->upstream.ssl_verify_peer) { > + if (conf->upstream.ssl_ca_certificate.len == 0) { > + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, > + "no \"proxy_ssl_ca_certificate\" is defined > for " > + "the \"proxy_ssl_verify_peer\" directive"); > + > + return NGX_CONF_ERROR; > + } > + } > #endif > > ngx_conf_merge_value(conf->redirect, prev->redirect, 1); > diff -Nrpu nginx-1.4.1/src/http/ngx_http_upstream.c > nginx-1.4.1-proxy-ssl-verify/src/http/ngx_http_upstream.c > --- nginx-1.4.1/src/http/ngx_http_upstream.c 2013-05-06 13:26:50.000000000 > +0300 > +++ nginx-1.4.1-proxy-ssl-verify/src/http/ngx_http_upstream.c 2013-08-20 > 14:59:29.437251122 +0300 > @@ -1281,6 +1281,15 @@ ngx_http_upstream_ssl_init_connection(ng > { > ngx_int_t rc; > > + if (ngx_ssl_set_verify_options(u->conf->ssl, > + &u->conf->ssl_ca_certificate, u->conf->ssl_verify_depth) > + != NGX_OK) > + { > + ngx_http_upstream_finalize_request(r, u, > + NGX_HTTP_INTERNAL_SERVER_ERROR); > + return; > + } > + Calling this on every connection attempt is silly. > if (ngx_ssl_create_connection(u->conf->ssl, c, > NGX_SSL_BUFFER|NGX_SSL_CLIENT) > != NGX_OK) > @@ -1324,6 +1333,12 @@ ngx_http_upstream_ssl_handshake(ngx_conn > u = r->upstream; > > if (c->ssl->handshaked) { > + if (u->conf->ssl_verify_peer && ngx_ssl_verify_result(c) != > NGX_OK) { > + ngx_log_error(NGX_LOG_ERR, c->log, 0, "upstream ssl > certificate validation failed"); > + ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR); > + goto fail; > + } > + > > if (u->conf->ssl_session_reuse) { > u->peer.save_session(&u->peer, u->peer.data); > @@ -1334,6 +1349,11 @@ ngx_http_upstream_ssl_handshake(ngx_conn > > ngx_http_upstream_send_request(r, u); > > +fail: > + c = r->connection; > + > + ngx_http_run_posted_requests(c); The "c = r->connection;" part should be before the ngx_http_upstream_next() call where a request could be freed. > + > return; > } > > diff -Nrpu nginx-1.4.1/src/http/ngx_http_upstream.h > nginx-1.4.1-proxy-ssl-verify/src/http/ngx_http_upstream.h > --- nginx-1.4.1/src/http/ngx_http_upstream.h 2013-05-06 13:26:50.000000000 > +0300 > +++ nginx-1.4.1-proxy-ssl-verify/src/http/ngx_http_upstream.h 2013-08-20 > 15:00:10.281251422 +0300 > @@ -191,6 +191,9 @@ typedef struct { > #if (NGX_HTTP_SSL) > ngx_ssl_t *ssl; > ngx_flag_t ssl_session_reuse; > + ngx_flag_t ssl_verify_peer; > + ngx_uint_t ssl_verify_depth; > + ngx_str_t ssl_ca_certificate; > #endif > > ngx_str_t module; > _______________________________________________ > nginx-devel mailing list > [email protected] > http://mailman.nginx.org/mailman/listinfo/nginx-devel -- Maxim Dounin http://nginx.org/en/donation.html _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
