Adds in-toto flavored GPG Key, Signer, and Signature #538
Conversation
Adds in-toto flavored legacy GPGKey, GPGSigner, and GPGSignature to be used with other Signer implementation added with the DSSE. DSSE won't support this legacy GPG api but it is useful in supporting old signature wrapper with gpg compatibilities, i.e. `Metablock` to generate in-toto Metadata and signing with GPG. This newer `GPGSigner` and `GPGKey` implementation provided by securesystemslib utilizes updated metadata formats, that is incompatible with current in-toto signature wrappers. Hence, these `_LegacyGPGSigner` and `_LegacyGPGKey` implementation is provided to continue old wrapper support without any breaking change. Signed-off-by: Pradyumna Krishna <git@onpy.in>
Adds testcase to validate the working of _LegacyGPGSigner, _LegacyGPGKey and _LegacyGPGSignature. Signed-off-by: Pradyumna Krishna <git@onpy.in>
There was a problem hiding this comment.
Thanks for the PR, @PradyumnaKrishna! Here's a first batch of comments. I'll leave more tomorrow.
in_toto/models/gpg.py
Outdated
| gpg.py | ||
|
|
||
| <Author> | ||
| Lukas Puehringer <lukas.puehringer@nyu.edu> |
There was a problem hiding this comment.
I'm flattered, but I think you should be the author. :)
in_toto/models/gpg.py
Outdated
| ) | ||
|
|
||
| @classmethod | ||
| def from_legacy_dict(cls, key_dict: Dict[str, Any]): |
There was a problem hiding this comment.
I don't think this class needs separate from_dict and from_legacy_dict
There was a problem hiding this comment.
from_dict is a derived method and pylint throws arguments-differ if I remove keyid argument from it.
There was a problem hiding this comment.
Okay, makes sense. _LegacyGPGKey.from_legacy_dict will be the pendant to SSlibKey.from_securesystemslib_key.
in_toto/models/gpg.py
Outdated
| Boolean. True if the signature is valid, False otherwise. | ||
| """ | ||
|
|
||
| return gpg.verify_signature(signature.to_dict(), self.to_dict(), data) |
There was a problem hiding this comment.
Can you take a look at verify_signature functions in securesystemslib? They should raise or return None (see e.g. secure-systems-lab/securesystemslib#488)
_LegacyGPGKey verify_signature method returns True if verification succeeds and False otherwise. This method lacks information behind the failure in verification. Hence, the method now returns None if verification passes, and raises an exception accordingly for the verification failure. * Tests updated accordingly. Signed-off-by: Pradyumna Krishna <git@onpy.in>
There was a problem hiding this comment.
This should work. Thanks, @PradyumnaKrishna!! Let's merge and see how it plays with your dsse PR.
Though, maybe we can first make the module non-public? What do you think about calling it _signer? IMO then it would even be fine to just call the classes GPGSignature, GPGSigner and GPGKey.
Please also address my comments inline and this is good to go.
in_toto/models/gpg.py
Outdated
| The executed base command is defined in | ||
| securesystemslib.gpg.constants.GPG_SIGN_COMMAND. |
There was a problem hiding this comment.
| The executed base command is defined in | |
| securesystemslib.gpg.constants.GPG_SIGN_COMMAND. |
This constant has changed. I think we can just remove the comment.
tests/models/test_gpg.py
Outdated
| with self.assertRaises(VerificationError): | ||
| key.verify_signature(signature, self.test_data) | ||
|
|
||
| def test_gpg_signer_serialization(self): |
There was a problem hiding this comment.
Minor nit: This should be
| def test_gpg_signer_serialization(self): | |
| def test_gpg_signature_serialization(self): |
Right?
models.gpg apis made non-public and moved to models._signer All ``_LegacyGPGSignature``, ``_LegacyGPGSigner``, and ``_LegacyGPGKey`` are renamed ``GPGSignature``, ``GPGSigner`` and ``GPGKey`` respectively. Tests are updated accordingly. Signed-off-by: Pradyumna Krishna <git@onpy.in>
Fixes issue #534: Add custom Key, Signer and Signature implementations for in-toto flavored GPG
Description of the changes being introduced by the pull request:
Adds in-toto flavored legacy GPGKey, GPGSigner, and GPGSignature to be used with other Signer implementation added with the DSSE.
DSSE won't support this legacy GPG api but it is useful in supporting old signature wrapper with gpg compatibilities, i.e.
Metablockto generate in-toto Metadata and signing with GPG.This newer
GPGSignerandGPGKeyimplementation provided by securesystemslib utilizes updated metadata formats, that is incompatible with current in-toto signature wrappers.Hence, these
_LegacyGPGSignerand_LegacyGPGKeyimplementation is provided to continue old wrapper support without any breaking change.Signed-off-by: Pradyumna Krishna git@onpy.in
Please verify and check that the pull request fulfills the following
requirements: