From 62e8868314f38b309bd5ea8dbc80c46d415aa21b Mon Sep 17 00:00:00 2001 From: Gil Bregman Date: Mon, 11 Nov 2024 14:41:04 +0200 Subject: [PATCH] Move inband authentication controller key from host to subsystem. Fixes #937 Signed-off-by: Gil Bregman --- control/cli.py | 88 ++++++--- control/grpc.py | 329 ++++++++++++++++++++++++++-------- control/proto/gateway.proto | 18 +- control/server.py | 10 +- control/state.py | 136 ++++++++++---- mk/demosecuredhchap.mk | 27 +-- tests/ha/demo_test.sh | 190 ++++++++++++++------ tests/test_cli_change_keys.py | 35 ++-- tests/test_dhchap.py | 63 ++++--- tests/test_psk.py | 2 +- 10 files changed, 637 insertions(+), 261 deletions(-) diff --git a/control/cli.py b/control/cli.py index 8af7bb25..a5813bc5 100644 --- a/control/cli.py +++ b/control/cli.py @@ -674,7 +674,8 @@ def subsystem_add(self, args): serial_number=args.serial_number, max_namespaces=args.max_namespaces, enable_ha=True, - no_group_append=args.no_group_append) + no_group_append=args.no_group_append, + dhchap_key=args.dhchap_key) try: ret = self.stub.create_subsystem(req) except Exception as ex: @@ -770,8 +771,9 @@ def subsystem_list(self, args): err_func("Failure listing subsystem with serial number {args.serial_number}: Got serial number {s.serial_number} instead") return errno.ENODEV ctrls_id = f"{s.min_cntlid}-{s.max_cntlid}" - ha_str = "enabled" if s.enable_ha else "disabled" - one_subsys = [s.subtype, s.nqn, ha_str, s.serial_number, ctrls_id, s.namespace_count, s.max_namespaces] + has_dhchap = "Yes" if s.has_dhchap_key else "No" + allow_any = "Yes" if s.allow_any_host else "No" + one_subsys = [s.subtype, s.nqn, s.serial_number, ctrls_id, s.namespace_count, s.max_namespaces, allow_any, has_dhchap] subsys_list.append(one_subsys) if len(subsys_list) > 0: if args.format == "text": @@ -779,8 +781,8 @@ def subsystem_list(self, args): else: table_format = "plain" subsys_out = tabulate(subsys_list, - headers = ["Subtype", "NQN", "HA State", "Serial\nNumber", "Controller IDs", - "Namespace\nCount", "Max\nNamespaces"], + headers = ["Subtype", "NQN", "Serial\nNumber", "Controller IDs", + "Namespace\nCount", "Max\nNamespaces", "Allow\nAny Host", "DHCHAP\nKey"], tablefmt=table_format) prefix = "Subsystems" if args.subsystem: @@ -815,11 +817,48 @@ def subsystem_list(self, args): return subsystems.status + def subsystem_change_key(self, args): + """Change subsystem's inband authentication key.""" + + rc = 0 + out_func, err_func = self.get_output_functions(args) + + req = pb2.change_subsystem_key_req(subsystem_nqn=args.subsystem, dhchap_key=args.dhchap_key) + try: + ret = self.stub.change_subsystem_key(req) + except Exception as ex: + errmsg = f"Failure changing key for subsystem {args.subsystem}" + ret = pb2.req_status(status = errno.EINVAL, error_message = f"{errmsg}:\n{ex}") + + if args.format == "text" or args.format == "plain": + if ret.status == 0: + out_func(f"Changing key for subsystem {args.subsystem}: Successful") + else: + err_func(f"{ret.error_message}") + elif args.format == "json" or args.format == "yaml": + ret_str = json_format.MessageToJson( + ret, + indent=4, + including_default_value_fields=True, + preserving_proto_field_name=True) + if args.format == "json": + out_func(f"{ret_str}") + elif args.format == "yaml": + obj = json.loads(ret_str) + out_func(yaml.dump(obj)) + elif args.format == "python": + return ret + else: + assert False + + return ret.status + subsys_add_args = [ argument("--subsystem", "-n", help="Subsystem NQN", required=True), argument("--serial-number", "-s", help="Serial number", required=False), argument("--max-namespaces", "-m", help="Maximum number of namespaces", type=int, required=False), argument("--no-group-append", help="Do not append gateway group name to the NQN", action='store_true', required=False), + argument("--dhchap-key", "-k", help="Subsystem DH-HMAC-CHAP key", required=False), ] subsys_del_args = [ argument("--subsystem", "-n", help="Subsystem NQN", required=True), @@ -829,10 +868,15 @@ def subsystem_list(self, args): argument("--subsystem", "-n", help="Subsystem NQN", required=False), argument("--serial-number", "-s", help="Serial number", required=False), ] + subsys_change_key_args = [ + argument("--subsystem", "-n", help="Subsystem NQN", required=True), + argument("--dhchap-key", "-k", help="Subsystem DH-HMAC-CHAP key", required=False), + ] subsystem_actions = [] subsystem_actions.append({"name" : "add", "args" : subsys_add_args, "help" : "Create a subsystem"}) subsystem_actions.append({"name" : "del", "args" : subsys_del_args, "help" : "Delete a subsystem"}) subsystem_actions.append({"name" : "list", "args" : subsys_list_args, "help" : "List subsystems"}) + subsystem_actions.append({"name" : "change_key", "args" : subsys_change_key_args, "help" : "Change subsystem key"}) subsystem_choices = get_actions(subsystem_actions) @cli.cmd(subsystem_actions) def subsystem(self, args): @@ -843,6 +887,8 @@ def subsystem(self, args): return self.subsystem_del(args) elif args.action == "list": return self.subsystem_list(args) + elif args.action == "change_key": + return self.subsystem_change_key(args) if not args.action: self.cli.parser.error(f"missing action for subsystem command (choose from {GatewayClient.subsystem_choices})") @@ -1061,10 +1107,6 @@ def host_add(self, args): if len(args.host_nqn) > 1: self.cli.parser.error(f"Can't have more than one host NQN when DH-HMAC-CHAP keys are used") - if args.dhchap_ctrlr_key: - if not args.dhchap_key: - self.cli.parser.error(f"DH-HMAC-CHAP controller keys can not be used without DH-HMAC-CHAP keys") - for one_host_nqn in args.host_nqn: if one_host_nqn == "*" and args.psk: self.cli.parser.error(f"PSK key is only allowed for specific hosts") @@ -1073,7 +1115,7 @@ def host_add(self, args): self.cli.parser.error(f"DH-HMAC-CHAP key is only allowed for specific hosts") req = pb2.add_host_req(subsystem_nqn=args.subsystem, host_nqn=one_host_nqn, - psk=args.psk, dhchap_key=args.dhchap_key, dhchap_ctrlr_key=args.dhchap_ctrlr_key) + psk=args.psk, dhchap_key=args.dhchap_key) try: ret = self.stub.add_host(req) except Exception as ex: @@ -1165,7 +1207,7 @@ def host_del(self, args): return rc - def host_change_keys(self, args): + def host_change_key(self, args): """Change host's inband authentication keys.""" rc = 0 @@ -1174,21 +1216,17 @@ def host_change_keys(self, args): if args.host_nqn == "*": self.cli.parser.error(f"Can't change keys for host NQN '*', please use a real NQN") - if args.dhchap_ctrlr_key: - if not args.dhchap_key: - self.cli.parser.error(f"DH-HMAC-CHAP controller keys can not be used without DH-HMAC-CHAP keys") - - req = pb2.change_host_keys_req(subsystem_nqn=args.subsystem, host_nqn=args.host_nqn, - dhchap_key=args.dhchap_key, dhchap_ctrlr_key=args.dhchap_ctrlr_key) + req = pb2.change_host_key_req(subsystem_nqn=args.subsystem, host_nqn=args.host_nqn, + dhchap_key=args.dhchap_key) try: - ret = self.stub.change_host_keys(req) + ret = self.stub.change_host_key(req) except Exception as ex: - errmsg = f"Failure changing keys for host {args.host_nqn} on subsystem {args.subsystem}" + errmsg = f"Failure changing key for host {args.host_nqn} on subsystem {args.subsystem}" ret = pb2.req_status(status = errno.EINVAL, error_message = f"{errmsg}:\n{ex}") if args.format == "text" or args.format == "plain": if ret.status == 0: - out_func(f"Changing keys for host {args.host_nqn} on subsystem {args.subsystem}: Successful") + out_func(f"Changing key for host {args.host_nqn} on subsystem {args.subsystem}: Successful") else: err_func(f"{ret.error_message}") elif args.format == "json" or args.format == "yaml": @@ -1267,23 +1305,21 @@ def host_list(self, args): argument("--host-nqn", "-t", help="Host NQN list", nargs="+", required=True), argument("--psk", "-p", help="Hosts PSK key", required=False), argument("--dhchap-key", "-k", help="Host DH-HMAC-CHAP key", required=False), - argument("--dhchap-ctrlr-key", "-c", help="Host DH-HMAC-CHAP controller key", required=False), ] host_del_args = host_common_args + [ argument("--host-nqn", "-t", help="Host NQN list", nargs="+", required=True), ] host_list_args = host_common_args + [ ] - host_change_keys_args = host_common_args + [ + host_change_key_args = host_common_args + [ argument("--host-nqn", "-t", help="Host NQN", required=True), argument("--dhchap-key", "-k", help="Host DH-HMAC-CHAP key", required=False), - argument("--dhchap-ctrlr-key", "-c", help="Host DH-HMAC-CHAP controller key", required=False), ] host_actions = [] host_actions.append({"name" : "add", "args" : host_add_args, "help" : "Add host access to a subsystem"}) host_actions.append({"name" : "del", "args" : host_del_args, "help" : "Remove host access from a subsystem"}) host_actions.append({"name" : "list", "args" : host_list_args, "help" : "List subsystem's host access"}) - host_actions.append({"name" : "change_keys", "args" : host_change_keys_args, "help" : "Change host's inband authentication keys"}) + host_actions.append({"name" : "change_key", "args" : host_change_key_args, "help" : "Change host's inband authentication keys"}) host_choices = get_actions(host_actions) @cli.cmd(host_actions) def host(self, args): @@ -1294,8 +1330,8 @@ def host(self, args): return self.host_del(args) elif args.action == "list": return self.host_list(args) - elif args.action == "change_keys": - return self.host_change_keys(args) + elif args.action == "change_key": + return self.host_change_key(args) if not args.action: self.cli.parser.error(f"missing action for host command (choose from {GatewayClient.host_choices})") diff --git a/control/grpc.py b/control/grpc.py index b59243a1..b2935de9 100644 --- a/control/grpc.py +++ b/control/grpc.py @@ -67,47 +67,60 @@ class SubsystemHostAuth: def __init__(self): self.subsys_allow_any_hosts = defaultdict(dict) - self.host_has_psk = defaultdict(dict) - self.host_has_dhchap = defaultdict(dict) + self.subsys_dhchap_key = defaultdict(dict) + self.host_dhchap_key = defaultdict(dict) + self.host_psk_key = defaultdict(dict) def clean_subsystem(self, subsys): - self.host_has_psk.pop(subsys, None) - self.host_has_dhchap.pop(subsys, None) + self.host_psk_key.pop(subsys, None) + self.host_dhchap_key.pop(subsys, None) self.subsys_allow_any_hosts.pop(subsys, None) + self.subsys_dhchap_key.pop(subsys, None) - def add_psk_host(self, subsys, host): - self.host_has_psk[subsys][host] = True + def add_psk_host(self, subsys, host, key): + if key: + self.host_psk_key[subsys][host] = key def remove_psk_host(self, subsys, host): - if subsys in self.host_has_psk: - self.host_has_psk[subsys].pop(host, None) - if len(self.host_has_psk[subsys]) == 0: - self.host_has_psk.pop(subsys, None) # last host was removed from subsystem - - def is_psk_host(self, subsys, host = None) -> bool: - if subsys in self.host_has_psk: - if not host: - return len(self.host_has_psk[subsys]) != 0 - if host in self.host_has_psk[subsys]: - return True - return False + if subsys in self.host_psk_key: + self.host_psk_key[subsys].pop(host, None) + if len(self.host_psk_key[subsys]) == 0: + self.host_psk_key.pop(subsys, None) # last host was removed from subsystem + + def is_psk_host(self, subsys, host) -> bool: + key = self.get_host_psk_key(subsys, host) + return True if key else False - def add_dhchap_host(self, subsys, host): - self.host_has_dhchap[subsys][host] = True + def get_host_psk_key(self, subsys, host) -> str: + key = None + if subsys in self.host_psk_key and host in self.host_psk_key[subsys]: + key = self.host_psk_key[subsys][host] + return key + + def add_dhchap_host(self, subsys, host, key): + if key: + self.host_dhchap_key[subsys][host] = key def remove_dhchap_host(self, subsys, host): - if subsys in self.host_has_dhchap: - self.host_has_dhchap[subsys].pop(host, None) - if len(self.host_has_dhchap[subsys]) == 0: - self.host_has_dhchap.pop(subsys, None) # last host was removed from subsystem - - def is_dhchap_host(self, subsys, host = None) -> bool: - if subsys in self.host_has_dhchap: - if not host: - return len(self.host_has_dhchap[subsys]) != 0 - if host in self.host_has_dhchap[subsys]: - return True - return False + if subsys in self.host_dhchap_key: + self.host_dhchap_key[subsys].pop(host, None) + if len(self.host_dhchap_key[subsys]) == 0: + self.host_dhchap_key.pop(subsys, None) # last host was removed from subsystem + + def is_dhchap_host(self, subsys, host) -> bool: + key = self.get_host_dhchap_key(subsys, host) + return True if key else False + + def get_host_dhchap_key(self, subsys, host) -> str: + key = None + if subsys in self.host_dhchap_key and host in self.host_dhchap_key[subsys]: + key = self.host_dhchap_key[subsys][host] + return key + + def get_hosts_with_dhchap_key(self, subsys): + if subsys in self.host_dhchap_key: + return self.host_dhchap_key[subsys] + return {} def allow_any_host(self, subsys): self.subsys_allow_any_hosts[subsys] = True @@ -118,6 +131,23 @@ def disallow_any_host(self, subsys): def is_any_host_allowed(self, subsys) -> bool: return subsys in self.subsys_allow_any_hosts + def add_dhchap_key_to_subsystem(self, subsys, key): + if key: + self.subsys_dhchap_key[subsys] = key + + def remove_dhchap_key_from_subsystem(self, subsys): + self.subsys_dhchap_key.pop(subsys, None) + + def does_subsystem_have_dhchap_key(self, subsys) -> bool: + key = self.get_subsystem_dhchap_key(subsys) + return True if key else False + + def get_subsystem_dhchap_key(self, subsys) -> str: + key = None + if subsys in self.subsys_dhchap_key: + key = self.subsys_dhchap_key[subsys] + return key + class NamespaceInfo: def __init__(self, nsid, bdev, uuid, anagrpid, no_auto_visible): self.nsid = nsid @@ -843,7 +873,7 @@ def create_subsystem_safe(self, request, context): peer_msg = self.get_peer_message(context) self.logger.info( - f"Received request to create subsystem {request.subsystem_nqn}, enable_ha: {request.enable_ha}, max_namespaces: {request.max_namespaces}, no group append: {request.no_group_append}, context: {context}{peer_msg}") + f"Received request to create subsystem {request.subsystem_nqn}, enable_ha: {request.enable_ha}, max_namespaces: {request.max_namespaces}, no group append: {request.no_group_append}, dhchap_key: {request.dhchap_key}, context: {context}{peer_msg}") if not request.enable_ha: errmsg = f"{create_subsystem_error_prefix}: HA must be enabled for subsystems" @@ -920,6 +950,8 @@ def create_subsystem_safe(self, request, context): ana_reporting = True, ) self.subsys_max_ns[request.subsystem_nqn] = request.max_namespaces if request.max_namespaces else 32 + if request.dhchap_key: + self.host_info.add_dhchap_key_to_subsystem(request.subsystem_nqn, request.dhchap_key) self.logger.debug(f"create_subsystem {request.subsystem_nqn}: {ret}") except Exception as ex: self.logger.exception(create_subsystem_error_prefix) @@ -2210,6 +2242,21 @@ def matching_host_exists(self, context, subsys_nqn, host_nqn) -> bool: return True return False + def get_subsystem_hosts(self, subsys_nqn): + hosts = [] + state = self.gateway_state.local.get_state() + host_key_prefix = GatewayState.build_host_key(subsys_nqn, None) + for key, val in state.items(): + if key.startswith(host_key_prefix): + try: + host = json.loads(val) + host_nqn = host["host_nqn"] + hosts.append(host_nqn) + except Exception: + self.logger.exception(f"Error parsing {val}") + pass + return hosts + def _create_dhchap_key_files(self, subsystem_nqn, host_nqn, dhchap_key, dhchap_ctrlr_key, err_prefix): assert dhchap_key, "DH-HMAC-CHAP key value can't be empty" dhchap_file = None @@ -2272,6 +2319,12 @@ def add_host_safe(self, request, context): """Adds a host to a subsystem.""" peer_msg = self.get_peer_message(context) + if request.host_nqn == "*": + self.logger.info(f"Received request to allow any host access for {request.subsystem_nqn}, context: {context}{peer_msg}") + else: + self.logger.info( + f"Received request to add host {request.host_nqn} to {request.subsystem_nqn}, psk: {request.psk}, dhchap: {request.dhchap_key}, context: {context}{peer_msg}") + all_host_failure_prefix=f"Failure allowing open host access to {request.subsystem_nqn}" host_failure_prefix=f"Failure adding host {request.host_nqn} to {request.subsystem_nqn}" @@ -2285,6 +2338,11 @@ def add_host_safe(self, request, context): self.logger.error(f"{errmsg}") return pb2.req_status(status = errno.EINVAL, error_message = errmsg) + if request.host_nqn == "*" and self.host_info.does_subsystem_have_dhchap_key(request.subsystem_nqn): + errmsg=f"{all_host_failure_prefix}: Can't allow any host access on a subsystem having a DH-HMAC-CHAP key" + self.logger.error(f"{errmsg}") + return pb2.req_status(status = errno.EINVAL, error_message = errmsg) + if self.verify_nqns: rc = GatewayService.is_valid_host_nqn(request.host_nqn) if rc.status != 0: @@ -2306,19 +2364,29 @@ def add_host_safe(self, request, context): return pb2.req_status(status=errno.EINVAL, error_message=errmsg) if request.psk and request.host_nqn == "*": - errmsg=f"{host_failure_prefix}: PSK is only allowed for specific hosts" + errmsg=f"{all_host_failure_prefix}: PSK is only allowed for specific hosts" self.logger.error(f"{errmsg}") return pb2.req_status(status=errno.EINVAL, error_message=errmsg) if request.dhchap_key and request.host_nqn == "*": - errmsg=f"{host_failure_prefix}: DH-HMAC-CHAP key is only allowed for specific hosts" + errmsg=f"{all_host_failure_prefix}: DH-HMAC-CHAP key is only allowed for specific hosts" self.logger.error(f"{errmsg}") return pb2.req_status(status=errno.EINVAL, error_message=errmsg) - if request.dhchap_ctrlr_key and not request.dhchap_key: - errmsg=f"{host_failure_prefix}: DH-HMAC-CHAP controller key can only be used with a DH-HMAC-CHAP key" - self.logger.error(f"{errmsg}") - return pb2.req_status(status=errno.EINVAL, error_message=errmsg) + if request.dhchap_key and not self.host_info.does_subsystem_have_dhchap_key(request.subsystem_nqn): + self.logger.warning(f"Host {request.host_nqn} has a DH-HMAC-CHAP key but subsystem {request.subsystem_nqn} has no key, a unidirectional authentication will be used") + + if request.host_nqn == "*": + secure = False + try: + for listener in self.subsystem_listeners[request.subsystem_nqn]: + (_, _, _, secure) = listener + if secure: + errmsg=f"{all_host_failure_prefix}: Can't allow any host on a subsystem with secure listeners" + self.logger.error(f"{errmsg}") + return pb2.req_status(status=errno.EINVAL, error_message=errmsg) + except Exception: + pass host_already_exist = self.matching_host_exists(context, request.subsystem_nqn, request.host_nqn) if host_already_exist: @@ -2331,6 +2399,15 @@ def add_host_safe(self, request, context): self.logger.error(f"{errmsg}") return pb2.req_status(status=errno.EEXIST, error_message=errmsg) + dhchap_ctrlr_key = self.host_info.get_subsystem_dhchap_key(request.subsystem_nqn) + if dhchap_ctrlr_key: + self.logger.info(f"Got DHCHAP key {dhchap_ctrlr_key} for subsystem {request.subsystem_nqn}") + + if dhchap_ctrlr_key and not request.dhchap_key: + errmsg=f"{host_failure_prefix}: Host must have a DH-HMAC-CHAP key if the subsystem has one" + self.logger.error(f"{errmsg}") + return pb2.req_status(status=errno.EINVAL, error_message=errmsg) + psk_file = None psk_key_name = None if request.psk: @@ -2358,7 +2435,7 @@ def add_host_safe(self, request, context): dhchap_ctrlr_file, dhchap_ctrlr_key_name) = self._create_dhchap_key_files( request.subsystem_nqn, request.host_nqn, - request.dhchap_key, request.dhchap_ctrlr_key, host_failure_prefix) + request.dhchap_key, dhchap_ctrlr_key, host_failure_prefix) if key_files_status != 0: if psk_file: self.remove_host_psk_file(request.subsystem_nqn, request.host_nqn) @@ -2368,7 +2445,6 @@ def add_host_safe(self, request, context): with omap_lock: try: if request.host_nqn == "*": # Allow any host access to subsystem - self.logger.info(f"Received request to allow any host access for {request.subsystem_nqn}, context: {context}{peer_msg}") ret = rpc_nvmf.nvmf_subsystem_allow_any_host( self.spdk_rpc_client, nqn=request.subsystem_nqn, @@ -2377,8 +2453,6 @@ def add_host_safe(self, request, context): self.logger.debug(f"add_host *: {ret}") self.host_info.allow_any_host(request.subsystem_nqn) else: # Allow single host access to subsystem - self.logger.info( - f"Received request to add host {request.host_nqn} to {request.subsystem_nqn}, psk: {request.psk}, dhchap: {request.dhchap_key}, dhchap controller: {request.dhchap_ctrlr_key}, context: {context}{peer_msg}") self._add_key_to_keyring("PSK", psk_file, psk_key_name) self._add_key_to_keyring("DH-HMAC-CHAP", dhchap_file, dhchap_key_name) self._add_key_to_keyring("DH-HMAC-CHAP controller", dhchap_ctrlr_file, dhchap_ctrlr_key_name) @@ -2392,14 +2466,14 @@ def add_host_safe(self, request, context): ) self.logger.debug(f"add_host {request.host_nqn}: {ret}") if psk_file: - self.host_info.add_psk_host(request.subsystem_nqn, request.host_nqn) + self.host_info.add_psk_host(request.subsystem_nqn, request.host_nqn, request.psk) self.remove_host_psk_file(request.subsystem_nqn, request.host_nqn) try: rpc_keyring.keyring_file_remove_key(self.spdk_rpc_client, psk_key_name) except Exception: pass if dhchap_file: - self.host_info.add_dhchap_host(request.subsystem_nqn, request.host_nqn) + self.host_info.add_dhchap_host(request.subsystem_nqn, request.host_nqn, request.dhchap_key) except Exception as ex: if request.host_nqn == "*": self.logger.exception(all_host_failure_prefix) @@ -2557,13 +2631,13 @@ def remove_host_safe(self, request, context): def remove_host(self, request, context=None): return self.execute_grpc_function(self.remove_host_safe, request, context) - def change_host_keys_safe(self, request, context): - """Changes host's inband authentication keys.""" + def change_host_key_safe(self, request, context): + """Changes host's inband authentication key.""" peer_msg = self.get_peer_message(context) - failure_prefix=f"Failure changing keys for host {request.host_nqn} on subsystem {request.subsystem_nqn}" + failure_prefix=f"Failure changing DH-HMAC-CHAP key for host {request.host_nqn} on subsystem {request.subsystem_nqn}" self.logger.info( - f"Received request to change inband authentication keys for host {request.host_nqn} on subsystem {request.subsystem_nqn}, dhchap: {request.dhchap_key}, dhchap controller: {request.dhchap_ctrlr_key}, context: {context}{peer_msg}") + f"Received request to change inband authentication key for host {request.host_nqn} on subsystem {request.subsystem_nqn}, dhchap: {request.dhchap_key}, context: {context}{peer_msg}") if request.host_nqn == "*": errmsg=f"{failure_prefix}: Host NQN can't be '*'" @@ -2581,22 +2655,37 @@ def change_host_keys_safe(self, request, context): return pb2.req_status(status = errno.EINVAL, error_message = errmsg) if self.verify_nqns: + rc = GatewayUtils.is_valid_nqn(request.subsystem_nqn) + if rc[0] != 0: + errmsg = f"{failure_prefix}: {rc[1]}" + self.logger.error(f"{errmsg}") + return pb2.req_status(status = rc[0], error_message = errmsg) + rc = GatewayUtils.is_valid_nqn(request.host_nqn) if rc[0] != 0: errmsg = f"{failure_prefix}: {rc[1]}" self.logger.error(f"{errmsg}") return pb2.req_status(status = rc[0], error_message = errmsg) + if GatewayUtils.is_discovery_nqn(request.subsystem_nqn): + errmsg=f"{failure_prefix}: Can't use a discovery NQN as subsystem's" + self.logger.error(f"{errmsg}") + return pb2.req_status(status=errno.EINVAL, error_message=errmsg) + if GatewayUtils.is_discovery_nqn(request.host_nqn): errmsg=f"{failure_prefix}: Can't use a discovery NQN as host's" self.logger.error(f"{errmsg}") return pb2.req_status(status=errno.EINVAL, error_message=errmsg) - if request.dhchap_ctrlr_key and not request.dhchap_key: - errmsg=f"{failure_prefix}: DH-HMAC-CHAP controller key can only be used with a DH-HMAC-CHAP key" + dhchap_ctrlr_key = self.host_info.get_subsystem_dhchap_key(request.subsystem_nqn) + if dhchap_ctrlr_key and not request.dhchap_key: + errmsg=f"{failure_prefix}: Host must have a DH-HMAC-CHAP key if the subsystem has one" self.logger.error(f"{errmsg}") return pb2.req_status(status=errno.EINVAL, error_message=errmsg) + if request.dhchap_key and not dhchap_ctrlr_key: + self.logger.warning(f"Host {request.host_nqn} has a DH-HMAC-CHAP key but subsystem {request.subsystem_nqn} has no key, a unidirectional authentication will be used") + host_already_exist = self.matching_host_exists(context, request.subsystem_nqn, request.host_nqn) if not host_already_exist and context: errmsg=f"{failure_prefix}: Can't find host on subsystem" @@ -2615,32 +2704,16 @@ def change_host_keys_safe(self, request, context): dhchap_ctrlr_file, dhchap_ctrlr_key_name) = self._create_dhchap_key_files( request.subsystem_nqn, request.host_nqn, - request.dhchap_key, request.dhchap_ctrlr_key, failure_prefix) + request.dhchap_key, dhchap_ctrlr_key, failure_prefix) if key_files_status != 0: return pb2.req_status(status=key_files_status, error_message=key_file_errmsg) omap_lock = self.omap_lock.get_omap_lock_to_use(context) with omap_lock: - host_entry = None host_psk = None if context: - # notice that the local state might not be up to date in case we're in the middle of update() but as the - # context is not None, we are not in an update(), the omap lock made sure that we got here with an updated local state - state = self.gateway_state.local.get_state() - host_key = GatewayState.build_host_key(request.subsystem_nqn, request.host_nqn) - try: - state_host = state[host_key] - host_entry = json.loads(state_host) - except Exception as ex: - errmsg = f"{failure_prefix}: Can't find entry for host {request.host_nqn} in {request.subsystem_nqn}" - self.logger.error(errmsg) - return pb2.req_status(status=errno.ENOENT, error_message=errmsg) - - try: - host_psk = host_entry["psk"] - except Exception: - pass + host_psk = self.host_info.get_host_psk_key(request.subsystem_nqn, request.host_nqn) try: self._add_key_to_keyring("DH-HMAC-CHAP", dhchap_file, dhchap_key_name) @@ -2670,35 +2743,33 @@ def change_host_keys_safe(self, request, context): return pb2.req_status(status=errno.EINVAL, error_message=errmsg) if dhchap_key_name: - self.host_info.add_dhchap_host(request.subsystem_nqn, request.host_nqn) + self.host_info.add_dhchap_host(request.subsystem_nqn, request.host_nqn, request.dhchap_key) else: self.host_info.remove_dhchap_host(request.subsystem_nqn, request.host_nqn) self.remove_all_host_key_files(request.subsystem_nqn, request.host_nqn) self.remove_all_host_keys_from_keyring(request.subsystem_nqn, request.host_nqn) if context: - assert host_entry, "Host entry is None for non-update call" # Update gateway state try: add_req = pb2.add_host_req(subsystem_nqn=request.subsystem_nqn, host_nqn=request.host_nqn, psk=host_psk, - dhchap_key=request.dhchap_key, - dhchap_ctrlr_key=request.dhchap_ctrlr_key) + dhchap_key=request.dhchap_key) json_req = json_format.MessageToJson( add_req, preserving_proto_field_name=True, including_default_value_fields=True) self.gateway_state.add_host(request.subsystem_nqn, request.host_nqn, json_req) except Exception as ex: - errmsg = f"Error persisting host changet keys for host {request.host_nqn} in {request.subsystem_nqn}" + errmsg = f"Error persisting host change key for host {request.host_nqn} in {request.subsystem_nqn}" self.logger.exception(errmsg) errmsg = f"{errmsg}:\n{ex}" return pb2.req_status(status=errno.EINVAL, error_message=errmsg) return pb2.req_status(status=0, error_message=os.strerror(0)) - def change_host_keys(self, request, context=None): - """Changes host's inband authentication keys.""" - return self.execute_grpc_function(self.change_host_keys_safe, request, context) + def change_host_key(self, request, context=None): + """Changes host's inband authentication key.""" + return self.execute_grpc_function(self.change_host_key_safe, request, context) def list_hosts_safe(self, request, context): """List hosts.""" @@ -3278,9 +3349,11 @@ def list_subsystems_safe(self, request, context): self.subsystem_nsid_bdev_and_uuid.remove_namespace(s["nqn"]) s["namespace_count"] = ns_count s["enable_ha"] = True + s["has_dhchap_key"] = self.host_info.does_subsystem_have_dhchap_key(s["nqn"]) else: s["namespace_count"] = 0 s["enable_ha"] = False + s["has_dhchap_key"] = False # Parse the JSON dictionary into the protobuf message subsystem = pb2.subsystem_cli() json_format.Parse(json.dumps(s), subsystem, ignore_unknown_fields=True) @@ -3307,6 +3380,7 @@ def get_subsystems_safe(self, request, context): for s in ret: try: + s["has_dhchap_key"] = self.host_info.does_subsystem_have_dhchap_key(s["nqn"]) ns_key = "namespaces" if ns_key in s: for n in s[ns_key]: @@ -3334,6 +3408,107 @@ def get_subsystems(self, request, context): def list_subsystems(self, request, context=None): return self.execute_grpc_function(self.list_subsystems_safe, request, context) + def change_subsystem_key_safe(self, request, context): + """Change subsystem key.""" + peer_msg = self.get_peer_message(context) + failure_prefix=f"Failure changing DH-HMAC-CHAP key for subsystem {request.subsystem_nqn}" + self.logger.info( + f"Received request to change inband authentication key for subsystem {request.subsystem_nqn}, dhchap: {request.dhchap_key}, context: {context}{peer_msg}") + + if not GatewayState.is_key_element_valid(request.subsystem_nqn): + errmsg = f"{failure_prefix}: Invalid subsystem NQN \"{request.subsystem_nqn}\", contains invalid characters" + self.logger.error(f"{errmsg}") + return pb2.req_status(status = errno.EINVAL, error_message = errmsg) + + if self.verify_nqns: + rc = GatewayUtils.is_valid_nqn(request.subsystem_nqn) + if rc[0] != 0: + errmsg = f"{failure_prefix}: {rc[1]}" + self.logger.error(f"{errmsg}") + return pb2.req_status(status = rc[0], error_message = errmsg) + + if GatewayUtils.is_discovery_nqn(request.subsystem_nqn): + errmsg = f"{failure_prefix}: Can't change DH-HMAC-CHAP key for a discovery subsystem" + self.logger.error(f"{errmsg}") + return pb2.req_status(status = errno.EINVAL, error_message = errmsg) + + omap_lock = self.omap_lock.get_omap_lock_to_use(context) + with omap_lock: + subsys_entry = None + if context: + # notice that the local state might not be up to date in case we're in the middle of update() but as the + # context is not None, we are not in an update(), the omap lock made sure that we got here with an updated local state + state = self.gateway_state.local.get_state() + if request.dhchap_key: + # We set the subsystem key, this requires that all hosts have keys too + host_key_prefix = GatewayState.build_host_key(request.subsystem_nqn, None) + for key, val in state.items(): + if not key.startswith(host_key_prefix): + continue + sub_nqn, host_nqn = self.gateway_state.break_host_key(key) + if not sub_nqn: + errmsg = f"{failure_prefix}: Can't get subsystem NQN from host key {key}" + self.logger.error(errmsg) + return pb2.req_status(status=errno.ENOENT, error_message=errmsg) + if not host_nqn: + errmsg = f"{failure_prefix}: Can't get host NQN from host key {key}" + self.logger.error(errmsg) + return pb2.req_status(status=errno.ENOENT, error_message=errmsg) + assert sub_nqn == request.subsystem_nqn, f"Subsystem NQN {sub_nqn}, taken from host key {key}, should be equal to {request.subsystem_nqn}" + if not self.host_info.is_dhchap_host(request.subsystem_nqn, host_nqn): + errmsg = f"{failure_prefix}: Can't set a subsystem DH-HMAC-CHAP key when it has hosts with no key, like host {host_nqn}" + self.logger.error(f"{errmsg}") + return pb2.req_status(status = errno.EINVAL, error_message = errmsg) + + subsys_key = GatewayState.build_subsystem_key(request.subsystem_nqn) + try: + state_subsys = state[subsys_key] + subsys_entry = json.loads(state_subsys) + except Exception as ex: + errmsg = f"{failure_prefix}: Can't find entry for subsystem {request.subsystem_nqn}" + self.logger.error(errmsg) + return pb2.req_status(status=errno.ENOENT, error_message=errmsg) + + assert subsys_entry, f"Can't find entry for subsystem {request.subsystem_nqn}" + try: + create_req = pb2.create_subsystem_req(subsystem_nqn=request.subsystem_nqn, + serial_number=subsys_entry["serial_number"], + max_namespaces=subsys_entry["max_namespaces"], + enable_ha=subsys_entry["enable_ha"], + no_group_append=subsys_entry["no_group_append"], + dhchap_key=request.dhchap_key) + json_req = json_format.MessageToJson( + create_req, preserving_proto_field_name=True, including_default_value_fields=True) + self.gateway_state.add_subsystem(request.subsystem_nqn, json_req) + except Exception as ex: + errmsg = f"Error persisting subsystem key change for {request.subsystem_nqn}" + self.logger.exception(errmsg) + errmsg = f"{errmsg}:\n{ex}" + return pb2.req_status(status=errno.EINVAL, error_message=errmsg) + + hosts = self.host_info.get_hosts_with_dhchap_key(request.subsystem_nqn).copy() + # We need to change the subsystem key before calling the host change key functions, so the new subsystem key will be used + # As we change the list now, we have to use a copy having the old values + if request.dhchap_key: + self.host_info.add_dhchap_key_to_subsystem(request.subsystem_nqn, request.dhchap_key) + else: + self.host_info.remove_dhchap_key_from_subsystem(request.subsystem_nqn) + for hnqn in hosts.keys(): + change_req = pb2.change_host_key_req(subsystem_nqn=request.subsystem_nqn, + host_nqn=hnqn, + dhchap_key=hosts[hnqn]) + try: + self.change_host_key_safe(change_req, context) + except Excpetion: + pass + + + return pb2.req_status(status=0, error_message=os.strerror(0)) + + def change_subsystem_key(self, request, context=None): + """Change subsystem key.""" + return self.execute_grpc_function(self.change_subsystem_key_safe, request, context) + def get_spdk_nvmf_log_flags_and_level_safe(self, request, context): """Gets spdk nvmf log flags, log level and log print level""" peer_msg = self.get_peer_message(context) diff --git a/control/proto/gateway.proto b/control/proto/gateway.proto index 45cbed3f..7ab76216 100644 --- a/control/proto/gateway.proto +++ b/control/proto/gateway.proto @@ -42,6 +42,9 @@ service Gateway { // Deletes a subsystem rpc delete_subsystem(delete_subsystem_req) returns(req_status) {} + // Changes subsystem key + rpc change_subsystem_key(change_subsystem_key_req) returns(req_status) {} + // List namespaces rpc list_namespaces(list_namespaces_req) returns(namespaces_info) {} @@ -73,7 +76,7 @@ service Gateway { rpc remove_host(remove_host_req) returns (req_status) {} // Changes a host inband authentication keys - rpc change_host_keys(change_host_keys_req) returns (req_status) {} + rpc change_host_key(change_host_key_req) returns (req_status) {} // List hosts rpc list_hosts(list_hosts_req) returns(hosts_info) {} @@ -188,6 +191,7 @@ message create_subsystem_req { optional uint32 max_namespaces = 3; bool enable_ha = 4; optional bool no_group_append = 5; + optional string dhchap_key = 6; } message delete_subsystem_req { @@ -195,6 +199,11 @@ message delete_subsystem_req { optional bool force = 2; } +message change_subsystem_key_req { + string subsystem_nqn = 1; + optional string dhchap_key = 2; +} + message list_namespaces_req { string subsystem = 1; optional uint32 nsid = 2; @@ -206,14 +215,12 @@ message add_host_req { string host_nqn = 2; optional string psk = 3; optional string dhchap_key = 4; - optional string dhchap_ctrlr_key = 5; } -message change_host_keys_req { +message change_host_key_req { string subsystem_nqn = 1; string host_nqn = 2; optional string dhchap_key = 3; - optional string dhchap_ctrlr_key = 4; } message remove_host_req { @@ -347,6 +354,7 @@ message subsystem { optional uint32 min_cntlid = 9; optional uint32 max_cntlid = 10; repeated namespace namespaces = 11; + optional bool has_dhchap_key = 12; } message listen_address { @@ -386,6 +394,8 @@ message subsystem_cli { uint32 namespace_count = 7; string subtype = 8; uint32 max_namespaces = 9; + optional bool has_dhchap_key = 10; + optional bool allow_any_host = 11; } message gateway_info { diff --git a/control/server.py b/control/server.py index 94a24ea6..16d05b9c 100644 --- a/control/server.py +++ b/control/server.py @@ -760,7 +760,11 @@ def gateway_rpc_caller(self, requests, is_add_req): else: req = json_format.Parse(val, pb2.namespace_delete_host_req(), ignore_unknown_fields=True) self.gateway_rpc.namespace_delete_host(req) - elif key.startswith(GatewayState.HOST_KEYS_PREFIX): + elif key.startswith(GatewayState.HOST_KEY_PREFIX): if is_add_req: - req = json_format.Parse(val, pb2.change_host_keys_req(), ignore_unknown_fields=True) - self.gateway_rpc.change_host_keys(req) + req = json_format.Parse(val, pb2.change_host_key_req(), ignore_unknown_fields=True) + self.gateway_rpc.change_host_key(req) + elif key.startswith(GatewayState.SUBSYSTEM_KEY_PREFIX): + if is_add_req: + req = json_format.Parse(val, pb2.change_subsystem_key_req(), ignore_unknown_fields=True) + self.gateway_rpc.change_subsystem_key(req) diff --git a/control/state.py b/control/state.py index d4f52ab0..cad8a20c 100644 --- a/control/state.py +++ b/control/state.py @@ -30,8 +30,9 @@ class GatewayState(ABC): OMAP_KEY_DELIMITER = "_" NAMESPACE_PREFIX = "namespace" + OMAP_KEY_DELIMITER SUBSYSTEM_PREFIX = "subsystem" + OMAP_KEY_DELIMITER + SUBSYSTEM_KEY_PREFIX = "key-subsystem" + OMAP_KEY_DELIMITER HOST_PREFIX = "host" + OMAP_KEY_DELIMITER - HOST_KEYS_PREFIX = "keys-host" + OMAP_KEY_DELIMITER + HOST_KEY_PREFIX = "key-host" + OMAP_KEY_DELIMITER LISTENER_PREFIX = "listener" + OMAP_KEY_DELIMITER NAMESPACE_QOS_PREFIX = "qos" + OMAP_KEY_DELIMITER NAMESPACE_LB_GROUP_PREFIX = "lbgroup" + OMAP_KEY_DELIMITER @@ -78,12 +79,15 @@ def build_host_key(subsystem_nqn: str, host_nqn: str) -> str: key += GatewayState.OMAP_KEY_DELIMITER + host_nqn return key - def build_host_keys_key(subsystem_nqn: str, host_nqn: str) -> str: - key = GatewayState.HOST_KEYS_PREFIX + subsystem_nqn + def build_host_key_key(subsystem_nqn: str, host_nqn: str) -> str: + key = GatewayState.HOST_KEY_PREFIX + subsystem_nqn if host_nqn is not None: key += GatewayState.OMAP_KEY_DELIMITER + host_nqn return key + def build_subsystem_key_key(subsystem_nqn: str) -> str: + return GatewayState.SUBSYSTEM_KEY_PREFIX + subsystem_nqn + def build_partial_listener_key(subsystem_nqn: str, host: str) -> str: key = GatewayState.LISTENER_PREFIX + subsystem_nqn if host: @@ -735,6 +739,7 @@ def _update_caller(self, notify_event): notify_event.clear() def namespace_only_lb_group_id_changed(self, old_val, new_val): + # If only the lb group id field has changed we can use change_lb_group request instead of re-adding the namespace old_req = None new_req = None try: @@ -757,38 +762,63 @@ def namespace_only_lb_group_id_changed(self, old_val, new_val): return (False, None) return (True, new_req.anagrpid) - def host_only_keys_changed(self, old_val, new_val): + def host_only_key_changed(self, old_val, new_val): + # If only the dhchap key has changed we can use change_key request instead of re-adding the host old_req = None new_req = None try: old_req = json_format.Parse(old_val, pb2.add_host_req(), ignore_unknown_fields=True ) except Exception as ex: self.logger.exception(f"Got exception parsing {old_val}") - return (False, None, None) + return (False, None) try: new_req = json_format.Parse(new_val, pb2.add_host_req(), ignore_unknown_fields=True) except Exception as ex: self.logger.exeption(f"Got exception parsing {new_val}") - return (False, None, None) + return (False, None) if not old_req or not new_req: self.logger.debug(f"Failed to parse requests, old: {old_val} -> {old_req}, new: {new_val} -> {new_req}") - return (False, None, None) + return (False, None) assert old_req != new_req, f"Something was wrong we shouldn't get identical old and new values ({old_req})" # Because of Json formatting of empty fields we might get a difference here, so just use the same values for empty if not old_req.dhchap_key: old_req.dhchap_key = "" - if not old_req.dhchap_ctrlr_key: - old_req.dhchap_ctrlr_key = "" if not new_req.dhchap_key: new_req.dhchap_key = "" - if not new_req.dhchap_ctrlr_key: - new_req.dhchap_ctrlr_key = "" old_req.dhchap_key = new_req.dhchap_key - old_req.dhchap_ctrlr_key = new_req.dhchap_ctrlr_key if old_req != new_req: # Something besides the keys is different - return (False, None, None) - return (True, new_req.dhchap_key, new_req.dhchap_ctrlr_key) + return (False, None) + return (True, new_req.dhchap_key) + + def subsystem_only_key_changed(self, old_val, new_val): + # If only the dhchap key field has changed we can use change_key request instead of re-adding the subsystem + old_req = None + new_req = None + try: + old_req = json_format.Parse(old_val, pb2.create_subsystem_req(), ignore_unknown_fields=True ) + except Exception as ex: + self.logger.exception(f"Got exception parsing {old_val}") + return (False, None) + try: + new_req = json_format.Parse(new_val, pb2.create_subsystem_req(), ignore_unknown_fields=True) + except Exception as ex: + self.logger.exeption(f"Got exception parsing {new_val}") + return (False, None) + if not old_req or not new_req: + self.logger.debug(f"Failed to parse requests, old: {old_val} -> {old_req}, new: {new_val} -> {new_req}") + return (False, None) + assert old_req != new_req, f"Something was wrong we shouldn't get identical old and new values ({old_req})" + # Because of Json formatting of empty fields we might get a difference here, so just use the same values for empty + if not old_req.dhchap_key: + old_req.dhchap_key = "" + if not new_req.dhchap_key: + new_req.dhchap_key = "" + old_req.dhchap_key = new_req.dhchap_key + if old_req != new_req: + # Something besides the keys is different + return (False, None) + return (True, new_req.dhchap_key) def break_namespace_key(self, ns_key: str): if not ns_key.startswith(GatewayState.NAMESPACE_PREFIX): @@ -831,6 +861,21 @@ def break_host_key(self, host_key: str): return (subsys_nqn, host_nqn) + def break_subsystem_key(self, subsys_key: str): + if not subsys_key.startswith(GatewayState.SUBSYSTEM_PREFIX): + self.logger.warning(f"Invalid subsystem key \"{subsys_key}\", can't find key") + return None + key_parts = subsys_key.split(GatewayState.OMAP_KEY_DELIMITER) + if len(key_parts) != 2: + self.logger.warning(f"Invalid subsystem key \"{subsys_key}\", can't find key") + return None + if not GatewayUtils.is_valid_nqn(key_parts[1]): + self.logger.warning(f"Invalid subsystem NQN \"{key_parts[0]}\" found for subsystem key \"{subsys_key}\", can't find key") + return None + subsys_nqn = key_parts[1] + + return subsys_nqn + def get_str_from_bytes(val): val_str = val.decode() if type(val) == type(b'') else val return val_str @@ -888,9 +933,11 @@ def update(self) -> bool: # Handle namespace changes in which only the load balancing group id was changed only_lb_group_changed = [] - only_keys_changed = [] + only_host_key_changed = [] + only_subsystem_key_changed = [] ns_prefix = GatewayState.build_namespace_key("nqn", None) host_prefix = GatewayState.build_host_key("nqn", None) + subsystem_prefix = GatewayState.build_subsystem_key("nqn") for key in changed.keys(): if key.startswith(ns_prefix): try: @@ -899,20 +946,29 @@ def update(self) -> bool: if should_process: assert new_lb_grp_id, "Shouldn't get here with an empty lb group id" self.logger.debug(f"Found {key} where only the load balancing group id has changed. The new group id is {new_lb_grp_id}") - only_lb_group_changed.insert(0, (key, new_lb_grp_id)) + only_lb_group_changed.append((key, new_lb_grp_id)) except Exception as ex: self.logger.warning("Got exception checking namespace for load balancing group id change") elif key.startswith(host_prefix): try: (should_process, - new_dhchap_key, - new_dhchap_ctrl_key) = self.host_only_keys_changed(local_state_dict[key], omap_state_dict[key]) + new_dhchap_key) = self.host_only_key_changed(local_state_dict[key], omap_state_dict[key]) + if should_process: + assert new_dhchap_key, "Shouldn't get here with an empty dhchap key" + self.logger.debug(f"Found {key} where only the key has changed. The new DHCHAP key is {new_dhchap_key}") + only_host_key_changed.append((key, new_dhchap_key)) + except Exception as ex: + self.logger.warning("Got exception checking host for key change") + elif key.startswith(subsystem_prefix): + try: + (should_process, + new_dhchap_key) = self.subsystem_only_key_changed(local_state_dict[key], omap_state_dict[key]) if should_process: assert new_dhchap_key, "Shouldn't get here with an empty dhchap key" - self.logger.debug(f"Found {key} where only the keys have changed. The new dhchap key is {new_dhchap_key}, the new dhchap controller key is {new_dhchap_ctrl_key}") - only_keys_changed.insert(0, (key, new_dhchap_key, new_dhchap_ctrl_key)) + self.logger.debug(f"Found {key} where only the key has changed. The new DHCHAP key is {new_dhchap_key}") + only_subsystem_key_changed.append((key, new_dhchap_key)) except Exception as ex: - self.logger.warning("Got exception checking host for keys change") + self.logger.warning("Got exception checking subsystem for key change") for ns_key, new_lb_grp in only_lb_group_changed: ns_nqn = None @@ -933,7 +989,7 @@ def update(self) -> bool: except Exception as ex: self.logger.error(f"Exception formatting change namespace load balancing group request:\n{ex}") - for host_key, new_dhchap_key, new_dhchap_ctrl_key in only_keys_changed: + for host_key, new_dhchap_key in only_host_key_changed: subsys_nqn = None host_nqn = None try: @@ -946,22 +1002,40 @@ def update(self) -> bool: continue if subsys_nqn and host_nqn: try: - host_keys_key = GatewayState.build_host_keys_key(subsys_nqn, host_nqn) - req = pb2.change_host_keys_req(subsystem_nqn=subsys_nqn, host_nqn=host_nqn, - dhchap_key=new_dhchap_key, - dhchap_ctrlr_key=new_dhchap_ctrl_key) + host_key_key = GatewayState.build_host_key_key(subsys_nqn, host_nqn) + req = pb2.change_host_key_req(subsystem_nqn=subsys_nqn, host_nqn=host_nqn, + dhchap_key=new_dhchap_key) + json_req = json_format.MessageToJson(req, preserving_proto_field_name=True, + including_default_value_fields=True) + added[host_key_key] = json_req + except Exception as ex: + self.logger.error(f"Exception formatting change host key request:\n{ex}") + + for subsys_key, new_dhchap_key in only_subsystem_key_changed: + subsys_nqn = None + try: + changed.pop(subsys_key) + subsys_nqn = self.break_subsystem_key(subsys_key) + except Exception as ex: + self.logger.error(f"Exception removing {subsys_key} from {changed}:\n{ex}") + if subsys_nqn: + try: + subsys_key_key = GatewayState.build_subsystem_key_key(subsys_nqn) + req = pb2.change_subsystem_key_req(subsystem_nqn=subsys_nqn, dhchap_key=new_dhchap_key) json_req = json_format.MessageToJson(req, preserving_proto_field_name=True, including_default_value_fields=True) - added[host_keys_key] = json_req + added[subsys_key_key] = json_req except Exception as ex: - self.logger.error(f"Exception formatting change host keys request:\n{ex}") + self.logger.error(f"Exception formatting change subsystem key request:\n{ex}") - if len(only_lb_group_changed) > 0 or len(only_keys_changed) > 0: + if len(only_lb_group_changed) > 0 or len(only_host_key_changed) > 0 or len(only_subsystem_key_changed) > 0: grouped_changed = self._group_by_prefix(changed, prefix_list) + if len(only_subsystem_key_changed) > 0: + prefix_list += [GatewayState.SUBSYSTEM_KEY_PREFIX] if len(only_lb_group_changed) > 0: prefix_list += [GatewayState.NAMESPACE_LB_GROUP_PREFIX] - if len(only_keys_changed) > 0: - prefix_list += [GatewayState.HOST_KEYS_PREFIX] + if len(only_host_key_changed) > 0: + prefix_list += [GatewayState.HOST_KEY_PREFIX] grouped_added = self._group_by_prefix(added, prefix_list) # Find OMAP removals diff --git a/mk/demosecuredhchap.mk b/mk/demosecuredhchap.mk index 862c47c6..e75d2e4b 100644 --- a/mk/demosecuredhchap.mk +++ b/mk/demosecuredhchap.mk @@ -1,5 +1,7 @@ ## Demo secure DHCHAP: +SUBNQN1=$(NQN) +SUBNQN2=$(NQN)2 HOSTNQN=`cat /etc/nvme/hostnqn` HOSTNQN2=`cat /etc/nvme/hostnqn | sed 's/......$$/ffffff/'` HOSTNQN3=`cat /etc/nvme/hostnqn | sed 's/......$$/fffffe/'` @@ -14,17 +16,18 @@ DHCHAPKEY4=$(DHCHAP_KEY4) PSKKEY1=$(PSK_KEY1) # demosecuredhchap demosecuredhchap: - $(NVMEOF_CLI) subsystem add --subsystem $(NQN) --no-group-append - $(NVMEOF_CLI) namespace add --subsystem $(NQN) --rbd-pool $(RBD_POOL) --rbd-image $(RBD_IMAGE_NAME) --size $(RBD_IMAGE_SIZE) --rbd-create-image - $(NVMEOF_CLI) namespace add --subsystem $(NQN) --rbd-pool $(RBD_POOL) --rbd-image $(RBD_IMAGE_NAME)2 --size $(RBD_IMAGE_SIZE) --rbd-create-image --no-auto-visible - $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT) - $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT2) - $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT3) - $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT4) --secure - $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn $(HOSTNQN) --dhchap-key $(DHCHAPKEY1) - $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn $(HOSTNQN2) --dhchap-key $(DHCHAPKEY2) --dhchap-ctrlr-key $(DHCHAPKEY3) - $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn $(HOSTNQN3) - $(NVMEOF_CLI) namespace add_host --subsystem $(NQN) --nsid 2 --host-nqn $(HOSTNQN) - $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn $(HOSTNQN4) --dhchap-key $(DHCHAPKEY4) --psk $(PSKKEY1) + $(NVMEOF_CLI) subsystem add --subsystem $(SUBNQN1) --no-group-append + $(NVMEOF_CLI) subsystem add --subsystem $(SUBNQN2) --no-group-append --dhchap-key $(DHCHAPKEY3) + $(NVMEOF_CLI) namespace add --subsystem $(SUBNQN1) --rbd-pool $(RBD_POOL) --rbd-image $(RBD_IMAGE_NAME) --size $(RBD_IMAGE_SIZE) --rbd-create-image + $(NVMEOF_CLI) namespace add --subsystem $(SUBNQN1) --rbd-pool $(RBD_POOL) --rbd-image $(RBD_IMAGE_NAME)2 --size $(RBD_IMAGE_SIZE) --rbd-create-image --no-auto-visible + $(NVMEOF_CLI) listener add --subsystem $(SUBNQN1) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT) + $(NVMEOF_CLI) listener add --subsystem $(SUBNQN2) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT2) + $(NVMEOF_CLI) listener add --subsystem $(SUBNQN1) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT3) + $(NVMEOF_CLI) listener add --subsystem $(SUBNQN1) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT4) --secure + $(NVMEOF_CLI) host add --subsystem $(SUBNQN1) --host-nqn $(HOSTNQN) --dhchap-key $(DHCHAPKEY1) + $(NVMEOF_CLI) host add --subsystem $(SUBNQN2) --host-nqn $(HOSTNQN2) --dhchap-key $(DHCHAPKEY2) + $(NVMEOF_CLI) host add --subsystem $(SUBNQN1) --host-nqn $(HOSTNQN3) + $(NVMEOF_CLI) namespace add_host --subsystem $(SUBNQN1) --nsid 2 --host-nqn $(HOSTNQN) + $(NVMEOF_CLI) host add --subsystem $(SUBNQN1) --host-nqn $(HOSTNQN4) --dhchap-key $(DHCHAPKEY4) --psk $(PSKKEY1) .PHONY: demosecuredhchap diff --git a/tests/ha/demo_test.sh b/tests/ha/demo_test.sh index 208f185b..7a501782 100755 --- a/tests/ha/demo_test.sh +++ b/tests/ha/demo_test.sh @@ -48,14 +48,16 @@ function demo_test_dhchap() echo -n "${DHCHAP_KEY7}" > /tmp/temp-dhchap/dhchap/${NQN}/key4 echo -n "${DHCHAP_KEY8}" > /tmp/temp-dhchap/dhchap/${NQN}/key5 echo -n "${PSK_KEY1}" > /tmp/temp-dhchap/dhchap/${NQN}/key6 + echo -n "${DHCHAP_KEY9}" > /tmp/temp-dhchap/dhchap/${NQN}/key7 chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key1 chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key2 chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key3 chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key4 chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key5 chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key6 + chmod 0600 /tmp/temp-dhchap/dhchap/${NQN}/key7 - make demosecuredhchap OPTS=-T HOSTNQN="${NQN}host" HOSTNQN2="${NQN}host2" HOSTNQN3="${NQN}host3" HOSTNQN4="${NQN}host4" NVMEOF_IO_PORT2=${port2} NVMEOF_IO_PORT3=${port3} NVMEOF_IO_PORT4=${port4} DHCHAPKEY1="${DHCHAP_KEY4}" DHCHAPKEY2="${DHCHAP_KEY5}" DHCHAPKEY3="${DHCHAP_KEY6}" DHCHAPKEY4="${DHCHAP_KEY8}" PSKKEY1="${PSK_KEY1}" + make demosecuredhchap OPTS=-T SUBNQN1="${NQN}" SUBNQN2="${NQN}2" HOSTNQN="${NQN}host" HOSTNQN2="${NQN}host2" HOSTNQN3="${NQN}host3" HOSTNQN4="${NQN}host4" NVMEOF_IO_PORT2=${port2} NVMEOF_IO_PORT3=${port3} NVMEOF_IO_PORT4=${port4} DHCHAPKEY1="${DHCHAP_KEY4}" DHCHAPKEY2="${DHCHAP_KEY5}" DHCHAPKEY3="${DHCHAP_KEY6}" DHCHAPKEY4="${DHCHAP_KEY8}" PSKKEY1="${PSK_KEY1}" } function demo_bdevperf_unsecured() @@ -358,6 +360,7 @@ function demo_bdevperf_dhchap() make exec SVC=bdevperf OPTS=-T CMD="chmod 0600 ${dhchap_path}/key4" make exec SVC=bdevperf OPTS=-T CMD="chmod 0600 ${dhchap_path}/key5" make exec SVC=bdevperf OPTS=-T CMD="chmod 0600 ${dhchap_path}/key6" + make exec SVC=bdevperf OPTS=-T CMD="chmod 0600 ${dhchap_path}/key7" rm -rf /tmp/temp-dhchap echo "ℹ️ bdevperf add DHCHAP key name key1 to keyring" @@ -372,6 +375,8 @@ function demo_bdevperf_dhchap() make -s exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET keyring_file_add_key key5 ${dhchap_path}/key5" echo "ℹ️ bdevperf add DHCHAP key name key6 to keyring" make -s exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET keyring_file_add_key key6 ${dhchap_path}/key6" + echo "ℹ️ bdevperf add DHCHAP key name key7 to keyring" + make -s exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET keyring_file_add_key key7 ${dhchap_path}/key7" echo "ℹ️ bdevperf list keyring" make -s exec SVC=bdevperf OPTS=-T CMD="$rpc -s $BDEVPERF_SOCKET keyring_get_keys" @@ -397,7 +402,7 @@ function demo_bdevperf_dhchap() set +e echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${port2} nqn: ${NQN}host2 using DHCHAP controller, wrong key" - make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme1 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port2} -f ipv4 -n ${NQN} -q ${NQN}host2 -l -1 -o 10 --dhchap-key key2 --dhchap-ctrlr-key key1" + make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme1 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port2} -f ipv4 -n ${NQN}2 -q ${NQN}host2 -l -1 -o 10 --dhchap-key key2 --dhchap-ctrlr-key key1" if [[ $? -eq 0 ]]; then echo "Connecting using the wrong DHCAP controller key should fail" exit 1 @@ -405,7 +410,7 @@ function demo_bdevperf_dhchap() set -e echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${port2} nqn: ${NQN}host2 using DHCHAP controller" - make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme1 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port2} -f ipv4 -n ${NQN} -q ${NQN}host2 -l -1 -o 10 --dhchap-key key2 --dhchap-ctrlr-key key3" + make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme1 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port2} -f ipv4 -n ${NQN}2 -q ${NQN}host2 -l -1 -o 10 --dhchap-key key2 --dhchap-ctrlr-key key3" echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${port3} nqn: ${NQN}host3 not using DHCHAP" make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme2 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port3} -f ipv4 -n ${NQN} -q ${NQN}host3 -l -1 -o 10" @@ -415,6 +420,7 @@ function demo_bdevperf_dhchap() echo "ℹ️ verify connection list" conns=`cephnvmf_func --output stdio --format json connection list --subsystem $NQN` + conns2=`cephnvmf_func --output stdio --format json connection list --subsystem ${NQN}2` [[ `echo $conns | jq -r '.status'` == "0" ]] [[ `echo $conns | jq -r '.subsystem_nqn'` == "${NQN}" ]] @@ -431,42 +437,47 @@ function demo_bdevperf_dhchap() [[ `echo $conns | jq -r '.connections[0].use_psk'` == "false" ]] [[ `echo $conns | jq -r '.connections[0].use_dhchap'` == "true" ]] - [[ `echo $conns | jq -r '.connections[1].nqn'` == "${NQN}host2" ]] - [[ `echo $conns | jq -r '.connections[1].trsvcid'` == "${port2}" ]] + [[ `echo $conns | jq -r '.connections[1].nqn'` == "${NQN}host3" ]] + [[ `echo $conns | jq -r '.connections[1].trsvcid'` == "${port3}" ]] [[ `echo $conns | jq -r '.connections[1].traddr'` == "${NVMEOF_IP_ADDRESS}" ]] [[ `echo $conns | jq -r '.connections[1].adrfam'` == "ipv4" ]] [[ `echo $conns | jq -r '.connections[1].trtype'` == "TCP" ]] [[ `echo $conns | jq -r '.connections[1].qpairs_count'` == "1" ]] - [[ `echo $conns | jq -r '.connections[1].controller_id'` == "5" ]] + [[ `echo $conns | jq -r '.connections[1].controller_id'` == "4" ]] [[ `echo $conns | jq -r '.connections[1].connected'` == "true" ]] [[ `echo $conns | jq -r '.connections[1].secure'` == "false" ]] [[ `echo $conns | jq -r '.connections[1].use_psk'` == "false" ]] - [[ `echo $conns | jq -r '.connections[1].use_dhchap'` == "true" ]] + [[ `echo $conns | jq -r '.connections[1].use_dhchap'` == "false" ]] - [[ `echo $conns | jq -r '.connections[2].nqn'` == "${NQN}host3" ]] - [[ `echo $conns | jq -r '.connections[2].trsvcid'` == "${port3}" ]] - [[ `echo $conns | jq -r '.connections[2].traddr'` == "${NVMEOF_IP_ADDRESS}" ]] + [[ `echo $conns | jq -r '.connections[2].nqn'` == "${NQN}host4" ]] + [[ `echo $conns | jq -r '.connections[2].trsvcid'` == 0 ]] + [[ `echo $conns | jq -r '.connections[2].traddr'` == "" ]] [[ `echo $conns | jq -r '.connections[2].adrfam'` == "ipv4" ]] - [[ `echo $conns | jq -r '.connections[2].trtype'` == "TCP" ]] - [[ `echo $conns | jq -r '.connections[2].qpairs_count'` == "1" ]] - [[ `echo $conns | jq -r '.connections[2].controller_id'` == "6" ]] - [[ `echo $conns | jq -r '.connections[2].connected'` == "true" ]] - [[ `echo $conns | jq -r '.connections[2].secure'` == "false" ]] - [[ `echo $conns | jq -r '.connections[2].use_psk'` == "false" ]] - [[ `echo $conns | jq -r '.connections[2].use_dhchap'` == "false" ]] + [[ `echo $conns | jq -r '.connections[2].trtype'` == "" ]] + [[ `echo $conns | jq -r '.connections[2].qpairs_count'` == -1 ]] + [[ `echo $conns | jq -r '.connections[2].controller_id'` == -1 ]] + [[ `echo $conns | jq -r '.connections[2].connected'` == "false" ]] + [[ `echo $conns | jq -r '.connections[2].use_psk'` == "true" ]] + [[ `echo $conns | jq -r '.connections[2].use_dhchap'` == "true" ]] + + [[ `echo $conns | jq -r '.connections[3]'` == "null" ]] - [[ `echo $conns | jq -r '.connections[3].nqn'` == "${NQN}host4" ]] - [[ `echo $conns | jq -r '.connections[3].trsvcid'` == 0 ]] - [[ `echo $conns | jq -r '.connections[3].traddr'` == "" ]] - [[ `echo $conns | jq -r '.connections[3].adrfam'` == "ipv4" ]] - [[ `echo $conns | jq -r '.connections[3].trtype'` == "" ]] - [[ `echo $conns | jq -r '.connections[3].qpairs_count'` == -1 ]] - [[ `echo $conns | jq -r '.connections[3].controller_id'` == -1 ]] - [[ `echo $conns | jq -r '.connections[3].connected'` == "false" ]] - [[ `echo $conns | jq -r '.connections[3].use_psk'` == "true" ]] - [[ `echo $conns | jq -r '.connections[3].use_dhchap'` == "true" ]] + [[ `echo $conns2 | jq -r '.status'` == "0" ]] + [[ `echo $conns2 | jq -r '.subsystem_nqn'` == "${NQN}2" ]] - [[ `echo $conns | jq -r '.connections[4]'` == "null" ]] + [[ `echo $conns2 | jq -r '.connections[0].nqn'` == "${NQN}host2" ]] + [[ `echo $conns2 | jq -r '.connections[0].trsvcid'` == "${port2}" ]] + [[ `echo $conns2 | jq -r '.connections[0].traddr'` == "${NVMEOF_IP_ADDRESS}" ]] + [[ `echo $conns2 | jq -r '.connections[0].adrfam'` == "ipv4" ]] + [[ `echo $conns2 | jq -r '.connections[0].trtype'` == "TCP" ]] + [[ `echo $conns2 | jq -r '.connections[0].qpairs_count'` == "1" ]] + [[ `echo $conns2 | jq -r '.connections[0].controller_id'` == "2" ]] + [[ `echo $conns2 | jq -r '.connections[0].connected'` == "true" ]] + [[ `echo $conns2 | jq -r '.connections[0].secure'` == "false" ]] + [[ `echo $conns2 | jq -r '.connections[0].use_psk'` == "false" ]] + [[ `echo $conns2 | jq -r '.connections[0].use_dhchap'` == "true" ]] + + [[ `echo $conns2 | jq -r '.connections[1]'` == "null" ]] echo "ℹ️ bdevperf perform_tests" eval $(make run SVC=bdevperf OPTS="--entrypoint=env" | grep BDEVPERF_TEST_DURATION | tr -d '\n\r' ) @@ -496,11 +507,22 @@ function demo_bdevperf_dhchap() name5_pre=`echo ${dhchap_key_list_pre_change} | jq -r '.[4].name'` [[ `echo $dhchap_key_list_pre_change | jq -r '.[4].removed'` == "true" ]] make exec SVC=nvmeof OPTS=-T CMD="test -f ${path1_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path2_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path3_pre}" make exec SVC=nvmeof OPTS=-T CMD="test -f ${path4_pre}" echo "ℹ️ change the key for host ${NQN}host" - cephnvmf_func host change_keys --subsystem $NQN --host-nqn ${NQN}host --dhchap-key "${DHCHAP_KEY7}" + cephnvmf_func host change_key --subsystem $NQN --host-nqn ${NQN}host --dhchap-key "${DHCHAP_KEY7}" --force make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path1_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path2_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path3_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path4_pre}" + + echo "ℹ️ change the key for subsystem ${NQN}2" + cephnvmf_func subsystem change_key --subsystem ${NQN}2 --dhchap-key "${DHCHAP_KEY9}" + make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path2_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path3_pre}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path4_pre}" echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${NVMEOF_IO_PORT} nqn: ${NQN}host using previous DHCHAP key" set +e @@ -515,6 +537,19 @@ function demo_bdevperf_dhchap() make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme5 -t tcp -a $NVMEOF_IP_ADDRESS -s ${NVMEOF_IO_PORT} -f ipv4 -n ${NQN} -q ${NQN}host -l -1 -o 10 --dhchap-key key4" make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_detach_controller Nvme5" + echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${port2} nqn: ${NQN}host2 using previous DHCHAP controller key" + set +e + make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme7 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port2} -f ipv4 -n ${NQN}2 -q ${NQN}host2 -l -1 -o 10 --dhchap-key key2 --dhchap-ctrlr-key key1" + if [[ $? -eq 0 ]]; then + echo "Connecting using the previous DHCAP controller key should fail" + exit 1 + fi + set -e + + echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${port2} nqn: ${NQN}host2 using the new DHCHAP controller key" + make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme8 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port2} -f ipv4 -n ${NQN}2 -q ${NQN}host2 -l -1 -o 10 --dhchap-key key2 --dhchap-ctrlr-key key7" + make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_detach_controller Nvme8" + echo "ℹ️ bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: ${port4} nqn: ${NQN}host4 using no PSK key" set +e make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme6 -t tcp -a $NVMEOF_IP_ADDRESS -s ${port4} -f ipv4 -n ${NQN} -q ${NQN}host4 -l -1 -o 10 --dhchap-key key5" @@ -529,6 +564,7 @@ function demo_bdevperf_dhchap() echo "ℹ️ verify connection list with PSK" conns=`cephnvmf_func --output stdio --format json connection list --subsystem $NQN` + conns2=`cephnvmf_func --output stdio --format json connection list --subsystem ${NQN}2` [[ `echo $conns | jq -r '.status'` == "0" ]] [[ `echo $conns | jq -r '.subsystem_nqn'` == "${NQN}" ]] @@ -539,7 +575,7 @@ function demo_bdevperf_dhchap() [[ `echo $conns | jq -r '.connections[0].adrfam'` == "ipv4" ]] [[ `echo $conns | jq -r '.connections[0].trtype'` == "TCP" ]] [[ `echo $conns | jq -r '.connections[0].qpairs_count'` == "1" ]] - [[ `echo $conns | jq -r '.connections[0].controller_id'` == "9" ]] + [[ `echo $conns | jq -r '.connections[0].controller_id'` == "7" ]] [[ `echo $conns | jq -r '.connections[0].connected'` == "true" ]] [[ `echo $conns | jq -r '.connections[0].secure'` == "true" ]] [[ `echo $conns | jq -r '.connections[0].use_psk'` == "true" ]] @@ -556,7 +592,7 @@ function demo_bdevperf_dhchap() [[ `echo $conns | jq -r '.connections[1].use_psk'` == "false" ]] [[ `echo $conns | jq -r '.connections[1].use_dhchap'` == "false" ]] - [[ `echo $conns | jq -r '.connections[2].nqn'` == "${NQN}host2" ]] + [[ `echo $conns | jq -r '.connections[2].nqn'` == "${NQN}host" ]] [[ `echo $conns | jq -r '.connections[2].trsvcid'` == 0 ]] [[ `echo $conns | jq -r '.connections[2].traddr'` == "" ]] [[ `echo $conns | jq -r '.connections[2].adrfam'` == "ipv4" ]] @@ -567,18 +603,23 @@ function demo_bdevperf_dhchap() [[ `echo $conns | jq -r '.connections[2].use_psk'` == "false" ]] [[ `echo $conns | jq -r '.connections[2].use_dhchap'` == "true" ]] - [[ `echo $conns | jq -r '.connections[3].nqn'` == "${NQN}host" ]] - [[ `echo $conns | jq -r '.connections[3].trsvcid'` == 0 ]] - [[ `echo $conns | jq -r '.connections[3].traddr'` == "" ]] - [[ `echo $conns | jq -r '.connections[3].adrfam'` == "ipv4" ]] - [[ `echo $conns | jq -r '.connections[3].trtype'` == "" ]] - [[ `echo $conns | jq -r '.connections[3].qpairs_count'` == -1 ]] - [[ `echo $conns | jq -r '.connections[3].controller_id'` == -1 ]] - [[ `echo $conns | jq -r '.connections[3].connected'` == "false" ]] - [[ `echo $conns | jq -r '.connections[3].use_psk'` == "false" ]] - [[ `echo $conns | jq -r '.connections[3].use_dhchap'` == "true" ]] + [[ `echo $conns | jq -r '.connections[3]'` == "null" ]] + + [[ `echo $conns2 | jq -r '.status'` == "0" ]] + [[ `echo $conns2 | jq -r '.subsystem_nqn'` == "${NQN}2" ]] - [[ `echo $conns | jq -r '.connections[4]'` == "null" ]] + [[ `echo $conns2 | jq -r '.connections[0].nqn'` == "${NQN}host2" ]] + [[ `echo $conns2 | jq -r '.connections[0].trsvcid'` == 0 ]] + [[ `echo $conns2 | jq -r '.connections[0].traddr'` == "" ]] + [[ `echo $conns2 | jq -r '.connections[0].adrfam'` == "ipv4" ]] + [[ `echo $conns2 | jq -r '.connections[0].trtype'` == "" ]] + [[ `echo $conns2 | jq -r '.connections[0].qpairs_count'` == -1 ]] + [[ `echo $conns2 | jq -r '.connections[0].controller_id'` == -1 ]] + [[ `echo $conns2 | jq -r '.connections[0].connected'` == "false" ]] + [[ `echo $conns2 | jq -r '.connections[0].use_psk'` == "false" ]] + [[ `echo $conns2 | jq -r '.connections[0].use_dhchap'` == "true" ]] + + [[ `echo $conns2 | jq -r '.connections[1]'` == "null" ]] echo "ℹ️ verify DHCHAP key files removal" dhchap_key_list=`make -s exec SVC=nvmeof OPTS=-T CMD="/usr/local/bin/spdk_rpc -s /var/tmp/spdk.sock keyring_get_keys"` @@ -586,6 +627,7 @@ function demo_bdevperf_dhchap() path2=`echo ${dhchap_key_list} | jq -r '.[1].path'` path3=`echo ${dhchap_key_list} | jq -r '.[2].path'` path4=`echo ${dhchap_key_list} | jq -r '.[3].path'` + path5=`echo ${dhchap_key_list} | jq -r '.[4].path'` name1=`echo ${dhchap_key_list} | jq -r '.[0].name'` name2=`echo ${dhchap_key_list} | jq -r '.[1].name'` name3=`echo ${dhchap_key_list} | jq -r '.[2].name'` @@ -595,21 +637,50 @@ function demo_bdevperf_dhchap() [[ "$path1_pre" != "$path2" ]] [[ "$path1_pre" != "$path3" ]] [[ "$path1_pre" != "$path4" ]] + [[ "$path1_pre" != "$path5" ]] [[ "$name1_pre" != "$name1" ]] - [[ "$name1_pre" != "$name2" ]] + [[ "$name1_pre" == "$name2" ]] [[ "$name1_pre" != "$name3" ]] - [[ "$name1_pre" == "$name4" ]] + [[ "$name1_pre" != "$name4" ]] [[ "$name1_pre" != "$name5" ]] - [[ "$path2_pre" == "$path1" ]] - [[ "$name2_pre" == "$name1" ]] - [[ "$path3_pre" == "$path2" ]] - [[ "$name3_pre" == "$name2" ]] - [[ "$path4_pre" == "$path3" ]] - [[ "$name4_pre" == "$name3" ]] + [[ "$path2_pre" != "$path1" ]] + [[ "$path2_pre" != "$path2" ]] + [[ "$path2_pre" != "$path3" ]] + [[ "$path2_pre" != "$path4" ]] + [[ "$path2_pre" != "$path5" ]] + [[ "$name2_pre" != "$name1" ]] + [[ "$name2_pre" != "$name2" ]] + [[ "$name2_pre" == "$name3" ]] + [[ "$name2_pre" != "$name4" ]] + [[ "$name2_pre" != "$name5" ]] + [[ "$path3_pre" != "$path1" ]] + [[ "$path3_pre" != "$path2" ]] + [[ "$path3_pre" != "$path3" ]] + [[ "$path3_pre" != "$path4" ]] + [[ "$path3_pre" != "$path5" ]] + [[ "$name3_pre" != "$name1" ]] + [[ "$name3_pre" != "$name2" ]] + [[ "$name3_pre" != "$name3" ]] + [[ "$name3_pre" == "$name4" ]] + [[ "$name3_pre" != "$name5" ]] + [[ "$path4_pre" == "$path1" ]] + [[ "$path4_pre" != "$path2" ]] + [[ "$path4_pre" != "$path3" ]] + [[ "$path4_pre" != "$path4" ]] + [[ "$path4_pre" != "$path5" ]] + [[ "$name4_pre" == "$name1" ]] + [[ "$name4_pre" != "$name2" ]] + [[ "$name4_pre" != "$name3" ]] + [[ "$name4_pre" != "$name4" ]] + [[ "$name4_pre" != "$name5" ]] + [[ "$name5_pre" != "$name1" ]] + [[ "$name5_pre" != "$name2" ]] + [[ "$name5_pre" != "$name3" ]] + [[ "$name5_pre" != "$name4" ]] [[ "$name5_pre" == "$name5" ]] - [[ "$path3" != "$path2_pre" ]] - [[ "$path3" != "$path3_pre" ]] + subsys_dir=`dirname ${path1}` + subsys2_dir=`dirname ${path3}` [[ `echo $dhchap_key_list | jq -r '.[0].removed'` == "false" ]] [[ `echo $dhchap_key_list | jq -r '.[1].removed'` == "false" ]] [[ `echo $dhchap_key_list | jq -r '.[2].removed'` == "false" ]] @@ -621,15 +692,18 @@ function demo_bdevperf_dhchap() make exec SVC=nvmeof OPTS=-T CMD="test -f ${path3}" make exec SVC=nvmeof OPTS=-T CMD="test -f ${path4}" make exec SVC=nvmeof OPTS=-T CMD="test -d ${subsys_dir}" - cephnvmf_func host del --subsystem $NQN --host-nqn ${NQN}host2 - make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path1}" - make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path2}" - make exec SVC=nvmeof OPTS=-T CMD="test -f ${path3}" - make exec SVC=nvmeof OPTS=-T CMD="test -f ${path4}" - cephnvmf_func subsystem del --subsystem $NQN --force + cephnvmf_func host del --subsystem ${NQN}2 --host-nqn ${NQN}host2 make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path3}" make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path4}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path1}" + make exec SVC=nvmeof OPTS=-T CMD="test -f ${path2}" + make exec SVC=nvmeof OPTS=-T CMD="test -d ${subsys2_dir}" + cephnvmf_func subsystem del --subsystem $NQN --force + make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path1}" + make exec SVC=nvmeof OPTS=-T CMD="test ! -f ${path2}" make exec SVC=nvmeof OPTS=-T CMD="test ! -d ${subsys_dir}" + cephnvmf_func subsystem del --subsystem ${NQN}2 --force + make exec SVC=nvmeof OPTS=-T CMD="test ! -d ${subsys2_dir}" dhchap_key_list=`make -s exec SVC=nvmeof OPTS=-T CMD="/usr/local/bin/spdk_rpc -s /var/tmp/spdk.sock keyring_get_keys"` [[ `echo $dhchap_key_list | jq -r '.[0]'` == "null" ]] diff --git a/tests/test_cli_change_keys.py b/tests/test_cli_change_keys.py index e0ec913c..145dc2de 100644 --- a/tests/test_cli_change_keys.py +++ b/tests/test_cli_change_keys.py @@ -73,7 +73,7 @@ def two_gateways(config): gatewayA.server.stop(grace=1) gatewayB.server.stop(grace=1) -def test_change_host_keys(caplog, two_gateways): +def test_change_host_key(caplog, two_gateways): gatewayA, stubA, gatewayB, stubB = two_gateways gwA = gatewayA.gateway_rpc gwB = gatewayB.gateway_rpc @@ -86,19 +86,21 @@ def test_change_host_keys(caplog, two_gateways): caplog.clear() cli(["--server-port", "5501", "host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn2, "--dhchap-key", key1]) assert f"Adding host {hostnqn2} to {subsystem}: Successful" in caplog.text + assert f"Host {hostnqn2} has a DH-HMAC-CHAP key but subsystem {subsystem} has no key, a unidirectional authentication will be used" in caplog.text caplog.clear() - cli(["--server-port", "5501", "host", "change_keys", "--subsystem", subsystem, "--host-nqn", hostnqn1, "--dhchap-key", key2]) - assert f"Changing keys for host {hostnqn1} on subsystem {subsystem}: Successful" in caplog.text + cli(["--server-port", "5501", "host", "change_key", "--subsystem", subsystem, "--host-nqn", hostnqn1, "--dhchap-key", key2]) + assert f"Changing key for host {hostnqn1} on subsystem {subsystem}: Successful" in caplog.text + assert f"Host {hostnqn1} has a DH-HMAC-CHAP key but subsystem {subsystem} has no key, a unidirectional authentication will be used" in caplog.text time.sleep(15) - assert f"Received request to change inband authentication keys for host {hostnqn1} on subsystem {subsystem}, dhchap: {key2}, dhchap controller: , context: