Skip to content
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

Metadata API: implement verify in Key, store id in key #1417

Closed
jku opened this issue May 26, 2021 · 3 comments · Fixed by #1423
Closed

Metadata API: implement verify in Key, store id in key #1417

jku opened this issue May 26, 2021 · 3 comments · Fixed by #1423
Assignees
Labels
backlog Issues to address with priority for current development goals
Milestone

Comments

@jku
Copy link
Member

jku commented May 26, 2021

This is a potential API improvement I'm considering (both ideas originally from @sechkova i think):

  • Implement verify_signature() in Key class
  • Store keyid in Key object

Implementing verify() in Key would make sense because:

  • having both verify() and verify_with_threshold()/verify_delegate() in Metadata is tricky because former verifies a sig on itself, and latter would verifiy threshold of sigs on a delegated metadata)
  • verify() is not actually usually needed by the API users -- they are only interested in threshold verification
  • Asking key to verify a signature makes sense and makes the SSLib integration more a Key class implementation detail

The only reason against this is that now sign() and verify() are in different places -- I don't think this is a major issue, and I think the solution maybe to move sign() away from Metadata as well (not sure if we can implement a MetadataSigner based on SSLib.Signer?) -- this would make some sense because signing is not needed by client and may require specific dependencies.

Storing keyid with key makes sense since when keys are used (e.g. if we have Key.verify_signature()), the id is needed. This means from_dict() now has to take an extra argument

@jku jku added the backlog Issues to address with priority for current development goals label May 26, 2021
@sechkova sechkova added this to the weeks22-23 milestone May 26, 2021
@sechkova
Copy link
Contributor

sechkova commented May 27, 2021

Just linking here a request for revising the "key" representation in sslib: secure-systems-lab/securesystemslib#310 in case the ideas in the description come in handy.

@trishankatdatadog
Copy link
Member

💯 💯

@jku
Copy link
Member Author

jku commented May 28, 2021

Just linking here a request for revising the "key" representation in sslib: secure-systems-lab/securesystemslib#310 in case the ideas in the description come in handy.

I think a potential future development is moving Key (partly) to SSLib: it seems to fit there with Signer and Signature. Also, this change will further make the SSLib integration a Key implementation detail (now no other code needs to call format_metadata_to_key()) -- this seems to further point to same direction.

I think this change should be compatible with that long term plan -- I could see possibly tuf.api.metadata.Key deriving from securesystemslib.Key and just handling the "find the correct signature and serialize the data" parts itself and then calling parents verify_signature(sig, data)

This also seems to bring the "definition of what key is" more in line with SSLib and in-toto (while the in-toto spec doesn't include the id, the implementation apparently does so this is now a closer match)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants