Skip to content
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

Support npm auth-and-writes 2FA setting #93

Open
pvdlg opened this issue Jul 30, 2018 · 10 comments
Open

Support npm auth-and-writes 2FA setting #93

pvdlg opened this issue Jul 30, 2018 · 10 comments

Comments

@pvdlg
Copy link
Member

pvdlg commented Jul 30, 2018

Follow-up to #92.

It should be possible to support npm auth-and-writes level of two-factor-authentication by generating the OTP on the CI with a tool like otplib.

Users would have to enable npm 2FA via command line and on the step where the QRCode is displayed, copy the secret key displayed below it (Something like Or enter code: XXXXXXXX....).
The secret would have to be added on the CI as a secured environment variable (e.g. NPM_2FA_SECRET).

The only constraint is that if 2FA is already enabled, there is no way to retrieve the secret key. So users would have to disable 2FA and re-enable it via CLI to obtain a new secret key.

Then the plugin would generate the OTP with:

const {authenticator} = require('otplib');

const otp =authenticator.generate(process.env.NPM_2FA_SECRET);

And call npm publish with the --otp option.

We would also need to implement #11 and make sure that if the auth-and-writes option is enabled the environment variable NPM_2FA_SECRET is set. Unfortunately it doesn't seems there is a way to verify if NPM_2FA_SECRET is correct, as all the npm command that requires an OTP are things that makes changes server side, so we can't use them to test.

@travi
Copy link
Member

travi commented Jul 30, 2018

this approach would concern me a bit. considering a worm like was attempted with eslint, the environment variables could be stolen just as easily as the token. while this is a creative way to make this work, i'd really hesitate to expose the 2fa secret to anything other than my 2fa app, such as google authenticator or authy because of opening up the account to further risk.

imho, i think a better approach would be to have the ability to require 2fa for auth-and-writes, but exclude very specific tokens, such as the ones used by semantic-release. since semantic-release is intended to be run from CI, limiting the CIDR ranges allowed for that token should be enough to make up for that exception. obviously, npm does not support 2fa enforcement exceptions, but i've mentioned the idea to the team. maybe a few more voices around that topic would help reinforce the importance of the automated use case.

@travi
Copy link
Member

travi commented Jul 30, 2018

i'm actually interested in thoughts that the npm security team might have in this area. @evilpacket, would you or someone from you team be willing to discuss this use case in more detail? i think it would be a big help those of use that publish automatically from a non-interactive CI service to make sure we are considering what your team might already have in mind. (is there a better place for that conversation than this issue thread?)

@pvdlg
Copy link
Member Author

pvdlg commented Jul 30, 2018

I didn't know you could limit the source IP with --cidr. That's a great point and we should at least mention it in the documentation.
However anyone can run anything on a Travis image. So if your token is compromised that won't help you much, as the person having your token can just create a Travis job to publish with the token.

In the proposal above nothing would change if you don't enable 2FA auth-and-writes for the user associated with your token. semantic-release would look for NPM_2FA_SECRET only if the npm account associated with the token has 2FA enabled with auth-and-writes.
So you could continue to use semantic-release with npm the way you do now.

Finally the point of enabling 2FA with auth-and-writes is to prevent letting a hacker to publish a package when your password (not your token) is compromised.
Having NPM_TOKEN in a Travis variable is the same as having NPM_TOKEN + NPM_2FA_SECRET. If your Travis variables are compromised then in both case your account is compromised. There is no way around that as we want Travis to be able to publish.

As a recap:

  1. With NPM_TOKEN on Travis and 2FA auth-only:
    • If your Travis variables are compromised, then the hacker can publish a new version of your package
    • If your password is compromised, then the hacker can publish a new version of your package
  2. With NPM_TOKEN and NPM_2FA_SECRET on Travis and 2FA auth-and-writes:
    • If your Travis variables are compromised, then the hacker can publish a new version of your package
    • If your password is compromised, then the hacker cannot do anything

So the solution 2 is still a lot better as it protect you if your password is compromised. This is what happened to the ESLint contributor. And this is a lot more likely to happens than having Travis variables compromised as users tends to have weak passwords and re-use the same one everywhere.

@travi
Copy link
Member

travi commented Jul 30, 2018

However anyone can run anything on a Travis image. So if your token is compromised that won't help you much, as the person having your token can just create a Travis job to publish with the token.

agreed, but the difficulty of using the token is significantly reduced. while it is not difficult to run on a travis instance, it does take more work than just using the token on the attacker's machine. it also puts the attacker at more risk of being discovered using the stolen token. even beyond all of that though, the attacker needs to somehow know what the cidr restriction of the token is, which would require them to have tracked where they stole the token from (i just verified that npm token ls responds with a 403 if used a cidr restricted token is used outside of the IPs defined in the restriction).

So you could continue to use semantic-release with npm the way you do now.

this would be what I would probably do due to the risk of possibly exposing the 2fa secret. however, the reason i raised the question was that it seems to me like a severe enough risk, even if unlikely (though the eslint worm could have easily grabbed this info if it was looking for it), that we may not want to enable/encourage the potential of the leak.

If your Travis variables are compromised then in both case your account is compromised.

yes and no. as mentioned above, the attacker would somehow have to know what CIDRs are allowed for the token (hard), have access to run on an instance within the restriction (simple for travis, but maybe impossible in other cases), and actually run on said instance (more work and potentially higher risk for the attacker).

If your password is compromised, then the hacker can publish a new version of your package

So the solution 2 is still a lot better as it protect you if your password is compromised.

2fa for auth only still provides much of the same protection, at least if the token used by semantic-release is CIDR restricted. even if you have someone's password and can log into their account, you cannot create a new token for the account without entering the one-time token (this must have been changed after the eslint incident). unless an attacker gets access to an existing token and that token is not CIDR restricted or the attacker can figure out how to use it within the restriction, the password alone does not give them the ability to publish.

this gets better if we can convince npm to allow enabling 2fa for auth-and-writes, but exclude certain tokens (maybe only if they are CIDR restricted). even better than that, i'd love to only enable our bot user to even be able to get publish tokens, only allowing read-only tokens for everyone else. pre-releases are the only reason we publish anything outside of semantic-release today, and you are already working on supporting those use cases.

@travi
Copy link
Member

travi commented Aug 1, 2018

i've started an RFC related to the token 2fa rules i mentioned above. still needs work, but wanted to add visibility here. feel free to provide thoughts there if it will help here.

@dominykas
Copy link
Contributor

dominykas commented Apr 27, 2019

Would you accept a PR to add an option for otpUrl? The implementation would fire a GET request to the URL, and use the response as the OTP.

The implementation of the server can, e.g., send a push notification to a mobile phone, and the phone can then respond with the OTP if the user permits that. I know a sample server is in the works, I don't know when it's going public, but I'd like to start testing it :)

A more flexible approach could be an option for otpCommand (which would exec a command, e.g. curl or whatever floats your boat). A less flexible approach would be to rely on an env var (which means another plugin could set it; setting it before calling semantic-release is probably a bad idea, as things may take long enough for the OTP to expire).

@dominykas
Copy link
Contributor

dominykas commented Jun 17, 2019

Getting back to this - there is now a PoC server for remote OTP requests: https://github.com/nearform/optic

How it works:

  1. It's a web app which allows you to store a local OTP secret to generate OTP codes (the easy bit)
  2. There's an API which allows you to "request" a specific OTP
  3. The client receives a notification and can approve/reject

All secrets always stay on the client (i.e. your phone).

You can try it out, I've deployed it here (no SLA): https://optic.goodnight.to

  1. Use Chrome/Chrome for Android
  2. Sign-in - only Google for now, mostly due to lack of time - it's a PoC after all
  3. Allow notifications (that's the whole point, right)
  4. Add a secret (go for manual, if you're just playing, enter whatever you like in Issuer/Secret)
  5. Click the + to generate a token
  6. You should now be able to curl -s https://optic.goodnight.to/api/generate/[your token]
  7. When you hit the URL, a notification should appear in your browser
  8. Hit "Approve" - the curl will now receive a response with the currently active OTP

I'm more than happy to answer any questions or jump on a call to give a quick demo and build a PR to support the concept. This brings a true 2FA for fully automated semantic-releases.

There's gotchas and caveats and more work to be done, but I figure it's a start?

@travi
Copy link
Member

travi commented Jun 17, 2019

The client receives a notification and can approve/reject

not suggesting that this is a problem for all use cases, but i would still find this a step backward. a big part of the value of semantic-release for me is that it can publish new versions while i'm sleeping, etc. needing to interact even to approve/reject removes the ability to automate to that level.

@dominykas
Copy link
Contributor

not suggesting that this is a problem for all use cases, but i would still find this a step backward

I very much agree with your suggestion to have tokens which are non-2FA (or rather to limit their use to a CIDR range)!

However, that is not always enough. There's different levels of security required for different contexts. There's also different ways to implement this (e.g. you could have a 2FA service which automatically responds with an OTP without a user interaction).

@dominykas
Copy link
Contributor

Opened a PR for otpUrl option: #176, tested using https://github.com/nearform/optic on one of my own packages.

Still not sure how to assert in tests that --otp was passed through - possibly need to add a proxy around the couch app - ideas welcome.

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

No branches or pull requests

3 participants