Fix small bug in srv_parse_agent_check

[Dirkjan Bussink]

Hi all,

I was building HAProxy using scan-build to see if there were any issues and it 
called out an unused variable write. I think this was due to a bug that the 
err_code was not used in srv_parse_agent_check.

I’ve attached a patch to fix this issue. 

Cheers,

Dirkjan Bussink



0001-BUG-MINOR-checks-return-correct-error-code-for-srv_p.patch
Description: Binary data

Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.

[Dirkjan Bussink]

Hi Willy,

On 22 Jan 2019, at 23:17, Willy Tarreau  wrote:
> 
> As you can see it will enable this code when SSL_OP_NO_RENEGOTIATION=0,
> which is what BoringSSL does and it needs this code to be disabled. Thus
> I think it's better to simply do this :
> 
> +#ifndef SSL_OP_NO_RENEGOTIATION
> + /* Please note that BoringSSL defines this macro to zero so don't
> +  * change this to #if and do not assign a default value to this macro!
> +  */
> 

Of course, you’re right. New version of the patch attached!

Cheers,

Dirkjan




0001-BUG-MEDIUM-ssl-Fix-handling-of-TLS-1.3-KeyUpdate-mes.patch
Description: Binary data

Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.

[Dirkjan Bussink]

Hi Willy,

> On 22 Jan 2019, at 07:07, Willy Tarreau  wrote:
> 
> Hi guys,
> 
> On Tue, Jan 22, 2019 at 03:22:38PM +0100, Emeric Brun wrote:
>> I think you can merge this.
> 
> OK. I still find it very fragile in that we usually don't make a
> difference between an absent define and the same declared as zero, and
> most SSL_OP_* entries are defined this way in ssl_sock.c, but I don't
> see that many other options here. I think that the #ifndef at least
> deserves a comment indicating that it may also match a zero value to
> detect safe implementations so that we are not tempted later to refactor
> this and break BoringSSL.
> 
> We can also add a Reported-By to ack Adam's original work on the issue.
> 
> Just let me know if I need to adjust it myself or if anyone wants to take
> care of it.

I have adjusted the patch to make it more robust and more match the style of 
how we use other options. How does this look to you?

Cheers,

Dirkjan



0001-BUG-MEDIUM-ssl-Fix-handling-of-TLS-1.3-KeyUpdate-mes.patch
Description: Binary data

Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.

[Dirkjan Bussink]

Hi Adam,

> On 21 Jan 2019, at 10:09, Adam Langley  wrote:
> 
> HAProxy isn't a user that we have on our radar, but BoringSSL dislikes
> pushing compatibility hacks into downstream projects. (You can always
> ask for these things to be included in BoringSSL instead.)

Ah ok, I recently added support in HAProxy to handle the new 
SSL_CTX_set_ciphersuites option since OpenSSL handles setting TLS 1.3 ciphers 
separate from the regular ones. Are those things that BoringSSL would also want 
to adopt then? 

Cheers,

Dirkjan Bussink

Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.

[Dirkjan Bussink]

Hi Manu,

> On 21 Jan 2019, at 09:49, Emmanuel Hocdet  wrote:
> 
> Boringssl does not have SSL_OP_NO_RENEGOTIATION and need KeyUpdate to work.
> As workaround, SSL_OP_NO_RENEGOTIATION could be set to 0 in openssl-compat.h.

Hmm, then we will need a different #define though since we can’t rely own the 
constant not being defined in that case to disable the logic. We would need a 
separate way to detect this then. Is there a good example of this or should I 
change the logic then to version checks instead? And how about LibreSSL in that 
case?

Does BoringSSL need any of the logic in the first place? There’s not really 
versions of it, so is the target there always current master or something else? 

Cheers,

Dirkjan

Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.

[Dirkjan Bussink]

Hi Emeric,

