subject:"Re\: Contribution for HAProxy\: Peer Cipher based SSL CTX switching"

Hey Willy,

On 12/8/15, 5:27 PM, "Willy Tarreau"  wrote:
>
>In my opinion, these suffixes should be used only after the real cert
>file name. So when you load "foobar.ecdsa", you should only consider
>"foobar.ecdsa.ocsp" and so on. And from what I remember, on the CLI
>we mention the cert name when feeding an OCSP entry so that should
>continue to work perfectly.

I agree, the limitation here is that the way HAProxy is current designed
only allows for 1 OCSP staple per SSL_CTX. This will have to change to
multiple staples for SSL_CTX¹s with multiple certs.
>
>I do think so. We'll just have to remerge 4, 5 and 6 into their respective
>patches (2 apparently) and we're good to go. If Emeric doesn't raise any
>objection (apparently you addressed his concerns) I can merge all that
>myself.
>If you prefer to remerge the patches above yourself, no problem for me.

I can remerge everything into 3 patches, it will be cleaner that way. I¹ll
send them out tomorrow.

Thanks!
-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-08 Thread Willy Tarreau

Hi Dave,

On Tue, Dec 08, 2015 at 09:12:58PM +, Dave Zhu (yanbzhu) wrote:
> There are also 2 issues here.
> 
> 
>   1.  Loading certs from a directory doesn't process multiple certs at the
>   same time. This I can fix with another patch to add that functionality

I didn't think about this one indeed.

>   2.  .issuer, .ocsp and .sctl only apply to a single cert, not multiple
>   certs. This is tricker, since we'd have to load multiple OCSP responses for
>   stapling in the case of multiple certs, which would mean that we would have
>   to set the OCSP response based on which certificate is presented. I could
>   look into this as well, since it shouldn't be impossible to do given
>   current HAProxy infrastructure. However, I would prefer that the
>   functionality as it is today makes it into the code base. Similar with
>   SCTL, although I have zero experience in that matter and would need
>   guidance.

In my opinion, these suffixes should be used only after the real cert
file name. So when you load "foobar.ecdsa", you should only consider
"foobar.ecdsa.ocsp" and so on. And from what I remember, on the CLI
we mention the cert name when feeding an OCSP entry so that should
continue to work perfectly.

> I'll look into #1 and the ocsp portion of #2. I'll let you know when I have 
> updates.
> 
> In the mean time, is the code and functionality as of today acceptable? Could
> the feature be merged as is, with features added in the future?

I do think so. We'll just have to remerge 4, 5 and 6 into their respective
patches (2 apparently) and we're good to go. If Emeric doesn't raise any
objection (apparently you addressed his concerns) I can merge all that myself.
If you prefer to remerge the patches above yourself, no problem for me.

Thanks !
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Bryan,

Now, about using the crt bind option with a directory of certs
https://cbonte.github.io/haproxy-dconv/configuration-1.6.html#crt (Bind options)

How should that work, especially if there are .ocsp and .issuer data in the crt 
directory? Currently, the ECDSA certificate seems to always be used even for 
non-ECC capable clients but I suspect that's due to the .ecdsa cert being 
loaded first and your patches do not cover that use case yet.

In this case, it would work the same as it does today. So the .rsa cert and the 
.ecdsa cert would get loaded as separate certificates, and the ECDSA would get 
added to the SNI tree first due to alphabetical loading as you postulated.

There are also 2 issues here.


  1.  Loading certs from a directory doesn't process multiple certs at the same 
time. This I can fix with another patch to add that functionality
  2.  .issuer, .ocsp and .sctl only apply to a single cert, not multiple certs. 
This is tricker, since we'd have to load multiple OCSP responses for stapling 
in the case of multiple certs, which would mean that we would have to set the 
OCSP response based on which certificate is presented. I could look into this 
as well, since it shouldn't be impossible to do given current HAProxy 
infrastructure. However, I would prefer that the functionality as it is today 
makes it into the code base. Similar with SCTL, although I have zero experience 
in that matter and would need guidance.

I'll look into #1 and the ocsp portion of #2. I'll let you know when I have 
updates.

In the mean time, is the code and functionality as of today acceptable? Could 
the feature be merged as is, with features added in the future?

-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Tuesday, December 8, 2015 at 4:02 PM
To: Yanbo Zhu mailto:yanb...@cisco.com>>
Cc: Bryan Talbot mailto:bryan.tal...@ijji.com>>, Willy 
Tarreau mailto:w...@1wt.eu>>, Olivier Doucet 
mailto:webmas...@ajeux.com>>, Emeric Brun 
mailto:eb...@haproxy.com>>, Lukas Tribus 
mailto:luky...@hotmail.com>>, Remi Gacogne 
mailto:rgaco...@coredump.fr>>, Nenad Merdanovic 
mailto:ni...@nimzo.info>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On Tue, Dec 8, 2015 at 11:18 AM, Dave Zhu (yanbzhu) 
mailto:yanb...@cisco.com>> wrote:
Hey Bryan,

I believe I have gotten to the bottom of the behavior that you are seeing:


  1.  0.9.8 client cannot connect to dual cert port: This was a bug on my part. 
I neglected to set a DHE keys for the SSL_CTX with multiple certs. I've 
attached another set of patches (1-5 are identical, 6 is new) that fixes this.

yep, patch 6 fixes this problem for me.



  1.  ECC capable client does not use ECC cipher: I believe this is due to your 
test configuration. Openssl prefers RSA ciphers by default, and so if you don't 
specify an ECC cipher first, it will always pick an RSA cipher. Your test uses 
"./openssl-1.0.2e/apps/openssl s_client -connect 127.0.0.1:8443" as the 
command, which will use the default cipher list. Try specifying an ECC cipher 
as the first cipher and it should work.

Of course, I should have realized that too. I've updated the bind ciphers to 
prioritize ECDSA over RSA and that fixes the issue. So the basic tests I 
defined before are all passing now but only when the crt line specifies a "pem" 
file that doesn't exist and .ecdsa / .rsa files are loaded from that base.


Now, about using the crt bind option with a directory of certs
https://cbonte.github.io/haproxy-dconv/configuration-1.6.html#crt (Bind options)

How should that work, especially if there are .ocsp and .issuer data in the crt 
directory? Currently, the ECDSA certificate seems to always be used even for 
non-ECC capable clients but I suspect that's due to the .ecdsa cert being 
loaded first and your patches do not cover that use case yet.



-Bryan

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-08 Thread Bryan Talbot

On Tue, Dec 8, 2015 at 11:18 AM, Dave Zhu (yanbzhu) 
wrote:

> Hey Bryan,
>
> I believe I have gotten to the bottom of the behavior that you are seeing:
>
>
>1. 0.9.8 client cannot connect to dual cert port: This was a bug on my
>part. I neglected to set a DHE keys for the SSL_CTX with multiple certs.
>I’ve attached another set of patches (1-5 are identical, 6 is new) that
>fixes this.
>
>
yep, patch 6 fixes this problem for me.



>
>1. ECC capable client does not use ECC cipher: I believe this is due
>to your test configuration. Openssl prefers RSA ciphers by default, and so
>if you don’t specify an ECC cipher first, it will always pick an RSA
>cipher. Your test uses "./openssl-1.0.2e/apps/openssl s_client -connect
>127.0.0.1:8443” as the command, which will use the default cipher
>list. Try specifying an ECC cipher as the first cipher and it should work.
>
>
Of course, I should have realized that too. I've updated the bind ciphers
to prioritize ECDSA over RSA and that fixes the issue. So the basic tests I
defined before are all passing now but only when the crt line specifies a
"pem" file that doesn't exist and .ecdsa / .rsa files are loaded from that
base.


Now, about using the crt bind option with a directory of certs
https://cbonte.github.io/haproxy-dconv/configuration-1.6.html#crt (Bind
options)

How should that work, especially if there are .ocsp and .issuer data in the
crt directory? Currently, the ECDSA certificate seems to always be used
even for non-ECC capable clients but I suspect that's due to the .ecdsa
cert being loaded first and your patches do not cover that use case yet.



-Bryan

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Bryan,

I believe I have gotten to the bottom of the behavior that you are seeing:

  1.  0.9.8 client cannot connect to dual cert port: This was a bug on my part. 
I neglected to set a DHE keys for the SSL_CTX with multiple certs. I’ve 
attached another set of patches (1-5 are identical, 6 is new) that fixes this.
  2.  ECC capable client does not use ECC cipher: I believe this is due to your 
test configuration. Openssl prefers RSA ciphers by default, and so if you don’t 
specify an ECC cipher first, it will always pick an RSA cipher. Your test uses 
"./openssl-1.0.2e/apps/openssl s_client -connect 127.0.0.1:8443” as the 
command, which will use the default cipher list. Try specifying an ECC cipher 
as the first cipher and it should work.

Please take a look.

-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Monday, December 7, 2015 at 5:58 PM
To: Willy Tarreau mailto:w...@1wt.eu>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, Olivier Doucet 
mailto:webmas...@ajeux.com>>, Emeric Brun 
mailto:eb...@haproxy.com>>, Lukas Tribus 
mailto:luky...@hotmail.com>>, Remi Gacogne 
mailto:rgaco...@coredump.fr>>, Nenad Merdanovic 
mailto:ni...@nimzo.info>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>, Bryan Talbot 
mailto:bryan.tal...@ijji.com>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Glad you were able to get to the bottom of the crash.

With the newest 5 patches, I'm still not seeing the behavior I am expecting. To 
make my testing and expectations hopefully more clear, I've pushed them to 
github (https://github.com/btalbot/dual-stack-test)  From a laptop with Vagrant 
installed, it should be a simple process to provision a host for testing and 
run the test script.

What I am expecting is that OpenSSL 0.9.8 client will be able to connect to an 
haproxy port that is bound to both ECDSA and RSA certificates. This doesn't 
work for me and the connection fails the SSL handshake.

I'm also expecting that a newer OpenSSL which support ECC will connect AND 
negotiate and use the 256 bit ECDSA certificate and not the RSA cert. My tests 
always show the ECC capable client still getting the RSA certificate.

-Bryan

On Mon, Dec 7, 2015 at 1:44 PM, Willy Tarreau mailto:w...@1wt.eu>> 
wrote:
On Mon, Dec 07, 2015 at 08:48:53PM +, Dave Zhu (yanbzhu) wrote:
> Hey Willy
>
> On 12/7/15, 3:11 PM, "Willy Tarreau" mailto:w...@1wt.eu>> wrote:
> >
> >Yep, thanks for the pointer. So indeed gcc's inline version of strncpy
> >*is*
> >bogus. strncpy() has no right to guess the destination size.
> >
> >I suspect that if you just do this it would work (prefix the array with
> >'&'
> >and use [0] :
> >
> >   strncpy((char *)&s_kt->name.key[0], trash.str, i);
> >
> >Thanks,
> >Willy
>
> You would be correct in this guess :)
>
> So what零 the preference? Should I change it to use this weird version of
> strcpy, or change it to memcpy?

I'd prefer the memcpy() anyway. Please keep your comment and add the
link to gcc's bugzilla so that nobody is tempted to change this later
for any reason, and please mention that it's the inlined version of
strncpy() which refuses to write into a char[0].

You have my full support if you want to add some dirty words there to
express your feelings about the compiler which dies on valid C code...

Thanks,
Willy

0001-MINOR-ssl-Added-cert_key_and_chain-struct.patch
Description: 0001-MINOR-ssl-Added-cert_key_and_chain-struct.patch

0002-MEDIUM-ssl-Added-support-for-creating-SSL_CTX-with-m.patch
Description: 0002-MEDIUM-ssl-Added-support-for-creating-SSL_CTX-with-m.patch

0003-MINOR-ssl-Added-multi-cert-support-for-crt-list-conf.patch
Description: 0003-MINOR-ssl-Added-multi-cert-support-for-crt-list-conf.patch

0004-BUG-MINOR-ssl-Fixed-code-that-crashed-under-optimiza.patch
Description: 0004-BUG-MINOR-ssl-Fixed-code-that-crashed-under-optimiza.patch

0005-MINOR-ssl-Clean-up-unused-code-fixed-spelling-error.patch
Description: 0005-MINOR-ssl-Clean-up-unused-code-fixed-spelling-error.patch

0006-BUG-MINOR-ssl-Fixed-error-where-multi-certs-didn-t-s.patch
Description: 0006-BUG-MINOR-ssl-Fixed-error-where-multi-certs-didn-t-s.patch

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Thanks for posting this.

I’m looking into it.

-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Monday, December 7, 2015 at 5:58 PM
To: Willy Tarreau mailto:w...@1wt.eu>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, Olivier Doucet 
mailto:webmas...@ajeux.com>>, Emeric Brun 
mailto:eb...@haproxy.com>>, Lukas Tribus 
mailto:luky...@hotmail.com>>, Remi Gacogne 
mailto:rgaco...@coredump.fr>>, Nenad Merdanovic 
mailto:ni...@nimzo.info>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>, Bryan Talbot 
mailto:bryan.tal...@ijji.com>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Glad you were able to get to the bottom of the crash.

With the newest 5 patches, I'm still not seeing the behavior I am expecting. To 
make my testing and expectations hopefully more clear, I've pushed them to 
github (https://github.com/btalbot/dual-stack-test)  From a laptop with Vagrant 
installed, it should be a simple process to provision a host for testing and 
run the test script.

What I am expecting is that OpenSSL 0.9.8 client will be able to connect to an 
haproxy port that is bound to both ECDSA and RSA certificates. This doesn't 
work for me and the connection fails the SSL handshake.

I'm also expecting that a newer OpenSSL which support ECC will connect AND 
negotiate and use the 256 bit ECDSA certificate and not the RSA cert. My tests 
always show the ECC capable client still getting the RSA certificate.

-Bryan

On Mon, Dec 7, 2015 at 1:44 PM, Willy Tarreau mailto:w...@1wt.eu>> 
wrote:
On Mon, Dec 07, 2015 at 08:48:53PM +, Dave Zhu (yanbzhu) wrote:
> Hey Willy
>
> On 12/7/15, 3:11 PM, "Willy Tarreau" mailto:w...@1wt.eu>> wrote:
> >
> >Yep, thanks for the pointer. So indeed gcc's inline version of strncpy
> >*is*
> >bogus. strncpy() has no right to guess the destination size.
> >
> >I suspect that if you just do this it would work (prefix the array with
> >'&'
> >and use [0] :
> >
> >   strncpy((char *)&s_kt->name.key[0], trash.str, i);
> >
> >Thanks,
> >Willy
>
> You would be correct in this guess :)
>
> So what零 the preference? Should I change it to use this weird version of
> strcpy, or change it to memcpy?

I'd prefer the memcpy() anyway. Please keep your comment and add the
link to gcc's bugzilla so that nobody is tempted to change this later
for any reason, and please mention that it's the inlined version of
strncpy() which refuses to write into a char[0].

You have my full support if you want to add some dirty words there to
express your feelings about the compiler which dies on valid C code...

Thanks,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-07 Thread Bryan Talbot

Glad you were able to get to the bottom of the crash.

With the newest 5 patches, I'm still not seeing the behavior I am
expecting. To make my testing and expectations hopefully more clear, I've
pushed them to github (https://github.com/btalbot/dual-stack-test)  From a
laptop with Vagrant installed, it should be a simple process to provision a
host for testing and run the test script.

What I am expecting is that OpenSSL 0.9.8 client will be able to connect to
an haproxy port that is bound to both ECDSA and RSA certificates. This
doesn't work for me and the connection fails the SSL handshake.

I'm also expecting that a newer OpenSSL which support ECC will connect AND
negotiate and use the 256 bit ECDSA certificate and not the RSA cert. My
tests always show the ECC capable client still getting the RSA certificate.

-Bryan

On Mon, Dec 7, 2015 at 1:44 PM, Willy Tarreau  wrote:

> On Mon, Dec 07, 2015 at 08:48:53PM +, Dave Zhu (yanbzhu) wrote:
> > Hey Willy
> >
> > On 12/7/15, 3:11 PM, "Willy Tarreau"  wrote:
> > >
> > >Yep, thanks for the pointer. So indeed gcc's inline version of strncpy
> > >*is*
> > >bogus. strncpy() has no right to guess the destination size.
> > >
> > >I suspect that if you just do this it would work (prefix the array with
> > >'&'
> > >and use [0] :
> > >
> > >   strncpy((char *)&s_kt->name.key[0], trash.str, i);
> > >
> > >Thanks,
> > >Willy
> >
> > You would be correct in this guess :)
> >
> > So what零 the preference? Should I change it to use this weird version of
> > strcpy, or change it to memcpy?
>
> I'd prefer the memcpy() anyway. Please keep your comment and add the
> link to gcc's bugzilla so that nobody is tempted to change this later
> for any reason, and please mention that it's the inlined version of
> strncpy() which refuses to write into a char[0].
>
> You have my full support if you want to add some dirty words there to
> express your feelings about the compiler which dies on valid C code...
>
> Thanks,
> Willy
>
>

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-07 Thread Willy Tarreau

On Mon, Dec 07, 2015 at 08:48:53PM +, Dave Zhu (yanbzhu) wrote:
> Hey Willy
> 
> On 12/7/15, 3:11 PM, "Willy Tarreau"  wrote:
> >
> >Yep, thanks for the pointer. So indeed gcc's inline version of strncpy
> >*is*
> >bogus. strncpy() has no right to guess the destination size.
> >
> >I suspect that if you just do this it would work (prefix the array with
> >'&'
> >and use [0] :
> >
> >   strncpy((char *)&s_kt->name.key[0], trash.str, i);
> >
> >Thanks,
> >Willy
> 
> You would be correct in this guess :)
> 
> So what¹s the preference? Should I change it to use this weird version of
> strcpy, or change it to memcpy?

I'd prefer the memcpy() anyway. Please keep your comment and add the
link to gcc's bugzilla so that nobody is tempted to change this later
for any reason, and please mention that it's the inlined version of
strncpy() which refuses to write into a char[0].

You have my full support if you want to add some dirty words there to
express your feelings about the compiler which dies on valid C code...

Thanks,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Willy

On 12/7/15, 3:11 PM, "Willy Tarreau"  wrote:
>
>Yep, thanks for the pointer. So indeed gcc's inline version of strncpy
>*is*
>bogus. strncpy() has no right to guess the destination size.
>
>I suspect that if you just do this it would work (prefix the array with
>'&'
>and use [0] :
>
>   strncpy((char *)&s_kt->name.key[0], trash.str, i);
>
>Thanks,
>Willy

You would be correct in this guess :)

So what¹s the preference? Should I change it to use this weird version of
strcpy, or change it to memcpy?

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-07 Thread Willy Tarreau

On Mon, Dec 07, 2015 at 08:04:30PM +, Dave Zhu (yanbzhu) wrote:
> One more thing :)
> 
> Out of curiosity, I changed the code as specified in that bugzilla from:
> 
> strncpy((char *)s_kt->name.key, trash.str, i);
> 
> To
> 
>   node = &s_kt->name;
> strncpy((char *)node->key, trash.str, i);
> 
> And the code ran without an issue. I believe that this is the issue that
> Bryan first saw, and that there isn¹t some malicious underlying memory
> corruption that¹s happening here.

Yep, thanks for the pointer. So indeed gcc's inline version of strncpy *is*
bogus. strncpy() has no right to guess the destination size.

I suspect that if you just do this it would work (prefix the array with '&'
and use [0] :

   strncpy((char *)&s_kt->name.key[0], trash.str, i);

Thanks,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

One more thing :)

Out of curiosity, I changed the code as specified in that bugzilla from:

strncpy((char *)s_kt->name.key, trash.str, i);

To

  node = &s_kt->name;
strncpy((char *)node->key, trash.str, i);

And the code ran without an issue. I believe that this is the issue that
Bryan first saw, and that there isn¹t some malicious underlying memory
corruption that¹s happening here.

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Willy,

On 12/7/15, 2:22 PM, "Willy Tarreau"  wrote:
>I'm sorry but this is not acceptable. We're hiding a deeper bug somewhere.
>Strncpy() is used at other places in the code (a few I agree) and is used
>by *a lot* of programs, it's a basic building block. If it's broken in one
>version of gcc, it will quickly be fixed, and/or we may have to deploy
>workarounds for other places as well. Instead I guess that strncpy() is
>detecting a bug that memcpy() doesn't detect so until we find the real
>root cause of all of this with a proof that strncpy() contains the real
>bug and why it doesn't affect other places, we must not hide it because
>I'm pretty sure it will strike back later. Or at least I'd like to see
>a pointer to the corresponding bugzilla entry in gcc's bugtracker. I
>could imagine one reason being that the sni_keytype would be packed
>(for what reason ?) and that the string pointer would start unaligned
>and would cause some of strncpy()'s vector optims to fail. But that
>sounds extremely strange. By experience when a program dies from such
>a small change, it's a latent memory dereference that gets optimized
>differently and which manifests itself via memory manipulation functions.
>Note that I'm not defending gcc, you might have seen a number of angry
>comments from me against it in the code. I just want to be sure we're
>addressing the real issue instead of hiding it.

I completely understand where you¹re coming from so lets try to fix this
as best as we can. So I followed this to the gcc bugzilla as you suggested
and voila:

bogus buffer overflow warning and abort on flexible array member in a
child object
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60792


Which is exactly what I¹m doing. This isn¹t a bug though, just a different
use pattern than what GCC allows for. In this case, the suggestion was to
use memcpy. I feel that my patch does exactly what is suggested in that
thread.


>
>(...)
>
>I'm seeing a buffer overflow in the code prior to the bug seeing the
>crash,
>though its unlikely to trigger but it's here :
>
>   for (i = 0; i < trash.size; i++) {
> 
>   if (!str[i])
> 
>   break;
> 
>   trash.str[i] = tolower(str[i]);
> 
>   }  
> 
>   trash.str[i] = 0;
> 
>
>As you can see, if you don't find a 0 in the trash, it will overwrite the
>first character not part of the trash buffer.

This pattern I copied from other code in HAProxy, so I thought it would be
acceptable. Also, I¹m not sure what you mean by "don't find a 0 in the
trash". It¹s looking through str for the null terminator, which came from
reading the CN/SAN.


>
>(...)
>
>@@ -1808,7 +1808,10 @@ static void
>ssl_sock_populate_sni_keytypes_hplr(const char *str, struct eb_root
>   if (!node) {
>   /* CN not found in tree */
>   s_kt = malloc(sizeof(struct sni_keytype) + i + 1);
>-  strncpy((char *)s_kt->name.key, trash.str, i);
>+  /* Using memcpy here instead of strncpy.
>+   * strncpy will cause sig_abrt errors under certain version of 
>gcc
>with -O2
>+   */
>+  memcpy(s_kt->name.key, trash.str, i);
>   s_kt->name.key[i] = 0;
>   s_kt->keytypes = 0;
>   ebst_insert(sni_keytypes, &s_kt->name);
>
>It might be useful to display s_kt, s_kt->name.key, sizeof(struct
>sni_keytype)
>and i at this place to understand what upsets gcc/strncpy(). If we have to
>switch to memcpy() (which I prefer in general), you can remove the manual
>addition of the trailing 0 by adding 1 to the size of the memcpy() since
>it's guaranteed to cover the zero.

Given the above, did you still want me to remove the code to add the
trailing 0?

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-07 Thread Willy Tarreau

Hi Dave,

On Mon, Dec 07, 2015 at 06:49:54PM +, Dave Zhu (yanbzhu) wrote:
> Bryan found an interesting bug in the code, which I??ve root caused to an
> optimization bug(?)/eccentricity in gcc 4.8.4.

I'm sorry but this is not acceptable. We're hiding a deeper bug somewhere.
Strncpy() is used at other places in the code (a few I agree) and is used
by *a lot* of programs, it's a basic building block. If it's broken in one
version of gcc, it will quickly be fixed, and/or we may have to deploy
workarounds for other places as well. Instead I guess that strncpy() is
detecting a bug that memcpy() doesn't detect so until we find the real
root cause of all of this with a proof that strncpy() contains the real
bug and why it doesn't affect other places, we must not hide it because
I'm pretty sure it will strike back later. Or at least I'd like to see
a pointer to the corresponding bugzilla entry in gcc's bugtracker. I
could imagine one reason being that the sni_keytype would be packed
(for what reason ?) and that the string pointer would start unaligned
and would cause some of strncpy()'s vector optims to fail. But that
sounds extremely strange. By experience when a program dies from such
a small change, it's a latent memory dereference that gets optimized
differently and which manifests itself via memory manipulation functions.
Note that I'm not defending gcc, you might have seen a number of angry
comments from me against it in the code. I just want to be sure we're
addressing the real issue instead of hiding it.

(...)

I'm seeing a buffer overflow in the code prior to the bug seeing the crash,
though its unlikely to trigger but it's here :

   for (i = 0; i < trash.size; i++) {  
   if (!str[i])
   break;  
   trash.str[i] = tolower(str[i]); 
   }   
   trash.str[i] = 0;   

As you can see, if you don't find a 0 in the trash, it will overwrite the
first character not part of the trash buffer.

(...)

@@ -1808,7 +1808,10 @@ static void ssl_sock_populate_sni_keytypes_hplr(const 
char *str, struct eb_root
if (!node) {
/* CN not found in tree */
s_kt = malloc(sizeof(struct sni_keytype) + i + 1);
-   strncpy((char *)s_kt->name.key, trash.str, i);
+   /* Using memcpy here instead of strncpy.
+* strncpy will cause sig_abrt errors under certain version of 
gcc with -O2
+*/
+   memcpy(s_kt->name.key, trash.str, i);
s_kt->name.key[i] = 0;
s_kt->keytypes = 0;
ebst_insert(sni_keytypes, &s_kt->name);

It might be useful to display s_kt, s_kt->name.key, sizeof(struct sni_keytype)
and i at this place to understand what upsets gcc/strncpy(). If we have to
switch to memcpy() (which I prefer in general), you can remove the manual
addition of the trailing 0 by adding 1 to the size of the memcpy() since
it's guaranteed to cover the zero.

Thanks,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Bryan found an interesting bug in the code, which I’ve root caused to an
optimization bug(?)/eccentricity in gcc 4.8.4.

Either way, I’ve fixed the error and have attached 2 more patches on top
of the 3 already provided. 0004 fixed the bug, and 0005 cleans up some of
the code.

I’m reposting all 5 here so people don’t have to track them down. Patches
1,2, and 3 are identical to the original.

Please take a look.

-Dave

On 12/3/15, 2:35 PM, "Willy Tarreau"  wrote:

>On Thu, Dec 03, 2015 at 07:24:10PM +, Dave Zhu (yanbzhu) wrote:
>> HAProxy will use the first ³crt² file that it loads as the default
>> cert(represented by bind_conf->default_ctx).
>> 
>> So, if you loaded multiple certs in one operation as your first cert,
>> HAProxy will have to determine WHICH cert is the bind_conf->default_ctx.
>> This operation happens during loading of the config, way before any
>>users
>> can connect.
>
>Ah indeed, I had not thought about that.
>
>> What I¹m saying is that the logic for loading multiple certs might
>> generate multiple SSL_CTX¹s depending on CN/SAN overlap. In that case,
>>it
>> will pick the SSL_CTX that has the highest number of different key types
>> and set it as bind_conf->default_ctx if bind_conf->default_ctx has not
>> been set previously.
>> 
>> Does that make sense?
>
>Yes it does. I just feel that it adds some uncertainty (for the admin)
>regarding the choice and that the risk that the default one changes will
>change as individual certs are expired/renewed/updated/replaced.
>
>Maybe at some point we'll have to make it possible to specify (or to
>document) the selection order so that it's stable in time and easy to
>determine.
>
>By the way this ordering may be required as well for other certs if some
>people decide for example to suddenly make RSA picked before ECDSA (if
>a vulnerability is reported or whatever for example). Then in this case
>we could use the same selection rules.
>
>Thanks for your clear explanation!
>Willy
>



0001-MINOR-ssl-Added-cert_key_and_chain-struct.patch
Description: 0001-MINOR-ssl-Added-cert_key_and_chain-struct.patch


0002-MEDIUM-ssl-Added-support-for-creating-SSL_CTX-with-m.patch
Description: 0002-MEDIUM-ssl-Added-support-for-creating-SSL_CTX-with-m.patch


0003-MINOR-ssl-Added-multi-cert-support-for-crt-list-conf.patch
Description: 0003-MINOR-ssl-Added-multi-cert-support-for-crt-list-conf.patch


0004-BUG-MINOR-ssl-Fixed-code-that-crashed-under-optimiza.patch
Description: 0004-BUG-MINOR-ssl-Fixed-code-that-crashed-under-optimiza.patch


0005-MINOR-ssl-Clean-up-unused-code-fixed-spelling-error.patch
Description: 0005-MINOR-ssl-Clean-up-unused-code-fixed-spelling-error.patch

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

I’ve root caused this, it’s a compiler optimization issue with gcc 4.8.4 on 
Ubuntu. I’m trying to figure out why it’s happening as this pattern isn’t new…

If you build with -O0 it goes away, but that’s not acceptable for a production 
software product.

I’ll post a new set of patches when I find the issue.
-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Saturday, December 5, 2015 at 7:16 PM
To: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On Fri, Dec 4, 2015 at 10:17 AM, Bryan Talbot 
mailto:bryan.tal...@ijji.com>> wrote:
On Fri, Dec 4, 2015 at 6:15 AM, Dave Zhu (yanbzhu) 
mailto:yanb...@cisco.com>> wrote:
Hey Bryan,
it’s strange that it’s always loading the ECC cert. I just tested the code on 
my end and I’m not seeing this behavior.


I see it on OSX, I'll test on Linux today.



On Ubuntu VERSION="14.04.3 LTS, Trusty Tahr" with OpenSSL 1.0.2e compiled from 
source, haproxy is crashing with your patches and a bind line of
  bind :8443 ssl crt ./var/tls/localhost.pem

If I change the bind to be
  bind :8443 ssl crt ./var/tls/
it doesn't crash.

OpenSSL 1.0.2e was built and installed to /usr/local/ssl/ with "./config && 
make && make test && sudo make install"
haproxy 1.6.2 was built from source

make -j 4 TARGET=linux2628 USE_OPENSSL=1 SSL_INC=/usr/local/ssl/include 
SSL_LIB=/usr/local/ssl/lib USE_ZLIB=1 ADDLIB=-ldl all

$> ./haproxy -vv
HA-Proxy version 1.6.2 2015/11/03
Copyright 2000-2015 Willy Tarreau mailto:wi...@haproxy.org>>

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_OPENSSL=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): yes
Built with zlib version : 1.2.8
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
Running on OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built without PCRE support (using libc's regex instead)
Built without Lua support
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT 
IP_FREEBIND

Available polling systems :
  epoll : pref=300,  test result OK
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.



$>  ./haproxy -f ./tls-test-haproxy.cfg -c
*** buffer overflow detected ***: ./haproxy terminated
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7338f)[0x7f59577da38f]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f5957871c9c]
/lib/x86_64-linux-gnu/libc.so.6(+0x109b60)[0x7f5957870b60]
/lib/x86_64-linux-gnu/libc.so.6(__stpncpy_chk+0x0)[0x7f595786ffc0]
./haproxy[0x48dc4e]
./haproxy[0x490ec8]
./haproxy[0x493090]
./haproxy[0x4932d1]
./haproxy[0x41e27d]
./haproxy[0x42a680]
./haproxy[0x406676]
./haproxy[0x40490c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f5957788ec5]
./haproxy[0x405963]
=== Memory map: 
0040-006cb000 r-xp  08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008ca000-008cb000 r--p 002ca000 08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008cb000-008dc000 rw-p 002cb000 08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008dc000-008ed000 rw-p  00:00 0
01aee000-01b0f000 rw-p  00:00 0  [heap]
7f5957551000-7f5957567000 r-xp  08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957567000-7f5957766000 ---p 00016000 08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957766000-7f5957767000 rw-p 00015000 08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957767000-7f5957922000 r-xp  08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957922000-7f5957b21000 ---p 001bb000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b21000-7f5957b25000 r--p 001ba000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b25000-7f5957b27000 rw-p 001be000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b27000-7f5957b2c000 rw-p  00:00 0
7f5957b2c000-7f5957b2f000 r-xp  08:01 2138

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

I’ve replicated the core dump. I’m debugging it now. Don’t worry about sending 
the certs.

-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Saturday, December 5, 2015 at 7:16 PM
To: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On Fri, Dec 4, 2015 at 10:17 AM, Bryan Talbot 
mailto:bryan.tal...@ijji.com>> wrote:
On Fri, Dec 4, 2015 at 6:15 AM, Dave Zhu (yanbzhu) 
mailto:yanb...@cisco.com>> wrote:
Hey Bryan,
it’s strange that it’s always loading the ECC cert. I just tested the code on 
my end and I’m not seeing this behavior.


I see it on OSX, I'll test on Linux today.



On Ubuntu VERSION="14.04.3 LTS, Trusty Tahr" with OpenSSL 1.0.2e compiled from 
source, haproxy is crashing with your patches and a bind line of
  bind :8443 ssl crt ./var/tls/localhost.pem

If I change the bind to be
  bind :8443 ssl crt ./var/tls/
it doesn't crash.

OpenSSL 1.0.2e was built and installed to /usr/local/ssl/ with "./config && 
make && make test && sudo make install"
haproxy 1.6.2 was built from source

make -j 4 TARGET=linux2628 USE_OPENSSL=1 SSL_INC=/usr/local/ssl/include 
SSL_LIB=/usr/local/ssl/lib USE_ZLIB=1 ADDLIB=-ldl all

$> ./haproxy -vv
HA-Proxy version 1.6.2 2015/11/03
Copyright 2000-2015 Willy Tarreau mailto:wi...@haproxy.org>>

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_OPENSSL=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): yes
Built with zlib version : 1.2.8
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
Running on OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built without PCRE support (using libc's regex instead)
Built without Lua support
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT 
IP_FREEBIND

Available polling systems :
  epoll : pref=300,  test result OK
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.



$>  ./haproxy -f ./tls-test-haproxy.cfg -c
*** buffer overflow detected ***: ./haproxy terminated
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7338f)[0x7f59577da38f]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f5957871c9c]
/lib/x86_64-linux-gnu/libc.so.6(+0x109b60)[0x7f5957870b60]
/lib/x86_64-linux-gnu/libc.so.6(__stpncpy_chk+0x0)[0x7f595786ffc0]
./haproxy[0x48dc4e]
./haproxy[0x490ec8]
./haproxy[0x493090]
./haproxy[0x4932d1]
./haproxy[0x41e27d]
./haproxy[0x42a680]
./haproxy[0x406676]
./haproxy[0x40490c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f5957788ec5]
./haproxy[0x405963]
=== Memory map: 
0040-006cb000 r-xp  08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008ca000-008cb000 r--p 002ca000 08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008cb000-008dc000 rw-p 002cb000 08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008dc000-008ed000 rw-p  00:00 0
01aee000-01b0f000 rw-p  00:00 0  [heap]
7f5957551000-7f5957567000 r-xp  08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957567000-7f5957766000 ---p 00016000 08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957766000-7f5957767000 rw-p 00015000 08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957767000-7f5957922000 r-xp  08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957922000-7f5957b21000 ---p 001bb000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b21000-7f5957b25000 r--p 001ba000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b25000-7f5957b27000 rw-p 001be000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b27000-7f5957b2c000 rw-p  00:00 0
7f5957b2c000-7f5957b2f000 r-xp  08:01 2138   
/lib/x86_64-linux-gnu/libdl-2.19.so<http://libdl-2.19.so>
7f5957b2f000-7f5957d2e000 ---p 3000 08:01 2138   
/lib/x86_64-linux-gnu/libdl-2.19.so<http://l

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Bryan,

Are these test certificates? If so could you send them to me so that I can test 
them on my side?

Thanks!
-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Saturday, December 5, 2015 at 7:16 PM
To: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On Fri, Dec 4, 2015 at 10:17 AM, Bryan Talbot 
mailto:bryan.tal...@ijji.com>> wrote:
On Fri, Dec 4, 2015 at 6:15 AM, Dave Zhu (yanbzhu) 
mailto:yanb...@cisco.com>> wrote:
Hey Bryan,
it’s strange that it’s always loading the ECC cert. I just tested the code on 
my end and I’m not seeing this behavior.


I see it on OSX, I'll test on Linux today.



On Ubuntu VERSION="14.04.3 LTS, Trusty Tahr" with OpenSSL 1.0.2e compiled from 
source, haproxy is crashing with your patches and a bind line of
  bind :8443 ssl crt ./var/tls/localhost.pem

If I change the bind to be
  bind :8443 ssl crt ./var/tls/
it doesn't crash.

OpenSSL 1.0.2e was built and installed to /usr/local/ssl/ with "./config && 
make && make test && sudo make install"
haproxy 1.6.2 was built from source

make -j 4 TARGET=linux2628 USE_OPENSSL=1 SSL_INC=/usr/local/ssl/include 
SSL_LIB=/usr/local/ssl/lib USE_ZLIB=1 ADDLIB=-ldl all

$> ./haproxy -vv
HA-Proxy version 1.6.2 2015/11/03
Copyright 2000-2015 Willy Tarreau mailto:wi...@haproxy.org>>

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_OPENSSL=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): yes
Built with zlib version : 1.2.8
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
Running on OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built without PCRE support (using libc's regex instead)
Built without Lua support
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT 
IP_FREEBIND

Available polling systems :
  epoll : pref=300,  test result OK
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.



$>  ./haproxy -f ./tls-test-haproxy.cfg -c
*** buffer overflow detected ***: ./haproxy terminated
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7338f)[0x7f59577da38f]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f5957871c9c]
/lib/x86_64-linux-gnu/libc.so.6(+0x109b60)[0x7f5957870b60]
/lib/x86_64-linux-gnu/libc.so.6(__stpncpy_chk+0x0)[0x7f595786ffc0]
./haproxy[0x48dc4e]
./haproxy[0x490ec8]
./haproxy[0x493090]
./haproxy[0x4932d1]
./haproxy[0x41e27d]
./haproxy[0x42a680]
./haproxy[0x406676]
./haproxy[0x40490c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f5957788ec5]
./haproxy[0x405963]
=== Memory map: 
0040-006cb000 r-xp  08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008ca000-008cb000 r--p 002ca000 08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008cb000-008dc000 rw-p 002cb000 08:01 268022 
/home/vagrant/haproxy-1.6.2/haproxy
008dc000-008ed000 rw-p  00:00 0
01aee000-01b0f000 rw-p  00:00 0  [heap]
7f5957551000-7f5957567000 r-xp  08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957567000-7f5957766000 ---p 00016000 08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957766000-7f5957767000 rw-p 00015000 08:01 2286   
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957767000-7f5957922000 r-xp  08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957922000-7f5957b21000 ---p 001bb000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b21000-7f5957b25000 r--p 001ba000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b25000-7f5957b27000 rw-p 001be000 08:01 2269   
/lib/x86_64-linux-gnu/libc-2.19.so<http://libc-2.19.so>
7f5957b27000-7f5957b2c000 rw-p  00:00 0
7f5957b2c000-7f5957b2f000 r-xp  08:01 2138   
/lib/x86_64-linux-gnu/libdl-2.19.so<http://libdl-2.19.so>
7f5957b2f000-7f5957d2e000 ---p 3000 08:01 2138   
/lib/x86_64-linux-gnu/libdl

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-05 Thread Bryan Talbot

On Fri, Dec 4, 2015 at 10:17 AM, Bryan Talbot  wrote:

> On Fri, Dec 4, 2015 at 6:15 AM, Dave Zhu (yanbzhu) 
> wrote:
>
>> Hey Bryan,
>> it’s strange that it’s always loading the ECC cert. I just tested the
>> code on my end and I’m not seeing this behavior.
>>
>>
> I see it on OSX, I'll test on Linux today.
>
>

On Ubuntu VERSION="14.04.3 LTS, Trusty Tahr" with OpenSSL 1.0.2e compiled
from source, haproxy is crashing with your patches and a bind line of
  bind :8443 ssl crt ./var/tls/localhost.pem

If I change the bind to be
  bind :8443 ssl crt ./var/tls/
it doesn't crash.

OpenSSL 1.0.2e was built and installed to /usr/local/ssl/ with "./config &&
make && make test && sudo make install"
haproxy 1.6.2 was built from source

make -j 4 TARGET=linux2628 USE_OPENSSL=1 SSL_INC=/usr/local/ssl/include
SSL_LIB=/usr/local/ssl/lib USE_ZLIB=1 ADDLIB=-ldl all

$> ./haproxy -vv
HA-Proxy version 1.6.2 2015/11/03
Copyright 2000-2015 Willy Tarreau 

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_OPENSSL=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): yes
Built with zlib version : 1.2.8
Compression algorithms supported : identity("identity"),
deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
Running on OpenSSL version : OpenSSL 1.0.2e 3 Dec 2015
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built without PCRE support (using libc's regex instead)
Built without Lua support
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
IP_FREEBIND

Available polling systems :
  epoll : pref=300,  test result OK
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.



$>  ./haproxy -f ./tls-test-haproxy.cfg -c
*** buffer overflow detected ***: ./haproxy terminated
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7338f)[0x7f59577da38f]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f5957871c9c]
/lib/x86_64-linux-gnu/libc.so.6(+0x109b60)[0x7f5957870b60]
/lib/x86_64-linux-gnu/libc.so.6(__stpncpy_chk+0x0)[0x7f595786ffc0]
./haproxy[0x48dc4e]
./haproxy[0x490ec8]
./haproxy[0x493090]
./haproxy[0x4932d1]
./haproxy[0x41e27d]
./haproxy[0x42a680]
./haproxy[0x406676]
./haproxy[0x40490c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f5957788ec5]
./haproxy[0x405963]
=== Memory map: 
0040-006cb000 r-xp  08:01 268022
/home/vagrant/haproxy-1.6.2/haproxy
008ca000-008cb000 r--p 002ca000 08:01 268022
/home/vagrant/haproxy-1.6.2/haproxy
008cb000-008dc000 rw-p 002cb000 08:01 268022
/home/vagrant/haproxy-1.6.2/haproxy
008dc000-008ed000 rw-p  00:00 0
01aee000-01b0f000 rw-p  00:00 0
 [heap]
7f5957551000-7f5957567000 r-xp  08:01 2286
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957567000-7f5957766000 ---p 00016000 08:01 2286
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957766000-7f5957767000 rw-p 00015000 08:01 2286
/lib/x86_64-linux-gnu/libgcc_s.so.1
7f5957767000-7f5957922000 r-xp  08:01 2269
/lib/x86_64-linux-gnu/libc-2.19.so
7f5957922000-7f5957b21000 ---p 001bb000 08:01 2269
/lib/x86_64-linux-gnu/libc-2.19.so
7f5957b21000-7f5957b25000 r--p 001ba000 08:01 2269
/lib/x86_64-linux-gnu/libc-2.19.so
7f5957b25000-7f5957b27000 rw-p 001be000 08:01 2269
/lib/x86_64-linux-gnu/libc-2.19.so
7f5957b27000-7f5957b2c000 rw-p  00:00 0
7f5957b2c000-7f5957b2f000 r-xp  08:01 2138
/lib/x86_64-linux-gnu/libdl-2.19.so
7f5957b2f000-7f5957d2e000 ---p 3000 08:01 2138
/lib/x86_64-linux-gnu/libdl-2.19.so
7f5957d2e000-7f5957d2f000 r--p 2000 08:01 2138
/lib/x86_64-linux-gnu/libdl-2.19.so
7f5957d2f000-7f5957d3 rw-p 3000 08:01 2138
/lib/x86_64-linux-gnu/libdl-2.19.so
7f5957d3-7f5957d48000 r-xp  08:01 2166
/lib/x86_64-linux-gnu/libz.so.1.2.8
7f5957d48000-7f5957f47000 ---p 00018000 08:01 2166
/lib/x86_64-linux-gnu/libz.so.1.2.8
7f5957f47000-7f5957f48000 r--p 00017000 08:01 2166
/lib/x86_64-linux-gnu/libz.so.1.2.8
7f5957f48000-7f5957f49000 rw-p 00018000 08:01 2166
/lib/x86_64-linux-gnu/libz.so.1.2.8
7f5957f49000-7f5957f52000 r-xp  08:01 2314
/lib/x86_64-linux-gnu/libcrypt-2.19.so
7f5957f52000-7f5958152000 ---p 9000 08:01 2314
/lib/x86_64-linux-gnu/libcrypt-2.19.so
7f5958152000-7f5958153000 r--p 9000 08:01 2314
/lib/x86_64-linux-gnu/libcrypt-2.19.so
7f5958153000-7f5958154000 rw-p a000 08:01 2314
/lib/x86_64-linux-gnu/libcrypt-2.19.so
7f5958154000-7f5958182000 rw-p  00:00 0
7f5958182000-7f59581a5000 r-xp  08:01 2235
/lib/x86_64-linux-gnu/ld-2.19.so
7f5958396000-7f595839a000 rw-p  00:00 0
7f59583a-7f59583a4000 rw-p  00:00 0
7f59583a4000-7f59583a5000 r--p 00022000 08:01 2235

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-04 Thread Bryan Talbot

On Fri, Dec 4, 2015 at 6:15 AM, Dave Zhu (yanbzhu) 
wrote:

> Hey Bryan,
> it’s strange that it’s always loading the ECC cert. I just tested the code
> on my end and I’m not seeing this behavior.
>
>
I see it on OSX, I'll test on Linux today.



> Back to your original problem though, do the certs share a CN or SAN?
> That’s the only way that they would get loaded together into a shared
> context.
>
>
Yes, the entire DN is identical for the two certs including the CN. There
is no SAN on these.


btalbot-lt:haproxy-1.6$ openssl x509 -subject -issuer -noout -pubkey -in
var/tls/localhost.pem.rsa
subject= /C=US/ST=CA/L=San Jose/O=iJJi Engineering/OU=Test
Certificate/CN=localhost.local
issuer= /C=US/ST=CA/L=San Jose/O=iJJi Engineering/OU=Test
Certificate/CN=localhost.local
-BEGIN PUBLIC KEY-
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzfd+4oUNDoF0xAjWfsg0
Ch/SVr6IOzLZPjU1z7OpNMgBbn0AQZ8znc070EJlkLdk8AjSp8EaLktz3vCPcT/J
wAJgc28/7RUIcUpLMEfSVYXyGhBDFJS0rUDM9FKXOkrxGt22e6zlrvarpQTW/05W
NLJq5ZmsvydNEEG55KBouBU/e2PlMOiRHwgOGZU4i+5XnVfvkd90A+TQiC2PhVh3
56cslp8wfcULmJ2dF3EpuiwNSaQZ8YbNWBqO2vZ7FGUwjiLD0atf9ysVJp87trvp
lA57R4TjiOAQpEdcgdiGUjJ2SjPPApS6XZUxjrlazkeL27ZPkezB3pn+NQ7BQQU1
6wIDAQAB
-END PUBLIC KEY-


btalbot-lt:haproxy-1.6$ openssl x509 -subject -issuer -noout -pubkey -in
var/tls/localhost.pem.ecdsa
subject= /C=US/ST=CA/L=San Jose/O=iJJi Engineering/OU=Test
Certificate/CN=localhost.local
issuer= /C=US/ST=CA/L=San Jose/O=iJJi Engineering/OU=Test
Certificate/CN=localhost.local
-BEGIN PUBLIC KEY-
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEQFfhz8mRC3sRZp8+hJKBTx1Qz3Mm
FPVD/Wt9giz4E0oH/a8XLnvul0q+RqzW9K7v/IFQtGxxRjgahHlUW7fw/Q==
-END PUBLIC KEY-

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-04 Thread Dave Zhu (yanbzhu)

Hey Bryan,
it's strange that it's always loading the ECC cert. I just tested the code on 
my end and I'm not seeing this behavior.

Back to your original problem though, do the certs share a CN or SAN? That's 
the only way that they would get loaded together into a shared context.

-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Thursday, December 3, 2015 at 5:24 PM
To: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Another odd thing is that both certs are loaded even if the ECC cert doesn't 
have the proper name.

In my testing with a bind line of
  bind :8443 ssl crt ./var/tls/localhost.pem

the ECC cert is loaded if it is in that directory no matter what the file name 
is.

-Bryan




On Thu, Dec 3, 2015 at 2:15 PM, Bryan Talbot 
mailto:bryan.tal...@ijji.com>> wrote:
On Thu, Dec 3, 2015 at 2:00 PM, Dave Zhu (yanbzhu) 
mailto:yanb...@cisco.com>> wrote:
Hey Bryan.

I noticed that you gave HAProxy a directory. You have to give it the name of 
the cert instead of the directory.

So your config should be:

  bind :8443 ssl crt ./var/tls/localhost.pem




I get the same behavior with that configuration.

Hopefully loading certs from a directory instead of naming them all will be 
enabled in a future patch since I think a lot of existing configs load them 
that way.

-Bryan

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-03 Thread Bryan Talbot

Another odd thing is that both certs are loaded even if the ECC cert
doesn't have the proper name.

In my testing with a bind line of
  bind :8443 ssl crt ./var/tls/localhost.pem

the ECC cert is loaded if it is in that directory no matter what the file
name is.

-Bryan




On Thu, Dec 3, 2015 at 2:15 PM, Bryan Talbot  wrote:

> On Thu, Dec 3, 2015 at 2:00 PM, Dave Zhu (yanbzhu) 
> wrote:
>
>> Hey Bryan.
>>
>> I noticed that you gave HAProxy a directory. You have to give it the name
>> of the cert instead of the directory.
>>
>> So your config should be:
>>
>>   bind :8443 ssl crt ./var/tls/localhost.pem
>>
>>
>>
>
> I get the same behavior with that configuration.
>
> Hopefully loading certs from a directory instead of naming them all will
> be enabled in a future patch since I think a lot of existing configs load
> them that way.
>
> -Bryan
>
>

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-03 Thread Bryan Talbot

On Thu, Dec 3, 2015 at 2:00 PM, Dave Zhu (yanbzhu) 
wrote:

> Hey Bryan.
>
> I noticed that you gave HAProxy a directory. You have to give it the name
> of the cert instead of the directory.
>
> So your config should be:
>
>   bind :8443 ssl crt ./var/tls/localhost.pem
>
>
>

I get the same behavior with that configuration.

Hopefully loading certs from a directory instead of naming them all will be
enabled in a future patch since I think a lot of existing configs load them
that way.

-Bryan

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Bryan.

I noticed that you gave HAProxy a directory. You have to give it the name of 
the cert instead of the directory.

So your config should be:

  bind :8443 ssl crt ./var/tls/localhost.pem

-Dave

From: Bryan Talbot mailto:bryan.tal...@ijji.com>>
Date: Thursday, December 3, 2015 at 4:45 PM
To: Yanbo Zhu mailto:yanb...@cisco.com>>
Cc: "haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hi Dave.

I've applied the patches but things are not working as I expected. It could be 
that my expectations are incorrect though. I'm expecting that with two (ECC and 
RSA) self-signed testing certificates deployed with the haproxy config shown 
below that ECC capable clients will connect and use the ECC certificate while 
old clients that do not support ECC will connect and use the RSA certificate.

What I'm seeing is that when an older OpenSSL client that does not support ECC 
attempts to connect, it fails to handshake if the ECC certificate is available 
in haproxy. If I remove the ECC certificate completely, the handshake completes 
and a suitable RSA cipher is used.

OpenSSL from OSX fails when haproxy has RSA and ECC cert in ./var/tls/

btalbot-lt:tls$ /usr/bin/openssl version
OpenSSL 0.9.8zg 14 July 2015

btalbot-lt:tls$ echo | /usr/bin/openssl s_client -connect localhost:8443
CONNECTED(0003)
78356:error:14077410:SSL routines:SSL23_GET_SERVER_HELLO:sslv3 alert handshake 
failure:/BuildRoot/Library/Caches/com.apple.xbs/Sources/OpenSSL098/OpenSSL098-59/src/ssl/s23_clnt.c:593:



but works when haproxy has only RSA cert in ./var/tls/

btalbot-lt:tls$ echo | /usr/bin/openssl s_client -connect localhost:8443
CONNECTED(0003)
depth=0 /C=US/ST=CA/L=San Jose/O=iJJi Engineering/OU=Test 
Certificate/CN=localhost.local
verify error:num=18:self signed certificate
...
New, TLSv1/SSLv3, Cipher is DHE-RSA-AES128-SHA
Server public key is 2048 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
SSL-Session:
Protocol  : TLSv1
Cipher: DHE-RSA-AES128-SHA
Session-ID: 7715FF5B9964E190619862C0D7D926E5B5519A3D40661264C7451D9D6BD1B0C9
Session-ID-ctx:
Master-Key: 
CC09E45F63C345EA9400D8E2AA34985CC85151BE8358D338FA526A3D3F02ED9E2E69AFD6D0DF01B325036FCCAEF940C8
Key-Arg   : None
Start Time: 1449175301
Timeout   : 300 (sec)
Verify return code: 18 (self signed certificate)




btalbot-lt:haproxy-1.6$ ./haproxy -vv
HA-Proxy version 1.6.2-5f5296-22 2015/12/03
Copyright 2000-2015 Willy Tarreau mailto:wi...@haproxy.org>>

Build options :
  TARGET  = generic
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_OPENSSL=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): no
Built with zlib version : 1.2.5
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.2d 9 Jul 2015
Running on OpenSSL version : OpenSSL 1.0.2d 9 Jul 2015
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built without PCRE support (using libc's regex instead)
Built without Lua support

Available polling systems :
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 2 (2 usable), will use poll.




btalbot-lt:haproxy-1.6$ cat tls-test-haproxy.cfg
global
  log 127.0.0.1:1514<http://127.0.0.1:1514> local2
  ssl-default-bind-options no-sslv3
  ssl-default-bind-ciphers 
ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!3DES:!MD5:!PSK
  tune.ssl.default-dh-param 1024
  tune.bufsize 16384
  tune.maxrewrite 1024


defaults
  timeout connect 5s
  timeout queue  50s
  timeout client 50s
  timeout server 50s
  log global
  modehttp
  option  httplog
  option  dontlognull
  option  http-keep-alive


listen https
  bind :8443 ssl crt ./var/tls/
  monitor-uri /test





btalbot-lt:haproxy-1.6$ ls -l1 ./var/tls/
localhost.pem.ecdsa
localhost.pem.rsa




btalbot-lt:haproxy-1.6$ git remote -v
origin http://git.haproxy.org/git/haproxy-1.6.git (fetch)
origin http://git.haproxy.org/git/haproxy-1.6.git (push)




btalbot-lt:haproxy-1.6$ git log origin..HEAD
commit 5

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-03 Thread Bryan Talbot

Hi Dave.

I've applied the patches but things are not working as I expected. It could
be that my expectations are incorrect though. I'm expecting that with two
(ECC and RSA) self-signed testing certificates deployed with the haproxy
config shown below that ECC capable clients will connect and use the ECC
certificate while old clients that do not support ECC will connect and use
the RSA certificate.

What I'm seeing is that when an older OpenSSL client that does not support
ECC attempts to connect, it fails to handshake if the ECC certificate is
available in haproxy. If I remove the ECC certificate completely, the
handshake completes and a suitable RSA cipher is used.

OpenSSL from OSX fails when haproxy has RSA and ECC cert in ./var/tls/

btalbot-lt:tls$ /usr/bin/openssl version
OpenSSL 0.9.8zg 14 July 2015

btalbot-lt:tls$ echo | /usr/bin/openssl s_client -connect localhost:8443
CONNECTED(0003)
78356:error:14077410:SSL routines:SSL23_GET_SERVER_HELLO:sslv3 alert
handshake
failure:/BuildRoot/Library/Caches/com.apple.xbs/Sources/OpenSSL098/OpenSSL098-59/src/ssl/s23_clnt.c:593:



but works when haproxy has only RSA cert in ./var/tls/

btalbot-lt:tls$ echo | /usr/bin/openssl s_client -connect localhost:8443
CONNECTED(0003)
depth=0 /C=US/ST=CA/L=San Jose/O=iJJi Engineering/OU=Test
Certificate/CN=localhost.local
verify error:num=18:self signed certificate
...
New, TLSv1/SSLv3, Cipher is DHE-RSA-AES128-SHA
Server public key is 2048 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
SSL-Session:
Protocol  : TLSv1
Cipher: DHE-RSA-AES128-SHA
Session-ID:
7715FF5B9964E190619862C0D7D926E5B5519A3D40661264C7451D9D6BD1B0C9
Session-ID-ctx:
Master-Key:
CC09E45F63C345EA9400D8E2AA34985CC85151BE8358D338FA526A3D3F02ED9E2E69AFD6D0DF01B325036FCCAEF940C8
Key-Arg   : None
Start Time: 1449175301
Timeout   : 300 (sec)
Verify return code: 18 (self signed certificate)




btalbot-lt:haproxy-1.6$ ./haproxy -vv
HA-Proxy version 1.6.2-5f5296-22 2015/12/03
Copyright 2000-2015 Willy Tarreau 

Build options :
  TARGET  = generic
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_OPENSSL=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): no
Built with zlib version : 1.2.5
Compression algorithms supported : identity("identity"),
deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.2d 9 Jul 2015
Running on OpenSSL version : OpenSSL 1.0.2d 9 Jul 2015
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built without PCRE support (using libc's regex instead)
Built without Lua support

Available polling systems :
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 2 (2 usable), will use poll.




btalbot-lt:haproxy-1.6$ cat tls-test-haproxy.cfg
global
  log 127.0.0.1:1514 local2
  ssl-default-bind-options no-sslv3
  ssl-default-bind-ciphers
ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!3DES:!MD5:!PSK
  tune.ssl.default-dh-param 1024
  tune.bufsize 16384
  tune.maxrewrite 1024


defaults
  timeout connect 5s
  timeout queue  50s
  timeout client 50s
  timeout server 50s
  log global
  modehttp
  option  httplog
  option  dontlognull
  option  http-keep-alive


listen https
  bind :8443 ssl crt ./var/tls/
  monitor-uri /test





btalbot-lt:haproxy-1.6$ ls -l1 ./var/tls/
localhost.pem.ecdsa
localhost.pem.rsa




btalbot-lt:haproxy-1.6$ git remote -v
origin http://git.haproxy.org/git/haproxy-1.6.git (fetch)
origin http://git.haproxy.org/git/haproxy-1.6.git (push)




btalbot-lt:haproxy-1.6$ git log origin..HEAD
commit 5f5296f7d766a37f6c55ddcb728ba436172a94ad
Author: yanbzhu 
Date:   Wed Dec 2 13:54:14 2015 -0500

MINOR: ssl: Added multi cert support for crt-list config keyword

Same functionality as previous commit, but added support to crt-list
keyword.

Note that it's not practical to support SNI filters with multicerts, so
any SNI filters that's provided to the crt-list is ignored if a
multi-cert opertion is used.

commit 98c7a958dbc93f2f58acde0b851f8423bac86005
Author: yanbzhu 
Date:   Wed Dec 2 13:01:29 2015 -0500

MEDIUM: ssl: Added support for creating SSL_CTX with multiple certs

Added ability for users to specify multiple certificates that

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-03 Thread Willy Tarreau

On Thu, Dec 03, 2015 at 07:24:10PM +, Dave Zhu (yanbzhu) wrote:
> HAProxy will use the first ³crt² file that it loads as the default
> cert(represented by bind_conf->default_ctx).
> 
> So, if you loaded multiple certs in one operation as your first cert,
> HAProxy will have to determine WHICH cert is the bind_conf->default_ctx.
> This operation happens during loading of the config, way before any users
> can connect.

Ah indeed, I had not thought about that.

> What I¹m saying is that the logic for loading multiple certs might
> generate multiple SSL_CTX¹s depending on CN/SAN overlap. In that case, it
> will pick the SSL_CTX that has the highest number of different key types
> and set it as bind_conf->default_ctx if bind_conf->default_ctx has not
> been set previously.
> 
> Does that make sense?

Yes it does. I just feel that it adds some uncertainty (for the admin)
regarding the choice and that the risk that the default one changes will
change as individual certs are expired/renewed/updated/replaced.

Maybe at some point we'll have to make it possible to specify (or to
document) the selection order so that it's stable in time and easy to
determine.

By the way this ordering may be required as well for other certs if some
people decide for example to suddenly make RSA picked before ECDSA (if
a vulnerability is reported or whatever for example). Then in this case
we could use the same selection rules.

Thanks for your clear explanation!
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Willy

On 12/3/15, 1:34 PM, "Willy Tarreau"  wrote:

>
>I'm sorry but I'm missing something. In which case could we have the
>choice
>between multiple SSL_CTX ? My understanding is that if the SNI is not
>found
>in the list, we currenlty fall back to the default cert. Now the
>difference
>is supposed to be that for the "default cert" (maybe we should call it the
>default name) we may have different certs depending on their types. Then I
>don't understand where your example above fits. Sorry for being bold on
>this,
>I'm just at the user's level and want to understand how I'll know what
>cert
>is being presented by default.

HAProxy will use the first ³crt² file that it loads as the default
cert(represented by bind_conf->default_ctx).

So, if you loaded multiple certs in one operation as your first cert,
HAProxy will have to determine WHICH cert is the bind_conf->default_ctx.
This operation happens during loading of the config, way before any users
can connect.

What I¹m saying is that the logic for loading multiple certs might
generate multiple SSL_CTX¹s depending on CN/SAN overlap. In that case, it
will pick the SSL_CTX that has the highest number of different key types
and set it as bind_conf->default_ctx if bind_conf->default_ctx has not
been set previously.

Does that make sense?
-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-03 Thread Willy Tarreau

Hi Dave,

On Thu, Dec 03, 2015 at 05:36:36PM +, Dave Zhu (yanbzhu) wrote:
> On 12/3/15, 1:40 AM, "Willy Tarreau"  wrote:
> 
> >I didn't understand what you meant with this last sentence, it sounds like
> >there could be multiple default contexts which are more or less randomly
> >chosen so that confuses me.
> 
> Sorry if that was confusing. I was merely trying to indicate that the
> logic to pick the default context will prefer SSL_CTX¹s with multiple keys
> over SSL_CTX¹s with fewer keys. So for example: Lets say that after doing
> the checks on all the names, we end up with 3 SSL_CTX¹s. One is for RSA
> only names, one for ECDSA only names and one for shared names of RSA and
> ECDSA. The code will use the shared SSL_CTX as the default SSL_CTX if none
> has yet been set.

I'm sorry but I'm missing something. In which case could we have the choice
between multiple SSL_CTX ? My understanding is that if the SNI is not found
in the list, we currenlty fall back to the default cert. Now the difference
is supposed to be that for the "default cert" (maybe we should call it the
default name) we may have different certs depending on their types. Then I
don't understand where your example above fits. Sorry for being bold on this,
I'm just at the user's level and want to understand how I'll know what cert
is being presented by default.

Regards,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Emeric,

On 12/3/15, 9:56 AM, "Emeric Brun"  wrote:

>
>But i notice some inconsistencies.
>
>Patch2 (crt conf keywoard):
>If the file without key extension is present, this file is loaded but
>also the multi_load is called.
>
>However in Patch3 (crt-list)
>If the file without key extension is present, this file is loaded but the
>multi_load is NOT called.

That shouldn¹t be the case. If the file w/o the key extension is present,
it will be found with stat, found not to be a directory, and then
ssl_sock_load_cert will return.


>
>There is a lot indentation issues in patch 2

Hopefully I¹ve fixed them in this next set of patches


>
>In patch 2 it remains a FIXME comment:
>// YANBZHU: FIXME

That was removed in patch 3, but I¹ve removed it from patch 2 a well.

The latest set of patches are attached to this as per Willy¹s suggestion.

-Dave



0001-MINOR-ssl-Added-cert_key_and_chain-struct.patch
Description: 0001-MINOR-ssl-Added-cert_key_and_chain-struct.patch


0002-MEDIUM-ssl-Added-support-for-creating-SSL_CTX-with-m.patch
Description: 0002-MEDIUM-ssl-Added-support-for-creating-SSL_CTX-with-m.patch


0003-MINOR-ssl-Added-multi-cert-support-for-crt-list-conf.patch
Description: 0003-MINOR-ssl-Added-multi-cert-support-for-crt-list-conf.patch

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Willy

On 12/3/15, 1:40 AM, "Willy Tarreau"  wrote:

>I didn't understand what you meant with this last sentence, it sounds like
>there could be multiple default contexts which are more or less randomly
>chosen so that confuses me.

Sorry if that was confusing. I was merely trying to indicate that the
logic to pick the default context will prefer SSL_CTX¹s with multiple keys
over SSL_CTX¹s with fewer keys. So for example: Lets say that after doing
the checks on all the names, we end up with 3 SSL_CTX¹s. One is for RSA
only names, one for ECDSA only names and one for shared names of RSA and
ECDSA. The code will use the shared SSL_CTX as the default SSL_CTX if none
has yet been set.

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Emeric,

I’m in the process of cleaning up the patches, indentation and style so
I’ll post up another set to the mailing list as Willy suggested.

-Dave

On 12/3/15, 9:56 AM, "Emeric Brun"  wrote:

>On 12/02/2015 08:17 PM, Dave Zhu (yanbzhu) wrote:
>> Hello all,
>> 
>> I¹ve written up Willy and Emeric¹s proposal and it seems to test fine,
>>at
>> least from a functionality standpoint.
>> 
>> I would appreciate it if interested parties would beat on this harder
>>than
>> I did to work out kinks.
>> 
>> To recap for those that are new:
>> 
>> You can now specify  as a crt or a crt-list entry, but
>>  will not actually exist. Instead, there will be
>> .rsa, .ecdsa and/or .dsa. The
>> code will load what¹s available and create (up to) 7 unique SSL_CTX with
>> the correct sets of keys and certs. It then adds these to the SNI lookup
>> tree, and sets a default context if one has not been set.
>> 
>> A couple of things to note:
>> 
>> 1) The default context will be set to the SSL_CTX that contains the set
>>of
>> most oftenly used key formats. So currently, it will prefer contexts
>>that
>> contain RSA, ECDSA then DSA in that order. SSL_CTX¹s with more key types
>> will be preferred over SSL_CTX¹s with fewer key types.
>> 2) The code for processing the CN/SAN¹s is quite complex. I¹ve added as
>> many comments as I thought was needed, but it still is not simple to
>>read.
>> The logic though is simple:
>> 
>> - Load info of each crt entry.
>> - Iterate through all CN/SAN entries and map each entry to which key
>>types
>> contain it
>> - Iterate through list of entries to create the requisite SSL_CTX¹s and
>> add the SSL_CTX to the sni_tree based on the current CN/SAN
>> 
>> Time to process is O(N) where N=# of CN/SANs.
>> 
>> In the interest of not clogging up everyone¹s inboxes, I¹ve put the
>> patches on pastebin, let me know if that works.
>> 
>> Patch 1 - http://pastebin.com/B9KXnEZN
>> Patch 2 - http://pastebin.com/qFXq2Pbe
>> Patch 3 - http://pastebin.com/F9Y1N2YN
>> 
>> Please take a look.
>> -Dave
>> 
>> 
>> On 12/1/15, 10:09 AM, "Willy Tarreau"  wrote:
>> 
>>> Hi Dave,
>>>
>>> On Tue, Dec 01, 2015 at 03:04:21PM +, Dave Zhu (yanbzhu) wrote:
 I apologize for not responding sooner, I was waiting for more comments
 before
 starting implementation, then this fell off my radar when other
 responsibilities picked up.
>>>
>>> No problem, we're all in the same situation, don't worry!
>>>
 I???ve got some downtime and can start working on Willy???s proposal,
 if that
 will meet the requirements of the people here.
>>>
>>> That would be awesome! That said, don't put yourself under pressure,
>>> yes it's something that people would love to have but don't stop all
>>> your activities for this. I tend to think that there are people
>>> volunteering for testing here given the demand, so do not hesitate
>>> to post preview patches as you did initially.
>>>
>>> Cheers,
>>> willy
>>>
>> 
>> 
>
>Hi Dave,
>
>Thank you, now the feature is clearly less intrusive.
>
>But i notice some inconsistencies.
>
>Patch2 (crt conf keywoard):
>If the file without key extension is present, this file is loaded but
>also the multi_load is called.
>
>However in Patch3 (crt-list)
>If the file without key extension is present, this file is loaded but the
>multi_load is NOT called.
>
>There is a lot indentation issues in patch 2
>
>In patch 2 it remains a FIXME comment:
>// YANBZHU: FIXME
>
>R,
>Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-03 Thread Emeric Brun

On 12/02/2015 08:17 PM, Dave Zhu (yanbzhu) wrote:
> Hello all,
> 
> I¹ve written up Willy and Emeric¹s proposal and it seems to test fine, at
> least from a functionality standpoint.
> 
> I would appreciate it if interested parties would beat on this harder than
> I did to work out kinks.
> 
> To recap for those that are new:
> 
> You can now specify  as a crt or a crt-list entry, but
>  will not actually exist. Instead, there will be
> .rsa, .ecdsa and/or .dsa. The
> code will load what¹s available and create (up to) 7 unique SSL_CTX with
> the correct sets of keys and certs. It then adds these to the SNI lookup
> tree, and sets a default context if one has not been set.
> 
> A couple of things to note:
> 
> 1) The default context will be set to the SSL_CTX that contains the set of
> most oftenly used key formats. So currently, it will prefer contexts that
> contain RSA, ECDSA then DSA in that order. SSL_CTX¹s with more key types
> will be preferred over SSL_CTX¹s with fewer key types.
> 2) The code for processing the CN/SAN¹s is quite complex. I¹ve added as
> many comments as I thought was needed, but it still is not simple to read.
> The logic though is simple:
> 
> - Load info of each crt entry.
> - Iterate through all CN/SAN entries and map each entry to which key types
> contain it
> - Iterate through list of entries to create the requisite SSL_CTX¹s and
> add the SSL_CTX to the sni_tree based on the current CN/SAN
> 
> Time to process is O(N) where N=# of CN/SANs.
> 
> In the interest of not clogging up everyone¹s inboxes, I¹ve put the
> patches on pastebin, let me know if that works.
> 
> Patch 1 - http://pastebin.com/B9KXnEZN
> Patch 2 - http://pastebin.com/qFXq2Pbe
> Patch 3 - http://pastebin.com/F9Y1N2YN
> 
> Please take a look.
> -Dave
> 
> 
> On 12/1/15, 10:09 AM, "Willy Tarreau"  wrote:
> 
>> Hi Dave,
>>
>> On Tue, Dec 01, 2015 at 03:04:21PM +, Dave Zhu (yanbzhu) wrote:
>>> I apologize for not responding sooner, I was waiting for more comments
>>> before
>>> starting implementation, then this fell off my radar when other
>>> responsibilities picked up.
>>
>> No problem, we're all in the same situation, don't worry!
>>
>>> I???ve got some downtime and can start working on Willy???s proposal,
>>> if that
>>> will meet the requirements of the people here.
>>
>> That would be awesome! That said, don't put yourself under pressure,
>> yes it's something that people would love to have but don't stop all
>> your activities for this. I tend to think that there are people
>> volunteering for testing here given the demand, so do not hesitate
>> to post preview patches as you did initially.
>>
>> Cheers,
>> willy
>>
> 
> 

Hi Dave,

Thank you, now the feature is clearly less intrusive.

But i notice some inconsistencies.

Patch2 (crt conf keywoard):
If the file without key extension is present, this file is loaded but also the 
multi_load is called.

However in Patch3 (crt-list)
If the file without key extension is present, this file is loaded but the 
multi_load is NOT called.

There is a lot indentation issues in patch 2

In patch 2 it remains a FIXME comment:
// YANBZHU: FIXME

R,
Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-02 Thread Willy Tarreau

Hi Dave,

On Wed, Dec 02, 2015 at 07:17:36PM +, Dave Zhu (yanbzhu) wrote:
> Hello all,
> 
> I¹ve written up Willy and Emeric¹s proposal and it seems to test fine, at
> least from a functionality standpoint.

Thanks a lot for doing this work!

> I would appreciate it if interested parties would beat on this harder than
> I did to work out kinks.

I'll have to study it with Emeric.

> To recap for those that are new:
> 
> You can now specify  as a crt or a crt-list entry, but
>  will not actually exist. Instead, there will be
> .rsa, .ecdsa and/or .dsa. The
> code will load what¹s available and create (up to) 7 unique SSL_CTX with
> the correct sets of keys and certs. It then adds these to the SNI lookup
> tree, and sets a default context if one has not been set.
> 
> A couple of things to note:
> 
> 1) The default context will be set to the SSL_CTX that contains the set of
> most oftenly used key formats. So currently, it will prefer contexts that
> contain RSA, ECDSA then DSA in that order. SSL_CTX¹s with more key types
> will be preferred over SSL_CTX¹s with fewer key types.

I didn't understand what you meant with this last sentence, it sounds like
there could be multiple default contexts which are more or less randomly
chosen so that confuses me.

> 2) The code for processing the CN/SAN¹s is quite complex. I¹ve added as
> many comments as I thought was needed, but it still is not simple to read.
> The logic though is simple:
> 
> - Load info of each crt entry.
> - Iterate through all CN/SAN entries and map each entry to which key types
> contain it
> - Iterate through list of entries to create the requisite SSL_CTX¹s and
> add the SSL_CTX to the sni_tree based on the current CN/SAN
> 
> Time to process is O(N) where N=# of CN/SANs.

OK thanks for explaining. Anyway I don't see how you could do less than
O(N) if you have to register N names, so that sounds fine.

> In the interest of not clogging up everyone¹s inboxes, I¹ve put the
> patches on pastebin, let me know if that works.
> 
> Patch 1 - http://pastebin.com/B9KXnEZN
> Patch 2 - http://pastebin.com/qFXq2Pbe
> Patch 3 - http://pastebin.com/F9Y1N2YN

You should have posted them here, it's more convenient for everyone to
review and respond. Don't worry for people's mailboxes, those who don't
want to receive patches nor spams don't subscribe. And these patches are
not *that* big anyway.

Thanks!
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-02 Thread Dave Zhu (yanbzhu)

Hello all,

I¹ve written up Willy and Emeric¹s proposal and it seems to test fine, at
least from a functionality standpoint.

I would appreciate it if interested parties would beat on this harder than
I did to work out kinks.

To recap for those that are new:

You can now specify  as a crt or a crt-list entry, but
 will not actually exist. Instead, there will be
.rsa, .ecdsa and/or .dsa. The
code will load what¹s available and create (up to) 7 unique SSL_CTX with
the correct sets of keys and certs. It then adds these to the SNI lookup
tree, and sets a default context if one has not been set.

A couple of things to note:

1) The default context will be set to the SSL_CTX that contains the set of
most oftenly used key formats. So currently, it will prefer contexts that
contain RSA, ECDSA then DSA in that order. SSL_CTX¹s with more key types
will be preferred over SSL_CTX¹s with fewer key types.
2) The code for processing the CN/SAN¹s is quite complex. I¹ve added as
many comments as I thought was needed, but it still is not simple to read.
The logic though is simple:

- Load info of each crt entry.
- Iterate through all CN/SAN entries and map each entry to which key types
contain it
- Iterate through list of entries to create the requisite SSL_CTX¹s and
add the SSL_CTX to the sni_tree based on the current CN/SAN

Time to process is O(N) where N=# of CN/SANs.

In the interest of not clogging up everyone¹s inboxes, I¹ve put the
patches on pastebin, let me know if that works.

Patch 1 - http://pastebin.com/B9KXnEZN
Patch 2 - http://pastebin.com/qFXq2Pbe
Patch 3 - http://pastebin.com/F9Y1N2YN

Please take a look.
-Dave

On 12/1/15, 10:09 AM, "Willy Tarreau"  wrote:

>Hi Dave,
>
>On Tue, Dec 01, 2015 at 03:04:21PM +, Dave Zhu (yanbzhu) wrote:
>> I apologize for not responding sooner, I was waiting for more comments
>>before
>> starting implementation, then this fell off my radar when other
>> responsibilities picked up.
>
>No problem, we're all in the same situation, don't worry!
>
>> I???ve got some downtime and can start working on Willy???s proposal,
>>if that
>> will meet the requirements of the people here.
>
>That would be awesome! That said, don't put yourself under pressure,
>yes it's something that people would love to have but don't stop all
>your activities for this. I tend to think that there are people
>volunteering for testing here given the demand, so do not hesitate
>to post preview patches as you did initially.
>
>Cheers,
>willy
>

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-01 Thread Willy Tarreau

Hi Dave,

On Tue, Dec 01, 2015 at 03:04:21PM +, Dave Zhu (yanbzhu) wrote:
> I apologize for not responding sooner, I was waiting for more comments before
> starting implementation, then this fell off my radar when other
> responsibilities picked up.

No problem, we're all in the same situation, don't worry!

> I???ve got some downtime and can start working on Willy???s proposal, if that
> will meet the requirements of the people here.

That would be awesome! That said, don't put yourself under pressure,
yes it's something that people would love to have but don't stop all
your activities for this. I tend to think that there are people
volunteering for testing here given the demand, so do not hesitate
to post preview patches as you did initially.

Cheers,
willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-01 Thread Dave Zhu (yanbzhu)

I apologize for not responding sooner, I was waiting for more comments before 
starting implementation, then this fell off my radar when other 
responsibilities picked up.

I’ve got some downtime and can start working on Willy’s proposal, if that will 
meet the requirements of the people here.

-Dave

From: Olivier Doucet mailto:webmas...@ajeux.com>>
Date: Monday, November 30, 2015 at 6:32 PM
To: Willy Tarreau mailto:w...@1wt.eu>>
Cc: Yanbo Zhu mailto:yanb...@cisco.com>>, Emeric Brun 
mailto:eb...@haproxy.com>>, Lukas Tribus 
mailto:luky...@hotmail.com>>, Remi Gacogne 
mailto:rgaco...@coredump.fr>>, Nenad Merdanovic 
mailto:ni...@nimzo.info>>, 
"haproxy@formilux.org<mailto:haproxy@formilux.org>" 
mailto:haproxy@formilux.org>>
Subject: Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hello,

I'm digging out this thread, because having multiple certificate for one single 
domain (SNI) but with different key types (RSA/ECDSA) can really be a great 
functionality. Is there some progress ? How can we help ?

A subsidiary question is : how much ECDSA certificates are supported ? So if I 
use a single ECDSA certificate, how many people wont be able to see my content ?

Olivier

2015-08-25 18:16 GMT+02:00 Willy Tarreau mailto:w...@1wt.eu>>:
Hi Dave,

On Tue, Aug 25, 2015 at 03:50:23PM +, Dave Zhu (yanbzhu) wrote:
> Hey Willy,
>
> On 8/25/15, 10:36 AM, "Willy Tarreau" mailto:w...@1wt.eu>> wrote:
>
> >This means that the RSA/DSA/ECDSA cert names must be derived from the
> >original cert name.
>
> Iąve thought of a way to avoid this behavior, with the end result being
> very similar to what you/Emeric proposed.
>
> What if we delayed the creation of the SSL_CTX until we have all the certs
> specified by the config?

In my opinion that only adds extra burden because this delay adds loss of
knowledge or association between the certs that were initially loaded at
the same time.

> We would read in all the certificates first and
> store them based on the CN/SAN inside the cert, or the SNIs specified by
> the admin. We would also store the auxiliary information as well at this
> point. Your tree would look like:
>
>   Names -> Certificates + aux info
>
>
> We then iterate on all of the Names and create an SSL_CTX for each Name
> based on the certificates available + any wildcard/negation filters we
> have. This will fill out our FQDN tree. After creating the SSL_CTXąs we
> could free the original tree, as it would no longer be needed.
>
> In this scenario, each FQDN would have an SSL_CTX associated with it,
> which is a departure from the current model. While this may seem like a
> huge spike in memory footprint, consider that OpenSSL uses references for
> keys and certificates.

I'm not much concerned by this for now because when you have many FQDN,
you already have as many SSL_CTX today. I tend to consider that large
configs (the ones where memory footprint starts to matter) don't have
many names for each of their certs. For example the config that led to
crt-list being designed was working with 5 certificates. I really
doubt that there were more than 1-2 names per cert on average, I'd even
bet something around 1.01 or so in average.

> Therefore, the additional impact is limited to the
> extra pointers in SSL_CTX, instead of duplicating X509 or PKEY buffers. We
> could also add additional logic to search through the current FQDN tree
> for łduplicate˛ SSL_CTX that contain the same cert/keys, and just use the
> pointer instead of creating a new SSL_CTX. Given enough metadata around
> the SSL_CTX in the FQDN tree, this shouldnąt be too hard.

That's the part I tend to dislike. If we later add extra parameters in
crt-list, we'll be happy to keep each line separate and *not* to merge
them. The example of validity dates was one such case but there could
be other ones.

While this may seem a stupid or completely made up example, imagine that
we could specify on each line of the crt-list a filter on the source network
to decide if the cert has to be presented or not. This way users could
decide that certs signed with official CAs are delivered to the public
while certs signed with internal CAs are delivered inside. Or even just
to use different algos depending on the network, for example test ECDSA
just on internal users first. As long as we keep all the elements of one
crt-list entry tied together, all such fantasy remains possible. When we
tear them apart and only focus on names to pack everything together, this
is not possible anymore. You said yourself that the memory usage doesn't
matter much here, let's not risk to sacrify extensivity for the sake of
trying to compress just a few more bytes.

> I feel that this would solve the problem of

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-01 Thread Julien Vehent


On 2015-12-01 02:03, Willy Tarreau wrote:

On Mon, Nov 30, 2015 at 04:20:15PM -0800, Bryan Talbot wrote:

If your clients are all "modern" browsers and mobile devices, you're
probably good. If there are old clients, or other systems calling an 
API

there can be issues especially if they are using Java <= 7.


I recently stumbled on a site (which I forgot) which reported that 
about 75%
of their visitors support ECDSA. So in short, if we can divide the CPU 
usage
by 20 for 75% of the visitors, that's roughly a 3.5x performance 
improvement

to be expected, that would be nice!


For what it's worth, the next version of Mozilla's modern guidelines 
will most
likely prefer ECDSA certificates and only have ECDHE ciphers in the 
ciphersuite.


More testing is needed, but it seems that client support is mature 
enough.


https://github.com/mozilla/server-side-tls/pull/97

- Julien

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-12-01 Thread Nenad Merdanovic

Hello Oliver,

On 12/1/2015 12:32 AM, Olivier Doucet wrote:
> Hello,
> 
> I'm digging out this thread, because having multiple certificate for one
> single domain (SNI) but with different key types (RSA/ECDSA) can really
> be a great functionality. Is there some progress ? How can we help ?
> 

In the mean time, you can use this:
http://blog.haproxy.com/2015/07/15/serving-ecc-and-rsa-certificates-on-same-ip-with-haproxy/

> A subsidiary question is : how much ECDSA certificates are supported ?
> So if I use a single ECDSA certificate, how many people wont be able to
> see my content ?

On a fairly busy site (couple tens of millions requests per day),
roughly 85% of the clients have been found ECDSA capable.

Regards,
Nenad

> 
> 
> Olivier
> 
> 
> 2015-08-25 18:16 GMT+02:00 Willy Tarreau mailto:w...@1wt.eu>>:
> 
> Hi Dave,
> 
> On Tue, Aug 25, 2015 at 03:50:23PM +, Dave Zhu (yanbzhu) wrote:
> > Hey Willy,
> >
> > On 8/25/15, 10:36 AM, "Willy Tarreau"  > wrote:
> >
> > >This means that the RSA/DSA/ECDSA cert names must be derived from the
> > >original cert name.
> >
> > Iąve thought of a way to avoid this behavior, with the end result
> being
> > very similar to what you/Emeric proposed.
> >
> > What if we delayed the creation of the SSL_CTX until we have all the 
> certs
> > specified by the config?
> 
> In my opinion that only adds extra burden because this delay adds
> loss of
> knowledge or association between the certs that were initially loaded at
> the same time.
> 
> > We would read in all the certificates first and
> > store them based on the CN/SAN inside the cert, or the SNIs specified by
> > the admin. We would also store the auxiliary information as well at this
> > point. Your tree would look like:
> >
> >   Names -> Certificates + aux info
> >
> >
> > We then iterate on all of the Names and create an SSL_CTX for each Name
> > based on the certificates available + any wildcard/negation filters we
> > have. This will fill out our FQDN tree. After creating the
> SSL_CTXąs we
> > could free the original tree, as it would no longer be needed.
> >
> > In this scenario, each FQDN would have an SSL_CTX associated with it,
> > which is a departure from the current model. While this may seem like a
> > huge spike in memory footprint, consider that OpenSSL uses references 
> for
> > keys and certificates.
> 
> I'm not much concerned by this for now because when you have many FQDN,
> you already have as many SSL_CTX today. I tend to consider that large
> configs (the ones where memory footprint starts to matter) don't have
> many names for each of their certs. For example the config that led to
> crt-list being designed was working with 5 certificates. I really
> doubt that there were more than 1-2 names per cert on average, I'd even
> bet something around 1.01 or so in average.
> 
> > Therefore, the additional impact is limited to the
> > extra pointers in SSL_CTX, instead of duplicating X509 or PKEY buffers. 
> We
> > could also add additional logic to search through the current FQDN tree
> > for łduplicate˛ SSL_CTX that contain the same cert/keys, and just
> use the
> > pointer instead of creating a new SSL_CTX. Given enough metadata around
> > the SSL_CTX in the FQDN tree, this shouldnąt be too hard.
> 
> That's the part I tend to dislike. If we later add extra parameters in
> crt-list, we'll be happy to keep each line separate and *not* to merge
> them. The example of validity dates was one such case but there could
> be other ones.
> 
> While this may seem a stupid or completely made up example, imagine that
> we could specify on each line of the crt-list a filter on the source
> network
> to decide if the cert has to be presented or not. This way users could
> decide that certs signed with official CAs are delivered to the public
> while certs signed with internal CAs are delivered inside. Or even just
> to use different algos depending on the network, for example test ECDSA
> just on internal users first. As long as we keep all the elements of one
> crt-list entry tied together, all such fantasy remains possible. When we
> tear them apart and only focus on names to pack everything together,
> this
> is not possible anymore. You said yourself that the memory usage doesn't
> matter much here, let's not risk to sacrify extensivity for the sake of
> trying to compress just a few more bytes.
> 
> > I feel that this would solve the problem of admins having to keep track 
> of
> > the certificate names, and keep the current behavior of łLet HAProxy
> > figure out the certs, hereąs a bunch of them˛.
> >
> > It would also solve the issue of conflicting names. For example:
>

Re: [SPAM] Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-11-30 Thread Willy Tarreau

On Mon, Nov 30, 2015 at 04:20:15PM -0800, Bryan Talbot wrote:
> On Mon, Nov 30, 2015 at 3:32 PM, Olivier Doucet  wrote:
> 
> > Hello,
> >
> > I'm digging out this thread, because having multiple certificate for one
> > single domain (SNI) but with different key types (RSA/ECDSA) can really be
> > a great functionality. Is there some progress ? How can we help ?
> >
> 
> 
> I'd love to see better support for multiple certificate key types for the
> same SNI too.
> 
> That said, it is possible to serve both EC and RSA keyed certificates using
> haproxy 1.6 now. See
> http://blog.haproxy.com/2015/07/15/serving-ecc-and-rsa-certificates-on-same-ip-with-haproxy/
> for details. It's not exactly pretty but it does seem to work.

Sure, it was an efficient solution : simple to implement and reliable.
But now we clearly need to finish the work that was started a few months
ago on the subject.

> > A subsidiary question is : how much ECDSA certificates are supported ? So
> > if I use a single ECDSA certificate, how many people wont be able to see my
> > content ?
> >
> >
> >
> They're pretty well supported by modern clients. Exactly what that means is
> a bit fuzzy though: see
> https://wiki.mozilla.org/Security/Server_Side_TLS#DHE_and_ECDHE_support for
> additional details.
> 
> If your clients are all "modern" browsers and mobile devices, you're
> probably good. If there are old clients, or other systems calling an API
> there can be issues especially if they are using Java <= 7.

I recently stumbled on a site (which I forgot) which reported that about 75%
of their visitors support ECDSA. So in short, if we can divide the CPU usage
by 20 for 75% of the visitors, that's roughly a 3.5x performance improvement
to be expected, that would be nice!

Regards,
Willy

Re: [SPAM] Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-11-30 Thread Bryan Talbot

On Mon, Nov 30, 2015 at 3:32 PM, Olivier Doucet  wrote:

> Hello,
>
> I'm digging out this thread, because having multiple certificate for one
> single domain (SNI) but with different key types (RSA/ECDSA) can really be
> a great functionality. Is there some progress ? How can we help ?
>

I'd love to see better support for multiple certificate key types for the
same SNI too.

That said, it is possible to serve both EC and RSA keyed certificates using
haproxy 1.6 now. See
http://blog.haproxy.com/2015/07/15/serving-ecc-and-rsa-certificates-on-same-ip-with-haproxy/
for details. It's not exactly pretty but it does seem to work.

>
> A subsidiary question is : how much ECDSA certificates are supported ? So
> if I use a single ECDSA certificate, how many people wont be able to see my
> content ?
>
>
>
They're pretty well supported by modern clients. Exactly what that means is
a bit fuzzy though: see
https://wiki.mozilla.org/Security/Server_Side_TLS#DHE_and_ECDHE_support for
additional details.

If your clients are all "modern" browsers and mobile devices, you're
probably good. If there are old clients, or other systems calling an API
there can be issues especially if they are using Java <= 7.

I've also discovered that Amazon CloudFront doesn't support EC certificates
at all. Can't use them in CloudFront distributions and CloudFront won't
connect to an Origin that uses them.

-Bryan

[SPAM] Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-11-30 Thread Olivier Doucet

Hello,

I'm digging out this thread, because having multiple certificate for one
single domain (SNI) but with different key types (RSA/ECDSA) can really be
a great functionality. Is there some progress ? How can we help ?

A subsidiary question is : how much ECDSA certificates are supported ? So
if I use a single ECDSA certificate, how many people wont be able to see my
content ?


Olivier


2015-08-25 18:16 GMT+02:00 Willy Tarreau :

> Hi Dave,
>
> On Tue, Aug 25, 2015 at 03:50:23PM +, Dave Zhu (yanbzhu) wrote:
> > Hey Willy,
> >
> > On 8/25/15, 10:36 AM, "Willy Tarreau"  wrote:
> >
> > >This means that the RSA/DSA/ECDSA cert names must be derived from the
> > >original cert name.
> >
> > Iąve thought of a way to avoid this behavior, with the end result being
> > very similar to what you/Emeric proposed.
> >
> > What if we delayed the creation of the SSL_CTX until we have all the
> certs
> > specified by the config?
>
> In my opinion that only adds extra burden because this delay adds loss of
> knowledge or association between the certs that were initially loaded at
> the same time.
>
> > We would read in all the certificates first and
> > store them based on the CN/SAN inside the cert, or the SNIs specified by
> > the admin. We would also store the auxiliary information as well at this
> > point. Your tree would look like:
> >
> >   Names -> Certificates + aux info
> >
> >
> > We then iterate on all of the Names and create an SSL_CTX for each Name
> > based on the certificates available + any wildcard/negation filters we
> > have. This will fill out our FQDN tree. After creating the SSL_CTXąs we
> > could free the original tree, as it would no longer be needed.
> >
> > In this scenario, each FQDN would have an SSL_CTX associated with it,
> > which is a departure from the current model. While this may seem like a
> > huge spike in memory footprint, consider that OpenSSL uses references for
> > keys and certificates.
>
> I'm not much concerned by this for now because when you have many FQDN,
> you already have as many SSL_CTX today. I tend to consider that large
> configs (the ones where memory footprint starts to matter) don't have
> many names for each of their certs. For example the config that led to
> crt-list being designed was working with 5 certificates. I really
> doubt that there were more than 1-2 names per cert on average, I'd even
> bet something around 1.01 or so in average.
>
> > Therefore, the additional impact is limited to the
> > extra pointers in SSL_CTX, instead of duplicating X509 or PKEY buffers.
> We
> > could also add additional logic to search through the current FQDN tree
> > for łduplicate˛ SSL_CTX that contain the same cert/keys, and just use the
> > pointer instead of creating a new SSL_CTX. Given enough metadata around
> > the SSL_CTX in the FQDN tree, this shouldnąt be too hard.
>
> That's the part I tend to dislike. If we later add extra parameters in
> crt-list, we'll be happy to keep each line separate and *not* to merge
> them. The example of validity dates was one such case but there could
> be other ones.
>
> While this may seem a stupid or completely made up example, imagine that
> we could specify on each line of the crt-list a filter on the source
> network
> to decide if the cert has to be presented or not. This way users could
> decide that certs signed with official CAs are delivered to the public
> while certs signed with internal CAs are delivered inside. Or even just
> to use different algos depending on the network, for example test ECDSA
> just on internal users first. As long as we keep all the elements of one
> crt-list entry tied together, all such fantasy remains possible. When we
> tear them apart and only focus on names to pack everything together, this
> is not possible anymore. You said yourself that the memory usage doesn't
> matter much here, let's not risk to sacrify extensivity for the sake of
> trying to compress just a few more bytes.
>
> > I feel that this would solve the problem of admins having to keep track
> of
> > the certificate names, and keep the current behavior of łLet HAProxy
> > figure out the certs, hereąs a bunch of them˛.
> >
> > It would also solve the issue of conflicting names. For example:
> >
> > Cert A(RSA):
> >
> > CN: www.blahDomain.com
> > SANs: 1.blahDomain.com
> >   2.blahDomain.com
> >   3.blahDomain.com
> >
> > Cert B(ECDSA)
> >
> > CN: www.blahDomain.com
> >
> > SANs: 2.blahDomain.com
> >   3.blahDomain.com
> >   4.blahDomain.com
> >
> >
> > If we optimize the insertion logic via metadata, we would have the
> > following in our FQDN tree:
> >
> > 1: Name=www.blahDomain.com; SSL_CTX#1={Cert A, Cert B}
> > 2: Name=1.blahDomain.com;   SSL_CTX#2={Cert A}
> >
> > 3: Name=2.blahDomain.com;   SSL_CTX#1={Cert A, Cert B}
> >
> > 4: Name=3.blahDomain.com;   SSL_CTX#1={Cert A, Cert B}
> >
> > 5: Name=4.blahDomain.com;   SSL_CTX#3={Cert B}
> >
> >
> > Like your proposal, we would have 3 distinct

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-25 Thread Willy Tarreau

Hi Dave,

On Tue, Aug 25, 2015 at 03:50:23PM +, Dave Zhu (yanbzhu) wrote:
> Hey Willy,
> 
> On 8/25/15, 10:36 AM, "Willy Tarreau"  wrote:
> 
> >This means that the RSA/DSA/ECDSA cert names must be derived from the
> >original cert name.
> 
> I¹ve thought of a way to avoid this behavior, with the end result being
> very similar to what you/Emeric proposed.
> 
> What if we delayed the creation of the SSL_CTX until we have all the certs
> specified by the config?

In my opinion that only adds extra burden because this delay adds loss of
knowledge or association between the certs that were initially loaded at
the same time.

> We would read in all the certificates first and
> store them based on the CN/SAN inside the cert, or the SNIs specified by
> the admin. We would also store the auxiliary information as well at this
> point. Your tree would look like:
> 
>   Names -> Certificates + aux info
> 
> 
> We then iterate on all of the Names and create an SSL_CTX for each Name
> based on the certificates available + any wildcard/negation filters we
> have. This will fill out our FQDN tree. After creating the SSL_CTX¹s we
> could free the original tree, as it would no longer be needed.
>
> In this scenario, each FQDN would have an SSL_CTX associated with it,
> which is a departure from the current model. While this may seem like a
> huge spike in memory footprint, consider that OpenSSL uses references for
> keys and certificates.

I'm not much concerned by this for now because when you have many FQDN,
you already have as many SSL_CTX today. I tend to consider that large
configs (the ones where memory footprint starts to matter) don't have
many names for each of their certs. For example the config that led to
crt-list being designed was working with 5 certificates. I really
doubt that there were more than 1-2 names per cert on average, I'd even
bet something around 1.01 or so in average.

> Therefore, the additional impact is limited to the
> extra pointers in SSL_CTX, instead of duplicating X509 or PKEY buffers. We
> could also add additional logic to search through the current FQDN tree
> for ³duplicate² SSL_CTX that contain the same cert/keys, and just use the
> pointer instead of creating a new SSL_CTX. Given enough metadata around
> the SSL_CTX in the FQDN tree, this shouldn¹t be too hard.

That's the part I tend to dislike. If we later add extra parameters in
crt-list, we'll be happy to keep each line separate and *not* to merge
them. The example of validity dates was one such case but there could
be other ones.

While this may seem a stupid or completely made up example, imagine that
we could specify on each line of the crt-list a filter on the source network 
to decide if the cert has to be presented or not. This way users could
decide that certs signed with official CAs are delivered to the public
while certs signed with internal CAs are delivered inside. Or even just
to use different algos depending on the network, for example test ECDSA
just on internal users first. As long as we keep all the elements of one
crt-list entry tied together, all such fantasy remains possible. When we
tear them apart and only focus on names to pack everything together, this
is not possible anymore. You said yourself that the memory usage doesn't
matter much here, let's not risk to sacrify extensivity for the sake of
trying to compress just a few more bytes.

> I feel that this would solve the problem of admins having to keep track of
> the certificate names, and keep the current behavior of ³Let HAProxy
> figure out the certs, here¹s a bunch of them².
> 
> It would also solve the issue of conflicting names. For example:
> 
> Cert A(RSA):
> 
> CN: www.blahDomain.com
> SANs: 1.blahDomain.com
>   2.blahDomain.com
>   3.blahDomain.com
> 
> Cert B(ECDSA)
> 
> CN: www.blahDomain.com
> 
> SANs: 2.blahDomain.com
>   3.blahDomain.com
>   4.blahDomain.com
> 
> 
> If we optimize the insertion logic via metadata, we would have the
> following in our FQDN tree:
> 
> 1: Name=www.blahDomain.com; SSL_CTX#1={Cert A, Cert B}
> 2: Name=1.blahDomain.com;   SSL_CTX#2={Cert A}
> 
> 3: Name=2.blahDomain.com;   SSL_CTX#1={Cert A, Cert B}
> 
> 4: Name=3.blahDomain.com;   SSL_CTX#1={Cert A, Cert B}
> 
> 5: Name=4.blahDomain.com;   SSL_CTX#3={Cert B}
> 
> 
> Like your proposal, we would have 3 distinct SSL_CTX¹s in memory, and 5
> entries in the FQDN table.

In the case I proposed, it would be exactly the same, without requiring to
merge all of them by name between different lines.

> If the admin specified multiple certificates and wildcards that met the
> same criteria, we would have to decide on which cert goes into the SSL_CTX
> for that FQDN. My personal preference is to use the explicit before the
> wildcard, but either way the logic would not be difficult.

That's why I prefer not to change the existing selection logic which is
documented and deployed. Also, please keep in mind that changing anything
here is reall

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-25 Thread Dave Zhu (yanbzhu)

Hey Willy,

On 8/25/15, 10:36 AM, "Willy Tarreau"  wrote:

>This means that the RSA/DSA/ECDSA cert names must be derived from the
>original cert name.

I¹ve thought of a way to avoid this behavior, with the end result being
very similar to what you/Emeric proposed.

What if we delayed the creation of the SSL_CTX until we have all the certs
specified by the config? We would read in all the certificates first and
store them based on the CN/SAN inside the cert, or the SNIs specified by
the admin. We would also store the auxiliary information as well at this
point. Your tree would look like:

Names -> Certificates + aux info

We then iterate on all of the Names and create an SSL_CTX for each Name
based on the certificates available + any wildcard/negation filters we
have. This will fill out our FQDN tree. After creating the SSL_CTX¹s we
could free the original tree, as it would no longer be needed.

In this scenario, each FQDN would have an SSL_CTX associated with it,
which is a departure from the current model. While this may seem like a
huge spike in memory footprint, consider that OpenSSL uses references for
keys and certificates. Therefore, the additional impact is limited to the
extra pointers in SSL_CTX, instead of duplicating X509 or PKEY buffers. We
could also add additional logic to search through the current FQDN tree
for ³duplicate² SSL_CTX that contain the same cert/keys, and just use the
pointer instead of creating a new SSL_CTX. Given enough metadata around
the SSL_CTX in the FQDN tree, this shouldn¹t be too hard.

I feel that this would solve the problem of admins having to keep track of
the certificate names, and keep the current behavior of ³Let HAProxy
figure out the certs, here¹s a bunch of them².

It would also solve the issue of conflicting names. For example:

Cert A(RSA):

CN: www.blahDomain.com
SANs: 1.blahDomain.com
  2.blahDomain.com
  3.blahDomain.com

Cert B(ECDSA)

CN: www.blahDomain.com

SANs: 2.blahDomain.com
  3.blahDomain.com
  4.blahDomain.com

If we optimize the insertion logic via metadata, we would have the
following in our FQDN tree:

1: Name=www.blahDomain.com; SSL_CTX#1={Cert A, Cert B}
2: Name=1.blahDomain.com;   SSL_CTX#2={Cert A}

3: Name=2.blahDomain.com;   SSL_CTX#1={Cert A, Cert B}

4: Name=3.blahDomain.com;   SSL_CTX#1={Cert A, Cert B}

5: Name=4.blahDomain.com;   SSL_CTX#3={Cert B}

Like your proposal, we would have 3 distinct SSL_CTX¹s in memory, and 5
entries in the FQDN table.

If the admin specified multiple certificates and wildcards that met the
same criteria, we would have to decide on which cert goes into the SSL_CTX
for that FQDN. My personal preference is to use the explicit before the
wildcard, but either way the logic would not be difficult.

Thoughts?

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-25 Thread Robin Geuze


Hey willy,

One small comment. As of openssl v1.0.2 it actually supports loading 
multiple certificates with different chains. It requires calling 
SSL_CTX_add0_chain_cert (or SSL_CTX_add1_chain_cert, the exact 
difference can be found in the man page) instead of 
SSL_CTX_add_extra_chain_cert. I've actually hacked this in using a quick 
ad dirty method, but haven't gotten around to fixing it properly: 
https://github.com/haproxy/haproxy-1.5/compare/master...RobinGeuze:master


Besides that I agree with the idea of letting openssl do the actual 
cipher selection and such, since it keeps ssl specific logic from the 
haproxy code base, and openssl also does some specific checks that would 
need to be copied to haproxy (for example it checks whether the cipher 
fingerprint matches a bunch of safari versions and then disables ECDSA 
as an option since it was broken in those safari versions).


-Robin

On 8/25/2015 16:36, Willy Tarreau wrote:

Hi guys,

Yesterday Emeric and I brainstormed on this subject in the office. Emeric
brought on the table some cases which couldn't be reliably covered anymore,
and proposed a slightly different approach which finally convinced me.

I'll try to summarize here our long conversation and we'd like to get some
feedback on the various proposals.

A bit of background first.

1) it is currently possible to load multiple certs for the same domain in
the config, and in this case, the first one loaded will be considered,
regardless of its key type (RSA, DSA, ECDSA).

2) in crt-lists, it is possible to write filters which override the cert's CN
and alt names. Similarly, these ones are considered in their declaration
order so that the first matching one is used.

3) crt-lists support wildcards with exclusion. The cert registration works
like this :

a) the cert is declared :

 foo.pem *.mydomain.com !mail.mydomain.com

b) the cert is loaded and an SSL_CTX is created.

c) an entry is added into the wildcard tree at ".mydomain.com" with
   a reference to the SSL_CTX.

d) another entry is added into the FQDN tree at "mail.mydomain.com"
   with a negation flag set (to reflect the "!" in the filter).

   Lookup method works like this :

1) lookup for SNI name in FQDN tree, and skip entries with the
   negation flag. If found, return this SSL_CTX.

2) lookup for the SNI's domain in the wildcard tree. If found without
   a negation on the FQDN, then return it. (note there's currently a
   small bug there but that's a different story and out of scope for
   this brainstorming).

3) otherwise return default cert if not strict-sni.

4) currently, we have only one cert chain per SSL_CTX. OCSP Stapling
applies to an SSL_CTX as well since it related to a single cert.

5) a number of cert-specific configuration items can be found in auxiliary
files named based on the cert file's name, such as ".issuer", ".ocsp",
".sctl", maybe ".pwd" later, and all are loaded relative to a single
SSL_CTX, whether it makes sense or not for the future. For example,
OCSP entries are per-SSL_CTX while they should become per-certificate.


The latest proposal introduces some problems above when there's some
overlapping between certs, because each time a certificate is loaded, a
lookup in the tree will be performed to try to locate other instances of
the same name(s) and update the corresponding SSL_CTX with the new cert.
But if there are multiple instances, it doesn't work anymore.

Also even without this, another issue is that names may not match exactly.
For example, a hosting provider could be adding a new ECDSA cert for one
customer only while the original RSA cert covers multiple customers. This
already introduces a problem since the ECDSA cert would have to be added
to the same SSL_CTX as the first one, thus would be presented for all
other domains.

It's possible of course to try to detect such configurations, but if we're
realistic, they're going to be the most common ones, because new certs
with multiple names will problably not reflect obsolete domains just like
they may introduce new names. So it doesn't seem a good approach longterm-
wise.

Emeric's proposal consists in adopting a slightly different approach.

Since the first cause of the trouble relates to matching the correct
SSL_CTX when loading the second cert, and the second problem relates
to name matching differences between certs in the same SSL_CTX, there
are two important things to keep in mind :

   - certificates to be presented together must be loaded together
   - the SSL_CTX only matches a combination of names

Thus the idea would be that when a certificate is loaded (either on the
crt line or from the crt-list file), instead of loading only one cert
and trying to match it against another one, better load all possible
certs at once in the same SSL_CTX.

This means that the RSA/DSA/ECDSA cer

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-25 Thread Willy Tarreau

Hi guys,

Yesterday Emeric and I brainstormed on this subject in the office. Emeric
brought on the table some cases which couldn't be reliably covered anymore,
and proposed a slightly different approach which finally convinced me.

I'll try to summarize here our long conversation and we'd like to get some
feedback on the various proposals.

A bit of background first.

1) it is currently possible to load multiple certs for the same domain in
   the config, and in this case, the first one loaded will be considered,
   regardless of its key type (RSA, DSA, ECDSA).

2) in crt-lists, it is possible to write filters which override the cert's CN
   and alt names. Similarly, these ones are considered in their declaration
   order so that the first matching one is used.

3) crt-lists support wildcards with exclusion. The cert registration works
   like this :

   a) the cert is declared :

foo.pem *.mydomain.com !mail.mydomain.com

   b) the cert is loaded and an SSL_CTX is created.

   c) an entry is added into the wildcard tree at ".mydomain.com" with
  a reference to the SSL_CTX.

   d) another entry is added into the FQDN tree at "mail.mydomain.com"
  with a negation flag set (to reflect the "!" in the filter).

  Lookup method works like this :

   1) lookup for SNI name in FQDN tree, and skip entries with the
  negation flag. If found, return this SSL_CTX.

   2) lookup for the SNI's domain in the wildcard tree. If found without
  a negation on the FQDN, then return it. (note there's currently a
  small bug there but that's a different story and out of scope for
  this brainstorming).

   3) otherwise return default cert if not strict-sni.

4) currently, we have only one cert chain per SSL_CTX. OCSP Stapling
   applies to an SSL_CTX as well since it related to a single cert.

5) a number of cert-specific configuration items can be found in auxiliary
   files named based on the cert file's name, such as ".issuer", ".ocsp",
   ".sctl", maybe ".pwd" later, and all are loaded relative to a single
   SSL_CTX, whether it makes sense or not for the future. For example,
   OCSP entries are per-SSL_CTX while they should become per-certificate.


The latest proposal introduces some problems above when there's some
overlapping between certs, because each time a certificate is loaded, a
lookup in the tree will be performed to try to locate other instances of
the same name(s) and update the corresponding SSL_CTX with the new cert.
But if there are multiple instances, it doesn't work anymore.

Also even without this, another issue is that names may not match exactly.
For example, a hosting provider could be adding a new ECDSA cert for one
customer only while the original RSA cert covers multiple customers. This
already introduces a problem since the ECDSA cert would have to be added
to the same SSL_CTX as the first one, thus would be presented for all
other domains.

It's possible of course to try to detect such configurations, but if we're
realistic, they're going to be the most common ones, because new certs
with multiple names will problably not reflect obsolete domains just like
they may introduce new names. So it doesn't seem a good approach longterm-
wise.

Emeric's proposal consists in adopting a slightly different approach.

Since the first cause of the trouble relates to matching the correct
SSL_CTX when loading the second cert, and the second problem relates
to name matching differences between certs in the same SSL_CTX, there
are two important things to keep in mind :

  - certificates to be presented together must be loaded together
  - the SSL_CTX only matches a combination of names

Thus the idea would be that when a certificate is loaded (either on the
crt line or from the crt-list file), instead of loading only one cert
and trying to match it against another one, better load all possible
certs at once in the same SSL_CTX.

This means that the RSA/DSA/ECDSA cert names must be derived from the
original cert name. That's the first point to address for which we have
several proposals. One benefit is that it doesn't change anything at all
for existing configurations, and this is an important design aspect.

The second point is that with 3 different cert types, we have at worst
6 combinations of names, that's not huge, especially in a context where
not all certs will exist in multiple formats and where when certs are
finally provided with the two formats, the names will have had the time
to converge, so overall there shouldn't be too many differences.

The first point related to loading all certs at once comes with various
options and impacts for the administrator.

Among the options we have considered :
 
  - automatically load ".pem.1", ".pem.2", ... etc when ".pem" is loaded,
and ensure that they all have a different key type. I'm having a problem
with this, because how do we decide when to stop loading them ? And i

[SPAM] Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-24 Thread Emeric Brun

On 08/21/2015 08:08 PM, Emeric Brun wrote:
> Hey Dave,
> 
>>> the SNI tree a certificate regardless the CN/SAN. It's dirty i know, but
>>> some people use it.
>>>
>>> You will also notice, reading 'ssl_sock_process_crt_file' that if we use
>>> sni_filter (so filter in crt-list), a new ssl_ctx is always allocated and
>>> stored. But if no filter is set on the "crt-list" line,
>>> you will use SAN/CN to potentially complete an existing one. We reach an
>>> unpredictable behavior depending of the line order in crt-list.
>>
>> Correct me if I¹m wrong, but sni_filter is only applied to the crtfile
>> specified before the filter? So in the case, crt-list files without a user
>> supplied SNI will be treated as ordinary crt files.
>>
>> In the current model, these are added as SNI entries in the tree. But
>> these entries won¹t ever be used if there is another SNI entry that was
>> loaded before. So even though they are added to the tree, they wouldn¹t
>> ever be used? The behavior should remain consistent with what¹s there
>> today. Or am I mis-interpeting the code?
>>
>> -Dave 
>>
> 
> The issue is due to filters, I think the best is to write examples:
> 
> crt.list:
> (*.foobar.com RSA).pem *.foobar.com !foobar.foobar.com
> (foobar.foobar.com RSA).pem
> (foobar.foobar.com DSA).pem
> 
> 
> You will complete (*.foobar.com RSA) SSL_CTX's using the (foobar.foobar.com 
> DSA). Because first line register the certificate in tree (with the flag 
> "neg").
> 
> An user connecting with SNI dummy.foobar.com will potentially retrieve 
> (foobar.foobar.com DSA).pem as a certificate.
> 
> I think you can fix it looping on entries of same name and ignore those 
> flagged 'neg' in the tree.
> 
> 
> There is other cases:
> 
> (*.foobar.com RSA).pem
> (*.foobar.com DSA).pem *.foobar.com
> 
> Will always serve only the RSA.pem, because the second has a filter and in 
> your code it will not complete the first
> 
> Whereas
> (*.foobar.com RSA).pem *.foobar.com
> (*.foobar.com DSA).pem
> 
> Will serve both RSA and DSA, because the second will complete the first 
> regardless if the first was created with or without filter.
> 
> But if you fix it, adding a lookup based on the filter to complete existing 
> cert we will reach new inconsistencies
> 
> (*.foobar.com RSA).pem
> (*.foobar.com DSA).pem *.foobar.com !foobar.foobar.com
> 
> With that i expect to have both RSA/DSA available if dummy.foobar.com, but 
> always RSA for foobar.foobar.com.
> 
> It doesn't appear trivial to handle and i express a real doubt: Is the SNI 
> tree and CN/SAN the right way to complete SSL_CTX with different 
> certificates/keys?
> 
> If we're able to load the differents key/cert from the same pem file, we will 
> no have those inconsistencies.
> 
> Or we could use additionnal extensions as we do for .ocsp and .issuers.
> 
> R,
> Emeric
> 
> 
> 
> 
> 
>
Hi All,

