| Message ID | 20220518112234.24264-1-hare@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | nvme: In-band authentication support | expand |
On 5/18/22 13:22, Hannes Reinecke wrote: > Hi all, > > recent updates to the NVMe spec have added definitions for in-band > authentication, and seeing that it provides some real benefit > especially for NVMe-TCP here's an attempt to implement it. > > Thanks to Nicolai Stange the crypto DH framework has been upgraded > to provide us with a FFDHE implementation; I've updated the patchset > to use the ephemeral key generation provided there. > > Note that this is just for in-band authentication. Secure > concatenation (ie starting TLS with the negotiated parameters) > requires a TLS handshake, which the in-kernel TLS implementation > does not provide. This is being worked on with a different patchset > which is still WIP. > > The nvme-cli support has already been merged; please use the latest > nvme-cli git repository to build the most recent version. > > A copy of this patchset can be found at > git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel > branch auth.v12 > > It is being cut against the latest master branch from Linus. > > As usual, comments and reviews are welcome. > How do we proceed here? This has been lingering for quite some time now, without any real progress. Despite everyone agreeing that we would need to have it. Anything which is missing from my side? Any other obstacles? Thanks. Cheers, Hannes
>> Hi all, >> >> recent updates to the NVMe spec have added definitions for in-band >> authentication, and seeing that it provides some real benefit >> especially for NVMe-TCP here's an attempt to implement it. >> >> Thanks to Nicolai Stange the crypto DH framework has been upgraded >> to provide us with a FFDHE implementation; I've updated the patchset >> to use the ephemeral key generation provided there. >> >> Note that this is just for in-band authentication. Secure >> concatenation (ie starting TLS with the negotiated parameters) >> requires a TLS handshake, which the in-kernel TLS implementation >> does not provide. This is being worked on with a different patchset >> which is still WIP. >> >> The nvme-cli support has already been merged; please use the latest >> nvme-cli git repository to build the most recent version. >> >> A copy of this patchset can be found at >> git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel >> branch auth.v12 >> >> It is being cut against the latest master branch from Linus. >> >> As usual, comments and reviews are welcome. >> > How do we proceed here? > This has been lingering for quite some time now, without any real > progress. Despite everyone agreeing that we would need to have it. > Anything which is missing from my side? > Any other obstacles? I've been through it a number of times during the iterations, I feel comfortable with it. I'd be more comfortable to get a second review at least on this code. But regardless, for the patches where it is missing: Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
On Wed, May 25, 2022 at 11:54:54AM +0200, Hannes Reinecke wrote: > How do we proceed here? > This has been lingering for quite some time now, without any real progress. As said it is a high priority for the upcoming merge window. But we also really need reviews from the crypto maintainers for the crypto patches, without that I can't merge the series even if I'd like to.
On 5/26/22 11:00, Christoph Hellwig wrote: > On Wed, May 25, 2022 at 11:54:54AM +0200, Hannes Reinecke wrote: >> How do we proceed here? >> This has been lingering for quite some time now, without any real progress. > > As said it is a high priority for the upcoming merge window. But we > also really need reviews from the crypto maintainers for the crypto > patches, without that I can't merge the series even if I'd like to. Hmm. Guess I can remove those helpers; after all, both are just wrappers around existing exported helpers. I'll resend. Cheers, Hannes
On 5/27/22 07:50, Hannes Reinecke wrote: > On 5/26/22 11:00, Christoph Hellwig wrote: >> On Wed, May 25, 2022 at 11:54:54AM +0200, Hannes Reinecke wrote: >>> How do we proceed herese h >>> This has been lingering for quite some time now, without any real >>> progress. >> >> As said it is a high priority for the upcoming merge window. But we >> also really need reviews from the crypto maintainers for the crypto >> patches, without that I can't merge the series even if I'd like to. > > Hmm. Guess I can remove those helpers; after all, > both are just wrappers around existing exported helpers. > I'll resend. > Bummer. There was a reason after all why I coded those helpers; they require an internal pointer :-( But all of these are just checks if specific hash/kpp functions are supported; replacing them with appropriate CONFIG_ statements should work as well. I'll check. Cheers, Hannes
On Fri, May 27, 2022 at 07:50:42AM +0200, Hannes Reinecke wrote: > On 5/26/22 11:00, Christoph Hellwig wrote: > > On Wed, May 25, 2022 at 11:54:54AM +0200, Hannes Reinecke wrote: > > > How do we proceed here? > > > This has been lingering for quite some time now, without any real progress. > > > > As said it is a high priority for the upcoming merge window. But we > > also really need reviews from the crypto maintainers for the crypto > > patches, without that I can't merge the series even if I'd like to. > > Hmm. Guess I can remove those helpers; after all, > both are just wrappers around existing exported helpers. > I'll resend. I've just acked those two patches. Cheers,
On 5/27/22 12:06, Herbert Xu wrote: > On Fri, May 27, 2022 at 07:50:42AM +0200, Hannes Reinecke wrote: >> On 5/26/22 11:00, Christoph Hellwig wrote: >>> On Wed, May 25, 2022 at 11:54:54AM +0200, Hannes Reinecke wrote: >>>> How do we proceed here? >>>> This has been lingering for quite some time now, without any real progress. >>> >>> As said it is a high priority for the upcoming merge window. But we >>> also really need reviews from the crypto maintainers for the crypto >>> patches, without that I can't merge the series even if I'd like to. >> >> Hmm. Guess I can remove those helpers; after all, >> both are just wrappers around existing exported helpers. >> I'll resend. > > I've just acked those two patches. > Ah. Thanks for this. Christoph, you can pick either v12 or v13; the difference is just the check for available hash and kpp functions. v12 has the dynamic version using the crypto helpers, v13 has the static version checking compile-time configuration. Either way would work. Cheers, Hannes
On Fri, May 27, 2022 at 12:21:59PM +0200, Hannes Reinecke wrote: > Christoph, you can pick either v12 or v13; the difference is just the check > for available hash and kpp functions. v12 has the dynamic version > using the crypto helpers, v13 has the static version checking compile-time > configuration. > Either way would work. Please resend with these helpers included and any other fixups after rc2 is released.
Hi all, recent updates to the NVMe spec have added definitions for in-band authentication, and seeing that it provides some real benefit especially for NVMe-TCP here's an attempt to implement it. Thanks to Nicolai Stange the crypto DH framework has been upgraded to provide us with a FFDHE implementation; I've updated the patchset to use the ephemeral key generation provided there. Note that this is just for in-band authentication. Secure concatenation (ie starting TLS with the negotiated parameters) requires a TLS handshake, which the in-kernel TLS implementation does not provide. This is being worked on with a different patchset which is still WIP. The nvme-cli support has already been merged; please use the latest nvme-cli git repository to build the most recent version. A copy of this patchset can be found at git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel branch auth.v12 It is being cut against the latest master branch from Linus. As usual, comments and reviews are welcome. Changes to v11: - Fixup type for FAILURE2 message (Prashant Nayak) - Do not sent SUCCESS2 if bi-directional authentication is not requested (Martin George) Changes to v10: - Fixup error return value when authentication failed Changes to v9: - Include review from Chaitanya - Use sparse array for dhgroup and hash lookup - Common function for auth_send and auth_receive Changes to v8: - Rebased to Nicolais crypto DH rework - Fixed oops on non-fabrics devices Changes to v7: - Space out hash list and dhgroup list in nvme negotiate data to be conformant with the spec - Update sequence number handling to start with a random value and ignore '0' as mandated by the spec - Update nvme_auth_generate_key to return the key as suggested by Sagi - Add nvmet_parse_fabrics_io_cmd() as suggested by hch Changes to v6: - Use 'u8' for DH group id and hash id - Use 'struct nvme_dhchap_key' - Rename variables to drop 'DHCHAP' - Include reviews from Chaitanya Changes to v5: - Unify nvme_auth_generate_key() - Unify nvme_auth_extract_key() - Fixed bug where re-authentication with wrong controller key would not fail - Include reviews from Sagi Changes to v4: - Validate against blktest suite - Fixup base64 decoding - Transform secret with correct hmac algorithm Changes to v3: - Renamed parameter to 'dhchap_ctrl_key' - Fixed bi-directional authentication - Included reviews from Sagi - Fixed base64 algorithm for transport encoding Changes to v2: - Dropped non-standard algorithms - Reworked base64 based on fs/crypto/fname.c - Fixup crash with no keys Changes to the original submission: - Included reviews from Vladislav - Included reviews from Sagi - Implemented re-authentication support - Fixed up key handling Hannes Reinecke (11): crypto: add crypto_has_shash() crypto: add crypto_has_kpp() lib/base64: RFC4648-compliant base64 encoding nvme: add definitions for NVMe In-Band authentication nvme-fabrics: decode 'authentication required' connect error nvme: Implement In-Band authentication nvme-auth: Diffie-Hellman key exchange support nvmet: parse fabrics commands on io queues nvmet: Implement basic In-Band Authentication nvmet-auth: Diffie-Hellman key exchange support nvmet-auth: expire authentication sessions crypto/kpp.c | 6 + crypto/shash.c | 6 + drivers/nvme/host/Kconfig | 12 + drivers/nvme/host/Makefile | 1 + drivers/nvme/host/auth.c | 1464 ++++++++++++++++++++++++ drivers/nvme/host/auth.h | 40 + drivers/nvme/host/core.c | 141 ++- drivers/nvme/host/fabrics.c | 83 +- drivers/nvme/host/fabrics.h | 7 + drivers/nvme/host/nvme.h | 31 + drivers/nvme/host/rdma.c | 1 + drivers/nvme/host/tcp.c | 1 + drivers/nvme/host/trace.c | 32 + drivers/nvme/target/Kconfig | 13 + drivers/nvme/target/Makefile | 1 + drivers/nvme/target/admin-cmd.c | 4 +- drivers/nvme/target/auth.c | 525 +++++++++ drivers/nvme/target/configfs.c | 138 ++- drivers/nvme/target/core.c | 15 + drivers/nvme/target/fabrics-cmd-auth.c | 536 +++++++++ drivers/nvme/target/fabrics-cmd.c | 55 +- drivers/nvme/target/nvmet.h | 75 +- include/crypto/hash.h | 2 + include/crypto/kpp.h | 2 + include/linux/base64.h | 16 + include/linux/nvme.h | 204 +++- lib/Makefile | 2 +- lib/base64.c | 103 ++ 28 files changed, 3501 insertions(+), 15 deletions(-) create mode 100644 drivers/nvme/host/auth.c create mode 100644 drivers/nvme/host/auth.h create mode 100644 drivers/nvme/target/auth.c create mode 100644 drivers/nvme/target/fabrics-cmd-auth.c create mode 100644 include/linux/base64.h create mode 100644 lib/base64.c