Problem with multiline $ssl_client_cert HTTP header in proxy mode

[michal-niklas@…]

$ssl_client_cert variable is multiline while RFC7230 deprecated folding headers. I want to send certificate info to Jetty server which refuses it with HTTP/1.1 400 Bad Continuation error. I thought it was Jetty error and filled bug report to them: ​https://bugs.eclipse.org/bugs/show_bug.cgi?id=483795

In nginx documentation there is info that this folding is on purpose
​http://nginx.org/en/docs/http/ngx_http_ssl_module.html
:

$ssl_client_cert

returns the client certificate in the PEM format for an established SSL connection, with each line except the first prepended with the tab character; this is intended for the use in the proxy_set_header directive;

I think those headers should be sent in one line.

[gregw@…]

Header folding has been deprecated in the latest RFC ​http://tools.ietf.org/html/rfc7230#section-3.2.4

[Maxim Dounin]

This is certainly isn't a bug, as it works as intended and exactly as documented. Whether or not it will work with your backend is another question. But given the fact that header folding is indeed deprecated and not supported even by nginx itself, we certainly need a better way to send client certificates to backends.

[mmitar@…]

This has been widely reported in the past, and a hacky solution has been provided: ​https://forum.nginx.org/read.php?29,249804,249848#msg-249848

Furthermore, a patch has also been submitted: ​https://forum.nginx.org/read.php?2,236546,236596

[dren.dk@…]

I've been bitten by this exact problem, here's my proposed fix, which I'll post to the mailing list as soon as I'm allowed to.

My fix is as simple as removing all newlines from the PEM certificate, because it's already base64 encoded and newlines don't really matter when decoding base64.

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
index 5c7734d..48ddae0 100644
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -3434,6 +3434,42 @@ ngx_ssl_get_certificate(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
     return NGX_OK;
 }
 
+ngx_int_t
+ngx_ssl_get_bare_certificate(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
+{
+    size_t       len;
+    ngx_uint_t   i;
+    ngx_str_t    cert;
+
+    if (ngx_ssl_get_raw_certificate(c, pool, &cert) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    if (cert.len == 0) {
+        s->len = 0;
+        return NGX_OK;
+    }
+
+       // The stripped certificate is never going to be bigger than the input.
+       // but it's not going to be a whole lot longer, so we'll just allocate the same number of bytes.
+    s->data = ngx_pnalloc(pool, cert.len); 
+    if (s->data == NULL) {
+        return NGX_ERROR;
+    }
+
+       len = 0;
+    for (i = 0; i < cert.len - 1; i++) {
+           u_char ch = cert.data[i];
+        if (ch != LF) {
+                       s->data[len++] = ch;
+        }
+    }
+
+    s->len = len;
+
+    return NGX_OK;
+}
+
 
 ngx_int_t
 ngx_ssl_get_subject_dn(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
index d233c02..62cc6e8 100644
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -201,6 +201,8 @@ ngx_int_t ngx_ssl_get_raw_certificate(ngx_connection_t *c, ngx_pool_t *pool,
     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_bare_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 --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
index e75f5d8..889fd18 100644
--- a/src/http/modules/ngx_http_ssl_module.c
+++ b/src/http/modules/ngx_http_ssl_module.c
@@ -288,6 +288,9 @@ static ngx_http_variable_t  ngx_http_ssl_vars[] = {
     { ngx_string("ssl_client_cert"), NULL, ngx_http_ssl_variable,
       (uintptr_t) ngx_ssl_get_certificate, NGX_HTTP_VAR_CHANGEABLE, 0 },
 
+    { ngx_string("ssl_client_bare_cert"), NULL, ngx_http_ssl_variable,
+      (uintptr_t) ngx_ssl_get_bare_certificate, NGX_HTTP_VAR_CHANGEABLE, 0 },
+
     { ngx_string("ssl_client_raw_cert"), NULL, ngx_http_ssl_variable,
       (uintptr_t) ngx_ssl_get_raw_certificate,
       NGX_HTTP_VAR_CHANGEABLE, 0 },

[dmitry.rudenko.moneta.ru@…]

We use apache httpd as a backend server.

Changes with Apache 2.4.24:
SECURITY: CVE-2016-8743 (cve.mitre.org)

Enforce HTTP request grammar corresponding to RFC7230 for request lines
and request headers, to prevent response splitting and cache pollution by
malicious clients or downstream proxies. [William Rowe, Stefan Fritsch]

$ssl_client_cert with proxy_set_header does not work anymore.

We will be happy with any solution from thread ​https://forum.nginx.org/read.php?29,271154,271154#msg-271154

New variable, encoding, plain spaces, anything other than "ignore 857"

[Renkas@…]

Any progress on getting this fixed?

[Maxim Dounin]

The following patch introduces the $ssl_client_escaped_cert variable.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1502810183 -10800
#      Tue Aug 15 18:16:23 2017 +0300
# Node ID 4354e33536c62c9cd1b911d0fbd69e4243da83e8
# Parent  a2f5e25d6a283546f76435b9fc3e7e814b092bae
SSL: the $ssl_client_escaped_cert variable (ticket #857).

This variable contains URL-encoded client SSL certificate.  In contrast
to $ssl_client_cert, it doesn't depend on deprecated header continuation.
The NGX_ESCAPE_URI_COMPONENT variant of encoding is used, so the resulting
variable can be safely used not only in headers, but also as a request
argument.

The $ssl_client_cert variable should be considered deprecated now.
The $ssl_client_raw_cert variable will be eventually renambed back
to $ssl_client_cert.

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -3663,6 +3663,36 @@ ngx_ssl_get_certificate(ngx_connection_t
 
 
 ngx_int_t
+ngx_ssl_get_escaped_certificate(ngx_connection_t *c, ngx_pool_t *pool,
+    ngx_str_t *s)
+{
+    ngx_str_t  cert;
+    uintptr_t  n;
+
+    if (ngx_ssl_get_raw_certificate(c, pool, &cert) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    if (cert.len == 0) {
+        s->len = 0;
+        return NGX_OK;
+    }
+
+    n = ngx_escape_uri(NULL, cert.data, cert.len, NGX_ESCAPE_URI_COMPONENT);
+
+    s->len = cert.len + n * 2;
+    s->data = ngx_pnalloc(pool, s->len);
+    if (s->data == NULL) {
+        return NGX_ERROR;
+    }
+
+    ngx_escape_uri(s->data, cert.data, cert.len, NGX_ESCAPE_URI_COMPONENT);
+
+    return NGX_OK;
+}
+
+
+ngx_int_t
 ngx_ssl_get_subject_dn(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
 {
     BIO        *bio;
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -212,6 +212,8 @@ ngx_int_t ngx_ssl_get_raw_certificate(ng
     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 --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c
+++ b/src/http/modules/ngx_http_ssl_module.c
@@ -299,6 +299,10 @@ static ngx_http_variable_t  ngx_http_ssl
       (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 },
 
diff --git a/src/stream/ngx_stream_ssl_module.c b/src/stream/ngx_stream_ssl_module.c
--- a/src/stream/ngx_stream_ssl_module.c
+++ b/src/stream/ngx_stream_ssl_module.c
@@ -249,6 +249,10 @@ static ngx_stream_variable_t  ngx_stream
       (uintptr_t) ngx_ssl_get_raw_certificate,
       NGX_STREAM_VAR_CHANGEABLE, 0 },
 
+    { ngx_string("ssl_client_escaped_cert"), NULL, ngx_stream_ssl_variable,
+      (uintptr_t) ngx_ssl_get_escaped_certificate,
+      NGX_STREAM_VAR_CHANGEABLE, 0 },
+
     { ngx_string("ssl_client_s_dn"), NULL, ngx_stream_ssl_variable,
       (uintptr_t) ngx_ssl_get_subject_dn, NGX_STREAM_VAR_CHANGEABLE, 0 },
 

[Maxim Dounin <mdounin@…>]

In 7091:82f0b8dcca27/nginx:

SSL: the $ssl_client_escaped_cert variable (ticket #857).

This variable contains URL-encoded client SSL certificate. In contrast
to $ssl_client_cert, it doesn't depend on deprecated header continuation.
The NGX_ESCAPE_URI_COMPONENT variant of encoding is used, so the resulting
variable can be safely used not only in headers, but also as a request
argument.

The $ssl_client_cert variable should be considered deprecated now.
The $ssl_client_raw_cert variable will be eventually renambed back
to $ssl_client_cert.

[Maxim Dounin]

The $ssl_client_escaped_cert variable will be available in the next nginx release, 1.13.5. Thanks for prodding this.