From 688d7a86d65321f731a4d4c60c7c66b2d77be173 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Thu, 15 Dec 2022 17:30:49 +0100 Subject: [PATCH 01/13] signer: add GPGKey and use with GPGSigner GPGKey is a regular Key with additional GnuPG specific key fields, and verification method. It also has conversion helpers to translate from and to a non-in-toto/tuf-spec compliant key format, which is still used by the underlying securesystemslib.gpg subpackage. GPGSigner is updated to: - take a GPGKey as constructor argument, and implement - `from_priv_key_uri`, to load signer from `"gnupg:[][?id=]"` - `import_`, to import a public key from a GnuPG keyring and return it along with a uri to create the signer. Signed-off-by: Lukas Puehringer --- securesystemslib/signer/__init__.py | 6 +- securesystemslib/signer/_gpg_signer.py | 282 +++++++++++++++++++++++-- tests/test_signer.py | 43 ++-- 3 files changed, 294 insertions(+), 37 deletions(-) diff --git a/securesystemslib/signer/__init__.py b/securesystemslib/signer/__init__.py index d5c816d1..37032e34 100644 --- a/securesystemslib/signer/__init__.py +++ b/securesystemslib/signer/__init__.py @@ -5,7 +5,7 @@ Some implementations are provided by default but more can be added by users. """ from securesystemslib.signer._gcp_signer import GCPSigner -from securesystemslib.signer._gpg_signer import GPGSigner +from securesystemslib.signer._gpg_signer import GPGKey, GPGSigner from securesystemslib.signer._hsm_signer import HSMSigner from securesystemslib.signer._key import KEY_FOR_TYPE_AND_SCHEME, Key, SSlibKey from securesystemslib.signer._signature import Signature @@ -23,6 +23,7 @@ SSlibSigner.FILE_URI_SCHEME: SSlibSigner, GCPSigner.SCHEME: GCPSigner, HSMSigner.SCHEME: HSMSigner, + GPGSigner.SCHEME: GPGSigner, } ) @@ -47,5 +48,8 @@ ("rsa", "rsa-pkcs1v15-sha384"): SSlibKey, ("rsa", "rsa-pkcs1v15-sha512"): SSlibKey, ("sphincs", "sphincs-shake-128s"): SSlibKey, + ("rsa", "pgp+rsa-pkcsv1.5"): GPGKey, + ("dsa", "pgp+dsa-fips-180-2"): GPGKey, + ("eddsa", "pgp+eddsa-ed25519"): GPGKey, } ) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index 6d48b0ee..0b2c512d 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -1,20 +1,219 @@ """Signer implementation for OpenPGP """ -from typing import Dict, Optional -import securesystemslib.gpg.functions as gpg +import logging +from typing import Any, Dict, List, Optional, Tuple +from urllib import parse + +from securesystemslib import exceptions +from securesystemslib.gpg import exceptions as gpg_exceptions +from securesystemslib.gpg import functions as gpg from securesystemslib.signer._key import Key from securesystemslib.signer._signer import SecretsHandler, Signature, Signer +logger = logging.getLogger(__name__) + + +class GPGKey(Key): + """OpenPGP Key. + + *All parameters named below are not just constructor arguments but also + instance attributes.* + + Attributes: + keyid: Key identifier that is unique within the metadata it is used in. + ketytype: Key type, e.g. "rsa", "dsa" or "eddsa". + scheme: Signing schemes, e.g. "pgp+rsa-pkcsv1.5", "pgp+dsa-fips-180-2", + "pgp+eddsa-ed25519". + hashes: Hash algorithm to hash the data to be signed, e.g. "pgp+SHA2". + keyval: Opaque key content. + creation_time: Unix timestamp when key was created. + validity_period: Validity of key in days. + subkeys: A dictionary of keyids as keys and GPGKeys as values. + unrecognized_fields: Dictionary of all attributes that are not managed + by Securesystemslib + """ + + def __init__( + self, + keyid: str, + keytype: str, + scheme: str, + hashes: List[str], + keyval: Dict[str, Any], + creation_time: Optional[int] = None, + validity_period: Optional[int] = None, + subkeys: Optional[Dict[str, "GPGKey"]] = None, + unrecognized_fields: Optional[Dict[str, Any]] = None, + ): + + super().__init__(keyid, keytype, scheme, keyval, unrecognized_fields) + + self.hashes = hashes + self.creation_time = creation_time + self.validity_period = validity_period + self.subkeys = subkeys + + def __eq__(self, other: Any) -> bool: + if not isinstance(other, GPGKey): + return False + + return ( + super().__eq__(other) + and self.hashes == other.hashes + and self.creation_time == other.creation_time + and self.validity_period == other.validity_period + and self.subkeys == other.subkeys + ) + + @classmethod + def __from_dict( + cls, + keyid: str, + keytype: str, + scheme: str, + subkeys: Optional[Dict[str, "GPGKey"]], + key_dict: Dict[str, Any], + ) -> "GPGKey": + """Helper for common from_*dict operations.""" + + hashes = key_dict.pop("hashes") + keyval = key_dict.pop("keyval") + creation_time = key_dict.pop("creation_time", None) + validity_period = key_dict.pop("validity_period", None) + + return cls( + keyid, + keytype, + scheme, + hashes, + keyval, + creation_time, + validity_period, + subkeys, + key_dict, + ) + + @classmethod + def _from_legacy_dict(cls, key_dict: Dict[str, Any]) -> "GPGKey": + """Create GPGKey from legacy dictionary representation.""" + + keyid = key_dict.pop("keyid") + keytype = key_dict.pop("type") + scheme = key_dict.pop("method") + subkeys = key_dict.pop("subkeys", None) + + if subkeys is not None: + subkeys = { + keyid: cls._from_legacy_dict( + key + ) # pylint: disable=protected-access + for (keyid, key) in subkeys.items() + } + + return cls.__from_dict(keyid, keytype, scheme, subkeys, key_dict) + + @classmethod + def from_dict(cls, keyid: str, key_dict: Dict[str, Any]) -> "GPGKey": + keytype = key_dict.pop("keytype") + scheme = key_dict.pop("scheme") + subkeys = key_dict.pop("subkeys", None) + + if subkeys: + subkeys = { + keyid: cls.from_dict(keyid, key) + for (keyid, key) in subkeys.items() + } + + return cls.__from_dict(keyid, keytype, scheme, subkeys, key_dict) + + def __to_dict(self) -> Dict[str, Any]: + """Helper for common to_*dict operations.""" + + key_dict: Dict[str, Any] = { + "hashes": self.hashes, + "keyval": self.keyval, + } + if self.creation_time is not None: + key_dict["creation_time"] = self.creation_time + + if self.validity_period is not None: + key_dict["validity_period"] = self.validity_period + + return key_dict + + def _to_legacy_dict(self) -> Dict[str, Any]: + """Returns legacy dictionary representation of self.""" + + key_dict = self.__to_dict() + key_dict.update( + { + "keyid": self.keyid, + "type": self.keytype, + "method": self.scheme, + } + ) + + if self.subkeys: + key_dict["subkeys"] = { + keyid: key._to_legacy_dict() # pylint: disable=protected-access + for (keyid, key) in self.subkeys.items() + } + + return key_dict + + def to_dict(self) -> Dict[str, Any]: + key_dict = self.__to_dict() + key_dict.update( + { + "keytype": self.keytype, + "scheme": self.scheme, + } + ) + + if self.subkeys: + key_dict["subkeys"] = { + keyid: key.to_dict() for (keyid, key) in self.subkeys.items() + } + + return key_dict + + def verify_signature(self, signature: Signature, data: bytes) -> None: + try: + if not gpg.verify_signature( + GPGSigner._sig_to_legacy_dict( # pylint: disable=protected-access + signature + ), + self._to_legacy_dict(), + data, + ): + raise exceptions.UnverifiedSignatureError( + f"Failed to verify signature by {self.keyid}" + ) + except ( + exceptions.FormatError, + gpg_exceptions.KeyExpirationError, + ) as e: + logger.info("Key %s failed to verify sig: %s", self.keyid, str(e)) + raise exceptions.VerificationError( + f"Unknown failure to verify signature by {self.keyid}" + ) from e + class GPGSigner(Signer): """OpenPGP Signer - Runs command in ``GNUPG`` environment variable to sign, fallback commands are + Runs command in ``GNUPG`` environment variable to sign. Fallback commands are ``gpg2`` and ``gpg``. Supported signing schemes are: "pgp+rsa-pkcsv1.5", "pgp+dsa-fips-180-2" and "pgp+eddsa-ed25519", with SHA-256 hashing. + GPGSigner can be instantiated with Signer.from_priv_key_uri(). These private key URI + schemes are supported: + + * "gnupg:[][?id=]": + Signs with GnuPG key identified by keyid, in the keyring in home dir. If + homedir is not passed, the default homedir is used. Arguments: keyid: GnuPG local user signing key id. If not passed, the default key is used. @@ -22,11 +221,17 @@ class GPGSigner(Signer): """ + SCHEME = "gnupg" + def __init__( - self, keyid: Optional[str] = None, homedir: Optional[str] = None + self, + public_key: Key, + keyid: Optional[str] = None, + homedir: Optional[str] = None, ): self.keyid = keyid self.homedir = homedir + self.public_key = public_key @classmethod def from_priv_key_uri( @@ -35,41 +240,78 @@ def from_priv_key_uri( public_key: Key, secrets_handler: Optional[SecretsHandler] = None, ) -> "GPGSigner": - raise NotImplementedError("Incompatible with private key URIs") + if not isinstance(public_key, GPGKey): + raise ValueError(f"expected GPGKey for {priv_key_uri}") + + uri = parse.urlparse(priv_key_uri) + + if uri.scheme != cls.SCHEME: + raise ValueError(f"GPGSigner does not support {priv_key_uri}") + + if secrets_handler is not None: + raise ValueError("GPGSigner does not support a secrets handler") + + params = dict(parse.parse_qsl(uri.query)) + keyid = params.get("key") + homedir = uri.path or None + + return cls(public_key, keyid, homedir) @staticmethod - def _to_gpg_sig(sig: Signature) -> Dict: - """Helper to convert Signature -> internal gpg signature format.""" + def _sig_to_legacy_dict(sig: Signature) -> Dict: + """Helper to convert Signature to internal gpg signature dict format.""" sig_dict = sig.to_dict() sig_dict["signature"] = sig_dict.pop("sig") return sig_dict @staticmethod - def _from_gpg_sig(sig_dict: Dict) -> Signature: - """Helper to convert internal gpg signature format -> Signature.""" + def _sig_from_legacy_dict(sig_dict: Dict) -> Signature: + """Helper to convert internal gpg signature format to Signature.""" sig_dict["sig"] = sig_dict.pop("signature") return Signature.from_dict(sig_dict) + @classmethod + def import_(cls, keyid: str, homedir=None) -> Tuple[str, Key]: + """Load key and signer details from GnuPG keyring + + Args: + keyid: GnuPG local user signing key id. + homedir: GnuPG home directory path. If not passed, the default homedir is + used. + + Returns: + Tuple of private key uri and the public key. + + """ + uri = f"{cls.SCHEME}:{homedir or ''}{'?key=' + keyid}" + + public_key = ( + GPGKey._from_legacy_dict( # pylint: disable=protected-access + gpg.export_pubkey(keyid, homedir) + ) + ) + + return (uri, public_key) + def sign(self, payload: bytes) -> Signature: - """Signs payload with ``gpg``. + """Signs payload with GnuPG. Arguments: payload: bytes to be signed. Raises: - ValueError: The gpg command failed to create a valid signature. - OSError: the gpg command is not present or non-executable. - securesystemslib.exceptions.UnsupportedLibraryError: The gpg - command is not available, or the cryptography library is - not installed. - securesystemslib.gpg.exceptions.CommandError: The gpg command - returned a non-zero exit code. - securesystemslib.gpg.exceptions.KeyNotFoundError: The used gpg - version is not fully supported. + ValueError: gpg command failed to create a valid signature. + OSError: gpg command is not present or non-executable. + securesystemslib.exceptions.UnsupportedLibraryError: gpg command is not + available, or the cryptography library is not installed. + securesystemslib.gpg.exceptions.CommandError: gpg command returned a + non-zero exit code. + securesystemslib.gpg.exceptions.KeyNotFoundError: gpg version is not fully + supported. Returns: Signature. """ - return self._from_gpg_sig( + return self._sig_from_legacy_dict( gpg.create_signature(payload, self.keyid, self.homedir) ) diff --git a/tests/test_signer.py b/tests/test_signer.py index db114fd0..4767ad8b 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -17,11 +17,10 @@ UnverifiedSignatureError, ) from securesystemslib.gpg.constants import have_gpg -from securesystemslib.gpg.functions import export_pubkey -from securesystemslib.gpg.functions import verify_signature as verify_sig from securesystemslib.signer import ( KEY_FOR_TYPE_AND_SCHEME, SIGNER_FOR_URI_SCHEME, + GPGKey, GPGSigner, Key, SecretsHandler, @@ -38,6 +37,9 @@ class TestKey(unittest.TestCase): def test_key_from_to_dict(self): """Test to/from_dict for known keytype/scheme combos""" for (keytype, scheme), key_impl in KEY_FOR_TYPE_AND_SCHEME.items(): + if key_impl in [GPGKey]: # these keys require additional fields + continue + keydict = { "keytype": keytype, "scheme": scheme, @@ -401,27 +403,36 @@ def tearDownClass(cls): def test_gpg_sign_and_verify_object_with_default_key(self): """Create a signature using the default key on the keyring.""" - # pylint: disable=protected-access - signer = GPGSigner(homedir=self.gnupg_home) - signature = signer.sign(self.test_data) - signature_dict = GPGSigner._to_gpg_sig(signature) - key_data = export_pubkey(self.default_keyid, self.gnupg_home) + # Public key import requires a keyid, signer loading does not. Ignore the URI w/ + # keyid returned here, and construct one w/o keyid below, to test default key. + _, public_key = GPGSigner.import_( + self.signing_subkey_keyid, self.gnupg_home + ) + + uri = f"gnupg:{self.gnupg_home}" + signer = Signer.from_priv_key_uri(uri, public_key) + sig = signer.sign(self.test_data) + + public_key.verify_signature(sig, self.test_data) - self.assertTrue(verify_sig(signature_dict, key_data, self.test_data)) - self.assertFalse(verify_sig(signature_dict, key_data, self.wrong_data)) + with self.assertRaises(UnverifiedSignatureError): + self.assertFalse(public_key.verify_signature(sig, self.wrong_data)) def test_gpg_sign_and_verify_object(self): """Create a signature using a specific key on the keyring.""" - # pylint: disable=protected-access - signer = GPGSigner(self.signing_subkey_keyid, self.gnupg_home) - signature = signer.sign(self.test_data) - signature_dict = GPGSigner._to_gpg_sig(signature) - key_data = export_pubkey(self.signing_subkey_keyid, self.gnupg_home) + uri, public_key = GPGSigner.import_( + self.signing_subkey_keyid, self.gnupg_home + ) - self.assertTrue(verify_sig(signature_dict, key_data, self.test_data)) - self.assertFalse(verify_sig(signature_dict, key_data, self.wrong_data)) + signer = Signer.from_priv_key_uri(uri, public_key) + sig = signer.sign(self.test_data) + + public_key.verify_signature(sig, self.test_data) + + with self.assertRaises(UnverifiedSignatureError): + self.assertFalse(public_key.verify_signature(sig, self.wrong_data)) def test_gpg_signature_data_structure(self): """Test custom fields and legacy data structure in gpg signatures.""" From 71b8e27d89007290233c3340c0fea332fc1e0837 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Wed, 18 Jan 2023 18:29:58 +0100 Subject: [PATCH 02/13] signer: fix GPGSigner test after rebase Signed-off-by: Lukas Puehringer --- tests/test_signer.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_signer.py b/tests/test_signer.py index 4767ad8b..79b6528e 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -437,14 +437,17 @@ def test_gpg_sign_and_verify_object(self): def test_gpg_signature_data_structure(self): """Test custom fields and legacy data structure in gpg signatures.""" # pylint: disable=protected-access - signer = GPGSigner(homedir=self.gnupg_home) + _, public_key = GPGSigner.import_( + self.signing_subkey_keyid, self.gnupg_home + ) + signer = GPGSigner(public_key, homedir=self.gnupg_home) sig = signer.sign(self.test_data) self.assertIn("other_headers", sig.unrecognized_fields) - sig_dict = GPGSigner._to_gpg_sig(sig) + sig_dict = GPGSigner._sig_to_legacy_dict(sig) self.assertIn("signature", sig_dict) self.assertNotIn("sig", sig_dict) - sig2 = GPGSigner._from_gpg_sig(sig_dict) + sig2 = GPGSigner._sig_from_legacy_dict(sig_dict) self.assertEqual(sig, sig2) From 6f3ed0d6b3b1d65ce107ac1cb039fc7ae0f75d80 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Wed, 18 Jan 2023 18:30:50 +0100 Subject: [PATCH 03/13] signer: address GPGSigner review comments - add expected exception to verify method - warn on passed secrets handler, don't raise Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index 0b2c512d..8ba258eb 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -191,6 +191,7 @@ def verify_signature(self, signature: Signature, data: bytes) -> None: ) except ( exceptions.FormatError, + exceptions.UnsupportedLibraryError, gpg_exceptions.KeyExpirationError, ) as e: logger.info("Key %s failed to verify sig: %s", self.keyid, str(e)) @@ -249,7 +250,7 @@ def from_priv_key_uri( raise ValueError(f"GPGSigner does not support {priv_key_uri}") if secrets_handler is not None: - raise ValueError("GPGSigner does not support a secrets handler") + logger.warning("GPGSigner does not support a secrets handler") params = dict(parse.parse_qsl(uri.query)) keyid = params.get("key") From 831811d256f989b707bb0323f66dea12d2a4bb75 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Thu, 19 Jan 2023 09:34:41 +0100 Subject: [PATCH 04/13] signer: remove unused secrets handler warning Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index 8ba258eb..2efa103e 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -249,9 +249,6 @@ def from_priv_key_uri( if uri.scheme != cls.SCHEME: raise ValueError(f"GPGSigner does not support {priv_key_uri}") - if secrets_handler is not None: - logger.warning("GPGSigner does not support a secrets handler") - params = dict(parse.parse_qsl(uri.query)) keyid = params.get("key") homedir = uri.path or None From 61ddeb7b5ca0874422e36fe50ba024c7d7db356e Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Thu, 19 Jan 2023 16:23:17 +0100 Subject: [PATCH 05/13] signer: remove keyid attribute from GPGSigner The keyid is redundant with the keyid of the attached public key instance. Same goes for the keyid parameter in the related private key uri, which can also be read from the public key instance passed to the from_priv_key_uri method. Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index 2efa103e..fc6730e0 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -21,6 +21,7 @@ class GPGKey(Key): Attributes: keyid: Key identifier that is unique within the metadata it is used in. + It is also used to identify the GnuPG local user signing key. ketytype: Key type, e.g. "rsa", "dsa" or "eddsa". scheme: Signing schemes, e.g. "pgp+rsa-pkcsv1.5", "pgp+dsa-fips-180-2", "pgp+eddsa-ed25519". @@ -212,12 +213,13 @@ class GPGSigner(Signer): GPGSigner can be instantiated with Signer.from_priv_key_uri(). These private key URI schemes are supported: - * "gnupg:[][?id=]": - Signs with GnuPG key identified by keyid, in the keyring in home dir. If - homedir is not passed, the default homedir is used. + * "gnupg:[]": + Signs with GnuPG key in keyring in home dir. The signing key is + identified with the keyid of the passed public key. If homedir is not + passed, the default homedir is used. Arguments: - keyid: GnuPG local user signing key id. If not passed, the default key is used. + public_key: The related public key instance. homedir: GnuPG home directory path. If not passed, the default homedir is used. """ @@ -227,10 +229,8 @@ class GPGSigner(Signer): def __init__( self, public_key: Key, - keyid: Optional[str] = None, homedir: Optional[str] = None, ): - self.keyid = keyid self.homedir = homedir self.public_key = public_key @@ -249,11 +249,9 @@ def from_priv_key_uri( if uri.scheme != cls.SCHEME: raise ValueError(f"GPGSigner does not support {priv_key_uri}") - params = dict(parse.parse_qsl(uri.query)) - keyid = params.get("key") homedir = uri.path or None - return cls(public_key, keyid, homedir) + return cls(public_key, homedir) @staticmethod def _sig_to_legacy_dict(sig: Signature) -> Dict: @@ -281,7 +279,7 @@ def import_(cls, keyid: str, homedir=None) -> Tuple[str, Key]: Tuple of private key uri and the public key. """ - uri = f"{cls.SCHEME}:{homedir or ''}{'?key=' + keyid}" + uri = f"{cls.SCHEME}:{homedir or ''}" public_key = ( GPGKey._from_legacy_dict( # pylint: disable=protected-access @@ -311,5 +309,5 @@ def sign(self, payload: bytes) -> Signature: Signature. """ return self._sig_from_legacy_dict( - gpg.create_signature(payload, self.keyid, self.homedir) + gpg.create_signature(payload, self.public_key.keyid, self.homedir) ) From 68aff11ef6c94065ce64a1d0dde79427b1347d9d Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Thu, 19 Jan 2023 16:30:18 +0100 Subject: [PATCH 06/13] signer: fix gpg key serialization bug Include unrecognized fields in GPGKey.to_dict. Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index fc6730e0..fb07a3bb 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -168,6 +168,7 @@ def to_dict(self) -> Dict[str, Any]: { "keytype": self.keytype, "scheme": self.scheme, + **self.unrecognized_fields, } ) From e36b46c25bbaf4868f8d874545867d3910aed1e0 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Thu, 19 Jan 2023 16:31:50 +0100 Subject: [PATCH 07/13] signer: add missing type hint in gpg signer method Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index fb07a3bb..46d5c004 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -268,7 +268,9 @@ def _sig_from_legacy_dict(sig_dict: Dict) -> Signature: return Signature.from_dict(sig_dict) @classmethod - def import_(cls, keyid: str, homedir=None) -> Tuple[str, Key]: + def import_( + cls, keyid: str, homedir: Optional[str] = None + ) -> Tuple[str, Key]: """Load key and signer details from GnuPG keyring Args: From 7528f35874528be821bc41088889f1924bb52398 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Thu, 19 Jan 2023 16:46:50 +0100 Subject: [PATCH 08/13] signer: improve and add tests for gpg signer New tests for: - `sign` and `import_` failure but successful verification, if 'gpg' is not available. - verification failure, if 'cryptography' is not available. - key de/serialization (also legacy format) and comparison (__eq__) - expected failures on `from_priv_key` and `verify_signature` Improvements: - remove obsolete `assertFalse` in `with self.assertRaises` block - condense tests in `test_gpg_functions` (use DDT instead of copy-paste) Signed-off-by: Lukas Puehringer --- tests/check_public_interfaces.py | 15 +++++ tests/check_public_interfaces_gpg.py | 38 ++++++++---- tests/test_signer.py | 90 ++++++++++++++++++++++++++-- 3 files changed, 127 insertions(+), 16 deletions(-) diff --git a/tests/check_public_interfaces.py b/tests/check_public_interfaces.py index c35b2d24..ab4ccf34 100644 --- a/tests/check_public_interfaces.py +++ b/tests/check_public_interfaces.py @@ -41,6 +41,11 @@ import securesystemslib.gpg.util # pylint: disable=wrong-import-position import securesystemslib.interface # pylint: disable=wrong-import-position import securesystemslib.keys # pylint: disable=wrong-import-position +from securesystemslib.exceptions import ( + UnsupportedLibraryError, + VerificationError, +) +from securesystemslib.signer import GPGKey, Signature class TestPublicInterfaces( @@ -314,6 +319,16 @@ def test_gpg_functions(self): securesystemslib.gpg.functions.export_pubkey("f00") self.assertEqual(expected_error_msg, str(ctx.exception)) + def test_signer(self): + """Assert generic VerificationError from UnsupportedLibraryError.""" + key = GPGKey( + "aa", "rsa", "pgp+rsa-pkcsv1.5", ["pgp+SHA2"], {"public": "val"} + ) + sig = Signature("aa", "aaaaaaa", {"other_headers": "aaaaaa"}) + with self.assertRaises(VerificationError) as ctx: + key.verify_signature(sig, b"data") + self.assertIsInstance(ctx.exception.__cause__, UnsupportedLibraryError) + if __name__ == "__main__": unittest.main(verbosity=1, buffer=True) diff --git a/tests/check_public_interfaces_gpg.py b/tests/check_public_interfaces_gpg.py index 3ed236e9..4ce2c9f5 100644 --- a/tests/check_public_interfaces_gpg.py +++ b/tests/check_public_interfaces_gpg.py @@ -35,6 +35,8 @@ verify_signature, ) +from securesystemslib.signer import GPGKey, GPGSigner, Signer + class TestPublicInterfacesGPG( unittest.TestCase @@ -47,17 +49,29 @@ def setUpClass(cls): def test_gpg_functions(self): """Signing, key export and util functions must raise on missing gpg.""" - with self.assertRaises(UnsupportedLibraryError) as ctx: - create_signature("bar") - self.assertEqual(NO_GPG_MSG, str(ctx.exception)) - - with self.assertRaises(UnsupportedLibraryError) as ctx: - export_pubkey("f00") - self.assertEqual(NO_GPG_MSG, str(ctx.exception)) - with self.assertRaises(UnsupportedLibraryError) as ctx: - export_pubkeys(["f00"]) - self.assertEqual(NO_GPG_MSG, str(ctx.exception)) + # Hand-crafting a GPG public key and loading a signer works w/o gpg, but + # signing fails (see below). + mock_public_key = GPGKey( + "aa", + "rsa", + "pgp+rsa-pkcsv1.5", + ["pgp+SHA2"], + {"public": {"key": "value"}}, + ) + signer = Signer.from_priv_key_uri("gnupg:?id=abcd", mock_public_key) + + # Run commands that require gpg and assert error plus message + for fn, args in ( + (create_signature, ("bar",)), + (export_pubkey, ("f00",)), + (export_pubkeys, (["f00"],)), + (GPGSigner.import_, ("keyid",)), + (signer.sign, (b"data",)), + ): + with self.assertRaises(UnsupportedLibraryError) as ctx: + fn(*args) + self.assertEqual(NO_GPG_MSG, str(ctx.exception)) def test_gpg_verify(self): """Signature verification does not require gpg to be installed on the host. @@ -139,6 +153,10 @@ def test_gpg_verify(self): for key, sig in key_signature_pairs: self.assertTrue(verify_signature(sig, key, data)) + # pylint: disable=protected-access + GPGKey._from_legacy_dict(key).verify_signature( + GPGSigner._sig_from_legacy_dict(sig), data + ) if __name__ == "__main__": diff --git a/tests/test_signer.py b/tests/test_signer.py index 79b6528e..f121f9a8 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -15,8 +15,10 @@ FormatError, UnsupportedAlgorithmError, UnverifiedSignatureError, + VerificationError, ) from securesystemslib.gpg.constants import have_gpg +from securesystemslib.gpg.exceptions import KeyExpirationError from securesystemslib.signer import ( KEY_FOR_TYPE_AND_SCHEME, SIGNER_FOR_URI_SCHEME, @@ -37,13 +39,11 @@ class TestKey(unittest.TestCase): def test_key_from_to_dict(self): """Test to/from_dict for known keytype/scheme combos""" for (keytype, scheme), key_impl in KEY_FOR_TYPE_AND_SCHEME.items(): - if key_impl in [GPGKey]: # these keys require additional fields - continue - keydict = { "keytype": keytype, "scheme": scheme, "extra": "somedata", + "hashes": ["only recognized by GPGKey"], "keyval": { "public": "pubkeyval", "foo": "bar", @@ -417,7 +417,7 @@ def test_gpg_sign_and_verify_object_with_default_key(self): public_key.verify_signature(sig, self.test_data) with self.assertRaises(UnverifiedSignatureError): - self.assertFalse(public_key.verify_signature(sig, self.wrong_data)) + public_key.verify_signature(sig, self.wrong_data) def test_gpg_sign_and_verify_object(self): """Create a signature using a specific key on the keyring.""" @@ -432,9 +432,30 @@ def test_gpg_sign_and_verify_object(self): public_key.verify_signature(sig, self.test_data) with self.assertRaises(UnverifiedSignatureError): - self.assertFalse(public_key.verify_signature(sig, self.wrong_data)) + public_key.verify_signature(sig, self.wrong_data) + + public_key.subkeys[sig.keyid].creation_time = 1 + public_key.subkeys[sig.keyid].validity_period = 1 + with self.assertRaises(VerificationError) as ctx: + public_key.verify_signature(sig, self.test_data) + + self.assertIsInstance(ctx.exception.__cause__, KeyExpirationError) + + def test_gpg_signer_load_with_bad_scheme(self): + """Load from priv key uri with wrong uri scheme.""" + key = GPGKey( + "aa", "rsa", "pgp+rsa-pkcsv1.5", ["pgp+SHA2"], {"public": "val"} + ) + with self.assertRaises(ValueError): + GPGSigner.from_priv_key_uri("wrong:", key) + + def test_gpg_signer_load_with_bad_key(self): + """Load from priv key uri with wrong pubkey type.""" + key = SSlibKey("aa", "rsa", "rsassa-pss-sha256", {"public": "val"}) + with self.assertRaises(ValueError): + GPGSigner.from_priv_key_uri("gnupg:", key) - def test_gpg_signature_data_structure(self): + def test_gpg_signature_legacy_data_structure(self): """Test custom fields and legacy data structure in gpg signatures.""" # pylint: disable=protected-access _, public_key = GPGSigner.import_( @@ -450,6 +471,63 @@ def test_gpg_signature_data_structure(self): sig2 = GPGSigner._sig_from_legacy_dict(sig_dict) self.assertEqual(sig, sig2) + def test_gpg_key_legacy_data_structure(self): + """Test legacy data structure conversion in gpg keys.""" + # pylint: disable=protected-access + _, public_key = GPGSigner.import_( + self.signing_subkey_keyid, self.gnupg_home + ) + legacy_fields = {"keyid", "type", "method"} + fields = {"keytype", "scheme"} + + legacy_dict = public_key._to_legacy_dict() + for key in [legacy_dict] + list(legacy_dict["subkeys"].values()): + for field in legacy_fields: + self.assertIn(field, key) + for field in fields: + self.assertNotIn(field, key) + + self.assertEqual(public_key, GPGKey._from_legacy_dict(legacy_dict)) + + def test_gpg_key_optional_fields(self): + """Test gpg public key from/to_dict with optional fields.""" + + keydict_base = { + "keytype": "rsa", + "scheme": "pgp+rsa-pkcsv1.5", + "hashes": ["pgp+SHA2"], + "keyval": { + "public": "pubkeyval", + }, + } + + for name, val in ( + ("creation_time", 1), + ("validity_period", 1), + ("subkeys", {"bb": copy.deepcopy(keydict_base)}), + ): + keydict = copy.deepcopy(keydict_base) + keydict[name] = val + key = Key.from_dict("aa", copy.deepcopy(keydict)) + self.assertIsNotNone(getattr(key, name)) + self.assertDictEqual(keydict, key.to_dict(), name) + + def test_gpg_key__eq__(self): + """Test GPGKey.__eq__() .""" + key1 = GPGKey( + "aa", "rsa", "pgp+rsa-pkcsv1.5", ["pgp+SHA2"], {"public": "val"} + ) + key2 = copy.deepcopy(key1) + self.assertEqual(key1, key2) + + key2.keyid = "bb" + self.assertNotEqual(key1, key2) + + other_key = SSlibKey( + "aa", "rsa", "rsassa-pss-sha256", {"public": "val"} + ) + self.assertNotEqual(key1, other_key) + # Run the unit tests. if __name__ == "__main__": From 0deab057f7cb467be6f70c85da1398b437a07b5f Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 6 Mar 2023 14:32:26 +0100 Subject: [PATCH 09/13] signer: assert keyid match GPGSigner import_, sign Supporting subkeys in a GPGKey and considering them for verification adds, for no benefit, a PKI hierarchy to tuf/in-toto, which already have their own PKI hierarchies. In addition it makes the verification (and delegation) code more complex and error prone. This commit drops subkey support, by the following two changes: - import_: require exact match between passed keyid, and one of the keys in the bundle returned by gpg, and return a GPGKey only for that key, w/o subkeys - sign: require exact match between keyid on attached public key and keyid on the signature returned by gpg Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 38 +++++++++++++++++---- tests/test_signer.py | 47 +++++++++++++++++++------- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index 46d5c004..aa93384c 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -46,7 +46,6 @@ def __init__( subkeys: Optional[Dict[str, "GPGKey"]] = None, unrecognized_fields: Optional[Dict[str, Any]] = None, ): - super().__init__(keyid, keytype, scheme, keyval, unrecognized_fields) self.hashes = hashes @@ -278,17 +277,35 @@ def import_( homedir: GnuPG home directory path. If not passed, the default homedir is used. + Raises: + UnsupportedLibraryError: The gpg command or pyca/cryptography are + not available. + KeyNotFoundError: No key was found for the passed keyid. + Returns: Tuple of private key uri and the public key. """ uri = f"{cls.SCHEME}:{homedir or ''}" - public_key = ( - GPGKey._from_legacy_dict( # pylint: disable=protected-access - gpg.export_pubkey(keyid, homedir) + raw_key = gpg.export_pubkey(keyid, homedir) + raw_keys = [raw_key] + list(raw_key.pop("subkeys", {}).values()) + keyids = [] + + for key in raw_keys: + if key["keyid"] == keyid: + # TODO: Raise here if key is expired, revoked, incapable, ... + public_key = GPGKey._from_legacy_dict( # pylint: disable=protected-access + key + ) + break + keyids.append(key["keyid"]) + + else: + raise gpg_exceptions.KeyNotFoundError( + f"No exact match found for passed keyid" + f" {keyid}, found: {keyids}." ) - ) return (uri, public_key) @@ -311,6 +328,13 @@ def sign(self, payload: bytes) -> Signature: Returns: Signature. """ - return self._sig_from_legacy_dict( - gpg.create_signature(payload, self.public_key.keyid, self.homedir) + raw_sig = gpg.create_signature( + payload, self.public_key.keyid, self.homedir ) + if raw_sig["keyid"] != self.public_key.keyid: + raise ValueError( + f"The signing key {raw_sig['keyid']} does not" + f" match the attached public key {self.public_key.keyid}." + ) + + return self._sig_from_legacy_dict(raw_sig) diff --git a/tests/test_signer.py b/tests/test_signer.py index f121f9a8..c38e76f0 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -18,7 +18,7 @@ VerificationError, ) from securesystemslib.gpg.constants import have_gpg -from securesystemslib.gpg.exceptions import KeyExpirationError +from securesystemslib.gpg.exceptions import CommandError, KeyNotFoundError from securesystemslib.signer import ( KEY_FOR_TYPE_AND_SCHEME, SIGNER_FOR_URI_SCHEME, @@ -374,8 +374,8 @@ class TestGPGRSA(unittest.TestCase): @classmethod def setUpClass(cls): - cls.default_keyid = "8465A1E2E0FB2B40ADB2478E18FB3F537E0C8A17" - cls.signing_subkey_keyid = "C5A0ABE6EC19D0D65F85E2C39BE9DF5131D924E9" + cls.default_keyid = "8465a1e2e0fb2b40adb2478e18fb3f537e0c8a17" + cls.signing_subkey_keyid = "c5a0abe6ec19d0d65f85e2c39be9df5131d924e9" # Create directory to run the tests without having everything blow up. cls.working_dir = os.getcwd() @@ -434,12 +434,35 @@ def test_gpg_sign_and_verify_object(self): with self.assertRaises(UnverifiedSignatureError): public_key.verify_signature(sig, self.wrong_data) - public_key.subkeys[sig.keyid].creation_time = 1 - public_key.subkeys[sig.keyid].validity_period = 1 - with self.assertRaises(VerificationError) as ctx: + sig.keyid = 123456 + with self.assertRaises(VerificationError): public_key.verify_signature(sig, self.test_data) - self.assertIsInstance(ctx.exception.__cause__, KeyExpirationError) + def test_gpg_fail_sign_keyid_match(self): + """Fail signing because signature keyid does not match public key.""" + uri, public_key = GPGSigner.import_(self.default_keyid, self.gnupg_home) + signer = Signer.from_priv_key_uri(uri, public_key) + + # Fail because we imported main key, but gpg favors signing subkey + with self.assertRaises(ValueError): + signer.sign(self.test_data) + + def test_gpg_fail_import_keyid_match(self): + """Fail key import because passed keyid does not match returned key.""" + + # gpg exports the right key, but we require an exact keyid match + non_matching_keyid = self.default_keyid.upper() + with self.assertRaises(KeyNotFoundError): + GPGSigner.import_(non_matching_keyid, self.gnupg_home) + + def test_gpg_fail_sign_expired_key(self): + """Signing fails with non-zero exit code if key is expired.""" + expired_key = "e8ac80c924116dabb51d4b987cb07d6d2c199c7c" + + uri, public_key = GPGSigner.import_(expired_key, self.gnupg_home) + signer = Signer.from_priv_key_uri(uri, public_key) + with self.assertRaises(CommandError): + signer.sign(self.test_data) def test_gpg_signer_load_with_bad_scheme(self): """Load from priv key uri with wrong uri scheme.""" @@ -481,11 +504,11 @@ def test_gpg_key_legacy_data_structure(self): fields = {"keytype", "scheme"} legacy_dict = public_key._to_legacy_dict() - for key in [legacy_dict] + list(legacy_dict["subkeys"].values()): - for field in legacy_fields: - self.assertIn(field, key) - for field in fields: - self.assertNotIn(field, key) + for field in legacy_fields: + self.assertIn(field, legacy_dict) + + for field in fields: + self.assertNotIn(field, legacy_dict) self.assertEqual(public_key, GPGKey._from_legacy_dict(legacy_dict)) From 6244616abb0b20aa43c3b5ca626fdab787dee994 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 6 Mar 2023 16:47:42 +0100 Subject: [PATCH 10/13] signer: remove redundant gpg default key test GPGSigner does not support signing with a default key (unlike the lower level securesystemslib gpg signing function), thus the test is redundant with the non-default key test. Signed-off-by: Lukas Puehringer --- tests/test_signer.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/test_signer.py b/tests/test_signer.py index c38e76f0..78ab8f35 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -401,24 +401,6 @@ def tearDownClass(cls): os.chdir(cls.working_dir) shutil.rmtree(cls.test_dir) - def test_gpg_sign_and_verify_object_with_default_key(self): - """Create a signature using the default key on the keyring.""" - - # Public key import requires a keyid, signer loading does not. Ignore the URI w/ - # keyid returned here, and construct one w/o keyid below, to test default key. - _, public_key = GPGSigner.import_( - self.signing_subkey_keyid, self.gnupg_home - ) - - uri = f"gnupg:{self.gnupg_home}" - signer = Signer.from_priv_key_uri(uri, public_key) - sig = signer.sign(self.test_data) - - public_key.verify_signature(sig, self.test_data) - - with self.assertRaises(UnverifiedSignatureError): - public_key.verify_signature(sig, self.wrong_data) - def test_gpg_sign_and_verify_object(self): """Create a signature using a specific key on the keyring.""" From 0527b7c4fa000a05d686744603c7e78409c455e4 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 6 Mar 2023 14:50:09 +0100 Subject: [PATCH 11/13] signer: remove obsolete fields from GPGKey - creation_time + validity_period: key validity should be determined by the metadata expiration time alone, and not by an additional signer-specific key expiration, which is prone to be out of sync (we don't use gpg for verification) - subkeys: dropped support in a previous commit - hashes: static for currently supported schemes. If other hash algorithms are needed, they should be encoded it in the scheme string. Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 166 +++++-------------------- tests/check_public_interfaces.py | 4 +- tests/check_public_interfaces_gpg.py | 1 - tests/test_signer.py | 31 +---- 4 files changed, 32 insertions(+), 170 deletions(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index aa93384c..fe847529 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -1,10 +1,10 @@ """Signer implementation for OpenPGP """ import logging -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, Optional, Tuple from urllib import parse -from securesystemslib import exceptions +from securesystemslib import exceptions, formats from securesystemslib.gpg import exceptions as gpg_exceptions from securesystemslib.gpg import functions as gpg from securesystemslib.signer._key import Key @@ -25,158 +25,48 @@ class GPGKey(Key): ketytype: Key type, e.g. "rsa", "dsa" or "eddsa". scheme: Signing schemes, e.g. "pgp+rsa-pkcsv1.5", "pgp+dsa-fips-180-2", "pgp+eddsa-ed25519". - hashes: Hash algorithm to hash the data to be signed, e.g. "pgp+SHA2". keyval: Opaque key content. - creation_time: Unix timestamp when key was created. - validity_period: Validity of key in days. - subkeys: A dictionary of keyids as keys and GPGKeys as values. unrecognized_fields: Dictionary of all attributes that are not managed by Securesystemslib """ - def __init__( - self, - keyid: str, - keytype: str, - scheme: str, - hashes: List[str], - keyval: Dict[str, Any], - creation_time: Optional[int] = None, - validity_period: Optional[int] = None, - subkeys: Optional[Dict[str, "GPGKey"]] = None, - unrecognized_fields: Optional[Dict[str, Any]] = None, - ): - super().__init__(keyid, keytype, scheme, keyval, unrecognized_fields) - - self.hashes = hashes - self.creation_time = creation_time - self.validity_period = validity_period - self.subkeys = subkeys - - def __eq__(self, other: Any) -> bool: - if not isinstance(other, GPGKey): - return False - - return ( - super().__eq__(other) - and self.hashes == other.hashes - and self.creation_time == other.creation_time - and self.validity_period == other.validity_period - and self.subkeys == other.subkeys - ) - - @classmethod - def __from_dict( - cls, - keyid: str, - keytype: str, - scheme: str, - subkeys: Optional[Dict[str, "GPGKey"]], - key_dict: Dict[str, Any], - ) -> "GPGKey": - """Helper for common from_*dict operations.""" - - hashes = key_dict.pop("hashes") - keyval = key_dict.pop("keyval") - creation_time = key_dict.pop("creation_time", None) - validity_period = key_dict.pop("validity_period", None) - - return cls( - keyid, - keytype, - scheme, - hashes, - keyval, - creation_time, - validity_period, - subkeys, - key_dict, - ) - - @classmethod - def _from_legacy_dict(cls, key_dict: Dict[str, Any]) -> "GPGKey": - """Create GPGKey from legacy dictionary representation.""" - - keyid = key_dict.pop("keyid") - keytype = key_dict.pop("type") - scheme = key_dict.pop("method") - subkeys = key_dict.pop("subkeys", None) - - if subkeys is not None: - subkeys = { - keyid: cls._from_legacy_dict( - key - ) # pylint: disable=protected-access - for (keyid, key) in subkeys.items() - } - - return cls.__from_dict(keyid, keytype, scheme, subkeys, key_dict) - @classmethod def from_dict(cls, keyid: str, key_dict: Dict[str, Any]) -> "GPGKey": keytype = key_dict.pop("keytype") scheme = key_dict.pop("scheme") - subkeys = key_dict.pop("subkeys", None) - - if subkeys: - subkeys = { - keyid: cls.from_dict(keyid, key) - for (keyid, key) in subkeys.items() - } - - return cls.__from_dict(keyid, keytype, scheme, subkeys, key_dict) + keyval = key_dict.pop("keyval") - def __to_dict(self) -> Dict[str, Any]: - """Helper for common to_*dict operations.""" + return cls(keyid, keytype, scheme, keyval, key_dict) - key_dict: Dict[str, Any] = { - "hashes": self.hashes, + def to_dict(self) -> Dict: + return { + "keytype": self.keytype, + "scheme": self.scheme, "keyval": self.keyval, + **self.unrecognized_fields, } - if self.creation_time is not None: - key_dict["creation_time"] = self.creation_time - if self.validity_period is not None: - key_dict["validity_period"] = self.validity_period + @classmethod + def _from_legacy_dict(cls, key_dict: Dict[str, Any]) -> "GPGKey": + """Create GPGKey from legacy dictionary representation.""" + + keyid = key_dict["keyid"] + keytype = key_dict["type"] + scheme = key_dict["method"] + keyval = key_dict["keyval"] - return key_dict + return cls(keyid, keytype, scheme, keyval) def _to_legacy_dict(self) -> Dict[str, Any]: """Returns legacy dictionary representation of self.""" - key_dict = self.__to_dict() - key_dict.update( - { - "keyid": self.keyid, - "type": self.keytype, - "method": self.scheme, - } - ) - - if self.subkeys: - key_dict["subkeys"] = { - keyid: key._to_legacy_dict() # pylint: disable=protected-access - for (keyid, key) in self.subkeys.items() - } - - return key_dict - - def to_dict(self) -> Dict[str, Any]: - key_dict = self.__to_dict() - key_dict.update( - { - "keytype": self.keytype, - "scheme": self.scheme, - **self.unrecognized_fields, - } - ) - - if self.subkeys: - key_dict["subkeys"] = { - keyid: key.to_dict() for (keyid, key) in self.subkeys.items() - } - - return key_dict + return { + "keyid": self.keyid, + "type": self.keytype, + "method": self.scheme, + "hashes": [formats.GPG_HASH_ALGORITHM_STRING], + "keyval": self.keyval, + } def verify_signature(self, signature: Signature, data: bytes) -> None: try: @@ -193,7 +83,6 @@ def verify_signature(self, signature: Signature, data: bytes) -> None: except ( exceptions.FormatError, exceptions.UnsupportedLibraryError, - gpg_exceptions.KeyExpirationError, ) as e: logger.info("Key %s failed to verify sig: %s", self.keyid, str(e)) raise exceptions.VerificationError( @@ -270,7 +159,10 @@ def _sig_from_legacy_dict(sig_dict: Dict) -> Signature: def import_( cls, keyid: str, homedir: Optional[str] = None ) -> Tuple[str, Key]: - """Load key and signer details from GnuPG keyring + """Load key and signer details from GnuPG keyring. + + NOTE: Information about the key validity (expiration, revocation, etc.) + is discarded at import and not considered when verifying a signature. Args: keyid: GnuPG local user signing key id. diff --git a/tests/check_public_interfaces.py b/tests/check_public_interfaces.py index ab4ccf34..9b195d20 100644 --- a/tests/check_public_interfaces.py +++ b/tests/check_public_interfaces.py @@ -321,9 +321,7 @@ def test_gpg_functions(self): def test_signer(self): """Assert generic VerificationError from UnsupportedLibraryError.""" - key = GPGKey( - "aa", "rsa", "pgp+rsa-pkcsv1.5", ["pgp+SHA2"], {"public": "val"} - ) + key = GPGKey("aa", "rsa", "pgp+rsa-pkcsv1.5", {"public": "val"}) sig = Signature("aa", "aaaaaaa", {"other_headers": "aaaaaa"}) with self.assertRaises(VerificationError) as ctx: key.verify_signature(sig, b"data") diff --git a/tests/check_public_interfaces_gpg.py b/tests/check_public_interfaces_gpg.py index 4ce2c9f5..27914ca9 100644 --- a/tests/check_public_interfaces_gpg.py +++ b/tests/check_public_interfaces_gpg.py @@ -56,7 +56,6 @@ def test_gpg_functions(self): "aa", "rsa", "pgp+rsa-pkcsv1.5", - ["pgp+SHA2"], {"public": {"key": "value"}}, ) signer = Signer.from_priv_key_uri("gnupg:?id=abcd", mock_public_key) diff --git a/tests/test_signer.py b/tests/test_signer.py index 78ab8f35..472bcc1b 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -448,9 +448,7 @@ def test_gpg_fail_sign_expired_key(self): def test_gpg_signer_load_with_bad_scheme(self): """Load from priv key uri with wrong uri scheme.""" - key = GPGKey( - "aa", "rsa", "pgp+rsa-pkcsv1.5", ["pgp+SHA2"], {"public": "val"} - ) + key = GPGKey("aa", "rsa", "pgp+rsa-pkcsv1.5", {"public": "val"}) with self.assertRaises(ValueError): GPGSigner.from_priv_key_uri("wrong:", key) @@ -494,34 +492,9 @@ def test_gpg_key_legacy_data_structure(self): self.assertEqual(public_key, GPGKey._from_legacy_dict(legacy_dict)) - def test_gpg_key_optional_fields(self): - """Test gpg public key from/to_dict with optional fields.""" - - keydict_base = { - "keytype": "rsa", - "scheme": "pgp+rsa-pkcsv1.5", - "hashes": ["pgp+SHA2"], - "keyval": { - "public": "pubkeyval", - }, - } - - for name, val in ( - ("creation_time", 1), - ("validity_period", 1), - ("subkeys", {"bb": copy.deepcopy(keydict_base)}), - ): - keydict = copy.deepcopy(keydict_base) - keydict[name] = val - key = Key.from_dict("aa", copy.deepcopy(keydict)) - self.assertIsNotNone(getattr(key, name)) - self.assertDictEqual(keydict, key.to_dict(), name) - def test_gpg_key__eq__(self): """Test GPGKey.__eq__() .""" - key1 = GPGKey( - "aa", "rsa", "pgp+rsa-pkcsv1.5", ["pgp+SHA2"], {"public": "val"} - ) + key1 = GPGKey("aa", "rsa", "pgp+rsa-pkcsv1.5", {"public": "val"}) key2 = copy.deepcopy(key1) self.assertEqual(key1, key2) From e2819475236cc05cbc24933f36998ec9002c947d Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 6 Mar 2023 14:56:41 +0100 Subject: [PATCH 12/13] signer: move legacy gpg key conversion methods These methods are ugly no matter where they are implemented. I move them to the signer to keep them together with the legacy signature format conversion methods. Signed-off-by: Lukas Puehringer --- securesystemslib/signer/_gpg_signer.py | 52 +++++++++++++------------- tests/check_public_interfaces_gpg.py | 3 +- tests/test_signer.py | 6 ++- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index fe847529..5498609d 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -46,35 +46,15 @@ def to_dict(self) -> Dict: **self.unrecognized_fields, } - @classmethod - def _from_legacy_dict(cls, key_dict: Dict[str, Any]) -> "GPGKey": - """Create GPGKey from legacy dictionary representation.""" - - keyid = key_dict["keyid"] - keytype = key_dict["type"] - scheme = key_dict["method"] - keyval = key_dict["keyval"] - - return cls(keyid, keytype, scheme, keyval) - - def _to_legacy_dict(self) -> Dict[str, Any]: - """Returns legacy dictionary representation of self.""" - - return { - "keyid": self.keyid, - "type": self.keytype, - "method": self.scheme, - "hashes": [formats.GPG_HASH_ALGORITHM_STRING], - "keyval": self.keyval, - } - def verify_signature(self, signature: Signature, data: bytes) -> None: try: if not gpg.verify_signature( GPGSigner._sig_to_legacy_dict( # pylint: disable=protected-access signature ), - self._to_legacy_dict(), + GPGSigner._key_to_legacy_dict( # pylint: disable=protected-access + self + ), data, ): raise exceptions.UnverifiedSignatureError( @@ -155,6 +135,27 @@ def _sig_from_legacy_dict(sig_dict: Dict) -> Signature: sig_dict["sig"] = sig_dict.pop("signature") return Signature.from_dict(sig_dict) + @staticmethod + def _key_to_legacy_dict(key: GPGKey) -> Dict[str, Any]: + """Returns legacy dictionary representation of self.""" + return { + "keyid": key.keyid, + "type": key.keytype, + "method": key.scheme, + "hashes": [formats.GPG_HASH_ALGORITHM_STRING], + "keyval": key.keyval, + } + + @staticmethod + def _key_from_legacy_dict(key_dict: Dict[str, Any]) -> GPGKey: + """Create GPGKey from legacy dictionary representation.""" + keyid = key_dict["keyid"] + keytype = key_dict["type"] + scheme = key_dict["method"] + keyval = key_dict["keyval"] + + return GPGKey(keyid, keytype, scheme, keyval) + @classmethod def import_( cls, keyid: str, homedir: Optional[str] = None @@ -187,9 +188,7 @@ def import_( for key in raw_keys: if key["keyid"] == keyid: # TODO: Raise here if key is expired, revoked, incapable, ... - public_key = GPGKey._from_legacy_dict( # pylint: disable=protected-access - key - ) + public_key = cls._key_from_legacy_dict(key) break keyids.append(key["keyid"]) @@ -219,6 +218,7 @@ def sign(self, payload: bytes) -> Signature: Returns: Signature. + """ raw_sig = gpg.create_signature( payload, self.public_key.keyid, self.homedir diff --git a/tests/check_public_interfaces_gpg.py b/tests/check_public_interfaces_gpg.py index 27914ca9..eeadf18a 100644 --- a/tests/check_public_interfaces_gpg.py +++ b/tests/check_public_interfaces_gpg.py @@ -34,7 +34,6 @@ export_pubkeys, verify_signature, ) - from securesystemslib.signer import GPGKey, GPGSigner, Signer @@ -153,7 +152,7 @@ def test_gpg_verify(self): for key, sig in key_signature_pairs: self.assertTrue(verify_signature(sig, key, data)) # pylint: disable=protected-access - GPGKey._from_legacy_dict(key).verify_signature( + GPGSigner._key_from_legacy_dict(key).verify_signature( GPGSigner._sig_from_legacy_dict(sig), data ) diff --git a/tests/test_signer.py b/tests/test_signer.py index 472bcc1b..23b6761c 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -483,14 +483,16 @@ def test_gpg_key_legacy_data_structure(self): legacy_fields = {"keyid", "type", "method"} fields = {"keytype", "scheme"} - legacy_dict = public_key._to_legacy_dict() + legacy_dict = GPGSigner._key_to_legacy_dict(public_key) for field in legacy_fields: self.assertIn(field, legacy_dict) for field in fields: self.assertNotIn(field, legacy_dict) - self.assertEqual(public_key, GPGKey._from_legacy_dict(legacy_dict)) + self.assertEqual( + public_key, GPGSigner._key_from_legacy_dict(legacy_dict) + ) def test_gpg_key__eq__(self): """Test GPGKey.__eq__() .""" From cc14e519dcbf96d8f8595179d3c5afa36473e632 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 6 Mar 2023 14:57:13 +0100 Subject: [PATCH 13/13] gpg: change warning log statement to debug Change warning type log statement about exported subkeys to debug. This is no longer relevant for GPGSigner.import_, which don't return key bundles to the user, but only a key or a subkey. Signed-off-by: Lukas Puehringer --- securesystemslib/gpg/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securesystemslib/gpg/common.py b/securesystemslib/gpg/common.py index ffd6d67b..e1236f9d 100644 --- a/securesystemslib/gpg/common.py +++ b/securesystemslib/gpg/common.py @@ -635,7 +635,7 @@ def get_pubkey_bundle(data, keyid): ): if public_key and public_key["keyid"].endswith(keyid.lower()): if idx > 1: - log.warning( + log.debug( "Exporting master key '{}' including subkeys '{}' for" # pylint: disable=logging-format-interpolation,consider-using-f-string " passed keyid '{}'.".format( master_public_key["keyid"],