Commit 03b6183a authored by Krzysztof Karas's avatar Krzysztof Karas Committed by Konrad Sztyber
Browse files

tcp: change the way we pass PSKs



Read PSKs from a file, instead of passing them
around as command line arguments.
Rename arguments and fields "psk" and "psk_key"
to "psk_path".

Change-Id: I78bc18503879697c21e5650226ccc34d917111ae
Signed-off-by: default avatarKrzysztof Karas <krzysztof.karas@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/18005


Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Community-CI: Mellanox Build Bot
parent 49c4c7db
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -3985,7 +3985,7 @@ num_io_queues | Optional | number | The number of IO queues to
ctrlr_loss_timeout_sec     | Optional | number      | Time to wait until ctrlr is reconnected before deleting ctrlr.  -1 means infinite reconnects. 0 means no reconnect.
reconnect_delay_sec        | Optional | number      | Time to delay a reconnect trial. 0 means no reconnect.
fast_io_fail_timeout_sec   | Optional | number      | Time to wait until ctrlr is reconnected before failing I/O to ctrlr. 0 means no such timeout.
psk                        | Optional | string      | PSK in hexadecimal digits, e.g. 1234567890ABCDEF (Enables SSL socket implementation for TCP)
psk                        | Optional | string      | Path to a file contatining PSK for TLS (Enables SSL socket implementation for TCP)
max_bdevs                  | Optional | number      | The size of the name array for newly created bdevs. Default is 128.

#### Example
@@ -8128,6 +8128,7 @@ Name | Optional | Type | Description
nqn                     | Required | string      | Subsystem NQN
host                    | Required | string      | Host NQN to add to the list of allowed host NQNs
tgt_name                | Optional | string      | Parent NVMe-oF target name.
psk                     | Optional | string      | Path to a file containing PSK for TLS connection

#### Example

+25 −7
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@
#include "spdk/likely.h"
#include "spdk/sock.h"
#include "spdk/zipf.h"
#include "spdk/nvmf.h"

