[PATCH] Proxy SSL Verify
W. Andrew Loe III
andrew at andrewloe.com
Thu Sep 15 00:47:11 UTC 2011
Thank you for the help! I am now working against 1.1.2 which was the
latest release this morning. My code is available on github
(https://github.com/loe/nginx/tree/proxy_ssl_verify) but I will also
include it here.
I am having an issue with SSL_get_verify_result never failing, it
always returns 0 even with ngx_http_verify_callback logs that the
certificate has not been verified. I have inserted a few debugging
statements to illustrate the behavior. I have also included a location
you can put in the default nginx.conf that will proxy to an SSL
server.
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
index 259b1d8..078978b 100644
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -216,13 +216,10 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t
*ssl, ngx_str_t *cert,
return NGX_OK;
}
-
ngx_int_t
-ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert,
+ngx_ssl_set_verify_options(ngx_ssl_t *ssl, ngx_str_t *cert,
ngx_int_t depth)
{
- STACK_OF(X509_NAME) *list;
-
SSL_CTX_set_verify(ssl->ctx, SSL_VERIFY_PEER,
ngx_http_ssl_verify_callback);
SSL_CTX_set_verify_depth(ssl->ctx, depth);
@@ -231,10 +228,6 @@ ngx_ssl_client_certificate(ngx_conf_t *cf,
ngx_ssl_t *ssl, ngx_str_t *cert,
return NGX_OK;
}
- if (ngx_conf_full_name(cf->cycle, cert, 1) != NGX_OK) {
- return NGX_ERROR;
- }
-
if (SSL_CTX_load_verify_locations(ssl->ctx, (char *) cert->data, NULL)
== 0)
{
@@ -244,6 +237,23 @@ ngx_ssl_client_certificate(ngx_conf_t *cf,
ngx_ssl_t *ssl, ngx_str_t *cert,
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)
+{
+ STACK_OF(X509_NAME) *list;
+
+ if (ngx_ssl_set_verify_options(ssl, cert, depth) != NGX_OK) {
+ return NGX_ERROR;
+ }
+
+ if (ngx_conf_full_name(cf->cycle, cert, 1) != NGX_OK) {
+ return NGX_ERROR;
+ }
+
list = SSL_load_client_CA_file((char *) cert->data);
if (list == NULL) {
@@ -350,7 +360,7 @@ ngx_http_ssl_verify_callback(int ok,
X509_STORE_CTX *x509_store)
}
#endif
- return 1;
+ return ok;
}
@@ -566,7 +576,7 @@ ngx_ssl_set_session(ngx_connection_t *c,
ngx_ssl_session_t *session)
ngx_int_t
ngx_ssl_handshake(ngx_connection_t *c)
{
- int n, sslerr;
+ int n, sslerr, verify_err, verify_mode;
ngx_err_t err;
ngx_ssl_clear_error(c->log);
@@ -577,6 +587,22 @@ ngx_ssl_handshake(ngx_connection_t *c)
if (n == 1) {
+ if (SSL_get_peer_certificate(c->ssl->connection) != NULL)
+ {
+ ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
"SSL_get_peer_certificate is present");
+ }
+
+ verify_mode = SSL_get_verify_mode(c->ssl->connection);
+ ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
"SSL_get_verify_mode: %d", verify_mode);
+
+ verify_err = SSL_get_verify_result(c->ssl->connection);
+ ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
"SSL_get_verify_result: %d", verify_err);
+ if (verify_err != X509_V_OK)
+ {
+ ngx_ssl_error(NGX_LOG_ALERT, c->log, 0,
"SSL_get_verify_result() failed");
+ return NGX_ERROR;
+ }
+
if (ngx_handle_read_event(c->read, 0) != NGX_OK) {
return NGX_ERROR;
}
@@ -2354,3 +2380,4 @@ ngx_openssl_exit(ngx_cycle_t *cycle)
EVP_cleanup();
ENGINE_cleanup();
}
+
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
index 33cab7b..0aac3e8 100644
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -96,6 +96,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_crl(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *crl);
diff --git a/src/http/modules/ngx_http_proxy_module.c
b/src/http/modules/ngx_http_proxy_module.c
index 902cfb8..834301e 100644
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -440,7 +440,27 @@ static ngx_command_t ngx_http_proxy_commands[] = {
NGX_HTTP_LOC_CONF_OFFSET,
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 },
+
+ { 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
ngx_null_command
@@ -1697,6 +1717,8 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf)
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 */
@@ -1955,6 +1977,22 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf,
void *parent, void *child)
#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 --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
index 29432dc..474cf0d 100644
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -1210,6 +1210,15 @@
ngx_http_upstream_ssl_init_connection(ngx_http_request_t *r,
{
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;
+ }
+
if (ngx_ssl_create_connection(u->conf->ssl, c,
NGX_SSL_BUFFER|NGX_SSL_CLIENT)
!= NGX_OK)
@@ -4527,3 +4536,4 @@ ngx_http_upstream_init_main_conf(ngx_conf_t *cf,
void *conf)
return NGX_CONF_OK;
}
+
diff --git a/src/http/ngx_http_upstream.h b/src/http/ngx_http_upstream.h
index fa848c0..cc71ba9 100644
--- a/src/http/ngx_http_upstream.h
+++ b/src/http/ngx_http_upstream.h
@@ -177,6 +177,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;
On Tue, Sep 13, 2011 at 5:17 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> Hello!
>
> On Tue, Sep 13, 2011 at 02:44:48PM -0700, W. Andrew Loe III wrote:
>
>> This patch allows you to force OpenSSL to validate the certificate of
>> the server the http_proxy module is communicating with. Originally
>> built against 0.7.x branch, I will forward port when I can. I would
>> appreciate if anyone else has input on how to do this more elegantly,
>> my skills are rudimentary at best.
>>
>>
>> diff -uNr ../nginx-0.7.67/src/event/ngx_event_openssl.c
>> src/event/ngx_event_openssl.c
>> --- ../nginx-0.7.67/src/event/ngx_event_openssl.c 2010-06-07
>> 04:55:20.000000000 -0700
>> +++ src/event/ngx_event_openssl.c 2011-09-13 14:17:05.000000000 -0700
>> @@ -157,6 +157,12 @@
>> SSL_CTX_set_options(ssl->ctx, SSL_OP_NETSCAPE_CHALLENGE_BUG);
>> SSL_CTX_set_options(ssl->ctx, SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG);
>>
>> + /* verification options */
>> +
>> + SSL_CTX_load_verify_locations(ssl->ctx, (const char
>> *)ssl->ca_certificate.data, NULL);
>> + SSL_CTX_set_verify(ssl->ctx, ssl->verify, NULL);
>> + SSL_CTX_set_verify_depth(ssl->ctx, ssl->verify_depth);
>> +
>
> This should be done in separate function, similar to
> ngx_ssl_client_certificate() (actually, subset of it). And with
> appropriate error checking.
>
>> /* server side options */
>>
>> SSL_CTX_set_options(ssl->ctx, SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG);
>> diff -uNr ../nginx-0.7.67/src/event/ngx_event_openssl.h
>> src/event/ngx_event_openssl.h
>> --- ../nginx-0.7.67/src/event/ngx_event_openssl.h 2010-06-07
>> 03:09:14.000000000 -0700
>> +++ src/event/ngx_event_openssl.h 2011-09-13 14:17:05.000000000 -0700
>> @@ -27,6 +27,9 @@
>> typedef struct {
>> SSL_CTX *ctx;
>> ngx_log_t *log;
>> + ngx_uint_t verify;
>> + ngx_uint_t verify_depth;
>> + ngx_str_t ca_certificate;
>> } ngx_ssl_t;
>
> This shouldn't be here at all.
>
>>
>>
>> diff -uNr ../nginx-0.7.67/src/http/modules/ngx_http_proxy_module.c
>> src/http/modules/ngx_http_proxy_module.c
>> --- ../nginx-0.7.67/src/http/modules/ngx_http_proxy_module.c 2010-06-07
>> 05:23:23.000000000 -0700
>> +++ src/http/modules/ngx_http_proxy_module.c 2011-09-13 14:17:05.000000000 -0700
>> @@ -466,6 +466,27 @@
>> 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_num_slot,
>> + NGX_HTTP_LOC_CONF_OFFSET,
>> + offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_verify),
>> + NULL },
>
> You don't want to let users to control binary arguments passed to
> openssl.
>
> It should be either on/off switch (flag slot), or should go away
> completely, switched on by certificate file presense.
>
> If it stays, it probably should be named as
> "proxy_ssl_verify_peer" to be in line with "ssl_verify_client"
> directive (and see below).
>
>> +
>> + { 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"),
>
> Probably "proxy_ssl_peer_certificate" would be a better directive
> name. Not sure.
>
>> + 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
>>
>> ngx_null_command
>> @@ -1950,6 +1971,8 @@
>> 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_UINT;
>> + conf->upstream.ssl_verify_depth = NGX_CONF_UNSET_UINT;
>> #endif
>>
>> /* "proxy_cyclic_temp_file" is disabled */
>> @@ -2196,6 +2219,22 @@
>> #if (NGX_HTTP_SSL)
>> ngx_conf_merge_value(conf->upstream.ssl_session_reuse,
>> prev->upstream.ssl_session_reuse, 1);
>> + ngx_conf_merge_uint_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_ca_certificate,
>> + prev->upstream.ssl_ca_certificate, "");
>> +
>> + if (conf->upstream.ssl_verify) {
>> + 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\" directive");
>> +
>> + return NGX_CONF_ERROR;
>> + }
>
> No 2-space indentation, please.
>
>> + }
>> #endif
>>
>> ngx_conf_merge_value(conf->redirect, prev->redirect, 1);
>> @@ -3011,6 +3050,12 @@
>>
>> plcf->upstream.ssl->log = cf->log;
>>
>> + plcf->upstream.ssl->ca_certificate.len =
>> plcf->upstream.ssl_ca_certificate.len;
>> + plcf->upstream.ssl->ca_certificate.data =
>> plcf->upstream.ssl_ca_certificate.data;
>> +
>> + plcf->upstream.ssl->verify = plcf->upstream.ssl_verify;
>> + plcf->upstream.ssl->verify_depth = plcf->upstream.ssl_verify_depth;
>> +
>> if (ngx_ssl_create(plcf->upstream.ssl,
>> NGX_SSL_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1, NULL)
>> != NGX_OK)
>> diff -uNr ../nginx-0.7.67/src/http/ngx_http_upstream.h
>> src/http/ngx_http_upstream.h
>> --- ../nginx-0.7.67/src/http/ngx_http_upstream.h 2010-06-07
>> 05:23:23.000000000 -0700
>> +++ src/http/ngx_http_upstream.h 2011-09-13 14:17:05.000000000 -0700
>> @@ -173,6 +173,9 @@
>> #if (NGX_HTTP_SSL)
>> ngx_ssl_t *ssl;
>> ngx_flag_t ssl_session_reuse;
>> + ngx_uint_t ssl_verify;
>> + ngx_uint_t ssl_verify_depth;
>> + ngx_str_t ssl_ca_certificate;
>> #endif
>>
>> } ngx_http_upstream_conf_t;
>
> You may also want to add "proxy_ssl_crl" directive (trivial), as
> well as some form of remote CN checking.
>
> Please also note that posting patches against 0.7.* (as well as
> 0.8.*), isn't meaningful. Development branch is 1.1.*.
>
> Maxim Dounin
>
> _______________________________________________
> 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.conf
Type: application/octet-stream
Size: 4275 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20110914/6d121d13/attachment.obj>
More information about the nginx-devel
mailing list