This stupid anti-spam flagged my last mail as a spam because f o o b a r.

R,
Emeric

Re: [SPAM] Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-24 Thread Emeric Brun

On 08/21/2015 08:08 PM, Emeric Brun wrote:
> Hey Dave,
> 
>>> the SNI tree a certificate regardless the CN/SAN. It's dirty i know, but
>>> some people use it.
>>>
>>> You will also notice, reading 'ssl_sock_process_crt_file' that if we use
>>> sni_filter (so filter in crt-list), a new ssl_ctx is always allocated and
>>> stored. But if no filter is set on the "crt-list" line,
>>> you will use SAN/CN to potentially complete an existing one. We reach an
>>> unpredictable behavior depending of the line order in crt-list.
>>
>> Correct me if I¹m wrong, but sni_filter is only applied to the crtfile
>> specified before the filter? So in the case, crt-list files without a user
>> supplied SNI will be treated as ordinary crt files.
>>
>> In the current model, these are added as SNI entries in the tree. But
>> these entries won¹t ever be used if there is another SNI entry that was
>> loaded before. So even though they are added to the tree, they wouldn¹t
>> ever be used? The behavior should remain consistent with what¹s there
>> today. Or am I mis-interpeting the code?
>>
>> -Dave 
>>
> 
> The issue is due to filters, I think the best is to write examples:
> 
> crt.list:
> (*.foo-bar.com RSA).pem *.foo-bar.com !foo-bar.foo-bar.com
> (foo-bar.foo-bar.com RSA).pem
> (foo-bar.foo-bar.com DSA).pem
> 
> 
> You will complete (*.foo-bar.com RSA) SSL_CTX's using the 
> (foo-bar.foo-bar.com DSA). Because first line register the certificate in 
> tree (with the flag "neg").
> 
> An user connecting with SNI dummy.foo-bar.com will potentially retrieve 
> (foo-bar.foo-bar.com DSA).pem as a certificate.
> 
> I think you can fix it looping on entries of same name and ignore those 
> flagged 'neg' in the tree.
> 
> 
> There is other cases:
> 
> (*.foo-bar.com RSA).pem
> (*.foo-bar.com DSA).pem *.foo-bar.com
> 
> Will always serve only the RSA.pem, because the second has a filter and in 
> your code it will not complete the first
> 
> Whereas
> (*.foo-bar.com RSA).pem *.foo-bar.com
> (*.foo-bar.com DSA).pem
> 
> Will serve both RSA and DSA, because the second will complete the first 
> regardless if the first was created with or without filter.
> 
> But if you fix it, adding a lookup based on the filter to complete existing 
> cert we will reach new inconsistencies
> 
> (*.foo-bar.com RSA).pem
> (*.foo-bar.com DSA).pem *.foo-bar.com !foo-bar.foo-bar.com
> 
> With that i expect to have both RSA/DSA available if dummy.foo-bar.com, but 
> always RSA for foo-bar.foo-bar.com.
> 
> It doesn't appear trivial to handle and i express a real doubt: Is the SNI 
> tree and CN/SAN the right way to complete SSL_CTX with different 
> certificates/keys?
> 
> If we're able to load the differents key/cert from the same pem file, we will 
> no have those inconsistencies.
> 
> Or we could use additionnal extensions as we do for .ocsp and .issuers.
> 
> R,
> Emeric
> 
> 
> 
Hi All,

This stupid anti-spam flagged my last mail as a spam because f o o b a r.

R,
Emeric

[SPAM] Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Dave,

>> the SNI tree a certificate regardless the CN/SAN. It's dirty i know, but
>> some people use it.
>>
>> You will also notice, reading 'ssl_sock_process_crt_file' that if we use
>> sni_filter (so filter in crt-list), a new ssl_ctx is always allocated and
>> stored. But if no filter is set on the "crt-list" line,
>> you will use SAN/CN to potentially complete an existing one. We reach an
>> unpredictable behavior depending of the line order in crt-list.
> 
> Correct me if I¹m wrong, but sni_filter is only applied to the crtfile
> specified before the filter? So in the case, crt-list files without a user
> supplied SNI will be treated as ordinary crt files.
> 
> In the current model, these are added as SNI entries in the tree. But
> these entries won¹t ever be used if there is another SNI entry that was
> loaded before. So even though they are added to the tree, they wouldn¹t
> ever be used? The behavior should remain consistent with what¹s there
> today. Or am I mis-interpeting the code?
> 
> -Dave 
> 

The issue is due to filters, I think the best is to write examples:

crt.list:
(*.foobar.com RSA).pem *.foobar.com !foobar.foobar.com
(foobar.foobar.com RSA).pem
(foobar.foobar.com DSA).pem


You will complete (*.foobar.com RSA) SSL_CTX's using the (foobar.foobar.com 
DSA). Because first line register the certificate in tree (with the flag "neg").

An user connecting with SNI dummy.foobar.com will potentially retrieve 
(foobar.foobar.com DSA).pem as a certificate.

I think you can fix it looping on entries of same name and ignore those flagged 
'neg' in the tree.


There is other cases:

(*.foobar.com RSA).pem
(*.foobar.com DSA).pem *.foobar.com

Will always serve only the RSA.pem, because the second has a filter and in your 
code it will not complete the first

Whereas
(*.foobar.com RSA).pem *.foobar.com
(*.foobar.com DSA).pem

Will serve both RSA and DSA, because the second will complete the first 
regardless if the first was created with or without filter.

But if you fix it, adding a lookup based on the filter to complete existing 
cert we will reach new inconsistencies

(*.foobar.com RSA).pem
(*.foobar.com DSA).pem *.foobar.com !foobar.foobar.com

With that i expect to have both RSA/DSA available if dummy.foobar.com, but 
always RSA for foobar.foobar.com.

It doesn't appear trivial to handle and i express a real doubt: Is the SNI tree 
and CN/SAN the right way to complete SSL_CTX with different certificates/keys?

If we're able to load the differents key/cert from the same pem file, we will 
no have those inconsistencies.

Or we could use additionnal extensions as we do for .ocsp and .issuers.

R,
Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On Fri, Aug 21, 2015 at 05:09:08PM +, Dave Zhu (yanbzhu) wrote:
> On 8/21/15, 1:07 PM, "Dave Zhu (yanbzhu)"  wrote:
> 
> >Hey Emeric,
> >
> >>I think you don't notice that certificate in the wild card tree are not
> >>stored using their fullnames (we exclude the '*' and start at the first
> >>'.').
> >
> >No I did not notice this, but I believe this is actually a good thing.
> >This way, crt-list entries with a filter will always get processed and
> >added to the tree since they will always be a ³new² SNI entry.
> 
> I actually just realized what you meant by this. We could run into a
> situation where we have a negation for a given cert, but due to the way
> it??s stored, we may update the wrong ctx.
> 
> 
> I??ll add this to the list of updates.
> 
> Sorry for the confusion.

Really, don't feel sorry. The subject is more complex than it seems,
and that's why I wanted to ensure everyone had a chance to participate.
We must definitely not miss something here.

Thanks!
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On Fri, Aug 21, 2015 at 05:07:30PM +, Dave Zhu (yanbzhu) wrote:
> In the current model, these are added as SNI entries in the tree. But
> these entries won¹t ever be used if there is another SNI entry that was
> loaded before. So even though they are added to the tree, they wouldn¹t
> ever be used? The behavior should remain consistent with what¹s there
> today. Or am I mis-interpeting the code?

>From what I remember, the purpose is to be able to do something like
this in the crt-list :

wildcard.mydomain.pem  !secure.mydomain.com
secure.mydomain.pem secure.mydomain.com

The first cert is a wildcard and is not proposed for secure.mydomain.com
while the second one is.

Also I seem to remember about something related to being able to exclude
wildcard subdomains though I don't remember exactly how. I guess it's
like this :

wildcard.mydomain.pem .mydomain.com !.fr.mydomain.com
wildcard-fr.mydomain.pem  .fr.mydomain.com

The second one will handle all domains ending in .fr.mydomain.com and
the first one will carefully avoid them.

At some point we had a discussion about being able to load multiple
certs for the same domains and ignoring those out of the validity date,
which would make it convenient to load the old and new certs and let
haproxy present the first valid one matching the name, but I don't
think it was implemented (or maybe only at load time, I don't remember).

But that comes back to the original discussion : should haproxy or openssl
decide on what cert to present. From what I'm seeing now, we have downsides
with the two options :
  - openssl seems to offer limited options when there isn't a perfect
overlapping between certs (eg: one is a wildcard, the other not);

  - openssl supports a single DH param for both certs;

  - haproxy will have a hard time respecting the ciphers chain to
present the most suited cert;

If we add stricter rules to help in the first option, such as ensuring
that two apparently identical certs just differ by their algos and nothing
else, we risk to upset users trying to migrate from RSA to DSA who will
change a few things at the same time (such as taking this opportunity to
pick a larger dh-param or to extend the cert's life, or add extra alt names).

And if we're not that strict, they'll be upset if we can't ensure that the
correct cert is always delivered.

I'm starting to think that maybe we'd need to have 3 SSL_CTX when we support
two certs for the same domain :
  - one with only the first cert;
  - one with only the second;
  - one with both.

And depending on the differences in the crt-list rules, we'd point to
a different SSL_CTX.

Maybe I'm thinking too far ?

Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On 8/21/15, 1:07 PM, "Dave Zhu (yanbzhu)"  wrote:

>Hey Emeric,
>
>>I think you don't notice that certificate in the wild card tree are not
>>stored using their fullnames (we exclude the '*' and start at the first
>>'.').
>
>No I did not notice this, but I believe this is actually a good thing.
>This way, crt-list entries with a filter will always get processed and
>added to the tree since they will always be a ³new² SNI entry.

I actually just realized what you meant by this. We could run into a
situation where we have a negation for a given cert, but due to the way
it’s stored, we may update the wrong ctx.

I’ll add this to the list of updates.

Sorry for the confusion.

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Emeric,

>I think you don't notice that certificate in the wild card tree are not
>stored using their fullnames (we exclude the '*' and start at the first
>'.').

No I did not notice this, but I believe this is actually a good thing.
This way, crt-list entries with a filter will always get processed and
added to the tree since they will always be a ³new² SNI entry.

>
>A part from that,
>
>multiple cert can be inserted in the tree for a same name. I think it was
>done to manage the negation filter of the "crt-list" feature .
>
>In 'static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf
>*s)', we loop on multiple tree entries using the same name:
>
>/* lookup in full qualified names */
>node = ebst_lookup(&s->sni_ctx, trash.str);
>
>/* lookup a not neg filter */
>for (n = node; n; n = ebmb_next_dup(n)) {
>if (!container_of(n, struct sni_ctx, name)->neg) {
>node = n;
>break;
>}
>}
>
>In an other way, with none negative "crt-list" filter, we could store in
>the SNI tree a certificate regardless the CN/SAN. It's dirty i know, but
>some people use it.
>
>You will also notice, reading 'ssl_sock_process_crt_file' that if we use
>sni_filter (so filter in crt-list), a new ssl_ctx is always allocated and
>stored. But if no filter is set on the "crt-list" line,
>you will use SAN/CN to potentially complete an existing one. We reach an
>unpredictable behavior depending of the line order in crt-list.

Correct me if I¹m wrong, but sni_filter is only applied to the crtfile
specified before the filter? So in the case, crt-list files without a user
supplied SNI will be treated as ordinary crt files.

In the current model, these are added as SNI entries in the tree. But
these entries won¹t ever be used if there is another SNI entry that was
loaded before. So even though they are added to the tree, they wouldn¹t
ever be used? The behavior should remain consistent with what¹s there
today. Or am I mis-interpeting the code?

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Dave,

>> The first proposal was very less intrusive (the ones which don't used
>> 1.0.2 API but secret callback). But i understand the point of view of the
>> experts of the mailing list .
>>
>> Did you expore an intermediate solution:
>>
>> Continue to load keys/certificates etc in one new allocated SSL_CTX and
>> when you fill the SNI tree, if the certificate comes into collision with
>> an other, extract key/cert/chain from the early allocated SSL_CTX to
>> fill/complete the already existing one? I don't know
>> if Openssl API provide enougth primives to do that. But it will certainly
>> have less impact on the existing behavior.
> 
> I have considered doing this in the beginning. However, OpenSSL doesn’t
> really provide any good ways to pull key information out of an SSL CTX.
> But what you’ve described is basically what I’m doing, just with
> refactoring around the code that loads the certs.
> 
> -Dave
> 

I'm taking a look on how the feature interact with crt-list and its filters. 
And there is a lot of inconsistencies:

First of all  ssl_sock_get_ctx_from_eb_tree() is bugged

I think you don't notice that certificate in the wild card tree are not stored 
using their fullnames (we exclude the '*' and start at the first '.').

A part from that,

multiple cert can be inserted in the tree for a same name. I think it was done 
to manage the negation filter of the "crt-list" feature .

In 'static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf *s)', 
we loop on multiple tree entries using the same name:

/* lookup in full qualified names */
node = ebst_lookup(&s->sni_ctx, trash.str);

/* lookup a not neg filter */
for (n = node; n; n = ebmb_next_dup(n)) {
if (!container_of(n, struct sni_ctx, name)->neg) {
node = n;
break;
}
}

In an other way, with none negative "crt-list" filter, we could store in the 
SNI tree a certificate regardless the CN/SAN. It's dirty i know, but some 
people use it.

You will also notice, reading 'ssl_sock_process_crt_file' that if we use 
sni_filter (so filter in crt-list), a new ssl_ctx is always allocated and 
stored. But if no filter is set on the "crt-list" line, 
you will use SAN/CN to potentially complete an existing one. We reach an 
unpredictable behavior depending of the line order in crt-list.

These issues are not your fault, we always considered 1 certificate file == 1 
SSL_CTX and we start to pay our tribute :)

I've no solution for the moment.

R,
Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Emeric,

>> 
>> 
>> In 1.0.2, we can make use of API to load multiple certificate chains
>>into
>> a CTX. Prior to 1.0.2, you could not load the chains. So the checks
>>ensure
>> that we make use of the new APIs, and can then serve the correct
>> certificate to users based on the user¹s cipher suite.
>I see, but if the feature is only half-supported for openssl version <
>1.0.2 we should simply avoid to use it on these versions.

I don’t use it in earlier versions of OpenSSL, and I default back to
existing behavior. This should not be an issue


>Anyway, I admit i'm a little scared with your latest patches, you re-code
>a large part of certificates loading and i'm currently unable to
>guarantee that there is no regressions on linked features (SNI, ocsp,
>crt_list, passphrase management).
>
>The first proposal was very less intrusive (the ones which don't used
>1.0.2 API but secret callback). But i understand the point of view of the
>experts of the mailing list .
>
>Did you expore an intermediate solution:
>
>Continue to load keys/certificates etc in one new allocated SSL_CTX and
>when you fill the SNI tree, if the certificate comes into collision with
>an other, extract key/cert/chain from the early allocated SSL_CTX to
>fill/complete the already existing one? I don't know
>if Openssl API provide enougth primives to do that. But it will certainly
>have less impact on the existing behavior.

I have considered doing this in the beginning. However, OpenSSL doesn’t
really provide any good ways to pull key information out of an SSL CTX.
But what you’ve described is basically what I’m doing, just with
refactoring around the code that loads the certs.

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On 08/21/2015 03:51 PM, Dave Zhu (yanbzhu) wrote:
> 
> Emeric,
>>
>> Code initially use 'ctx->default_passwd_callback_userdata' to allow us to
>> manage a further way to manage passphrase via configuration.
> 
> As far as I could tell, this field was not getting set anywhere in the
> code. It¹s set with SSL_CTX_set_default_passwd_cb, which I did not find in
> the codebase. I had then assumed that this was just a harmless copy/paste
> of example code, which generally always use this CB.

Not exactly, it was passed to be sure to not reach a surprise the day we will 
start to use it .

We have some pending projects to manage externalized passphrase.

>>
>> I notice also you continue to load DH parameters for each files. It was
>> not a big deal by the past because for each file correspond a uniq
>> certificate.
>>
>> With your patch, i don't know what will be the behavior. In the way it is
>> loaded on SSL_CTX, the DH parameter doesn't seem specific to the used
>> DSA/RSA/ECDSA certificate. So which one will be used, first or latest
>> loaded?
>>
>> Some users will expect to use the DH parameter defined in the same file
>> than the used certificate, but i'm really not sure it will be the case.
> 
> 
> Unsure about this. I can change it to only load the first time the context
> is created.

It depends on what is expected by users.

I would prefer to use the DH parameter depending of the used RSA/ECDSA 
certificate. It would be possible with your first proposal
but it doesn't seem managed in the latest openssl-1.0.2 API.

> 
>>
>> Finally, i don't understand what will be the behavior about the
>> certificate chain with openssl < 1.0.2. I see you manage to load the
>> chain differently depending the version of openssl but i ignore if the
>> behavior will differ.
> 
> 
> In 1.0.2, we can make use of API to load multiple certificate chains into
> a CTX. Prior to 1.0.2, you could not load the chains. So the checks ensure
> that we make use of the new APIs, and can then serve the correct
> certificate to users based on the user¹s cipher suite.
I see, but if the feature is only half-supported for openssl version < 1.0.2 we 
should simply avoid to use it on these versions.

> 
> -Dave
> 
Anyway, I admit i'm a little scared with your latest patches, you re-code a 
large part of certificates loading and i'm currently unable to guarantee that 
there is no regressions on linked features (SNI, ocsp, crt_list, passphrase 
management).

The first proposal was very less intrusive (the ones which don't used 1.0.2 API 
but secret callback). But i understand the point of view of the experts of the 
mailing list .

Did you expore an intermediate solution:

Continue to load keys/certificates etc in one new allocated SSL_CTX and when 
you fill the SNI tree, if the certificate comes into collision with an other, 
extract key/cert/chain from the early allocated SSL_CTX to fill/complete the 
already existing one? I don't know
if Openssl API provide enougth primives to do that. But it will certainly have 
less impact on the existing behavior.

R,
Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-08-21 Thread Remi Gacogne

Hi,

I haven't had time to look at the entire patch set, so this is just some
remarks on the points that Emeric mentioned.

>> Code initially use 'ctx->default_passwd_callback_userdata' to allow us to
>> manage a further way to manage passphrase via configuration.
> 
> As far as I could tell, this field was not getting set anywhere in the
> code. It¹s set with SSL_CTX_set_default_passwd_cb, which I did not find in
> the codebase. I had then assumed that this was just a harmless copy/paste
> of example code, which generally always use this CB.

I faced the same issue with the patch I sent earlier to provide support
for OpenSSL 1.1.x, because the default_passwd_callback_userdata can't be
accessed anymore (opaque struct). I couldn't find a place where
callback/callback_userdata are used in the code either, so I just
replaced them with NULL, which is equivalent when the callback is not set.
Note that PEM_read_bio_PrivateKey accepts a pem_password_cb and the
associated user data, so I don't think it's an issue if we want to
handle password-protected key in a different way in the future.

>> I notice also you continue to load DH parameters for each files. It was
>> not a big deal by the past because for each file correspond a uniq
>> certificate.
>>
>> With your patch, i don't know what will be the behavior. In the way it is
>> loaded on SSL_CTX, the DH parameter doesn't seem specific to the used
>> DSA/RSA/ECDSA certificate. So which one will be used, first or latest
>> loaded?
>>
>> Some users will expect to use the DH parameter defined in the same file
>> than the used certificate, but i'm really not sure it will be the case.
> 
> Unsure about this. I can change it to only load the first time the context
> is created.

