-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[Experiment] feat(auth): backport rsa to 2.49.2 #16805
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,15 +102,15 @@ def verify(self, message: bytes, signature: bytes) -> bool: | |
|
|
||
| @classmethod | ||
| def from_string(cls, public_key: Union[str, bytes]) -> "EsVerifier": | ||
| """Construct a Verifier instance from a public key or public | ||
| """Construct an Verifier instance from a public key or public | ||
|
Contributor
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. |
||
| certificate string. | ||
|
|
||
| Args: | ||
| public_key (Union[str, bytes]): The public key in PEM format or the | ||
| x509 public key certificate. | ||
|
|
||
| Returns: | ||
| google.auth.crypt.Verifier: The constructed verifier. | ||
| Verifier: The constructed verifier. | ||
|
|
||
| Raises: | ||
| ValueError: If the public key can't be parsed. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,121 +12,19 @@ | |||||||||
| # See the License for the specific language governing permissions and | ||||||||||
| # limitations under the License. | ||||||||||
|
|
||||||||||
| """ | ||||||||||
| RSA cryptography signer and verifier. | ||||||||||
| """RSA cryptography signer and verifier.""" | ||||||||||
|
|
||||||||||
| This file provides a shared wrapper, that defers to _python_rsa or _cryptography_rsa | ||||||||||
| for implmentations using different third party libraries | ||||||||||
| """ | ||||||||||
|
|
||||||||||
| from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey | ||||||||||
| from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey | ||||||||||
| try: | ||||||||||
| # Prefer cryptograph-based RSA implementation. | ||||||||||
| from google.auth.crypt import _cryptography_rsa | ||||||||||
|
|
||||||||||
| from google.auth import _helpers | ||||||||||
| from google.auth.crypt import _cryptography_rsa | ||||||||||
| from google.auth.crypt import base | ||||||||||
| RSASigner = _cryptography_rsa.RSASigner | ||||||||||
| RSAVerifier = _cryptography_rsa.RSAVerifier | ||||||||||
| except ImportError: # pragma: NO COVER | ||||||||||
| # Fallback to pure-python RSA implementation if cryptography is | ||||||||||
| # unavailable. | ||||||||||
| from google.auth.crypt import _python_rsa | ||||||||||
|
|
||||||||||
| RSA_KEY_MODULE_PREFIX = "rsa.key" | ||||||||||
|
|
||||||||||
|
|
||||||||||
| class RSAVerifier(base.Verifier): | ||||||||||
| """Verifies RSA cryptographic signatures using public keys. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| public_key (Union["rsa.key.PublicKey", cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey]): | ||||||||||
| The public key used to verify signatures. | ||||||||||
| Raises: | ||||||||||
| ImportError: if called with an rsa.key.PublicKey, when the rsa library is not installed | ||||||||||
| ValueError: if an unrecognized public key is provided | ||||||||||
| """ | ||||||||||
|
|
||||||||||
| def __init__(self, public_key): | ||||||||||
| module_str = public_key.__class__.__module__ | ||||||||||
| if isinstance(public_key, RSAPublicKey): | ||||||||||
| impl_lib = _cryptography_rsa | ||||||||||
| elif module_str.startswith(RSA_KEY_MODULE_PREFIX): | ||||||||||
| from google.auth.crypt import _python_rsa | ||||||||||
|
|
||||||||||
| impl_lib = _python_rsa | ||||||||||
| else: | ||||||||||
| raise ValueError(f"unrecognized public key type: {type(public_key)}") | ||||||||||
| self._impl = impl_lib.RSAVerifier(public_key) | ||||||||||
|
|
||||||||||
| @_helpers.copy_docstring(base.Verifier) | ||||||||||
| def verify(self, message, signature): | ||||||||||
| return self._impl.verify(message, signature) | ||||||||||
|
|
||||||||||
| @classmethod | ||||||||||
| def from_string(cls, public_key): | ||||||||||
| """Construct a 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: | ||||||||||
| google.auth.crypt.Verifier: The constructed verifier. | ||||||||||
|
|
||||||||||
| Raises: | ||||||||||
| ValueError: If the public_key can't be parsed. | ||||||||||
| """ | ||||||||||
| instance = cls.__new__(cls) | ||||||||||
| instance._impl = _cryptography_rsa.RSAVerifier.from_string(public_key) | ||||||||||
| return instance | ||||||||||
|
|
||||||||||
|
|
||||||||||
| class RSASigner(base.Signer, base.FromServiceAccountMixin): | ||||||||||
| """Signs messages with an RSA private key. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| private_key (Union["rsa.key.PrivateKey", cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey]): | ||||||||||
| 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. | ||||||||||
|
|
||||||||||
| Raises: | ||||||||||
| ImportError: if called with an rsa.key.PrivateKey, when the rsa library is not installed | ||||||||||
| ValueError: if an unrecognized public key is provided | ||||||||||
| """ | ||||||||||
|
|
||||||||||
| def __init__(self, private_key, key_id=None): | ||||||||||
| module_str = private_key.__class__.__module__ | ||||||||||
| if isinstance(private_key, RSAPrivateKey): | ||||||||||
| impl_lib = _cryptography_rsa | ||||||||||
| elif module_str.startswith(RSA_KEY_MODULE_PREFIX): | ||||||||||
| from google.auth.crypt import _python_rsa | ||||||||||
|
|
||||||||||
| impl_lib = _python_rsa | ||||||||||
| else: | ||||||||||
| raise ValueError(f"unrecognized private key type: {type(private_key)}") | ||||||||||
| self._impl = impl_lib.RSASigner(private_key, key_id=key_id) | ||||||||||
|
|
||||||||||
| @property # type: ignore | ||||||||||
| @_helpers.copy_docstring(base.Signer) | ||||||||||
| def key_id(self): | ||||||||||
| return self._impl.key_id | ||||||||||
|
|
||||||||||
| @_helpers.copy_docstring(base.Signer) | ||||||||||
| def sign(self, message): | ||||||||||
| return self._impl.sign(message) | ||||||||||
|
|
||||||||||
| @classmethod | ||||||||||
| def from_string(cls, key, key_id=None): | ||||||||||
| """Construct a Signer instance from a private key in PEM format. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| key (str): Private key in PEM format. | ||||||||||
| key_id (str): An optional key id used to identify the private key. | ||||||||||
|
|
||||||||||
| Returns: | ||||||||||
| google.auth.crypt.Signer: The constructed signer. | ||||||||||
|
|
||||||||||
| Raises: | ||||||||||
| ValueError: If the key cannot be parsed as PKCS#1 or PKCS#8 in | ||||||||||
| PEM format. | ||||||||||
| """ | ||||||||||
| instance = cls.__new__(cls) | ||||||||||
| instance._impl = _cryptography_rsa.RSASigner.from_string(key, key_id=key_id) | ||||||||||
| return instance | ||||||||||
| RSASigner = _python_rsa.RSASigner # type: ignore | ||||||||||
| RSAVerifier = _python_rsa.RSAVerifier # type: ignore | ||||||||||
|
Comment on lines
+18
to
+30
Contributor
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 change introduces a functional regression. The previous implementation (on the LEFT) was a dynamic wrapper that supported both cryptography and rsa library key objects at runtime. By statically aliasing RSASigner and RSAVerifier to one implementation or the other at import time, the library loses its ability to handle different key types interchangeably if both libraries are present.
Comment on lines
+29
to
+30
Contributor
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. Use a more specific type ignore like # type: ignore[assignment] when assigning classes in an except ImportError block, consistent with the repository's general rules for optional dependencies.
Suggested change
References
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -41,7 +41,7 @@ | |||||
| "3.13", | ||||||
| "3.14", | ||||||
| ] | ||||||
| ALL_PYTHON = UNIT_TEST_PYTHON_VERSIONS.copy() | ||||||
| ALL_PYTHON = UNIT_TEST_PYTHON_VERSIONS | ||||||
|
Contributor
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. Removing .copy() causes ALL_PYTHON to reference the same list as the UNIT_TEST_PYTHON_VERSIONS constant. The subsequent .extend(["3.7"]) call will modify the global constant in-place, which is likely unintended and could affect other sessions.
Suggested change
|
||||||
| ALL_PYTHON.extend(["3.7"]) | ||||||
|
|
||||||
| # Error if a python version is missing | ||||||
|
|
@@ -100,7 +100,7 @@ def blacken(session): | |||||
| @nox.session(python=DEFAULT_PYTHON_VERSION) | ||||||
| def mypy(session): | ||||||
| """Verify type hints are mypy compatible.""" | ||||||
| session.install("-e", ".[aiohttp,rsa]") | ||||||
| session.install("-e", ".[aiohttp]") | ||||||
| session.install( | ||||||
| "mypy", | ||||||
| "types-certifi", | ||||||
|
|
@@ -115,29 +115,16 @@ def mypy(session): | |||||
|
|
||||||
|
|
||||||
| @nox.session(python=ALL_PYTHON) | ||||||
| @nox.parametrize(["install_deprecated_extras"], (True, False)) | ||||||
| def unit(session, install_deprecated_extras): | ||||||
| def unit(session): | ||||||
| # Install all test dependencies, then install this package in-place. | ||||||
|
|
||||||
| if session.python in ("3.7",): | ||||||
| session.skip("Python 3.7 is no longer supported") | ||||||
| min_py, max_py = UNIT_TEST_PYTHON_VERSIONS[0], UNIT_TEST_PYTHON_VERSIONS[-1] | ||||||
| if not install_deprecated_extras and session.python not in (min_py, max_py): | ||||||
| # only run double tests on first and last supported versions | ||||||
| session.skip( | ||||||
| f"Extended tests only run on boundary Python versions ({min_py}, {max_py}) to reduce CI load." | ||||||
| ) | ||||||
|
|
||||||
| constraints_path = str( | ||||||
| CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt" | ||||||
| ) | ||||||
| extras_str = "testing" | ||||||
| if install_deprecated_extras: | ||||||
| # rsa and oauth2client were both archived and support dropped, | ||||||
| # but we still test old code paths | ||||||
| session.install("oauth2client") | ||||||
| extras_str += ",rsa" | ||||||
| session.install("-e", f".[{extras_str}]", "-c", constraints_path) | ||||||
| session.install("-e", ".[testing]", "-c", constraints_path) | ||||||
| session.run( | ||||||
| "pytest", | ||||||
| f"--junitxml=unit_{session.python}_sponge_log.xml", | ||||||
|
|
@@ -148,7 +135,6 @@ def unit(session, install_deprecated_extras): | |||||
| "--cov-report=term-missing", | ||||||
| "tests", | ||||||
| "tests_async", | ||||||
| *session.posargs, | ||||||
| ) | ||||||
|
Contributor
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. |
||||||
|
|
||||||
|
|
||||||
|
|
||||||
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.
Following the repository's general rules, use the more specific # type: ignore[assignment] when assigning None to optional dependencies in an except ImportError block to improve type checking precision.
References