rust-lang / crates.io

The Rust package registry
https://crates.io
Apache License 2.0
3.01k stars 603 forks source link

security: Recovering from a GitHub account compromise #2630

Open jsha opened 4 years ago

jsha commented 4 years ago

Right now, if an attacker compromises a crate author's GitHub account, they can use it to log into crates.io and get a session cookie. That cookie is signed by an HMAC key owned by crates.io, and contains the user ID. During authentication, crates.io verifies the HMAC on that cookie and then trusts the user ID inside. The problem is that, once the crate author regains control of their GitHub account, the attacker's session cookie is still valid, and as far as I can tell there is no way to revoke it. There are a few possible solutions:

I think (1) is the best option but also the most work.

Another, closely related issue: If a user revokes the crates.io app on their GitHub account (for instance, as part of the cleanup after a compromise), it would be nice to temporarily disable all cookie sessions and tokens on their crates.io account until they reauthorize crates.io.

(Related: #815)

Turbo87 commented 3 years ago

@jsha FYI we shortly discussed this during our weekly team meeting and we're considering to implement the session table suggestion. would you be available for reviews once we have something that works?

jsha commented 3 years ago

Excellent! Yes, I'd definitely be happy to help with reviews.

jsha commented 3 years ago

Another item in this area: It might be good to re-verify the GitHub OAuth token at publish time. If the OAuth token has been revoked, the publish should fail.

Shnatsel commented 3 years ago

There technically is a way to recover: the HMAC key can be changed by crates.io. But that invalidates all the cookies for all users, and can only be performed by the crates.io team.

adsnaider commented 2 years ago

Hello! Jumping in as this issue is important to folks at Google (myself included). I've gone ahead and created a pull request (#4690) that addresses the issue.

Disclaimer: I used #3307 to help me get started, so thank you very much @Turbo87 for the help :)

The PR doesn't address some issues that I left opened as TODOs, here's he list of the next steps (assuming the PR gets merged)

Feel free to add more and/or correct me if I'm wrong about any of the above items.