go-tuf icon indicating copy to clipboard operation
go-tuf copied to clipboard

Incomplete support for delegations in repo methods which contain a role name as an argument

Open ethan-lowman-dd opened this issue 3 years ago • 3 comments

There are a number of methods which may require breaking API changes to support delegations. These include:

  • [ ] Repo.GetThreshold
  • [ ] Repo.SetThreshold
  • [ ] Repo.RevokeKeyWithExpires and caller Repo.RevokeKey
  • [ ] Repo.AddVerificationKeyWithExpiration and all callers, including Repo.GenKey, Repo.GenKeyWithExpires, Repo.AddPrivateKey, Repo.AddPrivateKeyWithExpires, and Repo.AddVerificationKey

These APIs might need to change change to support delegations, since they implement key management for roles. However, delegated targets keys are associated with delegations, not roles (see https://github.com/theupdateframework/specification/issues/214).

One option to avoid a breaking change or adding more complex APIs would be to implement them such that the key changes apply simultaneously to all incoming delegations to a given role. In most cases (except for non-tree delegation graphs such as those illustrated in https://github.com/theupdateframework/specification/issues/177), there will be only one incoming delegation to a given role, so this would be intuitive behavior.

ethan-lowman-dd avatar Apr 08 '22 21:04 ethan-lowman-dd

These APIs might need to change change to support delegations, since they implement key management for roles. However, delegated targets keys are associated with delegations, not roles (see https://github.com/theupdateframework/specification/issues/214).

Might be misinterpreting, but for GetThreshold and SetThreshold, why would the API need to change? For a delegation name, we can do a delegation search to find the delegatee role and modify the threshold thru the DelegatedRole

asraa avatar Apr 13 '22 20:04 asraa

I think what you're describing is the same as this suggestion above:

One option to avoid a breaking change or adding more complex APIs would be to implement them such that the key changes apply simultaneously to all incoming delegations to a given role.

What I'm unsure of is whether it makes sense from a user's perspective to update the threshold for all incoming DelegatedRoles, if there are multiple delegatees.

ethan-lowman-dd avatar Apr 13 '22 20:04 ethan-lowman-dd

What I'm unsure of is whether it makes sense from a user's perspective to update the threshold for all incoming DelegatedRoles, if there are multiple delegatees.

I don't think so. Some delegated roles will have different number of signers too -- however, I think in case it is easy, I think some library method should expose pulling a list of delegatees out of a delegated target file. So a user could iterate over them and set all incoming thresholds uniformly.

asraa avatar Apr 14 '22 17:04 asraa

Closing since the code base changed and so this is no longer relevant. With the new code base we support several types of delegations.

Thanks for raising this 👍

rdimitrov avatar Jan 31 '24 21:01 rdimitrov