The last DH parameters set with SSL_CTX_set_tmp_dh() will be used,
because the DSA/RSA/ECDSA certs will share the same SSL_CTX (that's the
whole point actually) and OpenSSL only supports one in a SSL_CTX.
We could change the way DH parameters are loaded for a bind, splitting
the certificate and the DH parameters in separate files. It would be
clearer but would not play nice with existing configuration.
Maybe we should just issue a warning when we detect that different DH
parameters are being loaded on the same SSL_CTX (ie, check in
ssl_sock_load_dh_params() if the SSL_CTX already has a tmp_dh, and if so
check if it's a different one)?





signature.asc
Description: OpenPGP digital signature

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hey Willy


>
>FWIW, some users ask that we can load a cert's passphrase from a file,
>I was supposed to look at this but didn't have time, maybe the callback
>above would be what will be needed for this, though I have no idea yet.
>
>Willy
>

If that¹s the case, then we should definitely include the call to the
CTX¹s callbacks, I can include this as part of my next changeset and that
way the infrastructure will be present for the future.

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Hi Dave,

On Fri, Aug 21, 2015 at 01:51:00PM +, Dave Zhu (yanbzhu) wrote:
> >Code initially use 'ctx->default_passwd_callback_userdata' to allow us to
> >manage a further way to manage passphrase via configuration.
> 
> As far as I could tell, this field was not getting set anywhere in the
> code. It¹s set with SSL_CTX_set_default_passwd_cb, which I did not find in
> the codebase. I had then assumed that this was just a harmless copy/paste
> of example code, which generally always use this CB.

FWIW, some users ask that we can load a cert's passphrase from a file,
I was supposed to look at this but didn't have time, maybe the callback
above would be what will be needed for this, though I have no idea yet.

Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching


Emeric,
>
>Code initially use 'ctx->default_passwd_callback_userdata' to allow us to
>manage a further way to manage passphrase via configuration.

As far as I could tell, this field was not getting set anywhere in the
code. It¹s set with SSL_CTX_set_default_passwd_cb, which I did not find in
the codebase. I had then assumed that this was just a harmless copy/paste
of example code, which generally always use this CB.

>
>I notice also you continue to load DH parameters for each files. It was
>not a big deal by the past because for each file correspond a uniq
>certificate.
>
>With your patch, i don't know what will be the behavior. In the way it is
>loaded on SSL_CTX, the DH parameter doesn't seem specific to the used
>DSA/RSA/ECDSA certificate. So which one will be used, first or latest
>loaded?
>
>Some users will expect to use the DH parameter defined in the same file
>than the used certificate, but i'm really not sure it will be the case.


Unsure about this. I can change it to only load the first time the context
is created.


>
>Finally, i don't understand what will be the behavior about the
>certificate chain with openssl < 1.0.2. I see you manage to load the
>chain differently depending the version of openssl but i ignore if the
>behavior will differ.


In 1.0.2, we can make use of API to load multiple certificate chains into
a CTX. Prior to 1.0.2, you could not load the chains. So the checks ensure
that we make use of the new APIs, and can then serve the correct
certificate to users based on the user¹s cipher suite.

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On 08/21/2015 02:53 PM, Willy Tarreau wrote:
> Guys,
> 
> I'm reviving this thread here. It turns out that Emeric had some concerns
> about possible impacts of the patch, maybe in part due to the openssl API
> though I'm not sure I understood everything right. I found it more efficient
> to involve all people who expressed opinions in this thread so that we end
> up with a solution which satisfies everyone.
> 
> Thanks,
> Willy
> 
> 

Yeah,

I will be easier to understand with the latest submitted patchset.

I've already made some comments to Dave in private. but i need your advice.

Dave,

I've just noticed you loose the management of the password callback reading the 
BIO file. I'm not sure if there is an impact on passphrase protected files.

Code initially use 'ctx->default_passwd_callback_userdata' to allow us to 
manage a further way to manage passphrase via configuration.

I notice also you continue to load DH parameters for each files. It was not a 
big deal by the past because for each file correspond a uniq certificate.

With your patch, i don't know what will be the behavior. In the way it is 
loaded on SSL_CTX, the DH parameter doesn't seem specific to the used 
DSA/RSA/ECDSA certificate. So which one will be used, first or latest loaded?

Some users will expect to use the DH parameter defined in the same file than 
the used certificate, but i'm really not sure it will be the case.

Finally, i don't understand what will be the behavior about the certificate 
chain with openssl < 1.0.2. I see you manage to load the chain differently 
depending the version of openssl but i ignore if the behavior will differ.

Emeric



>From ab919d36c8d0665602eb5797d508f07ffd24de3b Mon Sep 17 00:00:00 2001
From: yanbzhu 
Date: Thu, 13 Aug 2015 13:42:17 -0400
Subject: MINOR: ssl: Added keytype fields to sni_ctx

Added KeyType fields to sni_ctx and added functions to get and set these
fields from the sni_text. This will be used in conjuction with a later
commit to support multiple key types in a single context.
---
 include/types/ssl_sock.h |3 +++
 src/ssl_sock.c   |   41 +
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index e71ba79..dcf00fc 100644
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -29,6 +29,9 @@ struct sni_ctx {
 	SSL_CTX *ctx; /* context associated to the certificate */
 	int order;/* load order for the certificate */
 	int neg;  /* reject if match */
+	short has_rsa_cert;   /* 1 if CTX contains an RSA certificate */
+	short has_dsa_cert;   /* 1 if CTX contains an DSA certificate */
+	short has_ecc_cert;   /* 1 if CTX contains an ECC certificate */
 	struct ebmb_node name;/* node holding the servername value */
 };
 
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 2b91eed..37898d7 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1505,6 +1505,44 @@ end:
 }
 #endif
 
+/* Updates a sni_ctx* to indicate that it contains a pkey/cert pair
+ * of the given keytype
+ */
+static void ssl_sock_update_sni_ctx_keytype(struct sni_ctx *sc, int cert_keytype)
+{
+	switch(cert_keytype){
+	case EVP_PKEY_RSA:
+		sc->has_rsa_cert = 1;
+		break;
+	case EVP_PKEY_DSA:
+		sc->has_dsa_cert = 1;
+		break;
+	case EVP_PKEY_EC:
+		sc->has_ecc_cert = 1;
+		break;
+	}
+}
+
+/* Checks a ebmb_node to see if the sni_ctx inside has a cert of the
+ * given keytype
+ *
+ * Returns:
+ * 		1 If found
+ * 		0 if not found
+ */
+static int ssl_sock_node_contains_keytype(struct ebmb_node *node, int keytype){
+	switch(keytype){
+	case EVP_PKEY_RSA:
+		return container_of(node, struct sni_ctx, name)->has_rsa_cert;
+	case EVP_PKEY_DSA:
+		return container_of(node, struct sni_ctx, name)->has_dsa_cert;
+	case EVP_PKEY_EC:
+		return container_of(node, struct sni_ctx, name)->has_ecc_cert;
+	}
+	return 0;
+
+}
+
 static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char *name, int order)
 {
 	struct sni_ctx *sc;
@@ -1531,6 +1569,9 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char *name,
 		sc->ctx = ctx;
 		sc->order = order++;
 		sc->neg = neg;
+		sc->has_rsa_cert = 0;
+		sc->has_dsa_cert = 0;
+		sc->has_ecc_cert = 0;
 		if (wild)
 			ebst_insert(&s->sni_w_ctx, &sc->name);
 		else
-- 
1.7.1

>From 118d28066fd5856c92c91fd35da98ba4b0e6f132 Mon Sep 17 00:00:00 2001
From: yanbzhu 
Date: Thu, 13 Aug 2015 13:53:47 -0400
Subject: MINOR: ssl: Added cert_key_and_chain struct

Added cert_key_and_chain struct to ssl. This struct will store the
contents of a crt path (from the config file) into memory. This will
allow us to use the data stored in memory instead of reading the file
multiple times.

This will be used to support a later commit to load multiple pkeys/certs
into a single SSL_CTX
---
 src/ssl_sock.c |  124 
 1 files changed, 124 insertions(+), 0 dele

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

Guys,

I'm reviving this thread here. It turns out that Emeric had some concerns
about possible impacts of the patch, maybe in part due to the openssl API
though I'm not sure I understood everything right. I found it more efficient
to involve all people who expressed opinions in this thread so that we end
up with a solution which satisfies everyone.

Thanks,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-28 Thread Emeric Brun

> 
> I¹ve corrected the indentation to use tabs instead of spaces. Here¹s the
> new diff:
> 
> Is there a better method to do code reviews other than using the mailing
> list? I¹ll use whatever is easiest for you guys.
> 
> Again, sorry for the delayed response. I¹ll track the website from now on,
> -Dave
> 


Thank you Dave,

Could you re-post your patch using an attachment. Your mailer inserted some 
line returns and it's quite difficult to extract.

R,
Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-14 Thread Willy Tarreau

Hi Dave,

On Tue, Jul 14, 2015 at 03:38:47PM +, Dave Zhu (yanbzhu) wrote:
> >Emeric responded here:
> >http://marc.info/?l=haproxy&m=143643724320705&w=2
> >
> >Not sure what you mean by pushing this to master...?
> >
> >
> >
> >Lukas
> >
> 
> I¹ve corrected the indentation to use tabs instead of spaces. Here¹s the
> new diff:

Thank you. BTW, your mailer is wrapping lines. It remains readable for this
small patch but can become bothering in the future and will definitely prevent
the patch from being applied (which was Emeric's problem since he couldn't
apply it to review the code). If your mailer is problematic (which happens
with some of them), just join the patch as a text attachment, it will work
well and still be usable.

> Is there a better method to do code reviews other than using the mailing
> list? I¹ll use whatever is easiest for you guys.

E-mails to the list with relevant people in CC is the best method, as anyone
can spot anything there, propose some adjustments inline or alternatives.

> Again, sorry for the delayed response. I¹ll track the website from now on,

If you didn't receive Emeric's response, do you want me to send you a private
e-mail from my work address so see if there's anything related to the domain
itself ?

Cheers,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-14 Thread Dave Zhu (yanbzhu)

>Emeric responded here:
>http://marc.info/?l=haproxy&m=143643724320705&w=2
>
>Not sure what you mean by pushing this to master...?
>
>
>
>Lukas
>

I¹ve corrected the indentation to use tabs instead of spaces. Here¹s the
new diff:

Is there a better method to do code reviews other than using the mailing
list? I¹ll use whatever is easiest for you guys.

Again, sorry for the delayed response. I¹ll track the website from now on,
-Dave

diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index e71ba79..dcf00fc 100644
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -29,6 +29,9 @@ struct sni_ctx {
SSL_CTX *ctx; /* context associated to the certificate */
int order;/* load order for the certificate */
int neg;  /* reject if match */
+   short has_rsa_cert;   /* 1 if CTX contains an RSA certificate */
+   short has_dsa_cert;   /* 1 if CTX contains an DSA certificate */
+   short has_ecc_cert;   /* 1 if CTX contains an ECC certificate */
struct ebmb_node name;/* node holding the servername value */
 };
 
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 4e75549..ccf3251 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1505,7 +1505,23 @@ end:
 }
 #endif
 
-static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
*name, int order)
+static void ssl_sock_update_sni_ctx_keytype(struct sni_ctx *sc, int
cert_keytype)
+{
+   switch(cert_keytype){
+   case EVP_PKEY_RSA:
+   sc->has_rsa_cert = 1;
+   break;
+   case EVP_PKEY_DSA:
+   sc->has_dsa_cert = 1;
+   break;
+   case EVP_PKEY_EC:
+   sc->has_ecc_cert = 1;
+   break;
+   }
+
+}
+
+static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
*name, int order, int ctx_cert_keytype)
 {
struct sni_ctx *sc;
int wild = 0, neg = 0;
@@ -1531,6 +1547,12 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
struct bind_conf *s, char *name,
sc->ctx = ctx;
sc->order = order++;
sc->neg = neg;
+   sc->has_rsa_cert = 0;
+   sc->has_dsa_cert = 0;
+   sc->has_ecc_cert = 0;
+
+   ssl_sock_update_sni_ctx_keytype(sc,ctx_cert_keytype);
+
if (wild)
ebst_insert(&s->sni_w_ctx, &sc->name);
else
@@ -1539,21 +1561,17 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
struct bind_conf *s, char *name,
return order;
 }
 
+#if OPENSSL_VERSION_NUMBER < 0x1000200fL
+
 /* Loads a certificate key and CA chain from a file. Returns 0 on error,
-1 if
  * an early error happens and the caller must call SSL_CTX_free() by
itelf.
  */
