netdev - Re: [PATCH net-next v3 7/7] tls: rx: do not use the standard strparser

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: <89086da6-b559-f6c0-d73a-6c73ff74dff5@gmail.com>
Date:   Sun, 12 Mar 2023 19:59:57 +0200
From:   Tariq Toukan <ttoukan.linux@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, borisp@...dia.com, john.fastabend@...il.com,
        maximmi@...dia.com, tariqt@...dia.com, vfedorenko@...ek.ru,
        Gal Pressman <gal@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH net-next v3 7/7] tls: rx: do not use the standard
 strparser



On 09/03/2023 19:54, Jakub Kicinski wrote:
> On Thu, 9 Mar 2023 17:15:26 +0200 Tariq Toukan wrote:
>> A few fixes were introduced for this patch, but it seems to still cause
>> issues.
>>
>> I'm running simple client/server test with wrk and nginx and TLS RX
>> device offload on.
>> It fails with TlsDecryptError on the client side for the large file
>> (256000b), while succeeding for the small one (10000b). See repro
>> details below.
>>
>> I narrowed the issue down to this offending patch, by applying a few
>> reverts (had to solve trivial conflicts):
> 
> What's the sequence of records in terms of being offloaded vs fall back?
> Could you whip up a simple ring buffer to see if previous records were
> offloaded and what the skb geometries where?

Interesting. All records go through the sw fallback.

Command:
$ wrk_openssl_3_0_0 -b2.2.2.2 -t1 -c1 -d2 --timeout 5s 
https://2.2.2.3:20443/256000b.img

Debug code:
@@ -1712,8 +1723,13 @@ static int tls_rx_one_record(struct sock *sk, 
struct msghdr *msg,
         int err;

         err = tls_decrypt_device(sk, msg, tls_ctx, darg);
-       if (!err)
+       if (!err) {
                 err = tls_decrypt_sw(sk, tls_ctx, msg, darg);
+               printk("sk: %p, tls_decrypt_sw, err = %d\n", sk, err);
+       } else {
+               printk("sk: %p, tls_decrypt_device, err = %d\n", sk, err);
+       }
+       skb_dump(KERN_ERR, darg->skb, false);
         if (err < 0)
                 return err;

dmesg output including skb geometries is attached.

# cat /proc/net/tls_stat
TlsCurrTxSw                             0
TlsCurrRxSw                             0
TlsCurrTxDevice                         0
TlsCurrRxDevice                         0
TlsTxSw                                 0
TlsRxSw                                 0
TlsTxDevice                             12
TlsRxDevice                             12
TlsDecryptError                         22
TlsRxDeviceResync                       12
TlsDecryptRetry                         0
TlsRxNoPadViolation                     0

We start from out-of-sync state, so we expect to go through the sw 
fallback in the beginning.
I do see that the resync succeeds and device-offloaded packets start to 
flow, example ethtool output (1 sec interval):

       rx_tls_decrypted_packets: 356
         rx_tls_decrypted_bytes: 517,160 Bps          = 4.13 Mbps
          rx_tls_resync_req_pkt: 6
        rx_tls_resync_req_start: 6
          rx_tls_resync_req_end: 6
           rx_tls_resync_res_ok: 6
                     rx_tls_ctx: 6
                     rx_tls_del: 6
View attachment "dmesg_tls_rx_decrypt_err.txt" of type "text/plain" (131169 bytes)