> On 21 Jan 2019, at 08:06, Emeric Brun  wrote:
> 
> Interesting, it would be good to skip the check using the same method.
> 
> We must stay careful to not put the OP_NO_RENEG flag on the client part (when 
> haproxy connects to server), because reneg from server is authorized
> but i think infocbk is called only on frontend/accept side.
> 
> so a patch which do:
> 
> #ifdef  SSL_OP_NO_RENEGOTIATION
> SSL_set_options(ctx, SSL_OP_NO_RENEGOTIATION);
> #endif
> 
> without condition during init
> 
> and adding #ifndef SSL_OP_NO_RENEGOTIATION arround the CVE check, should fix 
> the issue mentionned about keyupdate and will fix the CVE using the clean way 
> if the version
> of openssl support.

I have implemented this and attached the patch for it. What do you think of 
this approach? 

Cheers,

Dirkjan Bussink



0001-BUG-MEDIUM-ssl-Fix-handling-of-TLS-1.3-KeyUpdate-mes.patch
Description: Binary data

Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.

[Dirkjan Bussink]

Hi all,

> On 21 Jan 2019, at 02:01, Emeric Brun  wrote:
> 
> Is there a way to check this is a keyupdate message which trigger the 
> callback (and not an other)?

Sadly there is not. I had taken a look at the OpenSSL code and it triggers the 
callback without any additional information available at 
https://github.com/openssl/openssl/blob/OpenSSL_1_1_1a/ssl/statem/statem_srvr.c#L4059.
 

> And what happen for natural i/o operation, are they return something 
> receiving the message or is this completely
> hide by openssl (i.e. what returns a SSL_read/write for instance when a 
> keyupdate has just been received)

This is completely hidden by OpenSSL and as a library user you don’t see this 
message happened as it’s transparently updated. 

I think the other option to consider is that since this logic was added, 
OpenSSL has gotten support for flags to control renegotiation and prevent 
insecure renegotiation so the question is if we need this check still at all. I 
think it can be removed (which is also what NGinx did in the commit that Janusz 
mentioned in another email in this thread. From the commit message at 
https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c:

--- 

Following 7319:dcab86115261, as long as SSL_OP_NO_RENEGOTIATION is
defined, it is OpenSSL library responsibility to prevent renegotiation,
so the checks are meaningless.

Additionally, with TLSv1.3 OpenSSL tends to report SSL_CB_HANDSHAKE_START
at various unexpected moments - notably, on KeyUpdate? messages and
when sending tickets. This change prevents unexpected connection
close on KeyUpdate? messages and when finishing handshake with upcoming
early data changes.

--- 

Cheers,

Dirkjan Bussink

Re: [ANNOUNCE] haproxy-1.9-dev4

[Dirkjan Bussink]

Hi Willy,

> On 20 Nov 2018, at 16:19, Willy Tarreau  wrote:
> 
> Indeed it's already been two months, it would be the right time to emit
> a new one. But at the moment all the people able to work on this are
> fully loaded finishing their respective parts for 1.9 (or fixing it).
> Are you missing a specific fix at the moment ? I'm asking because the
> 1.8 queue doesn't look huge and most of the recently fixed bugs in 1.9
> do not affect 1.8.

Mostly looking for having my TLS 1.3 work in an official released version. 

Cheers,

Dirkjan

Re: [ANNOUNCE] haproxy-1.9-dev4

[Dirkjan Bussink]

Hi Willy,

> On 23 Oct 2018, at 14:48, Willy Tarreau  wrote:
> 
> You're right. I started backporting fixes for it last week. I think it
> would make sense to make one "soon" (maybe next week-end along dev5).
> In the mean time you can pick the latest maintenance snapshot if you
> want, it already contains your work.

Could I ask again for a release of 1.8 as well? 1.9 has seen a few more dev 
releases already and wondering if 1.8 can get one again?

Cheers,

Dirkjan

Re: [ANNOUNCE] haproxy-1.9-dev4

[Dirkjan Bussink]

Hi all,

> On 21 Oct 2018, at 21:05, Willy Tarreau  wrote:
> 
> The support for TLS 1.3 ciphersuites was merged. If you play with it,
> please report successes or failures, as this was backported to 1.8.
> Regarding TLS, certificates can now be generated on the fly on
> BoringSSL as well.

Is there any guess on a timeline for the next 1.8 release? Would love to play 
with the new TLS 1.3 options there then and test those out in our staging 
environments. 

Cheers,

Dirkjan

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi Lukas,

> On 7 Oct 2018, at 14:18, Lukas Tribus  wrote:
> 
> There is one space too much in the beginning of the penultimate line
> in the doc of "ssl-default-server-ciphersuites" (--> <--TLSv1.2 and
> earlier, please check).

Updated in the attached patch!

Cheers,

Dirkjan



0001-Add-support-for-ciphersuites-option-for-TLSv1.3.patch
Description: Binary data

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi Emeric,

> On 24 Sep 2018, at 15:33, Emeric Brun  wrote:
> 
> Seems good for me except for documentation:
> 
> Could you precise in the old "ciphers" description that this applies only for 
> TLSv <= 1.2. (and add a ref to the new keyword for TLSv1.3)

I have updated the documentation. Hopefully this is good then. Would it be 
possible to also backport this to 1.8 so that we could deploy a future 1.8 
stable version with OpenSSL 1.1.1 in the future?

Cheers,

Dirkjan



0001-Add-support-for-ciphersuites-option-for-TLSv1.3.patch
Description: Binary data

Re: Fix some warnings and a small bug in debug logic

[Dirkjan Bussink]

Hi all,

On 14 Sep 2018, at 14:43, Dirkjan Bussink  wrote:

> While working on the OpenSSL 1.1.1 and TLS 1.3 cipher support issue, I also 
> saw a number of compiler warnings that led me to investigate a bit. It 
> resulted in some small cleanups and also one bug I think in some of the 
> debugging logic for the h2 mux. 
> 
> Hopefully these are useful!

Is there any interest in these patches?

Cheers,

Dirkjan

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi all,

Given all the critical security issue and that you all were busy with that, I 
suspect this didn’t get much additional eyes. Now that that fix is out the 
door, I’m wondering if there’s any feedback or further input for the OpenSSL 
1.1.1 patches I wrote? 

Cheers,

Dirkjan

> On 14 Sep 2018, at 14:28, Dirkjan Bussink  wrote:
> 
> Hi all,
> 
>> On 14 Sep 2018, at 14:15, Emmanuel Hocdet  wrote:
>> 
>> It’s not necessary, BoringSSL and LibreSSL have, at best,  
>> OPENSSL_VERSION_NUMBER  set to 1.1.0 for API compatibilité.
> 
> Looking at LibreSSL, it’s defining this (in their latest Git code):
> 
> src/lib/libcrypto/opensslv.h:#define OPENSSL_VERSION_NUMBER   0x2000L
> 
> I also see this conditional used in other places to explicitly exclude 
> BoringSSL and LibreSSL, so that’s why I thought it would be needed here as 
> well. 
> 
> -- 
> Cheers,
> 
> Dirkjan

Fix some warnings and a small bug in debug logic

[Dirkjan Bussink]

Hi all,

While working on the OpenSSL 1.1.1 and TLS 1.3 cipher support issue, I also saw 
a number of compiler warnings that led me to investigate a bit. It resulted in 
some small cleanups and also one bug I think in some of the debugging logic for 
the h2 mux. 

Hopefully these are useful!

Cheers,

Dirkjan



0001-Remove-unused-variable.patch
Description: Binary data


0002-Fix-debug-warnings-for-h1-headers.patch
Description: Binary data


0003-Remove-unneeded-double-around-conditional-clause.patch
Description: Binary data

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi all,

> On 14 Sep 2018, at 14:15, Emmanuel Hocdet  wrote:
> 
> It’s not necessary, BoringSSL and LibreSSL have, at best,  
> OPENSSL_VERSION_NUMBER  set to 1.1.0 for API compatibilité.

Looking at LibreSSL, it’s defining this (in their latest Git code):

src/lib/libcrypto/opensslv.h:#define OPENSSL_VERSION_NUMBER 0x2000L

I also see this conditional used in other places to explicitly exclude 
BoringSSL and LibreSSL, so that’s why I thought it would be needed here as 
well. 

-- 
Cheers,

Dirkjan

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi all,

> On 14 Sep 2018, at 12:18, Emmanuel Hocdet  wrote:
> 
> Same deal with boringssl, TLSv <= 1.2 ciphers configuration and TLSv1.3 
> ciphers are segregated.
> https://boringssl.googlesource.com/boringssl/+/abbbee10ad4739648bcbf36a5ac52f23263a67dd%5E!/

This reminded me to double check with BoringSSL and LibreSSL how they handle 
this. Neither has this new method, so I have updated the conditional used in 
the patch to exclude these two as well.

Cheers,

Dirkjan



0001-Add-support-for-ciphersuites-option-for-TLS-1.3.patch
Description: Binary data

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi all,

I took the liberty of writing up a patch with what this could look like. I have 
named the option `ciphersuites` and also added the documentation for it as 
well. I have attached the patch to this email. 


> On 14 Sep 2018, at 11:12, Emeric Brun  wrote:
> 
> I think if TLSv <= 1.2 and TLSv1.3 ciphers are handled separately, this is 
> good reason
> to add a new keyword to manage both at a same line on an haproxy 
> configuration file's line .

This is what my patch does indeed. 

> I've just realized that it may be the openssl's response to an issue we faced 
> on earlier version of 
> openssl1.1.1 dev branch where forcing cipher suite on a SSL_CTX broke TLSv1.2 
> handshakes if
> no TLSv1.3 ciphers were specified in this list.
> 
> Doing this, managing differently TLS <= v1.2 and 1.3 ciphers permits the user 
> to not face regression issues
> upgrading  to v1.1.1 when suites where forced in configuration because 
> openssl-1.1.1 kept default
> TVSv1.3 ciphers.

Yeah, without the configurations setting it uses the 1.3 defaults which are 
already good safe defaults. 

> So i'm convinced we need to handle this new TLSv1.3 cipher suite with a new 
> config keyword, but I
> don't know how we should name it. I think it will be a mistake to make appear 
> 1.3 in the new name because
> there is no warranty that next TLS versions will specify specific cipher 
> lists. Openssl's
> API make the choice of "ciphersuites" ... perhaps a the right choice.

That’s what I did :). Hopefully it looks somewhat sensible. 

