[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