Commit 11cc2256 authored by Konrad Sztyber's avatar Konrad Sztyber
Browse files

nvmf/tcp: require TLS PSKs to be specified via keyring 



It's no longer possible to set TLS PSKs by specifying a path to the file
containing the key.  This method was deprecated and was supposed to be
removed in the upcoming release.

Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Idfa4c935bd97f20c020779d90b186607f24bb525
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/24805


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
parent 50f90bc9

deprecation.md

+0 −8
Original line number Diff line number Diff line
@@ -27,14 +27,6 @@ Passing NVMe/TLS pre-shared keys via `spdk_nvme_ctrlr_opts.psk` is deprecated an 
removed in the v24.09 release.  Instead, a key obtained from the keyring library should be passed

in `spdk_nvme_ctrlr_opts.tls_psk`.



### nvmf



#### `nvmf_subsystem_add_host`



The ability to specifying path to a PSK file via the `psk` parameter in `nvmf_subsystem_add_host` is

deprecated and will be removed in the v24.09 release.  Instead, the name of a key attached to the

keyring should be used.



### gpt



#### `old_gpt_guid`

lib/nvmf/tcp.c

+11 −75
Original line number Diff line number Diff line
@@ -48,8 +48,6 @@ 
#define SPDK_NVMF_TCP_DEFAULT_DIF_INSERT_OR_STRIP false

#define SPDK_NVMF_TCP_DEFAULT_ABORT_TIMEOUT_SEC 1



#define TCP_PSK_INVALID_PERMISSIONS 0177



const struct spdk_nvmf_transport_ops spdk_nvmf_transport_tcp;

static bool g_tls_log = false;
@@ -361,9 +359,6 @@ struct tcp_psk_entry { 
	char				pskid[NVMF_PSK_IDENTITY_LEN];

	uint8_t				psk[SPDK_TLS_PSK_MAX_LEN];

	struct spdk_key			*key;



	/* Original path saved to emit SPDK configuration via "save_config". */

	char				psk_path[PATH_MAX];

	uint32_t			psk_size;

	enum nvme_tcp_cipher_suite	tls_cipher_suite;

	TAILQ_ENTRY(tcp_psk_entry)	link;
@@ -3715,45 +3710,6 @@ static const struct spdk_json_object_decoder tcp_subsystem_add_host_opts_decoder 
	{"psk", offsetof(struct tcp_subsystem_add_host_opts, psk), spdk_json_decode_string, true},

};



static int

tcp_load_psk(const char *fname, char *buf, size_t bufsz)

{

	FILE *psk_file;

	struct stat statbuf;

	int rc;



	if (stat(fname, &statbuf) != 0) {

		SPDK_ERRLOG("Could not read permissions for PSK file\n");

		return -EACCES;

	}



	if ((statbuf.st_mode & TCP_PSK_INVALID_PERMISSIONS) != 0) {

		SPDK_ERRLOG("Incorrect permissions for PSK file\n");

		return -EPERM;

	}

	if ((size_t)statbuf.st_size > bufsz) {

		SPDK_ERRLOG("Invalid PSK: too long\n");

		return -EINVAL;

	}

	psk_file = fopen(fname, "r");

	if (psk_file == NULL) {

		SPDK_ERRLOG("Could not open PSK file\n");

		return -EINVAL;

	}



	rc = fread(buf, 1, statbuf.st_size, psk_file);

	if (rc != statbuf.st_size) {

		SPDK_ERRLOG("Failed to read PSK\n");

		fclose(psk_file);

		return -EINVAL;

	}



	fclose(psk_file);

	return 0;

}



SPDK_LOG_DEPRECATION_REGISTER(nvmf_tcp_psk_path, "PSK path", "v24.09", 0);



static int

nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,

			    const struct spdk_nvmf_subsystem *subsystem,
@@ -3798,29 +3754,19 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport, 
	}



	entry->key = spdk_keyring_get_key(opts.psk);

	if (entry->key != NULL) {

		rc = spdk_key_get_key(entry->key, psk_interchange, SPDK_TLS_PSK_MAX_LEN);

		if (rc < 0) {

			SPDK_ERRLOG("Failed to retrieve PSK '%s'\n", opts.psk);

			rc = -EINVAL;

			goto end;

		}

	} else {

		if (strlen(opts.psk) >= sizeof(entry->psk)) {

			SPDK_ERRLOG("PSK path too long\n");

	if (entry->key == NULL) {

		SPDK_ERRLOG("Key '%s' does not exist\n", opts.psk);

		rc = -EINVAL;

		goto end;

	}



		rc = tcp_load_psk(opts.psk, psk_interchange, SPDK_TLS_PSK_MAX_LEN);

		if (rc) {

			SPDK_ERRLOG("Could not retrieve PSK from file\n");

	rc = spdk_key_get_key(entry->key, psk_interchange, SPDK_TLS_PSK_MAX_LEN);

	if (rc < 0) {

		SPDK_ERRLOG("Failed to retrieve PSK '%s'\n", opts.psk);

		rc = -EINVAL;

		goto end;

	}



		SPDK_LOG_DEPRECATED(nvmf_tcp_psk_path);

	}



	/* Parse PSK interchange to get length of base64 encoded data.

	 * This is then used to decide which cipher suite should be used

	 * to generate PSK identity and TLS PSK later on. */
@@ -3890,15 +3836,6 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport, 
		entry->psk_size = rc;

	}



	if (entry->key == NULL) {

		rc = snprintf(entry->psk_path, sizeof(entry->psk_path), "%s", opts.psk);

		if (rc < 0 || (size_t)rc >= sizeof(entry->psk_path)) {

			SPDK_ERRLOG("Could not save PSK path!\n");

			rc = -ENAMETOOLONG;

			goto end;

		}

	}



	TAILQ_INSERT_TAIL(&ttransport->psks, entry, link);

	rc = 0;
@@ -3951,8 +3888,7 @@ nvmf_tcp_subsystem_dump_host(struct spdk_nvmf_transport *transport, 
	TAILQ_FOREACH(entry, &ttransport->psks, link) {

		if ((strncmp(entry->hostnqn, hostnqn, SPDK_NVMF_NQN_MAX_LEN)) == 0 &&

		    (strncmp(entry->subnqn, subsystem->subnqn, SPDK_NVMF_NQN_MAX_LEN)) == 0) {

			spdk_json_write_named_string(w, "psk", entry->key ?

						     spdk_key_get_name(entry->key) : entry->psk_path);

			spdk_json_write_named_string(w, "psk",  spdk_key_get_name(entry->key));

			break;

		}

	}

test/keyring/file.sh

+2 −1
Original line number Diff line number Diff line
@@ -36,7 +36,8 @@ rpc_cmd << CMD 
	bdev_null_create null0 100 4096

	nvmf_subsystem_add_ns "$subnqn" null0

	nvmf_subsystem_add_listener -t tcp -a 127.0.0.1 -s 4420 --secure-channel "$subnqn"

	nvmf_subsystem_add_host --psk "$key0path" "$subnqn" "$hostnqn"

	keyring_file_add_key key0 "$key0path"

	nvmf_subsystem_add_host --psk key0 "$subnqn" "$hostnqn"

CMD



# Test, if another listener cannot be added, with different secure channel value

test/nvmf/fips/fips.sh

+2 −1
Original line number Diff line number Diff line
@@ -28,8 +28,9 @@ setup_nvmf_tgt_conf() { 
		-a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -k

		bdev_malloc_create 32 4096 -b malloc0

		nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode1 malloc0 -n 1

		keyring_file_add_key key0 "$key"

		nvmf_subsystem_add_host nqn.2016-06.io.spdk:cnode1 nqn.2016-06.io.spdk:host1 \

		--psk $key

			--psk key0

	EOF

}

test/nvmf/host/async_init.sh

+2 −1
Original line number Diff line number Diff line
@@ -53,11 +53,12 @@ $rpc_py bdev_nvme_detach_controller $nvme_bdev 
key_path=$(mktemp)

echo -n "NVMeTLSkey-1:01:MDAxMTIyMzM0NDU1NjY3Nzg4OTlhYWJiY2NkZGVlZmZwJEiQ:" > $key_path

chmod 0600 $key_path

$rpc_py keyring_file_add_key key0 "$key_path"

$rpc_py nvmf_subsystem_allow_any_host nqn.2016-06.io.spdk:cnode0 --disable

$rpc_py nvmf_subsystem_add_listener nqn.2016-06.io.spdk:cnode0 -t $TEST_TRANSPORT \

	-a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT --secure-channel

$rpc_py nvmf_subsystem_add_host nqn.2016-06.io.spdk:cnode0 nqn.2016-06.io.spdk:host1 \

	--psk $key_path

	--psk key0



# Then attach NVMe bdev by connecting back to itself, with the target app running on a single core.

# This verifies that the initialization is completely asynchronous, as each blocking call would