rubygems / rfcs

RubyGems + Bundler RFCs
45 stars 40 forks source link

RFC: Require MFA on the top 100 most downloaded gems #36

Closed jenshenny closed 2 years ago

jenshenny commented 2 years ago

Here’s the RFC that was mentioned in this issue on a policy to require MFA on a cohort of gems.

Contributors: @bettymakes, @aellispierce, @jchestershopify, @jenshenny

We’re all available on the Bundler Slack to discuss this also!

jenshenny commented 2 years ago

Even though Shopify will be able to commit a certain number of developers to work on this policy if it's accepted, we acknowledge that there is time that needs to be spent by the Rubygems maintainers to oversee the contributions and policy changes. We would be happy to discuss ways to reduce this time and workload as much as possible.

simi commented 2 years ago

I went just briefly thru the RFC and I must say it is really nicely written and it makes total sense. The only problem I see is scope of the top 100 most downloaded gems. That is rolling scope. Gems can go in and out over the time.

Instead, I think we can make the threshold based on some metric and only add new gems to the scope. For example we can find threshold of total downloads for current top 100 gems and make it the rule.

When total amount of downloads of gem reaches XY, we will enforce MFA to publish new release. We can also ping the authors not using MFA in advance when 90% (or any reasonable part) of threshold is reached.

mensfeld commented 2 years ago

@simi I though about the same. Since it's a supply chain issue, if we decide to "2FA lock" a package, its dependencies should be locked as well. Otherwise we leave weak links in the chain for potential exploitation.

I am aware, that the dependency states change across the versions, so I would suggest locking the mostly used versions (if we have those metrics) based on one last year + any newly introduced.

This may also require monitoring of changes introduces to the dependency tree of "2fa locked" packages. I am willing to help with that as I have extensive experience in mining this data.

rst commented 2 years ago

Agree that 2fa-locking dependencies of the "top 100" makes the policy a whole lot more effective. But that puts restrictions on the maintainers of those top 100 packages which could get awkward for them. Most notably, they'd have to be barred from adding a new dependency until its maintainer got 2fa. It's probably worth thinking through the workflow here, for all parties, and making sure it lands in an acceptable place.

jchestershopify commented 2 years ago

Since it's a supply chain issue, if we decide to "2FA lock" a package, its dependencies should be locked as well. Otherwise we leave weak links in the chain for potential exploitation.

We do, yes.

The goal here is really to get the ball rolling. Top 100 most-downloaded is something of an arbitrary selection, so if we have a better heuristic (eg download thresholds as suggested by @simi), that would be fine too. So long as we start, stabilise and then continue to make steady progress towards high MFA coverage.

tiegz commented 2 years ago

Love this!

Do we know how effective locking access to Rubygems will be as an enforcer here? e.g. how many packages in the top 100 aren't maintained regularly, which would prevent the maintainer from being aware of the lockout?

