From 1ed0e9b36bf378274c39c3443989daefa42ee23f Mon Sep 17 00:00:00 2001
From: Jussi Kukkonen <jkukkonen@google.com>
Date: Thu, 3 Oct 2024 17:01:12 +0300
Subject: [PATCH] Fix keyid calculation issues

The spec currently still requires keyids to be calculated from the key
content: this is dumb but it is in the spec.

python-tuf/securesystemslib does not (and can't) make sure the keyid
matches the key content: do that manually in the cases where we modify
the key content.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
---
 tuf_conformance/test_basic.py | 48 ++++++++++++++++++++++++++++++++---
 tuf_conformance/test_keys.py  | 21 ---------------
 2 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/tuf_conformance/test_basic.py b/tuf_conformance/test_basic.py
index ae36875..23c9ced 100644
--- a/tuf_conformance/test_basic.py
+++ b/tuf_conformance/test_basic.py
@@ -3,12 +3,22 @@
 import os
 
 import pytest
-from tuf.api.metadata import Metadata, Root, Snapshot, Targets, Timestamp
+from securesystemslib.formats import encode_canonical
+from securesystemslib.hash import digest
+from tuf.api.metadata import Key, Metadata, Root, Snapshot, Targets, Timestamp
 
 from tuf_conformance.client_runner import ClientRunner
 from tuf_conformance.simulator_server import SimulatorServer
 
 
+def recalculate_keyid(key: Key) -> None:
+    """method to recalculate keyid: needed if key content is modified"""
+    data: bytes = encode_canonical(key.to_dict()).encode()
+    hasher = digest("sha256")
+    hasher.update(data)
+    key.keyid = hasher.hexdigest()
+
+
 def test_basic_refresh_requests(client: ClientRunner, server: SimulatorServer) -> None:
     """Test basic client functionality.
 
@@ -275,10 +285,13 @@ def test_custom_fields(client: ClientRunner, server: SimulatorServer) -> None:
     assert client.init_client(init_data) == 0
     assert client.refresh(init_data) == 0
 
+    signer = repo.new_signer()
+    signer.public_key.unrecognized_fields["extra-field"] = {"a": 1, "b": 2}
+    recalculate_keyid(signer.public_key)
+
     # Add some custom fields into root metadata, make a new root version
     repo.root.unrecognized_fields["custom-field"] = "value"
-    keyid = repo.root.roles[Root.type].keyids[0]
-    repo.root.keys[keyid].unrecognized_fields["extra-field"] = {"a": 1, "b": 2}
+    repo.add_key(Root.type, signer=signer)
     repo.root.roles[Root.type].unrecognized_fields["another-field"] = "value"
     repo.publish([Root.type])
 
@@ -287,6 +300,35 @@ def test_custom_fields(client: ClientRunner, server: SimulatorServer) -> None:
     assert client.version(Root.type) == 2
 
 
+def test_deprecated_keyid_hash_algorithms(
+    client: ClientRunner, server: SimulatorServer
+) -> None:
+    """This test sets a misleading "keyid_hash_algorithms" value: this field is not
+    a part of the TUF spec and should not affect clients.
+    """
+    init_data, repo = server.new_test(client.test_name)
+    assert client.init_client(init_data) == 0
+
+    # remove current snapshot key
+    old_keyid = repo.root.roles[Snapshot.type].keyids[0]
+    repo.root.revoke_key(old_keyid, Snapshot.type)
+    del repo.signers[Snapshot.type][old_keyid]
+
+    # Use a key with the custom field
+    signer = repo.new_signer()
+    signer.public_key.unrecognized_fields = {"keyid_hash_algorithms": "md5"}
+    recalculate_keyid(signer.public_key)
+    repo.add_key(Snapshot.type, signer=signer)
+
+    repo.publish([Root.type, Snapshot.type, Timestamp.type])  # v2
+
+    # All metadata should update; even though "keyid_hash_algorithms"
+    # value is "wrong", it is not a part of the TUF spec.
+    assert client.refresh(init_data) == 0
+    assert client.version(Root.type) == 2
+    assert client.version(Snapshot.type) == 2
+
+
 def test_snapshot_404(client: ClientRunner, server: SimulatorServer) -> None:
     """Verify that missing snapshot version is handled correctly"""
     init_data, repo = server.new_test(client.test_name)
diff --git a/tuf_conformance/test_keys.py b/tuf_conformance/test_keys.py
index 62e85bd..9acfcbf 100644
--- a/tuf_conformance/test_keys.py
+++ b/tuf_conformance/test_keys.py
@@ -41,27 +41,6 @@ def test_snapshot_does_not_meet_threshold(
     assert client.version(Snapshot.type) != 3
 
 
-def test_deprecated_keyid_hash_algorithms(
-    client: ClientRunner, server: SimulatorServer
-) -> None:
-    """This test sets a misleading "keyid_hash_algorithms" value: this field is not
-    a part of the TUF spec and should not affect clients.
-    """
-    init_data, repo = server.new_test(client.test_name)
-    assert client.init_client(init_data) == 0
-
-    # Set snapshot keys "keyid_hash_algorithms" to an incorrect algorithm.
-    valid_key = repo.root.roles[Snapshot.type].keyids[0]
-    repo.root.keys[valid_key].unrecognized_fields = {"keyid_hash_algorithms": "md5"}
-    repo.publish([Root.type])  # v2
-
-    # All metadata should update; even though "keyid_hash_algorithms"
-    # is wrong, it is not a part of the TUF spec.
-    assert client.refresh(init_data) == 0
-    assert client.version(Root.type) == 2
-    assert client.version(Snapshot.type) == 1
-
-
 def test_snapshot_has_too_few_keys(
     client: ClientRunner, server: SimulatorServer
 ) -> None: