netdev - Re: [PATCH net-next] tls: don't reset prot->aad_size and prot->tail_size for TLS

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]

Date: Sun, 22 Oct 2023 17:07:27 +0300
From: Tariq Toukan <ttoukan.linux@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, Sabrina Dubroca <sd@...asysnail.net>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
 Boris Pismenny <borisp@...dia.com>, Eric Dumazet <edumazet@...gle.com>,
 John Fastabend <john.fastabend@...il.com>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, tariq Toukan <tariqt@...dia.com>,
 Ran Rozenstein <ranro@...dia.com>
Subject: Re: [PATCH net-next] tls: don't reset prot->aad_size and
 prot->tail_size for TLS_HW

On 22/10/2023 15:02, Tariq Toukan wrote:
> 
> 
> On 21/10/2023 3:14, Jakub Kicinski wrote:
>> On Fri, 20 Oct 2023 16:00:55 +0200 Sabrina Dubroca wrote:
>>> Prior to commit 1a074f7618e8 ("tls: also use init_prot_info in
>>> tls_set_device_offload"), setting TLS_HW on TX didn't touch
>>> prot->aad_size and prot->tail_size. They are set to 0 during context
>>> allocation (tls_prot_info is embedded in tls_context, kzalloc'd by
>>> tls_ctx_create).
>>>
>>> When the RX key is configured, tls_set_sw_offload is called (for both
>>> TLS_SW and TLS_HW). If the TX key is configured in TLS_HW mode after
>>> the RX key has been installed, init_prot_info will now overwrite the
>>> correct values of aad_size and tail_size, breaking SW decryption and
>>> causing -EBADMSG errors to be returned to userspace.
>>>
>>> Since TLS_HW doesn't use aad_size and tail_size at all (for TLS1.2,
>>> tail_size is always 0, and aad_size is equal to TLS_HEADER_SIZE +
>>> rec_seq_size), we can simply drop this hunk.
>>>
>>> Fixes: 1a074f7618e8 ("tls: also use init_prot_info in 
>>> tls_set_device_offload")
>>> Signed-off-by: Sabrina Dubroca <sd@...asysnail.net>
>>> ---
>>> Tariq, does that solve the problem you reported in
>>> https://lore.kernel.org/netdev/3ace1e75-c0a5-4473-848d-91f9ac0a8f9c@gmail.com/
>>> ?
>>
>> In case Tariq replies before Monday and DaveM wants to take it, LGTM:
>>
>> Acked-by: Jakub Kicinski <kuba@...nel.org>
> 
> Hi,
> 
> We're testing this fix and will reply ASAP.
> 

Test passes:
Tested-by: Ran Rozenstein <ranro@...dia.com>

We suspect that it was not the only degradation introduced by this series.
We are going to run more comprehensive tests with the recent series and 
this new fix. Of course we'll update about any remaining issues.

Tariq