linux-kernel - Re: [PATCH v14 net-next 08/23] net/tcp: Add AO sign to RST packets

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]

Message-ID: <b1f1e94b-593a-4eb7-907c-cf5d61d74510@arista.com>
Date:   Wed, 11 Oct 2023 20:23:37 +0100
From:   Dmitry Safonov <dima@...sta.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     David Ahern <dsahern@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org,
        Andy Lutomirski <luto@...capital.net>,
        Ard Biesheuvel <ardb@...nel.org>,
        Bob Gilligan <gilligan@...sta.com>,
        Dan Carpenter <error27@...il.com>,
        David Laight <David.Laight@...lab.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Donald Cassidy <dcassidy@...hat.com>,
        Eric Biggers <ebiggers@...nel.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Francesco Ruggeri <fruggeri05@...il.com>,
        "Gaillardetz, Dominik" <dgaillar@...na.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Ivan Delalande <colona@...sta.com>,
        Leonard Crestez <cdleonard@...il.com>,
        "Nassiri, Mohammad" <mnassiri@...na.com>,
        Salam Noureddine <noureddine@...sta.com>,
        Simon Horman <simon.horman@...igine.com>,
        "Tetreault, Francois" <ftetreau@...na.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v14 net-next 08/23] net/tcp: Add AO sign to RST packets

On 10/11/23 19:04, Eric Dumazet wrote:
> On Tue, Oct 10, 2023 at 1:07 AM Dmitry Safonov <dima@...sta.com> wrote:
[..]
>>  struct tcp_key {
>>         union {
>> -               struct tcp_ao_key *ao_key;
>> +               struct {
>> +                       struct tcp_ao_key *ao_key;
>> +                       u32 sne;
>> +                       char *traffic_key;
> 
> Move sne after traffic_key to avoid a hole on 64bit arches.

Sure, thanks!

[..]
>> @@ -435,6 +495,46 @@ struct tcp_ao_key *tcp_v4_ao_lookup(const struct sock *sk, struct sock *addr_sk,
>>         return tcp_ao_do_lookup(sk, addr, AF_INET, sndid, rcvid);
>>  }
>>
>> +int tcp_ao_prepare_reset(const struct sock *sk, struct sk_buff *skb,
>> +                        const struct tcp_ao_hdr *aoh, int l3index,
>> +                        struct tcp_ao_key **key, char **traffic_key,
>> +                        bool *allocated_traffic_key, u8 *keyid, u32 *sne)
>> +{
>> +       struct tcp_ao_info *ao_info;
>> +
>> +       *allocated_traffic_key = false;
>> +       /* If there's no socket - than initial sisn/disn are unknown.
>> +        * Drop the segment. RFC5925 (7.7) advises to require graceful
>> +        * restart [RFC4724]. Alternatively, the RFC5925 advises to
>> +        * save/restore traffic keys before/after reboot.
>> +        * Linux TCP-AO support provides TCP_AO_ADD_KEY and TCP_AO_REPAIR
>> +        * options to restore a socket post-reboot.
>> +        */
>> +       if (!sk)
>> +               return -ENOTCONN;
>> +
>> +       if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_NEW_SYN_RECV)) {
>> +               return -1;
>> +       } else {
>> +               struct tcp_ao_key *rnext_key;
>> +
>> +               if (sk->sk_state == TCP_TIME_WAIT)
> 
> Why not adding TCPF_TIME_WAIT in the prior test ?

Well, twsk is wired up here in the next patch "net/tcp: Add TCP-AO sign
to twsk", while the condition for request sockets is the patch after that.
I probably can move it to the same condition just to polish the code in
the middle of patch set, I guess it felt not critical when I split it by
patches.

> 
>> +                       return -1;

[..]

Thanks,
           Dmitry