[RFC net-next 00/15] add basic PSP encryption for TCP connections

[RFC net-next 00/15] add basic PSP encryption for TCP connections

[Jakub Kicinski]

Hi!

Add support for PSP encryption of TCP connections.

PSP is a protocol out of Google:
https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
which shares some similarities with IPsec. I added some more info
in the first patch so I'll keep it short here.

The protocol can work in multiple modes including tunneling.
But I'm mostly interested in using it as TLS replacement because
of its superior offload characteristics. So this patch does three
things:

 - it adds "core" PSP code
   PSP is offload-centric, and requires some additional care and
   feeding, so first chunk of the code exposes device info.
   This part can be reused by PSP implementations in xfrm, tunneling etc.

 - TCP integration TLS style
   Reuse some of the existing concepts from TLS offload, such as
   attaching crypto state to a socket, marking skbs as "decrypted",
   egress validation. PSP does not prescribe key exchange protocols.
   To use PSP as a more efficient TLS offload we intend to perform
   a TLS handshake ("inline" in the same TCP connection) and negotiate
   switching to PSP based on capabilities of both endpoints.
   This is also why I'm not including a software implementation.
   Nobody would use it in production, software TLS is faster,
   it has larger crypto records.

 - mlx5 implementation
   That's mostly other people's work, not 100% sure those folks
   consider it ready hence the RFC in the title. But it works :)

Not posted, queued a branch [1] are follow up pieces:
 - standard stats
 - netdevsim implementation and tests

[1] https://github.com/kuba-moo/linux/tree/psp

Jakub Kicinski (8):
  psp: add documentation
  psp: base PSP device support
  net: modify core data structures for PSP datapath support
  tcp: add datapath logic for PSP with inline key exchange
  psp: add op for rotation of secret state
  net: psp: add socket security association code
  net: psp: update the TCP MSS to reflect PSP packet overhead
  psp: track generations of secret state

Raed Salem (7):
  net/mlx5e: Support PSP offload functionality
  net/mlx5e: Implement PSP operations .assoc_add and .assoc_del
  net/mlx5e: Implement PSP Tx data path
  net/mlx5e: Add PSP steering in local NIC RX
  net/mlx5e: Configure PSP Rx flow steering rules
  net/mlx5e: Add Rx data path offload
  net/mlx5e: Implement PSP key_rotate operation

 Documentation/netlink/specs/psp.yaml          | 186 +++++
 Documentation/networking/index.rst            |   1 +
 Documentation/networking/psp.rst              | 138 ++++
 .../net/ethernet/mellanox/mlx5/core/Kconfig   |  11 +
 .../net/ethernet/mellanox/mlx5/core/Makefile  |   5 +-
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |   7 +-
 .../net/ethernet/mellanox/mlx5/core/en/fs.h   |   2 +-
 .../ethernet/mellanox/mlx5/core/en/params.c   |   4 +-
 .../mellanox/mlx5/core/en_accel/en_accel.h    |  50 +-
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |   2 +-
 .../mellanox/mlx5/core/en_accel/nisp.c        | 209 +++++
 .../mellanox/mlx5/core/en_accel/nisp.h        |  55 ++
 .../mellanox/mlx5/core/en_accel/nisp_fs.c     | 737 ++++++++++++++++++
 .../mellanox/mlx5/core/en_accel/nisp_fs.h     |  30 +
 .../mlx5/core/en_accel/nisp_offload.c         |  52 ++
 .../mellanox/mlx5/core/en_accel/nisp_rxtx.c   | 304 ++++++++
 .../mellanox/mlx5/core/en_accel/nisp_rxtx.h   | 124 +++
 .../net/ethernet/mellanox/mlx5/core/en_main.c |   9 +
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  10 +
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   |  10 +-
 drivers/net/ethernet/mellanox/mlx5/core/fw.c  |   6 +
 .../ethernet/mellanox/mlx5/core/lib/crypto.h  |   1 +
 .../mellanox/mlx5/core/lib/psp_defs.h         |  28 +
 .../net/ethernet/mellanox/mlx5/core/main.c    |   5 +
 .../net/ethernet/mellanox/mlx5/core/nisp.c    |  24 +
 .../net/ethernet/mellanox/mlx5/core/nisp.h    |  15 +
 include/linux/mlx5/device.h                   |   4 +
 include/linux/mlx5/driver.h                   |   2 +
 include/linux/mlx5/mlx5_ifc.h                 |  98 ++-
 include/linux/netdevice.h                     |   4 +
 include/linux/skbuff.h                        |   3 +
 include/linux/tcp.h                           |   3 +
 include/net/dropreason-core.h                 |   6 +
 include/net/psp.h                             |  12 +
 include/net/psp/functions.h                   | 150 ++++
 include/net/psp/types.h                       | 182 +++++
 include/net/sock.h                            |   4 +
 include/uapi/linux/psp.h                      |  66 ++
 net/Kconfig                                   |   1 +
 net/Makefile                                  |   1 +
 net/core/gro.c                                |   2 +
 net/core/skbuff.c                             |   4 +
 net/core/sock.c                               |   2 +
 net/ipv4/inet_connection_sock.c               |   2 +
 net/ipv4/tcp.c                                |   2 +
 net/ipv4/tcp_ipv4.c                           |  13 +-
 net/ipv4/tcp_minisocks.c                      |  21 +-
 net/ipv4/tcp_output.c                         |  16 +-
 net/ipv6/ipv6_sockglue.c                      |   6 +-
 net/ipv6/tcp_ipv6.c                           |  22 +-
 net/mptcp/protocol.c                          |   2 +
 net/psp/Kconfig                               |  15 +
 net/psp/Makefile                              |   5 +
 net/psp/psp-nl-gen.c                          | 119 +++
 net/psp/psp-nl-gen.h                          |  39 +
 net/psp/psp.h                                 |  54 ++
 net/psp/psp_main.c                            | 144 ++++
 net/psp/psp_nl.c                              | 517 ++++++++++++
 net/psp/psp_sock.c                            | 276 +++++++
 tools/net/ynl/Makefile.deps                   |   1 +
 60 files changed, 3791 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/netlink/specs/psp.yaml
 create mode 100644 Documentation/networking/psp.rst
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp.h
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp_fs.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp_fs.h
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp_offload.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp_rxtx.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/nisp_rxtx.h
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/psp_defs.h
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/nisp.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/nisp.h
 create mode 100644 include/net/psp.h
 create mode 100644 include/net/psp/functions.h
 create mode 100644 include/net/psp/types.h
 create mode 100644 include/uapi/linux/psp.h
 create mode 100644 net/psp/Kconfig
 create mode 100644 net/psp/Makefile
 create mode 100644 net/psp/psp-nl-gen.c
 create mode 100644 net/psp/psp-nl-gen.h
 create mode 100644 net/psp/psp.h
 create mode 100644 net/psp/psp_main.c
 create mode 100644 net/psp/psp_nl.c
 create mode 100644 net/psp/psp_sock.c

-- 
2.45.0

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Paul Wouters]

Jakub Kicinski wrote:

> Add support for PSP encryption of TCP connections.
> 
> PSP is a protocol out of Google:
> https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
> which shares some similarities with IPsec. I added some more info
> in the first patch so I'll keep it short here.

Speaking as an IETF contributor, I am little surprised here. I know
the google people reached out at IETF and were told their stuff is
so similar to IPsec, maybe they should talk to the IPsecME Working
Group. There, I believe Steffen Klassert started working on supporting
the PSP features requested using updates to the ESP/WESP IPsec protocol,
such as support for encryption offset to reveal protocol/ports for
routing encrypted traffic.

It is not very useful to add more very similar encryption techniques to
the kernel. It scatters development efforts and actually makes it harder
for people to use functionality by having to change to a whole new sub
system (and its configuration methods) just to get one different feature
of packet encryption.

> The protocol can work in multiple modes including tunneling.
> But I'm mostly interested in using it as TLS replacement because
> of its superior offload characteristics. So this patch does three
> things:

Is this issue related to draft-pismenny-tls-dtls-plaintext-sequence-number ?

https://datatracker.ietf.org/doc/draft-pismenny-tls-dtls-plaintext-sequence-number/

If so, then it seems it would be far better to re-engage the TLS WG to
see if instead of "having no interest, do it but elsewhere", we can
convince them to "there is interest, let's do it here at the TLS WG".
I could help with the latter.

Paul

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Boris Pismenny]

On 22.05.2024 14:56, Paul Wouters wrote:
> External email: Use caution opening links or attachments
>
>
> Jakub Kicinski wrote:
>
>> Add support for PSP encryption of TCP connections.
>>
>> PSP is a protocol out of Google:
>> https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
>> which shares some similarities with IPsec. I added some more info
>> in the first patch so I'll keep it short here.
>
> Speaking as an IETF contributor, I am little surprised here. I know
> the google people reached out at IETF and were told their stuff is
> so similar to IPsec, maybe they should talk to the IPsecME Working
> Group. There, I believe Steffen Klassert started working on supporting
> the PSP features requested using updates to the ESP/WESP IPsec protocol,
> such as support for encryption offset to reveal protocol/ports for
> routing encrypted traffic.
>
> It is not very useful to add more very similar encryption techniques to
> the kernel. It scatters development efforts and actually makes it harder
> for people to use functionality by having to change to a whole new sub
> system (and its configuration methods) just to get one different feature
> of packet encryption.
>
>> The protocol can work in multiple modes including tunneling.
>> But I'm mostly interested in using it as TLS replacement because
>> of its superior offload characteristics. So this patch does three
>> things:
>
> Is this issue related to 
> draft-pismenny-tls-dtls-plaintext-sequence-number ?


This has nothing to do with it.


>
> https://datatracker.ietf.org/doc/draft-pismenny-tls-dtls-plaintext-sequence-number/ 
>
>
> If so, then it seems it would be far better to re-engage the TLS WG to
> see if instead of "having no interest, do it but elsewhere", we can
> convince them to "there is interest, let's do it here at the TLS WG".
> I could help with the latter.
>
> Paul

