Proposed changeset to fix client cert from ngx_ssl_get_certificate passed as HTTP header value
Sam McKelvie
sam at surround.io
Wed Feb 10 22:38:43 UTC 2016
OK, sorry for the delay.
Here is a changeset that implements $ssl_client_escaped_certificate,
following your latest comments. It works well with node/express. OK
with this?
~Sam
# HG changeset patch
# User Sam McKelvie <sam at surround.io>
# Date 1455143619 28800
# Wed Feb 10 14:33:39 2016 -0800
# Node ID f3f2dbd8340f04adccd805ff5a8547e10e6fa874
# Parent e0d7c2f718515d4c48e5ed2a0643294da60cf2ba
Define a new $ssl_client_escaped_certificate variable that is the
URL-encoded form of the raw PEM client certificate. This variable
is always appropriate for inclusion in proxy_set_header values,
unlike the $ssl_client_cert variable, which includes '\n'
characters that are not prefixed by '\r', breaking some http
servers (e.g., node/express).
Per Maxim, $ssl_client_cert should eventually be deprecated. Users
of $ssl_client_cert should move to ssl_client_scaped_certificate.
Tested with node/express and nginx-tests.
diff -r e0d7c2f71851 -r f3f2dbd8340f src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Fri Feb 05 14:02:54 2016 +0300
+++ b/src/event/ngx_event_openssl.c Wed Feb 10 14:33:39 2016 -0800
@@ -3273,6 +3273,42 @@
ngx_int_t
+ngx_ssl_get_escaped_certificate(ngx_connection_t *c, ngx_pool_t *pool,
+ ngx_str_t *s)
+{
+ size_t len;
+ ngx_str_t cert;
+ uintptr_t escape;
+
+ s->data = NULL;
+ s->len = 0;
+
+ if (ngx_ssl_get_raw_certificate(c, pool, &cert) != NGX_OK) {
+ return NGX_ERROR;
+ }
+
+ escape = ngx_escape_uri(NULL, cert.data, cert.len,
+ NGX_ESCAPE_URI_COMPONENT);
+
+ /* Each escaped character adds 2 characters to result */
+ len = cert.len + 2 * escape;
+
+ s->data = ngx_pnalloc(pool, len);
+ if (s->data == NULL) {
+ ngx_pfree(pool, cert.data);
+ return NGX_ERROR;
+ }
+ s->len = len;
+
+ ngx_escape_uri(s->data, cert.data, cert.len, NGX_ESCAPE_URI_COMPONENT);
+
+ ngx_pfree(pool, cert.data);
+
+ return NGX_OK;
+}
+
+
+ngx_int_t
ngx_ssl_get_subject_dn(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
{
char *p;
diff -r e0d7c2f71851 -r f3f2dbd8340f src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h Fri Feb 05 14:02:54 2016 +0300
+++ b/src/event/ngx_event_openssl.h Wed Feb 10 14:33:39 2016 -0800
@@ -180,6 +180,8 @@
ngx_str_t *s);
ngx_int_t ngx_ssl_get_certificate(ngx_connection_t *c, ngx_pool_t *pool,
ngx_str_t *s);
+ngx_int_t ngx_ssl_get_escaped_certificate(ngx_connection_t *c,
ngx_pool_t *pool,
+ ngx_str_t *s);
ngx_int_t ngx_ssl_get_subject_dn(ngx_connection_t *c, ngx_pool_t *pool,
ngx_str_t *s);
ngx_int_t ngx_ssl_get_issuer_dn(ngx_connection_t *c, ngx_pool_t *pool,
diff -r e0d7c2f71851 -r f3f2dbd8340f src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c Fri Feb 05 14:02:54 2016 +0300
+++ b/src/http/modules/ngx_http_ssl_module.c Wed Feb 10 14:33:39 2016 -0800
@@ -292,6 +292,9 @@
(uintptr_t) ngx_ssl_get_raw_certificate,
NGX_HTTP_VAR_CHANGEABLE, 0 },
+ { ngx_string("ssl_client_escaped_cert"), NULL, ngx_http_ssl_variable,
+ (uintptr_t) ngx_ssl_get_escaped_certificate,
NGX_HTTP_VAR_CHANGEABLE, 0 },
+
{ ngx_string("ssl_client_s_dn"), NULL, ngx_http_ssl_variable,
(uintptr_t) ngx_ssl_get_subject_dn, NGX_HTTP_VAR_CHANGEABLE, 0 },
On Fri, Feb 5, 2016 at 7:58 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> Hello!
>
> On Thu, Feb 04, 2016 at 01:38:48PM -0800, Sam McKelvie wrote:
>
>> I think it is your call if you want to make a breaking change to
>> $ssl_client_cert to URL encode it; I'd be happy to submit the
>> changeset if you would approve that, but I personally don't feel
>> comfortable breaking any existing applications that parse/decode the
>> certificate.
>
> No, certainly not something I want to be done.
>
>> So my suggestion now is to define a new $ssl_client_cert_url_encoded
>> variable that is the URL-encoded form of the raw PEM certificate. With
>> your approval I will submit a changeset for that...
>
> Yes, adding a variable with an URL-escaped versions looks like a
> way to go. I disagree with the name you suggest though, I think
> that something like $ssl_client_escaped_cert would be more in line
> with $ssl_client_cert and $ssl_client_raw_cert variables we
> currently have and the ngx_escape_uri() function nginx uses
> internally.
>
> Some more background. As of now we have:
>
> - $ssl_client_raw_cert - client cert in PEM format
> - $ssl_client_cert - client cert in PEM format with \t added
>
> At some distant point in the future we probably want to have:
>
> - $ssl_client_cert - client cert in PEM format
> - a way to urlescape() things, see
> https://trac.nginx.org/nginx/ticket/52
>
> At this point, an escaped version of the client cert will be
> available as something like ${urlescape($ssl_client_cert)}. All
> uses of client cert with tabs are expected to disappear. There
> are a couple of problems though:
>
> - there are existing uses of $ssl_client_cert and
> $ssl_client_raw_cert, breaking them would be bad;
>
> - we don't have urlescape() function in configs, and probably
> won't have it in a near future.
>
> So we have to figure out some migration plan, e.g.:
>
> - introduce $ssl_client_escaped_cert, with urlescaped PEM cert;
>
> - introduce $ssl_client_tabbed_cert as an alias to
> $ssl_client_cert (with PEM cert with tabs);
>
> - change $ssl_client_cert back to be raw cert (preserving
> $ssl_client_raw_cert as a deprecated alias for some time);
>
> - do something with $ssl_client_tabbed_cert at some point, not
> sure;
>
> - once urlescape() functionality is added, deprecate
> $ssl_client_escaped_cert, suggesting to use
> urlescape($ssl_client_cert) instead.
>
> Not sure if it's an optimal plan and if we are actually going to
> follow it, but introducing $ssl_client_escaped_cert looks like a
> more or less obvious 1st step, at least if we don't expect
> urlescape() to appear in the near future.
>
> --
> Maxim Dounin
> http://nginx.org/
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
More information about the nginx-devel
mailing list