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@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel