theupdateframework / specification

The Update Framework specification
https://theupdateframework.github.io/specification/
Other
368 stars 54 forks source link

Add notes on uniqueness of keyids and role names #166

Closed jku closed 3 years ago

jku commented 3 years ago

I'm making explicit some requirements that I think are already implicit in the spec:

I guess the high level question is: is this really a "bug fix" or should it be handled as a change in spec? I think it's a fix that clarifies what is already the obvious/logical/safe implementation of the spec, but especially on the role names I can see someone could maybe argue otherwise.

mnm678 commented 3 years ago

The challenge for both of these requirements is what scope they need to be unique within. I agree that within a single targets metadata file, it would be wrong to have non-unique keyids, but what about between two different delegated targets metadata files? There might be a reason to have the same keyid mean different things in different role's delegations.

But with a well-defined scope, I think that this could be a useful clarification to the specification.

jku commented 3 years ago

Yes, agreed on the future goal: uniqueness of keys should mean uniqueness within the "signatures" object only.

But as a bug fix I think this is the best we can do -- leave the keyid definition as is but clarify the aspect that makes safer implementations possible (an aspect that is already implicit: if the keyid is a hash it should be unique).

Admittedly, delegated role uniqueness is not really even implicit in the spec, it's just that I think it's what the authors intended because otherwise the delegation verification is a mess? The fact that roles in Targets is not a dictionary is just a side-effect of json not having an ordered dictionary... otherwise I expect it would be a dict just like root roles are.

jku commented 3 years ago

I think it's a fix that clarifies what is already the obvious/logical/safe implementation of the spec, but especially on the role names I can see someone could maybe argue otherwise.

I said that but Teodora pointed me to language in the spec that pretty clearly implies my reading is correct, and role names are meant to be unique:

In order to accommodate prioritized delegations, the "roles" key in the DELEGATIONS object above points to an array of delegated roles, rather than to a hash table.

So delegations is an array only because "ordered hashtable" is not a thing in json.

jku commented 3 years ago

replaced "dictionary" with "array", sorry about doing that right after the reviews...