Boris

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Steffen Klassert]

On Wed, May 22, 2024 at 08:56:02AM -0400, Paul Wouters wrote:
> Jakub Kicinski wrote:
> 
> > Add support for PSP encryption of TCP connections.
> > 
> > PSP is a protocol out of Google:
> > https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
> > which shares some similarities with IPsec. I added some more info
> > in the first patch so I'll keep it short here.
> 
> Speaking as an IETF contributor, I am little surprised here. I know
> the google people reached out at IETF and were told their stuff is
> so similar to IPsec, maybe they should talk to the IPsecME Working
> Group. There, I believe Steffen Klassert started working on supporting
> the PSP features requested using updates to the ESP/WESP IPsec protocol,
> such as support for encryption offset to reveal protocol/ports for
> routing encrypted traffic.

This was somewhat semipublic information, so I did not talk about
it on the lists yet. Today we published the draft, it can be found here:

https://datatracker.ietf.org/doc/draft-klassert-ipsecme-wespv2/

Please note that the packet format specification is portable to other
protocol use cases, such as PSP. It uses IKEv2 as a negotiation
protocol and does not define any key derivation etc. as PSP does.
But it can be also used with other protocols for key negotiation
and key derivation.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Willem de Bruijn]

Steffen Klassert wrote:
> On Wed, May 22, 2024 at 08:56:02AM -0400, Paul Wouters wrote:
> > Jakub Kicinski wrote:
> > 
> > > Add support for PSP encryption of TCP connections.
> > > 
> > > PSP is a protocol out of Google:
> > > https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
> > > which shares some similarities with IPsec. I added some more info
> > > in the first patch so I'll keep it short here.
> > 
> > Speaking as an IETF contributor, I am little surprised here. I know
> > the google people reached out at IETF and were told their stuff is
> > so similar to IPsec, maybe they should talk to the IPsecME Working
> > Group. There, I believe Steffen Klassert started working on supporting
> > the PSP features requested using updates to the ESP/WESP IPsec protocol,
> > such as support for encryption offset to reveal protocol/ports for
> > routing encrypted traffic.
> 
> This was somewhat semipublic information, so I did not talk about
> it on the lists yet. Today we published the draft, it can be found here:
> 
> https://datatracker.ietf.org/doc/draft-klassert-ipsecme-wespv2/
> 
> Please note that the packet format specification is portable to other
> protocol use cases, such as PSP. It uses IKEv2 as a negotiation
> protocol and does not define any key derivation etc. as PSP does.
> But it can be also used with other protocols for key negotiation
> and key derivation.

Very nice. Thanks for posting, Steffen.

One point about why PSP is that the exact protocol and packet format
is already in use and supported by hardware.

It makes sense to work to get to an IETF standard protocol that
captures the same benefits. But that is independent from enabling what
is already implemented.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Paul Wouters]

On Tue, 28 May 2024, Willem de Bruijn wrote:

> One point about why PSP is that the exact protocol and packet format
> is already in use and supported by hardware.

Using mostly the IPsec hw code? :)

> It makes sense to work to get to an IETF standard protocol that
> captures the same benefits. But that is independent from enabling what
> is already implemented.

How many different packet encryption methods should the linux kernel
have? There are good reasons to go through standard bodies. Doing your
own thing and then saying "but we did it already" to me does not feel
like a strong argument. That's how we got wireguard with all of its
issues of being written for a single use case, and now being unfit for
generic use cases.

Going through standards organizations also gains you interoperability
with non-linux (hardware) vendors, again reducing the number of
different mostly similar schemes that need to be supported and
maintained for years or decades.

Paul

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Jakub Kicinski]

On Tue, 28 May 2024 11:33:33 -0400 (EDT) Paul Wouters wrote:
> > It makes sense to work to get to an IETF standard protocol that
> > captures the same benefits. But that is independent from enabling what
> > is already implemented.  
> 
> How many different packet encryption methods should the linux kernel
> have? There are good reasons to go through standard bodies. Doing your
> own thing and then saying "but we did it already" to me does not feel
> like a strong argument. That's how we got wireguard with all of its
> issues of being written for a single use case, and now being unfit for
> generic use cases.

Now you made me curious. What's wrong with wireguard?
I have only heard good things.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Willem de Bruijn]

Paul Wouters wrote:
> On Tue, 28 May 2024, Willem de Bruijn wrote:
> 
> > One point about why PSP is that the exact protocol and packet format
> > is already in use and supported by hardware.
> 
> Using mostly the IPsec hw code? :)

Not necessarily.

A goal of PSP was to allow an O(1) state device implementation that
does away with the SADB. Using key derivation on Rx and keys in
descriptor on Tx.
 
