Date:   Thu, 2 Feb 2023 18:32:08 +0100
From:   Hannes Reinecke <hare@...e.de>
To:     Xin Long <lucien.xin@...il.com>,
        Marcel Holtmann <marcel@...tmann.org>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Chuck Lever <chuck.lever@...cle.com>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        hare@...e.com, dhowells@...hat.com, kolga@...app.com,
        jmeneghi@...hat.com, bcodding@...hat.com, jlayton@...hat.com
Subject: Re: [PATCH v2 2/3] net/handshake: Add support for PF_HANDSHAKE

On 2/2/23 18:13, Xin Long wrote:
> On Tue, Jan 31, 2023 at 9:24 AM Marcel Holtmann <marcel@...tmann.org> wrote:
>>
>> I know, utilizing existing TLS libraries is a pain if you don’t do
>> exactly what they had in mind. I started looking at QUIC a while
>> back and quickly realized, I have to start looking at TLS 1.3 first.
>>
>> My past experience with GnuTLS and OpenSSL have been bad and that is
>> why iwd (our WiFi daemon) has its own TLS implementation utilizing
>> AF_ALG and keyctl.
> Hi Marcel,
> 
> I'm no expert on TLS, but I'm a supporter of in-kernel TLS 1.3 Handshake
> implementation :). When working on implementing in-kernel QUIC protocol,
> the code looks a lot simpler with the pure in-kernel TLS 1.3 Handshake APIs
> than the upcall method, and I believe the NFS over TLS 1.3 in kernel will
> feel the same.
> 
>>
>> While that might have been true in the past and with TLS 1.2 and earlier,
>> I am not sure that is all true today.
>>
>> Lets assume we start with TLS 1.3 and don’t have backwards compatibility
>> with TLS 1.2 and earlier. And for now we don’t worry about Middleboxes
>> compatibility mode since you don’t have to for all the modern protocols
>> that utilize just the TLS 1.3 handshake like QUIC.
>>
>> Now the key derivation is just choosing 1 out of 5 ciphers and using
>> its associated hash algorithm to derive the keys. This is all present
>> functionality in the kernel and so well tested that it doesn’t worry
>> me at all. We also have a separate RFC with just sample data so you
>> can check your derivation functionality. Especially if you check it
>> against AEAD encrypted sample data, any mistake is fatal.
>>
>> The shared key portion is just ECDHE or DHE and you either end up with
>> x25519 or secp256r1 and both are in the kernel. Bluetooth has been
>> using secp256r1 inside the kernel for many years now. We all know how
>> to handle and verify public keys from secp256r1 and neat part is that
>> it would be also offloaded to hardware if needed. So the private key
>> doesn’t need to stay even in kernel memory.
>>
>> So dealing with generating your key material for your cipher is really
>> simple and similar things have been done for Bluetooth for a long
>> time now. And it looks like NVMe is also utilizing KPP as of today.
>>
>> The tricky part is the authentication portion of TLS utilizing
>> certificates. That part is complicated, but then again, we already
>> decided the kernel needs to handle certificates for various places
>> and you have to assume that it is fairly secure.
>>
>> Now, you need to secure the handshake protocol like any other protocol
>> and the only difference is that it will lead to key material and
>> does authentication with certificates. All of it, the kernel already
>> does in one form or another.
>>
>> The TLS 1.3 spec is also really nicely written and explicit in
>> error behavior in case of attempts to attack the protocol. While
>> implementing my TLS 1.3 only prototype I have been positively
>> surprised on how clean it is. I personally think they went over
>> board with the key verification, but so be it.
>>
>> Once I have cleaned up my TLS 1.3 prototype, I am happy to take
>> a stab at a kernel version.
>>
> I'm glad to hear that you're planning to add this in kernel space, and I
> agree that there won't be a lot of things to do in kernel due to the kernel
> crypto APIs. There is also a TLS 1.3 Handshake prototype I worked on and
> based on the torvalds/linux kernel code. In case of any duplicate work
> when you're doing it, I just share my code here:
> 
>    https://github.com/lxin/tls_hs/blob/master/crypto/tls_hs.c
> 
> and the TLS_HS APIs docs for QUIC and NFS use are here:
> 
>    https://github.com/lxin/tls_hs
> 
> Hopefully, it will help.
> 
> Besides, There were some security concerns from others for which I didn't
> continue:
> 
>    https://github.com/lxin/tls_hs#the-security-issues
> 
> It will be great if we can have your opinions about it.
> 
> Thanks.
Wow.
That certainly looks good; I'll give it a go and try to integrate it 
with my NVMe-TLS stuff.
Thanks a lot for this!

As for your security concerns:
- Certification management _is_ complete, but we have the kernel keyring
   to help us out. The cert can be looked up in the keyring, and one can
   even call request_key() to get userspace to supply us with one.
   With that we can just use the kernel keyring to lookup certificates
   and don't worry about key management itself.
- TLS 1.3 is nailed down, and won't inherit more encryption algorithms.
   So nothing to worry about
- Corner cases ... yes, the code needs to be validated.
- RSA rewrite ... can't we disallow RSA for in-kernel usage?
- mpi code not constant-time safe; might be. But that goes for all
   in-kernel users, and I'm not sure if _we_ have to worry about that.
- DH and ECDH potentially broken: not anymore; got fixed up to
   be FIPS compliant
- Take advantage of decades of work in userspace: that's _precisely_
   why we're sticking with TLS 1.3. Most of the backwards-compat stuff
   got removed, so we _don't_ have to worry about that.

So it's at least worth a shot, to see where we end up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@...e.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Powered by blists - more mailing lists