Any thoughts on publicly exposing the MFA status on the gem page (or maybe the rubygems_mfa_required status since it's already public?)? Obviously it makes the gem more of a target and I'm not aware of another platform that does this, but it could also prompt consumers of gems to urge the maintainers to enable it.

tiegz commented 2 years ago

(or maybe the rubygems_mfa_required status since it's already public?)?

Woops, nevermind, just noticed this is already public! 👍🏻

jchestershopify commented 2 years ago

A nitpick that I just noticed: the filename should probably start with 0007- instead of 0000-.

mullermp commented 2 years ago

Hey there, I've updated the the awscloud account to have MFA with the "UI and gem signin" level. Using MFA for API is a bit tricky because of CI/CD and would require larger effort. Also, keys scoped to each gem might be infeasible for us because 1) we maintain many gems, 2) we often need to access all of them at the same time, and 3) add generate, store, and retrieve new ones for each service launch.

@hsbt - I got your request from Alex Wood who no longer works on AWS SDK For Ruby. Feel free to create an issue on the main repo if you need us :D

jenshenny commented 2 years ago

The CI/CD case is definitely tricky. Enforcing keys to be scoped to a gem on a non-MFA API key might be too restrictive as you said @mullermp, and a better alternative would need to be thought out. The Shopify Rubygem account is in a similar situation. With that being said, requiring users to have the UI and gem signin level is a good first step for this path.

jenshenny commented 2 years ago

The only problem I see is scope of the top 100 most downloaded gems. That is rolling scope. Gems can go in and out over the time.

Instead, I think we can make the threshold based on some metric and only add new gems to the scope. For example we can find threshold of total downloads for current top 100 gems and make it the rule.

To come back to this @simi, in the recommended prototype, to address the issue of gems going in and out of scope is to run a task to set a date for mfa enforcement for each user that fits in the metric. This task could be ran periodically to capture new gems entering the scope. According to the proposal, we selected a 6 mo period for the top 100 most downloaded gem accounts (can be changed). Would that approach work well?

Based on your suggestion of finding a threshold of total downloads for current top 100 gems and make it the rule, I assume the alternative would be to check if it reaches this threshold when a gem download is incremented?

When total amount of downloads of gem reaches XY, we will enforce MFA to publish new release. We can also ping the authors not using MFA in advance when 90% (or any reasonable part) of threshold is reached.

From the point of when we set the date on enforcement on a user, they will get messages on the CLI and UI to set up MFA until the required date is reached. Do you think it's just sufficient enough to just ping the user (I'm guessing via email) instead of these prompts?

Let me know if you want me to add a description of the technical approach in the md instead of having them in the linked prototype PR descriptions.

simi commented 2 years ago

What to do when user rejects to use MFA? Does it mean user is going to lose access to releasing gems?

What about to keep the possibility to release gem, but mark the gem as "unsafe" and let the end-user know during installation this gem was not released in secured way? That way we don't need to enforce users, but we will inform end-users, which can go and ask maintainer to release in secure way. Clearly there will be warning during gem push (we can also make it fail in later phase without explicitly passing --insecure flag to gem push for example).

By my experience, enforcing MFA (or actually enforcing anything in general) is painful process for users. Making it optional, but little more "painful" when not used seems like better option to me.

jenshenny commented 2 years ago

What to do when user rejects to use MFA? Does it mean user is going to lose access to releasing gems?

In the current state, yes. You raise a good point that users can’t release gems especially in an emergency situation.

At this moment, I’m open to the possibility of marking unsafe on gems. This is something we can do for the entire rubygems population vs a cohort (it would be confusing if 1. a gem without mfa in a subset is marked unsafe, vs 2. others that aren’t in the subset, doesn’t have mfa but isn’t marked unsafe).

However, from this discussion, I’m not sure if this will encourage shaming if we want to inform end users during installation especially for a highly downloaded gem, though I guess it might be warranted if prior notice was given. It could do more harm than good and prolong adoption. I’m leaning towards using this solution for the general population and catch gem dependencies to enable MFA, but still require MFA on the most frequently used gem user accounts if adoption isn’t high enough before marking things unsafe.

jchestershopify commented 2 years ago

I agree with @jenshenny -- I don't think a warning and shaming will significantly move the needle. Most end users are busy and almost all of them are CI/CD robots who don't read warnings.

What I'd like to do is reframe how we think about security issues like MFA. I think end users should be at the centre of every decision and their needs should be put first.

For example, consider an account takeover. For a gem maintainer, an account takeover is distressing and embarrassing. But for end users, it is potentially catastrophic. And end users outnumber maintainers by a large ratio. In terms of a calculus of harm, I think end users of gems deserve to be given priority. That means that they need maintainer accounts to be secured with MFA.

jenshenny commented 2 years ago

NPM announced that they are requiring MFA to the top 100 npm package maintainers that started last week, and rolling it in phases to all accounts by March. I believe Rubygems should also follow NPM's lead and follow a similar policy and rollout. If a maintainer doesn't want to enable MFA, we can make it a bit more "annoying" by making them enter an OTP sent to their email on the UI and gem signin.

schneems commented 2 years ago

My experience

I've been using MFA on all my gems https://rubygems.org/profiles/schneems for several years now. My employer, Heroku, required it. I think it's a nice layer of protection and while it does add another step to releasing, it's (to me) a small one.

I release via rake release with the bundler release tasks. It prompts me for a MFA code. I've got that set up on my laptop already so I open an app, put in a passcode, type in "ruby" copy and paste the MFA in and then I'm done. I could probably automate it more to require fewer steps, but I've not (personally) felt the need.

My opinion

It's my opinion/experience that MFA is a large bang-for-your-buck and is already expected in many/most places.

I would like us to be "secure by default" as much as possible.

There are some implementation questions around how to get there. If someone "rejects" the requirement to use MFA they can run their own RubyGems server to host their own gems and release in that way. If a gem says "add this other source to your Gemfile" that's at least a traceable artifact where someone could visually audit the Gemfile and identify all secondary sources that might not be as secure.

I would support a rolling process for making all gems MFA as it only takes one broken link to have a supply chain attack. Starting at the first 100 most popular is a good step, but doesn't go far enough (again, my opinion). If there's an escape valve of "run your own gemserver if you really hate it that much" I think that handles edge cases. I do think it's a good idea to think about the edge cases, but as long as there is at least one escape valve that covers them, I don't know that we need to overly go out of our way to try to please everyone.

I think with security stuff it's a balance between things that reasonably reduce attack surface area versus usability. One example of this going wrong might be a company that aggressively enforces password rotation frequently and then employees start writing their password down by their desk because they don't have enough time to memorize it. It's important to think on possible unintended consequences, but we also will not come to a case of 100% consensus of all cases perfectly handled.

sonalkr132 commented 2 years ago

I assume the alternative would be to check if it reaches this threshold when a gem download is incremented?

I think using the download threshold would be more straightforward. https://rubygems.org/gems/sinatra is the 100th gem, we can use its count as the threshold.

Looking at top 100 gems pusher (of latest_version) MFA status, I can see that 72 already have MFA level in ui_and_api and ui_and_gem_signin. 11 don't have pusher, because they haven't had a release since started we storing pusher https://github.com/rubygems/rubygems.org/pull/1954/. This means proposed changes here only affect 17 users 10 unique users. I can share the list privately to someone here if they want to reach out to these users privately to check their use-case (through twitter?) . Given that we already have good adoption, I think we should shrink the timeline. Start showing the warning to these users ASAP and enforce by july-aug 2022. I would also add writing a blog post explaining the changes and perhaps adding a job to send email reminders of coming changes to affected users.

sonalkr132 commented 2 years ago

I also want to highlight this post https://thehackerblog.com/zero-days-without-incident-compromising-angular-via-expired-npm-publisher-email-domains-7kZplW4x/ We have checks in place to mitigate it however it is not full proof, MFA is the ultimate solution.

hsbt commented 2 years ago

I'm targeted maintainer for this proposal. I understood the security is an important part of language ecosystem. So, I enforced to use MFA authentication in https://github.com/ruby and https://bugs.ruby-lang.org/.

I'm frustrated to enforce something for my OSS work from a big tech company's order. But I agreed to this proposal.

jenshenny commented 2 years ago

@simi @sonalkr132 Bumping this up to see if you have any feedback on the above changes.

simi commented 2 years ago

Bumping this up to see if you have any feedback on the above changes.

Thanks for the ping @jenshenny.

I'm frustrated to enforce something for my OSS work from a big tech company's order. But I agreed to this proposal.

I do hear you @hsbt. I'm thinking how to prevent this kind of feeling for other gem owners as well. If I understand it well, nothing is being enforced for now, right? We need to ensure that "enforcing" part would be smooth as possible to not make anyone feeling this way.

I think we basically all agree on phase 1 and 2. I was thinking how to move this forward. What about to split this RFC into two parts? Preparation + recommended part (phase 1 and 2) and "enforcing" part (phase 3 and 4). That way I think we can consider first part as agreed and merge this part of RFC. We can still do some small tweaks in there, but the overall goal would stay the same.

And for the second part I suggest to open separated RFC (we can scope this one only to first part) and we can continue in there. I think we can keep the dates open unless we find smooth solutionfor enforcing.

WDYT @jenshenny @hsbt @sonalkr132 @jchestershopify @tiegz @schneems @mensfeld @rst?

sonalkr132 commented 2 years ago

I am unsure about part 4 as well. We don't need to enfore mfa on gems if they are just personal projects. Anything below 20k downloads would fall this category for me (includes 83% of gems). I think we can go ahead with part 3 altough it doesn't mean much without same check on dependencies. Perhaps we should check if we were enfore this only top 100 and all their transitive runtime dependencies, how many gems/users will it affect? Asking to enable to MFA for user:password login seems like an acceptable ask to me, given that it won't affect CI pipelines and we already have decent adoption in top 100.

mensfeld commented 2 years ago

@simi I agree with the enforcement but I also see @hsbt point of view though I disagree with it. I think that 2fa in the case of popular packages should just become a standard.

I think improvements to the gem publishing flow as well as recommendations should be implemented as soon as doable for the benefit of the ecosystem. So phases 1 and 2 are :+1: from me for sure.

@sonalkr132

Anything below 20k downloads would fall this category for me (includes 83% of gems).

Then we have a case of new dependencies coming to the supply chain while not being popular (yet). For example, when faraday-rack was published not long ago (last year) it immediately became transitive dependency for many projects while still being (for a short period of time) < 20k. Projects are merged and splitted all the time and download count may not be the best metric for new packages being introduced into the chain.

Maybe we should make it not based on downloads but rather based on a collective dependency tree popularity? That way we would protect the whole supply chain for popular components (collectively) while not introducing any burden to private projects.

jenshenny commented 2 years ago

I hope we can also go ahead with phase 3. With its current adoption in the top 100, the small number of users to enforce this policy on, and the positive impact to the many consumers using these gems, it's a fair ask for users to enable MFA for UI and gem signin.

I am unsure about part 4 as well.

Phase 4 was intended to be vague and convey that a rollout wouldn't stop at just the top 100. There was a gap in context to determine whether it made sense to make it required by default or if they had to reach certain requirements. I clarified in Phase 4 that the details of this would be discussed in another RFC. From the discussion here however, I imagine using a download threshold is easier than using a dependency tree. Perhaps we can go with a download threshold and later refine it to use a dependency tree if it's valuable to.

jchestershopify commented 2 years ago

From the discussion here however, I imagine using a download threshold is easier than using a dependency tree.

That's my belief also. They'll be fairly well correlated in the long run, even if in the short run (such as faraday-rack) there could be outliers.

mensfeld commented 2 years ago

@jchestershopify It's not often that the chain is long. Calculating 1 or 2 previous links for the "forcing 2fa" would be trivial for new packages. For older downloads count would be enough. Even with the current setup of rubygems it would require at most 2 additional calls to the db (I may be wrong on it though) for this data and only in case of new versions of packages that are below threshold.

sonalkr132 commented 2 years ago

In my experience, dependency chain is not as easy to calculate. It would inevitably need recursion and we won't be able to add it for "on the fly" check. You also need to consider (all?) versions, for example:

Do we inforce mfa requirement on B even if latest of A doesn't require B? Do we inforce mfa on 0.9.x release of B as well even if it is not used by A? If we add an offline process to mark gems for mfa requirement, how do we ensure that we don't have flapping issue simi brought up.

jenshenny commented 2 years ago

@simi you suggested to only merge phases 1 and 2. How do you feel about merging the RFC in its current state with part 3 included since there’s support in doing so in subsequent comments?

Are there any other blockers that are noticeable regarding merging the RFC in this current state (with phase 4 discussed in another RFC)?

Perhaps we should check if we were enfore this only top 100 and all their transitive runtime dependencies, how many gems/users will it affect?

Using this script (which I hope is correct) with today's data dump, the total number of dependencies it'll affect would be ~750. Only counting runtime dependencies, there’s 425 and the least downloaded gem is aws-keyspaces which was created a week ago. The one after has more than 850 000 downloads. If we continue this rollout on a rolling basis and stop at the download threshold for personal projects, I think it would work well.

simi commented 2 years ago

@jenshenny thanks for moving this forward. RFC looks good to me now. Since phase 4 would be specified later in separate RFC and for phase 3 there is just estimate date (and also since nothing is set into stone even after we merge this) I'm ok to merge this.

To follow the process (https://github.com/rubygems/rfcs#what-the-process-is), should we call for final comment period? @indirect @sonalkr132 @deivid-rodriguez any idea?

sonalkr132 commented 2 years ago

We have not followed the process by the book until now. Generally we just merge if everything looks good. I would be okay with merging this as is.

simi commented 2 years ago

OK, let's do it.

simi commented 2 years ago

:partying_face:

ioquatix commented 5 months ago

Eric, the maintainer of Unicorn, has said that it is hard to release updates to Unicorn as he is unable to use MFA (* this is my best interpretation of the current situation).

This might be a net security problem if maintainers can't release security updates to gems, especially popular ones. That being said, I don't know of any other maintainer who is in this position.

As a compromise, what about allowing some users to opt out of MFA requirements? i.e. opt in by default as is current behaviour, but allow explicit opt out. We can also document that clearly, e.g. "This gem has opted out of MFA security."

simi commented 5 months ago

@ioquatix is there anything we can do to help release Unicorn? Is there any info why current MFA options are not usable for given usecase? I think it would be better to improve MFA and make it easier to adopt instead of adding additional feature to bypass the requirements.

indirect commented 5 months ago

One easier to use type of MFA that we have not (yet?) implemented is: send a code in an email, request the code to be entered. Since RubyGems.org accounts already require an email and password, it seems like that should be usable by anyone who already has a RubyGems.org account. @ioquatix do you know if that would suffice?

ioquatix commented 5 months ago

We'd have to contact Eric to see what would work for him.