From 452aad8d00c6f80ab8c67d3e47edb7c6c676e5f0 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 7 Jul 2021 13:58:24 +0300 Subject: [PATCH 1/2] Metadata API: fix Key validation exception In pr https://github.com/theupdateframework/tuf/pull/1449 we introduced Key validation and there decided to raise "ValueError" if one of keyid, scheme, keyval or keytype is not a string. That seems like a mistake given that in the python docs it's written: "Passing arguments of the wrong type (e.g. passing a list when an int is expected) should result in a TypeError, but passing arguments with the wrong value (e.g. a number outside expected boundaries) should result in a ValueError." Signed-off-by: Martin Vrachev --- tuf/api/metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 24cc8079..e3936697 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -427,7 +427,7 @@ def __init__( if not all( isinstance(at, str) for at in [keyid, keytype, scheme] ) or not isinstance(keyval, Dict): - raise ValueError("Unexpected Key attributes types!") + raise TypeError("Unexpected Key attributes types!") self.keyid = keyid self.keytype = keytype self.scheme = scheme From 9c8aa1da06fa4c4b88a2b33818573a1885b838f3 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 22 Jun 2021 14:34:25 +0300 Subject: [PATCH 2/2] Test new API: serialization tests for invalid arg A while ago we decided that it's best to research each of the individuals attributes one by one and identify what level of validation it needs compared to how we use it: https://github.com/theupdateframework/tuf/pull/1366#issuecomment-829288790. This work is ongoing and there are a couple of commits already merged for this: - https://github.com/theupdateframework/tuf/commit/6c5d970799ffe1f79d02e24a8ea56afeae4e636b - https://github.com/theupdateframework/tuf/commit/f20664d2fc967852d8e3aca7f951e4d1e7855e61 - https://github.com/theupdateframework/tuf/commit/41afb1e1342d5f8870180ba1ecea8894c03aec24 We want to be able to test the attributes validation against known bad values. The way we want to do that is with table testing we have added using decorators for our metadata classes defined in New API: https://github.com/theupdateframework/tuf/pull/1416. This gives us an easy way to add new cases for each of the attributes and not depend on external files. Signed-off-by: Martin Vrachev --- tests/test_metadata_serialization.py | 111 ++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 4 deletions(-) diff --git a/tests/test_metadata_serialization.py b/tests/test_metadata_serialization.py index 9662631a..813f8282 100644 --- a/tests/test_metadata_serialization.py +++ b/tests/test_metadata_serialization.py @@ -12,11 +12,13 @@ import unittest import copy -from typing import Dict, Callable +from typing import Dict, Callable, Optional, Mapping, Any +from datetime import datetime from tests import utils from tuf.api.metadata import ( + Signed, Root, Snapshot, Timestamp, @@ -50,6 +52,44 @@ def wrapper(test_cls: "TestSerialization"): class TestSerialization(unittest.TestCase): + # Snapshot instances with meta = {} are valid, but for a full valid + # repository it's required that meta has at least one element inside it. + invalid_signed: DataSet = { + "no _type": '{"spec_version": "1.0.0", "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "no spec_version": '{"_type": "signed", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "no version": '{"_type": "signed", "spec_version": "1.0.0", "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "no expires": '{"_type": "signed", "spec_version": "1.0.0", "version": 1, "meta": {}}', + "empty str _type": + '{"_type": "", "spec_version": "1.0.0", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "empty str spec_version": + '{"_type": "signed", "spec_version": "", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "_type wrong type": + '{"_type": "foo", "spec_version": "1.0.0", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "version wrong type": + '{"_type": "signed", "spec_version": "1.0.0", "version": "a", "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "invalid spec_version str": + '{"_type": "signed", "spec_version": "abc", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "two digit spec_version": + '{"_type": "signed", "spec_version": "1.2.a", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "no digit spec_version": + '{"_type": "signed", "spec_version": "a.b.c", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "different major spec_version": + '{"_type": "signed", "spec_version": "0.0.0", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "version 0": + '{"_type": "signed", "spec_version": "1.0.0", "version": 0, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "version below 0": + '{"_type": "signed", "spec_version": "1.0.0", "version": -1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', + "wrong datetime string": + '{"_type": "signed", "spec_version": "1.0.0", "version": 1, "expires": "abc", "meta": {}}', + } + + @run_sub_tests_with_dataset(invalid_signed) + def test_invalid_signed_serialization(self, test_case_data: Dict[str, str]): + case_dict = json.loads(test_case_data) + with self.assertRaises((KeyError, ValueError, TypeError)): + Snapshot.from_dict(copy.deepcopy(case_dict)) + + valid_keys: DataSet = { "all": '{"keytype": "rsa", "scheme": "rsassa-pss-sha256", \ "keyval": {"public": "foo"}}', @@ -60,12 +100,45 @@ class TestSerialization(unittest.TestCase): } @run_sub_tests_with_dataset(valid_keys) - def test_key_serialization(self, test_case_data: str): + def test_valid_key_serialization(self, test_case_data: str): case_dict = json.loads(test_case_data) key = Key.from_dict("id", copy.copy(case_dict)) self.assertDictEqual(case_dict, key.to_dict()) + invalid_keys: DataSet = { + "no keyid": '{"keytype": "rsa", "scheme": "rsassa-pss-sha256", "keyval": {"public": "abc"}}', + "no keytype": '{"keyid": "id", "scheme": "rsassa-pss-sha256", "keyval": {"public": "foo"}}', + "no scheme": '{"keyid": "id", "keytype": "rsa", "keyval": {"public": "foo"}}', + "no keyval": '{"keyid": "id", "keytype": "rsa", "scheme": "rsassa-pss-sha256"}', + "keyid wrong type": '{"keyid": 1, "keytype": "rsa", "scheme": "rsassa-pss-sha256", "keyval": {"public": "abc"}}', + "keytype wrong type": '{"keyid": "id", "keytype": 1, "scheme": "rsassa-pss-sha256", "keyval": {"public": "abc"}}', + "scheme wrong type": '{"keyid": "id", "keytype": "rsa", "scheme": 1, "keyval": {"public": "abc"}}', + "keyval wrong type": '{"keyid": "id", "keytype": "rsa", "scheme": "rsassa-pss-sha256", "keyval": 1}', + } + + @run_sub_tests_with_dataset(invalid_keys) + def test_invalid_key_serialization(self, test_case_data: Dict[str, str]): + case_dict = json.loads(test_case_data) + with self.assertRaises((TypeError, KeyError)): + keyid = case_dict.pop("keyid") + Key.from_dict(keyid, copy.copy(case_dict)) + + invalid_roles: DataSet = { + "no threshold": '{"keyids": ["keyid"]}', + "no keyids": '{"threshold": 3}', + "wrong threshold type": '{"keyids": ["keyid"], "threshold": "a"}', + "threshold below 1": '{"keyids": ["keyid"], "threshold": 0}', + "duplicate keyids": '{"keyids": ["keyid", "keyid"], "threshold": 3}', + } + + @run_sub_tests_with_dataset(invalid_roles) + def test_invalid_role_serialization(self, test_case_data: Dict[str, str]): + case_dict = json.loads(test_case_data) + with self.assertRaises((KeyError, TypeError, ValueError)): + Role.from_dict(copy.deepcopy(case_dict)) + + valid_roles: DataSet = { "all": '{"keyids": ["keyid"], "threshold": 3}', "unrecognized field": '{"keyids": ["keyid"], "threshold": 3, "foo": "bar"}', @@ -102,12 +175,28 @@ def test_root_serialization(self, test_case_data: str): root = Root.from_dict(copy.deepcopy(case_dict)) self.assertDictEqual(case_dict, root.to_dict()) + + invalid_metafiles: DataSet = { + "wrong length type": '{"version": 1, "length": "a", "hashes": {"sha256" : "abc"}}', + "length 0": '{"version": 1, "length": 0, "hashes": {"sha256" : "abc"}}', + "length below 0": '{"version": 1, "length": -1, "hashes": {"sha256" : "abc"}}', + "empty hashes dict": '{"version": 1, "length": 1, "hashes": {}}', + "hashes wrong type": '{"version": 1, "length": 1, "hashes": 1}', + "hashes values wrong type": '{"version": 1, "length": 1, "hashes": {"sha256": 1}}', + } + + @run_sub_tests_with_dataset(invalid_metafiles) + def test_invalid_metafile_serialization(self, test_case_data: Dict[str, str]): + case_dict = json.loads(test_case_data) + with self.assertRaises((TypeError, ValueError, AttributeError)): + MetaFile.from_dict(copy.deepcopy(case_dict)) + + valid_metafiles: DataSet = { "all": '{"hashes": {"sha256" : "abc"}, "length": 12, "version": 1}', "no length": '{"hashes": {"sha256" : "abc"}, "version": 1 }', "no hashes": '{"length": 12, "version": 1}', - "unrecognized field": '{"hashes": {"sha256" : "abc"}, "length": 12, "version": 1, \ - "foo": "bar"}', + "unrecognized field": '{"hashes": {"sha256" : "abc"}, "length": 12, "version": 1, "foo": "bar"}', } @run_sub_tests_with_dataset(valid_metafiles) @@ -181,6 +270,20 @@ def test_delegation_serialization(self, test_case_data: str): self.assertDictEqual(case_dict, delegation.to_dict()) + invalid_targetfiles: DataSet = { + "no hashes": '{"length": 1}', + "no length": '{"hashes": {"sha256": "abc"}}' + # The remaining cases are the same as for invalid_hashes and + # invalid_length datasets. + } + + @run_sub_tests_with_dataset(invalid_targetfiles) + def test_invalid_targetfile_serialization(self, test_case_data: Dict[str, str]): + case_dict = json.loads(test_case_data) + with self.assertRaises(KeyError): + TargetFile.from_dict(copy.deepcopy(case_dict)) + + valid_targetfiles: DataSet = { "all": '{"length": 12, "hashes": {"sha256" : "abc"}, \ "custom" : {"foo": "bar"} }',