> Did any of you check how this is endled on "openssl s_client" command line?

From the help for this command:

 -cipher valSpecify TLSv1.2 and below cipher list to be used
 -ciphersuites val  Specify TLSv1.3 ciphersuites to be used

So it does the same thing as my patch does. 

Cheers,

Dirkjan



0001-Add-support-for-ciphersuites-option-for-TLS-1.3.patch
Description: Binary data

Re: TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi Lukas,

> On 13 Sep 2018, at 16:14, Lukas Tribus  wrote:
> 
> Definitely not some by string matching, openssl could have done
> exactly that, they choose to make a new API call instead, and they
> expect applications to introduce new configuration knobs or use the
> generic configuration interface SSL_CONF, so no, let's not get crazy
> with string magic here and expose the API as-is to the users
> (SSL_CTX_set_ciphersuites() or the generic SSL_CONF().


So with a new API call, does that mean adding for example a `ciphersuites` 
option that works similar to `ciphers` today that it accepts a string and then 
calls `SSL_CTX_set_ciphersuites`? I can see if I can create a patch that does 
that (and ideally would be possible to backport to 1.8 as well, since I would 
like to be able to run TLS 1.3 then with 1.8 which works perfectly fine apart 
from a lack of tuning for this). 

> I assume we don't have to change anything regarding groups/curves,
> although they implemented the new SSL_CTX_set1_groups() call, but if I
> understand correctly SSL_CTX_set1_curves_list() works just as well
> with TLSv1.3, right?

Yes, these work with TLSv1.3 still, but according to 
https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set1_curves_list.html there 
is preference for the groups functions (but not something I think needs to be 
addressed in the same patch / change?):

---

The curve functions are synonyms for the equivalently named group functions and 
are identical in every respect. They exist because, prior to TLS1.3, there was 
only the concept of supported curves. In TLS1.3 this was renamed to supported 
groups, and extended to include Diffie Hellman groups. The group functions 
should be used in preference.

---


Cheers,

Dirkjan

TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi all,

With the release of OpenSSL 1.1.1, TLS 1.3 is now also available. It already is 
working fine in my testing with HAProxy 1.8, there is however one issue. 
Currently there is no way to control the ciphers for TLS 1.3 from HAProxy, as 
according to the OpenSSL documentation, ciphers are handled by a separate 
method for TLS 1.3 compared to TLS 1.2 and earlier:

https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_cipher_list.html

SSL_CTX_set_cipher_list() sets the list of available ciphers (TLSv1.2 and 
below) for ctx using the control string str.

SSL_CTX_set_ciphersuites() is used to configure the available TLSv1.3 
ciphersuites for ctx.


Before I jump into writing code for this, I’m wondering what the approach is 
that HAProxy wants to take here. Should a similar options as todays `ciphers` 
option be made available in HAProxy to control the TLS 1.3 ciphers? If so, what 
should that be named? 

Or is another approach preferred here? For example by still using the `ciphers` 
configuration setting, but by then filtering out ciphers that start with 
`TLS13` and set those separate with `SSL_CTX_set_ciphersuites`?

Cheers,

Dirkjan Bussink

Re: OpenSSL 1.1.0 support => merged

[Dirkjan Bussink]

Hi Willy,

Thanks for getting this merged!

Cheers,

Dirkjan

> On 8 Nov 2016, at 13:12, Willy Tarreau  wrote:
> 
> Hi Dirkjan,
> 
> I finally merged your patch after discussing with Emeric. He's fine with
> it as well. Both of us think that the breakage of openssl 0.9.8 is not a
> showstopper at the moment and that the best way to know if/how it needs to
> be fixed is to let it go in the wild. Given that openssl 1.1.0 was released
> 3 months ago and 0.9.8 has been dead for almost a year, I prefer that our
> new haproxy version focuses on the future than on the past, especially with
> the reduced life of new versions (eg: 1.0.2 only has two more years left).
> 
> I got two annoying warnings affecting either older versions or newer ones,
> each time due to a const being put at the right place in a function prototype.
> So I added another patch to deal with this, it defines a new macro called
> __OPENSSL_110_CONST__ which equals "const" on 1.1.0 and nothing on older
> versions. 
> 
> I also moved the include file to include/proto/openssl-compat.h since it
> only defines prototypes. But the main reason I must confess is that the
> previous choice (include/compat/openssl.h) was causing me a lot of pain by
> breaking my auto-completion, and after spending a few hours on it, I had
> to forfeit on trying to get my fingers to switch "comm" instead of the
> shorter "c" they've been trained to for a decade :-/
> 
> I'm appending the 3 patches I added on top of yours and that I merged as
> one single patch. As you said, 1.1.0 builds with a few "deprecated" warnings
> which are still OK for now as the affected functions are part of the API and
> that we can clean later. I tested 1.0.1 and 1.0.2 and they were OK as well.
> 
> Now if anyone is interested in trying to port the code to 0.9.8 (should not be
> too hard, just a few more #ifdef to add), proposals are welcome.
> 
> Thanks!
> Willy
> <0001-WIP-ssl-1.1.0-protect-compat-openssl.h-against-multi.patch><0002-WIP-ssl-1.1.0-add-a-const-type-modifier-only-for-1.1.patch><0003-WIP-ssl-1.1.0-move-include-to-proto-openssl-compat.h.patch>

Re: OpenSSL 1.1.0 support

[Dirkjan Bussink]

Hi Willy,

> On 26 Oct 2016, at 11:40, Willy Tarreau  wrote:
> 
> Yes I'd rather have it work with 0.9.8 as it's still supported and used by
> some LTS distros. For example, RHEL5's regular support is due till March 2017
> and extended support till november 2020. It's not very likely that such users
> will decide to upgrade to 1.7 now especially since some of the SSL-related
> features only work with newer versions. But if we can make it work as it used
> to with limited effort it's better. Is your current patch capable of 
> supporting
> 0.9.8 ?

I don't think there's a reason it couldn't work. I haven't tested it though 
yet, only against 1.0.1 and 1.1.0 at this point. 

> Yes definitely. Also your new patch is much more readable and will make it
> easier to drop older versions in the future, I like it. I'll ping Emeric
> again since we got no response (ie I'll yell in the office) :-)

Alright, hopefully it's not too complicated to get this in then. 

Cheers,

Dirkjan

Re: OpenSSL 1.1.0 support

[Dirkjan Bussink]

Hi Willy,


> On 29 Aug 2016, at 15:15, Willy Tarreau  wrote:
> 
> Hi Dirkjan,
> 
> CCing Emeric who will know better than me how to respond, but I have some
> questions at the end.

I've gone for a somewhat different approach in this patch with a compatibility 
header file. It defines some inline functions from 1.1.0 when they are not 
available. Right now it implements the minimal things that HAProxy needs, so 
they aren't full replacements of what OpenSSL 1.1.0 would provide.

It's in a separate header file, was not sure if this was something that should 
go in include/proto/ssl_sock.h then. Also brings up the question what the 
minimum version of OpenSSL is that is supported? A lot of the functions used 
are already available in 1.0.0 (which is already EOL), so not sure if the 
compatibility code is needed for 0.9.8 then?

Is this more the approach you were thinking about?

Cheers,

Dirkjan



0001-Add-support-for-OpenSSL-1.1.0.patch
Description: Binary data

OpenSSL 1.1.0 support

[Dirkjan Bussink]

Hi all,

I've attached a patch for support for OpenSSL 1.1.0. That version changes quite 
a few things, mostly it makes a lot of the structures now opaque and private 
and provides functions to interact with them. Most of this change consists of 
using these new functions on OpenSSL 1.1.0 and newer.

There are a few things worth calling out specifically in this patch:

- I was not 100% clear on the handshake logic. It looked like it tried to 
detect if a handshake was ever attempted and distinguish that from a failure 
case. I've used the new state mechanism available through SSL_get_state to 
hopefully mimic similar behavior. I might have gotten this totally wrong though.
- The Makefile change was needed so that linking the OpenSSL bits also pulls in 
dl if needed (OpenSSL uses this itself). Also OpenSSL will now use pthread by 
default, so maybe that also should be added? Although I've used 
USE_PTHREAD_PSHARED for now in testing to link that.
- The code guarded with #ifdef SSL_CTX_get_tlsext_status_arg ideally would also 
use that macro, but there seems to be a closing brace missing at 
https://github.com/openssl/openssl/blob/fddfc0afc84728f8a5140685163e66ce6471742d/include/openssl/tls1.h#L300-L301
 so it throws an error. That's why I've used the implementation of that macro 
in the code instead.

What this does not address at the moment is fixing the use of deprecated 
functions. These are the warnings still present with these changes:

[jessie-amd64]  src/ssl_sock.c: In function 'ssl_tlsext_ticket_key_cb':
[jessie-amd64]  src/ssl_sock.c:492:3: warning: 'RAND_pseudo_bytes' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/rand.h:47)
 [-Wdeprecated-declarations]
[jessie-amd64] if(!RAND_pseudo_bytes(iv, EVP_MAX_IV_LENGTH))
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c: In function 'ssl_sock_prepare_ctx':
[jessie-amd64]  src/ssl_sock.c:2731:3: warning: 'TLSv1_server_method' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1597)
 [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_server_method());
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c:2734:3: warning: 'TLSv1_1_server_method' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1603)
 [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_1_server_method());
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c:2738:3: warning: 'TLSv1_2_server_method' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1609)
 [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_2_server_method());
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c:2824:13: warning: assignment discards 'const' 
qualifier from pointer target type
[jessie-amd64]cipher = sk_SSL_CIPHER_value(ciphers, idx);
[jessie-amd64]   ^
[jessie-amd64]  src/ssl_sock.c: In function 'ssl_sock_prepare_srv_ctx':
[jessie-amd64]  src/ssl_sock.c:3111:3: warning: 'TLSv1_client_method' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1598)
 [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, 
TLSv1_client_method());
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c:3114:3: warning: 'TLSv1_1_client_method' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1604)
 [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, 
TLSv1_1_client_method());
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c:3118:3: warning: 'TLSv1_2_client_method' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1610)
 [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, 
TLSv1_2_client_method());
[jessie-amd64] ^
[jessie-amd64]  src/ssl_sock.c: In function '__ssl_sock_deinit':
[jessie-amd64]  src/ssl_sock.c:6254:9: warning: 'ERR_remove_state' is 
deprecated (declared at 
/data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/err.h:247)
 [-Wdeprecated-declarations]
[jessie-amd64]   ERR_remove_state(0);
[jessie-amd64]  


Let me know what you all think.

Cheers,

Dirkjan



0001-Add-support-for-OpenSSL-1.1.0.patch
Description: Binary data

Re: [PATCH 0/2] Add support for TLS ticket keys configuration

[Dirkjan Bussink]

Hi all,

 On Fri, Feb 27, 2015 at 07:56:48PM +0100, Nenad Merdanovic wrote:
 This patchset adds support to configure TLS ticket keys used for
 encryption and decryption of TLS tickets.
 
 This is the 2nd version of the patchset that has been updated based on 
 suggestions from Willy TaRreau, Emeric Brun, Lukas Tribus and Remi Gacogne.

This is a great addition. I do have one question however. Are there also plans 
to allow for rotating these tickets through the admin socket interface? In 
order to get a system that’s actually properly forward secure, it’s needed to 
rotate the session tickets from time to time so that forward secrecy is 
actually preserved.

Twitter actually posted a nice article about how they achieved this: 
https://blog.twitter.com/2013/forward-secrecy-at-twitter.

In general the approach would be something like the following:

- Generate a new ticket every twelve hours. This then gets updated through the 
admin socket.
- Keep the last X older tickets so connections coming from clients with an 
older ticket also can still be decrypted. This can be for example the last 3, 
so it’s possible to decrypt over the last 36 hours.
- All tickets get encrypted with the new key, even the ones that came in with 
an older ticket.

One thing here that is tricky though is the actual rotation of the tickets. I 
think this should be separate from the installation to prevent race conditions. 
So ideally something like the following:

- Generate a new ticket and install it on all the load balancers
- After this step rotate the tickets on the load balancers.

This allows for transparant handling of it. If installation and rotation happen 
in one step, it means that server that isn’t rotated yet might get the new 
connection with a new ticket not installed on that second load balancer. This 
means the session reuse doesn’t work properly for this new session ticket until 
it’s installed everywhere.

Installing it first everywhere prevents the problem since until the ticket is 
rotated, the new ticket isn’t used yet for connections. This means it can be 
used for decryption but not yet for encryption. This would then only start to 
happen when it’s actually rotated.

Hope this all makes sense. If this is something already planned / discussed 
apology for the noise!

Cheers,

Dirkjan Bussink

Re: Roadmap for 1.6

[Dirkjan Bussink]

On 28 Jul 2014, at 11:54, Apollon Oikonomopoulos apoi...@debian.org wrote:

 If anyone has any comment / question / suggestion, as usual feel free to
 keep the discussion going on.
 
 Could I also add shared SSL session cache over multiple boxes (like 
 stud), to aid SSL scalability behind LVS directors? It has been asked 
 for before in the mailing list if I recall correctly.

I believe the best way to go here is to do this with TLS Session Tickets. 
Twitter posted a good post about how they set this up:

https://blog.twitter.com/2013/forward-secrecy-at-twitter

I think HAProxy could add something very similar by allowing key rotation 
through the socket interface, much like how OCSP Stapling can now be done. This 
would allow for the creation of tickets and rotate them around a cluster of 
different loadbalancers without having to build a complicated and error prone 
session cache across multiple machines.

— 
Regards,

Dirkjan Bussink

Re: Patch with some small memory usage fixes

[Dirkjan Bussink]

On 02 May 2014, at 11:37, Willy Tarreau w...@1wt.eu wrote:

 That said, one of your fix introduces a bug here :
 
 diff --git a/src/haproxy.c b/src/haproxy.c
 index ed2ff21..c1ec783 100644
 --- a/src/haproxy.c
 +++ b/src/haproxy.c
 @@ -1607,6 +1607,7 @@ int main(int argc, char **argv)
   exit(0); /* parent must leave */
   }
 
 + free(children);
   /* if we're NOT in QUIET mode, we should now close the 3 first 
 FDs to ensure
* that we can detach from the TTY. We MUST NOT do it in other 
 cases since
* it would have already be done, and 0-2 would have been 
 affected to listening
 
 Indeed, children is used a few lines below :
 
if (proc == global.nbproc) {
if (global.mode  MODE_SYSTEMD) {
for (proc = 0; proc  global.nbproc; proc++)
while (waitpid(children[proc], NULL, 
 0) == -1  errno == EINTR);
}
exit(0); /* parent must leave */
}
 

