-
Notifications
You must be signed in to change notification settings - Fork 346
feat: add ecdsa p-384 support #1872
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
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8219b4d
feat: add ecdsa p-384 support
shengjiang3 add04f8
Merge branch 'main' into gdc-es384
daniel-sanche 548a725
chore: Update es based on PR comments
shengjiang3 456b47b
chore: update based on PR feedback
shengjiang3 4f1794d
chore: use alias from es256
shengjiang3 d754204
Merge branch 'main' into gdc-es384
daniel-sanche 1090a46
chore: Fix unit test failure
shengjiang3 c44d9c7
chore: add code coverage
shengjiang3 4e8d71a
chore: fix lint errors
shengjiang3 afc7b5d
chore: fix one more lint error
shengjiang3 0ac7b2d
Merge branch 'main' into gdc-es384
daniel-sanche 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
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
| @@ -0,0 +1,221 @@ | ||
| # Copyright 2017 Google Inc. | ||
|
Collaborator
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. Note to self (and other reviewers): most of this file is copied over from |
||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| """ECDSA verifier and signer that use the ``cryptography`` library. | ||
| """ | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import Any, Dict, Optional, Union | ||
|
|
||
| import cryptography.exceptions | ||
| from cryptography.hazmat import backends | ||
| from cryptography.hazmat.primitives import hashes | ||
| from cryptography.hazmat.primitives import serialization | ||
| from cryptography.hazmat.primitives.asymmetric import ec | ||
| from cryptography.hazmat.primitives.asymmetric import padding | ||
| from cryptography.hazmat.primitives.asymmetric.utils import decode_dss_signature | ||
| from cryptography.hazmat.primitives.asymmetric.utils import encode_dss_signature | ||
| import cryptography.x509 | ||
|
|
||
| from google.auth import _helpers | ||
| from google.auth.crypt import base | ||
|
|
||
|
|
||
| _CERTIFICATE_MARKER = b"-----BEGIN CERTIFICATE-----" | ||
| _BACKEND = backends.default_backend() | ||
| _PADDING = padding.PKCS1v15() | ||
|
|
||
|
|
||
| @dataclass | ||
| class _ESAttributes: | ||
| """A class that models ECDSA attributes. | ||
|
|
||
| Attributes: | ||
| rs_size (int): Size for ASN.1 r and s size. | ||
| sha_algo (hashes.HashAlgorithm): Hash algorithm. | ||
| algorithm (str): Algorithm name. | ||
| """ | ||
|
|
||
| rs_size: int | ||
| sha_algo: hashes.HashAlgorithm | ||
| algorithm: str | ||
|
|
||
| @classmethod | ||
| def from_key( | ||
| cls, key: Union[ec.EllipticCurvePublicKey, ec.EllipticCurvePrivateKey] | ||
| ): | ||
| return cls.from_curve(key.curve) | ||
|
|
||
| @classmethod | ||
| def from_curve(cls, curve: ec.EllipticCurve): | ||
| # ECDSA raw signature has (r||s) format where r,s are two | ||
| # integers of size 32 bytes for P-256 curve and 48 bytes | ||
| # for P-384 curve. For P-256 curve, we use SHA256 hash algo, | ||
| # and for P-384 curve we use SHA384 algo. | ||
| if isinstance(curve, ec.SECP384R1): | ||
| return cls(48, hashes.SHA384(), "ES384") | ||
| else: | ||
| # default to ES256 | ||
| return cls(32, hashes.SHA256(), "ES256") | ||
|
|
||
|
|
||
| class EsVerifier(base.Verifier): | ||
| """Verifies ECDSA cryptographic signatures using public keys. | ||
|
|
||
| Args: | ||
| public_key ( | ||
| cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePublicKey): | ||
| The public key used to verify signatures. | ||
| """ | ||
|
|
||
| def __init__(self, public_key: ec.EllipticCurvePublicKey) -> None: | ||
| self._pubkey = public_key | ||
| self._attributes = _ESAttributes.from_key(public_key) | ||
|
|
||
| @_helpers.copy_docstring(base.Verifier) | ||
| def verify(self, message: bytes, signature: bytes) -> bool: | ||
| # First convert (r||s) raw signature to ASN1 encoded signature. | ||
| sig_bytes = _helpers.to_bytes(signature) | ||
| if len(sig_bytes) != self._attributes.rs_size * 2: | ||
| return False | ||
| r = int.from_bytes(sig_bytes[: self._attributes.rs_size], byteorder="big") | ||
| s = int.from_bytes(sig_bytes[self._attributes.rs_size :], byteorder="big") | ||
| asn1_sig = encode_dss_signature(r, s) | ||
|
|
||
| message = _helpers.to_bytes(message) | ||
| try: | ||
| self._pubkey.verify(asn1_sig, message, ec.ECDSA(self._attributes.sha_algo)) | ||
| return True | ||
| except (ValueError, cryptography.exceptions.InvalidSignature): | ||
| return False | ||
|
|
||
| @classmethod | ||
| def from_string(cls, public_key: Union[str, bytes]) -> "EsVerifier": | ||
| """Construct an Verifier instance from a public key or public | ||
| certificate string. | ||
|
|
||
| Args: | ||
| public_key (Union[str, bytes]): The public key in PEM format or the | ||
| x509 public key certificate. | ||
|
|
||
| Returns: | ||
| Verifier: The constructed verifier. | ||
|
|
||
| Raises: | ||
| ValueError: If the public key can't be parsed. | ||
| """ | ||
| public_key_data = _helpers.to_bytes(public_key) | ||
|
|
||
| if _CERTIFICATE_MARKER in public_key_data: | ||
| cert = cryptography.x509.load_pem_x509_certificate( | ||
| public_key_data, _BACKEND | ||
| ) | ||
| pubkey = cert.public_key() # type: Any | ||
|
|
||
| else: | ||
| pubkey = serialization.load_pem_public_key(public_key_data, _BACKEND) | ||
|
|
||
| if not isinstance(pubkey, ec.EllipticCurvePublicKey): | ||
| raise TypeError("Expected public key of type EllipticCurvePublicKey") | ||
|
|
||
| return cls(pubkey) | ||
|
|
||
|
|
||
| class EsSigner(base.Signer, base.FromServiceAccountMixin): | ||
| """Signs messages with an ECDSA private key. | ||
|
|
||
| Args: | ||
| private_key ( | ||
| cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePrivateKey): | ||
| The private key to sign with. | ||
| key_id (str): Optional key ID used to identify this private key. This | ||
| can be useful to associate the private key with its associated | ||
| public key or certificate. | ||
| """ | ||
|
|
||
| def __init__( | ||
| self, private_key: ec.EllipticCurvePrivateKey, key_id: Optional[str] = None | ||
| ) -> None: | ||
| self._key = private_key | ||
| self._key_id = key_id | ||
| self._attributes = _ESAttributes.from_key(private_key) | ||
|
|
||
| @property | ||
| def algorithm(self) -> str: | ||
| """Name of the algorithm used to sign messages. | ||
| Returns: | ||
| str: The algorithm name. | ||
| """ | ||
| return self._attributes.algorithm | ||
|
|
||
| @property # type: ignore | ||
| @_helpers.copy_docstring(base.Signer) | ||
| def key_id(self) -> Optional[str]: | ||
| return self._key_id | ||
|
|
||
| @_helpers.copy_docstring(base.Signer) | ||
| def sign(self, message: bytes) -> bytes: | ||
| message = _helpers.to_bytes(message) | ||
| asn1_signature = self._key.sign(message, ec.ECDSA(self._attributes.sha_algo)) | ||
|
|
||
| # Convert ASN1 encoded signature to (r||s) raw signature. | ||
| (r, s) = decode_dss_signature(asn1_signature) | ||
| return r.to_bytes(self._attributes.rs_size, byteorder="big") + s.to_bytes( | ||
| self._attributes.rs_size, byteorder="big" | ||
| ) | ||
|
|
||
| @classmethod | ||
| def from_string( | ||
| cls, key: Union[bytes, str], key_id: Optional[str] = None | ||
| ) -> "EsSigner": | ||
| """Construct a RSASigner from a private key in PEM format. | ||
|
|
||
| Args: | ||
| key (Union[bytes, str]): Private key in PEM format. | ||
| key_id (str): An optional key id used to identify the private key. | ||
|
|
||
| Returns: | ||
| google.auth.crypt._cryptography_rsa.RSASigner: The | ||
| constructed signer. | ||
|
|
||
| Raises: | ||
| ValueError: If ``key`` is not ``bytes`` or ``str`` (unicode). | ||
| UnicodeDecodeError: If ``key`` is ``bytes`` but cannot be decoded | ||
| into a UTF-8 ``str``. | ||
| ValueError: If ``cryptography`` "Could not deserialize key data." | ||
| """ | ||
| key_bytes = _helpers.to_bytes(key) | ||
| private_key = serialization.load_pem_private_key( | ||
| key_bytes, password=None, backend=_BACKEND | ||
| ) | ||
|
|
||
| if not isinstance(private_key, ec.EllipticCurvePrivateKey): | ||
| raise TypeError("Expected private key of type EllipticCurvePrivateKey") | ||
|
|
||
| return cls(private_key, key_id=key_id) | ||
|
|
||
| def __getstate__(self) -> Dict[str, Any]: | ||
| """Pickle helper that serializes the _key attribute.""" | ||
| state = self.__dict__.copy() | ||
| state["_key"] = self._key.private_bytes( | ||
| encoding=serialization.Encoding.PEM, | ||
| format=serialization.PrivateFormat.PKCS8, | ||
| encryption_algorithm=serialization.NoEncryption(), | ||
| ) | ||
| return state | ||
|
|
||
| def __setstate__(self, state: Dict[str, Any]) -> None: | ||
| """Pickle helper that deserializes the _key attribute.""" | ||
| state["_key"] = serialization.load_pem_private_key(state["_key"], None) | ||
| self.__dict__.update(state) | ||
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
Added a comment.
I tried to add a unit test to test this, but found that it is not trivial to simulate an import error. The logic here is straight forward enough that it might be ok not to have code coverage for this part. What do you think?
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.
IIRC, you can unimport a package using something like
del cryptography. That should let you define a test method that doesn't have cryptography active. Did you try doing something like that?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.
Yeah, I tried using monkey patch to remove the library and then to reload imports but it wasn't working. I think spending additional time to figure out how to simulate an import error just to check that the ES* types are not defined has low ROI.