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: Improve signature verification #550

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

Conversation

feelepxyz
Copy link
Contributor

@feelepxyz feelepxyz commented Mar 10, 2022

Add a new CLI command audit signatures that verifies the npm signatures in a packages packument. It works on the current install.

Signatures are only useful if people verify them. Signature verification is currently a manual and complex process, involving the Keybase CLI to fetch the npm public key.

Our long-term goal for supply chain security is that all software is signed and verified in a transparent and user-friendly way.

Signing and verifying published packages protects users against a malicious mirror or proxy intercepting and tamptering with the package response (MITM attack). Mirrors and proxyies are common in the npm ecosystem, significantly increasing the possible attack surface.

References

Related to #76

Add a new top-level cli command to verify existing registry signatures,
with basic key rotation support.
@ljharb
Copy link
Contributor

ljharb commented Mar 10, 2022

What signatures in a packument? If i can use npm to verify them, how can i use npm to generate them?

@ljharb
Copy link
Contributor

ljharb commented Mar 10, 2022

Separately, I’d love some motivating examples of actual npm incidents where package signing would have prevented or mitigated the incident, to ensure that this doesn’t end up being security theater.

@feelepxyz
Copy link
Contributor Author

What signatures in a packument? If i can use npm to verify them, how can i use npm to generate them?

@ljharb 👋 The current dist.npm-signature field that's already in the packument is generated on npm's servers when the package is published using npm's private PGP key. This proposal suggests updating to use a new/more modern key, and bake in validation in the cli. Third-party registries could go also generate signatures on publish and populate the packument in the same way.

Supporting user-generated build time signatures is also something we're actively investigating but not as part of this work.

@ljharb
Copy link
Contributor

ljharb commented Mar 10, 2022

I’m a bit confused. You’re saying that all public npm registry packages have this signature automatically?

@feelepxyz
Copy link
Contributor Author

I’m a bit confused. You’re saying that all public npm registry packages have this signature automatically?

Yep. So when you publish a package to npm, a signature is created over the package_name@version:integrity string using a private PGP key, the public key is on keybase today, this gets added to the packument. Here are the docs on verifying using keybase.

This flow is pretty clunky and slow as you need to run this one-by-one over each package you want to verify.

There are two things proposed in this RFC:

    1. Update the signing key, move away from PGP and create light weight ECDSA signatures that can be validated native using node's crypto library
    1. Provide a streamlined way to verify these signatures using a new cli command

Separately, I’d love some motivating examples of actual npm incidents where package signing would have prevented or mitigated the incident, to ensure that this doesn’t end up being security theater.

This is a great question and will see if I can dig out some examples.

@ljharb
Copy link
Contributor

ljharb commented Mar 10, 2022

Why then would this require an opt-in, instead of just running as part of the install?

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Mar 11, 2022
@feelepxyz
Copy link
Contributor Author

feelepxyz commented Mar 11, 2022

Separately, I’d love some motivating examples of actual npm incidents where package signing would have prevented or mitigated the incident, to ensure that this doesn’t end up being security theater.

Three are no directly related attacks on npm that I'm aware of but I've found some similar incidents where users downloaded malicious content from a download server/mirror that would have been mitigated by signing and verifying:

Arguably, this proposal is only fixing what's already there (registry signatures), by making signature validation easy, it's not brining something fundamentally new to the table.

There is a whole other host of attack vectors that are not covered with this change that I'd love us to tackle in future. Particularly the ones outlined in TUF Attacks and Weaknesses.

Why then would this require an opt-in, instead of just running as part of the install?

Yes good point. I think doing validation on install is something we should aim for but adding it as a separate command to begin with will allow us to experiment with the user experience a bit more freely, compared to npm install. Once we're happy with the overall ergonomics we could fold it into install.

