| 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
| ||
|
Message-ID: <686aa894a8b6e_3ad0f32946d@willemb.c.googlers.com.notmuch>
Date: Sun, 06 Jul 2025 12:47:16 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Daniel Zahka <daniel.zahka@...il.com>,
Donald Hunter <donald.hunter@...il.com>,
Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>,
Jonathan Corbet <corbet@....net>,
Andrew Lunn <andrew+netdev@...n.ch>
Cc: Saeed Mahameed <saeedm@...dia.com>,
Leon Romanovsky <leon@...nel.org>,
Tariq Toukan <tariqt@...dia.com>,
Boris Pismenny <borisp@...dia.com>,
Kuniyuki Iwashima <kuniyu@...gle.com>,
Willem de Bruijn <willemb@...gle.com>,
David Ahern <dsahern@...nel.org>,
Neal Cardwell <ncardwell@...gle.com>,
Patrisious Haddad <phaddad@...dia.com>,
Raed Salem <raeds@...dia.com>,
Jianbo Liu <jianbol@...dia.com>,
Dragos Tatulea <dtatulea@...dia.com>,
Rahul Rameshbabu <rrameshbabu@...dia.com>,
Stanislav Fomichev <sdf@...ichev.me>,
Toke Høiland-Jørgensen <toke@...hat.com>,
Alexander Lobakin <aleksander.lobakin@...el.com>,
Jacob Keller <jacob.e.keller@...el.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH v3 08/19] net: psp: add socket security association code
Daniel Zahka wrote:
> From: Jakub Kicinski <kuba@...nel.org>
>
> Add the ability to install PSP Rx and Tx crypto keys on TCP
> connections. Netlink ops are provided for both operations.
> Rx side combines allocating a new Rx key and installing it
> on the socket. Theoretically these are separate actions,
> but in practice they will always be used one after the
> other. We can add distinct "alloc" and "install" ops later.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> Co-developed-by: Daniel Zahka <daniel.zahka@...il.com>
> Signed-off-by: Daniel Zahka <daniel.zahka@...il.com>
> ---
>
> Notes:
> v3:
> - lift pse/pas comparison code into new function:
> psp_pse_matches_pas().
> - explicitly mark rcu critical section psp_reply_set_decrypted()
> - use rcu_dereference_proteced() instead of rcu_read_lock() in
> psp_sk_assoc_free() and psp_twsk_assoc_free()
> - rename psp_is_nondata() to psp_is_allowed_nondata()
> - check for sk_is_inet() before casting to inet_twsk()
> - psp_reply_set_decrypted() should not call psp_sk_assoc()
> - lift common code into psp_sk_get_assoc_rcu()
> - Fix exports with EXPORT_IPV6_MOD_GPL()
>
> v2:
> - add pas->dev_id == pse->dev_id to policy checks
> - __psp_sk_rx_policy_check() now allows pure ACKs, FINs, and RSTs to
> be non-psp authenticated before "PSP Full" state.
> - assign tw_validate_skb funtion during psp_twsk_init()
> - psp_skb_get_rcu() also checks if sk is a tcp timewait sock when
> looking for psp assocs.
> - scan ofo queue non-psp data during psp_sock_recv_queue_check()
> - add tcp_write_collapse_fence() to psp_sock_assoc_set_tx()
> v1:
> - https://lore.kernel.org/netdev/20240510030435.120935-7-kuba@kernel.org/
>
> Documentation/netlink/specs/psp.yaml | 71 +++++++
> include/net/psp/functions.h | 113 +++++++++--
> include/net/psp/types.h | 58 ++++++
> include/uapi/linux/psp.h | 21 ++
> net/psp/Kconfig | 1 +
> net/psp/Makefile | 2 +-
> net/psp/psp-nl-gen.c | 39 ++++
> net/psp/psp-nl-gen.h | 7 +
> net/psp/psp.h | 22 +++
> net/psp/psp_main.c | 11 +-
> net/psp/psp_nl.c | 244 +++++++++++++++++++++++
> net/psp/psp_sock.c | 276 +++++++++++++++++++++++++++
> 12 files changed, 852 insertions(+), 13 deletions(-)
> create mode 100644 net/psp/psp_sock.c
>
> diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
> index 054cc02b65ad..57b24cd6f3f1 100644
> --- a/Documentation/netlink/specs/psp.yaml
> +++ b/Documentation/netlink/specs/psp.yaml
> @@ -38,6 +38,44 @@ attribute-sets:
> type: u32
> enum: version
> enum-as-flags: true
> + -
> + name: assoc
> + attributes:
> + -
> + name: dev-id
> + doc: PSP device ID.
> + type: u32
> + checks:
> + min: 1
> + -
> + name: version
> + doc: |
> + PSP versions (AEAD and protocol version) used by this association,
> + dictates the size of the key.
> + type: u32
> + enum: version
> + -
> + name: rx-key
> + type: nest
> + nested-attributes: keys
> + -
> + name: tx-key
> + type: nest
> + nested-attributes: keys
> + -
> + name: sock-fd
> + doc: Sockets which should be bound to the association immediately.
> + type: u32
> + -
> + name: keys
> + attributes:
> + -
> + name: key
> + type: binary
> + -
> + name: spi
> + doc: Security Parameters Index (SPI) of the association.
> + type: u32
>
> operations:
> list:
> @@ -107,6 +145,39 @@ operations:
> notify: key-rotate
> mcgrp: use
>
> + -
> + name: rx-assoc
> + doc: Allocate a new Rx key + SPI pair, associate it with a socket.
> + attribute-set: assoc
> + do:
> + request:
> + attributes:
> + - dev-id
> + - version
> + - sock-fd
> + reply:
> + attributes:
> + - dev-id
> + - version
Why return the same values as passed in the request?
> + - rx-key
> + pre: psp-assoc-device-get-locked
> + post: psp-device-unlock
> + -
> + name: tx-assoc
> + doc: Add a PSP Tx association.
> + attribute-set: assoc
> + do:
> + request:
> + attributes:
> + - dev-id
> + - version
Version must be the same for rx and tx alloc. It is already set for
rx, so no need to pass explicitly. Just adds the need to for a sanity
check in the handler.
> + - tx-key
> + - sock-fd
> + reply:
> + attributes: []
> + pre: psp-assoc-device-get-locked
> + post: psp-device-unlock
> +
> mcast-groups:
> list:
> -
> +static inline bool
> +psp_pse_matches_pas(struct psp_skb_ext *pse, struct psp_assoc *pas)
> +{
> + return pse && pas->rx.spi == pse->spi &&
> + pas->generation == pse->generation &&
> + pas->version == pse->version &&
> + pas->dev_id == pse->dev_id;
Since struct psp_skb_ext is 64 bits, could almost implement this as
a single 64-bit match. The only outlier is spi, which is rx.spi.
Which also indicates that this is psp_pse_matches_pas_rx.
Still, the other three fields could be a single 32b comparison.
Not sure if worth the effort (using a union or cast, say).
> +struct psp_assoc {
> + struct psp_dev *psd;
> +
> + u16 dev_id;
> + u8 generation;
> + u8 version;
> + u8 key_sz;
implied by version?
> + u8 peer_tx;
> +
> + u32 upgrade_seq;
> +
> + struct psp_key_parsed tx;
> + struct psp_key_parsed rx;
> +
> + refcount_t refcnt;
> + struct rcu_head rcu;
> + struct work_struct work;
> + struct list_head assocs_list;
> +
> + u8 drv_data[] __aligned(8);
> +};
> +
> +int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
> + struct sk_buff *skb, struct genl_info *info)
> +{
> + struct socket *socket;
> + struct psp_dev *psd;
> + struct nlattr *id;
> + struct sock *sk;
> + int fd, err;
> +
> + if (GENL_REQ_ATTR_CHECK(info, PSP_A_ASSOC_SOCK_FD))
> + return -EINVAL;
> +
> + fd = nla_get_u32(info->attrs[PSP_A_ASSOC_SOCK_FD]);
> + socket = sockfd_lookup(fd, &err);
> + if (!socket)
> + return err;
> +
> + sk = socket->sk;
> + if (sk->sk_family != AF_INET && sk->sk_family != AF_INET6) {
> + NL_SET_ERR_MSG_ATTR(info->extack,
> + info->attrs[PSP_A_ASSOC_SOCK_FD],
> + "Unsupported socket family");
Should this also check sk_type?
> +
> +struct psp_dev *psd_get_for_sock(struct sock *sk)
Same as other support functions, consider spelling it out fully as
psp_dev_get_for_sock.
> +{
> + struct dst_entry *dst;
> + struct psp_dev *psd;
> +
> + dst = sk_dst_get(sk);
> + if (!dst)
> + return NULL;
> +
> + rcu_read_lock();
> + psd = rcu_dereference(dst->dev->psp_dev);
> + if (psd && !psp_dev_tryget(psd))
> + psd = NULL;
> + rcu_read_unlock();
> +
> + dst_release(dst);
> +
> + return psd;
> +}
> +void psp_reply_set_decrypted(struct sk_buff *skb)
> +{
> + struct psp_assoc *pas;
> +
> + rcu_read_lock();
> + pas = psp_sk_get_assoc_rcu(skb->sk);
> + if (pas && pas->tx.spi)
> + skb->decrypted = 1;
> + rcu_read_unlock();
> +}
> +EXPORT_IPV6_MOD_GPL(psp_reply_set_decrypted)
semicolon
> --
> 2.47.1
>
Powered by blists - more mailing lists