Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
On Fri, Jul 28, 2017 at 03:28:53PM -0700, Kevin McArthur wrote:
> > I really think that for most users it will be fine this way as it has been
> > for 5 years, and for me that justifies not trying to go too far for the 
> > short
> > term.
> Fair enough, but don't forget that for the last 5 years folks have just been
> setting verify none in all the tutorials lol!

Yes but it's not surprizing. Most of the time haproxy is installed in front
of the servers on the local network and is the SSL termination. The main
reason for talking SSL to the server is to avoid having to touch its
configuration to make it accept clear communications. We've been keeping
SSLv3 solely for this reason for example. The docs are clear enough on the
impacts of "verify none" and it's not the default, so users are expected
to be aware of the risks.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Kevin McArthur

I really think that for most users it will be fine this way as it has been
for 5 years, and for me that justifies not trying to go too far for the short
term.
Fair enough, but don't forget that for the last 5 years folks have just 
been setting verify none in all the tutorials lol!


--

Kevin


On 2017-07-28 3:26 PM, Willy Tarreau wrote:

On Fri, Jul 28, 2017 at 03:11:20PM -0700, Kevin McArthur wrote:

More flexibility in deployment is usually a good thing so long as they have
sane defaults. Its certainly fine to say that one has to setup an internal
ca to get a secured check. Its just more moving parts.

Which is also more difficulty in troubleshooting sessions (ie: bisection
needed to find what causes this or that issue).


Ah my bad here. I was running off the patches from the mailing list and not
what you added to master. Rebuilding makes the new verifyhost behavior work
as above. The ML patchset doesn't do this which is where the disconnect is.

Ah cool!


You can do the same (except sending the SNI with the check) using this
today :

  server app2 internal.app2.example.ca:443 ssl verify required verifyhost 
app2.example.ca sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check 
check-ssl

Being able to send the SNI would be nice... ;)

I agree and I think that's the missing part. But I also think we *may*
have a solution by using a converter transforming an empty sample into
a string (I need to experiment a bit on this). We could for example have
"sni ssl_fc_sni,default(blah)". We know we need this at other places, and
if that can work here, it might be an interestin alternative solution as
well. But I agree in general that being able to set the SNI for health
checks would be better. It just ranks at a much lower priority than the
stuff that can be done in the same timeframe at the moment. Seeing the
little visible progress made on HTTP/2 over the last few weeks worries
me a bit for the upcoming release...

(...)

Need not be complex if the defaults are the same as not implementing them.
Just helps make some internal architectures easier to deploy securely.

I'm generally fine with adding more features but once there's a real
confirmed need for these. Keep in mind that the doc is already huge due
to the vast amount of keywords and options we support and that often
even when you know you can do something, it becomes hard to find it.
Keeping the focus on the important stuff is important.


That
said, this doesn't block me as I'll just happily verifyhost against a public
cert as a default and 'trick' haproxy that way even though its connecting to
an internal server. So its not critical.

I really think that for most users it will be fine this way as it has been
for 5 years, and for me that justifies not trying to go too far for the short
term.

Thanks!
Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
On Fri, Jul 28, 2017 at 03:11:20PM -0700, Kevin McArthur wrote:
> More flexibility in deployment is usually a good thing so long as they have
> sane defaults. Its certainly fine to say that one has to setup an internal
> ca to get a secured check. Its just more moving parts.

Which is also more difficulty in troubleshooting sessions (ie: bisection
needed to find what causes this or that issue).

> Ah my bad here. I was running off the patches from the mailing list and not
> what you added to master. Rebuilding makes the new verifyhost behavior work
> as above. The ML patchset doesn't do this which is where the disconnect is.

Ah cool!

> > You can do the same (except sending the SNI with the check) using this
> > today :
> > 
> >  server app2 internal.app2.example.ca:443 ssl verify required 
> > verifyhost app2.example.ca sni ssl_fc_sni ca-file 
> > /etc/ssl/certs/ca-certificates.crt check check-ssl
> 
> Being able to send the SNI would be nice... ;)

I agree and I think that's the missing part. But I also think we *may*
have a solution by using a converter transforming an empty sample into
a string (I need to experiment a bit on this). We could for example have
"sni ssl_fc_sni,default(blah)". We know we need this at other places, and
if that can work here, it might be an interestin alternative solution as
well. But I agree in general that being able to set the SNI for health
checks would be better. It just ranks at a much lower priority than the
stuff that can be done in the same timeframe at the moment. Seeing the
little visible progress made on HTTP/2 over the last few weeks worries
me a bit for the upcoming release...

(...)
> Need not be complex if the defaults are the same as not implementing them.
> Just helps make some internal architectures easier to deploy securely.

I'm generally fine with adding more features but once there's a real
confirmed need for these. Keep in mind that the doc is already huge due
to the vast amount of keywords and options we support and that often
even when you know you can do something, it becomes hard to find it.
Keeping the focus on the important stuff is important.

> That
> said, this doesn't block me as I'll just happily verifyhost against a public
> cert as a default and 'trick' haproxy that way even though its connecting to
> an internal server. So its not critical.

I really think that for most users it will be fine this way as it has been
for 5 years, and for me that justifies not trying to go too far for the short
term.

Thanks!
Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Kevin McArthur



On 2017-07-28 2:21 PM, Willy Tarreau wrote:

On Fri, Jul 28, 2017 at 10:24:47AM -0700, Kevin McArthur wrote:

I would propose something like the following:

New options:

check-ssl-sni (optional) .. set the value to send as sni. Defaults to the
value from the server hostname being connected. (Could be nullable? or
another setting like check-sni-disable added)

check-ssl-verify (optional). required/none. Affects only the check subsystem
validations. Defaults to required.

check-ssl-verifyhost (optional). Set a hostname to verify against. Defaults
to the value from the server hostname being connected to, or null if check
verify = none)

check-ssl-ca-file. (optional). Use a different bundle for ca certs for the
checks instead of the normal list, as this will likely be an internal ca.

For me it creates too many confusing options for stuff that mostly nobody
is interested in seeing different. Possibly that the check-ssl-sni could
be useful but then it would be useful. "verify" being set is enough to
say that you care about the server connections being verified and that
all protocol elements that are fed need to be checked ; that rules out
the need for "check-ssl-verify". "check-ssl-verifyhost" doesn't provide
any benefit as without SNI there is no reason for seeing a different name
being returned on the same connections. Regarding the CA file it's the same.
More flexibility in deployment is usually a good thing so long as they 
have sane defaults. Its certainly fine to say that one has to setup an 
internal ca to get a secured check. Its just more moving parts.



verifyhost-sni-or-string (optional) default.example.ca. Use the SNI value or
if no sni value a provided default string. default string would be the
hostname from the server line.

That's exactly what "verifyhost" does in the latest patchset (except this
"hostname" from the server line, which is a bad idea in many cases as a
lot of people use plain IP addresses).


Ah my bad here. I was running off the patches from the mailing list and 
not what you added to master. Rebuilding makes the new verifyhost 
behavior work as above. The ML patchset doesn't do this which is where 
the disconnect is.



This would make the following scenarios work;

--

An internal server is being used as a backend. There is no valid cert for
the internal.* variant of the domain. The server returns a valid cert for
app2.example.ca as the check system told the server to use app2.example.ca
SNI rather than the actual hostname being connected to. The check connection
is secure and SNI vhosts are too.

server app2 internal.app2.example.ca:443 ssl verify required
verifyhost-sni-or-string app2.example.ca sni ssl_fc_sni ca-file
/etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-sni
app2.example.ca

You can do the same (except sending the SNI with the check) using this
today :

 server app2 internal.app2.example.ca:443 ssl verify required verifyhost 
app2.example.ca sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check 
check-ssl


Being able to send the SNI would be nice... ;)



An internal server is being used as a backend, but doesn't have a valid tls
certificate for the server itself. In this case disable verify on checks but
maintain it for normal connections to the backend

server app2 internal.app2.example.ca:443 ssl verify required sni ssl_fc_sni
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-verify
none

That's what you do without verifyhost today, without having to add
"check-ssl-verify none".
I was thinking more like a self-signed localhost cert that wouldn't 
chain to a valid root and therefore fails the verify part, not one that 
only mismatches the hostname but is otherwise valid.



An internal server is being used as a backend, but used an internal CA for
the internal server domains but not for normal client domains. In this case
provide a different ca for the check system.

server app2 internal.app2.example.ca:443 ssl verify required sni ssl_fc_sni
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-ca-file
/path/to/local-ca.crt

I hardly see a reasonably valid use case for using a different CA for the
checks quite honnestly! Please keep in mind that all these possibilities
become very painful to configure for most users to cover the most common
cases (and by the way there wasn't any such request (not even a single
one) in the last 5 years). These "server" directives become quite long
and complex! I tend to prefer to adapt to real needs than provide features
complicating normal configs and whose benefits are hard to verify in field.


Need not be complex if the defaults are the same as not implementing 
them. Just helps make some internal architectures easier to deploy 
securely. That said, this doesn't block me as I'll just happily 
verifyhost against a public cert as a default and 'trick' haproxy that 
way even though its connecting to an internal server. So its not critical.


Regards,
Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
On Fri, Jul 28, 2017 at 10:24:47AM -0700, Kevin McArthur wrote:
> I would propose something like the following:
> 
> New options:
> 
> check-ssl-sni (optional) .. set the value to send as sni. Defaults to the
> value from the server hostname being connected. (Could be nullable? or
> another setting like check-sni-disable added)
> 
> check-ssl-verify (optional). required/none. Affects only the check subsystem
> validations. Defaults to required.
> 
> check-ssl-verifyhost (optional). Set a hostname to verify against. Defaults
> to the value from the server hostname being connected to, or null if check
> verify = none)
> 
> check-ssl-ca-file. (optional). Use a different bundle for ca certs for the
> checks instead of the normal list, as this will likely be an internal ca.

For me it creates too many confusing options for stuff that mostly nobody
is interested in seeing different. Possibly that the check-ssl-sni could
be useful but then it would be useful. "verify" being set is enough to
say that you care about the server connections being verified and that
all protocol elements that are fed need to be checked ; that rules out
the need for "check-ssl-verify". "check-ssl-verifyhost" doesn't provide
any benefit as without SNI there is no reason for seeing a different name
being returned on the same connections. Regarding the CA file it's the same.

> verifyhost-sni-or-string (optional) default.example.ca. Use the SNI value or
> if no sni value a provided default string. default string would be the
> hostname from the server line.

That's exactly what "verifyhost" does in the latest patchset (except this
"hostname" from the server line, which is a bad idea in many cases as a
lot of people use plain IP addresses).

> This would make the following scenarios work;
> 
> --
> 
> An internal server is being used as a backend. There is no valid cert for
> the internal.* variant of the domain. The server returns a valid cert for
> app2.example.ca as the check system told the server to use app2.example.ca
> SNI rather than the actual hostname being connected to. The check connection
> is secure and SNI vhosts are too.
> 
> server app2 internal.app2.example.ca:443 ssl verify required
> verifyhost-sni-or-string app2.example.ca sni ssl_fc_sni ca-file
> /etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-sni
> app2.example.ca

You can do the same (except sending the SNI with the check) using this
today :

server app2 internal.app2.example.ca:443 ssl verify required verifyhost 
app2.example.ca sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check 
check-ssl

> An internal server is being used as a backend, but doesn't have a valid tls
> certificate for the server itself. In this case disable verify on checks but
> maintain it for normal connections to the backend
> 
> server app2 internal.app2.example.ca:443 ssl verify required sni ssl_fc_sni
> ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-verify
> none

That's what you do without verifyhost today, without having to add
"check-ssl-verify none".

> An internal server is being used as a backend, but used an internal CA for
> the internal server domains but not for normal client domains. In this case
> provide a different ca for the check system.
> 
> server app2 internal.app2.example.ca:443 ssl verify required sni ssl_fc_sni
> ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-ca-file
> /path/to/local-ca.crt

I hardly see a reasonably valid use case for using a different CA for the
checks quite honnestly! Please keep in mind that all these possibilities
become very painful to configure for most users to cover the most common
cases (and by the way there wasn't any such request (not even a single
one) in the last 5 years). These "server" directives become quite long
and complex! I tend to prefer to adapt to real needs than provide features
complicating normal configs and whose benefits are hard to verify in field.

Regards,
Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
On Fri, Jul 28, 2017 at 10:04:54AM -0700, Kevin McArthur wrote:
> If I add a verifyhost directive, all the normal connections identified by
> SNI break.

Huh ? Are you sure you correctly applied the patch set ? That's the
behaviour of the previous one. I've got the exact opposite here. With
verifyhost set, for me the check is OK when the server cert matches
(as it used to do before we had the check on the SNI) and SNI
connections are now unaffected.

> Per the new patch leaving out verifyhost is not supposed to be no
> hostname check, but rather check based on SNI/hostname?

For the 4th or 5th time, there's no SNI with checks :-)

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Kevin McArthur

I would propose something like the following:

New options:

check-ssl-sni (optional) .. set the value to send as sni. Defaults to 
the value from the server hostname being connected. (Could be nullable? 
or another setting like check-sni-disable added)


check-ssl-verify (optional). required/none. Affects only the check 
subsystem validations. Defaults to required.


check-ssl-verifyhost (optional). Set a hostname to verify against. 
Defaults to the value from the server hostname being connected to, or 
null if check verify = none)


check-ssl-ca-file. (optional). Use a different bundle for ca certs for 
the checks instead of the normal list, as this will likely be an 
internal ca.


verifyhost-sni-or-string (optional) default.example.ca. Use the SNI 
value or if no sni value a provided default string. default string would 
be the hostname from the server line.



This would make the following scenarios work;

--

An internal server is being used as a backend. There is no valid cert 
for the internal.* variant of the domain. The server returns a valid 
cert for app2.example.ca as the check system told the server to use 
app2.example.ca SNI rather than the actual hostname being connected to. 
The check connection is secure and SNI vhosts are too.


server app2 internal.app2.example.ca:443 ssl verify required 
verifyhost-sni-or-string app2.example.ca sni ssl_fc_sni ca-file 
/etc/ssl/certs/ca-certificates.crt check check-ssl check-ssl-sni 
app2.example.ca


--

An internal server is being used as a backend, but doesn't have a valid 
tls certificate for the server itself. In this case disable verify on 
checks but maintain it for normal connections to the backend


server app2 internal.app2.example.ca:443 ssl verify required sni 
ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl 
check-ssl-verify none


--

An internal server is being used as a backend, but used an internal CA 
for the internal server domains but not for normal client domains. In 
this case provide a different ca for the check system.


server app2 internal.app2.example.ca:443 ssl verify required sni 
ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl 
check-ssl-ca-file /path/to/local-ca.crt


--

Kevin



On 2017-07-28 10:04 AM, Kevin McArthur wrote:



On 2017-07-28 10:02 AM, Willy Tarreau wrote:

On Fri, Jul 28, 2017 at 09:46:12AM -0700, Kevin McArthur wrote:
I think somethings missing here; the check system doesn't seem to be 
sending

the SNI or validating the result.

If I do a backend line like:

server app2 internal.app2.example.ca:443 ssl verify required sni 
ssl_fc_sni

ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl

This works fine, but my server has no tls cert for 
internal.app2.example.ca

and the checks still pass verify.

That's normal and matches the documentation :-) You have no "verifyhost"
directive. So the check doesn't have any name to check against. Just
add "verifyhost internal.app2.example.ca" and the check will match only
this one.
If I add a verifyhost directive, all the normal connections identified 
by SNI break. Per the new patch leaving out verifyhost is not supposed 
to be no hostname check, but rather check based on SNI/hostname?

The server side of things tells me the SNI
never gets sent on the check connection, hits the default cert 
(app2, no

internal).
We never send SNI with checks since there's no front connection to 
extract
it from. That's exactly why we wanted to have the ability to use 
verifyhost

for checks even when sni is enabled.

The SNI in this case should come from the server line.


Willy

--
Kevin





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Kevin McArthur



On 2017-07-28 10:02 AM, Willy Tarreau wrote:

On Fri, Jul 28, 2017 at 09:46:12AM -0700, Kevin McArthur wrote:

I think somethings missing here; the check system doesn't seem to be sending
the SNI or validating the result.

If I do a backend line like:

server app2 internal.app2.example.ca:443 ssl verify required sni ssl_fc_sni
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl

This works fine, but my server has no tls cert for internal.app2.example.ca
and the checks still pass verify.

That's normal and matches the documentation :-) You have no "verifyhost"
directive. So the check doesn't have any name to check against. Just
add "verifyhost internal.app2.example.ca" and the check will match only
this one.
If I add a verifyhost directive, all the normal connections identified 
by SNI break. Per the new patch leaving out verifyhost is not supposed 
to be no hostname check, but rather check based on SNI/hostname?

The server side of things tells me the SNI
never gets sent on the check connection, hits the default cert (app2, no
internal).

We never send SNI with checks since there's no front connection to extract
it from. That's exactly why we wanted to have the ability to use verifyhost
for checks even when sni is enabled.

The SNI in this case should come from the server line.


Willy

--
Kevin



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
On Fri, Jul 28, 2017 at 09:46:12AM -0700, Kevin McArthur wrote:
> I think somethings missing here; the check system doesn't seem to be sending
> the SNI or validating the result.
> 
> If I do a backend line like:
> 
> server app2 internal.app2.example.ca:443 ssl verify required sni ssl_fc_sni
> ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl
> 
> This works fine, but my server has no tls cert for internal.app2.example.ca
> and the checks still pass verify.

That's normal and matches the documentation :-) You have no "verifyhost"
directive. So the check doesn't have any name to check against. Just
add "verifyhost internal.app2.example.ca" and the check will match only
this one.

> The server side of things tells me the SNI
> never gets sent on the check connection, hits the default cert (app2, no
> internal).

We never send SNI with checks since there's no front connection to extract
it from. That's exactly why we wanted to have the ability to use verifyhost
for checks even when sni is enabled.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Kevin McArthur
I think somethings missing here; the check system doesn't seem to be 
sending the SNI or validating the result.


If I do a backend line like:

server app2 internal.app2.example.ca:443 ssl verify required sni 
ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl


This works fine, but my server has no tls cert for 
internal.app2.example.ca and the checks still pass verify. The server 
side of things tells me the SNI never gets sent on the check connection, 
hits the default cert (app2, no internal). Could be the same 
null/default pathway?


--
Kevin


On 2017-07-28 9:41 AM, Willy Tarreau wrote:

On Fri, Jul 28, 2017 at 08:45:37AM -0700, Kevin McArthur wrote:

Sounds good Willy, where did we leave the issue of the SNI,
verifypeer/verifyhost validation and the checks subsystem?

the checks are now covered by verifyhost as they used to, that
was the main purpose.

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
On Fri, Jul 28, 2017 at 08:45:37AM -0700, Kevin McArthur wrote:
> Sounds good Willy, where did we leave the issue of the SNI,
> verifypeer/verifyhost validation and the checks subsystem?

the checks are now covered by verifyhost as they used to, that
was the main purpose.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Kevin McArthur
Sounds good Willy, where did we leave the issue of the SNI, 
verifypeer/verifyhost validation and the checks subsystem?


--

Kevin
On 2017-07-28 3:11 AM, Willy Tarreau wrote:

Hi,

On Thu, Jul 27, 2017 at 05:17:36AM +0200, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 02:19:19PM -0700, Kevin McArthur wrote:

If a check is going to validate its sni/hostname
going forward I'll have to figure out some sort of self-signed setup for the
internal.* domains that cant easily be letsenc'd. Alternatively you guys can
add check-ssl-verifypeer none option or similar, or change the behavior of
verifyhost to match a default rather than be an override.

After a night on it, I now think I'll try to go down that route. But probably
not today.

I've now done it and run multiple tests on it, it's OK so I've just merged
it in the following commits :

   ad92a9a BUG/MINOR: ssl: make use of the name in SNI before verifyhost
   71d058c MINOR: ssl: add a new error codes for wrong server certificates
   46d5b08 BUG/MEDIUM: stream: don't retry SSL connections which fail the SNI 
name check

This way it doesn't affect whatever used to work before and enables
support of SNI without having to switch to "verify none". I think we'll
soon backport the whole series to 1.7 and maybe 1.6, though the current
behaviour matches documentation, it's just that it's impossible to secure
it by design.

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-28 Thread Willy Tarreau
Hi,

On Thu, Jul 27, 2017 at 05:17:36AM +0200, Willy Tarreau wrote:
> On Wed, Jul 26, 2017 at 02:19:19PM -0700, Kevin McArthur wrote:
> > If a check is going to validate its sni/hostname
> > going forward I'll have to figure out some sort of self-signed setup for the
> > internal.* domains that cant easily be letsenc'd. Alternatively you guys can
> > add check-ssl-verifypeer none option or similar, or change the behavior of
> > verifyhost to match a default rather than be an override.
> 
> After a night on it, I now think I'll try to go down that route. But probably
> not today.

I've now done it and run multiple tests on it, it's OK so I've just merged
it in the following commits :

  ad92a9a BUG/MINOR: ssl: make use of the name in SNI before verifyhost
  71d058c MINOR: ssl: add a new error codes for wrong server certificates
  46d5b08 BUG/MEDIUM: stream: don't retry SSL connections which fail the SNI 
name check

This way it doesn't affect whatever used to work before and enables
support of SNI without having to switch to "verify none". I think we'll
soon backport the whole series to 1.7 and maybe 1.6, though the current
behaviour matches documentation, it's just that it's impossible to secure
it by design.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 02:19:19PM -0700, Kevin McArthur wrote:
> TL;DR; The point is the haproxy needs to be able to properly pass on the SNI
> value if its going to verifyhost/verifypeer (even on the check's)... in this
> case the checks dont pass along a sni value, and dont validate a verifyhost
> coming back from the default. Thats probably ok as the purpose of the check
> is just to make sure the host is up, but my concern here would be changing
> behavior going forward.

We try hard never to change behaviours without requiring a config change.
This is important because load balancing is never easy and you don't want
stuff to change below your feet at a moment you don't remember anymore what
needed to be addressed. That's also why wee keep a lot of stuff that some
people think are useless but they are used by a few people. For this reason
we try to limit addition of possibly useless features (less forward
compatiblity to take care of).

> If a check is going to validate its sni/hostname
> going forward I'll have to figure out some sort of self-signed setup for the
> internal.* domains that cant easily be letsenc'd. Alternatively you guys can
> add check-ssl-verifypeer none option or similar, or change the behavior of
> verifyhost to match a default rather than be an override.

After a night on it, I now think I'll try to go down that route. But probably
not today.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur
The log files would capture whats going on at the HTTP level one would 
presume. I dont actually have a client that speaks HTTP enough to 
mismatch a host/sni and read the responses properly. S-client isnt 
helpful here.


TL;DR; The point is the haproxy needs to be able to properly pass on the 
SNI value if its going to verifyhost/verifypeer (even on the check's)... 
in this case the checks dont pass along a sni value, and dont validate a 
verifyhost coming back from the default. Thats probably ok as the 
purpose of the check is just to make sure the host is up, but my concern 
here would be changing behavior going forward. If a check is going to 
validate its sni/hostname going forward I'll have to figure out some 
sort of self-signed setup for the internal.* domains that cant easily be 
letsenc'd. Alternatively you guys can add check-ssl-verifypeer none 
option or similar, or change the behavior of verifyhost to match a 
default rather than be an override.


--

Kevin


On 2017-07-26 2:15 PM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 01:04:05PM -0700, Kevin McArthur wrote:

Here:

In the first example, a valid host, valid sni. Second is valid sni broken
host. Third is totally made up sni with broken host. Fourth is a valid Host
with a made up sni. The apache vhosts have separate log files. The
ssltest.example.ca sni with ssltest-broken.example.ca properly logs to the
ssltest log. The valid host ssltest.example.ca made-up-sni, logs to the app2
vhost (default) logfile. So pretty sure the Host header is being totally
ignored here.

That's very strange. However the test captures only show what is negociated
at the cert level. What matters is what is done at the HTTP layer. But as
long as you're happy with your setup... :-)

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 01:04:05PM -0700, Kevin McArthur wrote:
> Here:
> 
> In the first example, a valid host, valid sni. Second is valid sni broken
> host. Third is totally made up sni with broken host. Fourth is a valid Host
> with a made up sni. The apache vhosts have separate log files. The
> ssltest.example.ca sni with ssltest-broken.example.ca properly logs to the
> ssltest log. The valid host ssltest.example.ca made-up-sni, logs to the app2
> vhost (default) logfile. So pretty sure the Host header is being totally
> ignored here.

That's very strange. However the test captures only show what is negociated
at the cert level. What matters is what is done at the HTTP layer. But as
long as you're happy with your setup... :-)

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur

Here:

In the first example, a valid host, valid sni. Second is valid sni 
broken host. Third is totally made up sni with broken host. Fourth is a 
valid Host with a made up sni. The apache vhosts have separate log 
files. The ssltest.example.ca sni with ssltest-broken.example.ca 
properly logs to the ssltest log. The valid host ssltest.example.ca 
made-up-sni, logs to the app2 vhost (default) logfile. So pretty sure 
the Host header is being totally ignored here.


printf "GET / HTTP/1.1\r\nHost: ssltest.example.ca\r\n\r\n" | openssl 
s_client -connect internal.app2.example.ca:443 -servername 
ssltest.example.ca CONNECTED(0003) depth=2 O = Digital Signature 
Trust Co., CN = DST Root CA X3 verify return:1 depth=1 C = US, O = Let's 
Encrypt, CN = Let's Encrypt Authority X3 verify return:1 depth=0 CN = 
*ssltest*.example.ca verify return:1 printf "GET / HTTP/1.1\r\nHost: 
ssltest-broken.example.ca\r\n\r\n" | openssl s_client -connect 
internal.app2.example.ca:443 -servername ssltest.example.ca 
CONNECTED(0003) depth=2 O = Digital Signature Trust Co., CN = DST 
Root CA X3 verify return:1 depth=1 C = US, O = Let's Encrypt, CN = Let's 
Encrypt Authority X3 verify return:1 depth=0 CN = *ssltest*.example.ca 
verify return:1 printf "GET / HTTP/1.1\r\nHost: 
ssltest-broken.example.ca\r\n\r\n" | openssl s_client -connect 
internal.app2.example.ca:443 -servername made-up-sni.example.ca 
CONNECTED(0003) depth=2 O = Digital Signature Trust Co., CN = DST 
Root CA X3 verify return:1 depth=1 C = US, O = Let's Encrypt, CN = Let's 
Encrypt Authority X3 verify return:1 depth=0 CN = *app2*.example.ca 
verify return:1 printf "GET / HTTP/1.1\r\nHost: 
ssltest.example.ca\r\n\r\n" | openssl s_client -connect 
internal.app2.example.ca:443 -servername made-up-sni.example.ca 
CONNECTED(0003) depth=2 O = Digital Signature Trust Co., CN = DST 
Root CA X3 verify return:1 depth=1 C = US, O = Let's Encrypt, CN = Let's 
Encrypt Authority X3 verify return:1 depth=0 CN = *app2*.example.ca 
verify return:1



On 2017-07-26 12:49 PM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 12:28:55PM -0700, Kevin McArthur wrote:

No, it needs it to select the certificate to present. Then it should match
it against the Host header field, and use the Host header field to select
the vhost. The difference is subtle but it's important to keep each protocol
element one in its role. In the end for HTTP it's always the Host which
decides, regardless of the transport.

Perhaps how it should work but this isn't actually how apache matches vhosts
in a ssl context. They use the SNI, which selects the vhost in use. The host
header can be gibberish and it will still select the vhost via sni when
configured with ServerName directive.

Well it's the first time I hear about this and find this a bit shocking,
as Apache tends to try to follow standards and ignoring Host clearly
doesn't fall into that category and would even cause some security issues
when used as a reverse-proxy by routing the requests to the wrong places.
Also the doc here tends to disagree as well :

https://httpd.apache.org/docs/2.4/vhosts/name-based.html

 "With name-based virtual hosting, the server relies on the client
 to report the hostname as part of the HTTP headers."

So maybe you want to double-check what happens when you do this :

   $ printf "GET / HTTP/1.1\r\nHost: domain1\r\n\r\n | openssl s_client 
-connect host:port -servername domain2
   $ printf "GET / HTTP/1.1\r\nHost: domain2\r\n\r\n | openssl s_client 
-connect host:port -servername domain1

My guess is that either you'll get a 4xx error because there's a mismatch
or you'll get the domain referenced in the Host header delivered.

And by the way I'm seeing this in their changelog for v2.2.26 and v2.2.27 :

   *) mod_ssl: Check SNI hostname against Host header case-insensitively.

   *) mod_ssl: Do not perform SNI / Host header comparison in case of a
 forward proxy request

This tends to confirm that by default the comparison is performed.

Willy




Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 12:28:55PM -0700, Kevin McArthur wrote:
> > No, it needs it to select the certificate to present. Then it should match
> > it against the Host header field, and use the Host header field to select
> > the vhost. The difference is subtle but it's important to keep each protocol
> > element one in its role. In the end for HTTP it's always the Host which
> > decides, regardless of the transport.
> Perhaps how it should work but this isn't actually how apache matches vhosts
> in a ssl context. They use the SNI, which selects the vhost in use. The host
> header can be gibberish and it will still select the vhost via sni when
> configured with ServerName directive.

Well it's the first time I hear about this and find this a bit shocking,
as Apache tends to try to follow standards and ignoring Host clearly
doesn't fall into that category and would even cause some security issues
when used as a reverse-proxy by routing the requests to the wrong places.
Also the doc here tends to disagree as well :

   https://httpd.apache.org/docs/2.4/vhosts/name-based.html

"With name-based virtual hosting, the server relies on the client
to report the hostname as part of the HTTP headers."

So maybe you want to double-check what happens when you do this :

  $ printf "GET / HTTP/1.1\r\nHost: domain1\r\n\r\n | openssl s_client -connect 
host:port -servername domain2
  $ printf "GET / HTTP/1.1\r\nHost: domain2\r\n\r\n | openssl s_client -connect 
host:port -servername domain1

My guess is that either you'll get a 4xx error because there's a mismatch
or you'll get the domain referenced in the Host header delivered.

And by the way I'm seeing this in their changelog for v2.2.26 and v2.2.27 :

  *) mod_ssl: Check SNI hostname against Host header case-insensitively.

  *) mod_ssl: Do not perform SNI / Host header comparison in case of a
forward proxy request

This tends to confirm that by default the comparison is performed.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur

No, it needs it to select the certificate to present. Then it should match
it against the Host header field, and use the Host header field to select
the vhost. The difference is subtle but it's important to keep each protocol
element one in its role. In the end for HTTP it's always the Host which
decides, regardless of the transport.
Perhaps how it should work but this isn't actually how apache matches 
vhosts in a ssl context. They use the SNI, which selects the vhost in 
use. The host header can be gibberish and it will still select the vhost 
via sni when configured with ServerName directive.


--

Kevin


On 2017-07-26 12:26 PM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 11:49:22AM -0700, Kevin McArthur wrote:

I'm still thinking about something like this. What bothers me is that we
already have a ton of "check-something" which are specific to checks, and
if at all I'd rather avoid to add one more.

I was actually wondering if instead we should not consider that verifyhost
presents the *default* value to check against when there's no SNI. From the
perspective of the connection sequence it makes sense : forcing the name
check against an expectedly wrong certificate doesn't make much sense. But
by changing the logic so that SNI overrides verifyhost we could get rid of
this and ensure that health checks continue to work with the name presented
in the default cert.

Technically speaking it would cause a small additional complexity to report
the wrong cert name, but we could have two error codes, one for cert not
matching the SNI and another one for cert not maching verifyhost and I guess
that would solve it.

This would make sense to me. I cant think of a use-case where I'd want to
override the SNI name presented by the client, but a bunch where I'd want to
match the default presented by the backend.

In my case our server naming looks something like

internal.app2.example.ca which is a 10.something address that we cant easily
sign with letsenc (i have to do a dns01 challenge and its all manual
nonsense)...  so for the checks I wouldnt mind telling it to expect the
app2.example.ca hostname in the cert (and configure it to return by
default).

But when its a normal client-requested domain name, I need it to verify
properly against the client's SNI all the way through. If the client asks
for x.example.ca it needs to be secured to the haproxy and the haproxy to
the backend needs full security too.

I totally agree on all of these points. It would also ensure that a lack
of SNI from a client properly lands on the default and expected server
name.


The backend needs the SNI value to select the vhost to serve.

No, it needs it to select the certificate to present. Then it should match
it against the Host header field, and use the Host header field to select
the vhost. The difference is subtle but it's important to keep each protocol
element one in its role. In the end for HTTP it's always the Host which
decides, regardless of the transport.

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 11:49:22AM -0700, Kevin McArthur wrote:
> > I'm still thinking about something like this. What bothers me is that we
> > already have a ton of "check-something" which are specific to checks, and
> > if at all I'd rather avoid to add one more.
> > 
> > I was actually wondering if instead we should not consider that verifyhost
> > presents the *default* value to check against when there's no SNI. From the
> > perspective of the connection sequence it makes sense : forcing the name
> > check against an expectedly wrong certificate doesn't make much sense. But
> > by changing the logic so that SNI overrides verifyhost we could get rid of
> > this and ensure that health checks continue to work with the name presented
> > in the default cert.
> > 
> > Technically speaking it would cause a small additional complexity to report
> > the wrong cert name, but we could have two error codes, one for cert not
> > matching the SNI and another one for cert not maching verifyhost and I guess
> > that would solve it.
> This would make sense to me. I cant think of a use-case where I'd want to
> override the SNI name presented by the client, but a bunch where I'd want to
> match the default presented by the backend.
> 
> In my case our server naming looks something like
> 
> internal.app2.example.ca which is a 10.something address that we cant easily
> sign with letsenc (i have to do a dns01 challenge and its all manual
> nonsense)...  so for the checks I wouldnt mind telling it to expect the
> app2.example.ca hostname in the cert (and configure it to return by
> default).
> 
> But when its a normal client-requested domain name, I need it to verify
> properly against the client's SNI all the way through. If the client asks
> for x.example.ca it needs to be secured to the haproxy and the haproxy to
> the backend needs full security too.

I totally agree on all of these points. It would also ensure that a lack
of SNI from a client properly lands on the default and expected server
name.

> The backend needs the SNI value to select the vhost to serve.

No, it needs it to select the certificate to present. Then it should match
it against the Host header field, and use the Host header field to select
the vhost. The difference is subtle but it's important to keep each protocol
element one in its role. In the end for HTTP it's always the Host which
decides, regardless of the transport.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur

I'm still thinking about something like this. What bothers me is that we
already have a ton of "check-something" which are specific to checks, and
if at all I'd rather avoid to add one more.

I was actually wondering if instead we should not consider that verifyhost
presents the *default* value to check against when there's no SNI. From the
perspective of the connection sequence it makes sense : forcing the name
check against an expectedly wrong certificate doesn't make much sense. But
by changing the logic so that SNI overrides verifyhost we could get rid of
this and ensure that health checks continue to work with the name presented
in the default cert.

Technically speaking it would cause a small additional complexity to report
the wrong cert name, but we could have two error codes, one for cert not
matching the SNI and another one for cert not maching verifyhost and I guess
that would solve it.
This would make sense to me. I cant think of a use-case where I'd want 
to override the SNI name presented by the client, but a bunch where I'd 
want to match the default presented by the backend.


In my case our server naming looks something like

internal.app2.example.ca which is a 10.something address that we cant 
easily sign with letsenc (i have to do a dns01 challenge and its all 
manual nonsense)...  so for the checks I wouldnt mind telling it to 
expect the app2.example.ca hostname in the cert (and configure it to 
return by default).


But when its a normal client-requested domain name, I need it to verify 
properly against the client's SNI all the way through. If the client 
asks for x.example.ca it needs to be secured to the haproxy and the 
haproxy to the backend needs full security too. The backend needs the 
SNI value to select the vhost to serve.


--

Kevin



On 2017-07-26 11:39 AM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 11:25:18AM -0700, Kevin McArthur wrote:

One last thing; the health check process seems to be ignoring the cert
validation logic entirely. Could be the same pathway re default cert though.

In fact it's only enabled when verifyhost is in use, but here that
would mean forcing verifyhost which would be bad.


Its not actually important that we have tls protection on the health check,
but it should be explicitly configured I think, otherwise a future version
that corrects this will run into people needing to generate certificates for
internal servers or completely turn off checking.

Perhaps a check-ssl-verifypeer and check-ssl-verifyhost setting might make
sense to go with check-ssl?

I'm still thinking about something like this. What bothers me is that we
already have a ton of "check-something" which are specific to checks, and
if at all I'd rather avoid to add one more.

I was actually wondering if instead we should not consider that verifyhost
presents the *default* value to check against when there's no SNI. From the
perspective of the connection sequence it makes sense : forcing the name
check against an expectedly wrong certificate doesn't make much sense. But
by changing the logic so that SNI overrides verifyhost we could get rid of
this and ensure that health checks continue to work with the name presented
in the default cert.

Technically speaking it would cause a small additional complexity to report
the wrong cert name, but we could have two error codes, one for cert not
matching the SNI and another one for cert not maching verifyhost and I guess
that would solve it.

So the patches are probably not final...

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 11:38:47AM -0700, Kevin McArthur wrote:
> > However I have a good news. I found that it was possible to access the
> > connection from the verify callback! With a connection comes the ability
> > to place a specific error code which we can verify later. So I did this,
> > 1) add a new error code for a wrong certificate, and 2) add the check for
> > this specific use case (ie: cert name verification failed against a non-
> > hardcoded value, so fail immediately). It now immediately reports the
> > 503 and you don't have the retries anymore.
> 
> This patch is working flawlessly.
> 
> +1 to adding all three patches to master.

Thanks for testing. Let's sleep over this series to see if we can do
something better for the checks before merging all of this.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 11:25:18AM -0700, Kevin McArthur wrote:
> One last thing; the health check process seems to be ignoring the cert
> validation logic entirely. Could be the same pathway re default cert though.

In fact it's only enabled when verifyhost is in use, but here that
would mean forcing verifyhost which would be bad.

> Its not actually important that we have tls protection on the health check,
> but it should be explicitly configured I think, otherwise a future version
> that corrects this will run into people needing to generate certificates for
> internal servers or completely turn off checking.
> 
> Perhaps a check-ssl-verifypeer and check-ssl-verifyhost setting might make
> sense to go with check-ssl?

I'm still thinking about something like this. What bothers me is that we
already have a ton of "check-something" which are specific to checks, and
if at all I'd rather avoid to add one more.

I was actually wondering if instead we should not consider that verifyhost
presents the *default* value to check against when there's no SNI. From the
perspective of the connection sequence it makes sense : forcing the name
check against an expectedly wrong certificate doesn't make much sense. But
by changing the logic so that SNI overrides verifyhost we could get rid of
this and ensure that health checks continue to work with the name presented
in the default cert.

Technically speaking it would cause a small additional complexity to report
the wrong cert name, but we could have two error codes, one for cert not
matching the SNI and another one for cert not maching verifyhost and I guess
that would solve it.

So the patches are probably not final...

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur

However I have a good news. I found that it was possible to access the
connection from the verify callback! With a connection comes the ability
to place a specific error code which we can verify later. So I did this,
1) add a new error code for a wrong certificate, and 2) add the check for
this specific use case (ie: cert name verification failed against a non-
hardcoded value, so fail immediately). It now immediately reports the
503 and you don't have the retries anymore.


This patch is working flawlessly.

+1 to adding all three patches to master.

--

Kevin


On 2017-07-26 11:27 AM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 09:58:57AM -0700, Kevin McArthur wrote:

This seems to stop the primary vector. I can still tie up a valid sni with a
misconfigured backend, but I'm not sure that would be a client-controlled
condition.

And more importantly, the client's SNI is not the only source of SNI,
there are other valid use cases.


Perhaps strict-sni should be defaulted?

No, it would be a pain to a lot of people (ie making it impossible for
a hosting provider to present an error page by default). And as mentionned
above, it would only address the issue in your use case, not all of them.

However I have a good news. I found that it was possible to access the
connection from the verify callback! With a connection comes the ability
to place a specific error code which we can verify later. So I did this,
1) add a new error code for a wrong certificate, and 2) add the check for
this specific use case (ie: cert name verification failed against a non-
hardcoded value, so fail immediately). It now immediately reports the
503 and you don't have the retries anymore.

I'm attaching the two patches that I intend to merge if there's no
objection.

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur

Awesome. I'll try this out right now.

--

Kevin


On 2017-07-26 11:27 AM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 09:58:57AM -0700, Kevin McArthur wrote:

This seems to stop the primary vector. I can still tie up a valid sni with a
misconfigured backend, but I'm not sure that would be a client-controlled
condition.

And more importantly, the client's SNI is not the only source of SNI,
there are other valid use cases.


Perhaps strict-sni should be defaulted?

No, it would be a pain to a lot of people (ie making it impossible for
a hosting provider to present an error page by default). And as mentionned
above, it would only address the issue in your use case, not all of them.

However I have a good news. I found that it was possible to access the
connection from the verify callback! With a connection comes the ability
to place a specific error code which we can verify later. So I did this,
1) add a new error code for a wrong certificate, and 2) add the check for
this specific use case (ie: cert name verification failed against a non-
hardcoded value, so fail immediately). It now immediately reports the
503 and you don't have the retries anymore.

I'm attaching the two patches that I intend to merge if there's no
objection.

Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 09:58:57AM -0700, Kevin McArthur wrote:
> This seems to stop the primary vector. I can still tie up a valid sni with a
> misconfigured backend, but I'm not sure that would be a client-controlled
> condition.

And more importantly, the client's SNI is not the only source of SNI,
there are other valid use cases.

> Perhaps strict-sni should be defaulted?

No, it would be a pain to a lot of people (ie making it impossible for
a hosting provider to present an error page by default). And as mentionned
above, it would only address the issue in your use case, not all of them.

However I have a good news. I found that it was possible to access the
connection from the verify callback! With a connection comes the ability
to place a specific error code which we can verify later. So I did this,
1) add a new error code for a wrong certificate, and 2) add the check for
this specific use case (ie: cert name verification failed against a non-
hardcoded value, so fail immediately). It now immediately reports the
503 and you don't have the retries anymore.

I'm attaching the two patches that I intend to merge if there's no
objection.

Willy
>From d0880fd22431b8f65654b77a0c4c681265fe1131 Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Wed, 26 Jul 2017 20:09:56 +0200
Subject: MINOR: ssl: add a new error code for wrong server certificates

If a server presents an unexpected certificate to haproxy, that is, a
certificate that doesn't match the expected name as configured in
verifyhost or as requested using SNI, we want to store that precious
information. Fortunately we have access to the connection in the
verification callback so it's possible to store an error code there.

For this purpose we use CO_ER_SSL_WRONG_CRT.
---
 include/proto/connection.h | 1 +
 include/types/connection.h | 1 +
 src/ssl_sock.c | 3 +++
 3 files changed, 5 insertions(+)

diff --git a/include/proto/connection.h b/include/proto/connection.h
index 09467ba..2f00863 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -605,6 +605,7 @@ static inline const char *conn_err_code_str(struct 
connection *c)
case CO_ER_SSL_RENEG: return "Rejected a client-initiated SSL 
renegociation attempt";
case CO_ER_SSL_CA_FAIL:   return "SSL client CA chain cannot be 
verified";
case CO_ER_SSL_CRT_FAIL:  return "SSL client certificate not trusted";
+   case CO_ER_SSL_WRONG_CRT: return "Server presented an SSL certificate 
for a wrong name";
case CO_ER_SSL_HANDSHAKE: return "SSL handshake failure";
case CO_ER_SSL_HANDSHAKE_HB: return "SSL handshake failure after 
heartbeat";
case CO_ER_SSL_KILLED_HB: return "Stopped a TLSv1 heartbeat attack 
(CVE-2014-0160)";
diff --git a/include/types/connection.h b/include/types/connection.h
index 1e3fb73..4b5b5d3 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -179,6 +179,7 @@ enum {
CO_ER_SSL_RENEG,/* forbidden client renegociation */
CO_ER_SSL_CA_FAIL,  /* client cert verification failed in the CA 
chain */
CO_ER_SSL_CRT_FAIL, /* client cert verification failed on the 
certificate */
+   CO_ER_SSL_WRONG_CRT,/* server presented a certificate for a wrong 
server */
CO_ER_SSL_HANDSHAKE,/* SSL error during handshake */
CO_ER_SSL_HANDSHAKE_HB, /* SSL error during handshake with heartbeat 
present */
CO_ER_SSL_KILLED_HB,/* Stopped a TLSv1 heartbeat attack 
(CVE-2014-0160) */
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 42d27de..be75d5b 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4001,6 +4001,9 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX 
*ctx)
}
}
 
+   /* report the mismatch */
+   if (!ok && !conn->err_code)
+   conn->err_code = CO_ER_SSL_WRONG_CRT;
return ok;
 }
 
-- 
1.7.12.1

>From 88dc39b889b511304222f4eaf8498e5df8ed7610 Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Wed, 26 Jul 2017 20:13:37 +0200
Subject: BUG/MEDIUM: stream: don't retry SSL connections which fail the
 hostname check

Commits 2ab8867 ("MINOR: ssl: compare server certificate names to the
SNI on outgoing connections") and 96c7b8d ("BUG/MINOR: ssl: Fix check
against SNI during server certificate verification") made it possible
to check that the server's certificate matches the name presented in
the SNI field. While it solves a class of problems, it opens another
one which is that by failing such a connection, we'll retry it and put
more load on the server. It can be a real problem if a user can trigger
this issue, which is what will very often happen when the SNI is forwarded
from the client to the server.

This patch solves this by detecting that this very specific hostname
verification failed and that the hostname was provided using SNI, and
then it simply disables retries and the failure is immediate.

At the time of writing this 

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur
One last thing; the health check process seems to be ignoring the cert 
validation logic entirely. Could be the same pathway re default cert 
though.


Its not actually important that we have tls protection on the health 
check, but it should be explicitly configured I think, otherwise a 
future version that corrects this will run into people needing to 
generate certificates for internal servers or completely turn off checking.


Perhaps a check-ssl-verifypeer and check-ssl-verifyhost setting might 
make sense to go with check-ssl?


--

Kevin


On 2017-07-26 9:57 AM, Kevin McArthur wrote:



On 2017-07-26 9:55 AM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 09:39:03AM -0700, Kevin McArthur wrote:
Interesting. I'd probably recommend not pushing this patch out then 
until

this can be fixed as it will be trivial to resource-exploit a haproxy
instance that is exhibiting a client-controlled retry.

For now we're in development and the previous one was already merged, so
I'd rather merge Christopher's patch at least to benefit from his 
analysis
and from the better solution than the one I came up with. For the 
release

that's a different story though.

Fair enough, I just would be weary the release.



A quick try with a
script that generates randomized SNI names shows I can open connmax and
crash the haproxy from a single instance pretty readily.

What do you mean by "crash haproxy" ? Do you instead mean that it runs
out of connection and cannot connect to the server anymore or that you
managed to kill the process ? Because for me "crash" is this last one
and is a much more serious concern.

The former, just ties up all the connections.
If there's other errors that the client can control that lead to a 
retry

like this, they're probably worthy of a CVE.

It takes approximately 5s per connection to clear the connection in 
this

condition.

I'll see if retries 0 will work for our use case, but I'd hate to 
think we'd

have to give up non-client-controlled retry support entirely (ie for a
backend apache restart, retry to another app server...) due to this.

That's why I warned you that it was a tradeoff :-)

~_~


Willy

--
Kevin






Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur
This seems to stop the primary vector. I can still tie up a valid sni 
with a misconfigured backend, but I'm not sure that would be a 
client-controlled condition.


Perhaps strict-sni should be defaulted?

--

Kevin


On 2017-07-26 9:53 AM, Emmanuel Hocdet wrote:

Hi Kevin,


Le 26 juil. 2017 à 18:39, Kevin McArthur  a écrit :

Interesting. I'd probably recommend not pushing this patch out then until this 
can be fixed as it will be trivial to resource-exploit a haproxy instance that 
is exhibiting a client-controlled retry. A quick try with a script that 
generates randomized SNI names shows I can open connmax and crash the haproxy 
from a single instance pretty readily.

If there's other errors that the client can control that lead to a retry like 
this, they're probably worthy of a CVE.

It takes approximately 5s per connection to clear the connection in this 
condition.

I'll see if retries 0 will work for our use case, but I'd hate to think we'd 
have to give up non-client-controlled retry support entirely (ie for a backend 
apache restart, retry to another app server...) due to this.

—


Yon can add ‘strict-sni’ on bind line to reject all requests with an unknown 
sni.

Manu






Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur



On 2017-07-26 9:55 AM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 09:39:03AM -0700, Kevin McArthur wrote:

Interesting. I'd probably recommend not pushing this patch out then until
this can be fixed as it will be trivial to resource-exploit a haproxy
instance that is exhibiting a client-controlled retry.

For now we're in development and the previous one was already merged, so
I'd rather merge Christopher's patch at least to benefit from his analysis
and from the better solution than the one I came up with. For the release
that's a different story though.

Fair enough, I just would be weary the release.



A quick try with a
script that generates randomized SNI names shows I can open connmax and
crash the haproxy from a single instance pretty readily.

What do you mean by "crash haproxy" ? Do you instead mean that it runs
out of connection and cannot connect to the server anymore or that you
managed to kill the process ? Because for me "crash" is this last one
and is a much more serious concern.

The former, just ties up all the connections.

If there's other errors that the client can control that lead to a retry
like this, they're probably worthy of a CVE.

It takes approximately 5s per connection to clear the connection in this
condition.

I'll see if retries 0 will work for our use case, but I'd hate to think we'd
have to give up non-client-controlled retry support entirely (ie for a
backend apache restart, retry to another app server...) due to this.

That's why I warned you that it was a tradeoff :-)

~_~


Willy

--
Kevin




Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 09:39:03AM -0700, Kevin McArthur wrote:
> Interesting. I'd probably recommend not pushing this patch out then until
> this can be fixed as it will be trivial to resource-exploit a haproxy
> instance that is exhibiting a client-controlled retry.

For now we're in development and the previous one was already merged, so
I'd rather merge Christopher's patch at least to benefit from his analysis
and from the better solution than the one I came up with. For the release
that's a different story though.

> A quick try with a
> script that generates randomized SNI names shows I can open connmax and
> crash the haproxy from a single instance pretty readily.

What do you mean by "crash haproxy" ? Do you instead mean that it runs
out of connection and cannot connect to the server anymore or that you
managed to kill the process ? Because for me "crash" is this last one
and is a much more serious concern.

> If there's other errors that the client can control that lead to a retry
> like this, they're probably worthy of a CVE.
> 
> It takes approximately 5s per connection to clear the connection in this
> condition.
> 
> I'll see if retries 0 will work for our use case, but I'd hate to think we'd
> have to give up non-client-controlled retry support entirely (ie for a
> backend apache restart, retry to another app server...) due to this.

That's why I warned you that it was a tradeoff :-)

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Emmanuel Hocdet
Hi Kevin,

> Le 26 juil. 2017 à 18:39, Kevin McArthur  a écrit :
> 
> Interesting. I'd probably recommend not pushing this patch out then until 
> this can be fixed as it will be trivial to resource-exploit a haproxy 
> instance that is exhibiting a client-controlled retry. A quick try with a 
> script that generates randomized SNI names shows I can open connmax and crash 
> the haproxy from a single instance pretty readily.
> 
> If there's other errors that the client can control that lead to a retry like 
> this, they're probably worthy of a CVE.
> 
> It takes approximately 5s per connection to clear the connection in this 
> condition.
> 
> I'll see if retries 0 will work for our use case, but I'd hate to think we'd 
> have to give up non-client-controlled retry support entirely (ie for a 
> backend apache restart, retry to another app server...) due to this.
> 
> —


Yon can add ‘strict-sni’ on bind line to reject all requests with an unknown 
sni.

Manu




Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur
Interesting. I'd probably recommend not pushing this patch out then 
until this can be fixed as it will be trivial to resource-exploit a 
haproxy instance that is exhibiting a client-controlled retry. A quick 
try with a script that generates randomized SNI names shows I can open 
connmax and crash the haproxy from a single instance pretty readily.


If there's other errors that the client can control that lead to a retry 
like this, they're probably worthy of a CVE.


It takes approximately 5s per connection to clear the connection in this 
condition.


I'll see if retries 0 will work for our use case, but I'd hate to think 
we'd have to give up non-client-controlled retry support entirely (ie 
for a backend apache restart, retry to another app server...) due to this.


--

Kevin





On 2017-07-26 9:26 AM, Willy Tarreau wrote:

On Wed, Jul 26, 2017 at 09:14:08AM -0700, Kevin McArthur wrote:

Looks like this patch works re verifyhost but I think there's still a
problem here. A browser that tries to load an invalid sni name now produces
4 tries to the backend with about a second delay between each attempt,
amplifying the problem. It also takes a good 5 seconds for the connections
to cleanup/close on failure.

That's the normal behaviour here which confirms it works for you (the
previous code does exactly this for me, though after discussing with
Christopher we still fail to understand precisely why it works with
my version of openssl and none with his nor yours but that's another
story).

The thing is that the layer deciding to retry the connection does it
when there is a connection error. An SSL handshake failure is one of
the many possible connection errors. This could be caused by various
things including a server which is a bit slow to start or to load its
certificates.

For now we have no way to say "don't retry if there is this or that
specific type of connection error". Also passing the information that a
failed handshake is caused by a non-matching SNI is further complicated
as everything is done using callbacks at these layers.

The best I can recommend you for now is to set "retries 0" in your
backend to disable connection retries. Ideally we should try to
enumerate the type of errors that should lead to no retry because they
may be controlled by the client.

Regards,
Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Willy Tarreau
On Wed, Jul 26, 2017 at 09:14:08AM -0700, Kevin McArthur wrote:
> Looks like this patch works re verifyhost but I think there's still a
> problem here. A browser that tries to load an invalid sni name now produces
> 4 tries to the backend with about a second delay between each attempt,
> amplifying the problem. It also takes a good 5 seconds for the connections
> to cleanup/close on failure.

That's the normal behaviour here which confirms it works for you (the
previous code does exactly this for me, though after discussing with
Christopher we still fail to understand precisely why it works with
my version of openssl and none with his nor yours but that's another
story).

The thing is that the layer deciding to retry the connection does it
when there is a connection error. An SSL handshake failure is one of
the many possible connection errors. This could be caused by various
things including a server which is a bit slow to start or to load its
certificates.

For now we have no way to say "don't retry if there is this or that
specific type of connection error". Also passing the information that a
failed handshake is caused by a non-matching SNI is further complicated
as everything is done using callbacks at these layers.

The best I can recommend you for now is to set "retries 0" in your
backend to disable connection retries. Ideally we should try to
enumerate the type of errors that should lead to no retry because they
may be controlled by the client.

Regards,
Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Kevin McArthur
Looks like this patch works re verifyhost but I think there's still a 
problem here. A browser that tries to load an invalid sni name now 
produces 4 tries to the backend with about a second delay between each 
attempt, amplifying the problem. It also takes a good 5 seconds for the 
connections to cleanup/close on failure. Pretty sure this could lead to 
resource exhaustion, etc... Perhaps this needs a caching strategy?


[WARNING] 206/160620 (19914) : Health check for server 
www-backend-https/app2 succeeded, reason: Layer6 check passed, check 
duration: 6ms, status: 3/3 UP.:www-https.accept(0004)=0006 from 
[::::33139] 
ALPN=0001:www-https.accept(0004)=0005 from 
[::::45709] 
ALPN=:www-https.clireq[0006:]: GET / 
HTTP/1.1:www-https.clihdr[0006:]: Host: 
ssltest.example.ca:www-https.clihdr[0006:]: Connection: 
keep-alive:www-https.clihdr[0006:]: 
Upgrade-Insecure-Requests: 1:www-https.clihdr[0006:]: 
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) 
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 
Safari/537.36:www-https.clihdr[0006:]: Accept: 
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8:www-https.clihdr[0006:]: 
Accept-Encoding: gzip, deflate, 
br:www-https.clihdr[0006:]: Accept-Language: 
en-US,en;q=0.8:www-backend-https.srvrep[0006:0007]: HTTP/1.1 200 
OK:www-backend-https.srvhdr[0006:0007]: Date: Wed, 26 Jul 2017 
16:06:51 GMT:www-backend-https.srvhdr[0006:0007]: Server: 
Apache:www-backend-https.srvhdr[0006:0007]: Vary: 
Accept-Encoding:www-backend-https.srvhdr[0006:0007]: 
Content-Encoding: gzip:www-backend-https.srvhdr[0006:0007]: 
Content-Length: 458:www-backend-https.srvhdr[0006:0007]: 
Connection: close:www-backend-https.srvhdr[0006:0007]: 
Content-Type: text/html; 
charset=UTF-8:www-backend-https.srvcls[0006:0007]0002:www-https.clireq[0006:]: 
GET /favicon.ico HTTP/1.10002:www-https.clihdr[0006:]: Host: 
ssltest.example.ca0002:www-https.clihdr[0006:]: Connection: 
keep-alive0002:www-https.clihdr[0006:]: User-Agent: 
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 
(KHTML, like Gecko) Chrome/59.0.3071.115 
Safari/537.360002:www-https.clihdr[0006:]: Accept: 
image/webp,image/apng,image/*,*/*;q=0.80002:www-https.clihdr[0006:]: 
Referer: 
https://ssltest.example.ca/0002:www-https.clihdr[0006:]: 
Accept-Encoding: gzip, deflate, 
br0002:www-https.clihdr[0006:]: Accept-Language: 
en-US,en;q=0.80002:www-backend-https.srvrep[0006:0007]: HTTP/1.1 404 
Not Found0002:www-backend-https.srvhdr[0006:0007]: Date: Wed, 26 Jul 
2017 16:06:51 GMT0002:www-backend-https.srvhdr[0006:0007]: Server: 
Apache0002:www-backend-https.srvhdr[0006:0007]: Content-Length: 
2090002:www-backend-https.srvhdr[0006:0007]: Connection: 
close0002:www-backend-https.srvhdr[0006:0007]: Content-Type: 
text/html; 
charset=iso-8859-10002:www-backend-https.srvcls[0006:0007]0004:www-https.accept(0004)=0007 
from [::::41712] 
ALPN=0005:www-https.accept(0004)=0008 from 
[::::35597] 
ALPN=0004:www-https.clireq[0007:]: GET / 
HTTP/1.10004:www-https.clihdr[0007:]: Host: 
ssltest-broken.example.ca0004:www-https.clihdr[0007:]: 
Connection: keep-alive0004:www-https.clihdr[0007:]: 
Upgrade-Insecure-Requests: 10004:www-https.clihdr[0007:]: 
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) 
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 
Safari/537.360004:www-https.clihdr[0007:]: Accept: 
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.80004:www-https.clihdr[0007:]: 
Accept-Encoding: gzip, deflate, 
br0004:www-https.clihdr[0007:]: Accept-Language: 
en-US,en;q=0.8*fd[0009] OpenSSL error[0x14090086] 
ssl3_get_server_certificate: certificate verify failedfd[0009] 
OpenSSL error[0x14090086] ssl3_get_server_certificate: certificate 
verify failedfd[0009] OpenSSL error[0x14090086] 
ssl3_get_server_certificate: certificate verify failedfd[0009] 
OpenSSL error[0x14090086] ssl3_get_server_certificate: certificate 
verify 
failed*0004:www-backend-https.clicls[0007:adfd]0004:www-backend-https.closed[0007:adfd]0005:www-https.clireq[0008:]: 
GET /favicon.ico HTTP/1.10005:www-https.clihdr[0008:]: Host: 
ssltest-broken.example.ca0005:www-https.clihdr[0008:]: 
Connection: keep-alive0005:www-https.clihdr[0008:]: 
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) 
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 
Safari/537.360005:www-https.clihdr[0008:]: Accept: 

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-26 Thread Christopher Faulet

.Le 25/07/2017 à 19:37, Kevin McArthur a écrit :

Hi Willy,

I cant replicate your results here

I cloned from git and built the package with the debian/ubuntu build 
scripts from https://launchpad.net/~vbernat/+archive/ubuntu/haproxy-1.7 
... updating the changelog to add a 1.8-dev2 version and calling 
./debian/rules binary to build the package.


The git log shows:

commit 2ab88675ecbf960a6f33ffe9c6a27f264150b201
Author: Willy Tarreau 
Date:   Wed Jul 5 18:23:03 2017 +0200

 MINOR: ssl: compare server certificate names to the SNI on
outgoing connections




Hi,

There is a bug in this commit. I checked with openssl 1.0.2l and 1.1.0f 
and I observed the same behavior than Kevin's one. 
SSL_SESSION_get0_hostname seems to always return NULL when the server 
returns a default certificate.


It tried to explain why in my commit log.

Kevin, could you check the patch in attachment to confirm it works ?

--
Christopher Faulet
>From afe2d426c6aeb82aa11af842e8f75f54a2d9130d Mon Sep 17 00:00:00 2001
From: Christopher Faulet 
Date: Wed, 26 Jul 2017 11:50:01 +0200
Subject: [PATCH] BUG/MINOR: ssl: Fix check against SNI during server
 certificate verification

This patch fixes the commit 2ab8867 ("MINOR: ssl: compare server certificate
names to the SNI on outgoing connections")

When we check the certificate sent by a server, in the verify callback, we get
the SNI from the session (SSL_SESSION object). In OpenSSL, tlsext_hostname value
for this session is copied from the ssl connection (SSL object). But the copy is
done only if the "server_name" extension is found in the server hello
message. This means the server has found a certificate matching the client's
SNI.

When the server returns a default certificate not matching the client's SNI, it
doesn't set any "server_name" extension in the server hello message. So no SNI
is set on the SSL session and SSL_SESSION_get0_hostname always returns NULL.

To fix the problemn, we get the SNI directly from the SSL connection. It is
always defined with the value set by the client.

If the commit 2ab8867 is backported in 1.7 and/or 1.8, this one must be
backported too.
---
 include/proto/openssl-compat.h | 5 -
 src/ssl_sock.c | 6 +-
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/include/proto/openssl-compat.h b/include/proto/openssl-compat.h
index a1e75b47..ea92072e 100644
--- a/include/proto/openssl-compat.h
+++ b/include/proto/openssl-compat.h
@@ -94,11 +94,6 @@ static inline int SSL_SESSION_set1_id_context(SSL_SESSION *s, const unsigned cha
  * Functions introduced in OpenSSL 1.1.0 and not yet present in LibreSSL
  */
 
-static inline const char *SSL_SESSION_get0_hostname(const SSL_SESSION *sess)
-{
-   return sess->tlsext_hostname;
-}
-
 static inline const unsigned char *SSL_SESSION_get0_id_context(const SSL_SESSION *sess, unsigned int *sid_ctx_length)
 {
 	*sid_ctx_length = sess->sid_ctx_length;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index fa815715..42d27de9 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3951,11 +3951,7 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX *ctx)
 	 */
 	servername = objt_server(conn->target)->ssl_ctx.verify_host;
 	if (!servername) {
-		SSL_SESSION *ssl_sess = SSL_get_session(conn->xprt_ctx);
-		if (!ssl_sess)
-			return ok;
-
-		servername = SSL_SESSION_get0_hostname(ssl_sess);
+		servername = SSL_get_servername(conn->xprt_ctx, TLSEXT_NAMETYPE_host_name);
 		if (!servername)
 			return ok;
 	}
-- 
2.13.3



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-25 Thread Kevin McArthur



On 2017-07-25 10:51 AM, Willy Tarreau wrote:

On Tue, Jul 25, 2017 at 10:37:10AM -0700, Kevin McArthur wrote:

Hi Willy,

I cant replicate your results here

I cloned from git and built the package with the debian/ubuntu build scripts
from https://launchpad.net/~vbernat/+archive/ubuntu/haproxy-1.7 ... updating
the changelog to add a 1.8-dev2 version and calling ./debian/rules binary to
build the package.

The git log shows:

commit 2ab88675ecbf960a6f33ffe9c6a27f264150b201
Author: Willy Tarreau 
Date:   Wed Jul 5 18:23:03 2017 +0200

 MINOR: ssl: compare server certificate names to the SNI on
outgoing connections


So I'm sure its in there unless a  ./debian/rules binary build is breaking
something.

OK that's already a good thing.


this is my config.

(...)

So as you can see, ssltest-broken is hitting the app2 default vhost/cert.
The backend server has no knowledge of the ssltest-broken certificate. The
verifyhost is /not/ checked between the backend and the haproxy. Further, I
think the health check should probably fail too because its trying to load
via the ip-as-hostname and the cert im using doesnt have the IP in it. So
that should fail hostname check too.

No, the health check doesn't present any SNI since there's no ssl_fc_sni
for it. But anyway I don't understand the difference in the setup.
If the health check is connecting but not presenting any SNI, it would, 
in my setup, be getting back a cert for app2.example.ca. That obviously 
wont hostname match to 10.10.0.5 ip in the server line and thus the 
health check connection should fail. If i add an explicit verifyhost the 
health checks do indeed fail on anything but a static app2.example.ca 
string.



I'm confident that the verifyhost is not being done...  I suspect your test
case is failing because the dom4 is totally unknown to the haproxy, whereas
in my case, the haproxy has a cert for ssltest-broken but the backend does
not.

No, it's irrelevant, we're only relying on SNI here, the backend has no
info on the knowledge of what lies on the front or not. I'll try with the
same certs on the front for completeness. But in my case I clearly see
verifyhost fail on a mismatched name between the server and the client,
so I'll have to investigate further.
In my test case the backend is presenting a valid cert with 
SAN=app2.example.ca to the frontend when it is asked for a sni name it 
does not know. The frontend haproxy having the cert, serves the correct 
and valid cert (ssltest-broken.example.ca) to the browser. The content 
of the served page is the app2 default page. Perhaps where your test 
might be going sideways is that lack of an otherwise valid but 
incorrectly hostnamed cert coming from the backend? Ie, the cert from 
the backend from dom4 needs to pass verifypeer check but fail the 
verifyhost check to replicate the condition.




Willy

--
Kevin



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-25 Thread Willy Tarreau
On Tue, Jul 25, 2017 at 10:37:10AM -0700, Kevin McArthur wrote:
> Hi Willy,
> 
> I cant replicate your results here
> 
> I cloned from git and built the package with the debian/ubuntu build scripts
> from https://launchpad.net/~vbernat/+archive/ubuntu/haproxy-1.7 ... updating
> the changelog to add a 1.8-dev2 version and calling ./debian/rules binary to
> build the package.
> 
> The git log shows:
> 
>commit 2ab88675ecbf960a6f33ffe9c6a27f264150b201
>Author: Willy Tarreau 
>Date:   Wed Jul 5 18:23:03 2017 +0200
> 
> MINOR: ssl: compare server certificate names to the SNI on
>outgoing connections
> 
> 
> So I'm sure its in there unless a  ./debian/rules binary build is breaking
> something.

OK that's already a good thing.

> this is my config.
(...)
> So as you can see, ssltest-broken is hitting the app2 default vhost/cert.
> The backend server has no knowledge of the ssltest-broken certificate. The
> verifyhost is /not/ checked between the backend and the haproxy. Further, I
> think the health check should probably fail too because its trying to load
> via the ip-as-hostname and the cert im using doesnt have the IP in it. So
> that should fail hostname check too.

No, the health check doesn't present any SNI since there's no ssl_fc_sni
for it. But anyway I don't understand the difference in the setup.

> I'm confident that the verifyhost is not being done...  I suspect your test
> case is failing because the dom4 is totally unknown to the haproxy, whereas
> in my case, the haproxy has a cert for ssltest-broken but the backend does
> not.

No, it's irrelevant, we're only relying on SNI here, the backend has no
info on the knowledge of what lies on the front or not. I'll try with the
same certs on the front for completeness. But in my case I clearly see
verifyhost fail on a mismatched name between the server and the client,
so I'll have to investigate further.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-25 Thread Kevin McArthur

Hi Willy,

I cant replicate your results here

I cloned from git and built the package with the debian/ubuntu build 
scripts from https://launchpad.net/~vbernat/+archive/ubuntu/haproxy-1.7 
... updating the changelog to add a 1.8-dev2 version and calling 
./debian/rules binary to build the package.


The git log shows:

   commit 2ab88675ecbf960a6f33ffe9c6a27f264150b201
   Author: Willy Tarreau 
   Date:   Wed Jul 5 18:23:03 2017 +0200

MINOR: ssl: compare server certificate names to the SNI on
   outgoing connections


So I'm sure its in there unless a  ./debian/rules binary build is 
breaking something.


this is my config.

haproxy-min-sni.cfg

global
ca-base /etc/ssl/certs
crt-base /etc/ssl/private

ssl-default-bind-ciphers 
ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:DH+3DES:RSA+AESGCM:RSA+AES:RSA+3DES:!aNULL:!MD5:!DSS

ssl-default-bind-options no-sslv3

defaults
modehttp
optionhttplog
optiondontlognull
optionforwardfor
optionhttp-server-close
option  log-health-checks
timeout connect 5000
timeout client  5
timeout server  5

frontend www-https
bind :::443 v4v6 ssl crt /etc/haproxy/certs/www.example.ca.pem crt 
/etc/haproxy/certs/

reqadd X-Forwarded-Proto:\ https
use_backend www-backend-https

backend www-backend-https
http-response set-header X-Server %s
balance roundrobin
server app2 10.10.0.5:443 ssl verify required sni ssl_fc_sni 
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl


--

/usr/sbin/haproxy -d -f haproxy-min-sni.cfg

--

Loading ssltest-broken.example.ca (that the backend server has no cert 
for and so serves from the default tls vhost (app2.example.ca in this 
case))... This shows a secure page in the browser, however the 
connection to the backend cannot be secure.


[WARNING] 205/165327 (16816) : Health check for server 
www-backend-https/app2 succeeded, reason: Layer6 check passed, check 
duration: 5ms, status: 3/3 UP.
:www-https.accept(0004)=0007 from [::::36565] 
ALPN=
0001:www-https.accept(0004)=0006 from [::::45955] 
ALPN=
0002:www-https.accept(0004)=0005 from [::::44474] 
ALPN=

:www-https.clireq[0007:]: GET / HTTP/1.1
:www-https.clihdr[0007:]: Host: ssltest-broken.example.ca
:www-https.clihdr[0007:]: Connection: keep-alive
:www-https.clihdr[0007:]: Cache-Control: max-age=0
:www-https.clihdr[0007:]: Upgrade-Insecure-Requests: 1
:www-https.clihdr[0007:]: User-Agent: Mozilla/5.0 
(Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like 
Gecko) Chrome/59.0.3071.115 Safari/537.36
:www-https.clihdr[0007:]: Accept: 
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8

:www-https.clihdr[0007:]: Accept-Encoding: gzip, deflate, br
:www-https.clihdr[0007:]: Accept-Language: en-US,en;q=0.8
:www-backend-https.srvrep[0007:0008]: HTTP/1.1 200 OK
:www-backend-https.srvhdr[0007:0008]: Date: Tue, 25 Jul 2017 
16:49:19 GMT

:www-backend-https.srvhdr[0007:0008]: Server: Apache
:www-backend-https.srvhdr[0007:0008]: Vary: Accept-Encoding
:www-backend-https.srvhdr[0007:0008]: Content-Encoding: gzip
:www-backend-https.srvhdr[0007:0008]: Content-Length: 515
:www-backend-https.srvhdr[0007:0008]: Connection: close
:www-backend-https.srvhdr[0007:0008]: Content-Type: text/html; 
charset=UTF-8

:www-backend-https.srvcls[0007:0008]


Loading ssltest.example.ca
[WARNING] 205/165327 (16816) : Health check for server 
www-backend-https/app2 succeeded, reason: Layer6 check passed, check 
duration: 5ms, status: 3/3 UP.
:www-https.accept(0004)=0005 from [::::45095] 
ALPN=
0001:www-https.accept(0004)=0006 from [::::41897] 
ALPN=
0002:www-https.accept(0004)=0007 from [::::37526] 
ALPN=

:www-https.clireq[0005:]: GET / HTTP/1.1
:www-https.clihdr[0005:]: Host: ssltest.example.ca
:www-https.clihdr[0005:]: Connection: keep-alive
:www-https.clihdr[0005:]: Upgrade-Insecure-Requests: 1
:www-https.clihdr[0005:]: User-Agent: Mozilla/5.0 
(Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like 
Gecko) Chrome/59.0.3071.115 Safari/537.36
:www-https.clihdr[0005:]: Accept: 
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8

:www-https.clihdr[0005:]: Accept-Encoding: gzip, deflate, br
:www-https.clihdr[0005:]: Accept-Language: en-US,en;q=0.8
:www-backend-https.srvrep[0005:0008]: HTTP/1.1 200 OK
:www-backend-https.srvhdr[0005:0008]: Date: Tue, 25 Jul 2017 
16:53:33 GMT

:www-backend-https.srvhdr[0005:0008]: Server: Apache

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-25 Thread Willy Tarreau
Hi again Kevin,

On Tue, Jul 25, 2017 at 07:26:07AM +0200, Willy Tarreau wrote:
> > frontend www-https
> > bind :::443 v4v6 ssl crt /etc/haproxy/certs/default.example.ca.pem crt
> > /etc/haproxy/certs/
> > use_backend www-backend-https
> > 
> > backend www-backend-https
> > server app default.example.ca:443 ssl verify required sni ssl_fc_sni
> > ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl
> > 
> > If you visit https://should-be-broken.example.ca you will get the page for
> > default.example.ca, but the browser/visitor will show the
> > should-be-broken.example.ca cert from the haproxy and the page will appear
> > secure, despite the backend apache instance having no access to
> > should-be-broken's virtual host or certificate and serving a certificate for
> > default.example.ca to the haproxy.
> 
> Thanks, I'll retry it. I'm surprized because what you describe here is
> *exactly* what I did and it worked fine for me, I remember getting a 503
> when connecting with the wrong name. But obviously there must be a
> difference so I'll try to find it.

So I tried again to replicate it and cannot confirm your issues. Here's
what I've done :
  - I'm having haproxy serve as the origin because I don't have an apache
instance running and don't know how to set it up so I'm not going to
waste my time on it ;

  - this origin server responds on 3 different domain names and thus
serves 3 different certificates (dom{1,2,3}.example.com).

  - a front gateway responds on a dummy cert, and connects to the server
passing the front connection's SNI to the server.

  - the client connects to this front gateway with 4 different names, the
3 supported ones and an unsupported one

What I'm seeing is that the first 3 domains work well and the 4th fails.

Here's the config :

  listen gateway
mode http
bind :4430 ssl crt rsa2048.pem
server app 127.0.0.1:4431 ssl sni ssl_fc_sni verify required ca-file 
ca.pem check check-ssl

  frontend origin
mode http
bind :4431 ssl crt dom1.example.com.pem crt dom2.example.com.pem crt 
dom3.example.com.pem
http-request redirect location /called-with-%[ssl_fc_sni]

Command to start this and output :
  $ ./haproxy -d -f sni-srv-bug.cfg

Test with dom1..dom3 :
  $ printf "GET / HTTP/1.0\r\n\r\n" | openssl s_client -connect 127.0.0.1:4430 
-quiet -servername dom1.example.com

Haproxy's output :
  0004:origin.accept(0005)=0007 from [127.0.0.1:36664] ALPN=
  0004:origin.clicls[0007:]
  0004:origin.closed[0007:]
  0005:gateway.accept(0004)=0006 from [127.0.0.1:56942] ALPN=
  0005:gateway.clireq[0006:]: GET / HTTP/1.0
  0006:origin.accept(0005)=0008 from [127.0.0.1:36668] ALPN=
  0006:origin.clireq[0008:]: GET / HTTP/1.0
  0006:origin.clicls[0008:]
  0006:origin.closed[0008:]
  0005:gateway.srvrep[0006:0007]: HTTP/1.1 302 Found
  0005:gateway.srvhdr[0006:0007]: Cache-Control: no-cache
  0005:gateway.srvhdr[0006:0007]: Content-length: 0
  0005:gateway.srvhdr[0006:0007]: Location: /called-with-dom1.example.com
  0005:gateway.srvhdr[0006:0007]: Connection: close
  0005:gateway.srvcls[0006:0007]
  0005:gateway.clicls[0006:0007]
  0005:gateway.closed[0006:0007]
  0007:origin.accept(0005)=0007 from [127.0.0.1:36670] ALPN=
  0007:origin.clicls[0007:]
  0007:origin.closed[0007:]

OpenSSL output :
  depth=0 C = FR, ST = Some-State, O = test, CN = localhost
  verify error:num=18:self signed certificate
  verify return:1
  depth=0 C = FR, ST = Some-State, O = test, CN = localhost
  verify return:1
  HTTP/1.1 302 Found
  Cache-Control: no-cache
  Content-length: 0
  Location: /called-with-dom1.example.com
  Connection: close
  
Test with dom4:
  $ printf "GET / HTTP/1.0\r\n\r\n" | openssl s_client -connect 127.0.0.1:4430 
-quiet -servername dom4.example.com

Haproxy's output :
  :origin.accept(0005)=0007 from [127.0.0.1:36640] ALPN=
  :origin.clicls[0007:]
  :origin.closed[0007:]
  0001:gateway.accept(0004)=0006 from [127.0.0.1:56918] ALPN=
  0001:gateway.clireq[0006:]: GET / HTTP/1.0
  fd[0007] OpenSSL error[0x14090086] ssl3_get_server_certificate: certificate 
verify failed
  fd[0008] OpenSSL error[0x14094438] ssl3_read_bytes: tlsv1 alert internal error
  0002:origin.accept(0005)=0008 from [127.0.0.1:36646] ALPN=
  0002:origin.clicls[0008:]
  0002:origin.closed[0008:]
  fd[0007] OpenSSL error[0x14090086] ssl3_get_server_certificate: certificate 
verify failed
  fd[0008] OpenSSL error[0x14094438] ssl3_read_bytes: tlsv1 alert internal error
  fd[0007] OpenSSL error[0x14090086] ssl3_get_server_certificate: certificate 
verify failed
  fd[0008] OpenSSL error[0x14094438] ssl3_read_bytes: tlsv1 alert internal error
  0003:origin.accept(0005)=0008 from [127.0.0.1:36652] ALPN=
  

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-24 Thread Willy Tarreau
Hi Kevin,

On Mon, Jul 24, 2017 at 04:00:04PM -0700, Kevin McArthur wrote:
> To replicate my results:
> 
> Generate 3 ssl certificates (letsenc? I used a dns-01 challenge...)..
> 
> default.example.ca
> working.example.ca
> should-be-broken.example.ca
> 
> Configure an apache instance to serve only the first two via https.
> default.example.ca and working.example.ca; don't configure any virtualhost
> for should-be-broken.example.ca.
> 
> Configure the haproxy instance with all 3 certificates in the haproxy format
> with the intermediates and private keys included in a single file. Name the
> files like default.example.ca.pem, working.example.ca.pem,
> should-be-broken.example.ca.pem and toss em in /etc/haproxy/certs...
> 
> Install the ca-certificates package if you're on debian/ubuntu (otherwise
> adjust the ca-certificates location to whatever distro you're running)...
> 
> Then:
> 
> haproxy.cfg:
> 
> frontend www-https
> bind :::443 v4v6 ssl crt /etc/haproxy/certs/default.example.ca.pem crt
> /etc/haproxy/certs/
> use_backend www-backend-https
> 
> backend www-backend-https
> server app default.example.ca:443 ssl verify required sni ssl_fc_sni
> ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl
> 
> If you visit https://should-be-broken.example.ca you will get the page for
> default.example.ca, but the browser/visitor will show the
> should-be-broken.example.ca cert from the haproxy and the page will appear
> secure, despite the backend apache instance having no access to
> should-be-broken's virtual host or certificate and serving a certificate for
> default.example.ca to the haproxy.

Thanks, I'll retry it. I'm surprized because what you describe here is
*exactly* what I did and it worked fine for me, I remember getting a 503
when connecting with the wrong name. But obviously there must be a
difference so I'll try to find it.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-24 Thread Kevin McArthur

To replicate my results:

Generate 3 ssl certificates (letsenc? I used a dns-01 challenge...)..

default.example.ca
working.example.ca
should-be-broken.example.ca

Configure an apache instance to serve only the first two via https. 
default.example.ca and working.example.ca; don't configure any 
virtualhost for should-be-broken.example.ca.


Configure the haproxy instance with all 3 certificates in the haproxy 
format with the intermediates and private keys included in a single 
file. Name the files like default.example.ca.pem, 
working.example.ca.pem, should-be-broken.example.ca.pem and toss em in 
/etc/haproxy/certs...


Install the ca-certificates package if you're on debian/ubuntu 
(otherwise adjust the ca-certificates location to whatever distro you're 
running)...


Then:

haproxy.cfg:

frontend www-https
bind :::443 v4v6 ssl crt /etc/haproxy/certs/default.example.ca.pem 
crt /etc/haproxy/certs/

use_backend www-backend-https

backend www-backend-https
server app default.example.ca:443 ssl verify required sni 
ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl


If you visit https://should-be-broken.example.ca you will get the page 
for default.example.ca, but the browser/visitor will show the 
should-be-broken.example.ca cert from the haproxy and the page will 
appear secure, despite the backend apache instance having no access to 
should-be-broken's virtual host or certificate and serving a certificate 
for default.example.ca to the haproxy.


--
Kevin




On 2017-07-24 3:25 PM, Kevin McArthur wrote:


Hi Willy,

I can confirm the following line does _not_ verify the hostname on the 
backend.


server app2 ssltest.example.ca:443 ssl verify required sni 
ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl


I setup a default https vhost on the backend server, that responds to 
ssltest.example.ca ... in the above configuration the health checks 
pass and if you visit a sni-configured site (otherhost.example.ca) it 
will also work. However, you load another SNI virtualhost that only 
the haproxy has a cert for but for which the backend responds with a 
mismatching ssltest certificate, it will happily load that site, 
present it as TLS protected to the web browser and *NOT check the 
verifyhost against the backend SNI. *


That is to say, without verifyhost  no hostname 
verification is done between the haproxy and the cert presented by the 
backend and any valid certificate (even the default-configured ssltest 
one) will work on the backend.


--

Kevin McArthur


On 2017-07-23 9:40 PM, Willy Tarreau wrote:

Hi Kevin,

On Fri, Jul 21, 2017 at 02:06:52PM -0700, Kevin McArthur wrote:

Further... the odd/broken behavior might be being caused related to no sni
indication on the health checks...

This config sort of works:


*server app2 ssltest.example.ca:443 ssl verify required /verifyhost
ssltest.example.ca/ sni ssl_fc_sni ca-file
/etc/ssl/certs/ca-certificates.crt check check-ssl*

This lets me load ssltest.example.ca via the proxy.


*server app2 anotherdomain.example.ca:443 ssl verify required /verifyhost
anotherdomain.example.ca/ sni ssl_fc_sni ca-file
/etc/ssl/certs/ca-certificates.crt check check-ssl*

Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server
www-backend-https/app2 failed, reason: Layer6 invalid response, info: "SSL
handshake failure", check duration: 3ms, status: 0/2 DOWN.

Fails health check (no sni) verifyhost match (anotherdomain.example.ca isnt
the default on the backend server). So ends up in "No server is available to
handle this request."


*server app2 ssltest.example.ca:443 ssl verify required /verifyhost
ssl_fc_sni/ sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check
check-ssl*

Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server
www-backend-https/app2 failed, reason: Layer6 invalid response, info: "SSL
handshake failure", check duration: 3ms, status: 0/2 DOWN.

This fails health check.


*server app2 ssltest.example.ca:443 ssl verify required sni ssl_fc_sni
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl*

This works, but without verifying the host properly. Can load
anotherdomain.example.ca and the sni is passed along properly.


Perhaps its the host checks sni support and not this patch that are not
working correctly?

The "verifyhost" directive *forces* the host name to be checked and ignores
the SNI. By just removing it from your "server" lines, it must be OK. Your
last example above suggests it works. Why do you say that the host is not
properly verified ? Have you checked that you can connect to a server
presenting the wrong cert ? For me it refuses it and only accepts the
correct cert (the one having the same name as asked in the SNI).

Willy






Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-24 Thread Kevin McArthur

Hi Willy,

I can confirm the following line does _not_ verify the hostname on the 
backend.


server app2 ssltest.example.ca:443 ssl verify required sni 
ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl


I setup a default https vhost on the backend server, that responds to 
ssltest.example.ca ... in the above configuration the health checks pass 
and if you visit a sni-configured site (otherhost.example.ca) it will 
also work. However, you load another SNI virtualhost that only the 
haproxy has a cert for but for which the backend responds with a 
mismatching ssltest certificate, it will happily load that site, present 
it as TLS protected to the web browser and *NOT check the verifyhost 
against the backend SNI. *


That is to say, without verifyhost  no hostname verification 
is done between the haproxy and the cert presented by the backend and 
any valid certificate (even the default-configured ssltest one) will 
work on the backend.


--

Kevin McArthur


On 2017-07-23 9:40 PM, Willy Tarreau wrote:

Hi Kevin,

On Fri, Jul 21, 2017 at 02:06:52PM -0700, Kevin McArthur wrote:

Further... the odd/broken behavior might be being caused related to no sni
indication on the health checks...

This config sort of works:


*server app2 ssltest.example.ca:443 ssl verify required /verifyhost
ssltest.example.ca/ sni ssl_fc_sni ca-file
/etc/ssl/certs/ca-certificates.crt check check-ssl*

This lets me load ssltest.example.ca via the proxy.


*server app2 anotherdomain.example.ca:443 ssl verify required /verifyhost
anotherdomain.example.ca/ sni ssl_fc_sni ca-file
/etc/ssl/certs/ca-certificates.crt check check-ssl*

Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server
www-backend-https/app2 failed, reason: Layer6 invalid response, info: "SSL
handshake failure", check duration: 3ms, status: 0/2 DOWN.

Fails health check (no sni) verifyhost match (anotherdomain.example.ca isnt
the default on the backend server). So ends up in "No server is available to
handle this request."


*server app2 ssltest.example.ca:443 ssl verify required /verifyhost
ssl_fc_sni/ sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check
check-ssl*

Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server
www-backend-https/app2 failed, reason: Layer6 invalid response, info: "SSL
handshake failure", check duration: 3ms, status: 0/2 DOWN.

This fails health check.


*server app2 ssltest.example.ca:443 ssl verify required sni ssl_fc_sni
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl*

This works, but without verifying the host properly. Can load
anotherdomain.example.ca and the sni is passed along properly.


Perhaps its the host checks sni support and not this patch that are not
working correctly?

The "verifyhost" directive *forces* the host name to be checked and ignores
the SNI. By just removing it from your "server" lines, it must be OK. Your
last example above suggests it works. Why do you say that the host is not
properly verified ? Have you checked that you can connect to a server
presenting the wrong cert ? For me it refuses it and only accepts the
correct cert (the one having the same name as asked in the SNI).

Willy




Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-23 Thread Willy Tarreau
Hi Kevin,

On Fri, Jul 21, 2017 at 02:06:52PM -0700, Kevin McArthur wrote:
> Further... the odd/broken behavior might be being caused related to no sni
> indication on the health checks...
> 
> This config sort of works:
> 
> 
> *server app2 ssltest.example.ca:443 ssl verify required /verifyhost
> ssltest.example.ca/ sni ssl_fc_sni ca-file
> /etc/ssl/certs/ca-certificates.crt check check-ssl*
> 
> This lets me load ssltest.example.ca via the proxy.
> 
> 
> *server app2 anotherdomain.example.ca:443 ssl verify required /verifyhost
> anotherdomain.example.ca/ sni ssl_fc_sni ca-file
> /etc/ssl/certs/ca-certificates.crt check check-ssl*
> 
> Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server
> www-backend-https/app2 failed, reason: Layer6 invalid response, info: "SSL
> handshake failure", check duration: 3ms, status: 0/2 DOWN.
> 
> Fails health check (no sni) verifyhost match (anotherdomain.example.ca isnt
> the default on the backend server). So ends up in "No server is available to
> handle this request."
> 
> 
> *server app2 ssltest.example.ca:443 ssl verify required /verifyhost
> ssl_fc_sni/ sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt check
> check-ssl*
> 
> Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server
> www-backend-https/app2 failed, reason: Layer6 invalid response, info: "SSL
> handshake failure", check duration: 3ms, status: 0/2 DOWN.
> 
> This fails health check.
> 
> 
> *server app2 ssltest.example.ca:443 ssl verify required sni ssl_fc_sni
> ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl*
> 
> This works, but without verifying the host properly. Can load
> anotherdomain.example.ca and the sni is passed along properly.
> 
> 
> Perhaps its the host checks sni support and not this patch that are not
> working correctly?

The "verifyhost" directive *forces* the host name to be checked and ignores
the SNI. By just removing it from your "server" lines, it must be OK. Your
last example above suggests it works. Why do you say that the host is not
properly verified ? Have you checked that you can connect to a server
presenting the wrong cert ? For me it refuses it and only accepts the
correct cert (the one having the same name as asked in the SNI).

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-21 Thread Kevin McArthur
Further... the odd/broken behavior might be being caused related to no 
sni indication on the health checks...


This config sort of works:


*server app2 ssltest.example.ca:443 ssl verify required /verifyhost 
ssltest.example.ca/ sni ssl_fc_sni ca-file 
/etc/ssl/certs/ca-certificates.crt check check-ssl*


This lets me load ssltest.example.ca via the proxy.


*server app2 anotherdomain.example.ca:443 ssl verify required 
/verifyhost anotherdomain.example.ca/ sni ssl_fc_sni ca-file 
/etc/ssl/certs/ca-certificates.crt check check-ssl*


Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server 
www-backend-https/app2 failed, reason: Layer6 invalid response, info: 
"SSL handshake failure", check duration: 3ms, status: 0/2 DOWN.


Fails health check (no sni) verifyhost match (anotherdomain.example.ca 
isnt the default on the backend server). So ends up in "No server is 
available to handle this request."



*server app2 ssltest.example.ca:443 ssl verify required /verifyhost 
ssl_fc_sni/ sni ssl_fc_sni ca-file /etc/ssl/certs/ca-certificates.crt 
check check-ssl*


Jul 21 20:57:55 haproxy1 haproxy[8371]: Health check for server 
www-backend-https/app2 failed, reason: Layer6 invalid response, info: 
"SSL handshake failure", check duration: 3ms, status: 0/2 DOWN.


This fails health check.


*server app2 ssltest.example.ca:443 ssl verify required sni ssl_fc_sni 
ca-file /etc/ssl/certs/ca-certificates.crt check check-ssl*


This works, but without verifying the host properly. Can load 
anotherdomain.example.ca and the sni is passed along properly.



Perhaps its the host checks sni support and not this patch that are not 
working correctly?


--

Kevin


On 2017-07-21 1:01 PM, Kevin McArthur wrote:


Ok finally got around to testing this out today; running into a bit of 
an issue with the new syntax.


What I'm trying to achieve is:

frontend www-https

bind :::443 v4v6 ssl crt /etc/haproxy/certs/www.example.org.pem 
crt /etc/haproxy/certs/


backend www-backend-https

server ssl 10.0.0.1:443 ssl verify required verifyhost ssl_fc_sni 
sni ssl_fc_sni check-ssl


The closest config format from your patch email is:

server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem *sni req.hdr(host)*


But that is reading the host header and passing it along (which isn't 
the same as the SNI indication field per the protocol). The Host 
header is user controlled and occurs after sni and tls validation -- 
I'd need more time to play with exploiting this but I think you could 
use this to send a different-than-checked host header to the inside 
server as if it were the client SNI value. Translating an 
non-validated user header into the tls protected sni field?


So what I need here is the ability to pass the clients' SNI value 
along to the inside server I think this should be ssl_fc_sni ... 
but if i try the above syntax it doesn't work right.


The following criteria should all be true:

1. The sni indication should match the clients stated host header. 
(verifyhost = ssl_fc_sni)


2. The sni value should be passed along to the backend server. (sni = 
ssl_fc_sni)


3. The client to the haproxy and backend could be SNI indicating any 
domain name, but should be TLS terminated to a matching cert in 
/etc/haproxy/certs, or hit the default cert  and then both 
verify=required and verifyhost=ssl_fc_sni should pass against the 
backend server or an error should result.



I might just be missing something with the config here, but I don't 
think the patch allows for passing along the actual ssl_fc_scni?



--

Kevin




On 2017-07-06 7:20 AM, Kevin McArthur wrote:

I'll see if I can give this a test. Thanks for adding it to master!

--

Kevin


On 2017-07-06 6:19 AM, Willy Tarreau wrote:

Hi again,

I finally merged it in master as commit 2ab8867, it will ease testing
(and a test file was provided).

Cheers,
Willy








Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-21 Thread Kevin McArthur
Ok finally got around to testing this out today; running into a bit of 
an issue with the new syntax.


What I'm trying to achieve is:

frontend www-https

bind :::443 v4v6 ssl crt /etc/haproxy/certs/www.example.org.pem crt 
/etc/haproxy/certs/


backend www-backend-https

server ssl 10.0.0.1:443 ssl verify required verifyhost ssl_fc_sni 
sni ssl_fc_sni check-ssl


The closest config format from your patch email is:

server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem *sni req.hdr(host)*


But that is reading the host header and passing it along (which isn't 
the same as the SNI indication field per the protocol). The Host header 
is user controlled and occurs after sni and tls validation -- I'd need 
more time to play with exploiting this but I think you could use this to 
send a different-than-checked host header to the inside server as if it 
were the client SNI value. Translating an non-validated user header into 
the tls protected sni field?


So what I need here is the ability to pass the clients' SNI value along 
to the inside server I think this should be ssl_fc_sni ... but if i 
try the above syntax it doesn't work right.


The following criteria should all be true:

1. The sni indication should match the clients stated host header. 
(verifyhost = ssl_fc_sni)


2. The sni value should be passed along to the backend server. (sni = 
ssl_fc_sni)


3. The client to the haproxy and backend could be SNI indicating any 
domain name, but should be TLS terminated to a matching cert in 
/etc/haproxy/certs, or hit the default cert  and then both 
verify=required and verifyhost=ssl_fc_sni should pass against the 
backend server or an error should result.



I might just be missing something with the config here, but I don't 
think the patch allows for passing along the actual ssl_fc_scni?



--

Kevin




On 2017-07-06 7:20 AM, Kevin McArthur wrote:

I'll see if I can give this a test. Thanks for adding it to master!

--

Kevin


On 2017-07-06 6:19 AM, Willy Tarreau wrote:

Hi again,

I finally merged it in master as commit 2ab8867, it will ease testing
(and a test file was provided).

Cheers,
Willy






Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-06 Thread Kevin McArthur

I'll see if I can give this a test. Thanks for adding it to master!

--

Kevin


On 2017-07-06 6:19 AM, Willy Tarreau wrote:

Hi again,

I finally merged it in master as commit 2ab8867, it will ease testing
(and a test file was provided).

Cheers,
Willy





Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-06 Thread Willy Tarreau
Hi again,

I finally merged it in master as commit 2ab8867, it will ease testing
(and a test file was provided).

Cheers,
Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-06 Thread Willy Tarreau
Hi Manu,

On Thu, Jul 06, 2017 at 11:10:41AM +0200, Emmanuel Hocdet wrote:
> SSL_SESSION_get0_hostname take a ssl_session not a ssl structure.

Ah crap! I did create the ssl_sess variable but forgot to change it in
the struct. It's likely that the address is the same here because the
code worked! Thanks!

> in openssl-compat.h a section is used for others functions like 
> SSL_SESSION_get0_X introduce in openssl 1.1.0

Ah much better, thank you!

> With this diff, it build with boringssl and openssl 1.0.2k. It should also be 
> ok with libressl

Cool, I'll refine the patch with yours, many thanks!
Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-06 Thread Emmanuel Hocdet

Hi Willy

> Le 5 juil. 2017 à 18:38, Willy Tarreau  a écrit :
> 
> Hi guys,
> 
> back to this old discussion.
> 
> On Fri, May 12, 2017 at 04:10:20PM +0200, Willy Tarreau wrote:
>> On Tue, May 09, 2017 at 12:12:42AM +0200, Lukas Tribus wrote:
>>> Haproxy can verify the certificate of backend TLS servers since day 1.
>>> 
>>> The only thing missing is client SNI based backend certificate
>>> verification, which yes - since we can pass client SNI to the TLS server
>>> - we need to consider for the certificate verification process as well.
>> 
>> In fact the cert name is checked, it's just that it can only check against
>> a constant in the configuration. I agree that it's a problem when using SNI.
>> Furthermore it forces one to completely disable verifyhost in case SNI is
>> used.
>> 
>> I tend to think that the best approach would be to always enable it when
>> SNI is involved in fact, because if SNI is used to the server, it really
>> means we want to check what cert is provided. This could then possibly be
>> explicitly turned off by the "verify none" directive.
>> 
>> I have absolutely no idea how to do that however, I don't know if we can
>> retrieve the previously configured SNI using openssl's API after the
>> connection is established.
> 
> So after digging I found a way to implement this. The configured SNI value
> is indeed stored in the SSL session and can be extracted, more or less
> easily depending on the version. I came up with the attached patch that
> works for me on all cases below. I'm willing to merge it into 1.8-dev and
> later backport to older versions once we're sure it does the right thing
> and doesn't break compatibility with openssl forks.
> 
> Please give it a try, and if some can test it on libressl/boringssl, it
> would be nice.
> 
> ---
> tested with the following server configurations (cert on port 8443 is valid
> for "localhost" only) :
> 
># passes checks and traffic (no hostname check)
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem
> 
># passes checks and traffic (localhost is what the server presents)
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem verifyhost localhost
> 
># fails checks and traffic (foo not matched on the server)
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem verifyhost foo
> 
># passes checks and traffic (verify none ignores the host)
>server ssl 127.0.0.1:8443 ssl verify none check inter 100 ca-file 
> rsa2048.pem verifyhost foo
> 
># passes checks and traffic (localhost is fine)
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem sni str(localhost) verifyhost localhost
> 
># passes checks and traffic (verifyhost overrides sni)
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem sni str(foo) verifyhost localhost
> 
># passes checks and traffic (localhost always valid)
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem sni str(localhost)
> 
># passes checks, and traffic without host or with "host: localhost" 
> and fails other hosts.
>server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
> rsa2048.pem sni req.hdr(host)
> ---
> 
> Cheers,
> Willy
> <0001-MINOR-ssl-compare-server-certificate-names-to-the-SN.patch>

SSL_SESSION_get0_hostname take a ssl_session not a ssl structure.
in openssl-compat.h a section is used for others functions like 
SSL_SESSION_get0_X introduce in openssl 1.1.0

With this diff, it build with boringssl and openssl 1.0.2k. It should also be 
ok with libressl

diff --git a/include/proto/openssl-compat.h b/include/proto/openssl-compat.h
index 7261a7b..88ea01e 100644
--- a/include/proto/openssl-compat.h
+++ b/include/proto/openssl-compat.h
@@ -94,6 +94,11 @@ static inline int SSL_SESSION_set1_id_context(SSL_SESSION 
*s, const unsigned cha
  * Functions introduced in OpenSSL 1.1.0 and not yet present in LibreSSL
  */
 
+static inline const char *SSL_SESSION_get0_hostname(const SSL_SESSION *sess)
+{
+   return sess->tlsext_hostname;
+}
+
 static inline const unsigned char *SSL_SESSION_get0_id_context(const 
SSL_SESSION *sess, unsigned int *sid_ctx_length)
 {
*sid_ctx_length = sess->sid_ctx_length;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 6448a89..d90d64b 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -127,13 +127,6 @@
 #define HASH_FUNCT EVP_sha256
 #endif /* OPENSSL_NO_SHA256 */
 
-/* Needed to retrieve the configured SNI for an outgoing connection, but only
- * appeared in openssl 1.1.0. I don't know how it's done with forked versions.
- */
-#if OPENSSL_VERSION_NUMBER < 0x101fL
-#define SSL_SESSION_get0_hostname(s) s->tlsext_hostname
-#endif
-
 /* ssl_methods flags for ssl options */
 #define MC_SSL_O_ALL  

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-07-05 Thread Willy Tarreau
Hi guys,

back to this old discussion.

On Fri, May 12, 2017 at 04:10:20PM +0200, Willy Tarreau wrote:
> On Tue, May 09, 2017 at 12:12:42AM +0200, Lukas Tribus wrote:
> > Haproxy can verify the certificate of backend TLS servers since day 1.
> > 
> > The only thing missing is client SNI based backend certificate
> > verification, which yes - since we can pass client SNI to the TLS server
> > - we need to consider for the certificate verification process as well.
> 
> In fact the cert name is checked, it's just that it can only check against
> a constant in the configuration. I agree that it's a problem when using SNI.
> Furthermore it forces one to completely disable verifyhost in case SNI is
> used.
> 
> I tend to think that the best approach would be to always enable it when
> SNI is involved in fact, because if SNI is used to the server, it really
> means we want to check what cert is provided. This could then possibly be
> explicitly turned off by the "verify none" directive.
> 
> I have absolutely no idea how to do that however, I don't know if we can
> retrieve the previously configured SNI using openssl's API after the
> connection is established.

So after digging I found a way to implement this. The configured SNI value
is indeed stored in the SSL session and can be extracted, more or less
easily depending on the version. I came up with the attached patch that
works for me on all cases below. I'm willing to merge it into 1.8-dev and
later backport to older versions once we're sure it does the right thing
and doesn't break compatibility with openssl forks.

Please give it a try, and if some can test it on libressl/boringssl, it
would be nice.

---
tested with the following server configurations (cert on port 8443 is valid
for "localhost" only) :

# passes checks and traffic (no hostname check)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem

# passes checks and traffic (localhost is what the server presents)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem verifyhost localhost

# fails checks and traffic (foo not matched on the server)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem verifyhost foo

# passes checks and traffic (verify none ignores the host)
server ssl 127.0.0.1:8443 ssl verify none check inter 100 ca-file 
rsa2048.pem verifyhost foo

# passes checks and traffic (localhost is fine)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni str(localhost) verifyhost localhost

# passes checks and traffic (verifyhost overrides sni)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni str(foo) verifyhost localhost

# passes checks and traffic (localhost always valid)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni str(localhost)

# passes checks, and traffic without host or with "host: localhost" and 
fails other hosts.
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni req.hdr(host)
---

Cheers,
Willy
>From 14eea17b6f43f391a2ac7436ce5aafc740e121aa Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Wed, 5 Jul 2017 18:23:03 +0200
Subject: MINOR: ssl: compare server certificate names to the SNI on outgoing
 connections

When support for passing SNI to the server was added in 1.6-dev3, there
was no way to validate that the certificate presented by the server would
really match the name requested in the SNI, which is quite a problem as
it allows other (valid) certificates to be presented instead (when hitting
the wrong server or due to a man in the middle).

This patch adds the missing check against the value passed in the SNI.
The "verifyhost" value keeps precedence if set. If no SNI is used and
no verifyhost directive is specified, then the certificate name is not
checked (this is unchanged).

In order to extract the SNI value, it was necessary to make use of
SSL_SESSION_get0_hostname(), which appeared in openssl 1.1.0. This is
a trivial function which returns the value of s->tlsext_hostname, so
it was open coded on older versions. I don't know what it does for
libressl / boringssl nor any possible other fork.

After some careful observation period it may make sense to backport
this to 1.7 and 1.6 as some users rightfully consider this limitation
as a bug.
---
 doc/configuration.txt | 23 +++
 src/ssl_sock.c| 22 +-
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 7cac3ca..0f425f4 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -11474,7 +11474,9 @@ sni 
   string and uses the result as the host name sent in the SNI TLS extension to
   the server. A typical 

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-12 Thread Willy Tarreau
On Tue, May 09, 2017 at 12:12:42AM +0200, Lukas Tribus wrote:
> Haproxy can verify the certificate of backend TLS servers since day 1.
> 
> The only thing missing is client SNI based backend certificate
> verification, which yes - since we can pass client SNI to the TLS server
> - we need to consider for the certificate verification process as well.

In fact the cert name is checked, it's just that it can only check against
a constant in the configuration. I agree that it's a problem when using SNI.
Furthermore it forces one to completely disable verifyhost in case SNI is
used.

I tend to think that the best approach would be to always enable it when
SNI is involved in fact, because if SNI is used to the server, it really
means we want to check what cert is provided. This could then possibly be
explicitly turned off by the "verify none" directive.

I have absolutely no idea how to do that however, I don't know if we can
retrieve the previously configured SNI using openssl's API after the
connection is established.

Willy



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-11 Thread Kevin McArthur
So who do I bug to actually get this coded/patched? Not being familiar 
with the code base myself ;)


--

Kevin McArthur


On 2017-05-08 3:12 PM, Lukas Tribus wrote:

Hello,


Am 08.05.2017 um 10:56 schrieb Daniel Schneller:

Just my 2c, I very much support Kevin’s argument.
Even though we are not (yet) verifying backends — because currently we
_are_ in a private LAN — we are planning to deploy parts of our
application to public cloud infrastructure soon, so it would be a
quite important feature.


On 6. May. 2017, at 19:18, Kevin McArthur > wrote:

1. The Snowden leaks and the whole "SSL added and removed here"
issue, for example. TLS on internal networks is more important these
days due to local network implants and other security issues on LANs.

2. Our use case is actually DigitalOcean where there is "private
networking" but it is shared among many customers. Operating without
TLS on this semi-private network would be unwise.

3. Most of the public tutorials for re-encrypt bridged TLS are simply
incurring TLS overhead while providing no TLS security. (eg SSL on
but, verify none enabled, verifyhost not set, etc)

4. Use cases like CDN proxy of public servers. Think Cloudflare's
Full SSL (Strict) setup...




Haproxy can verify the certificate of backend TLS servers since day 1.

The only thing missing is client SNI based backend certificate
verification, which yes - since we can pass client SNI to the TLS server
- we need to consider for the certificate verification process as well.


Regards,
Lukas






Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-08 Thread Lukas Tribus
Hello,


Am 08.05.2017 um 10:56 schrieb Daniel Schneller:
> Just my 2c, I very much support Kevin’s argument.
> Even though we are not (yet) verifying backends — because currently we
> _are_ in a private LAN — we are planning to deploy parts of our
> application to public cloud infrastructure soon, so it would be a
> quite important feature.
>
>> On 6. May. 2017, at 19:18, Kevin McArthur > > wrote:
>>
>> 1. The Snowden leaks and the whole "SSL added and removed here"
>> issue, for example. TLS on internal networks is more important these
>> days due to local network implants and other security issues on LANs.
>>
>> 2. Our use case is actually DigitalOcean where there is "private
>> networking" but it is shared among many customers. Operating without
>> TLS on this semi-private network would be unwise.
>>
>> 3. Most of the public tutorials for re-encrypt bridged TLS are simply
>> incurring TLS overhead while providing no TLS security. (eg SSL on
>> but, verify none enabled, verifyhost not set, etc)
>>
>> 4. Use cases like CDN proxy of public servers. Think Cloudflare's
>> Full SSL (Strict) setup...
>>
>>


Haproxy can verify the certificate of backend TLS servers since day 1.

The only thing missing is client SNI based backend certificate
verification, which yes - since we can pass client SNI to the TLS server
- we need to consider for the certificate verification process as well.


Regards,
Lukas




Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-08 Thread Daniel Schneller
Just my 2c, I very much support Kevin’s argument.
Even though we are not (yet) verifying backends — because currently we _are_ in 
a private LAN — we are planning to deploy parts of our application to public 
cloud infrastructure soon, so it would be a quite important feature.

Regards,
Daniel


-- 
Daniel Schneller
Principal Cloud Engineer
 
CenterDevice GmbH  | Hochstraße 11
   | 42697 Solingen
tel: +49 1754155711| Deutschland
daniel.schnel...@centerdevice.de   | www.centerdevice.de

Geschäftsführung: Dr. Patrick Peschlow, Dr. Lukas Pustina,
Michael Rosbach, Handelsregister-Nr.: HRB 18655,
HR-Gericht: Bonn, USt-IdNr.: DE-815299431


> On 6. May. 2017, at 19:18, Kevin McArthur  wrote:
> 
> 1. The Snowden leaks and the whole "SSL added and removed here" issue, for 
> example. TLS on internal networks is more important these days due to local 
> network implants and other security issues on LANs.
> 
> 2. Our use case is actually DigitalOcean where there is "private networking" 
> but it is shared among many customers. Operating without TLS on this 
> semi-private network would be unwise.
> 3. Most of the public tutorials for re-encrypt bridged TLS are simply 
> incurring TLS overhead while providing no TLS security. (eg SSL on but, 
> verify none enabled, verifyhost not set, etc)
> 
> 4. Use cases like CDN proxy of public servers. Think Cloudflare's Full SSL 
> (Strict) setup... 
> --
> 
> Kevin
> On 2017-05-05 7:20 PM, Igor Cicimov wrote:
>> 
>> 
>> On 6 May 2017 2:04 am, "Kevin McArthur" > > wrote:
>> When doing tls->haproxy->tls (bridged https) re-encryption with SNI, we need 
>> to verify the backend certificate against the SNI value requested by the 
>> client.
>> 
>> Something like server options:
>> 
>> server app1 app1.example.ca:443  ssl no-sslv3 
>> sni ssl_fc_sni verify required verifyhost ssl_fc_sni
>> 
>> However, the "verifyhost ssl_fc_sni" part doesn't work at current. Is there 
>> any chance I could get this support patched in?
>> 
>> Most folks seem to be either ignoring the backend server validation, setting 
>> verify none, or are stripping tls altogether leaving a pretty big security 
>> hole.
>> Care to elaborate why is this a security hole if the backend servers are in 
>> internal LAN which usually is the case when terminating ssl on the proxy?
>> 
>> --
>> 
>> Kevin McArthur
>> 
> 



Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-06 Thread Kevin McArthur
1. The Snowden leaks and the whole "SSL added and removed here" issue, 
for example. TLS on internal networks is more important these days due 
to local network implants and other security issues on LANs.


2. Our use case is actually DigitalOcean where there is "private 
networking" but it is shared among many customers. Operating without TLS 
on this semi-private network would be unwise.


3. Most of the public tutorials for re-encrypt bridged TLS are simply 
incurring TLS overhead while providing no TLS security. (eg SSL on but, 
verify none enabled, verifyhost not set, etc)


4. Use cases like CDN proxy of public servers. Think Cloudflare's Full 
SSL (Strict) setup...


--

Kevin
On 2017-05-05 7:20 PM, Igor Cicimov wrote:



On 6 May 2017 2:04 am, "Kevin McArthur" > wrote:


When doing tls->haproxy->tls (bridged https) re-encryption with
SNI, we need to verify the backend certificate against the SNI
value requested by the client.

Something like server options:

server app1 app1.example.ca:443  ssl
no-sslv3 sni ssl_fc_sni verify required verifyhost ssl_fc_sni

However, the "verifyhost ssl_fc_sni" part doesn't work at current.
Is there any chance I could get this support patched in?

Most folks seem to be either ignoring the backend server
validation, setting verify none, or are stripping tls altogether
leaving a pretty big security hole.

Care to elaborate why is this a security hole if the backend servers 
are in internal LAN which usually is the case when terminating ssl on 
the proxy?



--

Kevin McArthur






Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-05 Thread Igor Cicimov
On 6 May 2017 2:04 am, "Kevin McArthur"  wrote:

When doing tls->haproxy->tls (bridged https) re-encryption with SNI, we
need to verify the backend certificate against the SNI value requested by
the client.

Something like server options:

server app1 app1.example.ca:443 ssl no-sslv3 sni ssl_fc_sni verify required
verifyhost ssl_fc_sni

However, the "verifyhost ssl_fc_sni" part doesn't work at current. Is there
any chance I could get this support patched in?

Most folks seem to be either ignoring the backend server validation,
setting verify none, or are stripping tls altogether leaving a pretty big
security hole.

Care to elaborate why is this a security hole if the backend servers are in
internal LAN which usually is the case when terminating ssl on the proxy?


--

Kevin McArthur


Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

2017-05-05 Thread Kevin McArthur
When doing tls->haproxy->tls (bridged https) re-encryption with SNI, we 
need to verify the backend certificate against the SNI value requested 
by the client.


Something like server options:

server app1 app1.example.ca:443 ssl no-sslv3 sni ssl_fc_sni verify 
required verifyhost ssl_fc_sni


However, the "verifyhost ssl_fc_sni" part doesn't work at current. Is 
there any chance I could get this support patched in?


Most folks seem to be either ignoring the backend server validation, 
setting verify none, or are stripping tls altogether leaving a pretty 
big security hole.


--

Kevin McArthur