#ifdef SPDK_CONFIG_URING
#include <liburing.h>
@@ -335,16 +336,30 @@ perf_set_sock_opts(const char *impl_name, const char *field, uint32_t val, const
		sock_opts.tls_version = val;
	} else if (strcmp(field, "ktls") == 0) {
		sock_opts.enable_ktls = val;
	} else if (strcmp(field, "psk_key") == 0) {
	} else if (strcmp(field, "psk_path") == 0) {
		if (!valstr) {
			fprintf(stderr, "No socket opts value specified\n");
			return;
		}
		g_psk = strdup(valstr);
		g_psk = calloc(1, SPDK_TLS_PSK_MAX_LEN + 1);
		if (g_psk == NULL) {
			fprintf(stderr, "Failed to allocate memory for psk\n");
			return;
		}
		FILE *psk_file = fopen(valstr, "r");
		if (psk_file == NULL) {
			fprintf(stderr, "Could not open PSK file\n");
			return;
		}
		if (fscanf(psk_file, "%" SPDK_STRINGIFY(SPDK_TLS_PSK_MAX_LEN) "s", g_psk) != 1) {
			fprintf(stderr, "Could not retrieve PSK from file\n");
			fclose(psk_file);
			return;
		}
		if (fclose(psk_file)) {
			fprintf(stderr, "Failed to close PSK file\n");
			return;
		}
	} else if (strcmp(field, "zerocopy_threshold") == 0) {
		sock_opts.zerocopy_threshold = val;
	} else {
@@ -1873,7 +1888,7 @@ usage(char *program_name)
	printf("\t[--disable-ktls disable Kernel TLS. Only valid for ssl impl. Default for ssl impl]\n");
	printf("\t[--enable-ktls enable Kernel TLS. Only valid for ssl impl]\n");
	printf("\t[--tls-version <val> TLS version to use. Only valid for ssl impl. Default: 0 (auto-negotiation)]\n");
	printf("\t[--psk-key <val> Default PSK KEY in hexadecimal digits, e.g. 1234567890ABCDEF (only applies when sock_impl == ssl)]\n");
	printf("\t[--psk-path <val> Path to PSK file (only applies when sock_impl == ssl)]\n");
	printf("\t[--psk-identity <val> Default PSK ID, e.g. psk.spdk.io (only applies when sock_impl == ssl)]\n");
	printf("\t[--zerocopy-threshold <val> data is sent with MSG_ZEROCOPY if size is greater than this val. Default: 0 to disable it]\n");
	printf("\t[--zerocopy-threshold-sock-impl <impl> specify the sock implementation to set zerocopy_threshold]\n");
@@ -2380,8 +2395,8 @@ static const struct option g_perf_cmdline_opts[] = {
	{"enable-ktls", no_argument, NULL, PERF_ENABLE_KTLS},
#define PERF_TLS_VERSION	262
	{"tls-version", required_argument, NULL, PERF_TLS_VERSION},
#define PERF_PSK_KEY		263
	{"psk-key", required_argument, NULL, PERF_PSK_KEY},
#define PERF_PSK_PATH		263
	{"psk-path", required_argument, NULL, PERF_PSK_PATH},
#define PERF_PSK_IDENTITY	264
	{"psk-identity ", required_argument, NULL, PERF_PSK_IDENTITY},
#define PERF_ZEROCOPY_THRESHOLD		265
@@ -2611,9 +2626,9 @@ parse_args(int argc, char **argv, struct spdk_env_opts *env_opts)
			}
			perf_set_sock_opts("ssl", "tls_version", val, NULL);
			break;
		case PERF_PSK_KEY:
		case PERF_PSK_PATH:
			ssl_used = true;
			perf_set_sock_opts("ssl", "psk_key", 0, optarg);
			perf_set_sock_opts("ssl", "psk_path", 0, optarg);
			break;
		case PERF_PSK_IDENTITY:
			ssl_used = true;
@@ -3101,6 +3116,7 @@ main(int argc, char **argv)
	opts.pci_allowed = g_allowed_pci_addr;
	rc = parse_args(argc, argv, &opts);
	if (rc != 0) {
		free(g_psk);
		return rc;
	}
	/* Transport statistics are printed from each thread.
@@ -3108,12 +3124,14 @@ main(int argc, char **argv)
	rc = pthread_mutex_init(&g_stats_mutex, NULL);
	if (rc != 0) {
		fprintf(stderr, "Failed to init mutex\n");
		free(g_psk);
		return -1;
	}
	if (spdk_env_init(&opts) < 0) {
		fprintf(stderr, "Unable to initialize SPDK env\n");
		unregister_trids();
		pthread_mutex_destroy(&g_stats_mutex);
		free(g_psk);
		return -1;
	}

+3 −0
Original line number Diff line number Diff line
@@ -17,6 +17,9 @@
extern "C" {
#endif

#define _SPDK_STRINGIFY(x) #x
#define SPDK_STRINGIFY(x) _SPDK_STRINGIFY(x)

/**
 * sprintf with automatic buffer allocation.
 *
+59 −13
Original line number Diff line number Diff line
@@ -46,6 +46,8 @@
#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;

/* spdk nvmf related structure */
@@ -347,6 +349,9 @@ struct tcp_psk_entry {
	char				subnqn[SPDK_NVMF_NQN_MAX_LEN + 1];
	char				psk_identity[NVMF_PSK_IDENTITY_LEN];
	uint8_t				psk[SPDK_TLS_PSK_MAX_LEN];

	/* 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;
@@ -3514,6 +3519,44 @@ 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;
	}

	memset(buf, 0, bufsz);
	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;
}

static int
nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,
			    const struct spdk_nvmf_subsystem *subsystem,
@@ -3525,8 +3568,8 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,
	struct tcp_psk_entry *entry;
	char psk_identity[NVMF_PSK_IDENTITY_LEN];
	uint8_t psk_configured[SPDK_TLS_PSK_MAX_LEN] = {};
	char psk_interchange[SPDK_TLS_PSK_MAX_LEN] = {};
	uint8_t tls_cipher_suite;
	uint64_t key_len;
	int rc = 0;
	uint8_t psk_retained_hash;
	uint64_t psk_configured_size;
@@ -3540,7 +3583,7 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,

	memset(&opts, 0, sizeof(opts));

	/* Decode PSK */
	/* Decode PSK file path */
	if (spdk_json_decode_object_relaxed(transport_specific, tcp_subsystem_add_host_opts_decoder,
					    SPDK_COUNTOF(tcp_subsystem_add_host_opts_decoder), &opts)) {
		SPDK_ERRLOG("spdk_json_decode_object failed\n");
@@ -3550,11 +3593,22 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,
	if (opts.psk == NULL) {
		return 0;
	}
	if (strlen(opts.psk) >= sizeof(entry->psk)) {
		SPDK_ERRLOG("PSK path too long\n");
		rc = -EINVAL;
		goto end;
	}

	rc = tcp_load_psk(opts.psk, psk_interchange, sizeof(psk_interchange));
	if (rc) {
		SPDK_ERRLOG("Could not retrieve PSK from file\n");
		goto end;
	}

	/* 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. */
	rc = nvme_tcp_parse_interchange_psk(opts.psk, psk_configured, sizeof(psk_configured),
	rc = nvme_tcp_parse_interchange_psk(psk_interchange, psk_configured, sizeof(psk_configured),
					    &psk_configured_size, &psk_retained_hash);
	if (rc < 0) {
		SPDK_ERRLOG("Failed to parse PSK interchange!\n");
@@ -3617,14 +3671,6 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,
	}
	entry->tls_cipher_suite = tls_cipher_suite;

	if (strlen(opts.psk) >= sizeof(entry->psk)) {
		SPDK_ERRLOG("PSK of length: %ld cannot fit in max buffer size: %ld\n", strlen(opts.psk),
			    sizeof(entry->psk));
		rc = -EINVAL;
		free(entry);
		goto end;
	}

	/* No hash indicates that Configured PSK must be used as Retained PSK. */
	if (psk_retained_hash == NVME_TCP_HASH_ALGORITHM_NONE) {
		/* Psk configured is either 32 or 48 bytes long. */
@@ -3646,8 +3692,8 @@ nvmf_tcp_subsystem_add_host(struct spdk_nvmf_transport *transport,

end:
	spdk_memset_s(psk_configured, sizeof(psk_configured), 0, sizeof(psk_configured));
	key_len = strnlen(opts.psk, SPDK_TLS_PSK_MAX_LEN);
	spdk_memset_s(opts.psk, key_len, 0, key_len);
	spdk_memset_s(psk_interchange, sizeof(psk_interchange), 0, sizeof(psk_interchange));

	free(opts.psk);

	return rc;
+46 −2
Original line number Diff line number Diff line
@@ -21,6 +21,8 @@
#include "spdk/log.h"
#include "spdk/bdev_module.h"

#define TCP_PSK_INVALID_PERMISSIONS 0177

static int
rpc_decode_action_on_timeout(const struct spdk_json_val *val, void *out)
{
@@ -313,6 +315,44 @@ rpc_bdev_nvme_attach_controller_done(void *cb_ctx, size_t bdev_count, int rc)
	spdk_bdev_wait_for_examine(rpc_bdev_nvme_attach_controller_examined, ctx);
}

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;
	}

	memset(buf, 0, bufsz);
	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;
}

static void
rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request,
				const struct spdk_json_val *params)
@@ -428,8 +468,12 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request,
	}

	if (ctx->req.psk) {
		snprintf(ctx->req.drv_opts.psk, sizeof(ctx->req.drv_opts.psk), "%s",
		rc = tcp_load_psk(ctx->req.psk, ctx->req.drv_opts.psk, sizeof(ctx->req.drv_opts.psk));
		if (rc) {
			spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "Could not retrieve PSK from file: %s",
							     ctx->req.psk);
			goto cleanup;
		}
	}

	if (ctx->req.hostaddr) {
Loading