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

rfc: Signed Packuments #76

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

rfc: Signed Packuments #76

wants to merge 6 commits into from

Conversation

retrohacker
Copy link

@retrohacker retrohacker commented Dec 14, 2019

Content in diff

William Blankenship and others added 6 commits December 13, 2019 17:09
We can just look for the header if the registry supports it, otherwise gracefully fall back to a separate request for the file based signature
@darcyclarke darcyclarke added Agenda will be discussed at the Open RFC call Enhancement new feature or improvement Needs Discussion is pending a discussion Registry labels Dec 17, 2019
@bnb
Copy link

bnb commented Dec 30, 2019

This pushes trusting the result of an installation into user land as opposed to being able to trust the npm cli. We should aim for npm to have solid security baked in, not offload that to external tooling.

I deeply appreciate this approach. I also think that it’s worth noting that this doesn’t prevent ecosystem tooling to enhance this feature from being built, but rather provides a solid and shared foundation for such tooling to be built, providing a cohesive experience to end-users no matter what innovations the ecosystem makes on top of it.


Notary is a project that allows anyone to have trust over arbitrary collections of data. This would allow us to validate the data uploaded to the npm registry against the _author's_ public keys. However, I believe, today packuments are generated by the registry meaning all packuments are generated by the same author.

Notary may be an effective tool moving forward for validating that packages originated from a trusted/authorized author, irrespective of what registry a package was initially uploaded to, but that isn't the problem we are trying to solve here.
Copy link

@wesleytodd wesleytodd Jan 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure this isn't forgotten, from the RFC meeting:

The major downside of this third party approach is that the install has already happened, compromising your machine. In this light it really needs to be a part of the install happening before the postinstall scripts.

@retrohacker
Copy link
Author

retrohacker commented Mar 4, 2020

Sorry for missing today's RFC

Summary from today's meeting relevant to this PR:

[9] Currently blocked. Concerned about the overhead this will create during installation.

Agree w/ this, could we get a bit more focused definition of our angst? Is it the amount of time it would take to validate the payload against the signature? I'd be happy to put together a benchmark.

[9] npm has to queue up the work needed on infra.
[9] Can we move forward the CLI part of just fetching signatures and validating it, regardless of the infra implementation?

Also agree here. I'd be happy for this to just land in the CLI without official registry support. But understand that the registry probably wants to support all the cli features 😉

[9] A signature failure is like an integrity failure.

In the sense that it means something you expected to be true about the payload was not true, yup!

[9] Two downloads, instead of one (packument and signature).

There is only one request for the common case. The second download is only needed if the server doesn't respond with the header. This is here to provide backwards compatibility w/ projects like registry-static.

[9] signed packuments vs signed versions? Modifications on which versions are available
[9] We have to outline the delta in threat models that signed packuments gets us, and signed tarballs doesn't.

Not super opinionated about what is signed as long as the signature is retroactively applied to 100% of the current registry's contents. The threat model we are trying to address is allowing the mirroring of existing packages in the global namespace without needing to have full trust in that mirror.

An example case would be a community maintained mirror for a conference. Developers on conference wifi can set their registry to npm.some-conference.io so their installs can be resolved on the local network instead of starving the conference's limited bandwidth (I remember us doing this at Walker Creek Ranch!). But this opens up conference attendees to be pwned by someone at the conference: if someone at the conference can access the mirror and modify the packument, they can swap metadata for a tarball and upload their own malicious version of the package. We are looking for a way to not have to trust a mirror in order to pull packages from it.

We have this same problem for a mirroring project I've been hacking away on. We solved for mirror discovery but we don't have a way to solve for trust without requiring a userland component. So now we have a way for users to discover 3rd party mirrors but there is no way to trust that those mirrors aren't tampering with data. Today this prevents us from mirroring packuments. All requests for packuments are proxied and fulfilled by npm instead of mirrors.

To return to the question above, this doesn't necessarily need to be implemented by the official registry. We have use cases in userland that could start using this today 😄

@isaacs
Copy link
Contributor

isaacs commented Mar 4, 2020

My suggestion in the RFC meeting was two-fold:

  1. Let's outline exactly what threat models this addresses, which signed tarballs doesn't (as @retrohacker has started doing that above).
  2. Before doing this, let's figure out the UX and implementation for validating the npmregistry signatures of tarballs, since there's going to be a lot of overlap in implementation and UX considerations. Specifically:
    1. How/where does the npm CLI store the npm registry key?
    2. What do we do when there is no dist.npm-signature present?
    3. How many packages today lack a signature, and how often are those versions downloaded?
    4. Assuming (iii) indicates it's relevant, and (ii) is some form of "not good", let's create a backfill job on the registry to add signatures where they are currently missing. (Not hard, but non-zero amount of work. I expect this is similar to regenerating minified packuments when we change the update field set.)
    5. What is the performance/complexity impact of checking that signature? (I'd suggest that we make it as transparent as possible in pacote, where we currently check the sri values with ssri.)

@retrohacker
Copy link
Author

Hey @isaacs ❤️ thank you for engaging!

let's figure out the UX and implementation for validating the npmregistry signatures of tarballs

Are we talking about dist.npm-signature in the packument?

If so, a couple of questions:

  1. Is npm-signature generated by the npm cli? I had assumed they were user generated when I came across them, couldn't find any documentation on how they came to be.
  2. Is npm already aware of user's public keys? I don't see any of that information in the packument.
  3. Does the cli rely on the packument for identifying what signatures are valid (i.e. contributors)?

pacote

This does make sense to me!

Before doing this

Do these features have to be coupled? Solving for end-user key management seems like a much larger scoped problem than fetching registry keys declared in an .npmrc. Inverting this seems like a more incremental approach at the surface: start with managing a small collection of keys for requests on a per-registry basis and then expand to supporting end user signatures on uploaded artifacts.

@isaacs
Copy link
Contributor

isaacs commented Mar 5, 2020

Are we talking about dist.npm-signature in the packument?

Yes.

  1. No, it's generated by the registry, server-side at publish time. It's a signature of the package integrity string, using the private key of the npmregistry keybase account. The associated public key is at https://keybase.io/npmregistry/pgp_keys.asc
  2. No. Just to be clear, are we talking about user signatures of packuments? Because that is probably not ever going to be possible, as several portions of the packument are modified server-side. (For example, the aforementioned signature being added, or how when you add a new user to a team in an npm org, it updates the maintainers array.)
  3. It's only being signed by the registry itself. User signatures would be interesting, but until we can work out how to solve even this orders-of-magnitude less complex case, I don't think we're ready to cross that bridge.

Without some clear way to solve how we're going to fetch and use publisher keys, in a way that's simple and performant enough to turn on by default, and create signatures in a way that's easy enough to be opt-out rather than opt-in, there's not much value in pursuing user/publisher signing. Let's solve this one easy case first, and then move on from there.

@retrohacker
Copy link
Author

Ah that is awesome! I fundamentally misunderstood what those signatures were.

This seems reasonable. If you can trust the tarball hasn't been tampered with, modifying a packument gives you much less wiggle room for doing nefarious stuff.

I'm 👍 on starting with tarball signatures! Thank you for your patience and talking me through this!

@isaacs
Copy link
Contributor

isaacs commented Mar 11, 2020

Sure thing, @retrohacker, happy to help. It's kinda my job, after all :)

So, the steps here (which maybe should be a new RFC to replace this one?) would be:

  1. add a npm-signature using the npmregistry key to all incoming packages on the registry. (done!)
    1. optional: write and run a backfill script to add signatures to older publishes where they're missing
  2. design and implement npmregistry signature-checking in the CLI. (not trivial. some talking, some thinking, mostly typing. Work out the kinks around key fetching, invalidating, and handling unsigned packages. May require adding more metadata to packument dist object, which will bring us back around to item (1).) This brings us from "zero signatures" to "one signature".
  3. design and implement key-publishing/fetching system for user keys. (pretty hard, actually. tricky bit will be doing this in a performant way at install time, such that it's more secure than theater, and worth doing by default. Any kinks in missing/invalid signatures or revoked keys become much more pressing at this stage.) This brings support from "one signature" to "infinity signatures".
  4. add publisher signatures of packages at publish time. given (3), this will probably be pretty easy, since presumably we'll have tested it out using some hacky solution already, so it's just a typing problem.
  5. Explore signing the packument with the npmregistry key. (this can never be done by the user, because the packument is mutated server-side and out-of-band from publish actions.)
  6. Pie in the sky ideas around web of trust, signing another user's signature to verify that they are who they say they are, re-signing packages when a key is revoked, signatures added when promoting from staged to published, etc. (Technically maybe (5) is an example of one such pie in the sky, but just calling out that there are several pies in that sky.)

@isaacs
Copy link
Contributor

isaacs commented Mar 11, 2020

Also: if we're going to make the npm cli start slamming keybase.io, we probably ought to sync up with them to make sure we don't take them down. We could also look into some other approach, like using https://api.github.com/users/isaacs/gpg_keys along with https://keybase.io/isaacs/pgp_keys.asc.

If we have multiple possible places to decide where to get keys from, then there has to be some discovery mechanism, and then that opens the door for the attack vector "pwned my account, and then changed the pubkey pointer to some other place", so we'll have to think through that aspect carefully as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement new feature or improvement Needs Discussion is pending a discussion Security security related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants