From 6eed5861bcf47a1f7230dd38a04f9c782f89d37e Mon Sep 17 00:00:00 2001 From: maybe-sybr <58414429+maybe-sybr@users.noreply.github.com> Date: Fri, 25 Jun 2021 10:27:36 +1000 Subject: [PATCH] DO NOT MERGE: draft: improv: Check reference prop types for all customs Previously the requirement that properties ending in `_ref(s)` were instances of an appropriate type capable of containing a STIX `identifier` (the current interpretation of section 3.1 of the spec) was only enforced for custom observables. This change refactors the property checks for custom objects to enforce this and the STIX 2.1 property name requirement (also from section 3.1) in a common helper, therefore extending the enforcement of both requirements to all custom object types created by downstream projects. DRAFT NOTES: There are some TODOs in here which need addressing! --- stix2/registration.py | 153 +++++++++++++++++++--------------- stix2/test/v20/test_custom.py | 6 +- stix2/test/v21/test_custom.py | 6 +- 3 files changed, 91 insertions(+), 74 deletions(-) diff --git a/stix2/registration.py b/stix2/registration.py index 4ec019a2..979c57bb 100644 --- a/stix2/registration.py +++ b/stix2/registration.py @@ -3,10 +3,83 @@ from . import registry from .base import _DomainObject, _Observable from .exceptions import DuplicateRegistrationError -from .properties import _validate_type -from .utils import PREFIX_21_REGEX, get_class_hierarchy_names +from .properties import ( + ListProperty, ObjectReferenceProperty, ReferenceProperty, _validate_type, +) +from .utils import PREFIX_21_REGEX from .version import DEFAULT_VERSION +# Properties ending in "_ref/s" need to be instances of specific types to meet +# our interpretation of section 3.1 of the spec +_VERSION_REF_TYPES_MAP = { + # TODO: The 2.0 code seems to use `ReferenceProperty`s - is that okay? + "2.0": (ObjectReferenceProperty, ReferenceProperty), + "2.1": (ReferenceProperty,), +} +# Any unknown versions are presumed to be newer than STIX 2.1 so we'll define +# this to enforce the 2.1 compatible type in such situations +_UNKNOWN_VERSION_REF_TYPES = _VERSION_REF_TYPES_MAP["2.1"] + + +def _validate_ref_props(props_map, version): + """ + Validate that reference properties contain an expected type. + + Args: + props_map (mapping): A mapping of STIX object properties to be checked. + version (str): Which STIX2 version the properties must confirm to. + + Raises: + ValueError: If the properties do not conform. + """ + try: + ref_prop_types = _VERSION_REF_TYPES_MAP[version] + except KeyError: + ref_prop_types = _UNKNOWN_VERSION_REF_TYPES + + for prop_name, prop_obj in props_map.items(): + tail = prop_name.rsplit("_", 1)[-1] + if tail == "ref" and not isinstance(prop_obj, ref_prop_types): + raise ValueError( + f"{prop_name!r} is named like a reference property but is not " + f"a subclass of any of {ref_prop_types!r}.", + ) + elif tail == "refs" and not all(( + isinstance(prop_obj, ListProperty), + isinstance(getattr(prop_obj, "contained", None), ref_prop_types), + )): + raise ValueError( + f"{prop_name!r} is named like a reference list property but is not" + f"a 'ListProperty' containing a subclass of any of {ref_prop_types!r}.", + ) + + +def _validate_props(props_map, version): + """ + Validate that a map of properties is conformant for this STIX `version`. + + Args: + props_map (mapping): A mapping of STIX object properties to be checked. + version (str): Which STIX2 version the properties must confirm to. + + Raises: + ValueError: If the properties do not conform. + """ + # Confirm conformance with STIX 2.1 requirements for property names + # + # TODO: Should this be 2.1 or anything unknown assuming 2.1+, like we do + # for identifier properties above? `version != "2.0"`? + if version == "2.1": + for prop_name, prop_value in props_map.items(): + # TODO: We could probably avoid doing a regex here just for the + # first char, but actually there are other constraints in 3.1 which + # we don't check which a regex would be suitable for + # (e.g. don't use hyphens in prop names, length min/max) + if not re.match(PREFIX_21_REGEX, prop_name): + raise ValueError("Property name '%s' must begin with an alpha character." % prop_name) + # Confirm conformance to section 3.1 regarding identifier properties + _validate_ref_props(props_map, version) + def _register_object(new_type, version=DEFAULT_VERSION): """Register a custom STIX Object type. @@ -29,15 +102,10 @@ def _register_object(new_type, version=DEFAULT_VERSION): new_type.__name__, ) - properties = new_type._properties - if not version: version = DEFAULT_VERSION - if version == "2.1": - for prop_name, prop in properties.items(): - if not re.match(PREFIX_21_REGEX, prop_name): - raise ValueError("Property name '%s' must begin with an alpha character" % prop_name) + _validate_props(new_type._properties, version) OBJ_MAP = registry.STIX2_OBJ_MAPS[version]['objects'] if new_type._type in OBJ_MAP.keys(): @@ -54,19 +122,12 @@ def _register_marking(new_marking, version=DEFAULT_VERSION): None, use latest version. """ - - mark_type = new_marking._type - properties = new_marking._properties - if not version: version = DEFAULT_VERSION + mark_type = new_marking._type _validate_type(mark_type, version) - - if version == "2.1": - for prop_name, prop_value in properties.items(): - if not re.match(PREFIX_21_REGEX, prop_name): - raise ValueError("Property name '%s' must begin with an alpha character." % prop_name) + _validate_props(new_marking._properties, version) OBJ_MAP_MARKING = registry.STIX2_OBJ_MAPS[version]['markings'] if mark_type in OBJ_MAP_MARKING.keys(): @@ -83,49 +144,10 @@ def _register_observable(new_observable, version=DEFAULT_VERSION): None, use latest version. """ - properties = new_observable._properties - if not version: version = DEFAULT_VERSION - if version == "2.0": - # If using STIX2.0, check properties ending in "_ref/s" are ObjectReferenceProperties - for prop_name, prop in properties.items(): - if prop_name.endswith('_ref') and ('ObjectReferenceProperty' not in get_class_hierarchy_names(prop)): - raise ValueError( - "'%s' is named like an object reference property but " - "is not an ObjectReferenceProperty." % prop_name, - ) - elif ( - prop_name.endswith('_refs') and ( - 'ListProperty' not in get_class_hierarchy_names(prop) or - 'ObjectReferenceProperty' not in get_class_hierarchy_names(prop.contained) - ) - ): - raise ValueError( - "'%s' is named like an object reference list property but " - "is not a ListProperty containing ObjectReferenceProperty." % prop_name, - ) - else: - # If using STIX2.1 (or newer...), check properties ending in "_ref/s" are ReferenceProperties - for prop_name, prop in properties.items(): - if not re.match(PREFIX_21_REGEX, prop_name): - raise ValueError("Property name '%s' must begin with an alpha character." % prop_name) - elif prop_name.endswith('_ref') and ('ReferenceProperty' not in get_class_hierarchy_names(prop)): - raise ValueError( - "'%s' is named like a reference property but " - "is not a ReferenceProperty." % prop_name, - ) - elif ( - prop_name.endswith('_refs') and ( - 'ListProperty' not in get_class_hierarchy_names(prop) or - 'ReferenceProperty' not in get_class_hierarchy_names(prop.contained) - ) - ): - raise ValueError( - "'%s' is named like a reference list property but " - "is not a ListProperty containing ReferenceProperty." % prop_name, - ) + _validate_props(new_observable._properties, version) OBJ_MAP_OBSERVABLE = registry.STIX2_OBJ_MAPS[version]['observables'] if new_observable._type in OBJ_MAP_OBSERVABLE.keys(): @@ -149,19 +171,11 @@ def _register_observable_extension( obs_class = observable if isinstance(observable, type) else \ type(observable) ext_type = new_extension._type - properties = new_extension._properties if not issubclass(obs_class, _Observable): raise ValueError("'observable' must be a valid Observable class!") _validate_type(ext_type, version) - - if not new_extension._properties: - raise ValueError( - "Invalid extension: must define at least one property: " + - ext_type, - ) - if version == "2.1": if not ext_type.endswith('-ext'): raise ValueError( @@ -169,9 +183,12 @@ def _register_observable_extension( ext_type, ) - for prop_name, prop_value in properties.items(): - if not re.match(PREFIX_21_REGEX, prop_name): - raise ValueError("Property name '%s' must begin with an alpha character." % prop_name) + if not new_extension._properties: + raise ValueError( + "Invalid extension: must define at least one property: " + + ext_type, + ) + _validate_props(new_extension._properties, version) try: observable_type = observable._type diff --git a/stix2/test/v20/test_custom.py b/stix2/test/v20/test_custom.py index 8d237e0e..c0d32b71 100644 --- a/stix2/test/v20/test_custom.py +++ b/stix2/test/v20/test_custom.py @@ -561,7 +561,7 @@ def test_custom_observable_object_invalid_ref_property(): ) class NewObs(): pass - assert "is named like an object reference property but is not an ObjectReferenceProperty" in str(excinfo.value) + assert "is named like a reference property but is not" in str(excinfo.value) def test_custom_observable_object_invalid_refs_property(): @@ -573,7 +573,7 @@ def test_custom_observable_object_invalid_refs_property(): ) class NewObs(): pass - assert "is named like an object reference list property but is not a ListProperty containing ObjectReferenceProperty" in str(excinfo.value) + assert "is named like a reference list property but is not" in str(excinfo.value) def test_custom_observable_object_invalid_refs_list_property(): @@ -585,7 +585,7 @@ def test_custom_observable_object_invalid_refs_list_property(): ) class NewObs(): pass - assert "is named like an object reference list property but is not a ListProperty containing ObjectReferenceProperty" in str(excinfo.value) + assert "is named like a reference list property but is not" in str(excinfo.value) def test_custom_observable_object_invalid_valid_refs(): diff --git a/stix2/test/v21/test_custom.py b/stix2/test/v21/test_custom.py index e51d76a6..4e357255 100644 --- a/stix2/test/v21/test_custom.py +++ b/stix2/test/v21/test_custom.py @@ -709,7 +709,7 @@ def test_custom_observable_object_invalid_ref_property(): ) class NewObs(): pass - assert "is named like a reference property but is not a ReferenceProperty" in str(excinfo.value) + assert "is named like a reference property but is not" in str(excinfo.value) def test_custom_observable_object_invalid_refs_property(): @@ -721,7 +721,7 @@ def test_custom_observable_object_invalid_refs_property(): ) class NewObs(): pass - assert "is named like a reference list property but is not a ListProperty containing ReferenceProperty" in str(excinfo.value) + assert "is named like a reference list property but is not" in str(excinfo.value) def test_custom_observable_object_invalid_refs_list_property(): @@ -733,7 +733,7 @@ def test_custom_observable_object_invalid_refs_list_property(): ) class NewObs(): pass - assert "is named like a reference list property but is not a ListProperty containing ReferenceProperty" in str(excinfo.value) + assert "is named like a reference list property but is not" in str(excinfo.value) def test_custom_no_properties_raises_exception():