[PATCH] Proxy remote server SSL certificate verification

Aviram Cohen aviram at adallom.com
Wed Aug 21 11:45:55 UTC 2013


Hello!

Thank you for the useful feedback!
I think it's better not to use ssl_trusted_certificate (for a more
extensible solution).
The following is the fixed patch. I've also attached it in case Gmail
corrupts it.

Thanks!


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-21 14:18:58.529251404 +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);
+
+    SSL_CTX_set_verify_depth(ssl->ctx, depth);
+
+    if (SSL_CTX_load_verify_locations(ssl->ctx, (char *) cert->data, NULL)
+        == 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-21 14:18:58.529251404 +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-21 14:18:58.517251370 +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"),
+      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),
+      NULL },
+
+    { 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_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_certificate),
+      NULL },
 #endif

       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 = 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,
+                              prev->upstream.ssl_verify, 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_certificate,
+                              prev->upstream.ssl_certificate, "");
+
+    if (conf->upstream.ssl_verify) {
+        if (conf->upstream.ssl_certificate.len == 0) {
+            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                               "no \"proxy_ssl_certificate\" is defined for "
+                               "the \"proxy_ssl_verify\" directive");
+
+            return NGX_CONF_ERROR;
+        }
+    }
 #endif

     ngx_conf_merge_value(conf->redirect, prev->redirect, 1);
@@ -3748,6 +3786,13 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n
         != NGX_OK)
     {
         return NGX_ERROR;
+    }
+
+    if (ngx_ssl_set_verify_options(plcf->upstream.ssl,
+          &plcf->upstream.ssl_certificate, plcf->upstream.ssl_verify_depth)
+        != NGX_OK)
+    {
+        return NGX_ERROR;
     }

     cln = ngx_pool_cleanup_add(cf->pool, 0);
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-21 14:18:58.521251394 +0300
@@ -1324,6 +1324,13 @@ ngx_http_upstream_ssl_handshake(ngx_conn
     u = r->upstream;

     if (c->ssl->handshaked) {
+        if (u->conf->ssl_verify && ngx_ssl_verify_result(c) != NGX_OK) {
+            ngx_log_error(NGX_LOG_ERR, c->log, 0, "upstream ssl
certificate validation failed");
+            c = r->connection;
+            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 +1341,11 @@ ngx_http_upstream_ssl_handshake(ngx_conn

         ngx_http_upstream_send_request(r, u);

+        c = r->connection;
+
+fail:
+        ngx_http_run_posted_requests(c);
+
         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-21 14:18:58.521251394 +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;
+    ngx_uint_t                       ssl_verify_depth;
+    ngx_str_t                        ssl_certificate;
 #endif

     ngx_str_t                        module;




On Tue, Aug 20, 2013 at 5:09 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 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
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nginx-1.4.1-proxy-ssl-verify.patch
Type: application/octet-stream
Size: 7445 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130821/0bf5b28d/attachment-0001.obj>


More information about the nginx-devel mailing list