Re: [RFC net-next 01/15] psp: add documentation

Jakub Kicinski
2024-06-05T00:08:51Z

On Fri, 31 May 2024 09:56:42 -0400 Willem de Bruijn wrote:
> > > If one peer can enter the state where it drops all plaintext, while
> > > the other decides to close the connection before completing the
> > > upgrade, and thus sends a plaintext FIN.
> > >
> > > If (big if) that can happen, then the connection cannot be cleanly
> > > closed.
> >
> > Hm. And we can avoid this by only enforcing encryption of data-less
> > segments once we've seen some encrypted data?
>
> That would help. It may also be needed to accept a pure ACK right at
> the upgrade seqno. Depends on the upgrade process.
>
> Which may be worth documenting explicitly: the system call and network
> packet exchange from when one peer initiates (by generating its local
> key) until the connection is fully encrypted. That also allows poking
> at the various edge cases that may happen if packets are lost, or when
> actions can race.

Dunno if the format below is good, but you're very right.
At least to me writing the diagram was an hour well spent :)

> One unexpected example of the latter that I came across was Tx SADB
> key insertion in tail edge cases taking longer than network RTT, for
> instance.
>
> The kernel API can be exercised in a variety of ways, not all of them
> will uphold the correctness. Documenting how it should be used should
> help.
>
> Even better when it reduces the option space. As it already does by
> failing a Tx key install until Rx is configured.

Something along these lines?

"Sequence" diagram of the worst case scenario:

01 p Host A Host B
02 l t ~~~~~~~~~~~[TCP 3 WHS]~~~~~~~~~~
03 a e ~~~~~~[crypto negotiation]~~~~~~
04 i x [Rx key alloc = K-B]
05 n t <--- [app] K-B key send
06 ------[Rx key alloc = K-A]-
07 [app] K-A key send -->|
08 [TCP] K-B input <-----
08 P [TCP] K-B ACK ---->|
09 S R [app] recv(K-B) |
10 P x [app] [Tx key set] |
11 --------------------------
12 P T [app] send(RPC) #####>|
13 S x |<---- [TCP] Seq OoO! queue RPC, SACK
14 P [TCP] retr K-A --->|
15 | `-> [TCP] K-A input
16 | <--- [TCP] K-A ACK (or FIN)
17 | [app] recv(K-A)
18 | [app] [Tx key set]
19 -----------------------------------
20

There is a causal dependency between Host B allocating the key (line 4),
sending it (line 5) and Host A receiving it (line 8). Since Host B will
accept PSP packets as soon as it allocated the key, Host A does not
need to wait to start using the key (line 12). Host B will queue the
RPC to the socket (line 13).

[Problem #1]

However, because Host B does not have a Tx key, the ACK / SACK packet
(line 13) will not be encrypted. (Similarly if Host B decided to close
the connection at this point, the resulting FIN packet would not be
encrypted.) Host B needs to accept unencrypted non-data segments
(pure acks, pure FIN) until it sees an encrypted packet from Host B.

[Problem #2]

The retansmissions of K-A are unencrypted, to avoid sending the same
data in encrypted and unencrypted form. This poses a risk if an ACK
gets lost but both hosts end up in the PSP Tx state. Assume that Host A
did not send the RPC (line 12), and the retransmission (line 14)
happens as an RTO or TLP. Host B may already reach PSP Tx state (line
"20") and expect encrypted data. Plain text retransmissions (with
sequence number before rcv_nxt) must be accepted until Host B sees
encrypted data from Host A.

With that I think the state machine needs to be amended:

Event | Normal TCP | Rx PSP | Tx PSP | PSP full |
-----------------------------------------------------------------------
Rx plain (new) | accept | accept | drop | drop |

Rx plain | accept | accept | accept | drop |
(ACK|FIN|rtx) | | | | |

Rx PSP (good) | drop | accept | accept | accept |

Rx PSP (bad | drop | drop | drop | drop |
(crypt, !=SPI) | | | | |

Tx | plain text | plain text | encrypted | encrypted |
| | | (excl. rtx) | (excl. rtx) |

> > > Another example where a peer stays open and stays retrying if it has
> > > upgraded and drops all plaintext.
>
> May want to always allow plaintext RSTs. This is a potential DoS
> vector.

Because of key exhaustion? Or we can be tricked into spamming someone
with retranmissions and ignoring their RST?

> In all these cases, I suppose this has already been figured
> out for TLS.

Assuming the answer above is "key exhaustion" - I wouldn't be surprised
if it wasn't :(