[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