lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <615aaad8-b216-ceaa-1553-d283a3c4531b@grimberg.me> Date: Thu, 17 Aug 2023 13:41:02 +0300 From: Sagi Grimberg <sagi@...mberg.me> To: Hannes Reinecke <hare@...e.de>, Christoph Hellwig <hch@....de> Cc: Keith Busch <kbusch@...nel.org>, linux-nvme@...ts.infradead.org, Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org Subject: Re: [PATCH 11/18] nvme-fabrics: parse options 'keyring' and 'tls_key' >>> + case NVMF_OPT_KEYRING: >>> + if (match_int(args, &key_id) || key_id <= 0) { >>> + ret = -EINVAL; >>> + goto out; >>> + } >>> + key = nvmf_parse_key(key_id, "Keyring"); >>> + if (IS_ERR(key)) { >>> + ret = PTR_ERR(key); >>> + goto out; >>> + } >>> + key_put(opts->keyring); >> >> Don't understand how keyring/tls_key are pre-populated though... >> > They are not. But they might, as there's nothing in the code preventing > the user to specify 'keyring' or 'tls_key' several times. > I can make it one-shot and error out if they are already populated, but > really haven't seen the need. OK, now I understand. its fine. > >>> + opts->keyring = key; >>> + break; >>> + case NVMF_OPT_TLS_KEY: >>> + if (match_int(args, &key_id) || key_id <= 0) { >>> + ret = -EINVAL; >>> + goto out; >>> + } >>> + key = nvmf_parse_key(key_id, "Key"); >>> + if (IS_ERR(key)) { >>> + ret = PTR_ERR(key); >>> + goto out; >>> + } >>> + key_put(opts->tls_key); >>> + opts->tls_key = key; >>> + break; >>> case NVMF_OPT_DISCOVERY: >>> opts->discovery_nqn = true; >>> break; >>> @@ -1168,6 +1216,8 @@ static int nvmf_check_allowed_opts(struct >>> nvmf_ctrl_options *opts, >>> void nvmf_free_options(struct nvmf_ctrl_options *opts) >>> { >>> nvmf_host_put(opts->host); >>> + key_put(opts->keyring); >>> + key_put(opts->tls_key); >>> kfree(opts->transport); >>> kfree(opts->traddr); >>> kfree(opts->trsvcid); >>> diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h >>> index dac17c3fee26..fbaee5a7be19 100644 >>> --- a/drivers/nvme/host/fabrics.h >>> +++ b/drivers/nvme/host/fabrics.h >>> @@ -71,6 +71,8 @@ enum { >>> NVMF_OPT_DHCHAP_SECRET = 1 << 23, >>> NVMF_OPT_DHCHAP_CTRL_SECRET = 1 << 24, >>> NVMF_OPT_TLS = 1 << 25, >>> + NVMF_OPT_KEYRING = 1 << 26, >>> + NVMF_OPT_TLS_KEY = 1 << 27, >>> }; >>> /** >>> @@ -103,6 +105,8 @@ enum { >>> * @dhchap_secret: DH-HMAC-CHAP secret >>> * @dhchap_ctrl_secret: DH-HMAC-CHAP controller secret for >>> bi-directional >>> * authentication >>> + * @keyring: Keyring to use for key lookups >>> + * @tls_key: TLS key for encrypted connections (TCP) >>> * @tls: Start TLS encrypted connections (TCP) >>> * @disable_sqflow: disable controller sq flow control >>> * @hdr_digest: generate/verify header digest (TCP) >>> @@ -130,6 +134,8 @@ struct nvmf_ctrl_options { >>> struct nvmf_host *host; >>> char *dhchap_secret; >>> char *dhchap_ctrl_secret; >>> + struct key *keyring; >>> + struct key *tls_key; >>> bool tls; >>> bool disable_sqflow; >>> bool hdr_digest; >>> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c >>> index ef9cf8c7a113..f48797fcc4ee 100644 >>> --- a/drivers/nvme/host/tcp.c >>> +++ b/drivers/nvme/host/tcp.c >>> @@ -1589,6 +1589,8 @@ static int nvme_tcp_start_tls(struct nvme_ctrl >>> *nctrl, >>> dev_dbg(nctrl->device, "queue %d: start TLS with key %x\n", >>> qid, pskid); >>> + if (nctrl->opts->keyring) >>> + keyring = key_serial(nctrl->opts->keyring); >> >> Maybe populate opts->keyring with nvme_keyring_id() to begin >> with and then you don't need this? >> > We could; one would lose the distinction between 'not specified' and > 'specified with the defaul keyring', but one could argue whether that > brings any benefit. Not sure I understand. What exactly is the distinction and how is it manifested?
Powered by blists - more mailing lists