8000 feat: setup.py: remove rsa requirement by ktdreyer · Pull Request #942 · googleapis/google-auth-library-python · GitHub
[go: up one dir, main page]
Skip to content

feat: setup.py: remove rsa requirement #942

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: setup.py: remove rsa requirement #942

wants to merge 1 commit into from

Conversation

ktdreyer
Copy link
@ktdreyer ktdreyer commented Jan 13, 2022 •
edited
Loading

The rsa library is slower and not as well-maintained as the cryptography library. Now that we require the cryptography library, drop the hard requirement on the rsa library.

Fixes: #941

@ktdreyer ktdreyer requested review from a team, arithmetic1728 and silvolu as code owners January 13, 2022 17:09
@ktdreyer ktdreyer force-pushed the require-cryptography branch 2 times, most recently from 2c6ceef to d406a4f Compare January 13, 2022 17:16
@ktdreyer ktdreyer changed the title setup.py: require cryptography instead of rsa feat: setup.py: require cryptography instead of rsa Jan 13, 2022
@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Jan 19, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 19, 2022
@ktdreyer ktdreyer force-pushed the require-cryptography branch from 91f3b2c to 39fe8ea Compare January 21, 2022 16:44
@arithmetic1728
Copy link
Contributor

I think simply removing the rsa dependency will break the library since it is still in use: https://github.com/googleapis/google-auth-library-python/blob/main/google/auth/crypt/_python_rsa.py#L28

@ktdreyer
Copy link
Author

Please see #646 , the code does not use python-rsa if python-cryptography is installed.

@parthea parthea mentioned this pull request Apr 5, 2022
Closed
1 task
@ktdreyer
Copy link
Author

This is still relevant because pip install google-auth still pulls in the old rsa library.

As I look at rebasing this today, I can remove the entry from setup.py, but it's now present in .kokoro/requirements.txt too. How do I make the change to that file?

@ktdreyer ktdreyer force-pushed the require-cryptography branch from 39fe8ea to 4894398 Compare February 21, 2024 16:18
@ktdreyer ktdreyer requested a review from a team as a code owner February 21, 2024 16:18
@conventional-commit-lint-gcf Conventional Commit Lint GCF
Copy link
conventional-commit-lint-gcf bot commented Feb 21, 2024 •
edited
Loading

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@ktdreyer ktdreyer force-pushed the require-cryptography branch from 4894398 to ed408cf Compare February 21, 2024 16:20
@ktdreyer ktdreyer changed the title feat: setup.py: require cryptography instead of rsa feat: setup.py: remove rsa requirement Feb 21, 2024
@ktdreyer
Copy link
Author

I researched kokoro to understand how to remove rsa from .kokoro/requirements.txt.

google-auth requires rsa, so we need to remove rsa from setup.py, then push a new release:

google-auth
  └rsa

The complicating factor is that .kokoro/requirements.in lists two modules that are not in setup.py: (gcp-docuploader and gcp-releasetool). These entries cause pip-compile to fetch google-auth=2.28.0 from PyPI. Because 2.28.0 still requires rsa, pip-compile still puts rsa into requirements.txt.

I think the solution here is to release a new google-auth version with this PR, then re-run pip-compile, and we should see the entry disappear from .kokoro/requirements.txt.

@ktdreyer ktdreyer force-pushed the require-cryptography branch from ed408cf to f31b1f5 Compare February 12, 2025 14:31
@prodigysml
Copy link

Any chance we can get some eyes on this one? There's a dependabot alert we can't get rid of, that would be fixed with this PR.

@miketheman
Copy link

This might need to be revived due to sybrenstuvel/python-rsa#245

@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels May 7, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 7, 2025
@parthea parthea removed the request for review from silvolu May 7, 2025 19:02
@parthea parthea requested review from sai-sunder-s and removed request for arithmetic1728 May 7, 2025 19:02
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels May 7, 2025
@miketheman
Copy link

I think the build failure can be resolved by adding the missing rsa package to

google-auth-library-python/docs/conf.py

Line 375 in 56b4d3b

autodoc_mock_imports = ["grpc"]

Ref: https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#confval-autodoc_mock_imports

That way, when sphinx autodoc traverses:

google.auth._jwt_async -> google.auth.jwt -> google.auth.crypt -> google.auth.crypt.rsa -> (sphinx follows fallback) google.auth.crypt._python_rsa -> import rsa

That final import to the missing rsa package will be ignored.

@ktdreyer ktdreyer force-pushed the require-cryptography branch from a354399 to 8c8c5af Compare October 9, 2025 17:16
@ktdreyer ktdreyer requested a review from a team as a code owner October 9, 2025 17:16
@ktdreyer
feat: setup.py: remove rsa requirement
f27b512 
The rsa library is slower and not as well-maintained as the cryptography
library. Now that we require the cryptography library, drop the hard
requirement on the rsa library.
@ktdreyer ktdreyer force-pushed the require-cryptography branch from 8c8c5af to f27b512 Compare October 9, 2025 17:18
@ktdreyer
Copy link
Author
ktdreyer commented Oct 9, 2025 •
edited
Loading

FYI https://pypi.org/project/rsa/ has been formally archived, so I think we should land this change soon. This PR will stop the rsa package from showing up in dependency chains for apps that depend on this.

Once we release that, we can have a separate PR to drop google.auth.crypt._python_rsa altogether.

@tiran
Copy link
tiran commented Oct 10, 2025

I would consider the use of rsa package as a security vulnerability. The rsa package implements the RSA algorithm with Python's built-in divmod function. divmod is not designed for cryptography and implements modulo division in a naive way. It's not constant timing, which makes rsa open to timing attacks (hello Bleichenbacher...).

rsa uses RSA blinding, but IMHO that's not good enough to address all potential timing attacks.

https://github.com/sybrenstuvel/python-rsa/blob/42b0e14ffbeeb9d99d1037e6440a2cc61780e4ea/CHANGELOG.md?plain=1#L78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sai-sunder-s sai-sunder-s Awaiting requested review from sai-sunder-s

At least 1 approving review is required to merge this pull request.

Assignees

@sai-sunder-s sai-sunder-s

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

require cryptography in packaging metadata (and remove rsa)

8 participants

@ktdreyer @arithmetic1728 @prodigysml @miketheman @tiran @sai-sunder-s @parthea @yoshi-kokoro
0