Proposed changeset to fix client cert from ngx_ssl_get_certificate passed as HTTP header value

[Sam McKelvie]

> Hello!
>
> On Wed, Feb 03, 2016 at 03:38:13PM -0800, Sam McKelvie wrote:
>
>> 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.
>
> IMHO, header line folding is wrong enough to don't bother with
> trying to fix this.  It doesn't work in way too many cases
> including with nginx itself, and it is deprecated by RFC7230.
>
> Much better approach would be to switch to something different -
> may be just properly urlencoded $ssl_client_raw_cert, or plain
> base64 without any newlines, or whatever.
>
I tend to agree. However, nix_ssl_get_certificate() is already folding
the certificate for HTTP headers by inserting '\t' at the beginning of
each line, so that seems to be its primary purpose. I can’t change
that function to return anything that is not itself a valid PEM
certificate, or it would certainly break any existing applications
that parse the results. So unless you think it is OK to tweak it to
insert '\r' as well,  I think you are saying this function is just
permanently unusable for the purposes of passing a certificate in a
header.

Would you be more amenable to defining a new "$ssl_base64_client_cert"
or “$ssl_escaped_client_cert” variable that is a properly encoded PEM
string? Would you prefer base64 (which would greatly increase the
size) or a quoted-string for with backslashes to escape control
characters? I would be willing to submit such a changeset…

Sam
> --
> Maxim Dounin
> http://nginx.org/