-
Notifications
You must be signed in to change notification settings - Fork 278
VerificationResult should include keys for keyids #2544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
IMO we should dismiss option 1. And I slightly lean towards option 3, because it doesn't require a promises about but actually provides a set-like behavior for the relevant fields. If we use a dicts we could also remove the
|
There's another subtle issue with On a single result, which isn't That said, it would be very nice to have an abstraction over the two verification results potentially needed for root, in order to reduce case handling in calling code. |
So the new requirements/wishlist based on these implementation experiences and a short chat with Lukas:
So possibly something like class _DelegatorMixin():
# this method signature is unchanged
def get_verification_result(
self,
delegated_role: str,
payload: bytes,
signatures: Dict[str, Signature],
) -> VerificationResult
class Root():
# new method to get a combined verification result from two roles (self and other)
def get_root_verification_result(
self,
other: Root,
payload: bytes,
signatures: Dict[str, Signature],
) -> RootVerificationResult
# build a RootVerificationResult using self.get_verification_result()
# and other.get_verification_result()
@dataclass
class VerificationResult:
# New field threshold, modified type of signed and unsigned
verified: bool
threshold: int
signed: Dict[str, Key]
unsigned: Dict[str, Key]
def __bool__(self) -> bool:
return self.verified
@dataclass
class RootVerificationResult:
# This dataclass operates like a VerificationResult except:
# * No threshold
# * you can access underlying VerificationResults if you need to
results: tuple[VerificationResult, VerificationResult]
def __bool__(self) -> bool:
return self.verified
@property
def verified(self) -> bool:
return self.results[0].verified and self.results[1].verified
@property
def signed(self) -> dict[str, Key]:
# return a "union" of both results[0].signed and results[1].signed
@property
def unsigned(self) -> dict[str, Key]:
# return a "union" of both results[0].unsigned and results[1].unsigned |
This look great. But we also need to take into account special case root v1, where |
The
get_verification_result
method returns aVerificationResult
, which contains two sets of TUF keyids.Typical usage of a VerificationResult entails retrieving the
Key
objects for those keyids from the corresponding root metadata keystore. It would be useful, ifVerificationResult
included Key objects right away.Options are:
Key
class hashable and immutable, which is possible but maybe overly complicated (see discussion))The text was updated successfully, but these errors were encountered: