Skip to content

Commit

Permalink
Error when passing signature without keys
Browse files Browse the repository at this point in the history
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:
npm/cli#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.
  • Loading branch information
feelepxyz committed May 23, 2022
1 parent ea656cc commit d59b3b5
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 38 deletions.
76 changes: 41 additions & 35 deletions lib/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,43 +165,49 @@ 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} ` +
`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' })
}
// validate and throw on error, then set _signatures
const _id = `${mani.name}@${mani.version}`
const message = `${_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(
`${_id} has a registry signature with keyid: ${signature.keyid} ` +
`but no corresponding public key can be found on ${this.registry}-/npm/v1/keys`
), { code: 'EMISSINGSIGNATUREKEY' })
}
const validPublicKey =
!publicKey.expires || (Date.parse(publicKey.expires) > Date.now())
if (!validPublicKey) {
throw Object.assign(new Error(
`${_id} has a registry signature with keyid: ${signature.keyid} ` +
`but the corresponding public key on ${this.registry}-/npm/v1/keys ` +
`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(
`${_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
}
Expand Down
17 changes: 14 additions & 3 deletions test/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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==',
}
)
})
Expand All @@ -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',
}
Expand All @@ -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 => {
Expand Down

0 comments on commit d59b3b5

Please sign in to comment.