-static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
struct bind_conf *s, char **sni_filter, int fcount)
+static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
struct bind_conf *s)
 {
BIO *in;
X509 *x = NULL, *ca;
-   int i, err;
+   int err;
int ret = -1;
-   int order = 0;
-   X509_NAME *xname;
-   char *str;
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-   STACK_OF(GENERAL_NAME) *names;
-#endif
 
in = BIO_new(BIO_s_file());
if (in == NULL)
@@ -1566,37 +1584,6 @@ static int ssl_sock_load_cert_chain_file(SSL_CTX
*ctx, const char *file, struct
if (x == NULL)
goto end;
 
-   if (fcount) {
-   while (fcount--)
-   order = ssl_sock_add_cert_sni(ctx, s, 
sni_filter[fcount], order);
-   }
-   else {
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-   names = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
-   if (names) {
-   for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
-   GENERAL_NAME *name = 
sk_GENERAL_NAME_value(names, i);
-   if (name->type == GEN_DNS) {
-   if (ASN1_STRING_to_UTF8((unsigned char 
**)&str, name->d.dNSName) >=
0) {
-   order = 
ssl_sock_add_cert_sni(ctx, s, str, order);
-   OPENSSL_free(str);
-   }
-   }
-   }
-   sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
-   }
-#endif /* SSL_CTRL_SET_TLSEXT_HOSTNAME */
-   xname = X509_get_subject_name(x);
-   i = -1;
-   while ((i = X509_NAME_get_index_by_NID(xname, NID_commonName, 
i)) !=
-1) {
-   X509_NAME_ENTRY *entry = X509_NAME_get_entry(xname, i);
-   if (ASN1_STRING_to_UTF8((unsigned char **)&str, 
entry->value) >= 0) {
-   order = ssl_sock_add_cert_sni(ctx, s, str, 
order);
-

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-14 Thread Dave Zhu (yanbzhu)

>
>Emeric responded here:
>http://marc.info/?l=haproxy&m=143643724320705&w=2
>
>Not sure what you mean by pushing this to master...?
>
>
>
>Lukas
>

I¹m so sorry for that. I did not get that email. Even though I am
subscribed to the list. IT must be filtering it somehow.

I¹ll use the website from now on.

I¹ll fix the indentations

-Dave

RE: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-14 Thread Lukas Tribus

> Hey guys,
>
> I haven’t gotten any feedback for this feature. Unless there’s severe
> objections, I’ll go ahead and push this to up to master.

Emeric responded here:
http://marc.info/?l=haproxy&m=143643724320705&w=2

Not sure what you mean by pushing this to master...?



Lukas

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-14 Thread Dave Zhu (yanbzhu)

Hey guys,

I haven’t gotten any feedback for this feature. Unless there’s severe
objections, I’ll go ahead and push this to up to master.

Thanks,
-Dave

On 7/7/15, 3:21 PM, "Dave Zhu (yanbzhu)"  wrote:

>Hey guys,
>
>Sorry for the radio silence, other projects picked up and so I was unable
>to put any time into this until now.
>
>I¹ve gotten the feature to work against OpenSSL 1.0.2. I am able to do it
>without peeking into any internal OpenSSL #defines and without using any
>additional callbacks.
>
>However, I did have to restructure the SNI and CTX processing logic.
>
>Here¹s a TL;DR:
>
>Previously - SSL Context was created and populated with cert/pkey/chain,
>then it is inserted into the SNI trees based on the CN/SANs.
>
>New - HAProxy will iterate through the CN/SAN¹s and determine whether or
>not a new CTX is needed based on existing entries in the tree. If an
>existing entry is found and does not contain the key type, HAProxy will
>add the new cert/pkey into the existing context. If no existing context is
>found a new context is created and populated. Only a single new context
>will be created for each file, regardless of how many CN¹s and SAN¹s.
>
>Let me know if there¹s a better way for you guys to code review it.
>
>Here is the diff:
>
>diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
>index e71ba79..dcf00fc 100644
>--- a/include/types/ssl_sock.h
>+++ b/include/types/ssl_sock.h
>@@ -29,6 +29,9 @@ struct sni_ctx {
>   SSL_CTX *ctx; /* context associated to the certificate */
>   int order;/* load order for the certificate */
>   int neg;  /* reject if match */
>+  short has_rsa_cert;   /* 1 if CTX contains an RSA certificate */
>+  short has_dsa_cert;   /* 1 if CTX contains an DSA certificate */
>+  short has_ecc_cert;   /* 1 if CTX contains an ECC certificate */
>   struct ebmb_node name;/* node holding the servername value */
> };
> 
>diff --git a/src/ssl_sock.c b/src/ssl_sock.c
>index 3bd6fa2..aca0b03 100644
>--- a/src/ssl_sock.c
>+++ b/src/ssl_sock.c
>@@ -1285,7 +1285,23 @@ end:
> }
> #endif
> 
>-static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
>*name, int order)
>+static void ssl_sock_update_sni_ctx_keytype(struct sni_ctx *sc, int
>cert_keytype)
>+{
>+switch(cert_keytype){
>+case EVP_PKEY_RSA:
>+sc->has_rsa_cert = 1;
>+break;
>+case EVP_PKEY_DSA:
>+sc->has_dsa_cert = 1;
>+break;
>+case EVP_PKEY_EC:
>+sc->has_ecc_cert = 1;
>+break;
>+}
>+
>+}
>+
>+static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
>*name, int order, int ctx_cert_keytype)
> {
>   struct sni_ctx *sc;
>   int wild = 0, neg = 0;
>@@ -1311,6 +1327,12 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
>struct bind_conf *s, char *name,
>   sc->ctx = ctx;
>   sc->order = order++;
>   sc->neg = neg;
>+sc->has_rsa_cert = 0;
>+sc->has_dsa_cert = 0;
>+sc->has_ecc_cert = 0;
>+
>+ssl_sock_update_sni_ctx_keytype(sc,ctx_cert_keytype);
>+
>   if (wild)
>   ebst_insert(&s->sni_w_ctx, &sc->name);
>   else
>@@ -1319,21 +1341,17 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
>struct bind_conf *s, char *name,
>   return order;
> }
> 
>+#if OPENSSL_VERSION_NUMBER < 0x1000200fL
>+
> /* Loads a certificate key and CA chain from a file. Returns 0 on error,
>-1 if
>  * an early error happens and the caller must call SSL_CTX_free() by
>itelf.
>  */
>-static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
>struct bind_conf *s, char **sni_filter, int fcount)
>+static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
>struct bind_conf *s)
> {
>   BIO *in;
>   X509 *x = NULL, *ca;
>-  int i, err;
>+  int err;
>   int ret = -1;
>-  int order = 0;
>-  X509_NAME *xname;
>-  char *str;
>-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
>-  STACK_OF(GENERAL_NAME) *names;
>-#endif
> 
>   in = BIO_new(BIO_s_file());
>   if (in == NULL)
>@@ -1346,37 +1364,6 @@ static int ssl_sock_load_cert_chain_file(SSL_CTX
>*ctx, const char *file, struct
>   if (x == NULL)
>   goto end;
> 
>-  if (fcount) {
>-  while (fcount--)
>-  order = ssl_sock_add_cert_sni(ctx, s, 
>sni_filter[fcount], order);
>-  }
>-  else {
>-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
>-  names = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
>-  if (names) {
>-  for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
>-  GENERAL_NAME *name = 
>sk_GENERAL_NAME_value(names, i);
>-  if (name->type == GEN_DNS) {
>-  if (ASN1_STRING_to_UTF8((unsigned char 
>**)&str, name->d.dNSName) >=
>0) {
>-

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-09 Thread Emeric Brun

Hi Dave,

Could you re-indent your code. You've created a lot of unnecessary diffs in 
your patch and there is too much interferences for a clean review.


i.e.

On 07/07/2015 09:21 PM, Dave Zhu (yanbzhu) wrote:
> - if (bind_conf->default_ctx) {
> - memprintf(err, "%sthis version of openssl cannot load multiple 
> SSL
> certificates.\n",
> -   err && *err ? *err : "");
> - return 1;
> - }
> +if (bind_conf->default_ctx) {
> +memprintf(err, "%sthis version of openssl cannot load
> multiple SSL certificates.\n",
> +err && *err ? *err : "");
> +goto end;
> +}

R,
Emeric

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-07-07 Thread Dave Zhu (yanbzhu)

Hey guys,

Sorry for the radio silence, other projects picked up and so I was unable
to put any time into this until now.

I¹ve gotten the feature to work against OpenSSL 1.0.2. I am able to do it
without peeking into any internal OpenSSL #defines and without using any
additional callbacks.

However, I did have to restructure the SNI and CTX processing logic.

Here¹s a TL;DR:

Previously - SSL Context was created and populated with cert/pkey/chain,
then it is inserted into the SNI trees based on the CN/SANs.

New - HAProxy will iterate through the CN/SAN¹s and determine whether or
not a new CTX is needed based on existing entries in the tree. If an
existing entry is found and does not contain the key type, HAProxy will
add the new cert/pkey into the existing context. If no existing context is
found a new context is created and populated. Only a single new context
will be created for each file, regardless of how many CN¹s and SAN¹s.

Let me know if there¹s a better way for you guys to code review it.

Here is the diff:

diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index e71ba79..dcf00fc 100644
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -29,6 +29,9 @@ struct sni_ctx {
SSL_CTX *ctx; /* context associated to the certificate */
int order;/* load order for the certificate */
int neg;  /* reject if match */
+   short has_rsa_cert;   /* 1 if CTX contains an RSA certificate */
+   short has_dsa_cert;   /* 1 if CTX contains an DSA certificate */
+   short has_ecc_cert;   /* 1 if CTX contains an ECC certificate */
struct ebmb_node name;/* node holding the servername value */
 };
 
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 3bd6fa2..aca0b03 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1285,7 +1285,23 @@ end:
 }
 #endif
 
-static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
*name, int order)
+static void ssl_sock_update_sni_ctx_keytype(struct sni_ctx *sc, int
cert_keytype)
+{
+switch(cert_keytype){
+case EVP_PKEY_RSA:
+sc->has_rsa_cert = 1;
+break;
+case EVP_PKEY_DSA:
+sc->has_dsa_cert = 1;
+break;
+case EVP_PKEY_EC:
+sc->has_ecc_cert = 1;
+break;
+}
+
+}
+
+static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
*name, int order, int ctx_cert_keytype)
 {
struct sni_ctx *sc;
int wild = 0, neg = 0;
@@ -1311,6 +1327,12 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
struct bind_conf *s, char *name,
sc->ctx = ctx;
sc->order = order++;
sc->neg = neg;
+sc->has_rsa_cert = 0;
+sc->has_dsa_cert = 0;
+sc->has_ecc_cert = 0;
+
+ssl_sock_update_sni_ctx_keytype(sc,ctx_cert_keytype);
+
if (wild)
ebst_insert(&s->sni_w_ctx, &sc->name);
else
@@ -1319,21 +1341,17 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
struct bind_conf *s, char *name,
return order;
 }
 
+#if OPENSSL_VERSION_NUMBER < 0x1000200fL
+
 /* Loads a certificate key and CA chain from a file. Returns 0 on error,
-1 if
  * an early error happens and the caller must call SSL_CTX_free() by
itelf.
  */
-static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
struct bind_conf *s, char **sni_filter, int fcount)
+static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
struct bind_conf *s)
 {
BIO *in;
X509 *x = NULL, *ca;
-   int i, err;
+   int err;
int ret = -1;
-   int order = 0;
-   X509_NAME *xname;
-   char *str;
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-   STACK_OF(GENERAL_NAME) *names;
-#endif
 
in = BIO_new(BIO_s_file());
if (in == NULL)
@@ -1346,37 +1364,6 @@ static int ssl_sock_load_cert_chain_file(SSL_CTX
*ctx, const char *file, struct
if (x == NULL)
goto end;
 
-   if (fcount) {
-   while (fcount--)
-   order = ssl_sock_add_cert_sni(ctx, s, 
sni_filter[fcount], order);
-   }
-   else {
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-   names = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
-   if (names) {
-   for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
-   GENERAL_NAME *name = 
sk_GENERAL_NAME_value(names, i);
-   if (name->type == GEN_DNS) {
-   if (ASN1_STRING_to_UTF8((unsigned char 
**)&str, name->d.dNSName) >=
0) {
-   order = 
ssl_sock_add_cert_sni(ctx, s, str, order);
-   OPENSSL_free(str);
-   }
-   }
-   }
-   sk_GENERAL_NAME_pop_free(names, GENERAL

RE: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-25 Thread Lukas Tribus

> Thank you for pointing this out, I missed it in my brief look of the code.
> To me, this is reason enough to move to 1.0.2 (in addition to all the
> other reasons given by you and Nenad).
>
> I¹ll start prototyping the code using 1.0.2.

Agreed.

What I would also urge is to not use any openssl internals at all. We
already have a few forward compatibility issues with openssl (haproxy
linked with -DOPENSSL_NO_SSL_INTERN against current stable openssl
or linking against the openssl 1.1.0 branch).

Openssl 1.1.0 is expected to be released by the end of 2015, we should
try hard to not introduce new compatibility issues - which mostly comes
from accessing openssl internals. Of course we can't predict API breakage,
but we do already know that direct access to internal APIs will no longer
be possible.


Thanks for this work, Dave, its much appreciated!


Regards,
Lukas

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-25 Thread Dave Zhu (yanbzhu)

Hey,

>Yes, sorry, I didn't realize it earlier but that's not true for all
>OpenSSL versions. Starting with OpenSSL 1.0.0, tls1_process_ticket()
>will decline decrypting session tickets sent by the client if the
>session_secret_cb is in use:
>
>if (s->tls_session_secret_cb)
>{
>/* Indicate cache miss here and instead
>of
>
> * generating the session from ticket
>now,
>
> * trigger abbreviated handshake based
>on
>
> * external mechanism to calculate the
>master
>
> * secret later. */
>return 0;
>}
>
>There is even a nice comment about it, starting from 1.0.1 I believe:
>
> * If s->tls_session_secret_cb is set then we are expecting a pre-shared
>key
>
> * ciphersuite, in which case we have no use for session tickets and one
>will
>
> * never be decrypted, nor will s->tlsext_ticket_expected be set to 1.

Thank you for pointing this out, I missed it in my brief look of the code.
To me, this is reason enough to move to 1.0.2 (in addition to all the
other reasons given by you and Nenad).

I¹ll start prototyping the code using 1.0.2.

Just as a final point of clarification, I wasn¹t suggesting that we always
use the client¹s list as the preference for all the operations. The server
would still have chosen the cipher suite based on its priority list. We
would just limit the Signature Algorithm to one that the client prefers.
However, if we switch to 1.0.2, this is a moot point.

Thanks all for your feedback.

Are there any other comments regarding requiring 1.0.2 for this feature?

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-25 Thread Nenad Merdanovic

Hello,

Everything said here is based on my opinion, so just add "IMO" in front
of every sentence :)

On 6/25/2015 6:01 PM, Remi Gacogne wrote:
> Hi,
> 
>> I was unaware that BoringSSL removed the callback, but in that case, could
>> we limit this feature to only OpenSSL? I¹m also not seeing how using this
>> callback prevents rfc5077, could you please elaborate.

Although we are expecting to gain a lot on server side CPU usage, we
absolutely must not disable TLS session tickets as there are many
clients not supporting EC.

> 
> Yes, choosing a common suite supported by both side is a necessity. But
> when there is more than one common suite, which happens most of the
> time, you can either follow the client's preference or the server one.
> Right now, it seems that we have a consensus to follow the server's
> choice (see ssl_prefer_server_ciphers on for nginx, SSLHonorCipherOrder
> on for Apache HTTPd, ..) and I believe we should continue to do that in
> HAproxy because legacy clients have a long history of choosing crappy
> ciphersuite (look at the recent export fiascos, for example).

I also consider this best practice and would like to keep current behavior.

> 
>> Tying this feature into 1.0.2 would definitely make it easier, I agree. It
>> just will hinder adoption.
> 
> That's true, but I am afraid doing otherwise would require adding a
> complex logic in the TLS stack of HAproxy, so sadly I am more enclined
> to require 1.0.2 for people willing to use this feature.

OpenSSL being pretty complex, I prefer this approach also. People who
want best performance will go with 1.0.2 anyways due to performance
improvements. For example, RSA2048 on 1.0.1e gives 850 signs/s, while
1.0.2c gives 1470 signs/s (Xeon v3 CPU).

Just my 2c.

Regards,
Nenad

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-25 Thread Remi Gacogne

Hi,

> I was unaware that BoringSSL removed the callback, but in that case, could
> we limit this feature to only OpenSSL? I¹m also not seeing how using this
> callback prevents rfc5077, could you please elaborate.

Yes, sorry, I didn't realize it earlier but that's not true for all
OpenSSL versions. Starting with OpenSSL 1.0.0, tls1_process_ticket()
will decline decrypting session tickets sent by the client if the
session_secret_cb is in use:

if (s->tls_session_secret_cb)
{
/* Indicate cache miss here and instead
of

 * generating the session from ticket
now,

 * trigger abbreviated handshake based
on

 * external mechanism to calculate the
master

 * secret later. */
return 0;
}

There is even a nice comment about it, starting from 1.0.1 I believe:

 * If s->tls_session_secret_cb is set then we are expecting a pre-shared
key

 * ciphersuite, in which case we have no use for session tickets and one
will

 * never be decrypted, nor will s->tlsext_ticket_expected be set to 1.

> I see that now. But in the current implementation, doesn¹t it always just
> use the default CTX if the client doesn¹t specify SNI? In that case, you¹d
> still be limited to the signature algorithm of the default CTX. It¹s
> possible to take the server preferences in mind. However, wouldn¹t that
> lead to a situation where the server wants to use Signature Algorithm A,
> but the client only supports B, and then the client can¹t connect?

Yes, of course, but remember that the client sends not one choice but an
ordered list of ciphersuites he is willing to use. If none of these is
accepted by the server, the connection will fail, true, but in practice
it seldom happens.

> In my mind, choosing the signature algorithm based on what the client
> supports is a good idea as it guarantees that the client can use that
> algorithm. Then, the server can use w/e cipher suite it wants in terms of
> Key-Exchange and symmetric-encryption.

Yes, choosing a common suite supported by both side is a necessity. But
when there is more than one common suite, which happens most of the
time, you can either follow the client's preference or the server one.
Right now, it seems that we have a consensus to follow the server's
choice (see ssl_prefer_server_ciphers on for nginx, SSLHonorCipherOrder
on for Apache HTTPd, ..) and I believe we should continue to do that in
HAproxy because legacy clients have a long history of choosing crappy
ciphersuite (look at the recent export fiascos, for example).

> Tying this feature into 1.0.2 would definitely make it easier, I agree. It
> just will hinder adoption.

That's true, but I am afraid doing otherwise would require adding a
complex logic in the TLS stack of HAproxy, so sadly I am more enclined
to require 1.0.2 for people willing to use this feature.





signature.asc
Description: OpenPGP digital signature

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-25 Thread Dave Zhu (yanbzhu)

On 6/25/15, 5:17 AM, "Remi Gacogne"  wrote:

Hey Remy, thanks for your feedback!

>
>However, I have some concerns about the use of
>SSL_set_session_secret_cb() for this feature, because it was clearly not
>designed for this kind of manipulation. It has been removed from
>BoringSSL [1] and simply enabling this callback in OpenSSL, regardless
>of what is done in the callback itself, disables TLS session ticket
>(rfc5077) [2], which I believe is an issue.
>It also means that you have to use flags that are internals to OpenSSL,
>and may change in the future.

I was unaware that BoringSSL removed the callback, but in that case, could
we limit this feature to only OpenSSL? I¹m also not seeing how using this
callback prevents rfc5077, could you please elaborate.

I wholeheartedly agree with you that using internal OpenSSL flags is not
ideal, but the workaround would be to check every single cipher for the
Signature Algorithm. I could change it to use these if that¹s preferred,
but it will definitely bloat the code, plus is less maintainable.

Change in the future is definitely something that is a flaw to this
implementation. I¹m still trying to find a way around this one.

>Lastly, switching the SSL_CTX based only on the suites sent by the
>client in ClientHello means that you effectively bypass the suites
>preference set on HAproxy, if any. If the first suite sent by the client
>requires an ECDSA certificate, you will switch to a SSL_CTX supporting
>only ECDSA suites, even if the server preferences prefer some suites
>using RSA better than ECDSA ones. This is quite a change from the
>HAproxy default, which is to take into account the server preferences
>first [3].

I see that now. But in the current implementation, doesn¹t it always just
use the default CTX if the client doesn¹t specify SNI? In that case, you¹d
still be limited to the signature algorithm of the default CTX. It¹s
possible to take the server preferences in mind. However, wouldn¹t that
lead to a situation where the server wants to use Signature Algorithm A,
but the client only supports B, and then the client can¹t connect?

In my mind, choosing the signature algorithm based on what the client
supports is a good idea as it guarantees that the client can use that
algorithm. Then, the server can use w/e cipher suite it wants in terms of
Key-Exchange and symmetric-encryption.

Tying this feature into 1.0.2 would definitely make it easier, I agree. It
just will hinder adoption.

Thoughts?

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-25 Thread Remi Gacogne

Hi all,

Dave, thank you for proposing this feature, I truly think that being
able to serve RSA or ECDSA certificates depending on what the client
supports would be an awesome addition to HAproxy.

However, I have some concerns about the use of
SSL_set_session_secret_cb() for this feature, because it was clearly not
designed for this kind of manipulation. It has been removed from
BoringSSL [1] and simply enabling this callback in OpenSSL, regardless
of what is done in the callback itself, disables TLS session ticket
(rfc5077) [2], which I believe is an issue.
It also means that you have to use flags that are internals to OpenSSL,
and may change in the future. Note that this could probably be avoided
in 1.0.2+ with the use of ssl_cipher_get_cert_index().
Lastly, switching the SSL_CTX based only on the suites sent by the
client in ClientHello means that you effectively bypass the suites
preference set on HAproxy, if any. If the first suite sent by the client
requires an ECDSA certificate, you will switch to a SSL_CTX supporting
only ECDSA suites, even if the server preferences prefer some suites
using RSA better than ECDSA ones. This is quite a change from the
HAproxy default, which is to take into account the server preferences
first [3].

IMHO, I would tend to prefer letting OpenSSL handle both the RSA and
ECDSA pairs in the same SSL_CTX. It could probably be done when a new
certificate is loaded by looking up in the existing certificates list if
there is already is one with the same subject name / SAN and a different
key type, and merging it to the existing SSL_CTX if it does.
I do realize that it would only work with 1.0.2+ because of the
inability of previous OpenSSL version of supporting more than one chain
in a given SSL_CTX, though.

Just my two cents :)

[1]
https://boringssl.googlesource.com/boringssl/+/d1681e614f8c3b4105efc42fb1fd35bd3ec09547%5E!/
[2] see tls1_process_ticket(), in ssl/t1_lib.c
[3] the default ssloptions contains SSL_OP_CIPHER_SERVER_PREFERENCE, in
ssl_sock_prepare_ctx(), src/ssl_sock.c

signature.asc
Description: OpenPGP digital signature

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

On 6/24/15, 2:58 PM, "Willy Tarreau"  wrote:
>What I'm understanding here is that instead of using SNI only as the key
>to pick a cert, we want the (SNI, algo) combination. Coudln't we have two
>certs per SNI entry ? One in RSA form, the other one in ECDSA, and only
>provide what is supported ? We could even consider that whenever a client
>advertises support for ECDSA we should provide it first as it saves CPU
>cycles. I guess that RSA won't go away for a long time and might still
>be there in 10 years anyway, so selecting first on SNI and second on the
>algo with a fallback to the only cert present in the SNI for most cases
>where there's a single one sounds the right approach.

I agree that this is the best approach to take. I¹ll begin to shift my
implementation to do this. It currently does not take SNI into account.

>I could be wrong but I think that we have as many SSL_CTX as we load
>certs,
>and we pick the correct one based on the SNI, so it should probably not be
>an issue in our case with either version.

Yup, this is true. I¹ll have to do some more research on how the EB trees
work in HAProxy. I think the current code just looks for the first match,
whereas the new code should look for all matches and pick based on algo.


>BTW, please take care of surrounding your code with the appropriate ifdefs
>if you think you're using features from recent libs.

Yup, I¹m trying to be as mindful as I can about this.

Thanks for the feedback!

-Dave

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-24 Thread Willy Tarreau

On Wed, Jun 24, 2015 at 03:06:32PM +, Dave Zhu (yanbzhu) wrote:
> Hey Willy,
> 
> Lukas explained it pretty well, but I can expound on it some more.
> 
> You can imagine a situation where HAProxy has 2 certificates of different
> key types; one ECDSA and one RSA. In the current codebase, if no SNI is
> used, the certificate that is used will be whichever certificate is the
> default (i.e. the one that is first specified in the config). So we would
> have 2 possible paths:
> 
> 1. ECDSA was specified first. This has the effect of only supporting
> cipher suites that has ECDSA. If the client does not support ECDSA, then
> it would mean that the connection will fail, even though the server has an
> RSA certificate.
> 2. RSA was specified first. This means that ECDSA cipher suites would
> never be used, which can decrease performance for initial handshakes as
> well as have a negative security impact.
>
> What I propose would address both of these issues. If the client prefers
> RSA or only supports RSA, then the RSA certificate is presented. However,
> if the client supports ECDSA, then we would use the ECDSA certificate.

Yes I understand that thanks to Lukas' explanation. Indeed this is quite
interesting.

What I'm understanding here is that instead of using SNI only as the key
to pick a cert, we want the (SNI, algo) combination. Coudln't we have two
certs per SNI entry ? One in RSA form, the other one in ECDSA, and only
provide what is supported ? We could even consider that whenever a client
advertises support for ECDSA we should provide it first as it saves CPU
cycles. I guess that RSA won't go away for a long time and might still
be there in 10 years anyway, so selecting first on SNI and second on the
algo with a fallback to the only cert present in the SNI for most cases
where there's a single one sounds the right approach.

> Lukas,
> I believe the reason that apache calls out 1.0.2 is this line in the
> OpenSSL (1.0.1l to 1.0.2a) changelog:
> 
> * Add support for certificate stores in CERT structure. This makes it
> possible to have different stores per SSL structure or one store in the
> parent SSL_CTX. Include distint stores for certificate chain verification
> and chain building. New ctrl SSL_CTRL_BUILD_CERT_CHAIN to build and store
> a certificate chain in CERT structure: returing(sic) an error if the chain
> cannot be built: this will allow applications to test if a chain is
> correctly configured.
> 
> In openssl <= 1.0.1, we can load multiple certs/keys into a single
> SSL_CTX. However, they must all have the same cert-chain. I believe that
> this 1.0.2 feature addresses this issue via certificate stores, and so can
> then use the existing s3server cipher suite selection code to select the
> correct certificate/key inside the library. This would alleviate the need
> to hook into a callback, which is what I¹m doing here.

I could be wrong but I think that we have as many SSL_CTX as we load certs,
and we pick the correct one based on the SNI, so it should probably not be
an issue in our case with either version.

> The diff I submitted is coded to OpenSSL 1.0.1, and will still work in
> 1.0.2. The 1.0.2 change will make it easier to support this feature in
> combination with SNI.

We do handle SNI by ourselves right now. We have a callback from which
we look up the hostname in the cert tree and pick the best match (either
the perfect match or a matching wildcard). We can apply the logic we want
there.

> We¹ll have to discuss which approach we want to take
> going forward; 1.0.2 is still relatively young, and so requiring adopters
> to have that version may cause additional friction.

Absolutely, especially if we find that it doesn't bring any benefit over
what we currently have due to a different architecture.

BTW, please take care of surrounding your code with the appropriate ifdefs
if you think you're using features from recent libs. Older ones are still
supported (and used). 0.9.8 is still supported by openssl, and older ones
are still deployed at some places or in older LTS distros or systems which
do ship their own patched versions. For example, RHEL4 ships 0.9.7, along
with a 0.9.6 compatibility package to port old programs from RHEL2.1!

I must confess I stopped testing the 0.9.6 builds 2 years ago when my last
machine having it was replaced :-)

If you really don't know and mess a bit, that's not dramatic, because the
possible build issues will sooner or later be reported. But it's better to
be careful with this in mind before breaking things too much.

cheers,
Willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-24 Thread Willy Tarreau

On Wed, Jun 24, 2015 at 04:26:58PM +0200, Lukas Tribus wrote:
> Currently we mostly use RSA certificates. ECC (ECDSA) are different 
> certificates and
> until RSA certificates are fully removed from the industry, we will have to
> support both.
> 
> The change, if I understand correctly, allows serving the ECC/ECDSA 
> certificate
> when the client supports it (via ciphers list), and RSA otherwise.

Ah OK, I didn't know the certificates were different, now I understand and of
course it makes a lot of sense!

> Do we need this? Absolutely yes.

Yes indeed, at least to benefit form the extra performance brought by ECDSA.

> But we will have to verify exactly whats the
> best way to do this, and how openssl can help with this. I believe openssl 
> 1.0.2
> introduces a new API which makes things simpler.
> 
> Apache 2.4 can already do this, nginx not yet.

OK.

> Some discussions and further informations:
> 
> https://github.com/igrigorik/istlsfastyet.com/issues/38
> http://mailman.nginx.org/pipermail/nginx-devel/2013-October/004376.html
> https://blog.cloudflare.com/ecdsa-the-digital-signature-algorithm-of-a-better-internet/
> https://blog.joelj.org/2015/06/19/dual-rsaecdsa-certificates-in-apache-2-4/
> https://securitypitfalls.wordpress.com/2014/10/06/rsa-and-ecdsa-performance/

Wow, efficient as usual :-)

Thanks very much for the explanation Lukas.
willy

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-24 Thread Olivier

And a quick update on why supporting both (ECDSA / RSA) certificates :
first one is newer and only supported in latest browsers, but is really
interesting with heavy SSL load because it is much cheaper on CPU cost
server side (main work is done on client browser).

2015-06-24 18:53 GMT+02:00 Dave Zhu (yanbzhu) :

> No, I do not handle that case at the moment. This is the kind of feedback
> I¹m looking for so thanks!
>
> I will start modifying the code to try to accommodate this.
>
> Any thoughts on the version of OpenSSL to try to code to?
>
> -Dave
>
> On 6/24/15, 11:54 AM, "Lukas Tribus"  wrote:
>
> >
> >Does your code correctly handy ECC vs RSA intermediate certificates
> >in all cases?
> >
> >
> >Lukas
> >
>
>
>

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

I’ve coded up the functionality to check all of the intermediate
certificates to ensure that they match the private key of the crt file.
I’ve decided to toggle this functionality as a config option. Users can
either choose to disable this entire feature (default), match only the
private key/cert, or match the entire certificate chain.

Should I keep sending out diffs?

-Dave

On 6/24/15, 11:54 AM, "Lukas Tribus"  wrote:

>> Hey Willy,
>>
>> Lukas explained it pretty well, but I can expound on it some more.
>>
>> You can imagine a situation where HAProxy has 2 certificates of
>>different
>> key types; one ECDSA and one RSA. In the current codebase, if no SNI is
>> used, the certificate that is used will be whichever certificate is the
>> default (i.e. the one that is first specified in the config). So we
>>would
>> have 2 possible paths:
>>
>> 1. ECDSA was specified first. This has the effect of only supporting
>> cipher suites that has ECDSA. If the client does not support ECDSA, then
>> it would mean that the connection will fail, even though the server has
>>an
>> RSA certificate.
>> 2. RSA was specified first. This means that ECDSA cipher suites would
>> never be used, which can decrease performance for initial handshakes as
>> well as have a negative security impact.
>>
>> What I propose would address both of these issues. If the client prefers
>> RSA or only supports RSA, then the RSA certificate is presented.
>>However,
>> if the client supports ECDSA, then we would use the ECDSA certificate.
>>
>> Lukas,
>> I believe the reason that apache calls out 1.0.2 is this line in the
>> OpenSSL (1.0.1l to 1.0.2a) changelog:
>>
>> * Add support for certificate stores in CERT structure. This makes it
>> possible to have different stores per SSL structure or one store in the
>> parent SSL_CTX. Include distint stores for certificate chain
>>verification
>> and chain building. New ctrl SSL_CTRL_BUILD_CERT_CHAIN to build and
>>store
>> a certificate chain in CERT structure: returing(sic) an error if the
>>chain
>> cannot be built: this will allow applications to test if a chain is
>> correctly configured.
>>
>> In openssl <= 1.0.1, we can load multiple certs/keys into a single
>> SSL_CTX. However, they must all have the same cert-chain. I believe that
>> this 1.0.2 feature addresses this issue via certificate stores, and so
>>can
>> then use the existing s3server cipher suite selection code to select the
>> correct certificate/key inside the library. This would alleviate the
>>need
>> to hook into a callback, which is what I¹m doing here.
>
>Does your code correctly handy ECC vs RSA intermediate certificates
>in all cases?
>
>
>Lukas
>
>

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

No, I do not handle that case at the moment. This is the kind of feedback
I¹m looking for so thanks!

I will start modifying the code to try to accommodate this.

Any thoughts on the version of OpenSSL to try to code to?

-Dave

On 6/24/15, 11:54 AM, "Lukas Tribus"  wrote:

>
>Does your code correctly handy ECC vs RSA intermediate certificates
>in all cases?
>
>
>Lukas
>

RE: Contribution for HAProxy: Peer Cipher based SSL CTX switching

2015-06-24 Thread Lukas Tribus

> Hey Willy,
>
> Lukas explained it pretty well, but I can expound on it some more.
>
> You can imagine a situation where HAProxy has 2 certificates of different
> key types; one ECDSA and one RSA. In the current codebase, if no SNI is
> used, the certificate that is used will be whichever certificate is the
> default (i.e. the one that is first specified in the config). So we would
> have 2 possible paths:
>
> 1. ECDSA was specified first. This has the effect of only supporting
> cipher suites that has ECDSA. If the client does not support ECDSA, then
> it would mean that the connection will fail, even though the server has an
> RSA certificate.
> 2. RSA was specified first. This means that ECDSA cipher suites would
> never be used, which can decrease performance for initial handshakes as
> well as have a negative security impact.
>
> What I propose would address both of these issues. If the client prefers
> RSA or only supports RSA, then the RSA certificate is presented. However,
> if the client supports ECDSA, then we would use the ECDSA certificate.
>
> Lukas,
> I believe the reason that apache calls out 1.0.2 is this line in the
> OpenSSL (1.0.1l to 1.0.2a) changelog:
>
> * Add support for certificate stores in CERT structure. This makes it
> possible to have different stores per SSL structure or one store in the
> parent SSL_CTX. Include distint stores for certificate chain verification
> and chain building. New ctrl SSL_CTRL_BUILD_CERT_CHAIN to build and store
> a certificate chain in CERT structure: returing(sic) an error if the chain
> cannot be built: this will allow applications to test if a chain is
> correctly configured.
>
> In openssl <= 1.0.1, we can load multiple certs/keys into a single
> SSL_CTX. However, they must all have the same cert-chain. I believe that
> this 1.0.2 feature addresses this issue via certificate stores, and so can
> then use the existing s3server cipher suite selection code to select the
> correct certificate/key inside the library. This would alleviate the need
> to hook into a callback, which is what I¹m doing here.

Does your code correctly handy ECC vs RSA intermediate certificates
in all cases?


Lukas

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching