[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