Re: Is it possible to capture the body of http responses?

2021-09-09 Thread Christopher Faulet

Le 8/11/21 à 2:53 AM, Ryan Burn a écrit :
I'm working on integrating HAProxy with traceable.ai 's 
security product.


As part of the integration, we'd like to capture the contents of any http 
responses processed by HAProxy and send them to a service either via SPOA or an 
RPC call from Lua. The response contents are used by the product to help 
identify possible security threats.


I've tried a few things, but haven't found a reliable way to capture the 
contents of response bodies. Is this possible with HAProxy?


Here are the approaches I've explored so far:

1. I used the "res.body" fetch but that only provides the contents sometimes (I 
presume if it's available in a buffer):
https://github.com/rnburn/haproxy-extcap/blob/master/test/docker/extcap.conf#L19 



2. I also tried accessing the contents of the response channel from a Lua 
action, but that fails with "Cannot manipulate HAProxy channels in HTTP mode"
https://github.com/rnburn/haproxy-extcap/blob/master/test/docker/response.lua#L5 



Hi Ryan,

About the SPOE, it is on the todo-list to add streaming feature to be able to 
send request/response payload to an agent and be able to rewrite it. 
Unfortunately, to make it clean and usable, it requires a total refactoring and 
for now, I haven't found the time to work on it.


About the sample fetches, on HAProxy 2.3 and lower, there is no way to get the 
response payload because it is not possible to wait for it. There is no 
equivalent to the "http-buffer-request" option on the response side. On 
HAProxy-2.4, it is possible by using "wait-for-body" HTTP rule, available on the 
request and the response side. However, it is still limited by the buffer size.


With the Lua, it is only possible by writing a filter using the experimental Lua 
filter API, available in HAProxy 2.5. This API is really experimental for now, 
but it may be a solution to analyze the whole message payload, regardless its 
size. However, It may be painful because the API may be incomplete and because 
dealing with multiple buffers is not simple, especially if you don't want to 
forward the payload before the end of analysis.


--
Christopher Faulet



Re: I just broke opentracing :-(

2021-09-09 Thread Miroslav Zagorac

Hello Willy,

On 09/09/2021 08:43 AM, Willy Tarreau wrote:

Yes, that's where I found it but didn't know what its use was. A few
points however:
   - sess.ot.uuid will be shared by all requests on the same session,
 which is probably not what you want (e.g. for requests coming in
 H2, the last one setting it will replace all other ones' value
 while they're working with it). I think you should change its
 scope to be limited to the transaction only (txn.ot.uuid). It's
 also used in debugging outputs where it will be per-request and
 may not match what the requests will log because of this.


Thanks, i'll change that.


   - I finally found its definition here in filter.h:
   #define FTL_OT_VAR_UUID   "sess", "ot", "uuid"

 Please note the typo in the macro name (FTL instead of FLT)


Strange that there is only one such typo. :)


   - the comment in struct flt_ot_runtime_context for uuid says
 "Randomly generated UUID". We already have a function to produce a
 random UUID (ha_generate_uuid()) but flt_ot_runtime_context_init()
 makes its own from elements that at first glance look like they're
 filled from other locations (uuid.node, uuid.clock_seq etc), but in
 fact are simply aliased on the random ints. In this case better use
 the existing function and avoid duplicating the painful output format
 generation. If you need to keep a copy of the two randoms, just
 modify the existing function to pass an optional array to store
 the randoms. At first glance they're not used though.


When ot filter source was created, I didn't know what would be used and
what wouldn't.  At the time I didn't even know what opentracing was,
what it was used for and how it was done.

So, adding 'useful' things to the source, there are also those members
of the structure that were not used anywhere later (because I thought
at the time that they might be used somewhere).

Not that they bother anything, but I will remove them.  I will also use
the haproxy function to generate uuid, it's a small change in ot source.

Then I didn't use that function because it doesn't set those two uuid
64-bit numbers and it was a bit of a weird way for me to generate uuid
because at the end of the haproxy function the value of v[2] rotates
to the right by 14 bits and the value of v[3] to the left by 18 bits
which for uint64_t[2] input gave me mixed bits.

For example, if both random numbers are equal to 0x0123456789abcdef,
the result is 89abcdef-4567-4123-8def-048d159e26af, while to me the
logical result would be 89abcdef-4567-4123-8def-0123456789ab.

This result is obtained if v[2] rotates to the right by 16 bits and
v[3] to the left by 16 bits.

If all the bits are equally good random, this is somehow a more logical
result for me, because they give the correct order of the bytes of the
input data (I'm not going into the correctness of the big/little
endian way of writing data now).


   - there's quite a bunch of functions with no description at all of
 their purpose, input values, return values etc. ..


Yes, writing comments always takes useful time, but they are necessary;
at least for weirder things..  I will fill in those function
descriptions over time and send patches.

--
Zaga

What can change the nature of a man?



Re: I just broke opentracing :-(

2021-09-09 Thread Willy Tarreau
Hi Miroslav,

On Wed, Sep 08, 2021 at 08:02:35PM +0200, Miroslav Zagorac wrote:
> On 09/08/2021 07:57 PM, Miroslav Zagorac wrote:
> > On 09/08/2021 07:42 PM, Willy Tarreau wrote:
> > > No rush on this one, I'll let you think about it, just let me know if we
> > > need to temporarily disable it from the CI, or if you just want a day or
> > > two of checking before knowing if you can get a quick solution or if it
> > > will take more time.
> > > 
> > 
> > Thank you for understanding. For a start it would be good to exclude
> > ot filter from the compilation process so as not to interfere with the
> > development and testing of other code.

OK I can do that.

> > Next, I could throw out the part related to context transfer via
> > variables with #ifdef/#endif blocks, so that the ot filter code is
> > compiled without it.
> > 
> > After that the problem with variables could be solved without
> > interfering with the rest of the coding/compilation process and the
> > ot filter will be functional (except for the part with context transfer
> > via variables, but the question is whether anyone uses it).

OK. Given that I really have no idea what it's used for (from the
user's perspective), I cannot judge. But maybe we can start with
this and ask for testers.

> > The variable 'sess.ot.uuid' is just set, it is not used anywhere (as
> > far as memory serves me). I imagined that this could be used if the
> > user needs it, but it is not necessary.
> > 
> 
> OK, it is still used but again only as an example that it can be used
> and not that it should be used.
> 
> In our examples uuid is set as one of the data from the opentracing
> 'baggage' data group:
> 
> test/ctx/ot.cfg:
> ---
>baggage "haproxy_id" var(sess.ot.uuid)
> ---

Yes, that's where I found it but didn't know what its use was. A few
points however:
  - sess.ot.uuid will be shared by all requests on the same session,
which is probably not what you want (e.g. for requests coming in
H2, the last one setting it will replace all other ones' value
while they're working with it). I think you should change its
scope to be limited to the transaction only (txn.ot.uuid). It's
also used in debugging outputs where it will be per-request and
may not match what the requests will log because of this.

  - I finally found its definition here in filter.h:
  #define FTL_OT_VAR_UUID   "sess", "ot", "uuid"

Please note the typo in the macro name (FTL instead of FLT)

  - the comment in struct flt_ot_runtime_context for uuid says
"Randomly generated UUID". We already have a function to produce a
random UUID (ha_generate_uuid()) but flt_ot_runtime_context_init()
makes its own from elements that at first glance look like they're
filled from other locations (uuid.node, uuid.clock_seq etc), but in
fact are simply aliased on the random ints. In this case better use
the existing function and avoid duplicating the painful output format
generation. If you need to keep a copy of the two randoms, just
modify the existing function to pass an optional array to store
the randoms. At first glance they're not used though.

  - there's quite a bunch of functions with no description at all of
their purpose, input values, return values etc. I know pretty well
how painful it is to document that, especially since every single
time we have to go through this we find bugs due to the caller and
callee disagreeing on input domain or output values (typically the
type of thing Coverity loves to look at). This is also why it's
important. Hint: at least fill the description entry and explain
limits/restrictions if any. The locations of empty descriptions
can be found with this:

  $ pcregrep -nMo1 '^ \* DESCRIPTION\n \*   -\n' addons/ot/src/*.c

I'm not asking you to add all of them immediately, but I'm willing
to apply cleanup patches that routinely add a few of them when you
have a bit of time to work on this. And of course bugfixes as well
if you find bugs while documenting.

Thanks,
Willy



Re: [ANNOUNCE] HTX vulnerability from 2.0 to 2.5-dev

2021-09-09 Thread bjun...@gmail.com
Hi,

is HAProxy 2.0.x with "no option http-use-htx" also affected by
this vulnerability?

Best regards / Mit freundlichen Grüßen
Bjoern

Am Di., 7. Sept. 2021 um 17:30 Uhr schrieb Willy Tarreau :

> Hi everyone,
>
> Right after the previous announce of HTTP/2 vulnerabilities, a group
> of security researchers from JFrog Security have been looking for the
> possibility of remaining issues around the same topic. While there was
> nothing directly exploitable, Ori Hollander found a bug in the HTTP
> header name length encoding in the HTX representation by which the most
> significant bit of the name's length can slip into the value's least
> significant bit, and figured he could craft a valid request that could
> inject a dummy content-length on input that would be produced on output
> in addition to the other one, resulting in the possibility of a blind
> request smuggling attack ("blind" because the response never gets back
> to the attacker). Quite honestly they've done an excellent job at
> spotting this one because it's not every day that you manage to turn
> a single-bit overflow into an extra request, and figuring this required
> to dig deeply into the layers! It's likely that they'll publish something
> shortly about their finding.
>
> CVE-2021-40346 was assigned to this issue, which affects versions 2.0
> and above. I'm going to emit new maintenance releases for 2.0, 2.2, 2.3
> and 2.4 (2.5 still being in development, it will be released a bit later).
>
> A possible workaround for those who cannot upgrade is to block requests
> and responses featuring more than one content-length header after the
> overflow occured; these ones are always invalid because they're always
> resolved during the parsing phase, hence this condition never reaches
> the HTTP layer:
>
>http-request  deny if { req.hdr_cnt(content-length) gt 1 }
>http-response deny if { res.hdr_cnt(content-length) gt 1 }
>
> I'd like to thank the usual distro maintainers for having accepted to
> produce yet another version of their packages in a short time. Hopefully
> now we can all get back to development!
>
> Thanks,
> Willy
>
>


Re: [ANNOUNCE] HTX vulnerability from 2.0 to 2.5-dev

2021-09-09 Thread Willy Tarreau
Hi Bjoern,

On Thu, Sep 09, 2021 at 08:18:24PM +0200, bjun...@gmail.com wrote:
> Hi,
> 
> is HAProxy 2.0.x with "no option http-use-htx" also affected by
> this vulnerability?

No it's not. I thought I mentioned it but it's possible that I forgot
it in the end.

Regards,
Willy