| Message ID | 20230824143925.9098-1-hare@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | nvme: In-kernel TLS support for TCP | expand |
> Hi all, Hannes, I think that this is in decent shape. Assuming that the recent reports on the tls tests are resolved, I think this is ready for inclusion. I also want to give it some time on the nvme tree. > > with the merge of Chuck Levers handshake upcall mechanism and > my tls_read_sock() implementation finally merged with net-next > it's now time to restart on the actual issue, namely implementing > in-kernel TLS support for nvme-tcp. > > The patchset is based on the recent net-next git tree; > everything after commit ba4a734e1aa0 ("net/tls: avoid TCP window > full during ->read_sock()") should work. > You might want to add the patch > 'nvmet-tcp: use 'spin_lock_bh' for state_lock()' > before applying this patchset; otherwise results might be ... > interesting. > > It also requires the 'tlshd' userspace daemon > (https://github.com/oracle/ktls-utils) > for the actual TLS handshake. > Changes for nvme-cli are already included in the upstream repository. > > Theory of operation: > A dedicated '.nvme' keyring is created to hold the pre-shared keys (PSKs) > for the TLS handshake. Keys will have to be provisioned before TLS handshake > is attempted; that can be done with the 'nvme gen-tls-key' command for nvme-cli > (patches are already merged upstream). > After connection to the remote TCP port the client side will use the > 'best' PSK (as inferred from the NVMe TCP spec) or the PSK specified > by the '--tls_key' option to nvme-cli and call the TLS userspace daemon > to initiate a TLS handshake. > The server side will then invoke the TLS userspace daemon to run the TLS > handshake. > If the TLS handshake succeeds the userspace daemon will be activating > kTLS on the socket, and control is passed back to the kernel. > > This implementation currently does not implement the so-called > 'secure concatenation' mode from NVMe-TCP; a TPAR is still pending > with fixes for it, so I'll wait until it's published before > posting patches for that. > > Patchset can be found at: > git.kernel.org/pub/scm/linux/kernel/git/hare/nvme.git > branch tls.v16 > > For testing I'm using this script, running on a nvme target > with NQN 'nqn.test' and using 127.0.0.1 as a port; the port > has to set 'addr_tsas' to 'tls1.3': > > modprobe nvmet-tcp > nvmetcli restore > modprobe nvme-tcp > ./nvme gen-tls-key --subsysnqn=nqn.test -i > ./nvme gen-tls-key --subsysnqn=nqn.2014-08.org.nvmexpress.discovery -i > tlshd -c /etc/tlshd.conf > > and then one can do a simple: > # nvme connect -t tcp -a 127.0.0.1 -s 4420 -n nqn.test --tls > to start the connection. > > As usual, comments and reviews are welcome. > > Changes to v10: > - Include reviews from Sagi > > Changes to v9: > - Close race between done() and timeout() > - Add logging message for icreq/icresp failure > - Sparse fixes > - Restrict TREQ setting to 'not required' or 'required' > when TLS is enabled > > Changes to v8: > - Simplify reference counting as suggested by Sagi > - Implement nvmf_parse_key() to simplify options parsing > - Add patch to peek at icreq to figure out whether TLS > should be started > > Changes to v7: > - Include reviews from Simon > - Do not call sock_release() when sock_alloc_file() fails > > Changes to v6: > - More reviews from Sagi > - Fixup non-tls connections > - Fixup nvme options handling > - Add reference counting to nvmet-tcp queues > > Changes to v5: > - Include reviews from Sagi > - Split off nvmet tsas/treq handling > - Sanitize sock_file handling > > Changes to v4: > - Split off network patches into a separate > patchset > - Handle TLS Alert notifications > > Changes to v3: > - Really handle MSG_EOR for TLS > - Fixup MSG_SENDPAGE_NOTLAST handling > - Conditionally disable fabric option > > Changes to v2: > - Included reviews from Sagi > - Removed MSG_SENDPAGE_NOTLAST > - Improved MSG_EOR handling for TLS > - Add config options NVME_TCP_TLS > and NVME_TARGET_TCP_TLS > > Changes to the original RFC: > - Add a CONFIG_NVME_TLS config option > - Use a single PSK for the TLS handshake > - Make TLS connections mandatory > - Do not peek messages for the server > - Simplify data_ready callback > - Implement read_sock() for TLS > > Hannes Reinecke (18): > nvme-keyring: register '.nvme' keyring > nvme-keyring: define a 'psk' keytype > nvme: add TCP TSAS definitions > nvme-tcp: add definitions for TLS cipher suites > nvme-keyring: implement nvme_tls_psk_default() > security/keys: export key_lookup() > nvme-tcp: allocate socket file > nvme-tcp: enable TLS handshake upcall > nvme-tcp: control message handling for recvmsg() > nvme-tcp: improve icreq/icresp logging > nvme-fabrics: parse options 'keyring' and 'tls_key' > nvmet: make TCP sectype settable via configfs > nvmet-tcp: make nvmet_tcp_alloc_queue() a void function > nvmet-tcp: allocate socket file > nvmet: Set 'TREQ' to 'required' when TLS is enabled > nvmet-tcp: enable TLS handshake upcall > nvmet-tcp: control messages for recvmsg() > nvmet-tcp: peek icreq before starting TLS > > drivers/nvme/common/Kconfig | 4 + > drivers/nvme/common/Makefile | 3 +- > drivers/nvme/common/keyring.c | 182 ++++++++++++++++++ > drivers/nvme/host/Kconfig | 15 ++ > drivers/nvme/host/core.c | 12 +- > drivers/nvme/host/fabrics.c | 67 ++++++- > drivers/nvme/host/fabrics.h | 9 + > drivers/nvme/host/nvme.h | 1 + > drivers/nvme/host/sysfs.c | 21 +++ > drivers/nvme/host/tcp.c | 184 ++++++++++++++++-- > drivers/nvme/target/Kconfig | 15 ++ > drivers/nvme/target/configfs.c | 128 ++++++++++++- > drivers/nvme/target/nvmet.h | 11 ++ > drivers/nvme/target/tcp.c | 334 ++++++++++++++++++++++++++++++--- > include/linux/key.h | 1 + > include/linux/nvme-keyring.h | 36 ++++ > include/linux/nvme-tcp.h | 6 + > include/linux/nvme.h | 10 + > security/keys/key.c | 1 + > 19 files changed, 991 insertions(+), 49 deletions(-) > create mode 100644 drivers/nvme/common/keyring.c > create mode 100644 include/linux/nvme-keyring.h >
On 9/12/23 13:34, Sagi Grimberg wrote: > >> Hi all, > > Hannes, I think that this is in decent shape. Assuming that > the recent reports on the tls tests are resolved, I think this > is ready for inclusion. > Thanks for that. > I also want to give it some time on the nvme tree. > I guess that will be a good idea. I had a look at the report for I/O timeouts with TLS, and currently it seems not to be TLS specific but rather generic issues with the NVMe stack, just being visible now due to delayed timing. Cheers, Hannes
Hi all, with the merge of Chuck Levers handshake upcall mechanism and my tls_read_sock() implementation finally merged with net-next it's now time to restart on the actual issue, namely implementing in-kernel TLS support for nvme-tcp. The patchset is based on the recent net-next git tree; everything after commit ba4a734e1aa0 ("net/tls: avoid TCP window full during ->read_sock()") should work. You might want to add the patch 'nvmet-tcp: use 'spin_lock_bh' for state_lock()' before applying this patchset; otherwise results might be ... interesting. It also requires the 'tlshd' userspace daemon (https://github.com/oracle/ktls-utils) for the actual TLS handshake. Changes for nvme-cli are already included in the upstream repository. Theory of operation: A dedicated '.nvme' keyring is created to hold the pre-shared keys (PSKs) for the TLS handshake. Keys will have to be provisioned before TLS handshake is attempted; that can be done with the 'nvme gen-tls-key' command for nvme-cli (patches are already merged upstream). After connection to the remote TCP port the client side will use the 'best' PSK (as inferred from the NVMe TCP spec) or the PSK specified by the '--tls_key' option to nvme-cli and call the TLS userspace daemon to initiate a TLS handshake. The server side will then invoke the TLS userspace daemon to run the TLS handshake. If the TLS handshake succeeds the userspace daemon will be activating kTLS on the socket, and control is passed back to the kernel. This implementation currently does not implement the so-called 'secure concatenation' mode from NVMe-TCP; a TPAR is still pending with fixes for it, so I'll wait until it's published before posting patches for that. Patchset can be found at: git.kernel.org/pub/scm/linux/kernel/git/hare/nvme.git branch tls.v16 For testing I'm using this script, running on a nvme target with NQN 'nqn.test' and using 127.0.0.1 as a port; the port has to set 'addr_tsas' to 'tls1.3': modprobe nvmet-tcp nvmetcli restore modprobe nvme-tcp ./nvme gen-tls-key --subsysnqn=nqn.test -i ./nvme gen-tls-key --subsysnqn=nqn.2014-08.org.nvmexpress.discovery -i tlshd -c /etc/tlshd.conf and then one can do a simple: # nvme connect -t tcp -a 127.0.0.1 -s 4420 -n nqn.test --tls to start the connection. As usual, comments and reviews are welcome. Changes to v10: - Include reviews from Sagi Changes to v9: - Close race between done() and timeout() - Add logging message for icreq/icresp failure - Sparse fixes - Restrict TREQ setting to 'not required' or 'required' when TLS is enabled Changes to v8: - Simplify reference counting as suggested by Sagi - Implement nvmf_parse_key() to simplify options parsing - Add patch to peek at icreq to figure out whether TLS should be started Changes to v7: - Include reviews from Simon - Do not call sock_release() when sock_alloc_file() fails Changes to v6: - More reviews from Sagi - Fixup non-tls connections - Fixup nvme options handling - Add reference counting to nvmet-tcp queues Changes to v5: - Include reviews from Sagi - Split off nvmet tsas/treq handling - Sanitize sock_file handling Changes to v4: - Split off network patches into a separate patchset - Handle TLS Alert notifications Changes to v3: - Really handle MSG_EOR for TLS - Fixup MSG_SENDPAGE_NOTLAST handling - Conditionally disable fabric option Changes to v2: - Included reviews from Sagi - Removed MSG_SENDPAGE_NOTLAST - Improved MSG_EOR handling for TLS - Add config options NVME_TCP_TLS and NVME_TARGET_TCP_TLS Changes to the original RFC: - Add a CONFIG_NVME_TLS config option - Use a single PSK for the TLS handshake - Make TLS connections mandatory - Do not peek messages for the server - Simplify data_ready callback - Implement read_sock() for TLS Hannes Reinecke (18): nvme-keyring: register '.nvme' keyring nvme-keyring: define a 'psk' keytype nvme: add TCP TSAS definitions nvme-tcp: add definitions for TLS cipher suites nvme-keyring: implement nvme_tls_psk_default() security/keys: export key_lookup() nvme-tcp: allocate socket file nvme-tcp: enable TLS handshake upcall nvme-tcp: control message handling for recvmsg() nvme-tcp: improve icreq/icresp logging nvme-fabrics: parse options 'keyring' and 'tls_key' nvmet: make TCP sectype settable via configfs nvmet-tcp: make nvmet_tcp_alloc_queue() a void function nvmet-tcp: allocate socket file nvmet: Set 'TREQ' to 'required' when TLS is enabled nvmet-tcp: enable TLS handshake upcall nvmet-tcp: control messages for recvmsg() nvmet-tcp: peek icreq before starting TLS drivers/nvme/common/Kconfig | 4 + drivers/nvme/common/Makefile | 3 +- drivers/nvme/common/keyring.c | 182 ++++++++++++++++++ drivers/nvme/host/Kconfig | 15 ++ drivers/nvme/host/core.c | 12 +- drivers/nvme/host/fabrics.c | 67 ++++++- drivers/nvme/host/fabrics.h | 9 + drivers/nvme/host/nvme.h | 1 + drivers/nvme/host/sysfs.c | 21 +++ drivers/nvme/host/tcp.c | 184 ++++++++++++++++-- drivers/nvme/target/Kconfig | 15 ++ drivers/nvme/target/configfs.c | 128 ++++++++++++- drivers/nvme/target/nvmet.h | 11 ++ drivers/nvme/target/tcp.c | 334 ++++++++++++++++++++++++++++++--- include/linux/key.h | 1 + include/linux/nvme-keyring.h | 36 ++++ include/linux/nvme-tcp.h | 6 + include/linux/nvme.h | 10 + security/keys/key.c | 1 + 19 files changed, 991 insertions(+), 49 deletions(-) create mode 100644 drivers/nvme/common/keyring.c create mode 100644 include/linux/nvme-keyring.h