* pre-release of v7 @ 2023-03-09 14:41 Chuck Lever III 2023-03-09 18:29 ` Hannes Reinecke 0 siblings, 1 reply; 4+ messages in thread From: Chuck Lever III @ 2023-03-09 14:41 UTC (permalink / raw) To: kernel-tls-handshake Hi- I'm now trying to take a step or two towards a more typical development process for the kernel patches and ktls-utils, as we replace the old upcall mechanism with netlink. This is in preparation for a release of ktls-utils-0.8, once netdev is close to happy with the kernel part. I have pushed pre-release branches of v7 so you can comment early, test it, or send me patches /before/ I post "Another crack at a handshake upcall mechanism" v7 on netdev@. What a concept, eh? Kernel part: The upcall-v7 branch in https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git User space part: The netlink-v7 branch in https://github.com/oracle/ktls-utils The one thing I'm waiting on now is for Jakub to deal properly with the license and copyright notices in the YAML specs and generated C source code. As an old SunRPC hand, I feel that the code generator needs to copy the spec's license and copyright notices verbatim to the generated C code. Jakub is not certain this is required; his code generator adds a license notice that is sometimes not compatible with the spec's license. We are planning to loop the Linux Foundation in to try to extract some legal advice. -- Chuck Lever ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: pre-release of v7 2023-03-09 14:41 pre-release of v7 Chuck Lever III @ 2023-03-09 18:29 ` Hannes Reinecke 2023-03-09 20:20 ` Chuck Lever III 0 siblings, 1 reply; 4+ messages in thread From: Hannes Reinecke @ 2023-03-09 18:29 UTC (permalink / raw) To: Chuck Lever III, kernel-tls-handshake On 3/9/23 15:41, Chuck Lever III wrote: > Hi- > > I'm now trying to take a step or two towards a more typical development > process for the kernel patches and ktls-utils, as we replace the old > upcall mechanism with netlink. This is in preparation for a release of > ktls-utils-0.8, once netdev is close to happy with the kernel part. > > I have pushed pre-release branches of v7 so you can comment early, test > it, or send me patches /before/ I post "Another crack at a handshake > upcall mechanism" v7 on netdev@. What a concept, eh? > <applause> > > Kernel part: The upcall-v7 branch in > > https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git > Hmm. Guess I'll branch off that, and push my patches on top of that. They probably have to go via a different maintainer (read: hch), so keeping them separately might be useful. Or having them on your tree, too? Dunno. > > User space part: The netlink-v7 branch in > > https://github.com/oracle/ktls-utils > Ah. There was the 'before' part of your mail... I've pushed a patchset for the 'netlink' branch, containing fixes to get PSK running. Will be rebasing them to v7. > > The one thing I'm waiting on now is for Jakub to deal properly with > the license and copyright notices in the YAML specs and generated C > source code. As an old SunRPC hand, I feel that the code generator > needs to copy the spec's license and copyright notices verbatim to > the generated C code. Jakub is not certain this is required; his code > generator adds a license notice that is sometimes not compatible with > the spec's license. > Hmm. Not that concerned; my company has not a strong policy here :-) But anyway, things look (mostly) good: [89683.623479] nvme nvme0: connecting queue 0 [89683.623610] nvme nvme0: queue 0: start TLS with key 21712227 [89683.623622] tls_client_hello_psk: peerid 21712227 [89683.623687] nvmet_tcp: queue 0: TLS ServerHello [89683.623695] tls_server_hello_psk: peerid 0 [89683.707089] nvmet_tcp: queue 0: TLS handshake done, key 21712227, status 0 [89683.707099] nvmet_tcp: queue 0: restarting queue after TLS handshake [89683.707109] nvme nvme0: queue 0: TLS handshake done, key 21712227, status 0 [89683.708138] nvme nvme0: queue 0: TLS handshake complete, error 0 [89683.708781] nvmet_tcp: queue 0: failed to map data [89745.034618] nvme nvme0: queue 0: timeout cid 0x0 type 4 opcode 0x7f (Connect) [89745.036161] nvme nvme0: Connect command failed, error wo/DNR bit: 881 [89745.037437] nvme nvme0: failed to connect queue: 0 ret=881 So, TLS handshake works; I 'just' have to work on the actual data transport. But still, a _big_ step forward. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: pre-release of v7 2023-03-09 18:29 ` Hannes Reinecke @ 2023-03-09 20:20 ` Chuck Lever III 2023-03-09 20:34 ` Hannes Reinecke 0 siblings, 1 reply; 4+ messages in thread From: Chuck Lever III @ 2023-03-09 20:20 UTC (permalink / raw) To: Hannes Reinecke; +Cc: kernel-tls-handshake > On Mar 9, 2023, at 1:29 PM, Hannes Reinecke <hare@suse.de> wrote: > > On 3/9/23 15:41, Chuck Lever III wrote: >> Hi- >> I'm now trying to take a step or two towards a more typical development >> process for the kernel patches and ktls-utils, as we replace the old >> upcall mechanism with netlink. This is in preparation for a release of >> ktls-utils-0.8, once netdev is close to happy with the kernel part. >> I have pushed pre-release branches of v7 so you can comment early, test >> it, or send me patches /before/ I post "Another crack at a handshake >> upcall mechanism" v7 on netdev@. What a concept, eh? > <applause> > >> Kernel part: The upcall-v7 branch in >> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git > Hmm. Guess I'll branch off that, and push my patches on top of > that. They probably have to go via a different maintainer (read: hch), > so keeping them separately might be useful. > > Or having them on your tree, too? > Dunno. The first two commits in topic-rpc-with-tls-upcall will go through netdev. The last three commits I can take through the nfsd tree. The remaining commits will go through the nfs client tree. IMO having the latest PSK work included will be good for anyone wanting a central place to get all of this work, and it will make it a little easier for me to test PSK. I'm not planning to send it all through one tree unless the other tree maintainers ask me to. >> User space part: The netlink-v7 branch in >> https://github.com/oracle/ktls-utils > Ah. There was the 'before' part of your mail... > I've pushed a patchset for the 'netlink' branch, containing fixes > to get PSK running. > Will be rebasing them to v7. • 8eaa23f netlink: de-constify nla_policy I'm dropping this one. My libnl headers have a "const", so the compiler complaint you see is clearly due to a locally aged version of libnl. This change would break my builds. • 7d2a2ea ktls: add 'use_psk' argument to tlshd_make_priorites_string() I'll rework this a bit and fold it into netlink-v7. • 363ad37 netlink: Reshuffle argument parsing in tlshd_genl_valid_handler() I don't understand this. There is no longer an ACCEPT_STATUS attribute -- the v7 kernel now returns an invalid message with a non-zero errno. The patch description does not help me understand why the attribute parsing order matters. • 2e6b71c netlink: No key is always an error It's not an error for anonymous handshakes. This patch introduces a layering violation -- the tlshd_genl_done() call shouldn't turn a "success" reply into an error reply -- it's job is only to form the reply message, not to enforce policy. If this condition needs to be an error, the handshake specific code needs to decide that before genl_done is ever called. But more preferably, the kernel consumer for PSK should work like x.509 (as we discussed last week). The consumer needs to get the DONE downcall and then decide that, since there's no peer ID, this is actually not a usable TLS session. (That will be the case for x.509 handshakes where the client did not offer a certificate, but the server's security policy requires one). • f0e9956 server: set peer id for PSK • 6651a4a client: set peer ID for PSK I think you misunderstand what the session_peerid is. It's the peer ID of the /remote/ peer, not the local peer ID that the remote chose. This attribute has been renamed in v7 to better reflect its purpose. If PSK consumers need to know the local peer ID chosen by the remote, that IMO needs to be a separate netlink attribute. -- Chuck Lever ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: pre-release of v7 2023-03-09 20:20 ` Chuck Lever III @ 2023-03-09 20:34 ` Hannes Reinecke 0 siblings, 0 replies; 4+ messages in thread From: Hannes Reinecke @ 2023-03-09 20:34 UTC (permalink / raw) To: Chuck Lever III; +Cc: kernel-tls-handshake On 3/9/23 21:20, Chuck Lever III wrote: > > >> On Mar 9, 2023, at 1:29 PM, Hannes Reinecke <hare@suse.de> wrote: >> >> On 3/9/23 15:41, Chuck Lever III wrote: >>> Hi- >>> I'm now trying to take a step or two towards a more typical development >>> process for the kernel patches and ktls-utils, as we replace the old >>> upcall mechanism with netlink. This is in preparation for a release of >>> ktls-utils-0.8, once netdev is close to happy with the kernel part. >>> I have pushed pre-release branches of v7 so you can comment early, test >>> it, or send me patches /before/ I post "Another crack at a handshake >>> upcall mechanism" v7 on netdev@. What a concept, eh? >> <applause> >> >>> Kernel part: The upcall-v7 branch in >>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git >> Hmm. Guess I'll branch off that, and push my patches on top of >> that. They probably have to go via a different maintainer (read: hch), >> so keeping them separately might be useful. >> >> Or having them on your tree, too? >> Dunno. > > The first two commits in topic-rpc-with-tls-upcall will go > through netdev. The last three commits I can take through > the nfsd tree. The remaining commits will go through the > nfs client tree. > > IMO having the latest PSK work included will be good for > anyone wanting a central place to get all of this work, and > it will make it a little easier for me to test PSK. > > I'm not planning to send it all through one tree unless the > other tree maintainers ask me to. > > >>> User space part: The netlink-v7 branch in >>> https://github.com/oracle/ktls-utils >> Ah. There was the 'before' part of your mail... >> I've pushed a patchset for the 'netlink' branch, containing fixes >> to get PSK running. >> Will be rebasing them to v7. > > • 8eaa23f netlink: de-constify nla_policy > > I'm dropping this one. My libnl headers have a "const", so > the compiler complaint you see is clearly due to a locally > aged version of libnl. This change would break my builds. > Yeah, possibly. Have to figure out what I can/will do here. > > • 7d2a2ea ktls: add 'use_psk' argument to tlshd_make_priorites_string() > > I'll rework this a bit and fold it into netlink-v7. > Ta. > > • 363ad37 netlink: Reshuffle argument parsing in tlshd_genl_valid_handler() > > I don't understand this. There is no longer an ACCEPT_STATUS > attribute -- the v7 kernel now returns an invalid message with > a non-zero errno. > > The patch description does not help me understand why the > attribute parsing order matters. > Thing is, in v6 any error will _just_ have the error status, with no additional value (auth type? mode?), playing havoc when debugging. But as I said, it'll probably needs to be reworked for v7. > > • 2e6b71c netlink: No key is always an error > > It's not an error for anonymous handshakes. This patch > introduces a layering violation -- the tlshd_genl_done() call > shouldn't turn a "success" reply into an error reply -- it's > job is only to form the reply message, not to enforce policy. > Right. > If this condition needs to be an error, the handshake specific > code needs to decide that before genl_done is ever called. > Okay, will do so. > But more preferably, the kernel consumer for PSK should work > like x.509 (as we discussed last week). The consumer needs to > get the DONE downcall and then decide that, since there's no > peer ID, this is actually not a usable TLS session. (That will > be the case for x.509 handshakes where the client did not offer > a certificate, but the server's security policy requires one). > Well, slightly different for PSK; the PSK identity is bound to the key material, so if there if the PSK identity is not found the connection won't have key material and consequently the TLS session can't be established. (That's the 'S' in Pre-Shared Key ...) > > • f0e9956 server: set peer id for PSK > • 6651a4a client: set peer ID for PSK > > I think you misunderstand what the session_peerid is. It's the > peer ID of the /remote/ peer, not the local peer ID that the > remote chose. This attribute has been renamed in v7 to better > reflect its purpose. > > If PSK consumers need to know the local peer ID chosen by the > remote, that IMO needs to be a separate netlink attribute. > That is how PSK works. The client sends a list of possible ids, where each id is bound to a key. The server compares those ids against the ids present on the the server, and picks one of those. This selected id is then sent back to the client to inform it which id from the list it should choose for the transaction. Then both sides have agreed on a key, and that will be used for the TLS session. So for PSK we don't have a 'server' or 'client' ID; there is just a single PSK identity on both sides. For me, the 'peerid' attribute works just fine, and I think an additional attribute is not necessary. But if you decide otherwise, fine. Just let me know. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-09 20:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-09 14:41 pre-release of v7 Chuck Lever III 2023-03-09 18:29 ` Hannes Reinecke 2023-03-09 20:20 ` Chuck Lever III 2023-03-09 20:34 ` Hannes Reinecke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).