netdev - Re: [RFC PATCH 28/28] tcp: AccECN sysctl documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 Mar 2020 12:03:29 -0700
From:   Yuchung Cheng <ycheng@...gle.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...helsinki.fi>
Cc:     Dave Taht <dave.taht@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Neal Cardwell <ncardwell@...gle.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Olivier Tilmans <olivier.tilmans@...ia-bell-labs.com>
Subject: Re: [RFC PATCH 28/28] tcp: AccECN sysctl documentation

On Mon, Mar 23, 2020 at 6:34 AM Ilpo Järvinen
<ilpo.jarvinen@...helsinki.fi> wrote:
>
> On Fri, 20 Mar 2020, Yuchung Cheng wrote:
>
> > On Fri, Mar 20, 2020 at 3:40 PM Ilpo Järvinen
> > <ilpo.jarvinen@...helsinki.fi> wrote:
> > >
> > > On Thu, 19 Mar 2020, Dave Taht wrote:
> > >
> > > > On Wed, Mar 18, 2020 at 2:44 AM Ilpo Järvinen <ilpo.jarvinen@...sinki.fi> wrote:
> > > > >
> > > > > From: Ilpo Järvinen <ilpo.jarvinen@...helsinki.fi>
> > > > >
> > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...helsinki.fi>
> > > > > ---
> > > > >  Documentation/networking/ip-sysctl.txt | 12 +++++++++---
> > > > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> > > > > index 5f53faff4e25..ecca6e1d6bea 100644
> > > > > --- a/Documentation/networking/ip-sysctl.txt
> > > > > +++ b/Documentation/networking/ip-sysctl.txt
> > > > > @@ -301,15 +301,21 @@ tcp_ecn - INTEGER
> > > > >                 0 Disable ECN.  Neither initiate nor accept ECN.
> > > > >                 1 Enable ECN when requested by incoming connections and
> > > > >                   also request ECN on outgoing connection attempts.
> > > > > -               2 Enable ECN when requested by incoming connections
> > > > > +               2 Enable ECN or AccECN when requested by incoming connections
> > > > >                   but do not request ECN on outgoing connections.
> > > >
> > > > Changing existing user-behavior for this default seems to be overly
> > > > optimistic. Useful for testing, but...
> > >
> > > I disagree.
> > >
> > > The kernel default on ECN is/has been "do nothing" like forever. Yet,
> > > passively allowing ECN on servers is a low risk operation because nothing
> > > will change before client actively asks for it. However, it was obvious
> > > that the servers didn't do that. The servers could have set tcp_ecn to 1
> > > (before 2 was there) which is low risk for _servers_ (unlike for clients)
> > > but only very very few did. I don't believe servers would now
> > > intentionally pick 2 when they clearly didn't pick 1 earlier either.
> > >
> > > Adding 2 is/was an attempt to side-step the need for both ends to make
> > > conscious decision by setting the sysctl (which servers didn't want to
> > > do). That is, 2 gives decision on what to do into the hands of the client
> > > side which was the true intent of 2 (in case you don't know, I made that
> > > change).
> > What can a server configure to process only RFC3168 ECN if it prefers to?
>
> That's why I suggested the flag-based approach?

That's assuming an admin that has control of sysctls can also change
individual applications (easily). In reality it often is not the case.
The default sysctl choices in this patch seem risky to me.



>
> > > If "full control" is the way to go, I think it should be made using flags
> > > instead, along these lines:
> > >
> > > 1: Enable RFC 3168 ECN in+out
> > > 2: Enable RFC 3168 ECN in (default on)
> > > 4: Enable Accurate ECN in (default on)
> > > 8: Enable Accurate ECN in+out
> > >
> > > Note that I intentionally reversed the in and in/out order for 4&8
> > > (something that couldn't be done with 1&2 to preserve meaning of 1).
>
> It should address any except "out" but no "in" (the meaning of 1 cannot
> be changed I think). But out w/o in doesn't sound very useful.
>
> --
>  i.