From 23017ba8c068b8f7165359fa448de494234fb042 Mon Sep 17 00:00:00 2001
From: Philip Harrison <philip@mailharrison.com>
Date: Mon, 23 May 2022 15:02:19 +0100
Subject: [PATCH] fix: error when passing signature without keys
Raise an error when the package has a signature, `verifySignatures` is true and there are no registry keys configured.
Updated the error message and fixed the undefined name@version in the error message to match the test cases here:
https://github.com/npm/cli/pull/4827
Possible anti-pattern: I've attached a bunch of error attributes if the signature is invalid so we can present this in the json output to ease debugging.
---
lib/registry.js | 77 ++++++++++++++++++++++++++----------------------
test/registry.js | 17 +++++++++--
2 files changed, 55 insertions(+), 39 deletions(-)
diff --git a/lib/registry.js b/lib/registry.js
index 2eb37bce..c8eb6b02 100644
--- a/lib/registry.js
+++ b/lib/registry.js
@@ -122,11 +122,12 @@ class RegistryFetcher extends Fetcher {
}
const packument = await this.packument()
- const mani = await pickManifest(packument, this.spec.fetchSpec, {
+ let mani = await pickManifest(packument, this.spec.fetchSpec, {
...this.opts,
defaultTag: this.defaultTag,
before: this.before,
})
+ mani = rpj.normalize(mani)
/* XXX add ETARGET and E403 revalidation of cached packuments here */
// add _resolved and _integrity from dist object
@@ -165,49 +166,53 @@ class RegistryFetcher extends Fetcher {
mani._integrity = String(this.integrity)
if (dist.signatures) {
if (this.opts.verifySignatures) {
- if (this.registryKeys) {
- // validate and throw on error, then set _signatures
- const message = `${mani._id}:${mani._integrity}`
- for (const signature of dist.signatures) {
- const publicKey = this.registryKeys.filter(key => (key.keyid === signature.keyid))[0]
- if (!publicKey) {
- throw Object.assign(new Error(
- `${mani._id} has a signature with keyid: ${signature.keyid} ` +
- 'but no corresponding public key can be found.'
- ), { code: 'EMISSINGSIGNATUREKEY' })
- }
- const validPublicKey =
- !publicKey.expires || (Date.parse(publicKey.expires) > Date.now())
- if (!validPublicKey) {
- throw Object.assign(new Error(
- `${mani._id} has a signature with keyid: ${signature.keyid} ` +
+ // validate and throw on error, then set _signatures
+ const message = `${mani._id}:${mani._integrity}`
+ for (const signature of dist.signatures) {
+ const publicKey = this.registryKeys &&
+ this.registryKeys.filter(key => (key.keyid === signature.keyid))[0]
+ if (!publicKey) {
+ throw Object.assign(new Error(
+ `${mani._id} has a registry signature with keyid: ${signature.keyid} ` +
+ 'but no corresponding public key can be found'
+ ), { code: 'EMISSINGSIGNATUREKEY' })
+ }
+ const validPublicKey =
+ !publicKey.expires || (Date.parse(publicKey.expires) > Date.now())
+ if (!validPublicKey) {
+ throw Object.assign(new Error(
+ `${mani._id} has a registry signature with keyid: ${signature.keyid} ` +
`but the corresponding public key has expired ${publicKey.expires}`
- ), { code: 'EEXPIREDSIGNATUREKEY' })
- }
- const verifier = crypto.createVerify('SHA256')
- verifier.write(message)
- verifier.end()
- const valid = verifier.verify(
- publicKey.pemkey,
- signature.sig,
- 'base64'
- )
- if (!valid) {
- throw Object.assign(new Error(
- 'Integrity checksum signature failed: ' +
- `key ${publicKey.keyid} signature ${signature.sig}`
- ), { code: 'EINTEGRITYSIGNATURE' })
- }
+ ), { code: 'EEXPIREDSIGNATUREKEY' })
+ }
+ const verifier = crypto.createVerify('SHA256')
+ verifier.write(message)
+ verifier.end()
+ const valid = verifier.verify(
+ publicKey.pemkey,
+ signature.sig,
+ 'base64'
+ )
+ if (!valid) {
+ throw Object.assign(new Error(
+ `${mani._id} has an invalid registry signature with ` +
+ `keyid: ${publicKey.keyid} and signature: ${signature.sig}`
+ ), {
+ code: 'EINTEGRITYSIGNATURE',
+ keyid: publicKey.keyid,
+ signature: signature.sig,
+ resolved: mani._resolved,
+ integrity: mani._integrity,
+ })
}
- mani._signatures = dist.signatures
}
- // if no keys, don't set _signatures
+ mani._signatures = dist.signatures
} else {
mani._signatures = dist.signatures
}
}
}
- this.package = rpj.normalize(mani)
+ this.package = mani
return this.package
}
diff --git a/test/registry.js b/test/registry.js
index 046514ea..55dc773b 100644
--- a/test/registry.js
+++ b/test/registry.js
@@ -240,9 +240,15 @@ t.test('verifySignatures invalid signature', async t => {
}],
})
return t.rejects(
+ /abbrev@1\.1\.1 has an invalid registry signature/,
f.manifest(),
{
code: 'EINTEGRITYSIGNATURE',
+ keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
+ signature: 'nope',
+ resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz',
+ // eslint-disable-next-line max-len
+ integrity: 'sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==',
}
)
})
@@ -258,6 +264,7 @@ t.test('verifySignatures no valid key', async t => {
})
return t.rejects(
f.manifest(),
+ /@isaacs\/namespace-test@1\.0\.0 has a registry signature/,
{
code: 'EMISSINGSIGNATUREKEY',
}
@@ -271,9 +278,13 @@ t.test('verifySignatures no registry keys at all', async t => {
verifySignatures: true,
[`//localhost:${port}/:_keys`]: null,
})
- return f.manifest().then(mani => {
- t.notOk(mani._signatures)
- })
+ return t.rejects(
+ f.manifest(),
+ /@isaacs\/namespace-test@1\.0\.0 has a registry signature/,
+ {
+ code: 'EMISSINGSIGNATUREKEY',
+ }
+ )
})
t.test('404 fails with E404', t => {