[PATCH] Mail: added support for SSL client certificate

[Maxim Dounin]

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

[...]

> > > +        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?

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