Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9ae5c977-ff9c-591d-3a32-ca9dd00d531e@arista.com>
Date:   Mon, 19 Jun 2023 17:41:48 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v7 04/22] net/tcp: Prevent TCP-MD5 with TCP-AO being set
Content-Language: en-US
To:     David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>
Cc:     linux-kernel@vger.kernel.org,
        Andy Lutomirski <luto@amacapital.net>,
        Ard Biesheuvel <ardb@kernel.org>,
        Bob Gilligan <gilligan@arista.com>,
        Dan Carpenter <error27@gmail.com>,
        David Laight <David.Laight@aculab.com>,
        Dmitry Safonov <0x7f454c46@gmail.com>,
        Donald Cassidy <dcassidy@redhat.com>,
        Eric Biggers <ebiggers@kernel.org>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        Francesco Ruggeri <fruggeri05@gmail.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        Ivan Delalande <colona@arista.com>,
        Leonard Crestez <cdleonard@gmail.com>,
        Salam Noureddine <noureddine@arista.com>,
        netdev@vger.kernel.org
References: <20230614230947.3954084-1-dima@arista.com>
 <20230614230947.3954084-5-dima@arista.com>
 <85077827-d11d-d3e6-0d23-9e60974cad0f@kernel.org>
 <1c2537d0-cf64-c010-fec6-9fa9ad758f42@arista.com>
From:   Dmitry Safonov <dima@arista.com>
In-Reply-To: <1c2537d0-cf64-c010-fec6-9fa9ad758f42@arista.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 6/19/23 17:31, Dmitry Safonov wrote:
> Hi David,
> 
> On 6/18/23 18:50, David Ahern wrote:
>> On 6/14/23 4:09 PM, Dmitry Safonov wrote:
>>> Be as conservative as possible: if there is TCP-MD5 key for a given peer
>>> regardless of L3 interface - don't allow setting TCP-AO key for the same
>>> peer. According to RFC5925, TCP-AO is supposed to replace TCP-MD5 and
>>> there can't be any switch between both on any connected tuple.
>>> Later it can be relaxed, if there's a use, but in the beginning restrict
>>> any intersection.
>>>
>>> Note: it's still should be possible to set both TCP-MD5 and TCP-AO keys
>>> on a listening socket for *different* peers.
>>
>> Does the testsuite cover use of both MD5 and AO for a single listening
>> socket with different peers and then other tests covering attempts to
>> use both for a same peer?
> 
> Thanks for the question, I have written the following tests for
> AO/MD5/unsigned listening socket [1]:
> 
> 1. Listener with TCP-AO key, which has addr = INADDR_ANY
> 2. Listener with TCP-MD5 key, which has tcpm_addr = INADDR_ANY
> 3. Listener without any key
> 
> Then there's AO_REQUIRED thing, which BGP folks asked to introduce,
> which is (7.3) from RFC5925, an option that is per-ao_info, which makes
> such socket accepting only TCP-AO enabled segments.
> 
> So, 4. Listener with TCP-AO, AO_REQUIRED flag.
> 
> And then, going to non-INADDR_ANY:
> 5. Listener with TCP-AO and TCP-MD5 keys for different peers.
> 
> Here again, for each of AO/MD5/unsigned methods, attempt to connect:
> 6. outside of both key peers
> 7. inside correct key: i.e. TCP-MD5 client to TCP-MD5 matching key
> 8. to a wrong key: i.e. TCP-AO client to TCP-MD5 matching key
> 
> And another type of checks are the ones expecting *setsockopt()* to fail:
> 9. Adding TCP-AO key that matches the same peer as TCP-MD5 key
> 10. The reverse situation
> 11. Adding TCP-MD5 key to AO_REQUIRED socket
> 12. Setting AO_REQUIRED on a socket with TCP-MD5 key
> 13. Adding TCP-AO key on already established connection without any key

Oh, yeah, forgot to mention, there are another 2 tests for TCP_CLOSE
socket (just a new one), that has both TCP-AO and TCP-MD5 keys and tries
to call connect(). In discussion with the team, it seems really
unexpected situation and better to force userspace to remove either AO
or MD5 key before calling connect(). Those from the output in [1] are:

> ok 39 AO+MD5 server: client with both [TCP-MD5] and TCP-AO keys:
connect() was prevented
> ok 40 AO+MD5 server: client with both TCP-MD5 and [TCP-AO] keys:
connect() was prevented

> 
> And then another bunch of tests that check TCP-AO/TCP-MD5/unsigned
> interaction in non/default VRFs.
> I think the output of selftest [1] is more-or-less self-descriptive,
> correct me if I could improve that.
> 
> [1]
> https://github.com/0x7f454c46/linux/commit/d7b321f2b5a481e5ff0e80e2e0b3503b1ddb9817

Thanks,
          Dmitry