diff --git a/sros2/setup.py b/sros2/setup.py index b09ba853..a0ac928e 100644 --- a/sros2/setup.py +++ b/sros2/setup.py @@ -66,7 +66,10 @@ def package_files(directory): ':CreatePermissionVerb', 'distribute_key = sros2.verb.distribute_key:DistributeKeyVerb', 'generate_artifacts = sros2.verb.generate_artifacts:GenerateArtifactsVerb', - 'generate_policy = sros2.verb.generate_policy:GeneratePolicyVerb', + # TODO(ivanpauno): Reactivate this after having a way to introspect + # security context names in rclpy. + # Related with https://github.com/ros2/rclpy/issues/529. + # 'generate_policy = sros2.verb.generate_policy:GeneratePolicyVerb', 'list_keys = sros2.verb.list_keys:ListKeysVerb', ], }, diff --git a/sros2/sros2/api/__init__.py b/sros2/sros2/api/__init__.py index fb505498..74c8d32c 100644 --- a/sros2/sros2/api/__init__.py +++ b/sros2/sros2/api/__init__.py @@ -14,8 +14,8 @@ from collections import namedtuple import datetime +import errno import os -import shutil import sys from cryptography import x509 @@ -28,9 +28,8 @@ from lxml import etree from rclpy.exceptions import InvalidNamespaceException -from rclpy.exceptions import InvalidNodeNameException +from rclpy.utilities import get_rmw_implementation_identifier from rclpy.validate_namespace import validate_namespace -from rclpy.validate_node_name import validate_node_name from sros2.policy import ( get_policy_default, @@ -48,6 +47,12 @@ NodeName = namedtuple('NodeName', ('node', 'ns', 'fqn')) TopicInfo = namedtuple('Topic', ('fqn', 'type')) +KS_CONTEXT = 'contexts' +KS_PUBLIC = 'public' +KS_PRIVATE = 'private' + +RMW_WITH_ROS_GRAPH_INFO_TOPIC = ('rmw_fastrtps_cpp', 'rmw_fastrtps_dynamic_cpp') + def get_node_names(*, node, include_hidden_nodes=False): node_names_and_namespaces = node.get_node_names_and_namespaces() @@ -141,24 +146,57 @@ def create_governance_file(path, domain_id): f.write(etree.tostring(governance_xml, pretty_print=True)) +def _create_symlink(*, src, dst): + if os.path.exists(dst): + src_abs_path = os.path.join(os.path.dirname(dst), src) + if os.path.samefile(src_abs_path, dst): + return + print(f"Existing symlink '{dst}' does not match '{src_abs_path}', overriding it!") + os.remove(dst) + os.symlink(src=src, dst=dst) + + def create_keystore(keystore_path): - if not os.path.exists(keystore_path): - print('creating directory: %s' % keystore_path) - os.makedirs(keystore_path, exist_ok=True) + if not is_valid_keystore(keystore_path): + print('creating keystore: %s' % keystore_path) else: - print('directory already exists: %s' % keystore_path) - - ca_key_path = os.path.join(keystore_path, 'ca.key.pem') - ca_cert_path = os.path.join(keystore_path, 'ca.cert.pem') + print('keystore already exists: %s' % keystore_path) + return + + os.makedirs(keystore_path, exist_ok=True) + os.makedirs(os.path.join(keystore_path, KS_PUBLIC), exist_ok=True) + os.makedirs(os.path.join(keystore_path, KS_PRIVATE), exist_ok=True) + os.makedirs(os.path.join(keystore_path, KS_CONTEXT), exist_ok=True) + + keystore_ca_cert_path = os.path.join(keystore_path, KS_PUBLIC, 'ca.cert.pem') + keystore_ca_key_path = os.path.join(keystore_path, KS_PRIVATE, 'ca.key.pem') + + keystore_permissions_ca_cert_path = os.path.join( + keystore_path, KS_PUBLIC, 'permissions_ca.cert.pem') + keystore_permissions_ca_key_path = os.path.join( + keystore_path, KS_PRIVATE, 'permissions_ca.key.pem') + keystore_identity_ca_cert_path = os.path.join(keystore_path, KS_PUBLIC, 'identity_ca.cert.pem') + keystore_identity_ca_key_path = os.path.join(keystore_path, KS_PRIVATE, 'identity_ca.key.pem') + + required_files = ( + keystore_permissions_ca_cert_path, + keystore_permissions_ca_key_path, + keystore_identity_ca_cert_path, + keystore_identity_ca_key_path, + ) - if not (os.path.isfile(ca_key_path) and os.path.isfile(ca_cert_path)): + if not all(os.path.isfile(x) for x in required_files): print('creating new CA key/cert pair') - create_ca_key_cert(ca_key_path, ca_cert_path) + create_ca_key_cert(keystore_ca_key_path, keystore_ca_cert_path) + _create_symlink(src='ca.cert.pem', dst=keystore_permissions_ca_cert_path) + _create_symlink(src='ca.key.pem', dst=keystore_permissions_ca_key_path) + _create_symlink(src='ca.cert.pem', dst=keystore_identity_ca_cert_path) + _create_symlink(src='ca.key.pem', dst=keystore_identity_ca_key_path) else: print('found CA key and cert, not creating new ones!') # create governance file - gov_path = os.path.join(keystore_path, 'governance.xml') + gov_path = os.path.join(keystore_path, KS_CONTEXT, 'governance.xml') if not os.path.isfile(gov_path): print('creating governance file: %s' % gov_path) domain_id = os.getenv(DOMAIN_ID_ENV, '0') @@ -167,10 +205,14 @@ def create_keystore(keystore_path): print('found governance file, not creating a new one!') # sign governance file - signed_gov_path = os.path.join(keystore_path, 'governance.p7s') + signed_gov_path = os.path.join(keystore_path, KS_CONTEXT, 'governance.p7s') if not os.path.isfile(signed_gov_path): print('creating signed governance file: %s' % signed_gov_path) - _create_smime_signed_file(ca_cert_path, ca_key_path, gov_path, signed_gov_path) + _create_smime_signed_file( + keystore_permissions_ca_cert_path, + keystore_permissions_ca_key_path, + gov_path, + signed_gov_path) else: print('found signed governance file, not creating a new one!') @@ -181,34 +223,36 @@ def create_keystore(keystore_path): def is_valid_keystore(path): return ( - os.path.isfile(os.path.join(path, 'ca.key.pem')) and - os.path.isfile(os.path.join(path, 'ca.cert.pem')) and - os.path.isfile(os.path.join(path, 'governance.p7s')) + os.path.isfile(os.path.join(path, KS_PUBLIC, 'permissions_ca.cert.pem')) and + os.path.isfile(os.path.join(path, KS_PUBLIC, 'identity_ca.cert.pem')) and + os.path.isfile(os.path.join(path, KS_PRIVATE, 'permissions_ca.key.pem')) and + os.path.isfile(os.path.join(path, KS_PRIVATE, 'identity_ca.key.pem')) and + os.path.isfile(os.path.join(path, KS_CONTEXT, 'governance.p7s')) ) def is_key_name_valid(name): - ns_and_name = name.rsplit('/', 1) - if len(ns_and_name) != 2: - print("The key name needs to start with '/'") - return False - node_ns = ns_and_name[0] if ns_and_name[0] else '/' - node_name = ns_and_name[1] - + # TODO(ivanpauno): Use validate_security_context_name when it's propagated to `rclpy`. + # This is not to bad for the moment. + # Related with https://github.com/ros2/rclpy/issues/528. try: - return (validate_namespace(node_ns) and validate_node_name(node_name)) - except (InvalidNamespaceException, InvalidNodeNameException) as e: - print('{}'.format(e)) + return validate_namespace(name) + except InvalidNamespaceException as e: + print(f'{e}') return False def create_permission_file(path, domain_id, policy_element): + print('creating permission') permissions_xsl_path = get_transport_template('dds', 'permissions.xsl') permissions_xsl = etree.XSLT(etree.parse(permissions_xsl_path)) permissions_xsd_path = get_transport_schema('dds', 'permissions.xsd') permissions_xsd = etree.XMLSchema(etree.parse(permissions_xsd_path)) - permissions_xml = permissions_xsl(policy_element) + kwargs = {} + if get_rmw_implementation_identifier() in RMW_WITH_ROS_GRAPH_INFO_TOPIC: + kwargs['allow_ros_discovery_topic'] = etree.XSLT.strparam('1') + permissions_xml = permissions_xsl(policy_element, **kwargs) domain_id_elements = permissions_xml.findall('permissions/grant/*/domains/id') for domain_id_element in domain_id_elements: @@ -229,20 +273,14 @@ def get_policy(name, policy_file_path): def get_policy_from_tree(name, policy_tree): - ns, node = name.rsplit('/', 1) - ns = '/' if not ns else ns - profile_element = policy_tree.find( - path='profiles/profile[@ns="{ns}"][@node="{node}"]'.format( - ns=ns, - node=node)) - if profile_element is None: - raise RuntimeError('unable to find profile "{name}"'.format( - name=name - )) - profiles_element = etree.Element('profiles') - profiles_element.append(profile_element) + context_element = policy_tree.find( + path=f'contexts/context[@path="{name}"]') + if context_element is None: + raise RuntimeError(f'unable to find context "{name}"') + contexts_element = etree.Element('contexts') + contexts_element.append(context_element) policy_element = etree.Element('policy') - policy_element.append(profiles_element) + policy_element.append(contexts_element) return policy_element @@ -255,14 +293,14 @@ def create_permission(keystore_path, identity, policy_file_path): def create_permissions_from_policy_element(keystore_path, identity, policy_element): domain_id = os.getenv(DOMAIN_ID_ENV, '0') relative_path = os.path.normpath(identity.lstrip('/')) - key_dir = os.path.join(keystore_path, relative_path) + key_dir = os.path.join(keystore_path, KS_CONTEXT, relative_path) print("creating permission file for identity: '%s'" % identity) permissions_path = os.path.join(key_dir, 'permissions.xml') create_permission_file(permissions_path, domain_id, policy_element) signed_permissions_path = os.path.join(key_dir, 'permissions.p7s') - keystore_ca_cert_path = os.path.join(keystore_path, 'ca.cert.pem') - keystore_ca_key_path = os.path.join(keystore_path, 'ca.key.pem') + keystore_ca_cert_path = os.path.join(keystore_path, KS_PUBLIC, 'ca.cert.pem') + keystore_ca_key_path = os.path.join(keystore_path, KS_PRIVATE, 'ca.key.pem') _create_smime_signed_file( keystore_ca_cert_path, keystore_ca_key_path, permissions_path, signed_permissions_path) @@ -276,63 +314,72 @@ def create_key(keystore_path, identity): print("creating key for identity: '%s'" % identity) relative_path = os.path.normpath(identity.lstrip('/')) - key_dir = os.path.join(keystore_path, relative_path) + key_dir = os.path.join(keystore_path, KS_CONTEXT, relative_path) os.makedirs(key_dir, exist_ok=True) - keystore_ca_key_path = os.path.join(keystore_path, 'ca.key.pem') - keystore_ca_cert_path = os.path.join(keystore_path, 'ca.cert.pem') - # symlink the CA cert in there public_certs = ['identity_ca.cert.pem', 'permissions_ca.cert.pem'] for public_cert in public_certs: dst = os.path.join(key_dir, public_cert) + keystore_ca_cert_path = os.path.join(keystore_path, KS_PUBLIC, public_cert) relativepath = os.path.relpath(keystore_ca_cert_path, key_dir) - try: - os.symlink(src=relativepath, dst=dst) - except FileExistsError as e: - if not os.path.samefile(keystore_ca_cert_path, dst): - print('Existing symlink does not match!') - raise RuntimeError(str(e)) - - # copy the governance file in there - keystore_governance_path = os.path.join(keystore_path, 'governance.p7s') + _create_symlink(src=relativepath, dst=dst) + + # symlink the governance file in there + keystore_governance_path = os.path.join(keystore_path, KS_CONTEXT, 'governance.p7s') dest_governance_path = os.path.join(key_dir, 'governance.p7s') - shutil.copyfile(keystore_governance_path, dest_governance_path) + relativepath = os.path.relpath(keystore_governance_path, key_dir) + _create_symlink(src=relativepath, dst=dest_governance_path) + + keystore_identity_ca_cert_path = os.path.join(keystore_path, KS_PUBLIC, 'identity_ca.cert.pem') + keystore_identity_ca_key_path = os.path.join(keystore_path, KS_PRIVATE, 'identity_ca.key.pem') cert_path = os.path.join(key_dir, 'cert.pem') key_path = os.path.join(key_dir, 'key.pem') if not os.path.isfile(cert_path) or not os.path.isfile(key_path): print('creating cert and key') _create_key_and_cert( - keystore_ca_cert_path, keystore_ca_key_path, identity, cert_path, key_path) + keystore_identity_ca_cert_path, + keystore_identity_ca_key_path, + identity, + cert_path, + key_path + ) else: print('found cert and key; not creating new ones!') # create a wildcard permissions file for this node which can be overridden # later using a policy if desired policy_file_path = get_policy_default('policy.xml') - policy_element = get_policy('/default', policy_file_path) - profile_element = policy_element.find('profiles/profile') - ns, node = identity.rsplit('/', 1) - ns = '/' if not ns else ns - profile_element.attrib['ns'] = ns - profile_element.attrib['node'] = node + policy_element = get_policy('/', policy_file_path) + context_element = policy_element.find('contexts/context') + context_element.attrib['path'] = identity permissions_path = os.path.join(key_dir, 'permissions.xml') domain_id = os.getenv(DOMAIN_ID_ENV, '0') create_permission_file(permissions_path, domain_id, policy_element) signed_permissions_path = os.path.join(key_dir, 'permissions.p7s') - keystore_ca_key_path = os.path.join(keystore_path, 'ca.key.pem') + keystore_permissions_ca_key_path = os.path.join( + keystore_path, KS_PRIVATE, 'permissions_ca.key.pem') _create_smime_signed_file( - keystore_ca_cert_path, keystore_ca_key_path, permissions_path, signed_permissions_path) + keystore_ca_cert_path, + keystore_permissions_ca_key_path, + permissions_path, + signed_permissions_path + ) return True def list_keys(keystore_path): - for name in os.listdir(keystore_path): - if os.path.isdir(os.path.join(keystore_path, name)): + contexts_path = os.path.join(keystore_path, KS_CONTEXT) + if not os.path.isdir(keystore_path): + raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), keystore_path) + if not os.path.isdir(contexts_path): + return True + for name in os.listdir(contexts_path): + if os.path.isdir(os.path.join(contexts_path, name)): print(name) return True @@ -364,9 +411,9 @@ def generate_artifacts(keystore_path=None, identity_names=[], policy_files=[]): return False for policy_file in policy_files: policy_tree = load_policy(policy_file) - profiles_element = policy_tree.find('profiles') - for profile in profiles_element: - identity_name = profile.get('ns').rstrip('/') + '/' + profile.get('node') + contexts_element = policy_tree.find('contexts') + for context in contexts_element: + identity_name = context.get('path') if identity_name not in identity_names: if not create_key(keystore_path, identity_name): return False diff --git a/sros2/sros2/policy/__init__.py b/sros2/sros2/policy/__init__.py index a0c0bc8c..68a30bc6 100644 --- a/sros2/sros2/policy/__init__.py +++ b/sros2/sros2/policy/__init__.py @@ -18,7 +18,7 @@ import pkg_resources -POLICY_VERSION = '0.1.0' +POLICY_VERSION = '0.2.0' def get_policy_default(name): diff --git a/sros2/sros2/policy/defaults/policy.xml b/sros2/sros2/policy/defaults/policy.xml index 552a2055..e1bbfa73 100644 --- a/sros2/sros2/policy/defaults/policy.xml +++ b/sros2/sros2/policy/defaults/policy.xml @@ -1,16 +1,21 @@ - - - - - /* - - - /* - - - /* - - - + + + + + + + /* + + + /* + + + /* + + + + + diff --git a/sros2/sros2/policy/schemas/policy.xsd b/sros2/sros2/policy/schemas/policy.xsd index 8086516e..09a28acb 100644 --- a/sros2/sros2/policy/schemas/policy.xsd +++ b/sros2/sros2/policy/schemas/policy.xsd @@ -10,15 +10,34 @@ + + + + + + + + + + + + + - + - - + + + + + + + + diff --git a/sros2/sros2/policy/templates/dds/permissions.xsl b/sros2/sros2/policy/templates/dds/permissions.xsl index 8a243da2..2c87370a 100644 --- a/sros2/sros2/policy/templates/dds/permissions.xsl +++ b/sros2/sros2/policy/templates/dds/permissions.xsl @@ -20,52 +20,75 @@ - + + + - - - - - - + - + - - CN= - - - - - - - - - - - - - - - - - - - - - - DENY - + + + CN= + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ros_discovery_info + + + + + ros_discovery_info + + + + + DENY + + - + + + + + @@ -292,4 +315,12 @@ + + + + + + + + diff --git a/sros2/sros2/policy/templates/policy.xsl b/sros2/sros2/policy/templates/policy.xsl index 238cb7d5..395fa15f 100644 --- a/sros2/sros2/policy/templates/policy.xsl +++ b/sros2/sros2/policy/templates/policy.xsl @@ -22,6 +22,8 @@ + + diff --git a/sros2/sros2/verb/create_key.py b/sros2/sros2/verb/create_key.py index e97d1d44..943b4f52 100644 --- a/sros2/sros2/verb/create_key.py +++ b/sros2/sros2/verb/create_key.py @@ -28,7 +28,7 @@ class CreateKeyVerb(VerbExtension): def add_arguments(self, parser, cli_name): arg = parser.add_argument('ROOT', help='root path of keystore') arg.completer = DirectoriesCompleter() - parser.add_argument('NAME', help='key name, aka ROS node name') + parser.add_argument('NAME', help='key name, aka ROS security context name') def main(self, *, args): success = create_key(args.ROOT, args.NAME) diff --git a/sros2/sros2/verb/create_permission.py b/sros2/sros2/verb/create_permission.py index 8214ccd0..5d84de2b 100644 --- a/sros2/sros2/verb/create_permission.py +++ b/sros2/sros2/verb/create_permission.py @@ -33,7 +33,7 @@ class CreatePermissionVerb(VerbExtension): def add_arguments(self, parser, cli_name): arg = parser.add_argument('ROOT', help='root path of keystore') arg.completer = DirectoriesCompleter() - parser.add_argument('NAME', help='key name, aka ROS node name') + parser.add_argument('NAME', help='key name, aka ROS security context name') arg = parser.add_argument( 'POLICY_FILE_PATH', help='path of the policy xml file') arg.completer = FilesCompleter( diff --git a/sros2/sros2/verb/generate_artifacts.py b/sros2/sros2/verb/generate_artifacts.py index 3667341c..c5d6de60 100644 --- a/sros2/sros2/verb/generate_artifacts.py +++ b/sros2/sros2/verb/generate_artifacts.py @@ -34,8 +34,8 @@ def add_arguments(self, parser, cli_name): arg = parser.add_argument('-k', '--keystore-root-path', help='root path of keystore') arg.completer = DirectoriesCompleter() parser.add_argument( - '-n', '--node-names', nargs='*', default=[], - help='list of identities, aka ROS node names') + '-c', '--security-contexts', nargs='*', default=[], + help='list of identities, aka ROS security contexts names') arg = parser.add_argument( '-p', '--policy-files', nargs='*', default=[], help='list of policy xml file paths') @@ -45,7 +45,7 @@ def add_arguments(self, parser, cli_name): def main(self, *, args): try: success = generate_artifacts( - args.keystore_root_path, args.node_names, args.policy_files) + args.keystore_root_path, args.security_contexts, args.policy_files) except FileNotFoundError as e: raise RuntimeError(str(e)) return 0 if success else 1 diff --git a/sros2/test/policies/add_two_ints.policy.xml b/sros2/test/policies/add_two_ints.policy.xml index 9becfffd..9969f4e2 100644 --- a/sros2/test/policies/add_two_ints.policy.xml +++ b/sros2/test/policies/add_two_ints.policy.xml @@ -1,20 +1,28 @@ - - - - - - add_two_ints - - - - - - add_two_ints - - - + + + + + + + add_two_ints + + + + + + + + + + add_two_ints + + + + + diff --git a/sros2/test/policies/minimal_action.policy.xml b/sros2/test/policies/minimal_action.policy.xml index f8e0f10f..1d99f25e 100644 --- a/sros2/test/policies/minimal_action.policy.xml +++ b/sros2/test/policies/minimal_action.policy.xml @@ -1,20 +1,28 @@ - - - - - - fibonacci - - - - - - fibonacci - - - + + + + + + + fibonacci + + + + + + + + + + fibonacci + + + + + diff --git a/sros2/test/policies/permissions/add_two_ints/permissions.xml b/sros2/test/policies/permissions/add_two_ints/permissions.xml index 9d811774..aecfa35c 100644 --- a/sros2/test/policies/permissions/add_two_ints/permissions.xml +++ b/sros2/test/policies/permissions/add_two_ints/permissions.xml @@ -1,7 +1,7 @@ - - CN=/add_two_ints_server + + CN=/add_two_ints/add_two_ints_server 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -51,8 +51,8 @@ DENY - - CN=/add_two_ints_client + + CN=/add_two_ints/add_two_ints_client 2013-10-26T00:00:00 2023-10-26T22:45:30 diff --git a/sros2/test/policies/permissions/minimal_action/permissions.xml b/sros2/test/policies/permissions/minimal_action/permissions.xml index 5f700361..ef4c3a1c 100644 --- a/sros2/test/policies/permissions/minimal_action/permissions.xml +++ b/sros2/test/policies/permissions/minimal_action/permissions.xml @@ -1,7 +1,7 @@ - - CN=/minimal_action_server + + CN=/minimal_action/minimal_action_server 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -57,8 +57,8 @@ DENY - - CN=/minimal_action_client + + CN=/minimal_action/minimal_action_client 2013-10-26T00:00:00 2023-10-26T22:45:30 diff --git a/sros2/test/policies/permissions/sample/permissions.xml b/sros2/test/policies/permissions/sample/permissions.xml index 1d979157..c079887b 100644 --- a/sros2/test/policies/permissions/sample/permissions.xml +++ b/sros2/test/policies/permissions/sample/permissions.xml @@ -1,7 +1,7 @@ - - CN=/talker + + CN=/talker_listener/talker 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -50,8 +50,8 @@ DENY - - CN=/listener + + CN=/talker_listener/listener 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -100,8 +100,8 @@ DENY - - CN=/add_two_ints_server + + CN=/add_two_ints/add_two_ints_server 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -151,8 +151,8 @@ DENY - - CN=/add_two_ints_client + + CN=/add_two_ints/add_two_ints_client 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -202,8 +202,8 @@ DENY - - CN=/minimal_action_server + + CN=/minimal_action/minimal_action_server 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -259,8 +259,8 @@ DENY - - CN=/minimal_action_client + + CN=/minimal_action/minimal_action_client 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -316,8 +316,8 @@ DENY - - CN=/admin + + CN=/sample_policy/admin 2013-10-26T00:00:00 2023-10-26T22:45:30 diff --git a/sros2/test/policies/permissions/single_context/permissions.xml b/sros2/test/policies/permissions/single_context/permissions.xml new file mode 100644 index 00000000..9bab954c --- /dev/null +++ b/sros2/test/policies/permissions/single_context/permissions.xml @@ -0,0 +1,195 @@ + + + + CN=/single_context + + 2013-10-26T00:00:00 + 2023-10-26T22:45:30 + + + + 0 + + + + rq/add_two_intsRequest + rq/add_two_ints_client/describe_parametersRequest + rq/add_two_ints_client/get_parameter_typesRequest + rq/add_two_ints_client/get_parametersRequest + rq/add_two_ints_client/list_parametersRequest + rq/add_two_ints_client/set_parametersRequest + rq/add_two_ints_client/set_parameters_atomicallyRequest + rq/add_two_ints_server/describe_parametersRequest + rq/add_two_ints_server/get_parameter_typesRequest + rq/add_two_ints_server/get_parametersRequest + rq/add_two_ints_server/list_parametersRequest + rq/add_two_ints_server/set_parametersRequest + rq/add_two_ints_server/set_parameters_atomicallyRequest + rq/fibonacci/_action/cancel_goalRequest + rq/fibonacci/_action/get_resultRequest + rq/fibonacci/_action/send_goalRequest + rq/listener/describe_parametersRequest + rq/listener/get_parameter_typesRequest + rq/listener/get_parametersRequest + rq/listener/list_parametersRequest + rq/listener/set_parametersRequest + rq/listener/set_parameters_atomicallyRequest + rq/minimal_action_client/describe_parametersRequest + rq/minimal_action_client/get_parameter_typesRequest + rq/minimal_action_client/get_parametersRequest + rq/minimal_action_client/list_parametersRequest + rq/minimal_action_client/set_parametersRequest + rq/minimal_action_client/set_parameters_atomicallyRequest + rq/minimal_action_server/describe_parametersRequest + rq/minimal_action_server/get_parameter_typesRequest + rq/minimal_action_server/get_parametersRequest + rq/minimal_action_server/list_parametersRequest + rq/minimal_action_server/set_parametersRequest + rq/minimal_action_server/set_parameters_atomicallyRequest + rq/talker/describe_parametersRequest + rq/talker/get_parameter_typesRequest + rq/talker/get_parametersRequest + rq/talker/list_parametersRequest + rq/talker/set_parametersRequest + rq/talker/set_parameters_atomicallyRequest + rr/add_two_intsReply + rr/add_two_ints_client/describe_parametersReply + rr/add_two_ints_client/get_parameter_typesReply + rr/add_two_ints_client/get_parametersReply + rr/add_two_ints_client/list_parametersReply + rr/add_two_ints_client/set_parametersReply + rr/add_two_ints_client/set_parameters_atomicallyReply + rr/add_two_ints_server/describe_parametersReply + rr/add_two_ints_server/get_parameter_typesReply + rr/add_two_ints_server/get_parametersReply + rr/add_two_ints_server/list_parametersReply + rr/add_two_ints_server/set_parametersReply + rr/add_two_ints_server/set_parameters_atomicallyReply + rr/fibonacci/_action/cancel_goalReply + rr/fibonacci/_action/get_resultReply + rr/fibonacci/_action/send_goalReply + rt/fibonacci/_action/feedback + rt/fibonacci/_action/status + rr/listener/describe_parametersReply + rr/listener/get_parameter_typesReply + rr/listener/get_parametersReply + rr/listener/list_parametersReply + rr/listener/set_parametersReply + rr/listener/set_parameters_atomicallyReply + rr/minimal_action_client/describe_parametersReply + rr/minimal_action_client/get_parameter_typesReply + rr/minimal_action_client/get_parametersReply + rr/minimal_action_client/list_parametersReply + rr/minimal_action_client/set_parametersReply + rr/minimal_action_client/set_parameters_atomicallyReply + rr/minimal_action_server/describe_parametersReply + rr/minimal_action_server/get_parameter_typesReply + rr/minimal_action_server/get_parametersReply + rr/minimal_action_server/list_parametersReply + rr/minimal_action_server/set_parametersReply + rr/minimal_action_server/set_parameters_atomicallyReply + rr/talker/describe_parametersReply + rr/talker/get_parameter_typesReply + rr/talker/get_parametersReply + rr/talker/list_parametersReply + rr/talker/set_parametersReply + rr/talker/set_parameters_atomicallyReply + rt/chatter + rt/parameter_events + rt/rosout + + + + + rq/add_two_intsRequest + rq/add_two_ints_client/describe_parametersRequest + rq/add_two_ints_client/get_parameter_typesRequest + rq/add_two_ints_client/get_parametersRequest + rq/add_two_ints_client/list_parametersRequest + rq/add_two_ints_client/set_parametersRequest + rq/add_two_ints_client/set_parameters_atomicallyRequest + rq/add_two_ints_server/describe_parametersRequest + rq/add_two_ints_server/get_parameter_typesRequest + rq/add_two_ints_server/get_parametersRequest + rq/add_two_ints_server/list_parametersRequest + rq/add_two_ints_server/set_parametersRequest + rq/add_two_ints_server/set_parameters_atomicallyRequest + rq/fibonacci/_action/cancel_goalRequest + rq/fibonacci/_action/get_resultRequest + rq/fibonacci/_action/send_goalRequest + rq/listener/describe_parametersRequest + rq/listener/get_parameter_typesRequest + rq/listener/get_parametersRequest + rq/listener/list_parametersRequest + rq/listener/set_parametersRequest + rq/listener/set_parameters_atomicallyRequest + rq/minimal_action_client/describe_parametersRequest + rq/minimal_action_client/get_parameter_typesRequest + rq/minimal_action_client/get_parametersRequest + rq/minimal_action_client/list_parametersRequest + rq/minimal_action_client/set_parametersRequest + rq/minimal_action_client/set_parameters_atomicallyRequest + rq/minimal_action_server/describe_parametersRequest + rq/minimal_action_server/get_parameter_typesRequest + rq/minimal_action_server/get_parametersRequest + rq/minimal_action_server/list_parametersRequest + rq/minimal_action_server/set_parametersRequest + rq/minimal_action_server/set_parameters_atomicallyRequest + rq/talker/describe_parametersRequest + rq/talker/get_parameter_typesRequest + rq/talker/get_parametersRequest + rq/talker/list_parametersRequest + rq/talker/set_parametersRequest + rq/talker/set_parameters_atomicallyRequest + rr/add_two_intsReply + rr/add_two_ints_client/describe_parametersReply + rr/add_two_ints_client/get_parameter_typesReply + rr/add_two_ints_client/get_parametersReply + rr/add_two_ints_client/list_parametersReply + rr/add_two_ints_client/set_parametersReply + rr/add_two_ints_client/set_parameters_atomicallyReply + rr/add_two_ints_server/describe_parametersReply + rr/add_two_ints_server/get_parameter_typesReply + rr/add_two_ints_server/get_parametersReply + rr/add_two_ints_server/list_parametersReply + rr/add_two_ints_server/set_parametersReply + rr/add_two_ints_server/set_parameters_atomicallyReply + rr/fibonacci/_action/cancel_goalReply + rr/fibonacci/_action/get_resultReply + rr/fibonacci/_action/send_goalReply + rt/fibonacci/_action/feedback + rt/fibonacci/_action/status + rr/listener/describe_parametersReply + rr/listener/get_parameter_typesReply + rr/listener/get_parametersReply + rr/listener/list_parametersReply + rr/listener/set_parametersReply + rr/listener/set_parameters_atomicallyReply + rr/minimal_action_client/describe_parametersReply + rr/minimal_action_client/get_parameter_typesReply + rr/minimal_action_client/get_parametersReply + rr/minimal_action_client/list_parametersReply + rr/minimal_action_client/set_parametersReply + rr/minimal_action_client/set_parameters_atomicallyReply + rr/minimal_action_server/describe_parametersReply + rr/minimal_action_server/get_parameter_typesReply + rr/minimal_action_server/get_parametersReply + rr/minimal_action_server/list_parametersReply + rr/minimal_action_server/set_parametersReply + rr/minimal_action_server/set_parameters_atomicallyReply + rr/talker/describe_parametersReply + rr/talker/get_parameter_typesReply + rr/talker/get_parametersReply + rr/talker/list_parametersReply + rr/talker/set_parametersReply + rr/talker/set_parameters_atomicallyReply + rt/chatter + rt/clock + rt/parameter_events + + + + DENY + + + diff --git a/sros2/test/policies/permissions/talker_listener/permissions.xml b/sros2/test/policies/permissions/talker_listener/permissions.xml index f7065f5f..03bf4876 100644 --- a/sros2/test/policies/permissions/talker_listener/permissions.xml +++ b/sros2/test/policies/permissions/talker_listener/permissions.xml @@ -1,7 +1,7 @@ - - CN=/talker + + CN=/talker_listener/talker 2013-10-26T00:00:00 2023-10-26T22:45:30 @@ -50,8 +50,8 @@ DENY - - CN=/listener + + CN=/talker_listener/listener 2013-10-26T00:00:00 2023-10-26T22:45:30 diff --git a/sros2/test/policies/sample.policy.xml b/sros2/test/policies/sample.policy.xml index 549a3e68..22194b15 100644 --- a/sros2/test/policies/sample.policy.xml +++ b/sros2/test/policies/sample.policy.xml @@ -1,25 +1,29 @@ - - + + xpointer="xpointer(/policy/contexts/*)"/> + xpointer="xpointer(/policy/contexts/*)"/> - - - - fibonacci - - - add_two_ints - - - chatter - - - + xpointer="xpointer(/policy/contexts/*)"/> + + + + + + fibonacci + + + add_two_ints + + + chatter + + + + + diff --git a/sros2/test/policies/single_context.policy.xml b/sros2/test/policies/single_context.policy.xml new file mode 100644 index 00000000..19eeb8fa --- /dev/null +++ b/sros2/test/policies/single_context.policy.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + diff --git a/sros2/test/policies/talker_listener.policy.xml b/sros2/test/policies/talker_listener.policy.xml index 88709bd9..de0b396f 100644 --- a/sros2/test/policies/talker_listener.policy.xml +++ b/sros2/test/policies/talker_listener.policy.xml @@ -1,20 +1,28 @@ - - - - - - chatter - - - - - - chatter - - - + + + + + + + chatter + + + + + + + + + + chatter + + + + + diff --git a/sros2/test/sros2/commands/security/verbs/test_create_key.py b/sros2/test/sros2/commands/security/verbs/test_create_key.py index 148c14dc..95c54395 100644 --- a/sros2/test/sros2/commands/security/verbs/test_create_key.py +++ b/sros2/test/sros2/commands/security/verbs/test_create_key.py @@ -13,7 +13,7 @@ # limitations under the License. import datetime -import os +from pathlib import Path import cryptography from cryptography import x509 @@ -34,19 +34,20 @@ # This fixture will run once for the entire module (as opposed to once per test) @pytest.fixture(scope='module') -def node_keys_dir(tmpdir_factory): - keystore_dir = str(tmpdir_factory.mktemp('keystore')) +def security_context_keys_dir(tmpdir_factory): + keystore_dir = Path(str(tmpdir_factory.mktemp('keystore'))) # First, create the keystore assert create_keystore(keystore_dir) # Now using that keystore, create a keypair along with other files required by DDS - assert cli.main(argv=['security', 'create_key', keystore_dir, '/test_node']) == 0 - node_dir = os.path.join(keystore_dir, 'test_node') - assert os.path.isdir(os.path.join(keystore_dir, 'test_node')) + assert cli.main( + argv=['security', 'create_key', str(keystore_dir), '/test_security_context']) == 0 + security_context_dir = keystore_dir / 'contexts' / 'test_security_context' + assert security_context_dir.is_dir() - # Return path to directory containing the node's files - return node_dir + # Return path to directory containing the security_context's files + return security_context_dir def load_cert(path): @@ -89,20 +90,20 @@ def verify_signature(cert, signatory): return True -def test_create_key(node_keys_dir): +def test_create_key(security_context_keys_dir): expected_files = ( 'cert.pem', 'governance.p7s', 'identity_ca.cert.pem', 'key.pem', 'permissions.p7s', 'permissions.xml', 'permissions_ca.cert.pem' ) - assert len(os.listdir(node_keys_dir)) == len(expected_files) + assert len(list(security_context_keys_dir.iterdir())) == len(expected_files) for expected_file in expected_files: - assert os.path.isfile(os.path.join(node_keys_dir, expected_file)) + assert (security_context_keys_dir / expected_file).is_file() -def test_cert_pem(node_keys_dir): - cert = load_cert(os.path.join(node_keys_dir, 'cert.pem')) - check_common_name(cert.subject, u'/test_node') +def test_cert_pem(security_context_keys_dir): + cert = load_cert(security_context_keys_dir / 'cert.pem') + check_common_name(cert.subject, u'/test_security_context') check_common_name(cert.issuer, _DEFAULT_COMMON_NAME) # Verify that the hash algorithm is as expected @@ -123,28 +124,28 @@ def test_cert_pem(node_keys_dir): assert value.path_length is None # Verify this cert is indeed signed by the keystore CA - signatory = load_cert(os.path.join(node_keys_dir, 'identity_ca.cert.pem')) + signatory = load_cert(security_context_keys_dir / 'identity_ca.cert.pem') assert verify_signature(cert, signatory) -def test_governance_p7s(node_keys_dir): +def test_governance_p7s(security_context_keys_dir): # Would really like to verify the signature, but ffi just can't use # that part of the OpenSSL API - with open(os.path.join(node_keys_dir, 'governance.p7s')) as f: + with open(security_context_keys_dir / 'governance.p7s') as f: lines = f.readlines() assert lines[0] == 'MIME-Version: 1.0\n' assert lines[1].startswith( 'Content-Type: multipart/signed; protocol="application/x-pkcs7-signature"; micalg="sha-256";') # noqa -def test_identity_ca_cert_pem(node_keys_dir): - cert = load_cert(os.path.join(node_keys_dir, 'identity_ca.cert.pem')) +def test_identity_ca_cert_pem(security_context_keys_dir): + cert = load_cert(security_context_keys_dir / 'identity_ca.cert.pem') check_common_name(cert.subject, _DEFAULT_COMMON_NAME) check_common_name(cert.issuer, _DEFAULT_COMMON_NAME) -def test_key_pem(node_keys_dir): - private_key = load_private_key(os.path.join(node_keys_dir, 'key.pem')) +def test_key_pem(security_context_keys_dir): + private_key = load_private_key(security_context_keys_dir / 'key.pem') assert isinstance(private_key, ec.EllipticCurvePrivateKey) assert private_key.key_size == 256 @@ -153,27 +154,27 @@ def test_key_pem(node_keys_dir): assert public_key.key_size == 256 -def test_permissions_p7s(node_keys_dir): +def test_permissions_p7s(security_context_keys_dir): # Would really like to verify the signature, but ffi just can't use # that part of the OpenSSL API - with open(os.path.join(node_keys_dir, 'permissions.p7s')) as f: + with open(security_context_keys_dir / 'permissions.p7s') as f: lines = f.readlines() assert lines[0] == 'MIME-Version: 1.0\n' assert lines[1].startswith( 'Content-Type: multipart/signed; protocol="application/x-pkcs7-signature"; micalg="sha-256";') # noqa -def test_permissions_xml(node_keys_dir): - permissions_xml = etree.parse(os.path.join(node_keys_dir, 'permissions.xml')) +def test_permissions_xml(security_context_keys_dir): + permissions_xml = etree.parse(str(security_context_keys_dir / 'permissions.xml')) permissions_xsd_path = get_transport_schema('dds', 'permissions.xsd') permissions_xsd = etree.XMLSchema(etree.parse(permissions_xsd_path)) permissions_xsd.assertValid(permissions_xml) -def test_permissions_ca_cert_pem(node_keys_dir): - cert = load_cert(os.path.join(node_keys_dir, 'permissions_ca.cert.pem')) +def test_permissions_ca_cert_pem(security_context_keys_dir): + cert = load_cert(security_context_keys_dir / 'permissions_ca.cert.pem') check_common_name(cert.subject, _DEFAULT_COMMON_NAME) check_common_name(cert.issuer, _DEFAULT_COMMON_NAME) - signatory = load_cert(os.path.join(node_keys_dir, 'identity_ca.cert.pem')) + signatory = load_cert(security_context_keys_dir / 'identity_ca.cert.pem') assert verify_signature(cert, signatory) diff --git a/sros2/test/sros2/commands/security/verbs/test_create_keystore.py b/sros2/test/sros2/commands/security/verbs/test_create_keystore.py index 9ceea63e..1874b8be 100644 --- a/sros2/test/sros2/commands/security/verbs/test_create_keystore.py +++ b/sros2/test/sros2/commands/security/verbs/test_create_keystore.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os +from pathlib import Path from xml.etree import ElementTree from cryptography import x509 @@ -34,21 +34,38 @@ def keystore_dir(tmpdir_factory): assert cli.main(argv=['security', 'create_keystore', keystore_dir]) == 0 # Return path to keystore directory - return keystore_dir + return Path(keystore_dir) def test_create_keystore(keystore_dir): - expected_files = ( - 'ca.cert.pem', 'ca.key.pem', 'governance.p7s', 'governance.xml' + public = keystore_dir / 'public' + private = keystore_dir / 'private' + contexts = keystore_dir / 'contexts' + expected_files_public = ( + public / 'ca.cert.pem', + public / 'permissions_ca.cert.pem', + public / 'identity_ca.cert.pem', + ) + expected_files_private = ( + private / 'ca.key.pem', + private / 'permissions_ca.key.pem', + private / 'identity_ca.key.pem', + ) + expected_files_contexts = ( + contexts / 'governance.p7s', + contexts / 'governance.xml', ) - assert len(os.listdir(keystore_dir)) == len(expected_files) - for expected_file in expected_files: - assert os.path.isfile(os.path.join(keystore_dir, expected_file)) + assert len(list(keystore_dir.iterdir())) == 3 + assert len(list(public.iterdir())) == len(expected_files_public) + assert len(list(private.iterdir())) == len(expected_files_private) + assert len(list(contexts.iterdir())) == len(expected_files_contexts) + expected_files = expected_files_public + expected_files_private + expected_files_contexts + assert all(x.is_file() for x in expected_files) def test_ca_cert(keystore_dir): - with open(os.path.join(keystore_dir, 'ca.cert.pem'), 'rb') as f: + with (keystore_dir / 'public' / 'ca.cert.pem').open('rb') as f: cert = x509.load_pem_x509_certificate(f.read(), cryptography_backend()) names = cert.subject.get_attributes_for_oid(x509.oid.NameOID.COMMON_NAME) assert len(names) == 1 @@ -58,7 +75,7 @@ def test_ca_cert(keystore_dir): def test_ca_key(keystore_dir): - with open(os.path.join(keystore_dir, 'ca.key.pem'), 'rb') as f: + with (keystore_dir / 'private' / 'ca.key.pem').open('rb') as f: key = load_pem_private_key(f.read(), password=None, backend=cryptography_backend()) public = key.public_key() assert public.curve.name == 'secp256r1' @@ -67,7 +84,7 @@ def test_ca_key(keystore_dir): def test_governance_p7s(keystore_dir): # Would really like to verify the signature, but ffi just can't use # that part of the OpenSSL API - with open(os.path.join(keystore_dir, 'governance.p7s')) as f: + with (keystore_dir / 'contexts' / 'governance.p7s').open('r') as f: lines = f.readlines() assert lines[0] == 'MIME-Version: 1.0\n' assert lines[1].startswith( @@ -76,4 +93,4 @@ def test_governance_p7s(keystore_dir): def test_governance_xml(keystore_dir): # Validates valid XML - ElementTree.parse(os.path.join(keystore_dir, 'governance.xml')) + ElementTree.parse(str(keystore_dir / 'contexts' / 'governance.xml')) diff --git a/sros2/test/sros2/commands/security/verbs/test_generate_policy.py b/sros2/test/sros2/commands/security/verbs/test_generate_policy.py index fe55fbfe..c21ab264 100644 --- a/sros2/test/sros2/commands/security/verbs/test_generate_policy.py +++ b/sros2/test/sros2/commands/security/verbs/test_generate_policy.py @@ -24,6 +24,8 @@ from test_msgs.srv import Empty +# TODO(ivanpauno): reactivate this test after updating generate policy +@pytest.mark.skip(reason='temporarily deactivated') def test_generate_policy_topics(): with tempfile.TemporaryDirectory() as tmpdir: # Create a test-specific context so that generate_policy can still init @@ -65,6 +67,8 @@ def test_generate_policy_topics(): assert len([t for t in topics if t.text == 'test_generate_policy_topics_pub']) == 0 +# TODO(ivanpauno): reactivate this test after updating generate policy +@pytest.mark.skip(reason='temporarily deactivated') def test_generate_policy_services(): with tempfile.TemporaryDirectory() as tmpdir: # Create a test-specific context so that generate_policy can still init @@ -106,6 +110,8 @@ def test_generate_policy_services(): assert len([s for s in services if s.text == 'test_generate_policy_services_server']) == 0 +# TODO(ivanpauno): reactivate this test after updating generate policy +@pytest.mark.skip(reason='temporarily deactivated') # TODO(jacobperron): On Windows, this test is flakey due to nodes left-over from tests in # other packages. # See: https://github.com/ros2/sros2/issues/143 @@ -119,6 +125,7 @@ def test_generate_policy_no_nodes(capsys): assert stderr == 'No nodes detected in the ROS graph. No policy file was generated.' +@pytest.mark.skip(reason='temporarily deactivated') def test_generate_policy_no_policy_file(capsys): with pytest.raises(SystemExit) as e: cli.main(argv=['security', 'generate_policy']) diff --git a/sros2/test/sros2/commands/security/verbs/test_list_keys.py b/sros2/test/sros2/commands/security/verbs/test_list_keys.py index a38f6622..0192c4bf 100644 --- a/sros2/test/sros2/commands/security/verbs/test_list_keys.py +++ b/sros2/test/sros2/commands/security/verbs/test_list_keys.py @@ -26,11 +26,11 @@ def test_list_keys(capsys): assert create_keystore(keystore_dir) # Now using that keystore, create a keypair - assert create_key(keystore_dir, '/test_node') + assert create_key(keystore_dir, '/test_context') # Now verify that the key we just created is included in the list assert cli.main(argv=['security', 'list_keys', keystore_dir]) == 0 - assert capsys.readouterr().out.strip() == 'test_node' + assert capsys.readouterr().out.strip() == 'test_context' def test_list_keys_no_keys(capsys): diff --git a/sros2/test/sros2/test_api.py b/sros2/test/sros2/test_api.py index dabb59ec..f0fca2da 100644 --- a/sros2/test/sros2/test_api.py +++ b/sros2/test/sros2/test_api.py @@ -24,7 +24,6 @@ def test_is_key_name_valid(): # Invalid cases assert not is_key_name_valid('') assert not is_key_name_valid(' ') - assert not is_key_name_valid('/') assert not is_key_name_valid('//') assert not is_key_name_valid('foo') assert not is_key_name_valid('foo/bar')