The ngx_ssl_get_certificate() changes “\n” to “\n\t” in the returned PEM string in an effort to make the string usable as an HTTP header value with $ssl_client_cert. However, bare ‘\n’ (without a preceding ‘\r’) is passed along as “\n\t". This causes some HTTP servers (including node/express) to hang up. This changeset fixes the problem by replacing occurrences of ‘\n’ that have no preceding ‘\r’ with "\r\n\t".
Tested with node.js/express and nginx-tests. I should note that a similar solution was proposed at https://forum.nginx.org/read.php?29,249804,249833 <https://forum.nginx.org/read.php?29,249804,249833>, but the thread never went anywhere. This solution is slightly more paranoid with edge cases and does not insert extra ‘\r’ characters if they are already present. Thank you, Sam McKelvie # HG changeset patch # User Sam McKelvie <[email protected] <mailto:[email protected]>> # Date 1454541255 28800 # Wed Feb 03 15:14:15 2016 -0800 # Node ID d70dae44196b5625f10080b4b869f33adbb83f54 # Parent 0e0e2e522fa2cb439b636a2c19aca5bdfbe8704f Fix ngx_ssl_get_certificate to properly format for HTTP header value. Previously, the value could contain lines ending in '\n' rather than '\r\n'. This caused some HTTP servers, including node/express, to reject the request entirely. Now, instances of bare '\n' are replaced with "\r\n\t". Instances of "\r\n" are still replaced with "\r\n\t". Tested with node.js/express and nginx-tests. diff -r 0e0e2e522fa2 -r d70dae44196b src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c Tue Feb 02 16:33:55 2016 +0300 +++ b/src/event/ngx_event_openssl.c Wed Feb 03 15:14:15 2016 -0800 @@ -3212,7 +3212,10 @@ goto failed; } - BIO_read(bio, s->data, len); + if (BIO_read(bio, s->data, len) != (int)len) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "BIO_read() failed"); + goto failed; + } BIO_free(bio); X509_free(cert); @@ -3235,22 +3238,41 @@ size_t len; ngx_uint_t i; ngx_str_t cert; + int prev_cr; if (ngx_ssl_get_raw_certificate(c, pool, &cert) != NGX_OK) { return NGX_ERROR; } + /* Strip off trailing whitespace (last newline) from the cert */ + while (cert.len > 0 && ( + cert.data[cert.len-1] == CR || + cert.data[cert.len-1] == LF || + cert.data[cert.len-1] == ' ')) { + --cert.len; + } + if (cert.len == 0) { s->len = 0; return NGX_OK; } - len = cert.len - 1; - - for (i = 0; i < cert.len - 1; i++) { + /* + * To make PEM text compatible as an HTTP header value, convert + * "\r\n" to "\r\n\t", and convert "\n" to "\r\n\t". Bare '\n' + * is rejected by some HTTP servers including node/express. + */ + prev_cr = 0; + len = 0; + for (i = 0; i < cert.len; i++) { if (cert.data[i] == LF) { - len++; + if (!prev_cr) { + len++; /* Insert CR before LF*/ + } + len++; /* Insert '\t' after LF */ } + len++; + prev_cr = (cert.data[i] == CR); } s->len = len; @@ -3261,11 +3283,18 @@ p = s->data; - for (i = 0; i < cert.len - 1; i++) { + prev_cr = 0; + for (i = 0; i < cert.len; i++) { + if (cert.data[i] == LF) { + if (!prev_cr) { + *p++ = CR; + } + *p++ = LF; + *p++ = '\t'; + } else { *p++ = cert.data[i]; - if (cert.data[i] == LF) { - *p++ = '\t'; - } + } + prev_cr = (cert.data[i] == CR); } return NGX_OK;
_______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
