Re: patch proposal: ssl_c_cert
Hello, On Fri, May 09, Willy Tarreau wrote: So basically the output format could be built using this string : ssl_c_cert,pem(CERTIFICATE) What kind of argument the CERTIFICATE should be ? ARGT_STR / ARGT_UINT ? 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) Here's a quick proof of concept patch (should apply to haproxy-ss-20140516 and latest git): - ssl_c_cert (smp_fetch_ssl_c_cert) function that gets client cert in der(raw) format (with openssl i2d_X509). (Maybe ssl_c_cert could take optional parameter to retrieve any certificate from client certificate chain (with SSL_get_peer_cert_chain) Something like: ssl_c_cert(0) or ssl_c_cert - get client cert and ssl_c_cert(1) - get first intermediate cert in chain. (like apache/mod_ssl SSL_CLIENT_CERT_CHAINn) - sample_conv_bin2pem function that base64 encodes the der/raw cert to pem. (This is still ugly... the base64 encoding loop can certainly use some cleanup). Could somebody take a look if the patch makes any sense ? Especially if I'm using the arguments to sample_conv_bin2pem correctly and that I'm calculating buffer sizes correctly - no over/underflows. With the patch it's possible to add for example SSL_CLIENT_CERT header: # Apache/mod_ssl style where pem cert \n is replaced with space http-request set-header SSL_CLIENT_CERT %[ssl_c_cert,pem(\ ,\ )] # Pound --enable-cert1l: pem cert with whitespace removed http-request set-header SSL_CLIENT_CERT %[ssl_c_cert,pem] # Pound / nginx: http-request set-header SSL_CLIENT_CERT %[ssl_c_cert,pem(\r\n\t)] -- pem(\r\n\t) doesn't quite work, because haproxy urlencodes \r\n to %0D%0A. Is it possible to pass the output w/out urlencoding ? -Jarno PS. Minimal config for testing: frontend https_8443 bind *:8443 name test ssl verify optional crt /pathto/server.pem ca-file /pathto/ca.pem crt-ignore-err 18,19,20,27,21 mode http http-request set-header SSL_CIPHER %{+Q}[ssl_fc_cipher] http-request set-header SSL_CIPHER_USEKEYSIZE %{+Q}[ssl_fc_alg_keysize] http-request set-header SSL_CLIENT_CERT %[ssl_c_cert,pem(\ ,\ )] #http-request set-header SSL_CLIENT_CERT %[ssl_c_cert,pem(\r\n\t)] default_backend BE_idp_tomcat backend BE_idp_tomcat mode http server netcat 127.0.0.1:8080 diff --git a/src/sample.c b/src/sample.c index a1e8012..ae8837f 100644 --- a/src/sample.c +++ b/src/sample.c @@ -1208,6 +1208,104 @@ static int sample_conv_bin2hex(const struct arg *arg_p, struct sample *smp) return 1; } +static int sample_conv_bin2pem(const struct arg *arg_p, struct sample *smp) +{ + struct chunk *trash = get_trash_chunk(); + const char *begin_crt = -BEGIN CERTIFICATE-; + const char *end_crt = -END CERTIFICATE-; + int b64_len; + int pem_len; + int src_incr = 0; + int dst_incr; + int i; + + trash-len = 0; + + /* Calc. output length: (base64 calc. from base64.c): +* pem_len = strlen(begin_crt) + arg[0].data.str.len + base64_len +* base64_len/64 * arg[0].data.str.len + +* if (base64_len%64) { arg[0].data.str.len } + +* strlen(end_crt) + arg[1].data.str.len */ + b64_len = ((smp-data.str.len + 2) / 3) * 4; + + pem_len = b64_len; + pem_len += strlen(begin_crt) + strlen(end_crt); + + if (arg_p arg_p[0].type == ARGT_STR) { + if (b64_len % 64) { + pem_len += arg_p[0].data.str.len; + } + pem_len += (b64_len / 64) * arg_p[0].data.str.len; + + pem_len += arg_p[0].data.str.len; + + /* User wants string after END CERT... line */ + if (arg_p[1].type == ARGT_STR) + pem_len += arg_p[1].data.str.len; + } + + /* fail if output won't fit in chunk */ + if (pem_len = trash-size) + return 0; + + memcpy(trash-str, begin_crt, strlen(begin_crt)); + dst_incr = strlen(begin_crt); + if (arg_p arg_p[0].type == ARGT_STR) { + memcpy(trash-str+dst_incr, arg_p[0].data.str.str, arg_p[0].data.str.len); + dst_incr += arg_p[0].data.str.len; + } + + /* base64 encode input in 48bytes chunks = 64bytes base64 encoded line */ + for (i=0; i (smp-data.str.len / 48); i++) { + int tmp_len; + tmp_len = a2base64(smp-data.str.str + src_incr, 48, trash-str+dst_incr, trash-size - dst_incr); + /* abort if a2base64 returns error */ + if (tmp_len 0) + return 0; + + dst_incr += tmp_len; + /* Append user supplied string to each line */ + if (arg_p arg_p[0].type == ARGT_STR) { +
Re: patch proposal: ssl_c_cert
Hi Willy, On Fri, May 09, Willy Tarreau wrote: 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 ? I don't know about hashes, but googling shows that there seems to be quite few slightly different formats (some examples below). 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) This sounds reasonable. With this approach it should be fairly easy to support other SSL_CLIENT_CERT header formats: - just plain base64: ssl_c_cert,base64 - base64(pem): ssl_c_cert,pem(CERTIFICATE,\n),base64 (https://devcentral.f5.com/questions/irule-to-pass-client-ssl-cert-to-the-application-server-pool-member) - pem encoded with newlines removed (pound compiled w/--enable-cert1l): ssl_c_cert,pem(CERTIFICATE) ? - nginx seems to be changing how they output the cert (http://forum.nginx.org/read.php?29,249804,249848): ssl_c_cert,pem(CERTIFICATE,\r\n\t) An optional t argument to the pem encoder would emit tabs instead of spaces to be compatible with pound/nginx. Apache adds space(' ') after -END CERTIFICATE- line, but I think nginx/pound compat. output should not add anything after -END CERTIFICATE- ? So should the pem converter take third argument: end marker? - so apache compat: ssl_c_cert,pem(CERTIFICATE, , ) - nginx/pound: ssl_c_cert,pem(CERTIFICATE, \r\n\t) I might have some time next week or so to try to do this. Can you give some pointers (for example which functions I could use to look for examples) ? I assume that ssl_c_cert would fetch function (similar to my earlier patch, but returning the cert in raw binary) and pem would be converter (like sample_conv_bin2base64) ? -Jarno (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 :-)
Re: patch proposal: ssl_c_cert
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.c2014-04-30 23:31:11.0 +0300 +++ haproxy-ss-20140501.new/src/ssl_sock.c2014-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; +
patch proposal: ssl_c_cert
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 ) 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). (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) ? -- the loop could be optimized to start from pos 27 (end of BEGIN line) and jump 65 chars until end of last line. Is this something that could be included in haproxy ? (The patch is against haproxy-ss-20140501). -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.0 +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; + + len = BIO_pending(bio); + smp_trash = get_trash_chunk(); + + if ((len 0) || (len = smp_trash-size)) + goto end; + + if ((len = BIO_read(bio, smp_trash-str, len)) = 0) + goto end; + + /* Tomcat SSLValve expects spaces instead of newlines. */ + for (i=0; i len; i++) { + if (smp_trash-str[i] == '\n') + smp_trash-str[i] = ' '; + } + + smp-data.str.str = smp_trash-str; + smp-type = SMP_T_STR; + smp-data.str.len = len; + ret = 1; + +end: + X509_free(crt); + BIO_free(bio); + return ret; +} + + /* boolean, returns true if front conn. transport layer is SSL. * This function is also usable on backend conn if the fetch keyword 5th * char is 'b'. @@ -3407,6 +3475,7 @@ { ssl_bc_session_id, smp_fetch_ssl_fc_session_id, 0, NULL,SMP_T_BIN, SMP_USE_L5SRV }, { ssl_c_ca_err, smp_fetch_ssl_c_ca_err, 0, NULL,SMP_T_UINT, SMP_USE_L5CLI }, { ssl_c_ca_err_depth, smp_fetch_ssl_c_ca_err_depth, 0, NULL,SMP_T_UINT, SMP_USE_L5CLI }, + { ssl_c_cert, smp_fetch_ssl_c_cert, 0, NULL,SMP_T_STR, SMP_USE_L5CLI }, { ssl_c_err, smp_fetch_ssl_c_err, 0, NULL,SMP_T_UINT, SMP_USE_L5CLI }, { ssl_c_i_dn, smp_fetch_ssl_x_i_dn, ARG2(0,STR,SINT),NULL,SMP_T_STR, SMP_USE_L5CLI }, { ssl_c_key_alg, smp_fetch_ssl_x_key_alg, 0, NULL,SMP_T_STR, SMP_USE_L5CLI },