> > It makes sense to work to get to an IETF standard protocol that
> > captures the same benefits. But that is independent from enabling what
> > is already implemented.
> 
> How many different packet encryption methods should the linux kernel
> have? There are good reasons to go through standard bodies. Doing your
> own thing and then saying "but we did it already" to me does not feel
> like a strong argument. That's how we got wireguard with all of its
> issues of being written for a single use case, and now being unfit for
> generic use cases.
> 
> Going through standards organizations also gains you interoperability
> with non-linux (hardware) vendors, again reducing the number of
> different mostly similar schemes that need to be supported and
> maintained for years or decades.

I don't disagree on the merits of a standards process, of course. It's
just moot at this point wrt PSP. Hardware support and some users are
here. A new packet format cannot be supported retroactively.

That said, an IETF (ESP) protocol is a potential upgrade path even for
existing users in the longer term. If we can make sure that it covers
all the key PSP features.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Boris Pismenny]

On 10.05.2024 05:04, Jakub Kicinski wrote:
> External email: Use caution opening links or attachments
>
>
> Hi!
>
> Add support for PSP encryption of TCP connections.
>
> PSP is a protocol out of Google:
> https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
> which shares some similarities with IPsec. I added some more info
> in the first patch so I'll keep it short here.
>
> The protocol can work in multiple modes including tunneling.
> But I'm mostly interested in using it as TLS replacement because
> of its superior offload characteristics. 

Hi!

Thank you for doing this. I agree that TLS-like socket support
is a main use-case. I'd like to hear what you think on a few
other use-cases that I think should be considered as well
since it may be difficult to add them as an afterthought:
- Tunnel mode. What are your plans for tunnel mode? Clearly it
is different from the current approach in some aspects, for
example, no sockets will be involved.
- RDMA. The ultra ethernet group has mentioned RDMA encryption
using PSP. Do you think that RDMA verbs will support PSP in
a similar manner to sockets? i.e., using netlink to pass
parameters to the device and linking QPs to PSP SAs?
- Virtualization. How does PSP work from a VM? is the key
shared with the hypervisor or is it private per-VM?
and what about containers?

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Jakub Kicinski]

On Wed, 29 May 2024 11:16:12 +0200 Boris Pismenny wrote:
> Thank you for doing this. I agree that TLS-like socket support
> is a main use-case. I'd like to hear what you think on a few
> other use-cases that I think should be considered as well
> since it may be difficult to add them as an afterthought:
> - Tunnel mode. What are your plans for tunnel mode? Clearly it
> is different from the current approach in some aspects, for
> example, no sockets will be involved.

The drivers should only decap for known L4 protos, I think that's
the only catch when we add tunnel support. Otherwise it should be
fairly straightforward. Open a UDP socket in the kernel. Get a key
+ SPI using existing ops. Demux within the UDP socket using SPI.

> - RDMA. The ultra ethernet group has mentioned RDMA encryption
> using PSP. Do you think that RDMA verbs will support PSP in
> a similar manner to sockets? i.e., using netlink to pass
> parameters to the device and linking QPs to PSP SAs?
> - Virtualization. How does PSP work from a VM? is the key
> shared with the hypervisor or is it private per-VM?

Depends on the deployment and security model, really, but I'd
expect the device key is shared, hypervisor is responsible for
rotations, and mediates all key ops from the guests.

> and what about containers?

I tried to apply some of the lessons learned from TLS offload and made
the "PSP device" a separate object. This should make it easy to
"forward" the offload to software/container netdevs.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Boris Pismenny]

On 29.05.2024 20:50, Jakub Kicinski wrote:
> On Wed, 29 May 2024 11:16:12 +0200 Boris Pismenny wrote:
>> Thank you for doing this. I agree that TLS-like socket support
>> is a main use-case. I'd like to hear what you think on a few
>> other use-cases that I think should be considered as well
>> since it may be difficult to add them as an afterthought:
>> - Tunnel mode. What are your plans for tunnel mode? Clearly it
>> is different from the current approach in some aspects, for
>> example, no sockets will be involved.
> The drivers should only decap for known L4 protos, I think that's
> the only catch when we add tunnel support. Otherwise it should be
> fairly straightforward. Open a UDP socket in the kernel. Get a key
> + SPI using existing ops. Demux within the UDP socket using SPI.

IIUC, you refer to tunnel mode as if it offloads
encryption alone while keeping headers intact. But,
what I had in mind is a fully offloaded tunnel.
This is called packet offload mode in IPsec,
and with encryption such offloads rely on TC.

Note that the main use-case for PSP tunnel mode,
unlike transport mode, is carrying VM traffic as
indicated by the spec:
"The tunnel mode packet format is typically used in
virtualized environments.". With virtualization, encap/decap offload is an implicit assumption if not a performance necessity.
>> - RDMA. The ultra ethernet group has mentioned RDMA encryption
>> using PSP. Do you think that RDMA verbs will support PSP in
>> a similar manner to sockets? i.e., using netlink to pass
>> parameters to the device and linking QPs to PSP SAs?
>> - Virtualization. How does PSP work from a VM? is the key
>> shared with the hypervisor or is it private per-VM?
> Depends on the deployment and security model, really, but I'd
> expect the device key is shared, hypervisor is responsible for
> rotations, and mediates all key ops from the guests.

