Hello Jarno,
On Tue, May 06, 2014 at 01:36:44PM +0300, Jarno Huuskonen wrote:
> Hello,
>
> This is a patch (proposal) to include ssl_c_cert keyword to add client
> certificate (in pem format) to backend requests. This is useful for
> offloading ssl for applications that need access to client certificate
> (for example with something like tomcat sslvalve:
> http://tomcat.apache.org/tomcat-7.0-doc/api/org/apache/catalina/valves/SSLValve.html
> and
> http://grepcode.com/file/repo1.maven.org/maven2/org.apache.tomcat/tomcat-catalina/7.0.0/org/apache/catalina/valves/SSLValve.java
> )
Interesting, that was what we originally wanted to provide, until we
thought that by having every field extractable and settable into a
header, we would not need to provide the full certificate anymore.
> This patch should be compatible with apache/mod_ssl
> (RequestHeader set SSL_CLIENT_CERT "%{SSL_CLIENT_CERT}s")
> (newlines in the pem cert are replaced with space chars).
I'm wondering whether there is a risk that this same cert could be used
encoded differently. Eg, currently we have it in PEM and replace new lines
with spaces, but would it make sense sometimes for example to hash it and
just pass a hash or whatever ?
My question comes from the fact that we have sample converters and that
in my opinion, the conversion to the PEM format is very simple (eg:
add "-----BEGIN CERTIFICATE-----\n", encode in base64 then END).
Similarly we could have the PEM converter chose the delimiting character
to be used. And BTW we already have the base64 converter, but I don't
think it would provide any value here.
So basically the output format could be built using this string :
ssl_c_cert,pem(CERTIFICATE)
With :
- ssl_c_cert outputting raw binary
- pem(type) being the PEM encoder (prepend the BEGIN and append
the END line, encode in base64 each block of 48 bytes, and emit
a spacexs)
An optional "t" argument to the pem encoder would emit tabs instead
of spaces to be compatible with pound/nginx.
> (pound / nginx use different formatting for the cert:
> X-SSL-certificate: -----BEGIN CERTIFICATE-----
> \t...
> \t-----END CERTIFICATE-----).
>
> The code to replace newlines with spaces is not optimized. If there's a
> guarantee that openssl PEM_write_bio_X509 will always format the pem
> cert in same way(
> -----BEGIN CERTIFICATE-----
> MIIEoTCCA4mgAwIBAgIQIHCGEzfaVEFkF5d4JxstDTANBgkqhkiG9w0BAQUFADA2
> ....
> r8q5R89n4IPaS0DaE4I+/W15CPs/AUlkUh6vy2v+PY+WRlie6g==
> -----END CERTIFICATE-----) --> are the base64 encoded lines always 64chars
> (except the last line) ?
We really cannot rely on this I think, it sounds quite dangerous. Also,
the cost of translating new lines into spaces is low compared to computing
the cert output!
> --> the loop could be optimized to start from pos 27 (end of BEGIN line)
> and jump 65 chars until end of last line.
Well, at least the with pem converter described above you would get this
for free :-)
> Is this something that could be included in haproxy ?
Sure, I'm just trying to figure what would be the best way to do it so
that we don't have to modify it in 6 months :-)
> (The patch is against haproxy-ss-20140501).
The patch looks clean at least !
Thanks,
Willy
> -Jarno
> diff -ur haproxy-ss-20140501/src/ssl_sock.c
> haproxy-ss-20140501.new/src/ssl_sock.c
> --- haproxy-ss-20140501/src/ssl_sock.c 2014-04-30 23:31:11.000000000
> +0300
> +++ haproxy-ss-20140501.new/src/ssl_sock.c 2014-05-06 13:29:11.620045574
> +0300
> @@ -2387,6 +2387,74 @@
> return 1;
> }
>
> +/* str, returns the client certificate in pem format. \n chars are
> + replaced with <SPC> (like apache mod_ssl/mod_header). */
> +static int
> +smp_fetch_ssl_c_cert(struct proxy *px, struct session *l4, void *l7,
> unsigned int opt,
> + const struct arg *args, struct sample *smp, const
> char *kw)
> +{
> + int ret = 0;
> + int i;
> + X509 *crt;
> + BIO *bio;
> + int len;
> + struct chunk *smp_trash;
> + struct connection *conn;
> +
> + if (!l4)
> + return 0;
> +
> + conn = objt_conn(l4->si[0].end);
> + if (!conn || conn->xprt != &ssl_sock)
> + return 0;
> +
> + if (!(conn->flags & CO_FL_CONNECTED)) {
> + smp->flags |= SMP_F_MAY_CHANGE;
> + return 0;
> + }
> +
> + /* SSL_get_peer_certificate increase X509 * ref count */
> + crt = SSL_get_peer_certificate(conn->xprt_ctx);
> + if (!crt)
> + return 0;
> +
> + bio = BIO_new(BIO_s_mem());
> +
> + if (bio == NULL) {
> + X509_free(crt);
> + return 0;
> + }
> +
> + if (!PEM_write_bio_X509(bio, crt))
> + goto end;
> +
