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

Make GPGSigner.sign return Signature #486

Merged
merged 4 commits into from
Jan 11, 2023

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Dec 15, 2022

Fixes #448
Closes #471

Description of the changes being introduced by the pull request:

GPGSigner.sign now returns Signature, which maps to a in-toto and tuf spec compliant signature object. The lower level gpg signing and verifying API stays as is, to allow existing gpg users (in-toto) to create and verify signature using the old format.

This is an acceptable API break, because GPGSigner is not yet used by in-toto or python-tuf.

To make GPGSigner useable for tuf and DSSE (#370) a spec compatible GPGKey implementation will be added in a follow-up PR.

Please verify and check that the pull request fulfils the following requirements:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh marked this pull request as ready for review December 15, 2022 09:43
Adds two private conversion helpers to translate from the old
signature format to Signature and vice-versa, i.e.
- change "signature" field name to "sig",
- list additional gpg signature field "other_headers" under
  Signature's "unrecognized_fields"

The helpers are used in GPGSigner.sign and (in a follow-up PR)
in GPGKey.verify_signature.

Also removes the no longer needed GPGSignature class.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Copy link
Collaborator

@jku jku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me.

@jku
Copy link
Collaborator

jku commented Dec 28, 2022

Although: the serialized signature format contains some extra fields when created with GPGSigner, right? It would be nice if these were obvious in the tests (this is fine to leave for future IMO)

@lukpueh
Copy link
Member Author

lukpueh commented Jan 11, 2023

@jku, let me know if the last commit roughly aligns with what you had in mind.

Assert that GPG Signature instances have the correct extra field,
and test conversion to and from legacy format.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@jku
Copy link
Collaborator

jku commented Jan 11, 2023

Yeah LGTM. It's not clear from the test what is actually in the custom data but it's now visible to reader that there's something there: I'm happy with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move GPGSigner to intoto GPGSigner (and gpg.functions.create_signature) produce invalid signature dicts
2 participants