how to handle error in haproxy action

2021-06-25 Thread reshma r
Hello all,
I have an haproxy action called get_some_data in which I return nil
whenever something goes wrong. In this case I don't want the
send-spoe-group action to be executed (see below config), since the data to
be sent is decided by the action and if there was any failure there, then
there is no point in trying to send data.

I guess I can set a variable in the lua code before returning nil, set an
acl based on this variable and then add an unless condition in the action
rule. This seems a bit cumbersome as I will have to set this variable in
multiple places so I just wanted to check if someone can suggest a
better solution.  Thanks for your help.

frontendhttp-in
bind *:80
modehttp

option httplog
log /dev/log local0 debug
filter spoe engine my-spoe config /PATH/TO/SPOE/shieldsquare-spoe.conf
http-request lua.get_some_data #1
http-request send-spoe-group my-spoe data-packet-grp #don't want to do
this if something goes wrong in 1


Re: SNI spoofing in HAproxy?

2021-06-25 Thread Tim Düsterhus

Dominik,

On 6/25/21 10:42 AM, Froehlich, Dominik wrote:

Your code sends a 421 if the SNI and host header don't match.
Is this the recommended behavior? The RFC is pretty thin here:

"   Since it is possible for a client to present a different server_name
in the application protocol, application server implementations that
rely upon these names being the same MUST check to make sure the
client did not present a different name in the application protocol."
(from https://datatracker.ietf.org/doc/html/rfc6066#section-11.1 )


Indeed it is. See the HTTP/2 RFC: 
https://datatracker.ietf.org/doc/html/rfc7540#section-9.1.1.


Additionally I would recommend using single-domain certificates (if 
possible), then a regular browser will never see a 421, because it will 
not perform connection reuse.



My initial idea was to simply pave over any incoming headers with the SNI:


http-request set-header Host %[ssl_fc_sni] if { ssl_fc_has_sni }


This wouldn't abort the request with a 421 but I am not sure if I MUST abort it 
to be compliant.


This is equivalent to routing based off the SNI directly and it's as 
unsafe. If a browser reuses a TCP connection for an unrelated host you 
will send the request to the wrong backend.



Regarding domain fronting, I thought I might be able to have the cake and eat 
it, too if I still allowed it but only prevented it for mTLS
Requests. Maybe like this:


http-request set-header Host %[ssl_fc_sni] if { ssl_c_used }





Only performing checks when `ssl_c_used` is true should be safe. But I 
*strongly* recommend sending a 421 instead of attempting to fiddle 
around with the 'host' header. It's just too easy to accidentally 
perform incorrect routing. It would then look like this:



http-request   set-var(txn.host)hdr(host)
http-request   deny deny_status 400 unless { req.hdr_cnt(host) eq 1 }
# Verify that SNI and Host header match if a client certificate is sent
http-request   deny deny_status 421 if { ssl_c_used } ! { 
ssl_fc_sni,strcmp(txn.host) eq 0 }


Best regards
Tim Düsterhus



Re: SNI spoofing in HAproxy?

2021-06-25 Thread Froehlich, Dominik
Tim,

Thank you for your reply.

Your code sends a 421 if the SNI and host header don't match.
Is this the recommended behavior? The RFC is pretty thin here:

"   Since it is possible for a client to present a different server_name
   in the application protocol, application server implementations that
   rely upon these names being the same MUST check to make sure the
   client did not present a different name in the application protocol."
(from https://datatracker.ietf.org/doc/html/rfc6066#section-11.1 )

My initial idea was to simply pave over any incoming headers with the SNI:

>   http-request set-header Host %[ssl_fc_sni] if { ssl_fc_has_sni }

This wouldn't abort the request with a 421 but I am not sure if I MUST abort it 
to be compliant.

Regarding domain fronting, I thought I might be able to have the cake and eat 
it, too if I still allowed it but only prevented it for mTLS
Requests. Maybe like this:

>   http-request set-header Host %[ssl_fc_sni] if { ssl_c_used }


What are your thoughts?

Best regards,
Dom

On 24.06.21, 16:05, "Tim Düsterhus"  wrote:

Dominik,

On 6/24/21 3:29 PM, Froehlich, Dominik wrote:
> Not sure if you would call this a security issue, hence I am asking this 
on the mailing list prior to opening a github issue:

This is also known as "Domain Fronting" 
(https://en.wikipedia.org/wiki/Domain_fronting). It's not necessarily a 
security issue and in fact might be the desired behavior in certain 
circumstances.

> I’ve noticed that it is really easy to bypass the check on client 
certificates of a domain when the client can present a valid certificate for 
another domain.

Indeed. I also use client certificates for authentication and I'm aware 
of this issue. That's why I validate that the SNI and Host match up for 
my set up. In fact I added the 'strcmp' converter to HAProxy just for 
this purpose. The documentation for 'strcmp' also gives an explicit 
example on how to use it to prevent domain fronting:

> http-request set-var(txn.host) hdr(host)
> # Check whether the client is attempting domain fronting.
> acl ssl_sni_http_host_match ssl_fc_sni,strcmp(txn.host) eq 0

https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#7.3.1-strcmp

My full rules look like this:

>   # Verify that SNI and Host header match
>   http-request   set-var(txn.host)hdr(host)
>   http-request   deny deny_status 400 unless { req.hdr_cnt(host) eq 1 }
>   http-request   deny deny_status 421 unless { 
ssl_fc_sni,strcmp(txn.host) eq 0 }

-

> […]
> 
> My questions:
> 
>*   HAproxy does seem to treat SNI (L5) and HTTP Host Header (L7) as 
unrelated. Is this true?

Yes.

>*   Applications offloading TLS to HAproxy usually trust that mTLS 
requests coming in are validated correctly. They usually don’t revalidate the 
entire certificate again and only check for the subject’s identity. Is there a 
way to make SNI vs host header checking more strict?

Yes, see above.

>*   What’s the best practice to dispatch mTLS requests to backends? 
I’ve used a host header based approach here but it shows the above 
vulnerabilities.

You *must* use the 'Host' header for routing. Using the SNI value for 
routing is even more unsafe. But you also must validate that the SNI 
matches up or that the client presented a valid certificate.

Best regards
Tim Düsterhus



Re: [PATCH spoa-server] BUG/MINOR: build: install binary inside bin/ directory

2021-06-25 Thread Willy Tarreau
On Sun, Jun 13, 2021 at 01:13:52AM +0100, Bertrand Jacquin wrote:
> Prior to the change, spoa is installed under DESTDIR with name `bin`

Thanks Bertrand. I've merged it now, after realizing that I still had
access to this now external repo :-)

Willy



Re: [EXTERNAL] Re: built in ACL, REQ_CONTENT

2021-06-25 Thread Willy Tarreau
On Tue, Jun 08, 2021 at 06:32:50PM +0200, Lukas Tribus wrote:
> Please try to ask the actual question directly next time, so we can
> help you right away (https://xyproblem.info/).

I didn't know this extremely common problem had a name, thanks Lukas
for the link, I'll share it more often!

Willy



Re: MINOR: fixes haiku linkage

2021-06-25 Thread Willy Tarreau
On Sat, Jun 19, 2021 at 02:47:52PM +0100, David CARLIER wrote:
> Hi here a little change proposal to fix haproxy at runtime in this platform.
> Cheers.

Thanks David, now merged.
Willy



Re: Fix small bug in srv_parse_agent_check

2021-06-25 Thread Willy Tarreau
Hi Dirkjan,

On Fri, Jun 18, 2021 at 10:03:17PM +0200, Dirkjan Bussink wrote:
> Hi all,
> 
> I was building HAProxy using scan-build to see if there were any issues and
> it called out an unused variable write. I think this was due to a bug that
> the err_code was not used in srv_parse_agent_check.

I agree with your analysis, I've now merged your patch.

Thank you!
Willy



Re: SSL Labs says my server isn't doing ssl session resumption

2021-06-25 Thread Willy Tarreau
On Sun, Jun 20, 2021 at 05:20:41PM -0600, Shawn Heisey wrote:
> On 6/20/2021 3:16 PM, Lukas Tribus wrote:
> > It's a haproxy bug, affecting 2.4 releases, I've filed an issue in our 
> > tracker:
> > 
> > https://github.com/haproxy/haproxy/issues/1297
> 
> Almost always when I report a problem I'm having with a mature piece of
> software, I expect the issue to be PEBCAK, not an actual bug.  Either way, I
> really enjoy being a part of an open source community.

Hehe we have no business trying to hide bugs, as they'll affect all of
us. It's much better to humbly recognize them and fix them :-)

> It took me a while to figure out how to get the proper code branch with git
> for 2.4 development.

2.4 is not in development anymore, it's stable (and LTS -- long term
supported). Please don't try to do any development on 2.4, you'll only
waste your time, because if you want to propose some changes, you'll
first be requested to rebase your changes on top of latest development
and that could take you a while in certain areas.

The right way to participate is to pull the development branch, and
update it once in a while (e.g. every time you're going back to your
developments, such as once a week). This way if something breaks,
chances are that the person who did the breaking change still has fresh
memories about it and will be able to help, and instantly fix the
trouble or explain how to adapt your code. Do not worry too much for
the risk of instabilities, the published code is tested and even the
haproxy.org site runs on it. It's quite sufficient for development and
even for either a personal site, or one that is properly monitored and
operated by skilled admins.

> I don't think it's on github.  This is the first time
> I've seen completely separate repositories for different versions.  Not
> complaining ... I bet there's a very good reason for it.

It precisely is because we don't want to receive pull reqs nor issues
on the wrong branch, and github doesn't offer sufficient flexibility to
allow to disable PRs on certain branches nor repos. I can't blame them
too much for this, it's their core business, they have no reason to
offer free hosting and resources with no hopes to see forks and PRs
in return that indirectly bring them new users. But overall the current
model looks like a good balance offering a good efficiency to everyone.

> I wish I knew C a lot better than I do ...

That's what about half of the long time contributors started to say
before become regular ones, welcome! You'll get back to it easily, do
not worry. C is easy to learn (or it wouldn't have lasted 50+ years),
it just has a number of traps, but those not using it all day long
are extra careful and generally do not fall into them. It's like with
parachutes, accidents only affect those routinely using them and who
are less careful.

> though I expect that for TLS
> code, even very strong C skills would leave me clueless without spending
> several weeks or months doing a deep dive.

I agree. Cryptography code is cryptic, that's the part I understand
the least. In addition, OpenSSL is even older than haproxy and has 2.5
decades of evolving API, naming schemes, coding styles and incompatible
features behind, that inevitably leave some confusion at plenty of
places. HAProxy itself is not exempt from this (it's only in 2.4 that
we finally renamed all "sess*" functions that were acting on streams
and not sessions). But that's why there are comments in the code to
explain what the developer tries to do and why.

If you find some areas that are too cryptic and not commented enough,
feel free to ask here for precisions. You can even use "git blame" on
the file then "git show" on the commit IDs to see who worked last on
the lines you don't understand. Sometimes you'll find more explanations
in the commit message. If you don't, feel free to ask, that's how it
works!

Willy



Re: Line 47 in src/queue.c "s * queue's lock."

2021-06-25 Thread Willy Tarreau
On Thu, Jun 24, 2021 at 11:35:51PM +0200, Aleksandar Lazic wrote:
> Hi.
> 
> when someone works again on src/queue.c could be this typo fixed.
> 
> http://git.haproxy.org/?p=haproxy.git;a=blob;f=src/queue.c;h=6d3aa9a12bcd6078d1b5a76969da4104a6adb1bd;hb=HEAD#l47
> 
> ```
>   44  *   - a pendconn_add() is only performed by the stream which will own 
> the
>   45  * pendconn ; the pendconn is allocated at this moment and returned 
> ; it is
>   46  * added to either the server or the proxy's queue while holding this
>   47 s * queue's lock.
>   48  *
> ```

Indeed, thanks Aleks!

Willy