Repository: oasis-open/cti-python-stix2
Pull request #458 (2020-09-18 01:21:10)

Versioning refinements
This intended to tighten up versionability detection and plug some holes.  It's not perfect, but hopefully an iterative improvement.

The changes include addressing versionability of a *type* separately from versionability of an *object* (i.e. an instance of a type).  This was kind of blurred before: STIX objects would be checked for presence of versioning properties; dicts would be checked for support from their object type.  This presents opportunities for bugs and misleading error messages.

For example, if a custom STIX type were created whose versioning properties were optional, and you didn't provide sufficient properties on an object, you'd get an error that you "cannot create new version of object of this type".  That's actually incorrect: you *can* creation new versions of instances of the type, you just have to provide sufficient properties on the object.  Also, if you tried to version a dict of a versionable type, didn't provide an explicit "modified" property for the new version as a kwarg, and it was also missing from the dict, you would get a crash because code just assumed the "modified" property would be in the dict.  In the first case, versionability of the object was checked but not the type; in the second case, versionability of the type was checked, but not the object.  So both types of error are made.

There is also the question of what to do if someone tries to version a dict of an unregistered STIX type.  Without registration, we can't tell what properties the type supports, so we can't determine its versionability.  Do we allow versioning?

Behavior included with this PR:
- If an object has all versioning properties, it is considered versionable.  Versionability of the type is not checked in this case.  This allows versioning properties to be added as custom properties as necessary, to enable versioning on an otherwise unversionable object.
- Versionability determination of a STIX type is the same as before.  All versioning properties must be supported.
- If a STIX type has not been registered and one attempts to version a dict for that type, a lax approach is taken, and the type will be considered versionable.  (This does not mean a particular instance of the type is versionable.)
- Except for the first bullet item, versionability determination of an object is only done if the type has been deemed versionable.  An object of versionable type is considered versionable if it has the "created" property.  If it does not have a "modified" property, it is assumed equal to "created", and the new version will have a "modified" property added with a timestamp greater than the created property.
- Exception types have been added to be clearer about why versioning fails.  You should no longer get an error saying a type is not versionable, when it is.  You should also no longer get a generic KeyError crash if a versionable instance of a versionable type does not have a "modified" property, and one was not given as a kwarg to new_version().

Possible future work: what if neither the type nor instance is considered versionable according to the above criteria, but *together*, the support/presence criteria are met for versionability?  E.g. type supports modified/revoked but not created, and instance has a custom "created" property but has neither "modified" nor "revoked".  The type would not be considered versionable, so versionability of the object would not even be considered.  But presence of the object's custom "created" property together with the type's support for "modified" and "revoked" put all the ingredients in place for versioning.

codecov-commenter: # [Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=h1) Report
> Merging [#458](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=desc) into [master](https://codecov.io/gh/oasis-open/cti-python-stix2/commit/5a34d529a8f29e8a1bce510ebc99aa7e3d74e2ef?el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/graphs/tree.svg?width=650&height=150&src=pr&token=vzHZ2LBVZq)](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #458      +/-   ##
==========================================
+ Coverage   98.24%   98.25%   +0.01%     
==========================================
  Files         142      142              
  Lines       15740    15838      +98     
==========================================
+ Hits        15463    15562      +99     
+ Misses        277      276       -1     
```


| [Impacted Files](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [stix2/exceptions.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvZXhjZXB0aW9ucy5weQ==) | `100.00% <100.00%> (ø)` | |
| [stix2/test/v20/test\_versioning.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjAvdGVzdF92ZXJzaW9uaW5nLnB5) | `100.00% <100.00%> (ø)` | |
| [stix2/test/v21/test\_versioning.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjEvdGVzdF92ZXJzaW9uaW5nLnB5) | `100.00% <100.00%> (ø)` | |
| [stix2/versioning.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdmVyc2lvbmluZy5weQ==) | `100.00% <100.00%> (+1.02%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=footer). Last update [5a34d52...bc8fdcd](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

chisholm: I checked the spec (e.g. sections 3.2 and 11.2), but couldn't find anything that put an all-or-nothing requirement on the usage of versioning properties for custom STIX types.  E.g. if you use "revoked", you must also use "created" and "modified".  It does say "The revoked property is only used by STIX Objects that support versioning...".  There is similar description for "modified", but not for "created".  That implies a policy which was used by spec authors for spec-defined objects.  There is not a "must" or "should" directing creators of custom types to add the other two properties.

It would be an odd situation to have a "revoked" property on an object/type you can never revoke, because it's unversionable.  I also don't think you're allowed to use properties named the same as common properties for purposes other than what the spec describes(?).  So, odd situation but it seems like the spec allows it.

Btw, I edited the PR description (updated the behavior bullets).  I hadn't quite got the correct behavioral description... maybe that is an indication of how tricky it is to get the logic right :)
codecov-io: # [Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=h1) Report
> Merging [#458](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=desc) (ac99e12) into [master](https://codecov.io/gh/oasis-open/cti-python-stix2/commit/03b3423cbb3db1cd11f075a1cd9bf3b020fa5cee?el=desc) (03b3423) will **decrease** coverage by `0.16%`.
> The diff coverage is `92.75%`.

[![Impacted file tree graph](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/graphs/tree.svg?width=650&height=150&src=pr&token=vzHZ2LBVZq)](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #458      +/-   ##
==========================================
- Coverage   89.27%   89.11%   -0.17%     
==========================================
  Files         146      146              
  Lines       16032    15768     -264     
==========================================
- Hits        14313    14052     -261     
+ Misses       1719     1716       -3     
```


| [Impacted Files](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [stix2/exceptions.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvZXhjZXB0aW9ucy5weQ==) | `58.08% <56.25%> (+0.23%)` | :arrow_up: |
| [stix2/versioning.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdmVyc2lvbmluZy5weQ==) | `82.56% <92.30%> (+1.76%)` | :arrow_up: |
| [stix2/test/v20/test\_versioning.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjAvdGVzdF92ZXJzaW9uaW5nLnB5) | `100.00% <100.00%> (ø)` | |
| [stix2/test/v21/test\_versioning.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjEvdGVzdF92ZXJzaW9uaW5nLnB5) | `100.00% <100.00%> (ø)` | |
| [stix2/test/v20/test\_datastore\_filesystem.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjAvdGVzdF9kYXRhc3RvcmVfZmlsZXN5c3RlbS5weQ==) | `99.20% <0.00%> (-0.02%)` | :arrow_down: |
| [stix2/test/v21/test\_datastore\_filesystem.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjEvdGVzdF9kYXRhc3RvcmVfZmlsZXN5c3RlbS5weQ==) | `99.22% <0.00%> (-0.02%)` | :arrow_down: |
| [stix2/test/v21/test\_deterministic\_ids.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjEvdGVzdF9kZXRlcm1pbmlzdGljX2lkcy5weQ==) | `99.21% <0.00%> (-0.01%)` | :arrow_down: |
| [stix2/test/v20/conftest.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjAvY29uZnRlc3QucHk=) | `100.00% <0.00%> (ø)` | |
| [stix2/test/v21/conftest.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjEvY29uZnRlc3QucHk=) | `100.00% <0.00%> (ø)` | |
| [stix2/test/v20/test\_tool.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree#diff-c3RpeDIvdGVzdC92MjAvdGVzdF90b29sLnB5) | `100.00% <0.00%> (ø)` | |
| ... and [68 more](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=footer). Last update [03b3423...ac99e12](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/458?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/stix2/exceptions.py b/stix2/exceptions.py
index edcc352..92e7e68 100644
--- a/stix2/exceptions.py
+++ b/stix2/exceptions.py
@@ -175,7 +175,14 @@ class ImmutableError(STIXError):
         return msg.format(self)
 
 
-class UnmodifiablePropertyError(STIXError):
+class VersioningError(STIXError):
+    """
+    Base class for object versioning errors.
+    """
+    pass
+
+
+class UnmodifiablePropertyError(VersioningError):
     """Attempted to modify an unmodifiable property of object when creating a new version."""
 
     def __init__(self, unchangable_properties):
@@ -187,6 +194,40 @@ class UnmodifiablePropertyError(STIXError):
         return msg.format(", ".join(self.unchangable_properties))
 
 
+class TypeNotVersionableError(VersioningError):
+    """
+    An object couldn't be versioned because it lacked the versioning properties
+    and its type does not support them.
+    """
+    def __init__(self, obj):
+        if isinstance(obj, dict):
+            type_name = obj.get("type")
+        else:
+            # try standard attribute of _STIXBase subclasses/instances
+            type_name = getattr(obj, "_type", None)
+
+        self.object = obj
+
+        msg = "Object type{}is not versionable.  Try a dictionary or " \
+              "instance of an SDO or SRO class.".format(
+                  " '{}' ".format(type_name) if type_name else " ",
+              )
+        super().__init__(msg)
+
+
+class ObjectNotVersionableError(VersioningError):
+    """
+    An object's type supports versioning, but the object couldn't be versioned
+    because it lacked sufficient versioning properties.
+    """
+    def __init__(self, obj):
+        self.object = obj
+
+        msg = "Creating a new object version requires at least the 'created'" \
+              " property: " + str(obj)
+        super().__init__(msg)
+
+
 class RevokeError(STIXError):
     """Attempted an operation on a revoked object."""
 
diff --git a/stix2/versioning.py b/stix2/versioning.py
index 01affe9..a6dc0bd 100644
--- a/stix2/versioning.py
+++ b/stix2/versioning.py
@@ -9,13 +9,13 @@ import uuid
 import stix2.base
 import stix2.registry
 from stix2.utils import (
-    detect_spec_version, get_timestamp, is_sco, is_sdo, is_sro,
-    parse_into_datetime,
+    detect_spec_version, get_timestamp, is_sco, parse_into_datetime,
 )
 import stix2.v20
 
 from .exceptions import (
-    InvalidValueError, RevokeError, UnmodifiablePropertyError,
+    InvalidValueError, ObjectNotVersionableError, RevokeError,
+    TypeNotVersionableError, UnmodifiablePropertyError,
 )
 
 # STIX object properties that cannot be modified
@@ -56,27 +56,16 @@ def _fudge_modified(old_modified, new_modified, use_stix21):
     return new_modified
 
 
-def _is_versionable(data):
+def _get_stix_version(data):
     """
-    Determine whether the given object is versionable.  This check is done on
-    the basis of support for three properties for the object type: "created",
-    "modified", and "revoked".  If all three are supported, the object is
-    versionable; otherwise it is not.  Dicts must have a "type" property whose
-    value is for a registered object type.  This is used to determine a
-    complete set of supported properties for the type.
-
-    Also, detect whether it represents a STIX 2.1 or greater spec version.
+    Bit of factored out functionality for getting/detecting the STIX version
+    of the given value.
 
-    :param data: The object to check.  Must be either a stix object, or a dict
-        with a "type" property.
-    :return: A 2-tuple of bools: the first is True if the object is versionable
-        and False if not; the second is True if the object is STIX 2.1+ and
-        False if not.
+    :param data: An object, e.g. _STIXBase instance or dict
+    :return: The STIX version as a string in "X.Y" notation, or None if the
+        version could not be determined.
     """
-
-    is_versionable = False
     stix_version = None
-
     if isinstance(data, Mapping):
 
         # First, determine spec version.  It's easy for our stix2 objects; more
@@ -88,36 +77,112 @@ def _is_versionable(data):
         elif isinstance(data, dict):
             stix_version = detect_spec_version(data)
 
-        # Then, determine versionability.
+    return stix_version
 
-        # This should be sufficient for STIX objects; maybe we get lucky with
-        # dicts here but probably not.
-        if data.keys() >= _VERSIONING_PROPERTIES:
-            is_versionable = True
 
-        # Tougher to handle dicts.  We need to consider STIX version, map to a
-        # registered class, and from that get a more complete picture of its
-        # properties.
-        elif isinstance(data, dict):
-            obj_type = data["type"]
+def _is_versionable_type(data):
+    """
+    Determine whether type of the given object is versionable.  This check is
+    done on the basis of support for three properties for the object type:
+    "created", "modified", and "revoked".  If all three are supported, the
+    object type is versionable; otherwise it is not.  Dicts must have a "type"
+    property.  This is used in STIX version detection and to determine a
+    complete set of supported properties for the type.
 
-            if is_sdo(obj_type, stix_version) or is_sro(obj_type, stix_version):
-                # Should we bother checking properties for SDOs/SROs?
-                # They were designed to be versionable.
-                is_versionable = True
+    If a dict is passed whose "type" is unregistered, then this library has no
+    knowledge of the type.  It can't determine what properties are "supported".
+    This function will be lax and treat the type as versionable.
 
-            elif is_sco(obj_type, stix_version):
-                # but do check SCOs
-                cls = stix2.registry.class_for_type(
-                    obj_type, stix_version, "observables",
-                )
+    Note that this support check is not sufficient for creating a new object
+    version.  Support for the versioning properties does not mean that
+    sufficient properties are actually present on the object.
+
+    Also, detect whether it represents a STIX 2.1 or greater spec version.
+
+    :param data: The object to check.  Must be either a stix object, or a dict
+        with a "type" property.
+    :return: A 2-tuple: the first element is True if the object is versionable
+        and False if not; the second is the STIX version as a string in "X.Y"
+        notation.
+    """
+
+    is_versionable = False
+    stix_version = None
+
+    if isinstance(data, Mapping):
+        # First, determine spec version
+        stix_version = _get_stix_version(data)
+
+        # Then, determine versionability.
+        if isinstance(data, stix2.base._STIXBase):
+            is_versionable = _VERSIONING_PROPERTIES.issubset(
+                data._properties,
+            )
+
+        elif isinstance(data, dict):
+            # Tougher to handle dicts.  We need to consider STIX version,
+            # map to a registered class, and from that get a more complete
+            # picture of its properties.
+
+            cls = stix2.registry.class_for_type(data.get("type"), stix_version)
+            if cls:
                 is_versionable = _VERSIONING_PROPERTIES.issubset(
                     cls._properties,
                 )
 
+            else:
+                # The type is not registered, so we have no knowledge of
+                # what properties are supported.  Let's be lax and let them
+                # version it.
+                is_versionable = True
+
     return is_versionable, stix_version
 
 
+def _check_versionable_object(data):
+    """
+    Determine whether there are or may be sufficient properties present on
+    an object to allow versioning.  Raises an exception if the object can't be
+    versioned.
+
+    Also detect STIX spec version.
+
+    :param data: The object to check, e.g. dict with a "type" property, or
+        _STIXBase instance
+    :return: True if the object is STIX 2.1+, or False if not
+    :raises TypeNotVersionableError: If the object didn't have the versioning
+        properties and the type was found to not support them
+    :raises ObjectNotVersionableError: If the type was found to support
+        versioning but there were insufficient properties on the object
+    """
+    if isinstance(data, Mapping):
+        if data.keys() >= _VERSIONING_PROPERTIES:
+            # If the properties all already exist in the object, assume they
+            # are either supported by the type, or are custom properties, and
+            # allow versioning.
+            stix_version = _get_stix_version(data)
+
+        else:
+            is_versionable_type, stix_version = _is_versionable_type(data)
+            if is_versionable_type:
+                # The type supports the versioning properties (or we don't
+                # recognize it and just assume it does).  The question shifts
+                # to whether the object has sufficient properties to create a
+                # new version.  Just require "created" for now.  We need at
+                # least that as a starting point for new version timestamps.
+                is_versionable = "created" in data
+
+                if not is_versionable:
+                    raise ObjectNotVersionableError(data)
+            else:
+                raise TypeNotVersionableError(data)
+
+    else:
+        raise TypeNotVersionableError(data)
+
+    return stix_version
+
+
 def new_version(data, allow_custom=None, **kwargs):
     """
     Create a new version of a STIX object, by modifying properties and
@@ -134,13 +199,7 @@ def new_version(data, allow_custom=None, **kwargs):
     :return: The new object.
     """
 
-    is_versionable, stix_version = _is_versionable(data)
-
-    if not is_versionable:
-        raise ValueError(
-            "cannot create new version of object of this type! "
-            "Try a dictionary or instance of an SDO or SRO class.",
-        )
+    stix_version = _check_versionable_object(data)
 
     if data.get('revoked'):
         raise RevokeError("new_version")
@@ -178,36 +237,34 @@ def new_version(data, allow_custom=None, **kwargs):
     # to know which rules to apply.
     precision_constraint = "min" if stix_version == "2.1" else "exact"
 
-    cls = type(data)
-    if 'modified' not in kwargs:
-        old_modified = parse_into_datetime(
-            data["modified"], precision="millisecond",
-            precision_constraint=precision_constraint,
-        )
-
-        new_modified = get_timestamp()
-        new_modified = _fudge_modified(
-            old_modified, new_modified, stix_version == "2.1",
-        )
+    old_modified = data.get("modified") or data.get("created")
+    old_modified = parse_into_datetime(
+        old_modified, precision="millisecond",
+        precision_constraint=precision_constraint,
+    )
 
-        kwargs['modified'] = new_modified
-
-    elif 'modified' in data:
-        old_modified_property = parse_into_datetime(
-            data.get('modified'), precision='millisecond',
-            precision_constraint=precision_constraint,
-        )
-        new_modified_property = parse_into_datetime(
+    cls = type(data)
+    if 'modified' in kwargs:
+        new_modified = parse_into_datetime(
             kwargs['modified'], precision='millisecond',
             precision_constraint=precision_constraint,
         )
-        if new_modified_property <= old_modified_property:
+        if new_modified <= old_modified:
             raise InvalidValueError(
                 cls, 'modified',
                 "The new modified datetime cannot be before than or equal to the current modified datetime."
                 "It cannot be equal, as according to STIX 2 specification, objects that are different "
                 "but have the same id and modified timestamp do not have defined consumer behavior.",
             )
+
+    else:
+        new_modified = get_timestamp()
+        new_modified = _fudge_modified(
+            old_modified, new_modified, stix_version != "2.0",
+        )
+
+        kwargs['modified'] = new_modified
+
     new_obj_inner.update(kwargs)
 
     # Set allow_custom appropriately if versioning an object.  We will ignore
diff --git a/stix2/workbench.py b/stix2/workbench.py
index 8be546f..e3e4f1d 100644
--- a/stix2/workbench.py
+++ b/stix2/workbench.py
@@ -161,6 +161,13 @@ def _setup_workbench():
         new_class = type(obj_type.__name__, (obj_type,), new_class_dict)
         factory_func = functools.partial(_environ.create, new_class)
 
+        # Copy over some class attributes that other code expects to find
+        factory_func._type = obj_type._type
+        factory_func._properties = obj_type._properties
+        if hasattr(obj_type, "_id_contributing_properties"):
+            factory_func._id_contributing_properties = \
+                obj_type._id_contributing_properties
+
         # Add our new "class" to this module's globals and to the library-wide
         # mapping.  This allows parse() to use the wrapped classes.
         globals()[obj_type.__name__] = factory_func


Test patch:
diff --git a/stix2/test/v20/test_versioning.py b/stix2/test/v20/test_versioning.py
index e2525e4..665ac4c 100644
--- a/stix2/test/v20/test_versioning.py
+++ b/stix2/test/v20/test_versioning.py
@@ -170,6 +170,60 @@ def test_versioning_error_dict_bad_modified_value():
         "but have the same id and modified timestamp do not have defined consumer behavior."
 
 
+def test_versioning_dict_unregistered_no_modified():
+    d = {
+        "type": "not-registered",
+        "id": "not-registered--4da54535-47b7-468c-88fa-d13b04033c4b",
+        "created": "1995-04-07T15:37:48.178Z",
+    }
+
+    new_d = stix2.versioning.new_version(d)
+    assert "modified" in new_d
+    assert new_d["modified"] > stix2.utils.parse_into_datetime(d["created"])
+
+    new_d = stix2.versioning.new_version(d, modified="1996-11-20T01:19:29.134Z")
+    assert new_d["modified"] == "1996-11-20T01:19:29.134Z"
+
+
+def test_versioning_dict_unregistered_unversionable():
+    d = {
+        "type": "not-registered",
+        "id": "not-registered--4da54535-47b7-468c-88fa-d13b04033c4b",
+        "modified": "1995-04-07T15:37:48.178Z",
+    }
+
+    with pytest.raises(stix2.exceptions.ObjectNotVersionableError):
+        stix2.versioning.new_version(d)
+
+    with pytest.raises(stix2.exceptions.ObjectNotVersionableError):
+        # should fail even if we provide a "created" kwarg.
+        stix2.versioning.new_version(d, created="1985-06-29T06:09:51.157Z")
+
+
+def test_versioning_custom_object():
+    @stix2.v20.CustomObject(
+        "x-versionable-all-optional-20", [
+            ("created", stix2.properties.TimestampProperty()),
+            ("modified", stix2.properties.TimestampProperty()),
+            ("revoked", stix2.properties.BooleanProperty()),
+        ],
+    )
+    class CustomSDO:
+        pass
+
+    obj = CustomSDO(created="1990-12-18T17:56:11.346234Z")
+    new_obj = stix2.versioning.new_version(obj)
+
+    assert "modified" in new_obj
+    assert new_obj["modified"] > new_obj["created"]
+
+    obj = CustomSDO()
+    with pytest.raises(stix2.exceptions.ObjectNotVersionableError):
+        # fails due to insufficient properties on the object, even though its
+        # type supports versioning.
+        stix2.versioning.new_version(obj)
+
+
 def test_versioning_error_dict_no_modified_value():
     campaign_v1 = {
         'type': 'campaign',
@@ -184,10 +238,10 @@ def test_versioning_error_dict_no_modified_value():
 
 def test_making_new_version_invalid_cls():
     campaign_v1 = "This is a campaign."
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError) as excinfo:
         stix2.versioning.new_version(campaign_v1, name="fred")
 
-    assert 'cannot create new version of object of this type' in str(excinfo.value)
+    assert excinfo.value.object is campaign_v1
 
 
 def test_revoke_dict():
@@ -206,7 +260,7 @@ def test_revoke_dict():
 
 def test_revoke_unversionable():
     sco = stix2.v20.File(name="data.txt")
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         sco.revoke()
 
 
@@ -277,7 +331,7 @@ def test_version_unversionable_dict():
         "name": "data.txt",
     }
 
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         stix2.versioning.new_version(f)
 
 
@@ -295,10 +349,10 @@ def test_version_sco_with_modified():
         "modified": "1991-05-13T19:24:57Z",
     }
 
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         stix2.versioning.new_version(file_sco, name="newname.txt")
 
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         stix2.versioning.revoke(file_sco)
 
     file_sco_obj = stix2.v20.File(
@@ -307,10 +361,10 @@ def test_version_sco_with_modified():
         modified="1991-05-13T19:24:57Z",
     )
 
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         stix2.versioning.new_version(file_sco_obj, name="newname.txt")
 
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         stix2.versioning.revoke(file_sco_obj)
 
 
@@ -337,6 +391,45 @@ def test_version_sco_with_custom():
     revoked_obj = stix2.versioning.revoke(new_file_sco_obj)
     assert revoked_obj.revoked
 
+    # Same thing with a dict
+    d = {
+        "type": "file",
+        "name": "data.txt",
+        "created": "1973-11-23T02:31:37Z",
+        "modified": "1991-05-13T19:24:57Z",
+        "revoked": False,
+    }
+
+    new_d = stix2.versioning.new_version(d, size=1234)
+    assert new_d["size"] == 1234
+
+    revoked_d = stix2.versioning.revoke(new_d)
+    assert revoked_d["revoked"]
+
+
+def test_version_marking():
+    m = stix2.v20.MarkingDefinition(
+        created="1982-11-29T12:20:13.723Z",
+        definition_type="statement",
+        definition={"statement": "Copyright (c) 2000-2020 Acme Corp"},
+    )
+
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
+        stix2.versioning.new_version(m)
+
+    m = {
+        "type": "marking-definition",
+        "id": "marking-definition--2a9f3f6e-5cbd-423b-a40d-02aefd29e612",
+        "created": "1982-11-29T12:20:13.723Z",
+        "definition_type": "statement",
+        "definition": {
+            "statement": "Copyright (c) 2000-2020 Acme Corp",
+        },
+    }
+
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
+        stix2.versioning.new_version(m)
+
 
 def test_version_disable_custom():
     m = stix2.v20.Malware(
diff --git a/stix2/test/v21/test_versioning.py b/stix2/test/v21/test_versioning.py
index 051fb2e..c7b6f11 100644
--- a/stix2/test/v21/test_versioning.py
+++ b/stix2/test/v21/test_versioning.py
@@ -4,6 +4,7 @@ import pytest
 
 import stix2
 import stix2.exceptions
+import stix2.properties
 import stix2.utils
 import stix2.v21
 import stix2.versioning
@@ -179,6 +180,62 @@ def test_versioning_error_dict_bad_modified_value():
         "but have the same id and modified timestamp do not have defined consumer behavior."
 
 
+def test_versioning_dict_unregistered_no_modified():
+    d = {
+        "type": "not-registered",
+        "id": "not-registered--4da54535-47b7-468c-88fa-d13b04033c4b",
+        "spec_version": "2.1",
+        "created": "1995-04-07T15:37:48.178Z",
+    }
+
+    new_d = stix2.versioning.new_version(d)
+    assert "modified" in new_d
+    assert new_d["modified"] > stix2.utils.parse_into_datetime(d["created"])
+
+    new_d = stix2.versioning.new_version(d, modified="1996-11-20T01:19:29.134Z")
+    assert new_d["modified"] == "1996-11-20T01:19:29.134Z"
+
+
+def test_versioning_dict_unregistered_unversionable():
+    d = {
+        "type": "not-registered",
+        "id": "not-registered--4da54535-47b7-468c-88fa-d13b04033c4b",
+        "spec_version": "2.1",
+        "modified": "1995-04-07T15:37:48.178Z",
+    }
+
+    with pytest.raises(stix2.exceptions.ObjectNotVersionableError):
+        stix2.versioning.new_version(d)
+
+    with pytest.raises(stix2.exceptions.ObjectNotVersionableError):
+        # should fail even if we provide a "created" kwarg.
+        stix2.versioning.new_version(d, created="1985-06-29T06:09:51.157Z")
+
+
+def test_versioning_custom_object():
+    @stix2.v21.CustomObject(
+        "x-versionable-all-optional-21", [
+            ("created", stix2.properties.TimestampProperty()),
+            ("modified", stix2.properties.TimestampProperty()),
+            ("revoked", stix2.properties.BooleanProperty()),
+        ],
+    )
+    class CustomSDO:
+        pass
+
+    obj = CustomSDO(created="1990-12-18T17:56:11.346234Z")
+    new_obj = stix2.versioning.new_version(obj)
+
+    assert "modified" in new_obj
+    assert new_obj["modified"] > new_obj["created"]
+
+    obj = CustomSDO()
+    with pytest.raises(stix2.exceptions.ObjectNotVersionableError):
+        # fails due to insufficient properties on the object, even though its
+        # type supports versioning.
+        stix2.versioning.new_version(obj)
+
+
 def test_versioning_error_dict_no_modified_value():
     campaign_v1 = {
         'type': 'campaign',
@@ -193,10 +250,10 @@ def test_versioning_error_dict_no_modified_value():
 
 def test_making_new_version_invalid_cls():
     campaign_v1 = "This is a campaign."
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError) as excinfo:
         stix2.versioning.new_version(campaign_v1, name="fred")
 
-    assert 'cannot create new version of object of this type' in str(excinfo.value)
+    assert excinfo.value.object is campaign_v1
 
 
 def test_revoke_dict():
@@ -216,7 +273,7 @@ def test_revoke_dict():
 
 def test_revoke_unversionable():
     sco = stix2.v21.File(name="data.txt")
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         sco.revoke()
 
 
@@ -318,7 +375,7 @@ def test_version_unversionable_dict():
         "name": "data.txt",
     }
 
-    with pytest.raises(ValueError):
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
         stix2.versioning.new_version(f)
 
 
@@ -345,6 +402,23 @@ def test_version_sco_with_custom():
     revoked_obj = stix2.versioning.revoke(new_file_sco_obj)
     assert revoked_obj.revoked
 
+    # Same thing with a dict
+    d = {
+        "type": "file",
+        "id": "file--d287f10a-98b4-4a47-8fa0-64b12695ea58",
+        "spec_version": "2.1",
+        "name": "data.txt",
+        "created": "1973-11-23T02:31:37Z",
+        "modified": "1991-05-13T19:24:57Z",
+        "revoked": False,
+    }
+
+    new_d = stix2.versioning.new_version(d, size=1234)
+    assert new_d["size"] == 1234
+
+    revoked_d = stix2.versioning.revoke(new_d)
+    assert revoked_d["revoked"]
+
 
 def test_version_sco_id_contributing_properties():
     file_sco_obj = stix2.v21.File(
@@ -378,6 +452,33 @@ def test_version_sco_id_contributing_properties_dict():
     assert e.value.unchangable_properties == {"name"}
 
 
+def test_version_marking():
+    m = stix2.v21.MarkingDefinition(
+        name="a name",
+        created="1982-11-29T12:20:13.723Z",
+        definition_type="statement",
+        definition={"statement": "Copyright (c) 2000-2020 Acme Corp"},
+    )
+
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
+        stix2.versioning.new_version(m)
+
+    m = {
+        "type": "marking-definition",
+        "id": "marking-definition--2a9f3f6e-5cbd-423b-a40d-02aefd29e612",
+        "spec_version": "2.1",
+        "name": "a name",
+        "created": "1982-11-29T12:20:13.723Z",
+        "definition_type": "statement",
+        "definition": {
+            "statement": "Copyright (c) 2000-2020 Acme Corp",
+        },
+    }
+
+    with pytest.raises(stix2.exceptions.TypeNotVersionableError):
+        stix2.versioning.new_version(m)
+
+
 def test_version_disable_custom():
     m = stix2.v21.Malware(
         name="foo", description="Steals your identity!", is_family=False,