This is very strange, because it looks like this code is right above the 
free(children) call, not below. At least that it what it looks like to me. I 
check against latest master just now and see no usage of children anymore after 
the free() call.

I did update the patch by setting NULL and also removing the conditionals for 
the other free() calls.

Attached the fixed patch, if I’m looking in the wrong place for the 
free(children) issue, please let me know!

— 
Dirkjan



0001-Fix-a-few-memory-usage-errors.patch
Description: Binary data

Patch with some small memory usage fixes

[Dirkjan Bussink]

Hi all,

When building HAProxy using the Clang Static Analyzer, it found a few cases of 
invalid memory usage and leaks. I’ve attached a patch to fix these cases.

— 
Regards,

Dirkjan Bussink



0001-Fix-a-few-memory-usage-errors.patch
Description: Binary data

Re: Patch for ALPN compatibility with OpenSSL development

[Dirkjan Bussink]

Hi all,

 Yes, of course! The ALPN patch was an early incantation I did based on
 initially available patches so that people could start to experiment
 with it. Now that we're finally getting closer to something official
 in openssl, better stick to it!
 
 Dirkjan, could you please adjust your patch with Lukas' suggestions
 above ? I'll happily merge it.

I’ve updated the patch with the suggestion given.

— 
Dirkjan



0001-Use-ALPN-support-as-it-will-be-available-in-OpenSSL-.patch
Description: Binary data

Re: Patch for ALPN compatibility with OpenSSL development

[Dirkjan Bussink]

On 14 Feb 2014, at 21:35, Lukas Tribus luky...@hotmail.com wrote:

 From the openssl source file ssl/ssl_lib.c:
 /* SSL_CTX_set_alpn_select_cb sets a callback function on |ctx| that is 
 called
   * during ClientHello processing in order to select an ALPN protocol from 
 the
   * client's list of offered protocols. */
   void SSL_CTX_set_alpn_select_cb(SSL_CTX* ctx,
 
 SSL_CTX_set_alpn_select_cb() is supposed to select the most suitable single
 protocol from the client hello.
 
 
 Looks like we need some additional logic to implement this correctly.
 
 
 btw: is there any way to easily test this? I don't have a spdy backend
 and I didn't find anything relevant in chrome://net-internals/ for
 alpn-upgraded http/1.1 connections.

I’ve updated the patch which now does actual negotiation. The logic comes from 
the example OpenSSL server application that also was committed in the commit 
that adds ALPN support to OpenSSL:

http://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=6f017a8f9db3a79f3a3406cf8d493ccd346db691

The way I have tested it is to run HAProxy with a sample configuration file and 
use openssl s_client to verify that the negotiation happens properly. If you 
run use the openssl binary from the self built newer version, s_client also 
supports ALPN and can be used to verify the behaviour. 

I’ve confirmed that this will negotiate the highest preferred protocol 
specified on the server and if there is no compatible protocol, no ALPN 
protocol is negotiated.

— 
Dirkjan




0001-Use-ALPN-support-as-it-will-be-available-in-OpenSSL-.patch
Description: Binary data

Patch for ALPN compatibility with OpenSSL development

[Dirkjan Bussink]

Hi all,

At GitHub we’ve worked on a patch to make HAProxy’s ALPN code compatible with 
the patches for it that have landed in OpenSSL:

http://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=6f017a8f9db3a79f3a3406cf8d493ccd346db691

This final version is slightly different from what HAProxy currently expects, 
which is based on some custom OpenSSL patches.

Let me know if this is a good approach towards fixing this problem or that it 
should be done differently.

- 
Dirkjan Bussink




0001-Use-ALPN-support-as-it-will-be-available-in-OpenSSL-.patch
Description: Binary data