-
Notifications
You must be signed in to change notification settings - Fork 50
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
Abstract methods added to Key for signing scheme dissection #837
Open
L77H
wants to merge
6
commits into
secure-systems-lab:main
Choose a base branch
from
L77H:signer-get-hash-algo-abstractmethod
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c6d2729
abstract methods get_hash_algorithm_str, get_hash_algorithm, get_padd…
8c0ff83
PSS salt Auto error fix
cbe34d8
PSS salt_length as function arg
84fc3ea
removed abstract methods from Key Class and only kept the methods on …
bc624ac
Merge branch 'main' into signer-get-hash-algo-abstractmethod
L77H 0aecbc6
merge fix
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,13 @@ | |
from abc import ABCMeta, abstractmethod | ||
from typing import Any, Dict, Optional, Tuple, Type, cast | ||
|
||
import securesystemslib.hash as sslib_hash | ||
from securesystemslib._vendor.ed25519.ed25519 import ( | ||
SignatureMismatch, | ||
checkvalid, | ||
) | ||
from securesystemslib.exceptions import ( | ||
UnsupportedAlgorithmError, | ||
UnsupportedLibraryError, | ||
UnverifiedSignatureError, | ||
VerificationError, | ||
|
@@ -56,6 +58,11 @@ | |
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class UnsupportedKeyType(Exception): # noqa: N818 | ||
pass | ||
Comment on lines
+62
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this could be marked private (_UnsupportedKeyType) since I don't think we intend to leak it outside... but that's a nit |
||
|
||
|
||
# NOTE Key dispatch table is defined here so it's usable by Key, | ||
# but is populated in __init__.py (and can be appended by users). | ||
KEY_FOR_TYPE_AND_SCHEME: Dict[Tuple[str, str], Type] = {} | ||
|
@@ -301,35 +308,6 @@ def from_crypto( | |
|
||
return SSlibKey(keyid, keytype, scheme, keyval) | ||
|
||
@staticmethod | ||
def _get_hash_algorithm(name: str) -> "HashAlgorithm": | ||
"""Helper to return hash algorithm for name.""" | ||
algorithm: HashAlgorithm | ||
if name == "sha224": | ||
algorithm = SHA224() | ||
if name == "sha256": | ||
algorithm = SHA256() | ||
if name == "sha384": | ||
algorithm = SHA384() | ||
if name == "sha512": | ||
algorithm = SHA512() | ||
|
||
return algorithm | ||
|
||
@staticmethod | ||
def _get_rsa_padding( | ||
name: str, hash_algorithm: "HashAlgorithm" | ||
) -> "AsymmetricPadding": | ||
"""Helper to return rsa signature padding for name.""" | ||
padding: AsymmetricPadding | ||
if name == "pss": | ||
padding = PSS(mgf=MGF1(hash_algorithm), salt_length=PSS.AUTO) | ||
|
||
if name == "pkcs1v15": | ||
padding = PKCS1v15() | ||
|
||
return padding | ||
|
||
def _verify_ed25519_fallback(self, signature: bytes, data: bytes) -> None: | ||
"""Helper to verify ed25519 sig if pyca/cryptography is unavailable.""" | ||
try: | ||
|
@@ -364,9 +342,8 @@ def _validate_curve(key, curve): | |
]: | ||
key = cast(RSAPublicKey, self._crypto_key()) | ||
_validate_type(key, RSAPublicKey) | ||
padding_name, hash_name = self.scheme.split("-")[1:] | ||
hash_algorithm = self._get_hash_algorithm(hash_name) | ||
padding = self._get_rsa_padding(padding_name, hash_algorithm) | ||
hash_algorithm = self.get_hash_algorithm() | ||
padding = self.get_padding_name(hash_algorithm, PSS.AUTO) | ||
key.verify(signature, data, padding, hash_algorithm) | ||
|
||
elif ( | ||
|
@@ -426,3 +403,75 @@ def verify_signature(self, signature: Signature, data: bytes) -> None: | |
raise VerificationError( | ||
f"Unknown failure to verify signature by {self.keyid}" | ||
) from e | ||
|
||
def get_hash_algorithm_str(self) -> str: | ||
"""Returns the hash algorithm from the key scheme as a string.""" | ||
# key scheme should always be of format xxx-xxx-xxx | ||
comps = self.scheme.split("-") | ||
if len(comps) != 3: # noqa: PLR2004 | ||
raise UnsupportedKeyType("Invalid scheme found") | ||
|
||
if self.keytype == "rsa": | ||
# hash algorithm is encoded as last scheme portion | ||
hash_algo = self.scheme.split("-")[-1] | ||
elif self.keytype in [ | ||
"ecdsa", | ||
"ecdsa-sha2-nistp256", | ||
"ecdsa-sha2-nistp384", | ||
]: | ||
# nistp256 uses sha-256, nistp384 uses sha-384 | ||
bits = self.scheme.split("-nistp")[-1] | ||
hash_algo = f"sha{bits}" | ||
else: | ||
raise UnsupportedAlgorithmError( | ||
f"Unsupported key type {self.keytype} in key {self.keyid}" | ||
) | ||
|
||
# trigger UnsupportedAlgorithm if appropriate | ||
_ = sslib_hash.digest(hash_algo) | ||
|
||
return hash_algo | ||
|
||
def get_hash_algorithm(self) -> "HashAlgorithm": | ||
"""Returns the hash algorithm from the key scheme as a HashAlgorithm""" | ||
name = self.get_hash_algorithm_str() | ||
algorithm: HashAlgorithm | ||
if name == "sha224": | ||
algorithm = SHA224() | ||
if name == "sha256": | ||
algorithm = SHA256() | ||
if name == "sha384": | ||
algorithm = SHA384() | ||
if name == "sha512": | ||
algorithm = SHA512() | ||
|
||
return algorithm | ||
|
||
def get_padding_name_str(self) -> str: | ||
"""Returns the padding name from the key scheme as a string""" | ||
padding_name = self.scheme.split("-")[1] | ||
return padding_name | ||
|
||
def get_padding_name( | ||
self, hash_algorithm: "HashAlgorithm", salt_length: Any | ||
) -> "AsymmetricPadding": | ||
"""Returns the padding name from the key scheme as a AsymmetricPadding | ||
|
||
Args: | ||
hash_algorithm: the hash algorithm used as a HashAlgorithm | ||
object, only for PSS. | ||
selt_length: the salt length to use for PSS. | ||
PSS.AUTO or PSS.DIGEST_LENGTH | ||
|
||
Returns: | ||
AsymmetricPadding | ||
|
||
""" | ||
name = self.get_padding_name_str() | ||
padding: AsymmetricPadding | ||
if name == "pss": | ||
padding = PSS(mgf=MGF1(hash_algorithm), salt_length=salt_length) | ||
if name == "pkcs1v15": | ||
padding = PKCS1v15() | ||
|
||
return padding |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These stubs are no longer needed, right?