Hi all, I ran the latest blktests (git hash: 607513e64e48) with the v6.8 kernel, and I observed three failures. I also checked CKI project blktests runs with the v6.8 kernel, and found two failures. In total, five failure symptoms are observed as listed below. Compared with the v6.8-rc1 kernel [1], nvme test group has greatly improved for fc transport (Thanks go to Daniel). Now test runs do not hang. A few test cases still fail, but it is a great improvement :) [1] https://lore.kernel.org/linux-block/44i4y3fyqcz6k2pmum6toqylc2lvveb7x37ngskzfof52hoi2r@vxdxdnmggbj5/ List of failures ================ #1: block/011 #2: nvme/041,044 (fc transport) #3: srp/002, 011 (rdma_rxe driver) #4: nbd/002 (CKI failure) #5: zbd/010 (CKI failure) Failure description =================== #1: block/011 The test case fails with NVME devices due to lockdep WARNING "possible circular locking dependency detected". Reported in Sep/2022 [2]. In LSF 2023, it was noted that this failure should be fixed. A RFC fix patch was posted recently [3]. It still needs more discussion to be fixed. [2] https://lore.kernel.org/linux-block/20220930001943.zdbvolc3gkekfmcv@shindev/ [3] https://lore.kernel.org/linux-nvme/20231213051704.783490-1-shinichiro.kawasaki@wdc.com/ #2: nvme/041,044 (fc transport) With the trtype=fc configuration, nvme/041 and 044 fail with similar error messages: nvme/041 (Create authenticated connections) [failed] runtime 2.677s ... 4.823s --- tests/nvme/041.out 2023-11-29 12:57:17.206898664 +0900 +++ /home/shin/Blktests/blktests/results/nodev/nvme/041.out.bad 2024-03-19 14:50:56.399101323 +0900 @@ -2,5 +2,5 @@ Test unauthenticated connection (should fail) disconnected 0 controller(s) Test authenticated connection -disconnected 1 controller(s) +disconnected 0 controller(s) Test complete nvme/044 (Test bi-directional authentication) [failed] runtime 4.740s ... 7.482s --- tests/nvme/044.out 2023-11-29 12:57:17.212898647 +0900 +++ /home/shin/Blktests/blktests/results/nodev/nvme/044.out.bad 2024-03-19 14:51:08.062067741 +0900 @@ -4,7 +4,7 @@ Test invalid ctrl authentication (should fail) disconnected 0 controller(s) Test valid ctrl authentication -disconnected 1 controller(s) +disconnected 0 controller(s) Test invalid ctrl key (should fail) disconnected 0 controller(s) ... (Run 'diff -u tests/nvme/044.out /home/shin/Blktests/blktests/results/nodev/nvme/044.out.bad' to see the entire diff) #3: srp/002, 011 (rdma_rxe driver) Test process hang is observed occasionally. Reported to the relevant mailing lists in Aug/2023 [4]. Blktests was modified to change the default driver from rdma_rxe to siw to avoid impacts on blktests users. The root cause is not yet understood. [4] https://lore.kernel.org/linux-rdma/18a3ae8c-145b-4c7f-a8f5-67840feeb98c@acm.org/T/#mee9882c2cfd0cfff33caa04e75418576f4c7a789 #4: nbd/002 (CKI failure) CKI reported the failure [5]. I confirmed the test case fail occasionally on my test machine. I think blktests script can be improved to avoid the failure. I plan to post a fix candidate patch. nbd/002 (tests on partition handling for an nbd device) [failed] runtime ... 0.414s --- tests/nbd/002.out 2024-02-19 19:25:07.453721466 +0900 +++ /home/shin/kts/kernel-test-suite/sets/blktests/log/runlog/nodev/nbd/002.out.bad 2024-03-19 14:53:56.320218177 +0900 @@ -1,4 +1,4 @@ Running nbd/002 Testing IOCTL path Testing the netlink path -Test complete +Didn't have partition on the netlink path [5] https://datawarehouse.cki-project.org/kcidb/tests/11634679 #5: zbd/010 (CKI failure) CKI observed the failure [6], and Yi Zhang reported it to relevant mailing lists [7]. Though the WARN was observed with the test case zbd/010 for zoned block devices, it can be recreated with non-zoned regular block devices, when f2fs is set up with multiple block devices. A fix in F2FS is expected. [6] https://datawarehouse.cki-project.org/issue/2508 [7] https://lore.kernel.org/linux-f2fs-devel/CAHj4cs-kfojYC9i0G73PRkYzcxCTex=-vugRFeP40g_URGvnfQ@mail.gmail.com/
在 2024/3/19 10:59, Chaitanya Kulkarni 写道: > On 3/13/24 19:03, Guixin Liu wrote: >> >>> Didn't say this is actually in use, but a use-case that would benefit. >> I will add a TODO-list to research main cluster softwares >> >> to see if this is actually a significant use-case, after my patch merged. >> >> I'll change my code then. >> >> Best regards, >> >> Guixin Liu >> > so which one is the latest version I use for testing ? I want to run some > tests so I can provide reviewed-by tag ... > > -ck I am adding a per-controller percpu-ref to ns to implement preempt_and_abort, the v9 will be soon. Thanks, Guixin Liu >
On 3/13/24 19:03, Guixin Liu wrote:
>
>
>> Didn't say this is actually in use, but a use-case that would benefit.
>
> I will add a TODO-list to research main cluster softwares
>
> to see if this is actually a significant use-case, after my patch merged.
>
> I'll change my code then.
>
> Best regards,
>
> Guixin Liu
>
so which one is the latest version I use for testing ? I want to run some
tests so I can provide reviewed-by tag ...
-ck
hi, Christoph Hellwig, On Sun, Mar 17, 2024 at 09:36:45PM +0100, Christoph Hellwig wrote: > On Fri, Mar 15, 2024 at 04:21:13PM +0800, kernel test robot wrote: > > > > > > Hello, > > > > kernel test robot noticed a 6.4% improvement of fsmark.files_per_sec on: > > > > > > commit: 63dfa1004322d596417f23da43cdc43cf6298c71 ("nvme: move NVME_QUIRK_DEALLOCATE_ZEROES out of nvme_config_discard") > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master > > > > testcase: fsmark > > test machine: 96 threads 2 sockets Intel(R) Xeon(R) Platinum 8260L CPU @ 2.40GHz (Cascade Lake) with 128G memory > > parameters: > > That is kinda odd and unexpected. Is this system using one of the old > Intel SSDs that this quirk is actually used for? > yeah, the nvme sdd on this system is quite old (DC P3608 serial): https://www.intel.com/content/dam/www/public/us/en/documents/product-specifications/ssd-dc-p3608-spec.pdf
On Mon, Mar 18, 2024 at 10:26:23AM +0530, Nilay Shroff wrote:
> I have just tested the above patch and it's working well as expected.
> Now I don't see any issue formatting NVMe disk with the block-size of 512.
> I think we should commit the above changes.
Unfortunately I think it's going to break splitting the bios for real
multipath setups where the capabilities between the controllers are not
identical, i.e. another symptom of the problem of what limits we should
actually inherit.
I'll go backto the drawing board and will send out another patch.
Hello, After switching from Kernel v6.6.2 to v6.6.21 we're now seeing these workqueue warnings. I found a discussion thread about the the Intel drm driver here https://lore.kernel.org/lkml/ZO-BkaGuVCgdr3wc@slm.duckdns.org/T/ and this related bug report https://gitlab.freedesktop.org/drm/intel/-/issues/9245 but that that drm fix isn't merged into v6.6.21. It appears that we may need the same WQ_UNBOUND change to the nvme host tcp driver among others. [Fri Mar 15 22:30:06 2024] workqueue: nvme_tcp_io_work [nvme_tcp] hogged CPU for >10000us 4 times, consider switching to WQ_UNBOUND [Fri Mar 15 23:44:58 2024] workqueue: drain_vmap_area_work hogged CPU for >10000us 4 times, consider switching to WQ_UNBOUND [Sat Mar 16 09:55:27 2024] workqueue: drain_vmap_area_work hogged CPU for >10000us 8 times, consider switching to WQ_UNBOUND [Sat Mar 16 17:51:18 2024] workqueue: nvme_tcp_io_work [nvme_tcp] hogged CPU for >10000us 8 times, consider switching to WQ_UNBOUND [Sat Mar 16 23:04:14 2024] workqueue: nvme_tcp_io_work [nvme_tcp] hogged CPU for >10000us 16 times, consider switching to WQ_UNBOUND [Sun Mar 17 21:35:46 2024] perf: interrupt took too long (2707 > 2500), lowering kernel.perf_event_max_sample_rate to 73750 [Sun Mar 17 21:49:34 2024] workqueue: drain_vmap_area_work hogged CPU for >10000us 16 times, consider switching to WQ_UNBOUND ... workqueue: drm_fb_helper_damage_work [drm_kms_helper] hogged CPU for >10000us 32 times, consider switching to WQ_UNBOUND Thanks, Kamaljit Singh
Hello: This series was applied to jaegeuk/f2fs.git (dev) by Jens Axboe <axboe@kernel.dk>: On Sun, 28 Jan 2024 23:52:15 -0800 you wrote: > Fueled by the LSFMM discussion on removing GFP_NOFS initiated by Willy, > I've looked into the sole GFP_NOFS allocation in zonefs. As it turned out, > it is only done for zone management commands and can be removed. > > After digging into more callers of blkdev_zone_mgmt() I came to the > conclusion that the gfp_mask parameter can be removed alltogether. > > [...] Here is the summary with links: - [f2fs-dev,v3,1/5] zonefs: pass GFP_KERNEL to blkdev_zone_mgmt() call https://git.kernel.org/jaegeuk/f2fs/c/9105ce591b42 - [f2fs-dev,v3,2/5] dm: dm-zoned: guard blkdev_zone_mgmt with noio scope https://git.kernel.org/jaegeuk/f2fs/c/218082010ace - [f2fs-dev,v3,3/5] btrfs: zoned: call blkdev_zone_mgmt in nofs scope https://git.kernel.org/jaegeuk/f2fs/c/d9d556755f16 - [f2fs-dev,v3,4/5] f2fs: guard blkdev_zone_mgmt with nofs scope https://git.kernel.org/jaegeuk/f2fs/c/147ec1c60e32 - [f2fs-dev,v3,5/5] block: remove gfp_flags from blkdev_zone_mgmt https://git.kernel.org/jaegeuk/f2fs/c/71f4ecdbb42a You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On Wed, Mar 13, 2024 at 08:38:09PM +0800, Li Feng wrote:
> Make the workqueue userspace visible for easy viewing and configuration.
Thanks, applied to nvme-6.9.
On Sat, Mar 16, 2024 at 03:27:49AM +0800, iBug wrote:
> From: "Jiawei Fu (iBug)" <i@ibugone.com>
>
> This commit adds NVME_QUIRK_NO_DEEPEST_PS and NVME_QUIRK_BOGUS_NID for
> device [126f:2262], which appears to be a generic VID:PID pair used for
> many SSDs based on the Silicon Motion SM2262/SM2262EN controller.
Thanks, applied to nvme-6.9.
From: Hannes Reinecke <hare@suse.de> Evaluate the SC_C flag during DH-CHAP-HMAC negotiation and insert the generated PSK once negotiation has finished. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/target/auth.c | 70 +++++++++++++++++++++++++- drivers/nvme/target/fabrics-cmd-auth.c | 46 +++++++++++++++-- drivers/nvme/target/fabrics-cmd.c | 29 ++++++++--- drivers/nvme/target/nvmet.h | 30 ++++++++--- drivers/nvme/target/tcp.c | 34 ++++++++++++- 5 files changed, 187 insertions(+), 22 deletions(-) diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c index 7327fdd46147..cb5592025289 100644 --- a/drivers/nvme/target/auth.c +++ b/drivers/nvme/target/auth.c @@ -15,6 +15,7 @@ #include <linux/ctype.h> #include <linux/random.h> #include <linux/nvme-auth.h> +#include <linux/nvme-keyring.h> #include <asm/unaligned.h> #include "nvmet.h" @@ -136,7 +137,7 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id) return ret; } -int nvmet_setup_auth(struct nvmet_ctrl *ctrl) +int nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_req *req) { int ret = 0; struct nvmet_host_link *p; @@ -163,6 +164,11 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl) goto out_unlock; } + if (nvmet_queue_is_tls(req->sq)) { + pr_debug("host %s tls enabled\n", ctrl->hostnqn); + goto out_unlock; + } + ret = nvmet_setup_dhgroup(ctrl, host->dhchap_dhgroup_id); if (ret < 0) pr_warn("Failed to setup DH group"); @@ -234,6 +240,9 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl) void nvmet_auth_sq_free(struct nvmet_sq *sq) { cancel_delayed_work(&sq->auth_expired_work); +#ifdef CONFIG_NVME_TARGET_TCP_TLS + sq->tls_key = 0; +#endif kfree(sq->dhchap_c1); sq->dhchap_c1 = NULL; kfree(sq->dhchap_c2); @@ -262,6 +271,12 @@ void nvmet_destroy_auth(struct nvmet_ctrl *ctrl) nvme_auth_free_key(ctrl->ctrl_key); ctrl->ctrl_key = NULL; } +#ifdef CONFIG_NVME_TARGET_TCP_TLS + if (ctrl->tls_key) { + key_put(ctrl->tls_key); + ctrl->tls_key = NULL; + } +#endif } bool nvmet_check_auth_status(struct nvmet_req *req) @@ -541,3 +556,56 @@ int nvmet_auth_ctrl_sesskey(struct nvmet_req *req, return ret; } + +void nvmet_auth_insert_psk(struct nvmet_sq *sq) +{ + int hash_len = nvme_auth_hmac_hash_len(sq->ctrl->shash_id); + u8 *psk, *digest, *tls_psk; + size_t psk_len; +#ifdef CONFIG_NVME_TARGET_TCP_TLS + struct key *tls_key = NULL; +#endif + + psk = nvme_auth_generate_psk(sq->ctrl->shash_id, + sq->dhchap_skey, + sq->dhchap_skey_len, + sq->dhchap_c1, sq->dhchap_c2, + hash_len, &psk_len); + if (IS_ERR(psk)) { + pr_warn("%s: ctrl %d qid %d failed to generate PSK, error %ld\n", + __func__, sq->ctrl->cntlid, sq->qid, PTR_ERR(psk)); + return; + } + digest = nvme_auth_generate_digest(sq->ctrl->shash_id, psk, psk_len, + sq->ctrl->subsysnqn, + sq->ctrl->hostnqn); + if (IS_ERR(digest)) { + pr_warn("%s: ctrl %d qid %d failed to generate digest, error %ld\n", + __func__, sq->ctrl->cntlid, sq->qid, PTR_ERR(digest)); + goto out_free_psk; + } + tls_psk = nvme_auth_derive_tls_psk(sq->ctrl->shash_id, psk, psk_len, digest); + if (IS_ERR(tls_psk)) { + pr_warn("%s: ctrl %d qid %d failed to derive TLS PSK, error %ld\n", + __func__, sq->ctrl->cntlid, sq->qid, PTR_ERR(tls_psk)); + goto out_free_digest; + } +#ifdef CONFIG_NVME_TARGET_TCP_TLS + tls_key = nvme_tls_psk_refresh(NULL, sq->ctrl->hostnqn, sq->ctrl->subsysnqn, + sq->ctrl->shash_id, true, tls_psk, psk_len, digest); + if (IS_ERR(tls_key)) { + pr_warn("%s: ctrl %d qid %d failed to refresh key, error %ld\n", + __func__, sq->ctrl->cntlid, sq->qid, PTR_ERR(tls_key)); + tls_key = NULL; + kfree_sensitive(tls_psk); + } + if (sq->ctrl->tls_key) + key_put(sq->ctrl->tls_key); + sq->ctrl->tls_key = tls_key; +#endif + +out_free_digest: + kfree_sensitive(digest); +out_free_psk: + kfree_sensitive(psk); +} diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c index eb7785be0ca7..fe7f4b3adb77 100644 --- a/drivers/nvme/target/fabrics-cmd-auth.c +++ b/drivers/nvme/target/fabrics-cmd-auth.c @@ -43,8 +43,26 @@ static u16 nvmet_auth_negotiate(struct nvmet_req *req, void *d) data->auth_protocol[0].dhchap.halen, data->auth_protocol[0].dhchap.dhlen); req->sq->dhchap_tid = le16_to_cpu(data->t_id); - if (data->sc_c) - return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + if (data->sc_c != NVME_AUTH_SECP_NOSC) { + if (!IS_ENABLED(CONFIG_NVME_TARGET_TCP_TLS)) + return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + /* Secure concatenation can only be enabled on the admin queue */ + if (req->sq->qid) + return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + switch (data->sc_c) { + case NVME_AUTH_SECP_NEWTLSPSK: + if (nvmet_queue_is_tls(req->sq)) + return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + break; + case NVME_AUTH_SECP_REPLACETLSPSK: + if (!nvmet_queue_is_tls(req->sq)) + return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + break; + default: + return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + } + ctrl->concat = true; + } if (data->napd != 1) return NVME_AUTH_DHCHAP_FAILURE_HASH_UNUSABLE; @@ -103,6 +121,13 @@ static u16 nvmet_auth_negotiate(struct nvmet_req *req, void *d) nvme_auth_dhgroup_name(fallback_dhgid)); ctrl->dh_gid = fallback_dhgid; } + if (ctrl->dh_gid == NVME_AUTH_DHGROUP_NULL && + ctrl->concat) { + pr_debug("%s: ctrl %d qid %d: NULL DH group invalid " + "for secure channel concatenation\n", __func__, + ctrl->cntlid, req->sq->qid); + return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; + } pr_debug("%s: ctrl %d qid %d: selected DH group %s (%d)\n", __func__, ctrl->cntlid, req->sq->qid, nvme_auth_dhgroup_name(ctrl->dh_gid), ctrl->dh_gid); @@ -154,6 +179,12 @@ static u16 nvmet_auth_reply(struct nvmet_req *req, void *d) kfree(response); pr_debug("%s: ctrl %d qid %d host authenticated\n", __func__, ctrl->cntlid, req->sq->qid); + if (!data->cvalid && ctrl->concat) { + pr_debug("%s: ctrl %d qid %d invalid challenge\n", + __func__, ctrl->cntlid, req->sq->qid); + return NVME_AUTH_DHCHAP_FAILURE_FAILED; + } + req->sq->dhchap_s2 = le32_to_cpu(data->seqnum); if (data->cvalid) { req->sq->dhchap_c2 = kmemdup(data->rval + data->hl, data->hl, GFP_KERNEL); @@ -163,11 +194,14 @@ static u16 nvmet_auth_reply(struct nvmet_req *req, void *d) pr_debug("%s: ctrl %d qid %d challenge %*ph\n", __func__, ctrl->cntlid, req->sq->qid, data->hl, req->sq->dhchap_c2); - } else { + } + if (req->sq->dhchap_s2 == 0) { + if (ctrl->concat) + nvmet_auth_insert_psk(req->sq); req->sq->authenticated = true; + kfree(req->sq->dhchap_c2); req->sq->dhchap_c2 = NULL; } - req->sq->dhchap_s2 = le32_to_cpu(data->seqnum); return 0; } @@ -240,7 +274,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req) pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__, ctrl->cntlid, req->sq->qid); if (!req->sq->qid) { - if (nvmet_setup_auth(ctrl) < 0) { + if (nvmet_setup_auth(ctrl, req) < 0) { status = NVME_SC_INTERNAL; pr_err("ctrl %d qid 0 failed to setup" "re-authentication", @@ -296,6 +330,8 @@ void nvmet_execute_auth_send(struct nvmet_req *req) } goto done_kfree; case NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2: + if (ctrl->concat) + nvmet_auth_insert_psk(req->sq); req->sq->authenticated = true; pr_debug("%s: ctrl %d qid %d ctrl authenticated\n", __func__, ctrl->cntlid, req->sq->qid); diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c index 08e9c6b6f551..5e3c98e1bd6a 100644 --- a/drivers/nvme/target/fabrics-cmd.c +++ b/drivers/nvme/target/fabrics-cmd.c @@ -199,10 +199,20 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req) return ret; } -static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl) +static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl, struct nvmet_req *req) { + bool needs_auth = nvmet_has_auth(ctrl, req); + + /* Do not authenticate I/O queues for secure concatenation */ + if (ctrl->concat && req->sq->qid) + needs_auth = false; + + pr_debug("%s: ctrl %d qid %d should %sauthenticate, tls psk %08x\n", + __func__, ctrl->cntlid, req->sq->qid, + needs_auth ? "" : "not ", + req->sq->tls_key ? key_serial(req->sq->tls_key) : 0); return (u32)ctrl->cntlid | - (nvmet_has_auth(ctrl) ? NVME_CONNECT_AUTHREQ_ATR : 0); + (needs_auth ? NVME_CONNECT_AUTHREQ_ATR : 0); } static void nvmet_execute_admin_connect(struct nvmet_req *req) @@ -254,7 +264,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req) uuid_copy(&ctrl->hostid, &d->hostid); - ret = nvmet_setup_auth(ctrl); + ret = nvmet_setup_auth(ctrl, req); if (ret < 0) { pr_err("Failed to setup authentication, error %d\n", ret); nvmet_ctrl_put(ctrl); @@ -271,12 +281,13 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req) goto out; } - pr_info("creating %s controller %d for subsystem %s for NQN %s%s%s.\n", + pr_info("creating %s controller %d for subsystem %s for NQN %s%s%s%s.\n", nvmet_is_disc_subsys(ctrl->subsys) ? "discovery" : "nvm", ctrl->cntlid, ctrl->subsys->subsysnqn, ctrl->hostnqn, - ctrl->pi_support ? " T10-PI is enabled" : "", - nvmet_has_auth(ctrl) ? " with DH-HMAC-CHAP" : ""); - req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl)); + ctrl->pi_support ? ", T10-PI" : "", + nvmet_has_auth(ctrl, req) ? ", DH-HMAC-CHAP" : "", + nvmet_queue_is_tls(req->sq) ? ", TLS" : ""); + req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl, req)); out: kfree(d); complete: @@ -319,6 +330,8 @@ static void nvmet_execute_io_connect(struct nvmet_req *req) ctrl = nvmet_ctrl_find_get(d->subsysnqn, d->hostnqn, le16_to_cpu(d->cntlid), req); if (!ctrl) { + pr_warn("invalid controller (%x)\n", + le16_to_cpu(d->cntlid)); status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR; goto out; } @@ -335,7 +348,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req) goto out_ctrl_put; pr_debug("adding queue %d to ctrl %d.\n", qid, ctrl->cntlid); - req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl)); + req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl, req)); out: kfree(d); complete: diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index 7c6e7e65b032..765da2b932c5 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -121,6 +121,9 @@ struct nvmet_sq { u32 dhchap_s2; u8 *dhchap_skey; int dhchap_skey_len; +#endif +#ifdef CONFIG_NVME_TARGET_TCP_TLS + struct key *tls_key; #endif struct completion free_done; struct completion confirm_done; @@ -235,6 +238,7 @@ struct nvmet_ctrl { u64 err_counter; struct nvme_error_slot slots[NVMET_ERROR_LOG_SLOTS]; bool pi_support; + bool concat; #ifdef CONFIG_NVME_TARGET_AUTH struct nvme_dhchap_key *host_key; struct nvme_dhchap_key *ctrl_key; @@ -244,6 +248,9 @@ struct nvmet_ctrl { u8 *dh_key; size_t dh_keysize; #endif +#ifdef CONFIG_NVME_TARGET_TCP_TLS + struct key *tls_key; +#endif }; struct nvmet_subsys { @@ -707,13 +714,21 @@ static inline void nvmet_req_bio_put(struct nvmet_req *req, struct bio *bio) bio_put(bio); } +#ifdef CONFIG_NVME_TARGET_TCP_TLS +static inline bool nvmet_queue_is_tls(struct nvmet_sq *sq) +{ + return !!sq->tls_key; +} +#else +static inline bool nvmet_queue_is_tls(struct nvmet_sq *sq) { return false; } +#endif #ifdef CONFIG_NVME_TARGET_AUTH void nvmet_execute_auth_send(struct nvmet_req *req); void nvmet_execute_auth_receive(struct nvmet_req *req); int nvmet_auth_set_key(struct nvmet_host *host, const char *secret, bool set_ctrl); int nvmet_auth_set_host_hash(struct nvmet_host *host, const char *hash); -int nvmet_setup_auth(struct nvmet_ctrl *ctrl); +int nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_req *req); void nvmet_auth_sq_init(struct nvmet_sq *sq); void nvmet_destroy_auth(struct nvmet_ctrl *ctrl); void nvmet_auth_sq_free(struct nvmet_sq *sq); @@ -723,16 +738,18 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response, unsigned int hash_len); int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response, unsigned int hash_len); -static inline bool nvmet_has_auth(struct nvmet_ctrl *ctrl) +static inline bool nvmet_has_auth(struct nvmet_ctrl *ctrl, struct nvmet_req *req) { - return ctrl->host_key != NULL; + return ctrl->host_key != NULL && !nvmet_queue_is_tls(req->sq); } int nvmet_auth_ctrl_exponential(struct nvmet_req *req, u8 *buf, int buf_size); int nvmet_auth_ctrl_sesskey(struct nvmet_req *req, u8 *buf, int buf_size); +void nvmet_auth_insert_psk(struct nvmet_sq *sq); #else -static inline int nvmet_setup_auth(struct nvmet_ctrl *ctrl) +static inline int nvmet_setup_auth(struct nvmet_ctrl *ctrl, + struct nvmet_req *req) { return 0; } @@ -745,11 +762,12 @@ static inline bool nvmet_check_auth_status(struct nvmet_req *req) { return true; } -static inline bool nvmet_has_auth(struct nvmet_ctrl *ctrl) +static inline bool nvmet_has_auth(struct nvmet_ctrl *ctrl, + struct nvmet_req *req) { return false; } static inline const char *nvmet_dhchap_dhgroup_name(u8 dhgid) { return NULL; } +static inline void nvmet_auth_insert_psk(struct nvmet_sq *sq) {}; #endif - #endif /* _NVMET_H */ diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c index c8655fc5aa5b..4718d4d87a85 100644 --- a/drivers/nvme/target/tcp.c +++ b/drivers/nvme/target/tcp.c @@ -1071,10 +1071,11 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue) if (unlikely(!nvmet_req_init(req, &queue->nvme_cq, &queue->nvme_sq, &nvmet_tcp_ops))) { - pr_err("failed cmd %p id %d opcode %d, data_len: %d\n", + pr_err("failed cmd %p id %d opcode %d, data_len: %d, status: %04x\n", req->cmd, req->cmd->common.command_id, req->cmd->common.opcode, - le32_to_cpu(req->cmd->common.dptr.sgl.length)); + le32_to_cpu(req->cmd->common.dptr.sgl.length), + le16_to_cpu(req->cqe->status)); nvmet_tcp_handle_req_failure(queue, queue->cmd, req); return 0; @@ -1605,6 +1606,9 @@ static void nvmet_tcp_release_queue_work(struct work_struct *w) /* stop accepting incoming data */ queue->rcv_state = NVMET_TCP_RECV_ERR; + if (queue->nvme_sq.tls_key) + key_put(queue->nvme_sq.tls_key); + nvmet_tcp_uninit_data_in_cmds(queue); nvmet_sq_destroy(&queue->nvme_sq); cancel_work_sync(&queue->io_work); @@ -1811,6 +1815,32 @@ static void nvmet_tcp_tls_handshake_done(void *data, int status, spin_unlock_bh(&queue->state_lock); cancel_delayed_work_sync(&queue->tls_handshake_tmo_work); + + if (!status) { + struct key *tls_key = key_lookup(peerid); + + if (IS_ERR(tls_key)) { + pr_warn("%s: queue %d failed to lookup key %x\n", + __func__, queue->idx, peerid); + spin_lock_bh(&queue->state_lock); + queue->state = NVMET_TCP_Q_FAILED; + spin_unlock_bh(&queue->state_lock); + status = PTR_ERR(tls_key); + } else if (test_bit(KEY_FLAG_REVOKED, &tls_key->flags) || + test_bit(KEY_FLAG_INVALIDATED, &tls_key->flags)) { + pr_warn("%s: queue %d key %08x invalid\n", + __func__, queue->idx, peerid); + key_put(tls_key); + spin_lock_bh(&queue->state_lock); + queue->state = NVMET_TCP_Q_FAILED; + spin_unlock_bh(&queue->state_lock); + status = -EKEYREVOKED; + } else { + pr_debug("%s: queue %d using TLS PSK %x\n", + __func__, queue->idx, peerid); + queue->nvme_sq.tls_key = tls_key; + } + } if (status) nvmet_tcp_schedule_release_queue(queue); else -- 2.35.3
nvmet_check_ctrl_status() checks the authentication status, so we don't need to do that prior to calling it. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/target/core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 6bbe4df0166c..1d1a7026e817 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -891,9 +891,6 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req *req) if (nvme_is_fabrics(cmd)) return nvmet_parse_fabrics_io_cmd(req); - if (unlikely(!nvmet_check_auth_status(req))) - return NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; - ret = nvmet_check_ctrl_status(req); if (unlikely(ret)) return ret; -- 2.35.3
nvmet_check_ctrl_status() checks the authentication status, so we don't need to do that prior to calling it. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/target/admin-cmd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index f5b7054a4a05..936194de4a52 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -1005,8 +1005,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req) if (nvme_is_fabrics(cmd)) return nvmet_parse_fabrics_admin_cmd(req); - if (unlikely(!nvmet_check_auth_status(req))) - return NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; + if (nvmet_is_disc_subsys(nvmet_req_subsys(req))) return nvmet_parse_discovery_cmd(req); -- 2.35.3
From: Hannes Reinecke <hare@suse.de> With TP8018 a new key will be generated from the DH-HMAC-CHAP protocol after reset or recovery, but we need to start over to establish a new TLS connection with the new keys. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/tcp.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 94152ded123a..3811ee9cd040 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2219,6 +2219,22 @@ static void nvme_tcp_reconnect_or_remove(struct nvme_ctrl *ctrl) } } +static bool nvme_tcp_reset_for_secure_concat(struct nvme_ctrl *ctrl) +{ + if (!ctrl->opts->concat) + return false; + /* + * If a key has been generated and TLS has not been enabled + * reset the queue to start TLS handshake. + */ + if (ctrl->opts->tls_key && !ctrl->tls_key) { + dev_info(ctrl->device, "Reset to enable TLS with generated PSK\n"); + nvme_reset_ctrl(ctrl); + return true; + } + return false; +} + static void nvme_tcp_revoke_generated_tls_key(struct nvme_ctrl *ctrl) { if (!ctrl->opts->concat) @@ -2321,6 +2337,9 @@ static void nvme_tcp_reconnect_ctrl_work(struct work_struct *work) if (nvme_tcp_setup_ctrl(ctrl, false)) goto requeue; + if (nvme_tcp_reset_for_secure_concat(ctrl)) + return; + dev_info(ctrl->device, "Successfully reconnected (%d attempt)\n", ctrl->nr_reconnects); @@ -2396,6 +2415,7 @@ static void nvme_reset_ctrl_work(struct work_struct *work) if (nvme_tcp_setup_ctrl(ctrl, false)) goto out_fail; + nvme_tcp_reset_for_secure_concat(ctrl); return; out_fail: -- 2.35.3
As we can set DH-HMAC-CHAP keys, we should also be able to unset them. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/target/auth.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c index 3ddbc3880cac..7327fdd46147 100644 --- a/drivers/nvme/target/auth.c +++ b/drivers/nvme/target/auth.c @@ -25,6 +25,18 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret, unsigned char key_hash; char *dhchap_secret; + if (!strlen(secret)) { + if (set_ctrl) { + kfree(host->dhchap_ctrl_secret); + host->dhchap_ctrl_secret = NULL; + host->dhchap_ctrl_key_hash = 0; + } else { + kfree(host->dhchap_secret); + host->dhchap_secret = NULL; + host->dhchap_key_hash = 0; + } + return 0; + } if (sscanf(secret, "DHHC-1:%hhd:%*s", &key_hash) != 1) return -EINVAL; if (key_hash > 3) { -- 2.35.3
When secure concatenation is requested the connection needs to be reset to enable TLS encryption on the new cnnection. That implies that the original connection used for the DH-CHAP negotiation really shouldn't be used, and we should reset as soon as the DH-CHAP negotiation has succeeded on the admin queue. The current implementation does not allow to easily skip connection attempts on the I/O queues, so we connect I/O queues, but disable namespace scanning on these queues. With that no I/O can be issued on these queues, so we can tear them down quickly without having to wait for quiescing etc. Once that is done we can reset the controller directly after the ->create_ctrl() callback. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/core.c | 8 +++++++- drivers/nvme/host/fabrics.c | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 9b601655f423..57b664d12863 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4513,6 +4513,8 @@ EXPORT_SYMBOL_GPL(nvme_stop_ctrl); void nvme_start_ctrl(struct nvme_ctrl *ctrl) { + bool start_scan = ctrl->queue_count > 1; + nvme_enable_aen(ctrl); /* @@ -4525,7 +4527,11 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl) nvme_discovery_ctrl(ctrl)) nvme_change_uevent(ctrl, "NVME_EVENT=rediscover"); - if (ctrl->queue_count > 1) { + /* Suppress namespace scanning during setting up secure concatenation */ + if (ctrl->opts->concat && !ctrl->tls_key) + start_scan = false; + + if (start_scan) { nvme_queue_scan(ctrl); nvme_unquiesce_io_queues(ctrl); nvme_mpath_update(ctrl); diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index ae091e0e4ecf..06418e01ab69 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -1331,6 +1331,12 @@ nvmf_create_ctrl(struct device *dev, const char *buf) goto out_module_put; } + /* Reset controller to start TLS */ + if (opts->concat) { + pr_debug("resetting for secure concatenation\n"); + nvme_reset_ctrl(ctrl); + } + module_put(ops->module); return ctrl; -- 2.35.3
From: Hannes Reinecke <hare@suse.de> Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/auth.c | 108 ++++++++++++++++++++++++++++++++++-- drivers/nvme/host/fabrics.c | 25 ++++++++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/sysfs.c | 2 + drivers/nvme/host/tcp.c | 53 ++++++++++++++++-- include/linux/nvme.h | 7 +++ 6 files changed, 186 insertions(+), 12 deletions(-) diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c index a264b3ae078b..a4033a8f9607 100644 --- a/drivers/nvme/host/auth.c +++ b/drivers/nvme/host/auth.c @@ -12,6 +12,7 @@ #include "nvme.h" #include "fabrics.h" #include <linux/nvme-auth.h> +#include <linux/nvme-keyring.h> #define CHAP_BUF_SIZE 4096 static struct kmem_cache *nvme_chap_buf_cache; @@ -131,7 +132,12 @@ static int nvme_auth_set_dhchap_negotiate_data(struct nvme_ctrl *ctrl, data->auth_type = NVME_AUTH_COMMON_MESSAGES; data->auth_id = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE; data->t_id = cpu_to_le16(chap->transaction); - data->sc_c = 0; /* No secure channel concatenation */ + if (!ctrl->opts->concat || chap->qid != 0) + data->sc_c = NVME_AUTH_SECP_NOSC; + else if (ctrl->opts->tls_key) + data->sc_c = NVME_AUTH_SECP_REPLACETLSPSK; + else + data->sc_c = NVME_AUTH_SECP_NEWTLSPSK; data->napd = 1; data->auth_protocol[0].dhchap.authid = NVME_AUTH_DHCHAP_AUTH_ID; data->auth_protocol[0].dhchap.halen = 3; @@ -311,8 +317,9 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, data->hl = chap->hash_len; data->dhvlen = cpu_to_le16(chap->host_key_len); memcpy(data->rval, chap->response, chap->hash_len); - if (ctrl->ctrl_key) { + if (ctrl->ctrl_key) chap->bi_directional = true; + if (ctrl->ctrl_key || ctrl->opts->concat) { get_random_bytes(chap->c2, chap->hash_len); data->cvalid = 1; memcpy(data->rval + chap->hash_len, chap->c2, @@ -322,7 +329,10 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, } else { memset(chap->c2, 0, chap->hash_len); } - chap->s2 = nvme_auth_get_seqnum(); + if (ctrl->opts->concat) + chap->s2 = 0; + else + chap->s2 = nvme_auth_get_seqnum(); data->seqnum = cpu_to_le32(chap->s2); if (chap->host_key_len) { dev_dbg(ctrl->device, "%s: qid %d host public key %*ph\n", @@ -677,6 +687,79 @@ static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap) crypto_free_kpp(chap->dh_tfm); } +static int nvme_auth_secure_concat(struct nvme_ctrl *ctrl, + struct nvme_dhchap_queue_context *chap) +{ + u8 *psk, *digest, *tls_psk; + struct key *tls_key; + size_t psk_len; + int ret = 0; + + if (!chap->sess_key) { + dev_warn(ctrl->device, + "%s: qid %d no session key negotiated\n", + __func__, chap->qid); + return -ENOKEY; + } + + psk = nvme_auth_generate_psk(chap->hash_id, chap->sess_key, + chap->sess_key_len, + chap->c1, chap->c2, + chap->hash_len, &psk_len); + if (IS_ERR(psk)) { + ret = PTR_ERR(psk); + dev_warn(ctrl->device, + "%s: qid %d failed to generate PSK, error %d\n", + __func__, chap->qid, ret); + return ret; + } + dev_dbg(ctrl->device, + "%s: generated psk %*ph\n", __func__, (int)psk_len, psk); + + digest = nvme_auth_generate_digest(chap->hash_id, psk, psk_len, + ctrl->opts->subsysnqn, + ctrl->opts->host->nqn); + if (IS_ERR(digest)) { + ret = PTR_ERR(digest); + dev_warn(ctrl->device, + "%s: qid %d failed to generate digest, error %d\n", + __func__, chap->qid, ret); + goto out_free_psk; + }; + dev_dbg(ctrl->device, "%s: generated digest %s\n", + __func__, digest); + tls_psk = nvme_auth_derive_tls_psk(chap->hash_id, psk, psk_len, digest); + if (IS_ERR(tls_psk)) { + ret = PTR_ERR(tls_psk); + dev_warn(ctrl->device, + "%s: qid %d failed to derive TLS psk, error %d\n", + __func__, chap->qid, ret); + goto out_free_digest; + }; + + tls_key = nvme_tls_psk_refresh(ctrl->opts->keyring, ctrl->opts->host->nqn, + ctrl->opts->subsysnqn, chap->hash_id, + true, tls_psk, psk_len, digest); + if (IS_ERR(tls_key)) { + ret = PTR_ERR(tls_key); + dev_warn(ctrl->device, + "%s: qid %d failed to insert generated key, error %d\n", + __func__, chap->qid, ret); + tls_key = NULL; + kfree_sensitive(tls_psk); + } + if (ctrl->opts->tls_key) { + key_revoke(ctrl->opts->tls_key); + key_put(ctrl->opts->tls_key); + } + ctrl->opts->tls_key = tls_key; +out_free_digest: + kfree_sensitive(digest); +out_free_psk: + kfree_sensitive(psk); + return ret; +} + static void nvme_queue_auth_work(struct work_struct *work) { struct nvme_dhchap_queue_context *chap = @@ -831,10 +914,21 @@ static void nvme_queue_auth_work(struct work_struct *work) if (ret) chap->error = ret; } - if (!ret) { + if (ret) + goto fail2; + if (chap->qid || !ctrl->opts->concat) { chap->error = 0; return; } + ret = nvme_auth_secure_concat(ctrl, chap); + if (ret) { + dev_warn(ctrl->device, + "%s: qid %d failed to enable secure concatenation\n", + __func__, chap->qid); + chap->error = ret; + } else + chap->error = 0; + return; fail2: if (chap->status == 0) @@ -912,6 +1006,12 @@ static void nvme_ctrl_auth_work(struct work_struct *work) "qid 0: authentication failed\n"); return; } + /* + * Only run authentication on the admin queue for + * secure concatenation + */ + if (ctrl->opts->concat) + return; for (q = 1; q < ctrl->queue_count; q++) { ret = nvme_auth_negotiate(ctrl, q); diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 75aa69457353..ae091e0e4ecf 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -463,8 +463,9 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) result = le32_to_cpu(res.u32); ctrl->cntlid = result & 0xFFFF; if (result & (NVME_CONNECT_AUTHREQ_ATR | NVME_CONNECT_AUTHREQ_ASCR)) { - /* Secure concatenation is not implemented */ - if (result & NVME_CONNECT_AUTHREQ_ASCR) { + /* Check for secure concatenation */ + if ((result & NVME_CONNECT_AUTHREQ_ASCR) && + !ctrl->opts->concat) { dev_warn(ctrl->device, "qid 0: secure concatenation is not supported\n"); ret = NVME_SC_AUTH_REQUIRED; @@ -682,6 +683,7 @@ static const match_table_t opt_tokens = { #endif #ifdef CONFIG_NVME_TCP_TLS { NVMF_OPT_TLS, "tls" }, + { NVMF_OPT_CONCAT, "concat" }, #endif { NVMF_OPT_ERR, NULL } }; @@ -711,6 +713,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, opts->tls = false; opts->tls_key = NULL; opts->keyring = NULL; + opts->concat = false; options = o = kstrdup(buf, GFP_KERNEL); if (!options) @@ -1029,6 +1032,14 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, } opts->tls = true; break; + case NVMF_OPT_CONCAT: + if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) { + pr_err("TLS is not supported\n"); + ret = -EINVAL; + goto out; + } + opts->concat = true; + break; default: pr_warn("unknown parameter or missing value '%s' in ctrl creation request\n", p); @@ -1055,6 +1066,16 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, pr_warn("failfast tmo (%d) larger than controller loss tmo (%d)\n", opts->fast_io_fail_tmo, ctrl_loss_tmo); } + if (opts->concat && opts->tls) { + pr_err("Secure concatenation over TLS is not supported\n"); + ret = -EINVAL; + goto out; + } + if (opts->concat && opts->tls_key) { + pr_err("Cannot specify a TLS key for secure concatenation\n"); + ret = -EINVAL; + goto out; + } opts->host = nvmf_host_add(hostnqn, &hostid); if (IS_ERR(opts->host)) { diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h index 06cc54851b1b..accfc7228366 100644 --- a/drivers/nvme/host/fabrics.h +++ b/drivers/nvme/host/fabrics.h @@ -73,6 +73,7 @@ enum { NVMF_OPT_TLS = 1 << 25, NVMF_OPT_KEYRING = 1 << 26, NVMF_OPT_TLS_KEY = 1 << 27, + NVMF_OPT_CONCAT = 1 << 28, }; /** @@ -108,6 +109,7 @@ enum { * @keyring: Keyring to use for key lookups * @tls_key: TLS key for encrypted connections (TCP) * @tls: Start TLS encrypted connections (TCP) + * @concat: Enabled Secure channel concatenation (TCP) * @disable_sqflow: disable controller sq flow control * @hdr_digest: generate/verify header digest (TCP) * @data_digest: generate/verify data digest (TCP) @@ -137,6 +139,7 @@ struct nvmf_ctrl_options { struct key *keyring; struct key *tls_key; bool tls; + bool concat; bool disable_sqflow; bool hdr_digest; bool data_digest; diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c index 07ec26de2d91..edd96f501d8a 100644 --- a/drivers/nvme/host/sysfs.c +++ b/drivers/nvme/host/sysfs.c @@ -675,6 +675,8 @@ static ssize_t tls_key_show(struct device *dev, if (!key) return 0; + if (ctrl->opts->concat) + return 0; if (test_bit(KEY_FLAG_REVOKED, &key->flags) || test_bit(KEY_FLAG_INVALIDATED, &key->flags)) return -EKEYREVOKED; diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 66675b2dc197..94152ded123a 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -219,7 +219,7 @@ static inline bool nvme_tcp_tls_configured(struct nvme_ctrl *ctrl) if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) return 0; - return ctrl->opts->tls; + return ctrl->opts->tls || ctrl->opts->concat; } static inline struct blk_mq_tags *nvme_tcp_tagset(struct nvme_tcp_queue *queue) @@ -1941,7 +1941,7 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl) if (nvme_tcp_tls_configured(ctrl)) { if (ctrl->opts->tls_key) pskid = key_serial(ctrl->opts->tls_key); - else { + else if (ctrl->opts->tls) { pskid = nvme_tls_psk_default(ctrl->opts->keyring, ctrl->opts->host->nqn, ctrl->opts->subsysnqn); @@ -1973,12 +1973,28 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) key_serial_t pskid = ctrl->tls_key ? key_serial(ctrl->tls_key) : 0; int i, ret; - if (nvme_tcp_tls_configured(ctrl) && !pskid) { - dev_err(ctrl->device, "no PSK negotiated\n"); - return -ENOKEY; + if (nvme_tcp_tls_configured(ctrl)) { + if (ctrl->opts->concat) { + /* + * The generated PSK is stored in the + * fabric options + */ + if (!ctrl->opts->tls_key) { + dev_err(ctrl->device, "no PSK generated\n"); + return -ENOKEY; + } + if (pskid && pskid != key_serial(ctrl->opts->tls_key)) { + dev_err(ctrl->device, "Stale PSK id %08x\n", pskid); + pskid = 0; + } + } else if (!pskid) { + dev_err(ctrl->device, "no PSK negotiated\n"); + return -ENOKEY; + } } for (i = 1; i < ctrl->queue_count; i++) { + WARN_ON(nvme_tcp_tls_enabled(&tcp_ctrl->queues[i])); ret = nvme_tcp_alloc_queue(ctrl, i, pskid); if (ret) goto out_free_queues; @@ -2203,6 +2219,26 @@ static void nvme_tcp_reconnect_or_remove(struct nvme_ctrl *ctrl) } } +static void nvme_tcp_revoke_generated_tls_key(struct nvme_ctrl *ctrl) +{ + if (!ctrl->opts->concat) + return; + /* No key generated, nothing to do */ + if (!ctrl->opts->tls_key) + return; + /* + * Key generated, and TLS enabled: + * Revoke the generated key. + */ + if (ctrl->tls_key) { + dev_dbg(ctrl->device, "Wipe generated TLS PSK %08x\n", + key_serial(ctrl->opts->tls_key)); + key_revoke(ctrl->opts->tls_key); + key_put(ctrl->opts->tls_key); + ctrl->opts->tls_key = NULL; + } +} + static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) { struct nvmf_ctrl_options *opts = ctrl->opts; @@ -2304,6 +2340,7 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work) struct nvme_tcp_ctrl, err_work); struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl; + nvme_tcp_revoke_generated_tls_key(ctrl); nvme_stop_keep_alive(ctrl); flush_work(&ctrl->async_event_work); nvme_tcp_teardown_io_queues(ctrl, false); @@ -2343,6 +2380,7 @@ static void nvme_reset_ctrl_work(struct work_struct *work) struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, reset_work); + nvme_tcp_revoke_generated_tls_key(ctrl); nvme_stop_ctrl(ctrl); nvme_tcp_teardown_ctrl(ctrl, false); @@ -2632,6 +2670,9 @@ static int nvme_tcp_get_address(struct nvme_ctrl *ctrl, char *buf, int size) len = nvmf_get_address(ctrl, buf, size); + if (ctrl->state != NVME_CTRL_LIVE) + return len; + mutex_lock(&queue->queue_lock); if (!test_bit(NVME_TCP_Q_LIVE, &queue->flags)) @@ -2817,7 +2858,7 @@ static struct nvmf_transport_ops nvme_tcp_transport = { NVMF_OPT_HDR_DIGEST | NVMF_OPT_DATA_DIGEST | NVMF_OPT_NR_WRITE_QUEUES | NVMF_OPT_NR_POLL_QUEUES | NVMF_OPT_TOS | NVMF_OPT_HOST_IFACE | NVMF_OPT_TLS | - NVMF_OPT_KEYRING | NVMF_OPT_TLS_KEY, + NVMF_OPT_KEYRING | NVMF_OPT_TLS_KEY | NVMF_OPT_CONCAT, .create_ctrl = nvme_tcp_create_ctrl, }; diff --git a/include/linux/nvme.h b/include/linux/nvme.h index ce0c1143c7e4..b29310424ee1 100644 --- a/include/linux/nvme.h +++ b/include/linux/nvme.h @@ -1663,6 +1663,13 @@ enum { NVME_AUTH_DHGROUP_INVALID = 0xff, }; +enum { + NVME_AUTH_SECP_NOSC = 0x00, + NVME_AUTH_SECP_SC = 0x01, + NVME_AUTH_SECP_NEWTLSPSK = 0x02, + NVME_AUTH_SECP_REPLACETLSPSK = 0x03, +}; + union nvmf_auth_protocol { struct nvmf_auth_dhchap_protocol_descriptor dhchap; }; -- 2.35.3
Print a newline for easier userspace handling. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c index 1f9e57fbfee3..07ec26de2d91 100644 --- a/drivers/nvme/host/sysfs.c +++ b/drivers/nvme/host/sysfs.c @@ -679,7 +679,7 @@ static ssize_t tls_key_show(struct device *dev, test_bit(KEY_FLAG_INVALIDATED, &key->flags)) return -EKEYREVOKED; - return sysfs_emit(buf, "%08x", key_serial(key)); + return sysfs_emit(buf, "%08x\n", key_serial(key)); } static DEVICE_ATTR_RO(tls_key); #endif -- 2.35.3
nvme_tcp_teardown_io_queues() is for I/O queues; the admin queue should be left untouched here. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/tcp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 7018dc0dd026..66675b2dc197 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2173,7 +2173,6 @@ static void nvme_tcp_teardown_io_queues(struct nvme_ctrl *ctrl, { if (ctrl->queue_count <= 1) return; - nvme_quiesce_admin_queue(ctrl); nvme_quiesce_io_queues(ctrl); nvme_sync_io_queues(ctrl); nvme_tcp_stop_io_queues(ctrl); -- 2.35.3
From: Hannes Reinecke <hare@suse.de> There is a difference between TLS configured (ie the user has provisioned/requested a key) and TLS enabled (ie the connection is encrypted with TLS). This becomes important for secure concatenation, where the initial authentication is run unencrypted (ie with TLS configured, but not enabled), and then the queue is reset to run over TLS (ie TLS configured _and_ enabled). So to differentiate between those two states store the provisioned key in opts->tls_key (as we're using the same TLS key for all queues) and only the key serial of the key negotiated by the TLS handshake in queue->tls_pskid. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/core.c | 1 - drivers/nvme/host/sysfs.c | 2 +- drivers/nvme/host/tcp.c | 54 +++++++++++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 2baf5786a92f..9b601655f423 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4567,7 +4567,6 @@ static void nvme_free_ctrl(struct device *dev) if (!subsys || ctrl->instance != subsys->instance) ida_free(&nvme_instance_ida, ctrl->instance); - key_put(ctrl->tls_key); nvme_free_cels(ctrl); nvme_mpath_uninit(ctrl); nvme_auth_stop(ctrl); diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c index ec581608f16c..1f9e57fbfee3 100644 --- a/drivers/nvme/host/sysfs.c +++ b/drivers/nvme/host/sysfs.c @@ -671,7 +671,7 @@ static ssize_t tls_key_show(struct device *dev, struct device_attribute *attr, char *buf) { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); - struct key *key = ctrl->tls_key; + struct key *key = ctrl->opts->tls_key; if (!key) return 0; diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 4a58886e1354..7018dc0dd026 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -163,6 +163,7 @@ struct nvme_tcp_queue { __le32 recv_ddgst; struct completion tls_complete; int tls_err; + key_serial_t tls_pskid; struct page_frag_cache pf_cache; void (*state_change)(struct sock *); @@ -205,7 +206,15 @@ static inline int nvme_tcp_queue_id(struct nvme_tcp_queue *queue) return queue - queue->ctrl->queues; } -static inline bool nvme_tcp_tls(struct nvme_ctrl *ctrl) +static inline bool nvme_tcp_tls_enabled(struct nvme_tcp_queue *queue) +{ + if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) + return 0; + + return (queue->tls_pskid != 0); +} + +static inline bool nvme_tcp_tls_configured(struct nvme_ctrl *ctrl) { if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) return 0; @@ -1418,7 +1427,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue) memset(&msg, 0, sizeof(msg)); iov.iov_base = icresp; iov.iov_len = sizeof(*icresp); - if (nvme_tcp_tls(&queue->ctrl->ctrl)) { + if (nvme_tcp_tls_enabled(queue)) { msg.msg_control = cbuf; msg.msg_controllen = sizeof(cbuf); } @@ -1430,7 +1439,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue) goto free_icresp; } ret = -ENOTCONN; - if (nvme_tcp_tls(&queue->ctrl->ctrl)) { + if (nvme_tcp_tls_enabled(queue)) { ctype = tls_get_record_type(queue->sock->sk, (struct cmsghdr *)cbuf); if (ctype != TLS_RECORD_TYPE_DATA) { @@ -1581,7 +1590,11 @@ static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid) key_put(tls_key); queue->tls_err = -EKEYREVOKED; } else { - ctrl->ctrl.tls_key = tls_key; + queue->tls_pskid = key_serial(tls_key); + if (qid == 0) + ctrl->ctrl.tls_key = tls_key; + else + key_put(tls_key); queue->tls_err = 0; } @@ -1762,7 +1775,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, } /* If PSKs are configured try to start TLS */ - if (IS_ENABLED(CONFIG_NVME_TCP_TLS) && pskid) { + if (nvme_tcp_tls_configured(nctrl) && pskid) { ret = nvme_tcp_start_tls(nctrl, queue, pskid); if (ret) goto err_init_connect; @@ -1823,6 +1836,12 @@ static void nvme_tcp_stop_queue(struct nvme_ctrl *nctrl, int qid) mutex_lock(&queue->queue_lock); if (test_and_clear_bit(NVME_TCP_Q_LIVE, &queue->flags)) __nvme_tcp_stop_queue(queue); + /* Stopping the queue will disable TLS, so clear the PSK ID */ + if (queue->tls_pskid) { + dev_dbg(nctrl->device, "queue %d clear TLS PSK %08x\n", + qid, queue->tls_pskid); + queue->tls_pskid = 0; + } mutex_unlock(&queue->queue_lock); } @@ -1919,16 +1938,17 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl) int ret; key_serial_t pskid = 0; - if (nvme_tcp_tls(ctrl)) { + if (nvme_tcp_tls_configured(ctrl)) { if (ctrl->opts->tls_key) pskid = key_serial(ctrl->opts->tls_key); - else + else { pskid = nvme_tls_psk_default(ctrl->opts->keyring, ctrl->opts->host->nqn, ctrl->opts->subsysnqn); - if (!pskid) { - dev_err(ctrl->device, "no valid PSK found\n"); - return -ENOKEY; + if (!pskid) { + dev_err(ctrl->device, "no valid PSK found\n"); + return -ENOKEY; + } } } @@ -1949,15 +1969,17 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl) static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) { + struct nvme_tcp_ctrl *tcp_ctrl = to_tcp_ctrl(ctrl); + key_serial_t pskid = ctrl->tls_key ? key_serial(ctrl->tls_key) : 0; int i, ret; - if (nvme_tcp_tls(ctrl) && !ctrl->tls_key) { + if (nvme_tcp_tls_configured(ctrl) && !pskid) { dev_err(ctrl->device, "no PSK negotiated\n"); return -ENOKEY; } + for (i = 1; i < ctrl->queue_count; i++) { - ret = nvme_tcp_alloc_queue(ctrl, i, - key_serial(ctrl->tls_key)); + ret = nvme_tcp_alloc_queue(ctrl, i, pskid); if (ret) goto out_free_queues; } @@ -2138,6 +2160,12 @@ static void nvme_tcp_teardown_admin_queue(struct nvme_ctrl *ctrl, if (remove) nvme_unquiesce_admin_queue(ctrl); nvme_tcp_destroy_admin_queue(ctrl, remove); + if (ctrl->tls_key) { + dev_dbg(ctrl->device, "Wipe negotiated TLS_PSK %08x\n", + key_serial(ctrl->tls_key)); + key_put(ctrl->tls_key); + ctrl->tls_key = NULL; + } } static void nvme_tcp_teardown_io_queues(struct nvme_ctrl *ctrl, -- 2.35.3
From: Hannes Reinecke <hare@suse.de> Add a function to refresh a generated PSK in the specified keyring. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/common/keyring.c | 50 +++++++++++++++++++++++++++++++++++ include/linux/nvme-keyring.h | 7 +++++ 2 files changed, 57 insertions(+) diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c index 2beac89b2246..d0e30e64d130 100644 --- a/drivers/nvme/common/keyring.c +++ b/drivers/nvme/common/keyring.c @@ -102,6 +102,56 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring, return key_ref_to_ptr(keyref); } +struct key *nvme_tls_psk_refresh(struct key *keyring, char *hostnqn, char *subnqn, + u8 hmac_id, bool generated, u8 *data, size_t data_len, char *digest) +{ + key_perm_t keyperm = + KEY_POS_SEARCH | KEY_POS_VIEW | KEY_POS_READ | + KEY_POS_WRITE | KEY_POS_LINK | KEY_POS_SETATTR | + KEY_USR_SEARCH | KEY_USR_VIEW | KEY_USR_READ; + char *identity; + size_t identity_len = (NVMF_NQN_SIZE) * 2 + 77; + key_ref_t keyref; + key_serial_t keyring_id; + struct key *key; + + if (!hostnqn || !subnqn || !data || !data_len) + return ERR_PTR(-EINVAL); + + identity = kzalloc(identity_len, GFP_KERNEL); + if (!identity) + return ERR_PTR(-ENOMEM); + + snprintf(identity, identity_len, "NVMe1%c%02d %s %s %s", + generated ? 'G' : 'R', hmac_id, hostnqn, subnqn, digest); + + if (!keyring) + keyring = nvme_keyring; + keyring_id = key_serial(keyring); + pr_debug("keyring %x refresh tls psk '%s'\n", + keyring_id, identity); + keyref = key_create_or_update(make_key_ref(keyring, true), + "psk", identity, data, data_len, + keyperm, KEY_ALLOC_NOT_IN_QUOTA | + KEY_ALLOC_BUILT_IN | + KEY_ALLOC_BYPASS_RESTRICTION); + if (IS_ERR(keyref)) { + pr_debug("refresh tls psk '%s' failed, error %ld\n", + identity, PTR_ERR(keyref)); + kfree(identity); + return ERR_PTR(-ENOKEY); + } + kfree(identity); + /* + * Set the default timeout to 1 hour + * as suggested in TP8018. + */ + key = key_ref_to_ptr(keyref); + key_set_timeout(key, 3600); + return key; +} +EXPORT_SYMBOL_GPL(nvme_tls_psk_refresh); + /* * NVMe PSK priority list * diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h index e10333d78dbb..d2b8a0783ad7 100644 --- a/include/linux/nvme-keyring.h +++ b/include/linux/nvme-keyring.h @@ -8,6 +8,8 @@ #if IS_ENABLED(CONFIG_NVME_KEYRING) +struct key *nvme_tls_psk_refresh(struct key *keyring, char *hostnqn, char *subnqn, + u8 hmac_id, bool generated, u8 *data, size_t data_len, char *digest); key_serial_t nvme_tls_psk_default(struct key *keyring, const char *hostnqn, const char *subnqn); @@ -15,6 +17,11 @@ key_serial_t nvme_keyring_id(void); #else +static struct key *nvme_tls_psk_refresh(struct key *keyring, char *hostnqn, char *subnqn, + u8 hmac_id, bool generated, u8 *data, size_t data_len, char *digest) +{ + return -ENOTSUPP; +} static inline key_serial_t nvme_tls_psk_default(struct key *keyring, const char *hostnqn, const char *subnqn) { -- 2.35.3
From: Hannes Reinecke <hare@suse.de> Add a function to derive the TLS PSK as specified TP8018. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/common/auth.c | 71 ++++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 1 + 2 files changed, 72 insertions(+) diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c index f56f08a4461e..90d525afc240 100644 --- a/drivers/nvme/common/auth.c +++ b/drivers/nvme/common/auth.c @@ -652,5 +652,76 @@ u8 *nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len, } EXPORT_SYMBOL_GPL(nvme_auth_generate_digest); +u8 *nvme_auth_derive_tls_psk(int hmac_id, u8 *psk, size_t psk_len, u8 *psk_digest) +{ + struct crypto_shash *hmac_tfm; + const char *hmac_name; + const char *psk_prefix = "tls13 nvme-tls-psk"; + size_t info_len, prk_len; + char *info; + unsigned char *prk, *tls_key; + int ret; + + hmac_name = nvme_auth_hmac_name(hmac_id); + if (!hmac_name) { + pr_warn("%s: invalid hash algoritm %d\n", + __func__, hmac_id); + return ERR_PTR(-EINVAL); + } + if (hmac_id == NVME_AUTH_HASH_SHA512) { + pr_warn("%s: unsupported hash algorithm %s\n", + __func__, hmac_name); + return ERR_PTR(-EINVAL); + } + + hmac_tfm = crypto_alloc_shash(hmac_name, 0, 0); + if (IS_ERR(hmac_tfm)) + return (u8 *)hmac_tfm; + + prk_len = crypto_shash_digestsize(hmac_tfm); + prk = kzalloc(prk_len, GFP_KERNEL); + if (!prk) { + ret = -ENOMEM; + goto out_free_shash; + } + + ret = hkdf_extract(hmac_tfm, psk, psk_len, prk); + if (ret) + goto out_free_prk; + + ret = crypto_shash_setkey(hmac_tfm, prk, prk_len); + if (ret) + goto out_free_prk; + + info_len = strlen(psk_digest) + strlen(psk_prefix) + 1; + info = kzalloc(info_len, GFP_KERNEL); + if (!info) + goto out_free_prk; + + memcpy(info, psk_prefix, strlen(psk_prefix)); + memcpy(info + strlen(psk_prefix), psk_digest, strlen(psk_digest)); + + tls_key = kzalloc(psk_len, GFP_KERNEL); + if (!tls_key) { + ret = -ENOMEM; + goto out_free_info; + } + ret = hkdf_expand(hmac_tfm, info, strlen(info), tls_key, psk_len); + if (ret) + goto out_free_key; + +out_free_key: + kfree(tls_key); +out_free_info: + kfree(info); +out_free_prk: + kfree(prk); +out_free_shash: + crypto_free_shash(hmac_tfm); + + return ret ? ERR_PTR(ret) : tls_key; +} +EXPORT_SYMBOL_GPL(nvme_auth_derive_tls_psk); + MODULE_DESCRIPTION("NVMe Authentication framework"); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h index 2cbb9249a8b3..335236fb2b73 100644 --- a/include/linux/nvme-auth.h +++ b/include/linux/nvme-auth.h @@ -44,5 +44,6 @@ u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, u8 *c1, u8 *c2, size_t hash_len, size_t *ret_len); u8 *nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len, char *subsysnqn, char *hostnqn); +u8 *nvme_auth_derive_tls_psk(int hmac_id, u8 *psk, size_t psk_len, u8 *psk_digest); #endif /* _NVME_AUTH_H */ -- 2.35.3
Hi all, here's my attempt to implement secure concatenation for NVMe-of TCP as outlined in TP8018. Secure concatenation means that a TLS PSK is generated from the key material negotiated by the DH-HMAC-CHAP protocol, and the TLS PSK is then used for a subsequent TLS connection. The difference between the original definition of secure concatenation and the method outlined in TP8018 is that with TP8018 the connection is reset after DH-HMAC-CHAP negotiation, and a new connection is setup with the generated TLS PSK. To implement that I have decided on resetting the connection from the nvme-tcp driver after the initial connection has been set up. Another way would have been to offload the connection reset to userspace, and let nvme-cli reset the connection. But that would be a modification to the userspace interface, and hence I didn't go that way. The drawback with this approach is that we'll create all I/O queues before resetting for TLS, even though these queues should never be used. But fixing that requires a larger rewrite of the TCP driver to unify the setup and reconnect paths. So keep it that way for now. As usual, comments and reviews are welcome. Changes to v2: - Fixup reset after dhchap negotiation - Disable namespace scanning on I/O queues after dhchap negotiation - Reworked TLS key handling (again) Changes to the original submission: - Sanitize TLS key handling - Fixup modconfig compilation Hannes Reinecke (17): nvme-keyring: restrict match length for version '1' identifiers nvme-tcp: check for invalidated or revoked key crypto,fs: Separate out hkdf_extract() and hkdf_expand() nvme: add nvme_auth_generate_psk() nvme: add nvme_auth_generate_digest() nvme: add nvme_auth_derive_tls_psk() nvme-keyring: add nvme_tls_psk_refresh() nvme-tcp: sanitize TLS key handling nvme-tcp: do not quiesce admin queue in nvme_tcp_teardown_io_queues() nvme: add a newline to the 'tls_key' sysfs attribute nvme-tcp: request secure channel concatenation nvme-fabrics: reset connection for secure concatenation nvme-tcp: reset after recovery for secure concatenation nvmet-auth: allow to clear DH-HMAC-CHAP keys nvme-target: do not check authentication status for admin commands twice nvme-target: do not check authentication status for I/O commands twice nvmet-tcp: support secure channel concatenation crypto/Makefile | 1 + crypto/hkdf.c | 112 +++++++++++ drivers/nvme/common/auth.c | 252 +++++++++++++++++++++++++ drivers/nvme/common/keyring.c | 84 ++++++++- drivers/nvme/host/auth.c | 108 ++++++++++- drivers/nvme/host/core.c | 9 +- drivers/nvme/host/fabrics.c | 38 +++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/sysfs.c | 11 +- drivers/nvme/host/tcp.c | 132 +++++++++++-- drivers/nvme/target/admin-cmd.c | 3 +- drivers/nvme/target/auth.c | 82 +++++++- drivers/nvme/target/core.c | 3 - drivers/nvme/target/fabrics-cmd-auth.c | 46 ++++- drivers/nvme/target/fabrics-cmd.c | 29 ++- drivers/nvme/target/nvmet.h | 30 ++- drivers/nvme/target/tcp.c | 34 +++- fs/crypto/hkdf.c | 68 +------ include/crypto/hkdf.h | 18 ++ include/linux/nvme-auth.h | 5 + include/linux/nvme-keyring.h | 7 + include/linux/nvme.h | 7 + 22 files changed, 958 insertions(+), 124 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h -- 2.35.3
From: Hannes Reinecke <hare@suse.de> Add a function to calculate the PSK digest as specified in TP8018. Signed-off-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> --- drivers/nvme/common/auth.c | 105 +++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 2 + 2 files changed, 107 insertions(+) diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c index 7a4b6589351d..f56f08a4461e 100644 --- a/drivers/nvme/common/auth.c +++ b/drivers/nvme/common/auth.c @@ -547,5 +547,110 @@ u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, } EXPORT_SYMBOL_GPL(nvme_auth_generate_psk); +u8 *nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len, + char *subsysnqn, char *hostnqn) +{ + struct crypto_shash *tfm; + struct shash_desc *shash; + u8 *digest, *hmac; + const char *hmac_name; + size_t digest_len, hmac_len; + int ret; + + if (WARN_ON(!subsysnqn || !hostnqn)) + return ERR_PTR(-EINVAL); + + hmac_name = nvme_auth_hmac_name(hmac_id); + if (!hmac_name) { + pr_warn("%s: invalid hash algoritm %d\n", + __func__, hmac_id); + return ERR_PTR(-EINVAL); + } + + switch (nvme_auth_hmac_hash_len(hmac_id)) { + case 32: + hmac_len = 44; + break; + case 48: + hmac_len = 64; + break; + default: + pr_warn("%s: invalid hash algorithm '%s'\n", + __func__, hmac_name); + return ERR_PTR(-EINVAL); + } + + hmac = kzalloc(hmac_len + 1, GFP_KERNEL); + if (!hmac) + return ERR_PTR(-ENOMEM); + + tfm = crypto_alloc_shash(hmac_name, 0, 0); + if (IS_ERR(tfm)) + goto out_free_hmac; + + digest_len = crypto_shash_digestsize(tfm); + digest = kzalloc(digest_len, GFP_KERNEL); + if (!digest) { + ret = -ENOMEM; + goto out_free_tfm; + } + + shash = kmalloc(sizeof(struct shash_desc) + + crypto_shash_descsize(tfm), + GFP_KERNEL); + if (!shash) { + ret = -ENOMEM; + goto out_free_digest; + } + + shash->tfm = tfm; + ret = crypto_shash_setkey(tfm, psk, psk_len); + if (ret) + goto out_free_shash; + + ret = crypto_shash_init(shash); + if (ret) + goto out_free_shash; + + ret = crypto_shash_update(shash, hostnqn, strlen(hostnqn)); + if (ret) + goto out_free_shash; + + ret = crypto_shash_update(shash, " ", 1); + if (ret) + goto out_free_shash; + + ret = crypto_shash_update(shash, subsysnqn, strlen(subsysnqn)); + if (ret) + goto out_free_shash; + + ret = crypto_shash_update(shash, " NVMe-over-Fabrics", 18); + if (ret) + goto out_free_shash; + + ret = crypto_shash_final(shash, digest); + if (ret) + goto out_free_shash; + + ret = base64_encode(digest, digest_len, hmac); + if (ret < hmac_len) + ret = -ENOKEY; + ret = 0; + +out_free_shash: + kfree_sensitive(shash); +out_free_digest: + kfree_sensitive(digest); +out_free_tfm: + crypto_free_shash(tfm); +out_free_hmac: + if (ret) { + kfree_sensitive(hmac); + hmac = NULL; + } + return ret ? ERR_PTR(ret) : hmac; +} +EXPORT_SYMBOL_GPL(nvme_auth_generate_digest); + MODULE_DESCRIPTION("NVMe Authentication framework"); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h index 31dc1db2d4d6..2cbb9249a8b3 100644 --- a/include/linux/nvme-auth.h +++ b/include/linux/nvme-auth.h @@ -42,5 +42,7 @@ int nvme_auth_gen_shared_secret(struct crypto_kpp *dh_tfm, u8 *sess_key, size_t sess_key_len); u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, u8 *c1, u8 *c2, size_t hash_len, size_t *ret_len); +u8 *nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len, + char *subsysnqn, char *hostnqn); #endif /* _NVME_AUTH_H */ -- 2.35.3
From: Hannes Reinecke <hare@suse.de> Add a function to generate a NVMe PSK from the shared credentials negotiated by DH-HMAC-CHAP. Signed-off-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> --- drivers/nvme/common/auth.c | 76 ++++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 2 + 2 files changed, 78 insertions(+) diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c index a3455f1d67fa..7a4b6589351d 100644 --- a/drivers/nvme/common/auth.c +++ b/drivers/nvme/common/auth.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <crypto/hash.h> #include <crypto/dh.h> +#include <crypto/hkdf.h> #include <linux/nvme.h> #include <linux/nvme-auth.h> @@ -471,5 +472,80 @@ int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key) } EXPORT_SYMBOL_GPL(nvme_auth_generate_key); +u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, + u8 *c1, u8 *c2, size_t hash_len, size_t *ret_len) +{ + struct crypto_shash *tfm; + struct shash_desc *shash; + u8 *psk; + const char *hmac_name; + int ret, psk_len; + + if (!c1 || !c2) { + pr_warn("%s: invalid parameter\n", __func__); + return ERR_PTR(-EINVAL); + } + + hmac_name = nvme_auth_hmac_name(hmac_id); + if (!hmac_name) { + pr_warn("%s: invalid hash algoritm %d\n", + __func__, hmac_id); + return ERR_PTR(-EINVAL); + } + + tfm = crypto_alloc_shash(hmac_name, 0, 0); + if (IS_ERR(tfm)) + return (u8 *)tfm; + + psk_len = crypto_shash_digestsize(tfm); + psk = kzalloc(psk_len, GFP_KERNEL); + if (!psk) { + ret = -ENOMEM; + goto out_free_tfm; + } + + shash = kmalloc(sizeof(struct shash_desc) + + crypto_shash_descsize(tfm), + GFP_KERNEL); + if (!shash) { + ret = -ENOMEM; + goto out_free_psk; + } + + shash->tfm = tfm; + ret = crypto_shash_setkey(tfm, skey, skey_len); + if (ret) + goto out_free_shash; + + ret = crypto_shash_init(shash); + if (ret) + goto out_free_shash; + + ret = crypto_shash_update(shash, c1, hash_len); + if (ret) + goto out_free_shash; + + ret = crypto_shash_update(shash, c2, hash_len); + if (ret) + goto out_free_shash; + + ret = crypto_shash_final(shash, psk); + if (!ret) + *ret_len = psk_len; + +out_free_shash: + kfree_sensitive(shash); +out_free_psk: + if (ret) { + kfree_sensitive(psk); + psk = NULL; + } +out_free_tfm: + crypto_free_shash(tfm); + + return ret ? ERR_PTR(ret) : psk; +} +EXPORT_SYMBOL_GPL(nvme_auth_generate_psk); + MODULE_DESCRIPTION("NVMe Authentication framework"); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h index c1d0bc5d9624..31dc1db2d4d6 100644 --- a/include/linux/nvme-auth.h +++ b/include/linux/nvme-auth.h @@ -40,5 +40,7 @@ int nvme_auth_gen_pubkey(struct crypto_kpp *dh_tfm, int nvme_auth_gen_shared_secret(struct crypto_kpp *dh_tfm, u8 *ctrl_key, size_t ctrl_key_len, u8 *sess_key, size_t sess_key_len); +u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, + u8 *c1, u8 *c2, size_t hash_len, size_t *ret_len); #endif /* _NVME_AUTH_H */ -- 2.35.3
From: Hannes Reinecke <hare@suse.de> Separate out the HKDF functions into a separate file to make them available to other callers. Signed-off-by: Hannes Reinecke <hare@suse.de> --- crypto/Makefile | 1 + crypto/hkdf.c | 112 ++++++++++++++++++++++++++++++++++++++++++ fs/crypto/hkdf.c | 68 ++++--------------------- include/crypto/hkdf.h | 18 +++++++ 4 files changed, 140 insertions(+), 59 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h diff --git a/crypto/Makefile b/crypto/Makefile index 408f0a1f9ab9..3c0e1916de7c 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_CRYPTO_ECHAINIV) += echainiv.o crypto_hash-y += ahash.o crypto_hash-y += shash.o +crypto_hash-y += hkdf.o obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o diff --git a/crypto/hkdf.c b/crypto/hkdf.c new file mode 100644 index 000000000000..22e343851c0b --- /dev/null +++ b/crypto/hkdf.c @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Implementation of HKDF ("HMAC-based Extract-and-Expand Key Derivation + * Function"), aka RFC 5869. See also the original paper (Krawczyk 2010): + * "Cryptographic Extraction and Key Derivation: The HKDF Scheme". + * + * This is used to derive keys from the fscrypt master keys. + * + * Copyright 2019 Google LLC + */ + +#include <crypto/hash.h> +#include <crypto/sha2.h> +#include <crypto/hkdf.h> + +/* + * HKDF consists of two steps: + * + * 1. HKDF-Extract: extract a pseudorandom key of length HKDF_HASHLEN bytes from + * the input keying material and optional salt. + * 2. HKDF-Expand: expand the pseudorandom key into output keying material of + * any length, parameterized by an application-specific info string. + * + */ + +/* HKDF-Extract (RFC 5869 section 2.2), unsalted */ +int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm, + unsigned int ikmlen, u8 *prk) +{ + unsigned int prklen = crypto_shash_digestsize(hmac_tfm); + u8 *default_salt; + int err; + + default_salt = kzalloc(prklen, GFP_KERNEL); + if (!default_salt) + return -ENOMEM; + err = crypto_shash_setkey(hmac_tfm, default_salt, prklen); + if (!err) + err = crypto_shash_tfm_digest(hmac_tfm, ikm, ikmlen, prk); + + kfree(default_salt); + return err; +} +EXPORT_SYMBOL_GPL(hkdf_extract); + +/* + * HKDF-Expand (RFC 5869 section 2.3). + * This expands the pseudorandom key, which was already keyed into @hmac_tfm, + * into @okmlen bytes of output keying material parameterized by the + * application-specific @info of length @infolen bytes. + * This is thread-safe and may be called by multiple threads in parallel. + */ +int hkdf_expand(struct crypto_shash *hmac_tfm, + const u8 *info, unsigned int infolen, + u8 *okm, unsigned int okmlen) +{ + SHASH_DESC_ON_STACK(desc, hmac_tfm); + unsigned int i, hashlen = crypto_shash_digestsize(hmac_tfm); + int err; + const u8 *prev = NULL; + u8 counter = 1; + u8 *tmp; + + if (WARN_ON(okmlen > 255 * hashlen)) + return -EINVAL; + + tmp = kzalloc(hashlen, GFP_KERNEL); + if (!tmp) + return -ENOMEM; + + desc->tfm = hmac_tfm; + + for (i = 0; i < okmlen; i += hashlen) { + + err = crypto_shash_init(desc); + if (err) + goto out; + + if (prev) { + err = crypto_shash_update(desc, prev, hashlen); + if (err) + goto out; + } + + err = crypto_shash_update(desc, info, infolen); + if (err) + goto out; + + BUILD_BUG_ON(sizeof(counter) != 1); + if (okmlen - i < hashlen) { + err = crypto_shash_finup(desc, &counter, 1, tmp); + if (err) + goto out; + memcpy(&okm[i], tmp, okmlen - i); + memzero_explicit(tmp, sizeof(tmp)); + } else { + err = crypto_shash_finup(desc, &counter, 1, &okm[i]); + if (err) + goto out; + } + counter++; + prev = &okm[i]; + } + err = 0; +out: + if (unlikely(err)) + memzero_explicit(okm, okmlen); /* so caller doesn't need to */ + shash_desc_zero(desc); + kfree(tmp); + return err; +} +EXPORT_SYMBOL_GPL(hkdf_expand); diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c index 5a384dad2c72..9c2f9aca9412 100644 --- a/fs/crypto/hkdf.c +++ b/fs/crypto/hkdf.c @@ -11,6 +11,7 @@ #include <crypto/hash.h> #include <crypto/sha2.h> +#include <crypto/hkdf.h> #include "fscrypt_private.h" @@ -44,20 +45,6 @@ * there's no way to persist a random salt per master key from kernel mode. */ -/* HKDF-Extract (RFC 5869 section 2.2), unsalted */ -static int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm, - unsigned int ikmlen, u8 prk[HKDF_HASHLEN]) -{ - static const u8 default_salt[HKDF_HASHLEN]; - int err; - - err = crypto_shash_setkey(hmac_tfm, default_salt, HKDF_HASHLEN); - if (err) - return err; - - return crypto_shash_tfm_digest(hmac_tfm, ikm, ikmlen, prk); -} - /* * Compute HKDF-Extract using the given master key as the input keying material, * and prepare an HMAC transform object keyed by the resulting pseudorandom key. @@ -118,61 +105,24 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context, u8 *okm, unsigned int okmlen) { SHASH_DESC_ON_STACK(desc, hkdf->hmac_tfm); - u8 prefix[9]; - unsigned int i; + u8 *prefix; int err; - const u8 *prev = NULL; - u8 counter = 1; - u8 tmp[HKDF_HASHLEN]; if (WARN_ON_ONCE(okmlen > 255 * HKDF_HASHLEN)) return -EINVAL; + prefix = kzalloc(okmlen + 9, GFP_KERNEL); + if (!prefix) + return -ENOMEM; desc->tfm = hkdf->hmac_tfm; memcpy(prefix, "fscrypt\0", 8); prefix[8] = context; + memcpy(prefix + 9, info, infolen); - for (i = 0; i < okmlen; i += HKDF_HASHLEN) { - - err = crypto_shash_init(desc); - if (err) - goto out; - - if (prev) { - err = crypto_shash_update(desc, prev, HKDF_HASHLEN); - if (err) - goto out; - } - - err = crypto_shash_update(desc, prefix, sizeof(prefix)); - if (err) - goto out; - - err = crypto_shash_update(desc, info, infolen); - if (err) - goto out; - - BUILD_BUG_ON(sizeof(counter) != 1); - if (okmlen - i < HKDF_HASHLEN) { - err = crypto_shash_finup(desc, &counter, 1, tmp); - if (err) - goto out; - memcpy(&okm[i], tmp, okmlen - i); - memzero_explicit(tmp, sizeof(tmp)); - } else { - err = crypto_shash_finup(desc, &counter, 1, &okm[i]); - if (err) - goto out; - } - counter++; - prev = &okm[i]; - } - err = 0; -out: - if (unlikely(err)) - memzero_explicit(okm, okmlen); /* so caller doesn't need to */ - shash_desc_zero(desc); + err = hkdf_expand(hkdf->hmac_tfm, prefix, infolen + 8, + okm, okmlen); + kfree(prefix); return err; } diff --git a/include/crypto/hkdf.h b/include/crypto/hkdf.h new file mode 100644 index 000000000000..bf06c080d7ed --- /dev/null +++ b/include/crypto/hkdf.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * HKDF: HMAC-based Key Derivation Function (HKDF), RFC 5869 + * + * Extracted from fs/crypto/hkdf.c, which has + * Copyright 2019 Google LLC + */ + +#ifndef _CRYPTO_HKDF_H +#define _CRYPTO_HKDF_H + +int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm, + unsigned int ikmlen, u8 *prk); +int hkdf_expand(struct crypto_shash *hmac_tfm, + const u8 *info, unsigned int infolen, + u8 *okm, unsigned int okmlen); + +#endif -- 2.35.3