Re: patch proposal: ssl_c_cert

2014-05-22 Thread Jarno Huuskonen
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

2014-05-12 Thread Jarno Huuskonen
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

2014-05-09 Thread Willy Tarreau
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

2014-05-06 Thread Jarno Huuskonen
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 },