I can imagine how this will work, but there are a few issues:
- Guests may run out of Tx keys, but they can't initiate key
rotation without affecting others. This fate sharing between
VMs seems undesirable.
- Unclear what sort of mediation is the hypervisor expected
to provide: on the one hand, block a key rotation request and
the requesting guest is denied service, on the other hand,
allow key rotation and a guest may spam these requests to
the hypervisor, which will also spam other VMs with
notifications of key rotation.

>
>> and what about containers?
> I tried to apply some of the lessons learned from TLS offload and made
> the "PSP device" a separate object. This should make it easy to
> "forward" the offload to software/container netdevs.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Jakub Kicinski]

On Wed, 29 May 2024 22:01:52 +0200 Boris Pismenny wrote:
> > The drivers should only decap for known L4 protos, I think that's
> > the only catch when we add tunnel support. Otherwise it should be
> > fairly straightforward. Open a UDP socket in the kernel. Get a key
> > + SPI using existing ops. Demux within the UDP socket using SPI.
> 
> IIUC, you refer to tunnel mode as if it offloads
> encryption alone while keeping headers intact. But,
> what I had in mind is a fully offloaded tunnel.
> This is called packet offload mode in IPsec,
> and with encryption such offloads rely on TC.

Do you see any challenge?

> > Depends on the deployment and security model, really, but I'd
> > expect the device key is shared, hypervisor is responsible for
> > rotations, and mediates all key ops from the guests.
> 
> I can imagine how this will work, but there are a few issues:
> - Guests may run out of Tx keys, but they can't initiate key
> rotation without affecting others. This fate sharing between
> VMs seems undesirable.
> - Unclear what sort of mediation is the hypervisor expected
> to provide: on the one hand, block a key rotation request and
> the requesting guest is denied service, on the other hand,
> allow key rotation and a guest may spam these requests to
> the hypervisor, which will also spam other VMs with
> notifications of key rotation.

I don't have much experience working with VMs, or a good understanding
of what mlx5 does internally. Without access to the details of even a
single device which does PSP - any comment I'd make would be too much
of a speculation for my taste :(

On the NFP I'm pretty sure we could have given every VM a separate
device key, no problem.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Steffen Klassert]

On Tue, May 28, 2024 at 09:49:34AM -0400, Willem de Bruijn wrote:
> Steffen Klassert wrote:
> > On Wed, May 22, 2024 at 08:56:02AM -0400, Paul Wouters wrote:
> > > Jakub Kicinski wrote:
> > > 
> > > > Add support for PSP encryption of TCP connections.
> > > > 
> > > > PSP is a protocol out of Google:
> > > > https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
> > > > which shares some similarities with IPsec. I added some more info
> > > > in the first patch so I'll keep it short here.
> > > 
> > > Speaking as an IETF contributor, I am little surprised here. I know
> > > the google people reached out at IETF and were told their stuff is
> > > so similar to IPsec, maybe they should talk to the IPsecME Working
> > > Group. There, I believe Steffen Klassert started working on supporting
> > > the PSP features requested using updates to the ESP/WESP IPsec protocol,
> > > such as support for encryption offset to reveal protocol/ports for
> > > routing encrypted traffic.
> > 
> > This was somewhat semipublic information, so I did not talk about
> > it on the lists yet. Today we published the draft, it can be found here:
> > 
> > https://datatracker.ietf.org/doc/draft-klassert-ipsecme-wespv2/
> > 
> > Please note that the packet format specification is portable to other
> > protocol use cases, such as PSP. It uses IKEv2 as a negotiation
> > protocol and does not define any key derivation etc. as PSP does.
> > But it can be also used with other protocols for key negotiation
> > and key derivation.
> 
> Very nice. Thanks for posting, Steffen.
> 
> One point about why PSP is that the exact protocol and packet format
> is already in use and supported by hardware.
> 
> It makes sense to work to get to an IETF standard protocol that
> captures the same benefits. But that is independent from enabling what
> is already implemented.

Sure, PSP is already implemented in hardware and needs to be supported.
I don't want to judge if it was a good idea to start this without
talking to the IETF, but maybe now Google can join the effort at the
IETF do standardize a modern encryption protocol that meets all the
requirements we have these days. This will be likely on the agenda of
the next IETF IPsecME working group meeting, so would be nice to
see somebody from the Google 'PSP team' there.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Willem de Bruijn]

