[PATCH] Proxy remote server SSL certificate verification

Aviram Cohen aviram at adallom.com
Tue Aug 27 08:47:38 UTC 2013


Added a new version, with all the required fixes.

diff -Nrpu nginx-1.4.1/src/http/modules/ngx_http_proxy_module.c
nginx-1.4.1-proxy-ssl-verification/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-verification/src/http/modules/ngx_http_proxy_module.c
2013-08-26 10:43:15.639557701 +0300
@@ -511,6 +511,27 @@ 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_trusted_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
@@ -2419,8 +2440,11 @@ ngx_http_proxy_create_loc_conf(ngx_conf_
     conf->upstream.pass_headers = NGX_CONF_UNSET_PTR;

     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 +2721,30 @@ 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_trusted_certificate\"
is defined for "
+                               "the \"proxy_ssl_verify\" directive");
+
+            return NGX_CONF_ERROR;
+        }
+    }
+
+    if (conf->upstream.ssl &&
+        ngx_ssl_trusted_certificate(cf, conf->upstream.ssl,
+          &conf->upstream.ssl_certificate,
conf->upstream.ssl_verify_depth) != NGX_OK)
+    {
+        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-verification/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-verification/src/http/ngx_http_upstream.c
2013-08-26 10:44:35.323558884 +0300
@@ -1324,7 +1324,13 @@ ngx_http_upstream_ssl_handshake(ngx_conn
     u = r->upstream;

     if (c->ssl->handshaked) {
-
+        if (u->conf->ssl_verify &&
SSL_get_verify_result(c->ssl->connection) != X509_V_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);
         }
@@ -1332,13 +1338,21 @@ ngx_http_upstream_ssl_handshake(ngx_conn
         c->write->handler = ngx_http_upstream_handler;
         c->read->handler = ngx_http_upstream_handler;

+        c = r->connection;
+
         ngx_http_upstream_send_request(r, u);

+fail:
+        ngx_http_run_posted_requests(c);
+
         return;
     }

+    c = r->connection;
+
     ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);

+    ngx_http_run_posted_requests(c);
 }

 #endif
diff -Nrpu nginx-1.4.1/src/http/ngx_http_upstream.h
nginx-1.4.1-proxy-ssl-verification/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-verification/src/http/ngx_http_upstream.h
2013-08-26 10:44:56.263558679 +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 Thu, Aug 22, 2013 at 5:00 PM, Aviram Cohen <aviram at adallom.com> wrote:
> Hello!
>
> I have a couple of questions regarding the two last comments:
>
> On Wed, Aug 21, 2013 at 5:30 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
>> Hello!
>>
> [..]
>>> @@ -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;
>>>      }
>>
>> This is called before options used are correctly set.
>
> Where do you think this call should be performed? Should we add a
> postconfiguration callback for
>  the proxy module from which this would be called?
> (BTW, I'll remove ngx_ssl_set_verify_options() and use
> ngx_ssl_trusted_certificate() directly instead, as it is better)
>
>>
>> (There is also a style problem here, but it doesn't really matter
>> as you'll have to rewrite the code anyway.)
>>
>> [...]
>>
>>> @@ -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;
>>>      }
>>>
>>
>> You probably missed my previous comment.  You have a use after
>> free problem here.  Try triggering an error in
>> ngx_http_upstream_send_request() with NGX_DEBUG_MALLOC defined, it
>> should segfault.
>
> You're right, I've missed it... Should we check before
> ngx_http_send_request() is called whether or not the
> request has a parent request, and accordingly decide later whether to
> call ngx_http_run_posted_requests() or not?
>
>>
>> [...]
>>
>> --
>> 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
>
> Best regrads,
> Aviram
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nginx-1.4.1-proxy-ssl-verification.patch
Type: application/octet-stream
Size: 5083 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130827/a85538a7/attachment-0001.obj>


More information about the nginx-devel mailing list