[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