[PATCH] Proxy remote server SSL certificate verification
Maxim Dounin
mdounin at mdounin.ru
Tue Aug 20 14:09:12 UTC 2013
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
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Maxim Dounin
http://nginx.org/en/donation.html
More information about the nginx-devel
mailing list