feat: add npm audit signatures

[feelepxyz]

Implements RFC: Improve signature verification

Adds a new sub-command to audit: npm audit signatures (following npm audit licenses)

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

It supports:

• 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
• json/human format output

[feelepxyz]

@wraithgar @ljharb thanks for the quick feedback, much appreciated! 🙇 I'm away next week so won't be getting to this until I'm back.

[wraithgar]

@feelepxyz that's fine, it'll give us enough time to react to this and hopefully move some of this logic into pacote proper.

[feelepxyz]

Ah good spot, I think this should be reversed, wanted to check the key used for the signature hasn’t expired before the version was created in case we need to backdate an expiry due to a compromise. Might be worth thinking through this scenario again though. At the very least we should check the key is still valid today.

…

On Sat, 30 Apr 2022 at 00:49, Jordan Harband ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/utils/verify-signatures.js <#4827 (comment)>: > + const { homepage } = packument + + const registry = fetch.pickRegistry(spec, this.npm.flatOptions) + + const versionPackument = pickManifest(packument, version, this.npm.flatOptions) + const versionCreated = packument.time && packument.time[version] + const dist = versionPackument.dist || {} + const { integrity } = dist + const message = `${name}@${version}:${integrity}` + const signatures = dist.signatures || [] + const keys = (await this.getKeys({ registry })) || [] + const validKeys = keys.filter((publicKey) => { + if (!publicKey.expires) { + return true + } + return Date.parse(versionCreated) < Date.parse(publicKey.expires) It could be, if the local system's clock and the registry's clock aren't the same. — Reply to this email directly, view it on GitHub <#4827 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE5RPVKCJSFMDMKBTHLBLVHRYPZANCNFSM5UWC6WCQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[feelepxyz]

@wraithgar I think this is ready for more thorough review now, I've updated to use your pacote changes, but pending these changes in my pacote PR 🙇

[ljharb]

What does this command do with -g? What should it do? Should it error out, or is there something useful it can do?

[feelepxyz]

What does this command do with -g? What should it do? Should it error out, or is there something useful it can do?

Good shout, currently it just ignores it and runs on the current install but maybe better to error unless it should work? I have never really used global so not sure what makes sense.

[ljharb]

Are the signatures being audited in the installed package.json, or in the lockfile? Global installs don't have a lockfile.

[feelepxyz]

Are the signatures being audited in the installed package.json, or in the lockfile? Global installs don't have a lockfile.

The ones installed from the lockfile. I've added an error case if running with the global flag, looks like this is done for a few commands already. Once we roll this into npm install we can also do this for global dep installs.

[wraithgar]

@feelepxyz let me know when you're done with this iteration of the thread and I'll fix the package/lock conflicts.

[feelepxyz]

@feelepxyz let me know when you're done with this iteration of the thread and I'll fix the package/lock conflicts.

Nice one, I'm all done 👍

[wraithgar]

@feelepxyz all done. How do you feel about squashing the commits in this PR while we wait for it to land? We're likely gonna have at least one more conflict w/ the package lock before we can land this and it'd be way easier to suss out if we hit "reset" and squash the commits so far.

[feelepxyz]

Yes sounds good, happy to squash the commits 👌

…

On Thu, 23 Jun 2022 at 17:53, Gar ***@***.***> wrote: @feelepxyz <https://github.com/feelepxyz> all done. How do you feel about squashing the commits in this PR while we wait for it to land? We're likely gonna have at least one more conflict w/ the package lock before we can land this and it'd be way easier to suss out if we hit "reset" and squash the commits so far. — Reply to this email directly, view it on GitHub <#4827 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE5RLD6Y5ZVFISSEPDVADVQSB6BANCNFSM5UWC6WCQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wraithgar]

Yes sounds good, happy to squash the commits 👌

Great! The branch is all yours.

[wraithgar]

Let's do this.

[wraithgar]

docs are preliminary, @feelepxyz can expand on them in another PR.