Add GPGKey Implementation #6
Conversation
tests/test_key.py
Outdated
| @@ -120,6 +126,120 @@ def test_sslib_equality(self): | |||
| self.assertEqual(sslib_key_2, sslib_key) | |||
|
|
|||
|
|
|||
| @unittest.skipIf(not HAVE_GPG, "gpg not found") | |||
| class TestGPGKey(unittest.TestCase): | |||
There was a problem hiding this comment.
Let's combine this test case with test_signer.TestGPGRSA to save some redundant test code in test_key, test_signer and test_gpg.
There was a problem hiding this comment.
you mean using GPGKey in test_signer.TestGPGRSA or something else?
There was a problem hiding this comment.
Yep, or keep your newly added test case and remove test_signer.TestGPGRSA. Either way is fine.
securesystemslib/key.py
Outdated
| if self.subkeys: | ||
| subkeys_dict = {} | ||
| for keyid, subkey in self.subkeys.items(): | ||
| subkeys_dict[keyid] = subkey.to_dict() |
There was a problem hiding this comment.
style nit: seems like a job for a dict comprehension
securesystemslib/key.py
Outdated
| if subkeys_dict: | ||
| gpg_subkeys = {} | ||
| for keyid, subkey_dict in subkeys_dict.items(): | ||
| gpg_subkeys[keyid] = GPGKey.from_dict(subkey_dict) |
There was a problem hiding this comment.
style nit: seems like a job for a dict comprehension
| @@ -95,7 +75,7 @@ def from_dict(cls, key_dict: Dict[str, Any], keyid: str) -> "SSlibKey": | |||
| keyval = key_dict.pop("keyval") | |||
|
|
|||
| # All fields left in the key_dict are unrecognized. | |||
| return cls(keyid, keytype, scheme, keyval, key_dict) | |||
| return cls(keytype, scheme, keyval, keyid, key_dict) | |||
There was a problem hiding this comment.
I thought in future keyid will get optional, so it's a better position for that.
There was a problem hiding this comment.
Okay. Would be good to note reasoning like this in the commit message. I think we still need to have more discussion about keyid and the other fields, both in the keys' in-memory representation (Key, see secure-systems-lab#310), and in the their metadata representation (see secure-systems-lab#308). I'll make some notes in the corresponding issues.
A container class of GPG public key required to verify GPG signatures in DSSE. Signed-off-by: Pradyumna Krishna <git@onpy.in>
Dataclass simplifies the SSlibKey by initilizing a constructor and other few methods like __eq__ for the SSlibKey. keyid argument order changed because of it's implementaion as optional in future. Signed-off-by: Pradyumna Krishna <git@onpy.in>
Tests all methods for a GPGKey and ensures that verification process works properly. Signed-off-by: Pradyumna Krishna <git@onpy.in>
There was a problem hiding this comment.
Great work!
Just realized that GPGSignature, GPGSigner and GPGKey don't support unrecognized_fields should we add them for consistency sake with the other implementations of Signature, Signer and Key? I suggest to not change this PR, but would you mind creating a downstream ticket.
|
Test failure is unrelated and has been fixed upstream (kudos to @PradyumnaKrishna, see secure-systems-lab#420). Merging |
Description of the changes being introduced by the pull request:
A container class representing Public Key portion of the GPG is added. it consist of a
verifymethod to verify theGPGSignatureand an arbitrary data. This GPGKey will be used to verify DSSE signatures in near future.For simplicity dataclasses are used, which initializes several methods like
__init__,__eq__etc.Please verify and check that the pull request fulfils the following requirements:
cc: @lukpueh