Patch for GitHub Issue 1530

[Vig Nesh]

Hello Team Haproxy,

Thanks for providing an opportunity to work with the product, I have submitted 
a patch for issue 1530  along 
with this email.

Thanks,
Vignesh SP


0001-BUG-MINOR-server-Make-SRV_STATE_LINE_MAXLEN-value-fr.patch
Description:  0001-BUG-MINOR-server-Make-SRV_STATE_LINE_MAXLEN-value-fr.patch

Re: 2.5: Possibility to upgrade http/1.0 clients to http/1.1?

[Willy Tarreau]

Hi Dominik,

On Mon, May 09, 2022 at 08:46:20AM +, Froehlich, Dominik wrote:
> Hi Willy,
> 
> Thanks for your response.
> 
> Yes, I agree an option that can be turned on would be the most feasible
> solution for us.
>
> I can think of a similar option like we have for "option
> h1-case-adjust-bogus-server"
> that allows you to tell HAProxy whether to touch header casing or not.

Yes I think it would make sense because overall when you know you have
to relax this, it's more based on the LB's location than on a specific
frontend or backend, so it could just be a global option.

> Could be called "option h1-reject-get-head-delete-with-payload-bogus-client" 
> or something.

Note, we want to leave it disabled by default so that only those with
this unusual requirement would have to set the option, so the option's
name should translate that it has to accept the requests instead, so it
could be something like "h1-no-method-restriction-for-payload" or
"h1-accept-payload-with-any-method", or something like this.

> As for your question how the clients ended up sending a body with such 
> requests:
> 
> The API the client is using allows them to send a DELETE request with a JSON
> document that lists all the resources to be deleted.

Ah I see, that kind of makes sense. I'm wondering if Elastic Search does
not do something similar, I seem to remember some discussions around this
when we were working on tightening the rule in the spec.

> So instead of
> 
> DELETE /books/1
> 
> They would send
> 
> DELETE /books
> {
>   "delete": [1,2,3]
> }
> 
> Weird, I know but it allowed them to delete more than one resource at a time.
> (of course, this could also be put on the path, but well... that's how they
> did it back then)

It's possible that it's easier to specify complex sets. It definitely
helps anticipate possible trouble to be expected at some places. Thanks
for the explanation!

Willy

Re: 2.5: Possibility to upgrade http/1.0 clients to http/1.1?

[Froehlich, Dominik]

Hi Willy,

Thanks for your response.

Yes, I agree an option that can be turned on would be the most feasible 
solution for us.
I can think of a similar option like we have for “option 
h1-case-adjust-bogus-server”
 that allows you to tell HAProxy whether to touch header casing or not.

Could be called “option h1-reject-get-head-delete-with-payload-bogus-client” or 
something.

As for your question how the clients ended up sending a body with such requests:

The API the client is using allows them to send a DELETE request with a JSON 
document that lists all the resources to be deleted.

So instead of

DELETE /books/1

They would send

DELETE /books
{
  “delete”: [1,2,3]
}

Weird, I know but it allowed them to delete more than one resource at a time. 
(of course, this could also be put on the path, but well… that’s how they did 
it back then)


Best Regards,
Dominik

From: Willy Tarreau 
Date: Sunday, 8. May 2022 at 11:36
To: Froehlich, Dominik 
Cc: haproxy@formilux.org 
Subject: Re: 2.5: Possibility to upgrade http/1.0 clients to http/1.1?
Hello Dominik,

On Thu, May 05, 2022 at 07:55:06AM +, Froehlich, Dominik wrote:
> Hello everyone,
>
> We recently bumped our HAproxy deployment to 2.5 and are now getting hit by 
> this fix:
>
> MEDIUM: mux-h1: Reject HTTP/1.0 GET/HEAD/DELETE requests with a payload
>
>
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.haproxy.org%2F%3Fp%3Dhaproxy-2.5.git%3Ba%3Dblob_plain%3Bf%3DCHANGELOG&data=05%7C01%7Cdominik.froehlich%40sap.com%7C467ec29be90d4e1fee0c08da30d64b24%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C637875994185150797%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Yvqb67xhddXiqNo9oikA9GjX6QgMPUzURq%2B1BiTBEFg%3D&reserved=0
>
> The issue is we have many legacy customers using very old systems and we
> can't tell all of them to rewrite their clients to http/1.1.

Of course... Do you have an idea how they ended up sending a body with
such requests ? Maybe adding a comment on the issue below for posterity
could be useful for future versions of the HTTP spec:

  
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fhttpwg%2Fhttp-core%2Fissues%2F904&data=05%7C01%7Cdominik.froehlich%40sap.com%7C467ec29be90d4e1fee0c08da30d64b24%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C637875994185150797%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=DZWvpaaPgEvkznNrwBIohppy1B59D0U78mS1JrX3EA0%3D&reserved=0

> I get the security fix to prevent request smuggling where some servers ignore
> the body and treat it as another request, I'm not arguing that.
>
> However, I was wondering if it was possible to intercept HTTP/1.0 client
> requests and upgrade them to HTTP/1.1 without hitting the rejection code of
> the commit here:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fhaproxy%2Fhaproxy%2Fcommit%2Fe136bd12a32970bc90d862d5fe09ea1952b62974&data=05%7C01%7Cdominik.froehlich%40sap.com%7C467ec29be90d4e1fee0c08da30d64b24%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C637875994185150797%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PuLtA4U1I%2FtKU4GsiDZ1Fse8WUFU20mHhW6kVSG%2FK2g%3D&reserved=0

"Upgrading" the version must really never ever be done, as a lot of HTTP
semantics changed between 1.0 and 1.1, and by telling a server that the
client is 1.1, the server will be allowed to use chunking, trailers,
100-continue and a lot of other stuff that 1.0 clients are unable to
understand.

For your use case, I guess the best solution would be to add an option
(possibly a global one) to relax that rule. It's self-contained inside
an "if" statement so it should be quite easy to add such an option and
be done with it, because when you need it, you definitely know that you
need it, you'll find the keyword in the doc and the accompanying warning
about the security impacts. Also the HTTP spec now says "unless support
is indicated via other means", so the intent clearly is to make this
configurable on a case-by-case basis.

> This way we would not have to downgrade to HAproxy 2.4 again - which would be
> very unfortunate as we need many of the nice features of 2.5.

We'll discuss this with Christopher tomorrow, but I'm not worried about
this for now.

Thanks!
Willy

Re: Fwd: Set environment variables

[Valerio Pachera]

Il giorno dom 8 mag 2022 alle ore 11:07 Willy Tarreau  ha scritto:

>
> With that said, I admit that it could be useful to have another variable
> such as HAPROXY_SERVER_SSL to indicate if SSL is being used, and possibly
> another one such as HAPROXY_SERVER_PROTO to indicate when a protocol is
> being forced (e.g. h2).
>
> If you think your only solution would be to have such variables, feel
> free to have a look at the code to see how to add them. That should be
> simple enough for a first contribution. That's something we can merge
> late in the cycle, and possibly even backport to 2.5.
>
>
> Thank you very much willy for your reply.
Unfortunately I'm not a developer so it will take too much time form me to
contribute to the code.
I think I'll use a naming convention.

Valerio.