Commit 6dbcb893 authored by Ben Walker's avatar Ben Walker Committed by Jim Harris
Browse files

nvmf: Correctly enforce subsystem listener access control 



Connections to a subsystem may only originate on listeners
that have been explicitly allowed for the given subsystem.

Change-Id: Ib575a5803cc7af75c15d1a2c76b9866594e7a1db
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/425024


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
parent ff628417

lib/nvmf/ctrlr.c

+17 −0
Original line number Diff line number Diff line
@@ -311,6 +311,7 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req) 
	struct spdk_nvmf_ctrlr *ctrlr;

	struct spdk_nvmf_subsystem *subsystem;

	const char *subnqn, *hostnqn;

	struct spdk_nvme_transport_id listen_trid = {};

	void *end;



	if (req->length < sizeof(struct spdk_nvmf_fabric_connect_data)) {
@@ -374,6 +375,22 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req) 
		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	if (spdk_nvmf_qpair_get_listen_trid(qpair, &listen_trid)) {

		SPDK_ERRLOG("Subsystem '%s' is unable to enforce access control due to an internal error.\n",

			    subnqn);

		rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		rsp->status.sc = SPDK_NVMF_FABRIC_SC_INVALID_HOST;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	if (!spdk_nvmf_subsystem_listener_allowed(subsystem, &listen_trid)) {

		SPDK_ERRLOG("Subsystem '%s' does not allow host '%s' to connect at this address.\n", subnqn,

			    hostnqn);

		rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		rsp->status.sc = SPDK_NVMF_FABRIC_SC_INVALID_HOST;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	/*

	 * SQSIZE is a 0-based value, so it must be at least 1 (minimum queue depth is 2) and

	 *  strictly less than max_queue_depth.

lib/nvmf/subsystem.c

+1 −4
Original line number Diff line number Diff line
@@ -785,16 +785,13 @@ spdk_nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem, 
	return 0;

}



/*

 * TODO: this is the whitelist and will be called during connection setup

 */

bool

spdk_nvmf_subsystem_listener_allowed(struct spdk_nvmf_subsystem *subsystem,

				     struct spdk_nvme_transport_id *trid)

{

	struct spdk_nvmf_listener *listener;



	if (TAILQ_EMPTY(&subsystem->listeners)) {

	if (!strcmp(subsystem->subnqn, SPDK_NVMF_DISCOVERY_NQN)) {

		return true;

	}

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

+10 −0
Original line number Diff line number Diff line
@@ -128,6 +128,16 @@ DEFINE_STUB(spdk_nvmf_request_free, 
	    (struct spdk_nvmf_request *req),

	    -1);



DEFINE_STUB(spdk_nvmf_qpair_get_listen_trid,

	    int,

	    (struct spdk_nvmf_qpair *qpair, struct spdk_nvme_transport_id *trid),

	    0);



DEFINE_STUB(spdk_nvmf_subsystem_listener_allowed,

	    bool,

	    (struct spdk_nvmf_subsystem *subsystem, struct spdk_nvme_transport_id *trid),

	    true);



static void

ctrlr_ut_pass_msg(spdk_thread_fn fn, void *ctx, void *thread_ctx)

{