Steffen Klassert wrote:
> On Tue, May 28, 2024 at 09:49:34AM -0400, Willem de Bruijn wrote:
> > Steffen Klassert wrote:
> > > On Wed, May 22, 2024 at 08:56:02AM -0400, Paul Wouters wrote:
> > > > Jakub Kicinski wrote:
> > > > 
> > > > > Add support for PSP encryption of TCP connections.
> > > > > 
> > > > > PSP is a protocol out of Google:
> > > > > https://github.com/google/psp/blob/main/doc/PSP_Arch_Spec.pdf
> > > > > which shares some similarities with IPsec. I added some more info
> > > > > in the first patch so I'll keep it short here.
> > > > 
> > > > Speaking as an IETF contributor, I am little surprised here. I know
> > > > the google people reached out at IETF and were told their stuff is
> > > > so similar to IPsec, maybe they should talk to the IPsecME Working
> > > > Group. There, I believe Steffen Klassert started working on supporting
> > > > the PSP features requested using updates to the ESP/WESP IPsec protocol,
> > > > such as support for encryption offset to reveal protocol/ports for
> > > > routing encrypted traffic.
> > > 
> > > This was somewhat semipublic information, so I did not talk about
> > > it on the lists yet. Today we published the draft, it can be found here:
> > > 
> > > https://datatracker.ietf.org/doc/draft-klassert-ipsecme-wespv2/
> > > 
> > > Please note that the packet format specification is portable to other
> > > protocol use cases, such as PSP. It uses IKEv2 as a negotiation
> > > protocol and does not define any key derivation etc. as PSP does.
> > > But it can be also used with other protocols for key negotiation
> > > and key derivation.
> > 
> > Very nice. Thanks for posting, Steffen.
> > 
> > One point about why PSP is that the exact protocol and packet format
> > is already in use and supported by hardware.
> > 
> > It makes sense to work to get to an IETF standard protocol that
> > captures the same benefits. But that is independent from enabling what
> > is already implemented.
> 
> Sure, PSP is already implemented in hardware and needs to be supported.
> I don't want to judge if it was a good idea to start this without
> talking to the IETF, but maybe now Google can join the effort at the
> IETF do standardize a modern encryption protocol that meets all the
> requirements we have these days. This will be likely on the agenda of
> the next IETF IPsecME working group meeting, so would be nice to
> see somebody from the Google 'PSP team' there.

Sounds good. We'll try to have someone join the next WG meeting
during IETF 120.

[RFC net-next 00/15] add basic PSP encryption for TCP connections

[Singhai, Anjali]

In reference to this patch series
https://lore.kernel.org/netdev/20240510030435.120935-1-kuba@kernel.org/#t

Thanks a lot  for the PSP crypto enabling patches in the kernel.
Some points that we noticed that could use some enhancements/fixes

1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
Even the Mellanox driver does not implement this set_config ndo_op. 
 
2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
In our device we have 2 options, 
             1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
              As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
             2. Passing key in the descriptor.
              For us we need both these options, so please allow for enhancements.

3. About the PSP and UDP header addition, why is the driver doing it? I guess it's because the SW equivalent for PSP support in the kernel does not exist and just an offload for the device. Again in this case the assumption is either the driver does it or the device will do it.
Hope that is irrelevant for the stack. In our case most likely it will be the device doing it.

4. Why is the driver adding the PSP trailer? Hoping this is between the driver and the device, in our case it's the device that will add the trailer.
 
5. Five way handshake, can this be optimized to 3 way?
Here is what we think is happening right now at the IKE level interaction for the two ends of the session, as PSP protocol does not define it but based on the implementation this is what we gathered.
  Looks like its 5 way handshake that is happening with the session partner.
   For example two sides are called Tx session partner and RX session partner, just because TX initiates the session creation, of course it's a full duplex session.
             1. TX session partner sends over sideband tls channel to the Rx session partner what all versions the TX can do based on caps learnt from the device driver.
             2. The Rx session partner replies with what common versions it can do back to TX session partner based on dev caps it learnt from the device.
             3. Tx session partner tells the driver to generate a spi an session key for the rx side by specifying the version it wants to use for this session. And then sends to Rx session partner the spi and session key and version selected etc.
             4. The Rx session partner at its end then talks to its driver to generate a spi an session key for the rx side by specifying the version sent from TX session partner. And then sends to Tx session partner the spi and session key and version selected etc. The RX session partner also programs the session key and spi it received from TX session partner in HW in its Tx SADB (psp_assoc_add)
            5. Tx session partner programs the received SPI and session key in its HW in its Tx SADB (psp_assoc_add). When done it sends an ACK back to Rx session partner that the session is ready.
 
Should this be optimized to a  3 way handshake to allow for faster session setup? 1 and 3 and 2 and 4 could be combined in an intelligent way. Again may be there is already an optimized way to do 3 way handshake here and the kernel-driver flow still looks the same.

Thanks
Anjali

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Willem de Bruijn]

Singhai, Anjali wrote:
> 
> In reference to this patch series
> https://lore.kernel.org/netdev/20240510030435.120935-1-kuba@kernel.org/#t
> 
> Thanks a lot  for the PSP crypto enabling patches in the kernel.
> Some points that we noticed that could use some enhancements/fixes
> 
> 1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
> Even the Mellanox driver does not implement this set_config ndo_op. 
>  
> 2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
> In our device we have 2 options, 
>              1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
>               As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
>              2. Passing key in the descriptor.
>               For us we need both these options, so please allow for enhancements.
> 
> 3. About the PSP and UDP header addition, why is the driver doing it? I guess it's because the SW equivalent for PSP support in the kernel does not exist and just an offload for the device. Again in this case the assumption is either the driver does it or the device will do it.
> Hope that is irrelevant for the stack. In our case most likely it will be the device doing it.
> 
> 4. Why is the driver adding the PSP trailer? Hoping this is between the driver and the device, in our case it's the device that will add the trailer.