There are a few prickly edge cases with validation that we'd need to carefully consider before adding it to install:

  • While we can enforce validation on all packages fetched from npmjs.org as they will have signatures set, we can't do the same for packages fetched from third-party registries until they add support. Users either need to manually trust keys or be warned some packages are unsigned (these warnings might be unwanted if a user didn't explicitly ask to verify).
  • AFAIK there's no way to reliably differentiate between a npmjs.org mirror and a third-party registry where users host their own packages, so we might need to resort to showing a warning when verifying installed packages that have "missing" signatures (the mirror could try circumventing validation by just omitting the signatures array from the served packument.
  • While we can use node's crypto library to do validation, it still uses OpenSSL installed on the machine, so there's a chance this versions is way out of date and doesn't support the elliptic curves we need (prime256v1 in OpenSSL). We can collect feedback from users running this validation command, and error with a useful message if OpenSSL needs updating. Update: node bundles openssl so this isn't a concern.

@MylesBorins
Copy link

Re: OpenSSL

nodejs vendors all dependencies including OpenSSL. So while it is possible to use a system version, that is an edge case and not the norm for systems running node.

@ljharb
Copy link
Contributor

ljharb commented Mar 11, 2022

An extra command for this seems harmless on the surface, but it really seems like it adds minimal value - since it’s confined to public registry packages, pending support in third-party registries), and unless it can actually mitigate a MitM attack, which seems only plausible if someone has hijacked your DNS and there’s no cert pinning/HSTS involved.

I’m assuming here that it would literally never fail for any package on the public registry that made it down to the user unmodified?

@MylesBorins
Copy link

@ljharb I think it makes sense to reframe this as "cleaning up existing debt" rather than thinking of anything as net new.

  1. We have signatures already, this improved them
  2. We have a documented way to verify them, this drops the requirement for 3rd party tools

as @feelepxyz pointed out above this is not by any means the "end" to the improvements or work we want to do around signatures... but the alternative imho is removing signatures altogether, not avoiding this work. Is that what you are advocating for?

Copy link

@trevrosen trevrosen left a comment

Choose a reason for hiding this comment

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

Very thorough, glad we're getting granular! Only hard request I have is to make sure that we have the exact deprecation and expiry timeframes called out very explicitly in this document.

accepted/0000-validate-signatures.md Outdated Show resolved Hide resolved
accepted/0000-validate-signatures.md Outdated Show resolved Hide resolved
accepted/0000-validate-signatures.md Outdated Show resolved Hide resolved
accepted/0000-validate-signatures.md Outdated Show resolved Hide resolved
@ljharb
Copy link
Contributor

ljharb commented Mar 11, 2022

@MylesBorins i'm not advocating for anything just yet; i'm trying to understand the use case and implications.

It seems like verifying the signatures automatically on install - as opposed to providing an explicit command for it - and ignoring any packages that lack signatures, would be a no-harm improvement; and then perhaps those who want to fail on packages that lack signatures could use a config option to do so?

By shipping a separate command, it is implied that using it increases security, ie, that it protects against an actual attack. I haven't yet heard any specific argument that it does so, and I think security theater is worse than no security at all (ie, i think removing the signatures would be a more secure outcome than providing this as a separate command if verifying the signatures didn't actually prevent anything).

Trusted keys are added to the `.npmrc`:

```
[public-signature-key]
Copy link

Choose a reason for hiding this comment

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

can you define more than one trusted key using this notation in the npmrc? (I've never written one).

Copy link

Choose a reason for hiding this comment

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

(Is that even desirable? Maybe not.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh this syntax isn't correct, it should be public-signature-key[]=, but maybe worth naming it something like trusted-signature-key[]= would be clearer what this is 🤔 Also thinking we might not want to include all the attributes here as you might want to be able to update expiry on the key to rotate it etc. Maybe just keyid and registry. You'll want to be able to set multiple keys, for different registries.

This syntax is similar to how you can configure ca's https://docs.npmjs.com/cli/v8/using-npm/config#ca

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah this is just a ini file, so the old syntax would have created object, so would not be possible to trust multiple keys if you add multiple in the config file, the last one "wins".

The downside of trusted-signature-key[]= is that we'd probably just need to reference the keyid without any clear reference for which registry it was added. Could generate a comment that references the registry host but feels hacky.

Copy link
Contributor

Choose a reason for hiding this comment

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

@feelepxyz ini does support Objects combining to form Arrays although if we want nested value setting it does look like the syntax has to change a bit.

Here's an example of a valid/supported syntax we could use:

[public-signature-key.SHA256:{{SHA256_PUBLIC_KEY}}]
expires=2023-12-17T23:57:40-05:00
keytype=ecdsa-sha2-nistp256
sigtype=ecdsa-sha2-nistp256
key={{B64_PUBLIC_KEY}}

In this case, you'd have to specify a unique key id (but that's already a requirement of the spec) & then the keys would be grouped into an Array of values/config properly.

Feel free to try this out:

const ini = require("ini")
let text = `
[public-signature-key.SHA256:123]
expires=2023-12-17T23:57:40-05:00
keytype=ecdsa-sha2-nistp256
sigtype=ecdsa-sha2-nistp256
key=123

[public-signature-key.SHA256:456]
expires=2023-12-17T23:57:40-05:00
keytype=ecdsa-sha2-nistp256
sigtype=ecdsa-sha2-nistp256
key=456

[public-signature-key.SHA256:789]
expires=2023-12-17T23:57:40-05:00
keytype=ecdsa-sha2-nistp256
sigtype=ecdsa-sha2-nistp256
key=789
`
console.log(ini.parse(text))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@darcyclarke thanks for pointing this out! I think is what we want 👍 I see that it gets nested in the object as expected when running npm config list --json 👌 :

"public-signature-key": {
  "SHA256:123": {
    "expires": "2023-12-17T23:57:40-05:00",
    "keytype": "ecdsa-sha2-nistp256",
    "sigtype": "ecdsa-sha2-nistp256",
    "key": "123"
  },
  "SHA256:456": {
    "expires": "2023-12-17T23:57:40-05:00",
    "keytype": "ecdsa-sha2-nistp256",
    "sigtype": "ecdsa-sha2-nistp256",
    "key": "456"
  }
}

Copy link

@phillmv phillmv left a comment

Choose a reason for hiding this comment

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

This is a really good technical document! I really like how you can see all the conversations we've had throughout it, the focus on attack vectors and the weighing of options.

@phillmv
Copy link

phillmv commented Mar 15, 2022

By shipping a separate command, it is implied that using it increases security, ie, that it protects against an actual attack. I haven't yet heard any specific argument that it does so

Hi @ljharb!

The cli will ship with a copy of the public key, so by default an attacker could no longer MITM packages. @feelepxyz talks about this in the Motivation section 🙂.

--

You've mentioned concerns around security theatre, and I think that's appropriate! I think it's correct that this work will not, by itself, add a lot of trust to the registry.

This work will basically only protect you against from someone running a fake mirror that injects its own code into the packages you download.

But it does allow us to start adding new and better trust guarantees over time!

@ljharb
Copy link
Contributor

ljharb commented Mar 15, 2022

@phillmv thanks for the context. so you're saying that this helps the case where someone MitMs the public registry (which, ftr, would be a drastically reduced attack surface if the npm cli used HSTS/cert pinning), and also tries to modify the contents of public packages?

If so, then I agree it's valuable, and provides a real (if minor) security benefit. However, why require a separate command? Why not just do this by default as part of npm install/npm ci? Who wouldn't want this? (assuming it doesn't fail on packages with missing keys, which could be an additional config to enable)

@MylesBorins
Copy link

Why not just do this by default as part of npm install/npm ci

@ljharb I think this is a step towards that, but the amount of scrutiny that npm install gets for performance + stability I think it would be pre-mature to introduce this functionality into the critical path so quickly.

We could introduce a flag for install and setting to support this functionality 🤔 , but with how npm works that would sadly just fail silently on older version that don't support the flag 🙃

@ljharb
Copy link
Contributor

ljharb commented Mar 15, 2022

Gotcha. ok so, the proposal is for a separate command, that could presumably be configured to fail, or ignore, packages that lack a signature (failing on it would probably break everyone that has an internal registry), with an eventual eye for including it in npm install?

If so, thanks for bearing with me, and that sounds great. I'd suggest npm audit signatures or something, to pair with npm audit licenses :-)

@feelepxyz
Copy link
Contributor Author

where someone MitMs the public registry (which, ftr, would be a drastically reduced attack surface if the npm cli used HSTS/cert pinning), and also tries to modify the contents of public packages?

Disappointingly, this won't protect you against the public registry being MITM'd (yet) as we'd loose guarantees that we're getting the right public keys. It protects you against the case where you are pulling packages from a npm proxy/mirror of the public registry that have been tampered with. Some companies run a mirror that they install packages from. Verifying signatures would verify that the package you got was the same one that was uploaded to npm.

This is arguably an incredibly niche attack vector, but we're planning a bunch of follow up work that will be transparent to users. One particular improvement is actually protecting against the public registry being MITD'd or worse, compromised entirely.

Yarn is an another example where they run their own proxy in front of the npm registry. So if Yarn CLI started verifying npm registry signatures they would give users some more guarantees that the package installed is the same one as the one published to the npm registry.

Gotcha. ok so, the proposal is for a separate command, that could presumably be configured to fail, or ignore, packages that lack a signature (failing on it would probably break everyone that has an internal registry), with an eventual eye for including it in npm install?

Yes exactly 💯 I believe starting with a separate command will give us some wiggle room to figure out what configuration is needed and what a good default will be for most users, that could then be included in install by default.

I'd suggest npm audit signatures or something, to pair with npm audit licenses :-)

Thanks for pointing this out! I had no idea about npm audit licenses, will investigate this.

@ljharb
Copy link
Contributor

ljharb commented Mar 15, 2022

see #182 for that one

@bnb
Copy link

bnb commented Mar 31, 2022

this is absolutely outside of the scope of this RFC, but wanted to share because I thought it might be nifty:

I've also been a fan of third-party hooks into the CLI and making it a platform for some time now (nobody else has ever been, though), but it'd be cool if I could install a dependency (npmDependencies lol) from GitHub or Snyk or WhiteSource that also let them hook into the above and do their own signature checking:

npm audit signatures

> verifying registry-generated package signatures: 1/1839
> verifying author-generated package signatures: 32/992
> verifying GitHub Security package signatures: 23/1728
> verifying WhiteSource package signatures: 77/1431

@feelepxyz
Copy link
Contributor Author

I think somehow communicating in CLI output that the current implementation is verifying registry-generated signatures once you've run the command, that'd be sufficent.

Yeah exactly 👍 I think we can make it clear in the output once you run it. Will try and incorporate this in the examples.

I've also been a fan of third-party hooks into the CLI and making it a platform for some time now (nobody else has ever been, though), but it'd be cool if I could install a dependency (npmDependencies lol) from GitHub or Snyk or WhiteSource that also let them hook into the above and do their own signature checking:

Interesting! I had thought about supporting third-party npm registries, e.g. GitHub Packages, Artifactory, Verdaccio etc but this seems slightly different. Do you mean the dependency can define a hook/script that is run by the CLI? Do you have an example of how something like this looks?

@bnb
Copy link

bnb commented Apr 1, 2022

Interesting! I had thought about supporting third-party npm registries, e.g. GitHub Packages, Artifactory, Verdaccio etc but this seems slightly different. Do you mean the dependency can define a hook/script that is run by the CLI? Do you have an example of how something like this looks?

Yeah, you basically nailed it. Given all this is about packages, leveraging that same package ecosystem to extend functionality helps make the CLI more of a platform. I think the closest example would be along the lines of VS Marketplace and VS Code Extensions, except instead of an Electron app it's just a CLI. There are obvious bounds that are needed (an example in VS Code is that certain rendering bits can't be messed with) but providing that sandbox allows for an ecosystem to grow and hook in the workflows you're already using with their products directly into the CLI.

@darcyclarke
Copy link
Contributor

@feelepxyz we'll keep the item on the agenda for next week for sure. Appreciate you stewarding this.

@feelepxyz
Copy link
Contributor Author

Feedback from the Open RFC Meeting:

  • We should aim to enable signature verification by default on install but seems sensible to launch it as a separate command to test it in the wild first, and do a fast follow to enable by default on install
  • We should probably pick defaults on install that ignore missing signatures and just fail on signatures that explicitly fail verification against a public key
  • We could give users options to turn on more strict checks for missing sigs etc when running install
  • Investigate using host/.well-known/npm-keys URL prefix for the public key

@ljharb
Copy link
Contributor

ljharb commented Apr 27, 2022

In order to enable by default on install, perhaps we should first add a config flag that controls whether it does run on install? that way, the semver-major change could be flipping the default of that config.

feelepxyz added a commit to npm/cli that referenced this pull request Apr 29, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
feelepxyz added a commit to npm/cli that referenced this pull request May 9, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label May 11, 2022
feelepxyz added a commit to npm/cli that referenced this pull request May 12, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
feelepxyz added a commit to npm/cli that referenced this pull request May 23, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
feelepxyz added a commit to npm/cli that referenced this pull request May 26, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
wraithgar pushed a commit to npm/cli that referenced this pull request Jun 2, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
feelepxyz added a commit to npm/cli that referenced this pull request Jun 6, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
wraithgar pushed a commit to npm/cli that referenced this pull request Jun 23, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
wraithgar pushed a commit to npm/cli that referenced this pull request Jun 23, 2022
Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

It currently supports:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired, compared to the version created date
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- json/human format output

TODO
- [ ] Fix tests and implement test cases
  - [ ] Expired public key
  - [ ] No public keys
  - [ ] Missing signatures with a public key on the registry
  - [ ] Missing signatures without a public key on the registry
  - [ ] Install with valid signatures
  - [ ] Install with invalid signatures
  - [ ] Third party registry with signatures and keys
  - [ ] Tests for the different formats (json, human)
  - [ ] Tests to omit type of dependency (e.g dev deps)
- [ ] Fetch signatures and integrity from `pacote.manifest`
- [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys
- [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct?
- [ ] What other checks do we want?
  - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys?
  - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?
feelepxyz added a commit to npm/cli that referenced this pull request Jun 28, 2022
Implemenents [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

Supporting:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- Output: json/human formats
feelepxyz added a commit to npm/cli that referenced this pull request Jun 28, 2022
Implemenents [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

Supporting:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- Output: json/human formats

Co-authored-by: Michael Garvin <[email protected]>
feelepxyz added a commit to npm/cli that referenced this pull request Jun 28, 2022
Implemenents [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

Supporting:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- Output: json/human formats

Co-authored-by: Michael Garvin <[email protected]>
wraithgar added a commit to npm/cli that referenced this pull request Jun 28, 2022
Implemenents [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

Supporting:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- Output: json/human formats

Co-authored-by: Michael Garvin <[email protected]>
feelepxyz added a commit to npm/cli that referenced this pull request Jun 30, 2022
Implemenents [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

Supporting:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- Output: json/human formats

Co-authored-by: Michael Garvin <[email protected]>
wraithgar pushed a commit to npm/cli that referenced this pull request Jul 11, 2022
* feat: add npm audit signatures

Implements [RFC: Improve signature verification](npm/rfcs#550)

Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452))

This command will verify registry signatures stored in the packument against a public key on the registry.

Supporting:
- Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object
- Validates public keys are not expired
- Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys`
- Errors when encountering invalid signatures
- Output: json/human formats
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants