Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Mail: added support for SSL client certificate

February 14, 2014 05:42AM
Hello.


2014-02-11 13:41 GMT+01:00 Maxim Dounin <mdounin@mdounin.ru>:

> Hello!
>
> On Mon, Feb 10, 2014 at 02:08:52PM +0100, Franck Levionnois wrote:
>
> > > > + b64_cert->len = ngx_base64_encoded_length(pem_cert.len);
> > > > + b64_cert->data = ngx_palloc(pool, b64_cert->len);
> > > > + if (b64_cert->data == NULL) {
> > > > + b64_cert->len = 0;
> > > > + return NGX_ERROR;
> > > > + }
> > > > + ngx_encode_base64(b64_cert, &pem_cert);
> > >
> > > Using a raw certificate escaped as other other Auth-* headers may
> > > be a better idea than inventing another method to pass things.
> > > Base64 encoding of base64 encoded data looks especially strange.
> > > :)
> > >
> >
> > Base64 encoding of the PEM certificate may looks strange, but it is done
> > for compatibility with other reverse proxy like F5 BigIp. It is also
> > possible to simply remove PEM header / footer and carriage returns (like
> > another reverse proxy)
>
> While compatibility with 3rd party code is a good thing, I don't
> think that it should be done at cost of consistency with other
> code.
>
> >
> > The function "ngx_ssl_get_certificate" is about to do the work, but it
> let
> > headers, and replaces carriage returns by tabulations. Modify this one to
> > remove the headers may have some consequences.
> > Although i would have preferred not to have the headers, i think i can do
> > with it, if you think this is better than adding a third function to get
> ssl
> > client certificate.
>
> The ngx_ssl_get_certificate() is for $ssl_client_cert variable in
> http[1], and it uses header continuation to make it possible to
> pass certificate to upstream servers. This aproach doesn't work
> very well as header continuation isn't really supported nowadays
> (in particular, by nginx itself) and deprecated by HTTPbis, so it
> probably needs revision. But I don't think it's relevant to this
> case, as we already have escaping applied to other Auth-* headers,
> and it should be trivial for auth script to unescape certificates
> as well.
>
> [1] http://nginx.org/en/docs/http/ngx_http_ssl_module.html#variables
>
>
Ok, it's trivial. I'll modify the patch to use the escape function.

[...]
>
> > > > + if (ngx_ssl_get_issuer_dn(s->connection, pool,
> > > > + &client_issuer) != NGX_OK) {
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + if (ngx_ssl_get_serial_number(s->connection, pool,
> > > > + &client_serial) != NGX_OK) {
> > > > + return NULL;
> > > > + }
> > >
> > > One of questions left open during Sven Peter's patch review was
> > > whether subject/issuer can contain CR/LF and require escaping.
> > > The code here suggests they can't. I would like to know if it was
> > > actually checked.
> > >
> > > It would be also cool to get Sven's review of the code (and/or his
> > > own patch improved instead if he don't happy with one from
> > > Franck). Added Sven to Cc.
> > >
> > >
> > Subject and Issuer DN may contains special chars but "X509_NAME_oneline"
> > function escapes every chars outside " " -> "~" range (in ASCII table).
> > This is the function used by "ngx_ssl_get_subject_dn" and
> > "ngx_ssl_get_issuer_dn" to get the DN
> > This is a sample output from the function of DN with carriage returns :
> > Issuer: /C=FR/ST=Some-State \x0D\x0A\x0D\x0A\x0D\x0Atest/
> > L=Paris/OU=An\x0D\x0Aign/CN=Autorite de certification
> >
> > Even if i've never seen Distinguished names with carriage returns, i
> > haven't seen such limitation in RFC 3280 / X500.
> > RFC 2253 shows a sample of distinguished name with carriage return.
>
> So escaping or CR/LF is already done by X509_NAME_oneline() and
> there is no need for additional one, right?
>
> Yes, it's right.

> --
> Maxim Dounin
> http://nginx.org/
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>

Kind regards.
Franck Levionnois.
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Mail: added support for SSL client certificate

Filipe da Silva 1327 January 25, 2014 03:48AM

Re: [PATCH] Mail: added support for SSL client certificate

Maxim Dounin 469 January 28, 2014 09:20AM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 579 January 28, 2014 12:42PM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 583 February 10, 2014 08:10AM

Re: [PATCH] Mail: added support for SSL client certificate

Maxim Dounin 513 February 11, 2014 07:42AM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 565 February 14, 2014 05:42AM

[PATCH] Mail: added support for SSL client certificate

Franck Levionnois 512 February 21, 2014 05:12AM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 492 February 21, 2014 06:48AM

[PATCH] Mail: added support for SSL client certificate

Franck Levionnois 644 February 21, 2014 06:50AM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 459 March 07, 2014 03:42AM

Re: [PATCH] Mail: added support for SSL client certificate

Maxim Dounin 459 March 07, 2014 06:32AM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 475 March 18, 2014 01:42PM

Re: [PATCH] Mail: added support for SSL client certificate

Franck Levionnois 475 April 14, 2014 03:34AM

Re: [PATCH] Mail: added support for SSL client certificate

Filipe Da Silva 478 June 16, 2014 04:24PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 103
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready