Repository navigation
Metadata API: add _Delegator.get_delegate_signing_status() #2449
Description
Activity
Use cases:
For tuf-on-ci use case I want:
- list of keys that have signed correctly
- list of keys that have not signed (I don't care if the sig is missing or incorrect)
- has threshold been reached
Things that I don't care that much about:
- the actual signatures (easy to lookup with keys if needed)
- threshold (easy to lookup)
- number of good sigs (easy to calculate)
so from my POV something like this:
@dataclass class SigningStatus: verified: bool signed_keys: list[Key] unsigned_keys: list[Key] def delegate_signing_status( delegated_role: str, delegated_metadata: "Metadata", signed_serializer: Optional[SignedSerializer] = None ) -> SigningStatus:
Reacted by Lukas PühringerOne additional related use case I saw in both tuf-on-ci and RSTUF:
- is a given signature valid for delegate according to delegator?
It's easy enough to implement in the application. I'm bringing it up here, because, if
delegate_signing_statusreturns the valid signatures, it could be used for that purpose. OTOH, the validity of a given signature might be interesting by itself, regardless of other signatures. So maybe we should treat this as separate feature. Any thoughts?Re name: I'm a bit unhappy with the verb/noun ambiguity of delegate.
delegate_signing_status, to me, sounds like the activity of delegating the signing status, for whatever that means. But I'm okay, if we clarify this in the docstring.is a given signature valid for delegate according to delegator?
It could be I just don't remember the specific location but I think tuf-on-ci question is (or should be) "has this signing key signed this delegated role?"
I think in 99% of cases applications should not touch or think about the actual signatures at all.
It could be I just don't remember the specific location but I think tuf-on-ci question is (or should be) "has this signing key signed this delegated role?"
You are right, I mis-skimmed the code I was referring to. nvm
[edit: fixed math after talking to Jussi]
I think Jussi's proposal is complete.
Returning "invalid signatures" as well does not make a lot of sense, because the signatures could be invalid according to one delegator, but valid according to another, which is not unlikely in the case of a root update. So, to get the desired information -- "does the metadata have invalid signatures?" -- we'd need to do set operations on multiple results anyway, and this can be done with the existing fields too. E.g. root update
stat_old = old_root.delegate_signing_status("root", new_root) stat_new = new_root.delegate_signing_status("root", new_root) has_invalid_sigs = len(metadata.signatures) > len(stat_old.signed_keys | stat_new.signed_keys) # assume signed_keys is a set of keyids invalid_sigs = metadata.signatures.keys() - (stat_old.signed_keys | stat_new.signed_keys)
Yes, I agree with what you said.
I think I like the unstated assumption in your examples that the key lists are actually sets -- then you can do unions more easily, and I can see that would be useful.
The "math" in your code has some issues but I don't think they're very important:
has_invalid_sigs = len(metadata.signatures) > (len(stat_old.signed_keys) + len(stat_new.signed_keys)) # assume signed_keys is a set of keyidsThat's not right (keys might be duplicate and anyway thresholds may be different). Like you said, for root you'd just have to duplicate whatever you do normally to find invalid sigs (whatever "invalid" means in this context): you'd then have two separate lists of "invalid sigs"... unsure what you would want to do with them.
I think there are more useful questions and they seem fairly easy to answer:
- "which root keys have not yet signed correctly, but could still do so":
unsigned_keys = stat_old.unsigned_keys | stat_new.unsigned_keys - "Which root keys may need to act for the metadata to become verified":
missing_keys = set() if not stat_old.verified: missing_keys |= stat_old.unsigned_keys if not stat_new.verified: missing_keys |= stat_new.unsigned_keys
- "which root keys have not yet signed correctly, but could still do so":
On the idea of using set instead of list for Keys: this does not currently work as Key is not hashable. We could easily make them hashable though
Reacted by Lukas Pühringer and Martin VrachevCross-posting from slack, by "invalid signature" I meant:
- there is no authorized keyid for a signature's keyid in any eligible delegator, or
- there is an authorized keyid but no public key in the corresponding keystore, or
- there is an authorized keyid and a public key but verification fails cryptographically
I edited my "math" in the comment above to correctly support these checks
On the idea of using set instead of list for Keys: this does not currently work as Key is not hashable. We could easily make them hashable though
Or maybe not: hashable objects are not supposed to be mutable (the hash should be a constant for the lifetime of the object). Strictly speaking that's not true for Keys even if our keys almost never mutate. We could return keyids instead of keys to get around this. Then my use case is still reasonable if not anymore trivial:
status = root.delegate_signing_status("timestamp", timestamp_metadata) signed = [root.get_key(id) for id in status.signed_keys]Or we could just say the Keys are immutable enough so that we can put them in this short lived set 🤷
- added 2 commits that reference this issue
on Sep 8, 2023 Or we could just say the Keys are immutable enough so that we can put them in this short lived set 🤷
Hm, I suppose it's okay, if we document the caveat. Alternatively, we could implement a custom key container, which provides all the desired set operations for contained Key objects. And which clearly documents that mutating the items breaks the container.
I think returning set of keyids is ok and fits the rest of the API.
If someone wants to take the WIP commit above and polish, I'm totally fine with that.
As a note on the implementation, I originally included threshold in the result but decided against that: A useful operation is to "union" two VerificationResults (in the root case) and I think providing that in the library might be good... Two thresholds however cannot be "unioned".Reacted by Lukas Pühringer- added 2 commits that reference this issue
on Oct 3, 2023
_Delegator.verify_delegateprovides a binary answer for whether a delegated role has enough valid signatures to meet the threshold of keys defined by its delegator or not.While this is sufficient for client validation, TUF applications might also be interested in more granular information, e.g. when implementing a distributed signing feature for TUF metadata, so they can tell users how many signatures by what keys are still missing.
Interesting information would be:
verify_thresholdonly verifies until threshold is met)Given that threshold verification is such a crucial part of the Metadata API and can be done wrong so easily, adding a new public function -- e.g.
_Delegator.get_delegate_signing_status()-- to Metadata API seems like a good idea.