Message-ID: <CAO7SqHBZMu1vX_a58ujOcNHaau19UtVbauB37Q0zRL-k3wqRXg@mail.gmail.com>
Date:   Mon, 22 Aug 2022 11:42:34 -0700
From:   Salam Noureddine <noureddine@...sta.com>
To:     Leonard Crestez <cdleonard@...il.com>
Cc:     Dmitry Safonov <dima@...sta.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        David Ahern <dsahern@...nel.org>,
        Andy Lutomirski <luto@...capital.net>,
        Ard Biesheuvel <ardb@...nel.org>,
        Bob Gilligan <gilligan@...sta.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Eric Biggers <ebiggers@...nel.org>,
        Francesco Ruggeri <fruggeri@...sta.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Ivan Delalande <colona@...sta.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Shuah Khan <shuah@...nel.org>,
        netdev@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/31] net/tcp: Add TCP-AO support

On Sun, Aug 21, 2022 at 1:34 PM Leonard Crestez <cdleonard@...il.com> wrote:
>
> On 8/18/22 19:59, Dmitry Safonov wrote:
> > This patchset implements the TCP-AO option as described in RFC5925. There
> > is a request from industry to move away from TCP-MD5SIG and it seems the time
> > is right to have a TCP-AO upstreamed. This TCP option is meant to replace
> > the TCP MD5 option and address its shortcomings.
...
> >
> > The patch set was written as a collaboration of three authors (in alphabetical
> > order): Dmitry Safonov, Francesco Ruggeri and Salam Noureddine. Additional
> > credits should be given to Prasad Koya, who was involved in early prototyping
> > a few years back. There is also a separate submission done by Leonard Crestez
> > whom we thank for his efforts getting an implementation of RFC5925 submitted
> > for review upstream [2]. This is an independent implementation that makes
> > different design decisions.
>
> Is this based on something that Arista has had running for a while now
> or is a recent new development?
>

This is based on prototype code we had worked on internally three years
ago. The implementation effort was restarted to get it over the finish
line. For business reasons we had to have our own implementation ready
and not tied to unmerged upstream code. Please also note that our
implementation is based on linux-4.19 and was only ported forward to
mainline recently. So it wasn’t ready to be submitted upstream.

> > For example, we chose a similar design to the TCP-MD5SIG implementation and
> > used setsockopt()s to program per-socket keys, avoiding the extra complexity
> > of managing a centralized key database in the kernel. A centralized database
> > in the kernel has dubious benefits since it doesn’t eliminate per-socket
> > setsockopts needed to specify which sockets need TCP-AO and what are the
> > currently preferred keys. It also complicates traffic key caching and
> > preventing deletion of in-use keys.
>
> My implementation started with per-socket lists but switched to a global
> list because this way is much easier to manage from userspace. In
> practice userspace apps will want to ensure that all sockets use the
> same set of keys anyway.
>

We did consider a global list early on but we didn’t find it
beneficial. We still believe that per-socket lists reduce complexity
of the implementation, are more scalable and ensure predictable
behavior. Our expectation is that TCP-AO will be only useful for a
limited set of routing applications, rather than used transparently
like IPSEC for non-routing apps. We would be happy to discuss this in
more detail.

> > In this implementation, a centralized database of keys can be thought of
> > as living in user space and user applications would have to program those
> > keys on matching sockets. On the server side, the user application programs
> > keys (MKTS in TCP-AO nomenclature) on the listening socket for all peers that
> > are expected to connect. Prefix matching on the peer address is supported.
...
>
> My series doesn't try to prevent inconsistencies inside the key lists
> because it's not clear that the kernel should prevent userspace from
> shooting itself in the foot. Worst case is connection failure on
> misconfiguration which seems fine.
>
> The RFC doesn't specify in detail how key management is to be performed,
> for example if two valid keys are available it doesn't mention which one
> should be used. Some guidance is found in RFC8177 but again not very much.
>
> I implemented an ABI that can be used by userspace for RFC8177-style key
> management and asked for feedback but received very little. If you had
> come with a clear ABI proposal I would have tried to implement it.
>
> Here's a link to our older discussion:
>
> https://lore.kernel.org/netdev/e7f0449a-2bad-99ad-4737-016a0e6b8b84@gmail.com/
>
> Seeing an entirely distinct unrelated implementation is very unexpected.
> What made you do this?
>
> --
> Regards,
> Leonard

Our goal was not to have a competing TCP-AO upstream submission but
to implement the RFC for our customers to use. Had there been an
already upstreamed implementation we would have used it and
implemented customer requirements on top of it. Just like we do with
all other kernel features. This is not a bad situation, we believe it
is good for the upstream community to have two fully functioning
implementations to consider. Possibly a third collaborative
submission might emerge that takes the best of both. A year ago, there
wasn’t much available online about TCP-AO besides the RFC. We are
excited with the current interest in TCP-AO and hope to see it
upstreamed soon.

Best,

Salam

Powered by blists - more mailing lists