One faulty version of root md can prevent updating to subsequent valid versions

[AdamKorcz]

During the ongoing work on TUFs conformance test suite, one of the tests uncovered a case whereby a faulty version of root metadata could prevent the client from updating to any subsequent valid versions of the root metadata.

Essentially, if the metadata in root version N is invalid (for example if it has wrong keytype), and the metadata in version N+1 is valid, then the client will not be able to update to version N+1, because python-tuf loops through all newer version and throws an exception if any of them are invalid:

https://github.com/theupdateframework/python-tuf/blob/970dd075f14fdff51a4f1facb51f669aab0b0e10/tuf/ngclient/updater.py#L325-L339

As such, if someone places invalid root metadata in the repository, the client will never be able to update ever again even if all versions after the invalid version are valid. This also includes a case where the error is irrelevant for blocking the update. For example, if the root metadata has 10 valid keys and one invalid key (wrong keytype or scheme for example), and the threshold is 5, python-tuf can still not update because python-tuf throws an exception if anything is wrong.

The test that caught this is this one, which creates a root metadata version where the keytype and scheme don't match the actual scheme and keytype of the key and then bumps the root version. The repository then sets a low threshold for which the root metadata has valid keys and then reinstates the correct keytype and scheme and bumps the root version. Now version N is invalid and version N+1 is valid. When the client updates, it fails at version N and aborts the update cycle.

Stacktrace:

  File "/home/adam/Documents/forked-tuf-conformance/env/lib/python3.11/site-packages/tuf/ngclient/updater.py", line 143, in refresh
    self._load_root()
  File "/home/adam/Documents/forked-tuf-conformance/env/lib/python3.11/site-packages/tuf/ngclient/updater.py", line 333, in _load_root
    self._trusted_set.update_root(data)
  File "/home/adam/Documents/forked-tuf-conformance/env/lib/python3.11/site-packages/tuf/ngclient/_internal/trusted_metadata_set.py", line 182, in update_root
    new_root, new_root_bytes, new_root_signatures = self._load_data(
                                                    ^^^^^^^^^^^^^^^^
  File "/home/adam/Documents/forked-tuf-conformance/env/lib/python3.11/site-packages/tuf/ngclient/_internal/trusted_metadata_set.py", line 463, in _load_from_metad
ata
    md = Metadata[T].from_bytes(data)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/adam/Documents/forked-tuf-conformance/env/lib/python3.11/site-packages/tuf/api/metadata.py", line 265, in from_bytes
    return deserializer.deserialize(data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/adam/Documents/forked-tuf-conformance/env/lib/python3.11/site-packages/tuf/api/serialization/json.py", line 42, in deserialize
    raise DeserializationError("Failed to deserialize JSON") from e
tuf.api.serialization.DeserializationError: Failed to deserialize JSON

In version N, the test sets the scheme to rsa which is wrong. When the client updates after version N+1 (the valid version) is available, the exception in json.py is Unsupported public key ecdsa/rsa.

The question here is, should TUF clients fail if version N is invalid but version N+1 is valid? If yes, then python-tuf behaves correctly.

For info @jku @DavidKorczynski @haydentherapper @bobcallaway

[jku]

There's two cases here:

1. if root vN is invalid, the spec is IMO clear: client should not continue with N+1. There is potential to investigate whether it would be safe to try N+1 and whether that could be a way to avoid some practical incompatibility issues we have seen, but this process should start in the spec, not in python-tuf
2. If there is a an unknown keytype in root vN, should that make the metadata invalid or not (assuming that signing threshold is still reached)? This is a case we've debated before and I'm not 100% sure which choice is better, currently python-tuf considers root vN invalid. There might be a case where the other approach would be more robust in practice but I am not sure I've seen that so far.

[jku]

I think I'll close this since the main question is not under debate:

should TUF clients fail if version N is invalid but version N+1 is valid?

according to current spec you need to be able process each root version to continue.