Re: dsr and haproxy
On Fri, Nov 04, 2022 at 05:33:40PM +0100, Lukas Tribus wrote: > On Fri, 4 Nov 2022 at 16:50, Szabo, Istvan (Agoda) > wrote: > > > > Yeah, that's why I'm curious anybody ever made it work somehow? > > Perhaps I should have been clearer. > > It's not supported because it's not possible. > > Haproxy the OSS uses the socket API, haproxy cannot forward IP packets > arbitrarily, which is required for DRS. And in fact it goes beyond the API. The first and foremost reason is that if you want to intercept TCP and work on contents, you have to accept an incoming connection first. For this you need do respond to a SYN with a SYN-ACK that holds a locally-chosen sequence number. Then assuming the connection is validated and you're going to pass it to a server, while you could imagine replicating the SYN sequence number from the client, the server will chose its own sequence number for the SYN-ACK, which will not match the one you chosed previously, and as such if you send the server's response directly to the client, this last one will never understand this traffic because it's shifted by the difference between the two sequence numbers. Some large hosting platforms had worked around this in the late 90s and early 2000s by prepending a header to TCP segments sent to the server, containing all the front connection's parameters (a bit like the proxy protocol), and the servers' TCP stack was heavily modified to use the parameters presented in this header to create the connection, including ports, sequence numbers, options etc that the server had to use. For obvious reasons such servers ought never be exposed to the net or it would have been trivial to DoS them or even to hijack their connections! I remember that others had proposed TCP extensions to tell a peer to skip a range of sequence numbers to make this possible (i.e. "I'm sending you a 1.3 GB hole then the data comes") as a way to splice a server connection to an already accepted one. But similarly this totally disappeared because it was hackish and totally insecure. > This is a hard no, not a "we do not support this configuration because > nobody ever tried it and we can't guarantee it will work". Definitely ;-) Cheers, Willy
Re: [PATCH]: BUILD: insecure-setuid-wanted support on FreeBSD
Hi David,
On Fri, Nov 04, 2022 at 07:34:46PM +, David CARLIER wrote:
> Hi,
>
> here a little patch to port the insecure-setuid-wanted directive on FreeBSD.
Thanks. I'm having a few comments below.
> From be693024d7e49173f7ff37566232238fc5ea1887 Mon Sep 17 00:00:00 2001
> From: David CARLIER
> Date: Fri, 4 Nov 2022 19:24:03 +
> Subject: [PATCH] BUILD: insecure-setuid-wanted support on FreeBSD.
>
> using the procctl api to ignore the suid/sgid bits to be ignored.
> ---
> src/haproxy.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 806497062..94b9bde4e 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -3003,7 +3003,7 @@ static void *run_thread_poll_loop(void *data)
> pthread_mutex_unlock(_mutex);
> #endif
>
> -#if defined(PR_SET_NO_NEW_PRIVS) && defined(USE_PRCTL)
> +#if (defined(PR_SET_NO_NEW_PRIVS) && defined(USE_PRCTL)) ||
> (defined(PROC_NO_NEW_PRIVS_CTL) && defined(USE_PROCCTL))
At this point this should move to compat.h, to do something like this
for example:
#if (defined(PR_SET_NO_NEW_PRIVS) && defined(USE_PRCTL)) || \ /* linux */
(defined(PROC_NO_NEW_PRIVS_CTL) && defined(USE_PROCCTL)) /* freebsd
*/
#define HAVE_NO_NEW_PRIVS
#endif
Then here you'd just use:
#ifdef HAVE_NO_NEW_PRIVS
> /* Let's refrain from using setuid executables. This way the impact of
>* an eventual vulnerability in a library remains limited. It may
>* impact external checks but who cares about them anyway ? In the
> @@ -3014,7 +3014,14 @@ static void *run_thread_poll_loop(void *data)
>*/
> if (!(global.tune.options & GTUNE_INSECURE_SETUID) && !master) {
> static int warn_fail;
> - if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1 &&
> !_HA_ATOMIC_FETCH_ADD(_fail, 1)) {
> +#if defined(USE_PRCTL)
> + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1
> +#else
> + int refrain_setuid = PROC_NO_NEW_PRIVS_ENABLE;
> + /* we can save one syscall once freebsd 14 becomes the minimum
> version, removing getpid */
> + if (procctl(P_PID, getpid(), PROC_NO_NEW_PRIVS_CTL,
> _setuid) == -1
> +#endif
> + && !_HA_ATOMIC_FETCH_ADD(_fail, 1)) {
And the part above is not nice, it breaks an "if" expression making it
much harder to follow, which is particularly problematic in code aimed
at improving security. I wouldn't be surprised if it would even break
auto-indent or parens matching on some editors.
Better put that in a separate function such as ha_no_new_privs() which
calls the suitable syscall and arguments depending on which approach is
desired. It could be left in this file I guess, even though for the long
term I think we'll finally create an "src/sys.c" file to support system
specific abstraction but we'd rather do that later.
Thanks,
Willy
Re: [PATCH] CI: switch to LibreSSL-3.6.1, enable QUIC
On Sat, Nov 05, 2022 at 10:12:00AM +0500, ??? wrote: > gentle ping Sorry Ilya, quite busy at the moment, now applied. Thanks! Willy
Re: [PATCH] fix spelling "choosen" --> "chosen"
On Wed, Nov 02, 2022 at 10:43:49AM +0100, William Lallemand wrote: > > > - if (!tp->choosen) > > > + if (!tp->chosen) > > > return; > > > > > > - chunk_appendf(b, "\n\tversion_information:(choosen=0x%08x", > > > tp->choosen); > > > + chunk_appendf(b, "\n\tversion_information:(chosen=0x%08x", tp->coosen); > > > I don't think it will even compile this way. I confirm it doesn't. I was about to fix it myself but I'd firt like to get a confirmation that it's OK to change this. Willy
