netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: netdev@vger.kernel.org, pabeni@redhat.com, borisp@nvidia.com,
	gal@nvidia.com, cratiu@nvidia.com, rrameshbabu@nvidia.com,
	steffen.klassert@secunet.com, tariqt@nvidia.com,
	mingtao@meta.com, knekritz@meta.com,
	Lance Richardson <lance604@gmail.com>
Subject: Re: [RFC net-next 01/15] psp: add documentation
Date: Wed, 5 Jun 2024 15:24:31 -0700	[thread overview]
Message-ID: <20240605152431.5f24cb45@kernel.org> (raw)
In-Reply-To: <6660c673921ff_35916d294ef@willemb.c.googlers.com.notmuch>

On Wed, 05 Jun 2024 16:11:31 -0400 Willem de Bruijn wrote:
> > 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.  
> 
> Is that sufficient if an initial encrypted packet could get reordered
> by the network to arrive before a plaintext retransmit of a lower
> seqno?

Yes, I believe that's fine. 

I will document this clearer but both sides must be pretty precise in
their understanding when the switchover happens. They must read what 
they expect to be clear text, and then install the Tx key thus locking
down the socket.

1. If they under-read and clear text data is already queued - the kernel
   will error out.
2. If they under-read and clear text arrives later - the connection will
   hang.
3. If they over-read they will presumably get PSP-protected data
   which they have no way of validating, since it won't be secured by
   user crypto.

We could protect from over-read (case 3) by refusing to give out
PSP-protected data until keys are installed. But it adds to the fast
path and I don't think it's all that beneficial, since there's no way
to protect a sloppy application from under-read (case 2).

Back to your question about reordering plain text with cipher text:
the application should not lock down the socket until it gets all
its clear text. So clear text retransmissions _after_ lock down must be
spurious. The only worry is that we lose an ACK and never tell
the other side that we got all the clear text. But we're guaranteed
to successfully ACK any PSP-protected data, so if we receive some
there is no way to get stuck.  Let me copy / paste the diagram:

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

Looking as Host A, if we receive encrypted data, we must have
allocated and sent key (line 7) so we will start accepting encrypted
data. But at this point we are also accepting plain text (until we
reach line 9). We will send a plain text (S)ACK to encrypted data, 
but that's fine too since Host B hasn't seen any encrypted data from us
and will accept such ACKs.

> Both scenarios make sense. It is unfortunately harder to be sure that
> we have captured all edge cases.

Are you trying to say packetdrill without saying packetdrill? :)

> An issue related to the rcv_nxt cut-point, not sure how important: the
> plaintext packet contents are protected by user crypto before upgrade.
> But the TCP headers are not. PSP relies on TCP PAWS against replay
> protection. It is possible for a MITM to offset all seqno from the
> start of connection establishment. I don't see an immediate issue. But
> at a minimum it could be possible to insert or delete before PSP is
> upgraded.

Yes, the "cut off" point must be quite clearly defined, because both
sides must precisely read out all the clear text. Then they install 
the Tx key and anything they read must have been PSP-protected.

Hope I understood the point.

  reply	other threads:[~2024-06-05 22:24 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-10  3:04 [RFC net-next 00/15] add basic PSP encryption for TCP connections Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 01/15] psp: add documentation Jakub Kicinski
2024-05-10 22:19   ` Saeed Mahameed
2024-05-11  0:11     ` Jakub Kicinski
2024-05-11  9:41       ` Vadim Fedorenko
2024-05-11 16:25         ` David Ahern
2024-06-26 13:57       ` Sasha Levin
2024-05-13  1:24   ` Willem de Bruijn
2024-05-29 17:35     ` Jakub Kicinski
2024-05-30  0:47       ` Willem de Bruijn
2024-05-30 19:51         ` Jakub Kicinski
2024-05-30 20:15           ` Jakub Kicinski
2024-05-30 21:03             ` Willem de Bruijn
2024-05-31 13:56           ` Willem de Bruijn
2024-06-05  0:08             ` Jakub Kicinski
2024-06-05 20:11               ` Willem de Bruijn
2024-06-05 22:24                 ` Jakub Kicinski [this message]
2024-06-06  2:40                   ` Willem de Bruijn
2024-06-27 15:14       ` Lance Richardson
2024-06-27 22:33         ` Jakub Kicinski
2024-06-28 19:33           ` Lance Richardson
2024-06-28 23:41             ` Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 02/15] psp: base PSP device support Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 03/15] net: modify core data structures for PSP datapath support Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 04/15] tcp: add datapath logic for PSP with inline key exchange Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 05/15] psp: add op for rotation of secret state Jakub Kicinski
2024-05-16 19:59   ` Lance Richardson
2024-05-29 17:43     ` Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 06/15] net: psp: add socket security association code Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 07/15] net: psp: update the TCP MSS to reflect PSP packet overhead Jakub Kicinski
2024-05-13  1:47   ` Willem de Bruijn
2024-05-29 17:48     ` Jakub Kicinski
2024-05-30  0:52       ` Willem de Bruijn
2024-05-10  3:04 ` [RFC net-next 08/15] psp: track generations of secret state Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 09/15] net/mlx5e: Support PSP offload functionality Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 10/15] net/mlx5e: Implement PSP operations .assoc_add and .assoc_del Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 11/15] net/mlx5e: Implement PSP Tx data path Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 12/15] net/mlx5e: Add PSP steering in local NIC RX Jakub Kicinski
2024-05-13  1:52   ` Willem de Bruijn
2024-05-10  3:04 ` [RFC net-next 13/15] net/mlx5e: Configure PSP Rx flow steering rules Jakub Kicinski
2024-05-10  3:04 ` [RFC net-next 14/15] net/mlx5e: Add Rx data path offload Jakub Kicinski
2024-05-13  1:54   ` Willem de Bruijn
2024-05-29 18:38     ` Jakub Kicinski
2024-05-30  9:04       ` Cosmin Ratiu
2024-05-10  3:04 ` [RFC net-next 15/15] net/mlx5e: Implement PSP key_rotate operation Jakub Kicinski
2024-05-29  9:16 ` [RFC net-next 00/15] add basic PSP encryption for TCP connections Boris Pismenny
2024-05-29 18:50   ` Jakub Kicinski
2024-05-29 20:01     ` Boris Pismenny
2024-05-29 20:38       ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240605152431.5f24cb45@kernel.org \
    --to=kuba@kernel.org \
    --cc=borisp@nvidia.com \
    --cc=cratiu@nvidia.com \
    --cc=gal@nvidia.com \
    --cc=knekritz@meta.com \
    --cc=lance604@gmail.com \
    --cc=mingtao@meta.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rrameshbabu@nvidia.com \
    --cc=steffen.klassert@secunet.com \
    --cc=tariqt@nvidia.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).