From 439ddd861509436adaac07cd4e1b15e586914190 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Tue, 23 Apr 2024 15:05:49 +0200 Subject: [PATCH] Do not raise non-public errors in GPGSigner * KeyNotFoundError, which is raised internally in multiple places, is caught and re-raised as ValueError in GPGSigner * CommandError, which was raised only in one place and close to the user boundary, is replaced directly with OSError. Using these builtin Error classes seems semantically correct, and consistent with other errors expected in GPGSigner. Signed-off-by: Lukas Puehringer --- securesystemslib/_gpg/exceptions.py | 4 ---- securesystemslib/_gpg/functions.py | 10 ++++----- securesystemslib/signer/_gpg_signer.py | 31 ++++++++++++++++---------- tests/test_gpg.py | 5 ++--- tests/test_signer.py | 5 ++--- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/securesystemslib/_gpg/exceptions.py b/securesystemslib/_gpg/exceptions.py index 90d63864..ee9e9bd7 100644 --- a/securesystemslib/_gpg/exceptions.py +++ b/securesystemslib/_gpg/exceptions.py @@ -38,10 +38,6 @@ class SignatureAlgorithmNotSupportedError(Exception): pass -class CommandError(Exception): - pass - - class KeyExpirationError(Exception): # pylint: disable=missing-class-docstring def __init__(self, key): super( # pylint: disable=super-with-arguments diff --git a/securesystemslib/_gpg/functions.py b/securesystemslib/_gpg/functions.py index d66b14b5..6db99e81 100644 --- a/securesystemslib/_gpg/functions.py +++ b/securesystemslib/_gpg/functions.py @@ -34,7 +34,7 @@ gpg_sign_command, have_gpg, ) -from securesystemslib._gpg.exceptions import CommandError, KeyExpirationError +from securesystemslib._gpg.exceptions import KeyExpirationError from securesystemslib._gpg.handlers import SIGNATURE_HANDLERS from securesystemslib._gpg.rsa import CRYPTO @@ -78,15 +78,13 @@ def create_signature(content, keyid=None, homedir=None, timeout=GPG_TIMEOUT): If the gpg command failed to create a valid signature. OSError: - If the gpg command is not present or non-executable. + If the gpg command is not present, or non-executable, + or returned a non-zero exit code securesystemslib.exceptions.UnsupportedLibraryError: If the gpg command is not available, or the cryptography library is not installed. - securesystemslib._gpg.exceptions.CommandError: - If the gpg command returned a non-zero exit code - securesystemslib._gpg.exceptions.KeyNotFoundError: If the used gpg version is not fully supported and no public key can be found for short keyid. @@ -134,7 +132,7 @@ def create_signature(content, keyid=None, homedir=None, timeout=GPG_TIMEOUT): # reporting, as there is no clear distinction between the return codes # https://lists.gnupg.org/pipermail/gnupg-devel/2005-December/022559.html if gpg_process.returncode != 0: - raise CommandError( + raise OSError( "Command '{}' returned " # pylint: disable=consider-using-f-string "non-zero exit status '{}', stderr was:\n{}.".format( gpg_process.args, diff --git a/securesystemslib/signer/_gpg_signer.py b/securesystemslib/signer/_gpg_signer.py index 5eaf3e07..9c7f4ce2 100644 --- a/securesystemslib/signer/_gpg_signer.py +++ b/securesystemslib/signer/_gpg_signer.py @@ -167,7 +167,7 @@ def import_( Raises: UnsupportedLibraryError: The gpg command or pyca/cryptography are not available. - KeyNotFoundError: No key was found for the passed keyid. + ValueError: No key was found for the passed keyid. Returns: Tuple of private key uri and the public key. @@ -175,7 +175,12 @@ def import_( """ uri = f"{cls.SCHEME}:{homedir or ''}" - raw_key = gpg.export_pubkey(keyid, homedir) + try: + raw_key = gpg.export_pubkey(keyid, homedir) + + except gpg_exceptions.KeyNotFoundError as e: + raise ValueError(e) from e + raw_keys = [raw_key] + list(raw_key.pop("subkeys", {}).values()) keyids = [] @@ -187,7 +192,7 @@ def import_( keyids.append(key["keyid"]) else: - raise gpg_exceptions.KeyNotFoundError( + raise ValueError( f"No exact match found for passed keyid" f" {keyid}, found: {keyids}." ) @@ -201,22 +206,24 @@ def sign(self, payload: bytes) -> Signature: payload: bytes to be signed. Raises: - ValueError: gpg command failed to create a valid signature. - OSError: gpg command is not present or non-executable. + ValueError: gpg command failed to create a valid signature, e.g. + because its keyid does not match the public key keyid. + OSError: gpg command is not present, or non-executable, or returned + a non-zero exit code. 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. """ - raw_sig = gpg.create_signature( - payload, self.public_key.keyid, self.homedir - ) + try: + raw_sig = gpg.create_signature( + payload, self.public_key.keyid, self.homedir + ) + except gpg_exceptions.KeyNotFoundError as e: + raise ValueError(e) from e + if raw_sig["keyid"] != self.public_key.keyid: raise ValueError( f"The signing key {raw_sig['keyid']} does not" diff --git a/tests/test_gpg.py b/tests/test_gpg.py index 4e72692d..c0707228 100644 --- a/tests/test_gpg.py +++ b/tests/test_gpg.py @@ -52,7 +52,6 @@ from securesystemslib._gpg.dsa import create_pubkey as dsa_create_pubkey from securesystemslib._gpg.eddsa import ED25519_SIG_LENGTH from securesystemslib._gpg.exceptions import ( - CommandError, KeyExpirationError, KeyNotFoundError, PacketParsingError, @@ -702,8 +701,8 @@ def test_gpg_sign_and_verify_object_default_keyring(self): del os.environ["GNUPGHOME"] def test_create_signature_with_expired_key(self): - """Test signing with expired key raises gpg CommandError.""" - with self.assertRaises(CommandError) as ctx: + """Test signing with expired key raises OSError.""" + with self.assertRaises(OSError) as ctx: create_signature( b"livestock", keyid=self.expired_key_keyid, diff --git a/tests/test_signer.py b/tests/test_signer.py index 73674344..71506812 100644 --- a/tests/test_signer.py +++ b/tests/test_signer.py @@ -16,7 +16,6 @@ ) from securesystemslib._gpg.constants import have_gpg -from securesystemslib._gpg.exceptions import CommandError, KeyNotFoundError from securesystemslib.exceptions import FormatError, UnverifiedSignatureError from securesystemslib.signer import ( KEY_FOR_TYPE_AND_SCHEME, @@ -452,7 +451,7 @@ def test_gpg_fail_import_keyid_match(self): # gpg exports the right key, but we require an exact keyid match non_matching_keyid = self.default_keyid.upper() - with self.assertRaises(KeyNotFoundError): + with self.assertRaises(ValueError): GPGSigner.import_(non_matching_keyid, self.gnupg_home) def test_gpg_fail_sign_expired_key(self): @@ -461,7 +460,7 @@ def test_gpg_fail_sign_expired_key(self): uri, public_key = GPGSigner.import_(expired_key, self.gnupg_home) signer = Signer.from_priv_key_uri(uri, public_key) - with self.assertRaises(CommandError): + with self.assertRaises(OSError): signer.sign(self.test_data) def test_gpg_signer_load_with_bad_scheme(self):