Open lukpueh opened 6 years ago
Related: Mention metadata conformance with specification (plus version number) in docstrings (see https://github.com/in-toto/in-toto/pull/181#discussion_r166678328)
Related:
This became an issue on https://github.com/cnabio/signy/pull/80/. The PR was initially using a root layout I wrote (by hand) and signed using a demo key, and I missed defining "private": ""
in the public key definitions. signy
uses in-toto-golang
to verify, and its representation of the same layout included "private": ""
, and therefore, signature verification failed.
Verification passes on reference implementation, fails on in-toto-golang
:
{
"signatures": [
{
"keyid": "737c25cc5ed977df2979fe029909d2d36b96adb4a65b9687fb8d150eaa29a4d8",
"sig": "27d2565297b39f1d8a95463c4b55d3e7c710737bd075244504e3eeaad2cc8b0bd2064ce7d346064eefbbd1fe8444274972a8010043f03bcbd9c6a03b85047e4cd3dce40452482e6568fb8c1b6e40bc4dd627ee82e852aac87efba195b07ba3adabd05ba992864ab1decfd3b77f77fd9207b3a504b481ba99e09de72ae3df8a20755cc2597312823aab99582bb438012843d3dcd809ed739ad885733271c121a4cb7614306a4c9486443af6c8430b6a0602f915cc33fcd0f634266ab0f49e7d2c27d1d64f8052202a323e0018ff9920de471d0b55759d3ac0b13cf5665ff0b19ac240b1af2214520e0fa0ff237930f3037fb840ad80d99b8f99a47ae529443749ea2b430021cfedaa4654274701b20e5bc6fa9ed943e4e46b24a197195ed01d4395b08ee3739fc4e0456cb48502696e9d5aa2b4d5d6b3fef61a8872e6a13041a9c2d266a8486432533c28cb3aff453b2ada9cc0ddf5762dc252fbbdab0e71e3fb52e41e0daba4fcb04ec90013f2579572974ac36f50659de6ad6d0bc17141e420"
}
],
"signed": {
"_type": "layout",
"expires": "2021-05-12T11:15:05Z",
"inspect": [],
"keys": {
"2d2e5437de978b123ed8a50692cdb38c198ced0f1f356398abe9dd5206e890e2": {
"keyid": "2d2e5437de978b123ed8a50692cdb38c198ced0f1f356398abe9dd5206e890e2",
"keyid_hash_algorithms": [
"sha256",
"sha512"
],
"keytype": "rsa",
"keyval": {
"public": "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEA4X062OOJPtd//g8acQ07\n+3xnCMCsQkMkOhJlaCWkxCFqSI8bUQrzRg1jfpCRCqzhEscTy/Vw/xWNvhgGOzcc\nYyCpnwDcr3CK3G58lVz2mnSyAW9PCOGvwgLfKMC7Z99q0tLRa3YYqzrajgYsL9D2\nPh+4DrlbAADfQIok7MkkjBqvKsFptr6N+KsB3pkQyGZUYAMdUUiQU8BV2oWNfJQo\n0PHW5fi5hfBCUGzDQB9U22fhD3I6sgKVfA+948mo+Y9QRp53WvmjR47xb5KjNJR8\niIq8htwXIe7JCJxcyEh7hjKE5E6Qm9OhN2e99uta9XiIX1hIqSdh+BcRuBb1SbJD\nX3OBqqZHtUJ9hiNLz+PmlMwLS+02sqIQud4Vhjfy83KnfOoZ0PRU9sUSQDDe/wnK\nC7PWwCfyBto7wf9EWOrRjlG/DwIPiRsxrZ2XhHJu3x4jjXjrkT6MIPWZESJvonZM\n5XIekFjaYt/mnPybuwMZCulAwQZ7ucJqf9zJUFhRYgZRAgMBAAE=\n-----END PUBLIC KEY-----"
},
"scheme": "rsassa-pss-sha256"
},
"dd035ca2388fffa593a9f0cb76f3ece27fe45560da0a5f93f5f404da2faac745": {
"keyid": "dd035ca2388fffa593a9f0cb76f3ece27fe45560da0a5f93f5f404da2faac745",
"keyid_hash_algorithms": [
"sha256",
"sha512"
],
"keytype": "rsa",
"keyval": {
"public": "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAqxqOAKWUWeiEBNtH1Nx2\n/8IKsSpbxy9NoTT31uTVK57oX0EomrBuCY4Ec6kcBWDPcqp6Z+2Z9baVzRB08SzO\nIJ3tGsTTdmJaFXFXjrDix/AioNmpCFh0ditOYoxUFu+skYV7PoHQ3BH7A5m2YxdH\n+KiLh2ccCvNnL/Wdio/6hwyVp4lZMFgQdhi7ba5EzuzwLt6dkB+9Fle2PMgeF+od\n/Ly7oXXaUi5lbPNLzf4Dkhr02qeKLFap35hBqXjqcx68vOfjuYFWwFaHmkCPjDpf\nTaLbPnDtXVWM8HzHdb4lVrZCpwpj859/QuDGb+uJyto3BsSHMli2v5e9QKcxtOzn\n6c5YtQkfL7u/n/vM3WuMsq78mFLoBNzHWrEsKp1ZKtBQDXfKDaW2io8BiWlREseh\nbk9ekBEvh8BmjBPS7A62z389kjUKZYhfaRSsaLGkUv0CnRCnuEQxCHpIUztHGiRx\nNpWz3dgp3t6+f/ZNC+ro8+FoHDGzEHX42oxD25E2H16BAgMBAAE=\n-----END PUBLIC KEY-----"
},
"scheme": "rsassa-pss-sha256"
}
},
"readme": "",
"steps": [
{
"_type": "step",
"expected_command": [],
"expected_materials": [
[
"DISALLOW",
"*"
]
],
"expected_products": [
[
"CREATE",
"cnab+json:*/bundle.json$*"
],
[
"DISALLOW",
"*"
]
],
"name": "developer",
"pubkeys": [
"2d2e5437de978b123ed8a50692cdb38c198ced0f1f356398abe9dd5206e890e2"
],
"threshold": 1
},
{
"_type": "step",
"expected_command": [
"./do-machine-stuff"
],
"expected_materials": [
[
"MATCH",
"cnab+json:*/bundle.json$*",
"WITH",
"PRODUCTS",
"FROM",
"developer"
],
[
"DISALLOW",
"*"
]
],
"expected_products": [
[
"MODIFY",
"cnab+json:*/bundle.json$/images/*/contentDigest"
],
[
"MODIFY",
"cnab+json:*/bundle.json$/images/*/description"
],
[
"MODIFY",
"cnab+json:*/bundle.json$/invocationImages/*/contentDigest"
],
[
"MATCH",
"cnab+json:*/bundle.json$*",
"WITH",
"MATERIALS",
"FROM",
"machine"
],
[
"DISALLOW",
"*"
]
],
"name": "machine",
"pubkeys": [
"dd035ca2388fffa593a9f0cb76f3ece27fe45560da0a5f93f5f404da2faac745"
],
"threshold": 1
}
]
}
}
Verification passes on both reference implementation and in-toto-golang
:
{
"signatures": [
{
"keyid": "737c25cc5ed977df2979fe029909d2d36b96adb4a65b9687fb8d150eaa29a4d8",
"sig": "bc4a54ac4e7b52d6dfd36e8e0ca1b78e4f4e7d0c2b388ddd6cc738f599fc6d32b52fdbf2e2109e17debfa1a6d37969f217fb9e6f9622f980d33ca67b722ab7ef804497e8f9f070e1f4d338fb34adeb009f966f3aa42ba95d94ec8852c3b7c0399469552a99e5ddbc97368f587600468aa321887a92d7b45eba06a5a4bb932918987cbfe2ee41f88eeb614d35fbc84111c82d58faa2574a2f7b0861ab2652ce23f2a013b06e487fdf29d17ce50d1e6daace0d36657c1b292a6c3065efe6071fd24267137859e49456d69dc6cd344fd4f44a98e6a796c2ac49dbde38458d7f04ddfde1a6c64df634aece3620768828433d3f2f0b381e2956226036cdb68d94c6bdd044ca4732bb136addc037ceb43b7a2d8fa2c536a326ae8304067daa3e59e979ac0f1a079b14c66de5afb6f8d50a241c366d4fccd07dc792713e5b3e2a6f3870971c8aace1b2137341168693135c75e3d341eaa39d33cee4b83336c732fd3c305654b9c9f794d5839464455a3e9cc3a6ca4dc2ca0064d668ef392184f5933ed1"
}
],
"signed": {
"_type": "layout",
"expires": "2021-05-12T11:15:05Z",
"inspect": [],
"keys": {
"2d2e5437de978b123ed8a50692cdb38c198ced0f1f356398abe9dd5206e890e2": {
"keyid": "2d2e5437de978b123ed8a50692cdb38c198ced0f1f356398abe9dd5206e890e2",
"keyid_hash_algorithms": [
"sha256",
"sha512"
],
"keytype": "rsa",
"keyval": {
"private": "",
"public": "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEA4X062OOJPtd//g8acQ07\n+3xnCMCsQkMkOhJlaCWkxCFqSI8bUQrzRg1jfpCRCqzhEscTy/Vw/xWNvhgGOzcc\nYyCpnwDcr3CK3G58lVz2mnSyAW9PCOGvwgLfKMC7Z99q0tLRa3YYqzrajgYsL9D2\nPh+4DrlbAADfQIok7MkkjBqvKsFptr6N+KsB3pkQyGZUYAMdUUiQU8BV2oWNfJQo\n0PHW5fi5hfBCUGzDQB9U22fhD3I6sgKVfA+948mo+Y9QRp53WvmjR47xb5KjNJR8\niIq8htwXIe7JCJxcyEh7hjKE5E6Qm9OhN2e99uta9XiIX1hIqSdh+BcRuBb1SbJD\nX3OBqqZHtUJ9hiNLz+PmlMwLS+02sqIQud4Vhjfy83KnfOoZ0PRU9sUSQDDe/wnK\nC7PWwCfyBto7wf9EWOrRjlG/DwIPiRsxrZ2XhHJu3x4jjXjrkT6MIPWZESJvonZM\n5XIekFjaYt/mnPybuwMZCulAwQZ7ucJqf9zJUFhRYgZRAgMBAAE=\n-----END PUBLIC KEY-----"
},
"scheme": "rsassa-pss-sha256"
},
"dd035ca2388fffa593a9f0cb76f3ece27fe45560da0a5f93f5f404da2faac745": {
"keyid": "dd035ca2388fffa593a9f0cb76f3ece27fe45560da0a5f93f5f404da2faac745",
"keyid_hash_algorithms": [
"sha256",
"sha512"
],
"keytype": "rsa",
"keyval": {
"private": "",
"public": "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAqxqOAKWUWeiEBNtH1Nx2\n/8IKsSpbxy9NoTT31uTVK57oX0EomrBuCY4Ec6kcBWDPcqp6Z+2Z9baVzRB08SzO\nIJ3tGsTTdmJaFXFXjrDix/AioNmpCFh0ditOYoxUFu+skYV7PoHQ3BH7A5m2YxdH\n+KiLh2ccCvNnL/Wdio/6hwyVp4lZMFgQdhi7ba5EzuzwLt6dkB+9Fle2PMgeF+od\n/Ly7oXXaUi5lbPNLzf4Dkhr02qeKLFap35hBqXjqcx68vOfjuYFWwFaHmkCPjDpf\nTaLbPnDtXVWM8HzHdb4lVrZCpwpj859/QuDGb+uJyto3BsSHMli2v5e9QKcxtOzn\n6c5YtQkfL7u/n/vM3WuMsq78mFLoBNzHWrEsKp1ZKtBQDXfKDaW2io8BiWlREseh\nbk9ekBEvh8BmjBPS7A62z389kjUKZYhfaRSsaLGkUv0CnRCnuEQxCHpIUztHGiRx\nNpWz3dgp3t6+f/ZNC+ro8+FoHDGzEHX42oxD25E2H16BAgMBAAE=\n-----END PUBLIC KEY-----"
},
"scheme": "rsassa-pss-sha256"
}
},
"readme": "",
"steps": [
{
"_type": "step",
"expected_command": [],
"expected_materials": [
[
"DISALLOW",
"*"
]
],
"expected_products": [
[
"CREATE",
"cnab+json:*/bundle.json$*"
],
[
"DISALLOW",
"*"
]
],
"name": "developer",
"pubkeys": [
"2d2e5437de978b123ed8a50692cdb38c198ced0f1f356398abe9dd5206e890e2"
],
"threshold": 1
},
{
"_type": "step",
"expected_command": [
"./do-machine-stuff"
],
"expected_materials": [
[
"MATCH",
"cnab+json:*/bundle.json$*",
"WITH",
"PRODUCTS",
"FROM",
"developer"
],
[
"DISALLOW",
"*"
]
],
"expected_products": [
[
"MODIFY",
"cnab+json:*/bundle.json$/images/*/contentDigest"
],
[
"MODIFY",
"cnab+json:*/bundle.json$/images/*/description"
],
[
"MODIFY",
"cnab+json:*/bundle.json$/invocationImages/*/contentDigest"
],
[
"MATCH",
"cnab+json:*/bundle.json$*",
"WITH",
"MATERIALS",
"FROM",
"machine"
],
[
"DISALLOW",
"*"
]
],
"name": "machine",
"pubkeys": [
"dd035ca2388fffa593a9f0cb76f3ece27fe45560da0a5f93f5f404da2faac745"
],
"threshold": 1
}
]
}
}
cc @radu-matei @trishankatdatadog
Thanks for reporting, @adityasaky! I'd say this is only mildly related to the original issue here, which requested checks for non-specified fields on the layout.
The issue you describe is more about a seemingly under-specified field. Although, the in-toto specification is actually clear about public keys:
Link and Layout metadata does not include the private portion of the key object:
{ "keytype" : "rsa", "scheme" : "rsassa-pss-sha256", "keyval" : { "public" : "<PUBLIC>"} }
So there are two problems:
I lean towards requiring strict adherence to the spec in both implementations, but that might break old layouts. @SantiagoTorres, what do you think?
Thanks, @lukpueh. I'd like to add that I think we should get rid of any mention of the private portion of the key from the spec to avoid any confusion. The TUF spec has done that as far as I can tell and our key formats section mirrors that one.
@adityasaky, we do need the "private" portion for signing.
Right, but that's a convenience in implementing signing, right? We don't technically need to define it in our metadata formats? I may be way off here, though.
@adityasaky I would like to work on this Issue
I am not convinced that this is a good first issue. We need to carefully consider what we actually want here and whether it will break any existing users.
For instance, the very similar TUF metadata API can parse metadata with unrecognized fields (see ADR 8), and this has proven indispensable in practice.
Generally, much thought has gone into how to best parse and validate TUF metadata (see https://github.com/theupdateframework/python-tuf/issues/1130).
Let's first review if there are more ideas in the TUF metadata API design that we want to adopt here, before we start making small changes.
Description of issue or feature request: Check that metadata (
Metablock
,Layout
,Step
,Inspection
,Link
) loaded from Python dicts (json files) is fully conformant.Thanks to @awwad for pointing out the issue in https://github.com/in-toto/in-toto/pull/179#pullrequestreview-94858221
Current behavior: Currently in-toto metadata classes are validated after instantiation, i.e. after the constructors have parsed out the relevant fields from the passed
**kwargs
and have assigned default values for missing fields.As a consequence metadata validation only catches supplied fields with faulty values, but doesn't fail if required fields are missing or unallowed fields were passed. Which in turn can lead to a valid metadata object created from an invalid metadata dictionary (json file).
Expected behavior: While the existing behavior is convenient in the constructor, i.e. to create a new empty metadata object, we should probably add additional checks in the classmethods that instantiate a metadata object from a passed dictionary, to fail in case of missing required fields or existing unallowed fields. The related methods are
Metablock.load
and{Layout, Step, Inspection, Link}.read
.