-
Notifications
You must be signed in to change notification settings - Fork 19
Add documentation of security implications #140
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
Conversation
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.
Thanks, great idea to add this!
Another area to cover could be the security of the upload process itself: what are we doing to prevent an issue where a malicious party could gain the ability to upload stub packages using the uploader's identity? (Relevant real-world example: https://blog.pypi.org/posts/2024-07-08-incident-report-leaked-admin-personal-access-token/.)
Co-authored-by: Jelle Zijlstra <[email protected]>
Ironically, I forgot to ping @Akuli, although it was #138 that prompted me to write this. Sorry! @JelleZijlstra What can we do about that? In a related matter: I can't check, but do we require 2FA to be enabled for this org/repository? If not, we should definitely enable it. |
I am 90% sure I enabled it a while ago, I will double-check now. |
Haven't thought too much about it, but it looks like stub packages are owned only by typeshed-bot (e.g., https://pypi.org/project/types-requests/). That's good because it means that if someone gains access to my PyPI account, they can't use it to upload a malicious stubs package. However, we should also review what kind of authentication typeshed-bot uses to upload packages. |
Actually it turns out "Require two-factor authentication for everyone in the typeshed-internal organization" was off. I just turned it on. (I also checked that all four members have 2FA enabled ;-)) |
It uses a PyPI token. FWIW we can rotate it from time to time. (There is even a GHA to test that PyPI token works.) |
We should probably document the typeshed-bot user in this file as well. Commits to the PR or text suggestions in the comments welcome. Otherwise I will look into this tomorrow. |
Since it's likely that a stub package gets installed alongside the | ||
upstream package, this does not introduce an additional security liability. | ||
* Dependencies that are explicitly allowlisted in the stub uploader. These | ||
dependencies are vetted to be from a trusted source. |
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.
It would be nice to expand upon this and actually describe and explain the possible attack in #61 (comment). This kind of hints at how the attack goes, but IMO does not describe it in enough detail. It is the biggest security issue in stub_uploader I'm aware of, and typeshed maintainers need to be aware of it.
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.
I'd like to hold off documenting this here at the moment. I kept this section quite vague for the moment, mostly because I think we should resolve and implement #90 first and can then discuss the measures and reasons in this document.
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.
Maybe:
dependencies are vetted to be from a trusted source. | |
dependencies are vetted to be from a trusted source. | |
Having one global allowlist is not ideal, because it allows somewhat obscure | |
dependencies to be added to popular stub packages (their security matters | |
the most). This may be changed in the near future. |
I got nothing to add. You covered everything I was already aware of 👍 |
Co-authored-by: Jelle Zijlstra <[email protected]>
Are there any more notes? |
Co-authored-by: Alex Waygood <[email protected]>
[typeshed_bot](https://pypi.org/user/typeshed_bot/) using an API token. | ||
The packages are owned solely by that user, so that no other user can | ||
upload new versions of the packages, in case the account of a | ||
maintainer becomes compromised. |
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.
Couldn't a maintainer account simply edit CI config to store/send the token somewhere? Or is this about maintainers who do not have commit access, like me?
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.
I think it's about the PyPI accounts, not GitHub accounts. If a GitHub account with commit access (e.g., mine) gets compromised, it could obviously be used to upload a malicious package just by changing the code. But if someone compromises my PyPI account, they can't use it to upload malicious stub packages. (They could use it to upload a malicious version of typing-extensions, which is probably worse, but that's a different repo than this one.)
Co-authored-by: Akuli <[email protected]>
The maintainers section is still missing, but the remaining sections are up for scrutiny.
Cc @AlexWaygood @hauntsaninja @Avasam