netdev - Re: [PATCH net-next v4 1/6] tls: block decryption when a rekey is pending

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: <fd808dab-1fbe-4530-970a-2b02fb27fc1e@microchip.com>
Date: Thu, 5 Dec 2024 12:30:49 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <sd@...asysnail.net>
CC: <vfedorenko@...ek.ru>, <fkrenzel@...hat.com>, <kuba@...nel.org>,
	<kuniyu@...zon.com>, <apoorvko@...zon.com>, <borisp@...dia.com>,
	<john.fastabend@...il.com>, <shuah@...nel.org>,
	<linux-kselftest@...r.kernel.org>, <gal@...dia.com>, <marcel@...tmann.org>,
	<horms@...nel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v4 1/6] tls: block decryption when a rekey is
 pending

Hi,

On 14/11/24 9:20 pm, Sabrina Dubroca wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> When a TLS handshake record carrying a KeyUpdate message is received,
> all subsequent records will be encrypted with a new key. We need to
> stop decrypting incoming records with the old key, and wait until
> userspace provides a new key.
> 
> Make a note of this in the RX context just after decrypting that
> record, and stop recvmsg/splice calls with EKEYEXPIRED until the new
> key is available.
> 
> key_update_pending can't be combined with the existing bitfield,
> because we will read it locklessly in ->poll.
> 
> v3:
>   - move key_update_pending check into tls_rx_rec_wait (Jakub)
>   - TLS_RECORD_TYPE_HANDSHAKE was added to include/net/tls_prot.h by
>     the tls handshake series, drop that from this patch
>   - move key_update_pending into an existing hole
> 
> v4:
>   - flip TLS_RECORD_TYPE_HANDSHAKE test and use likely() (Jakub)
>   - pass ctx rather than sk to tls_check_pending_rekey (Jakub)
>   - use WRITE_ONCE to set key_update_pending to pair with ->poll's
>     lockless read
> 
> Signed-off-by: Sabrina Dubroca <sd@...asysnail.net>
> ---
>   include/net/tls.h |  3 +++
>   net/tls/tls_sw.c  | 35 +++++++++++++++++++++++++++++++++++
>   2 files changed, 38 insertions(+)
> 
> diff --git a/include/net/tls.h b/include/net/tls.h
> index 3a33924db2bc..870e4421c599 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -59,6 +59,8 @@ struct tls_rec;
> 
>   #define TLS_CRYPTO_INFO_READY(info)    ((info)->cipher_type)
> 
> +#define TLS_HANDSHAKE_KEYUPDATE                24      /* rfc8446 B.3: Key update */
> +
>   #define TLS_AAD_SPACE_SIZE             13
> 
>   #define TLS_MAX_IV_SIZE                        16
> @@ -130,6 +132,7 @@ struct tls_sw_context_rx {
>          u8 async_capable:1;
>          u8 zc_capable:1;
>          u8 reader_contended:1;
> +       bool key_update_pending;
> 
>          struct tls_strparser strp;
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index bbf26cc4f6ee..db98710c4810 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1314,6 +1314,10 @@ tls_rx_rec_wait(struct sock *sk, struct sk_psock *psock, bool nonblock,
>          int ret = 0;
>          long timeo;
> 
> +       /* a rekey is pending, let userspace deal with it */
> +       if (unlikely(ctx->key_update_pending))
> +               return -EKEYEXPIRED;
> +
>          timeo = sock_rcvtimeo(sk, nonblock);
> 
>          while (!tls_strp_msg_ready(ctx)) {
> @@ -1720,6 +1724,32 @@ tls_decrypt_device(struct sock *sk, struct msghdr *msg,
>          return 1;
>   }
> 
> +static int tls_check_pending_rekey(struct tls_context *ctx, struct sk_buff *skb)
> +{
> +       const struct tls_msg *tlm = tls_msg(skb);
> +       const struct strp_msg *rxm = strp_msg(skb);
Missing reverse xmas tree format.
> +       char hs_type;
> +       int err;
> +
> +       if (likely(tlm->control != TLS_RECORD_TYPE_HANDSHAKE))
> +               return 0;
> +
> +       if (rxm->full_len < 1)
> +               return -EINVAL;
> +
> +       err = skb_copy_bits(skb, rxm->offset, &hs_type, 1);
> +       if (err < 0)
> +               return err;
> +
> +       if (hs_type == TLS_HANDSHAKE_KEYUPDATE) {
> +               struct tls_sw_context_rx *rx_ctx = ctx->priv_ctx_rx;
> +
> +               WRITE_ONCE(rx_ctx->key_update_pending, true);
> +       }
> +
> +       return 0;
> +}
> +
>   static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
>                               struct tls_decrypt_arg *darg)
>   {
> @@ -1739,6 +1769,10 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
>          rxm->full_len -= prot->overhead_size;
>          tls_advance_record_sn(sk, prot, &tls_ctx->rx);
> 
> +       err = tls_check_pending_rekey(tls_ctx, darg->skb);
I think you can directly return from here.

Best regards,
Parthiban V
> +       if (err < 0)
> +               return err;
> +
>          return 0;
>   }
> 
> @@ -2719,6 +2753,7 @@ int tls_set_sw_offload(struct sock *sk, int tx)
>                  crypto_info = &ctx->crypto_recv.info;
>                  cctx = &ctx->rx;
>                  aead = &sw_ctx_rx->aead_recv;
> +               sw_ctx_rx->key_update_pending = false;
>          }
> 
>          cipher_desc = get_cipher_desc(crypto_info->cipher_type);
> --
> 2.47.0
> 
>