Crate quarantine

[LawnGnome]

/cc @rust-lang/crates-io, @rust-lang/cargo, @rust-lang/mods

Rendered

[tarcieri]

I've been interested in a similar feature for internal use within a team of crates.io end users that releases crates: they could be configured to be immediately quarantined upon publication until approved by one (or potentially k of n) additional publisher beyond the person who published it before it goes live.

Such a 2-person (or more) system would prevent the compromise of a single publisher's credentials from translating directly to publication of a malicious crate. This would be useful for high-value crates. It would also help secure systems where crates are published by a release automation, then approved by a human/k human(s).

I'm curious if the design of the quarantine system could have an associated policy about how a quarantined crate can be published, i.e. approval of other crate publishers versus approval by crates.io administrators.

[epage]

I've been interested in a similar feature for internal use within a team of crates.io end users that releases crates: they could be configured to be immediately quarantined upon publication until approved by one (or potentially k of n) additional publisher beyond the person who published it before it goes live.

Quarantining for extreme cases is easier to deal with imo. When it becomes expected, things get a lot more complex if we also want to handle cases like rust-lang/cargo#1169

[tarcieri]

@epage if a quarantine system were available to end-users, it could be used to implement rust-lang/cargo#1169.

You could stage all of the releases in quarantine, then set them live at the same time.

[epage]

Yes, we could have sandboxed indexes that these quarantined packages go to.

That requires a lot of more design and implementation work than this RFC proposes.

[tarcieri]

I think all it requires out of the gate is attaching an AuthZ policy about how a crate can be un-quarantined/published to each quarantine state.

Arguably if you only have one AuthZ policy you don’t need that. It can be backfilled later.

I just hope you can keep the design flexible enough that it could potentially accommodate these features in the future.

[epage]

As a future possibility, I think its worth it. We just need to recognize that is likely not a simple addition on top of this. The design work alone would need to examine the whole publish pipeline and address parts at each stage (cargo publish, crates.io API, crates.io index and .crate storage, the local index and crate storage, etc).

[LawnGnome]

I've been interested in a similar feature for internal use within a team of crates.io end users that releases crates: they could be configured to be immediately quarantined upon publication until approved by one (or potentially k of n) additional publisher beyond the person who published it before it goes live.

That definitely sounds like an interesting area to explore! Consensus based peer approval is something that might go very well with distributed reviews in cargo vet, or other similar tooling. And, of course, one of my concerns in this RFC is workload on the crates.io team, so being able to distribute that more widely would be beneficial in the long run if this mechanism is used for more than just dealing with immediate spam attacks and bad actors.

That said, as @epage said, I'd prefer to keep this RFC focused on those more prosaic concerns right now — I don't believe anything in this RFC would prevent future work (and, indeed, simply decoupling the concept of a crate version being available in the index from running cargo publish will help significantly there), but this is largely around getting the crates.io team the tooling we need today, rather than trying to design a full review/quarantine system that is all things to all people for the future.

[ijackson]

Under this proposal, quarantining a crate could very seriously complicate publication of a multi-crate workspace (or other kind of "deep stack"). Rust wants us to make smaller crates for separate compilation reasons, but cargo then insists that these crates which can be conceptually part of a single program must be published separately. For release management to be practical, there must be a way to publish a workspace even if some of the involved crates end up quarantined.

This does not seem to be dicussed at all in the RFC. IMO it should be.

Solutions to this problem that occurred to me:

• Allow publication to refer to quarantined dependencies, linking the new crate to the quarantined of those dependencies. (This seems like it would be a lot of work.)
• Allow overriding of the "dependencies actually exist" check. The user would be expected to retry the publication of the failed crate(s) with the new flag. (This seems easy, but the resulting UX is poor.)
• Allow overriding of the "dependencies actually exist" check. Automatically apply this when a dependency doesn't exist but might be be resolvable using quarantined uploads. (This needs a way to check for the existence of a quarantined crate.)
• Do the server side "dependencies actually exist" check after quarantine review,; automatically quarantine any crate that has a quarantined dependency. (This may be doable; the UX will be poor, since the user would need to redo with --no-verify)
• Define circumastances where we promise not to apply quarantine. (But it seems like it would be difficult to define rules that wouldn't be exploitable.)

[ijackson]

Another thing that needs to be addressed in the RFC:

When a new crate is quarantined, the name must not be visible via any public API. Otherwise it would be possible for a malicious actor to see these, and engage in "front-running", grabbing the crate name with a crate tailored to not be quarantined.

(The DNS suffers badly from front-running, where that malicious practice can offer benefits that are valuable to the perpetrators. We should try to avoid creating similar sets of incentives.)

[Nemo157]

When a new crate is quarantined, the name must not be visible via any public API. Otherwise it would be possible for a malicious actor to see these, and engage in "front-running", grabbing the crate name with a crate tailored to not be quarantined.

This is not possible:

A version in the quarantined state [..] will appear in crates.io API responses as an unpublished crate, but will not be added to the index [..]

By being part of the API the crate has already been registered in the database as owned by the publishing user, and the published version registered on that crate. It's simply the index metadata telling cargo about the crate that hasn't been updated yet.

[Turbo87]

Suggested change

	Once a crate version has been quarantined, then a member of the crates.io team will review the quarantined crate, ideally within 1 week[^sla]. Once reviewed, the crate may be either approved and published to the index, or rejected and deleted. Either way, the crate owner(s) will be notified by e-mail, assuming crates.io has a verified e-mail available for their account.
	Once a crate version has been quarantined, then a member of the crates.io team will review the quarantined crate, ideally within 1 week[^sla]. Once reviewed, the crate may be either approved and published to the index, or rejected and deleted. Either way, the crate owner(s) will be notified by e-mail.

We only allow publishes from users with verified email addresses these days :)

[Turbo87]

Do we want to specify how long we will wait for an answer of the crate owner before potentially deleting a quarantined crate?

[Turbo87]

I'm wondering if we should use something like state: "published" and state: "quarantined" instead. this would allow us to add other states in the future without adding more fields to it. one example could be temporary suspension of a crate/version due to policy violations or retroactive quarantine due to potentially malicious code being discovered.

[Turbo87]

thinking about this some more, I guess we could also encode whether the version has been published to the indexes yet. OTOH we have two of them now, so a single field might not be enough for that then 🤔

[Turbo87]

What do you think about adding a version field instead, containing the version that was just created? That would automatically contain the quarantine status (see "API" section below) and might be useful in other ways too.

[Turbo87]

looks like the ^wait-loop reference is missing below 😉

[Turbo87]

another rule that might be useful: putting new crates into quarantine but allowing new versions of existing crates. not necessarily right now for this RFC, but we should keep it in mind when we implement this.