This does not adhere to the spec:

"An option must be provided that enables upper-level software to send packets that are
pre-formatted to include the headers required for PSP encapsulation. In this case, the
NIC will modify the contents of the headers appropriately, apply
encryption/authentication, and add the PSP trailer to the packet."

https://raw.githubusercontent.com/google/psp/main/doc/PSP_Arch_Spec.pdf

  
> 5. Five way handshake, can this be optimized to 3 way?
> Here is what we think is happening right now at the IKE level interaction for the two ends of the session, as PSP protocol does not define it but based on the implementation this is what we gathered.
>   Looks like its 5 way handshake that is happening with the session partner.
>    For example two sides are called Tx session partner and RX session partner, just because TX initiates the session creation, of course it's a full duplex session.
>              1. TX session partner sends over sideband tls channel to the Rx session partner what all versions the TX can do based on caps learnt from the device driver.
>              2. The Rx session partner replies with what common versions it can do back to TX session partner based on dev caps it learnt from the device.

In practice in reasonably homogeneous hyperscale environments, this
step can be skipped. To be in spec devices must support both 128b
and 256b AES-GCM (and nothing else). It is an organizational choice
which to deploy.

>              3. Tx session partner tells the driver to generate a spi an session key for the rx side by specifying the version it wants to use for this session. And then sends to Rx session partner the spi and session key and version selected etc.
>              4. The Rx session partner at its end then talks to its driver to generate a spi an session key for the rx side by specifying the version sent from TX session partner. And then sends to Tx session partner the spi and session key and version selected etc. The RX session partner also programs the session key and spi it received from TX session partner in HW in its Tx SADB (psp_assoc_add)
>             5. Tx session partner programs the received SPI and session key in its HW in its Tx SADB (psp_assoc_add). When done it sends an ACK back to Rx session partner that the session is ready.
>  
> Should this be optimized to a  3 way handshake to allow for faster session setup? 1 and 3 and 2 and 4 could be combined in an intelligent way. Again may be there is already an optimized way to do 3 way handshake here and the kernel-driver flow still looks the same.
> 
> Thanks
> Anjali
> 

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Willem de Bruijn]

> > 3. About the PSP and UDP header addition, why is the driver doing it? I guess it's because the SW equivalent for PSP support in the kernel does not exist and just an offload for the device. Again in this case the assumption is either the driver does it or the device will do it.
> > Hope that is irrelevant for the stack. In our case most likely it will be the device doing it.
> > 
> > 4. Why is the driver adding the PSP trailer? Hoping this is between the driver and the device, in our case it's the device that will add the trailer.
> 
> This does not adhere to the spec:
> 
> "An option must be provided that enables upper-level software to send packets that are
> pre-formatted to include the headers required for PSP encapsulation. In this case, the
> NIC will modify the contents of the headers appropriately, apply
> encryption/authentication, and add the PSP trailer to the packet."
> 
> https://raw.githubusercontent.com/google/psp/main/doc/PSP_Arch_Spec.pdf

I responded to the wrong statement. This is in response to point 3.

In general, PSP can work in tunnel and transport mode. In transport
mode, it is here assumed to be not transparent, but under control of
the operating system. That inserts the outer encapsulation headers and
prepares all fields as it sees fit. E.g., using the inner 4-tuple as
entropy for the outer UDP source port, and selecting the right SPI.

RE: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Singhai, Anjali]

> > 1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
> > Even the Mellanox driver does not implement this set_config ndo_op. 
>  >
Can you or Kuba comment on this?

> > 2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
> > In our device we have 2 options, 
> >             1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
> >             As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
> >            2. Passing key in the descriptor.
> >             For us we need both these options, so please allow for enhancements.
> >
Can you or Kuba comment on this? This is critical, also in the fast path, skb needs to carry the SA_index/handle (like the tls case) instead of the key or both so that either method can be used by the device driver/device.

> > 3. About the PSP and UDP header addition, why is the driver doing it? I guess it's because the SW equivalent for PSP support in the kernel does not exist and just an offload for the device. Again in this case the assumption is either the driver does it or the device will do it.
> > Hope that is irrelevant for the stack. In our case most likely it will be the device doing it.
> > 

> This does not adhere to the spec:

> "An option must be provided that enables upper-level software to send packets that are pre-formatted to include the headers required for PSP encapsulation. In this case, the NIC will modify the contents of the headers appropriately, apply encryption/authentication, and add the PSP trailer to the packet."

> https://raw.githubusercontent.com/google/psp/main/doc/PSP_Arch_Spec.pdf

Yes I guess this is just transport mode and not tunnel mode in which the device adds the header. So agreed, it should be upper-level software that should add this, but again not the driver as this is protocol specific and not device specific and all devices using a  common code would be the right thing.

> > 4. Why is the driver adding the PSP trailer? Hoping this is between the driver and the device, in our case it's the device that will add the trailer.

This for sure is by device or driver, ideally the device. Please comment.

A few more opens that we noticed later

1. Key rotation should be triggered from the device as a master key in the device can be shared in a virtualized environment by many interfaces which would mean only the device can decide based on the following when to trigger the key rotation 
	1. Time out cannot be independent for each IKE but at a device level configuration.
	2.  SPI roll over, the SPI domain is again shared with multiple Interfaces that share the master key and only the device can trigger the rotation when this happens.

Apart from this, in a virtualized environment, a trigger from top (IKE down to device) to rotate the master key can cause unnecessary side effects to other interfaces that can be considered malicious.

Anjali  	
 

RE: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Willem de Bruijn]

> > > 4. Why is the driver adding the PSP trailer? Hoping this is between the driver and the device, in our case it's the device that will add the trailer.
> 
> This for sure is by device or driver, ideally the device. Please comment.

Whether it is driver or device is a device specific implementation detail?

> A few more opens that we noticed later
> 
> 1. Key rotation should be triggered from the device as a master key in the device can be shared in a virtualized environment by many interfaces which would mean only the device can decide based on the following when to trigger the key rotation 
> 	1. Time out cannot be independent for each IKE but at a device level configuration.
> 	2.  SPI roll over, the SPI domain is again shared with multiple Interfaces that share the master key and only the device can trigger the rotation when this happens.
> 
> Apart from this, in a virtualized environment, a trigger from top (IKE down to device) to rotate the master key can cause unnecessary side effects to other interfaces that can be considered malicious.

It is possible to designate a privileged interface that is allowed to
request a key rotation. This should be supported.

For IDPF whether a driver is authorized to request a rotation can be
part of capability negotiation.

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Jakub Kicinski]

On Thu, 20 Jun 2024 21:32:14 +0000 Singhai, Anjali wrote:
> > > 1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
> > > Even the Mellanox driver does not implement this set_config ndo_op. 
> >  >  
> Can you or Kuba comment on this?

For now the only action the driver can perform is to disable PSP Rx
handling:

https://github.com/kuba-moo/linux/blob/psp/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c#L18

> > > 2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
> > > In our device we have 2 options, 
> > >             1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
> > >             As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
> > >            2. Passing key in the descriptor.
> > >             For us we need both these options, so please allow for enhancements.
> > >  
> Can you or Kuba comment on this? This is critical, also in the fast path, skb needs to carry the SA_index/handle (like the tls case) instead of the key or both so that either method can be used by the device driver/device.

The ID should go into the driver state of the association, specify how
much space you need by setting this:
https://github.com/kuba-moo/linux/blob/psp/include/net/psp/types.h#L110C6-L110C19
Then you can access it via psp_assoc_drv_data()

AFAICT Willem answered all the other points.

RE: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Singhai, Anjali]

On Thu, 20 Jun 2024 21:32:14 +0000 Singhai, Anjali wrote:
> > > > 1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
> > > > Even the Mellanox driver does not implement this set_config ndo_op. 
> > >  >
> > Can you or Kuba comment on this?

> For now the only action the driver can perform is to disable PSP Rx
> handling:

> https://github.com/kuba-moo/linux/blob/psp/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c#L18

Okay, can move into kernel as the device already fills its capabilities, but it doesn't matter for now, will do the same.

> > > > > 2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
> > > > In our device we have 2 options, 
> > > >             1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
> > > >             As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
> > > >            2. Passing key in the descriptor.
> > > >             For us we need both these options, so please allow for enhancements.
> > > >  
> > Can you or Kuba comment on this? This is critical, also in the fast path, skb needs to carry the SA_index/handle (like the tls case) instead of the key or both so that either method can be used by the device driver/device.

> The ID should go into the driver state of the association, specify how much space you need by setting this:
https://github.com/kuba-moo/linux/blob/psp/include/net/psp/types.h#L110C6-L110C19
Then you can access it via psp_assoc_drv_data()

Is this already posted as new patch series on netdev mailing list, maybe I missed it. This works for us, will follow the same.

> AFAICT Willem answered all the other points.

We still have two more that are not addressed
1. Master key rotation, this needs to be not just from top but also from device side because of the master key sharing in some cases and a common SPI space that can roll over. So need to have an event mechanism from device driver back to kernel for key rotation. Please add this.

2. Header additions in the driver, for now we can do this, but ideally it should move to the stack, so that it can be common for all devices.

Anjali

Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

[Jakub Kicinski]

On Tue, 25 Jun 2024 22:05:13 +0000 Singhai, Anjali wrote:
> 1. Master key rotation, this needs to be not just from top but also
> from device side because of the master key sharing in some cases and
> a common SPI space that can roll over. So need to have an event
> mechanism from device driver back to kernel for key rotation. Please
> add this.

Sorry I don't need it myself, you'll need to add that once my patches
are merged.

> 2. Header additions in the driver, for now we can do this, but
> ideally it should move to the stack, so that it can be common for all
> devices.

That's fair, I only have mlx5 implementation so I wasn't sure how much
of it is reusable. Once we have 2 drivers we can extrapolate?