Skip to content

feat: add checkSignatures to registry.manifest #170

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 17, 2022
Merged

feat: add checkSignatures to registry.manifest #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ resolved, and other properties, as they are determined.
is unlikely to change in the span of a single command.
* `silent` A boolean that determines whether the banner is displayed
when calling `@npmcli/run-script`.
* `verifySignatures` A boolean that will make pacote verify the
integrity signature of a manifest, if present. There must be a
configured `_keys` entry in the config that is scoped to the
registry the manifest is being fetched from.


### Advanced API
Expand Down
191 changes: 114 additions & 77 deletions lib/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ const npa = require('npm-package-arg')
const rpj = require('read-package-json-fast')
const pickManifest = require('npm-pick-manifest')
const ssri = require('ssri')
const crypto = require('crypto')

// Corgis are cute. 🐕🐶
const corgiDoc = 'application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*'
const fullDoc = 'application/json'

const fetch = require('npm-registry-fetch')

// TODO: memoize reg requests, so we don't even have to check cache

const _headers = Symbol('_headers')
class RegistryFetcher extends Fetcher {
constructor (spec, opts) {
Expand All @@ -39,28 +38,30 @@ class RegistryFetcher extends Fetcher {
this.packumentUrl = removeTrailingSlashes(this.registry) + '/' +
this.spec.escapedName

const parsed = new URL(this.registry)
const regKey = `//${parsed.host}${parsed.pathname}`
// unlike the nerf-darted auth keys, this one does *not* allow a mismatch
// of trailing slashes. It must match exactly.
if (this.opts[`${regKey}:_keys`]) {
this.registryKeys = this.opts[`${regKey}:_keys`]
}

// XXX pacote <=9 has some logic to ignore opts.resolved if
// the resolved URL doesn't go to the same registry.
// Consider reproducing that here, to throw away this.resolved
// in that case.
}

resolve () {
if (this.resolved) {
return Promise.resolve(this.resolved)
}

// fetching the manifest sets resolved and (usually) integrity
return this.manifest().then(() => {
if (this.resolved) {
return this.resolved
}

async resolve () {
// fetching the manifest sets resolved and (if present) integrity
await this.manifest()
if (!this.resolved) {
throw Object.assign(
new Error('Invalid package manifest: no `dist.tarball` field'),
{ package: this.spec.toString() }
)
})
}
return this.resolved
}

[_headers] () {
Expand All @@ -87,91 +88,127 @@ class RegistryFetcher extends Fetcher {
// npm-registry-fetch the packument
// set the appropriate header for corgis if fullMetadata isn't set
// return the res.json() promise
const p = fetch(this.packumentUrl, {
...this.opts,
headers: this[_headers](),
spec: this.spec,
// never check integrity for packuments themselves
integrity: null,
}).then(res => res.json().then(packument => {
try {
const res = await fetch(this.packumentUrl, {
...this.opts,
headers: this[_headers](),
spec: this.spec,
// never check integrity for packuments themselves
integrity: null,
})
const packument = await res.json()
packument._cached = res.headers.has('x-local-cache')
packument._contentLength = +res.headers.get('content-length')
if (this.packumentCache) {
this.packumentCache.set(this.packumentUrl, packument)
}
return packument
})).catch(er => {
} catch (err) {
if (this.packumentCache) {
this.packumentCache.delete(this.packumentUrl)
}
if (er.code === 'E404' && !this.fullMetadata) {
// possible that corgis are not supported by this registry
this.fullMetadata = true
return this.packument()
if (err.code !== 'E404' || this.fullMetadata) {
throw err
}
throw er
})
if (this.packumentCache) {
this.packumentCache.set(this.packumentUrl, p)
// possible that corgis are not supported by this registry
this.fullMetadata = true
return this.packument()
}
return p
}

manifest () {
async manifest () {
if (this.package) {
return Promise.resolve(this.package)
return this.package
}

return this.packument()
.then(packument => pickManifest(packument, this.spec.fetchSpec, {
...this.opts,
defaultTag: this.defaultTag,
before: this.before,
}) /* XXX add ETARGET and E403 revalidation of cached packuments here */)
.then(mani => {
// add _resolved and _integrity from dist object
const { dist } = mani
if (dist) {
this.resolved = mani._resolved = dist.tarball
mani._from = this.from
const distIntegrity = dist.integrity ? ssri.parse(dist.integrity)
: dist.shasum ? ssri.fromHex(dist.shasum, 'sha1', { ...this.opts })
: null
if (distIntegrity) {
if (!this.integrity) {
this.integrity = distIntegrity
} else if (!this.integrity.match(distIntegrity)) {
// only bork if they have algos in common.
// otherwise we end up breaking if we have saved a sha512
// previously for the tarball, but the manifest only
// provides a sha1, which is possible for older publishes.
// Otherwise, this is almost certainly a case of holding it
// wrong, and will result in weird or insecure behavior
// later on when building package tree.
for (const algo of Object.keys(this.integrity)) {
if (distIntegrity[algo]) {
throw Object.assign(new Error(
`Integrity checksum failed when using ${algo}: ` +
`wanted ${this.integrity} but got ${distIntegrity}.`
), { code: 'EINTEGRITY' })
}
}
// made it this far, the integrity is worthwhile. accept it.
// the setter here will take care of merging it into what we
// already had.
this.integrity = distIntegrity
const packument = await this.packument()
const mani = await pickManifest(packument, this.spec.fetchSpec, {
...this.opts,
defaultTag: this.defaultTag,
before: this.before,
})
/* XXX add ETARGET and E403 revalidation of cached packuments here */

// add _resolved and _integrity from dist object
const { dist } = mani
if (dist) {
this.resolved = mani._resolved = dist.tarball
mani._from = this.from
const distIntegrity = dist.integrity ? ssri.parse(dist.integrity)
: dist.shasum ? ssri.fromHex(dist.shasum, 'sha1', { ...this.opts })
: null
if (distIntegrity) {
if (this.integrity && !this.integrity.match(distIntegrity)) {
// only bork if they have algos in common.
// otherwise we end up breaking if we have saved a sha512
// previously for the tarball, but the manifest only
// provides a sha1, which is possible for older publishes.
// Otherwise, this is almost certainly a case of holding it
// wrong, and will result in weird or insecure behavior
// later on when building package tree.
for (const algo of Object.keys(this.integrity)) {
if (distIntegrity[algo]) {
throw Object.assign(new Error(
`Integrity checksum failed when using ${algo}: ` +
`wanted ${this.integrity} but got ${distIntegrity}.`
), { code: 'EINTEGRITY' })
}
}
}
if (this.integrity) {
mani._integrity = String(this.integrity)
if (dist.signatures) {
// made it this far, the integrity is worthwhile. accept it.
// the setter here will take care of merging it into what we already
// had.
this.integrity = distIntegrity
}
}
if (this.integrity) {
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' })
}
}
mani._signatures = dist.signatures
}
// if no keys, don't set _signatures
} else {
mani._signatures = dist.signatures
}
this.package = rpj.normalize(mani)
return this.package
})
}
}
this.package = rpj.normalize(mani)
return this.package
}

[_tarballFromResolved] () {
Expand Down
126 changes: 126 additions & 0 deletions test/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,132 @@ t.test('provide matching integrity, totes ok, includes signature', async t => {
})
})

t.test('verifySignatures valid signature', async t => {
const f = new RegistryFetcher('@isaacs/namespace-test', {
registry,
cache,
verifySignatures: true,
[`//localhost:${port}/:_keys`]: [{
expires: null,
keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
keytype: 'ecdsa-sha2-nistp256',
scheme: 'ecdsa-sha2-nistp256',
// eslint-disable-next-line max-len
key: 'MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg==',
// eslint-disable-next-line max-len
pemkey: '-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg==\n-----END PUBLIC KEY-----',
}],
})
return f.manifest().then(mani => {
t.ok(mani._signatures)
t.ok(mani._integrity)
})
})

t.test('verifySignatures expired signature', async t => {
const f = new RegistryFetcher('@isaacs/namespace-test', {
registry,
cache,
verifySignatures: true,
[`//localhost:${port}/:_keys`]: [{
expires: '2010-01-01',
keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
keytype: 'ecdsa-sha2-nistp256',
scheme: 'ecdsa-sha2-nistp256',
// eslint-disable-next-line max-len
key: 'MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg==',
// eslint-disable-next-line max-len
pemkey: '-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg==\n-----END PUBLIC KEY-----',
}],
})
return t.rejects(
f.manifest(),
{
code: 'EEXPIREDSIGNATUREKEY',
}
)
})

t.test('verifySignatures invalid signature', async t => {
tnock(t, 'https://registry.npmjs.org')
.get('/abbrev')
.reply(200, {
_id: 'abbrev',
_rev: 'deadbeef',
name: 'abbrev',
'dist-tags': { latest: '1.1.1' },
versions: {
'1.1.1': {
name: 'abbrev',
version: '1.1.1',
dist: {
// eslint-disable-next-line max-len
integrity: 'sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==',
shasum: 'f8f2c887ad10bf67f634f005b6987fed3179aac8',
tarball: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz',
signatures: [
{
keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
sig: 'nope',
},
],
},
},
},
})

const f = new RegistryFetcher('abbrev', {
registry: 'https://registry.npmjs.org',
cache,
verifySignatures: true,
[`//registry.npmjs.org/:_keys`]: [{
expires: null,
keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
keytype: 'ecdsa-sha2-nistp256',
scheme: 'ecdsa-sha2-nistp256',
// eslint-disable-next-line max-len
key: 'MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg==',
// eslint-disable-next-line max-len
pemkey: '-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg==\n-----END PUBLIC KEY-----',
}],
})
return t.rejects(
f.manifest(),
{
code: 'EINTEGRITYSIGNATURE',
}
)
})

t.test('verifySignatures no valid key', async t => {
const f = new RegistryFetcher('@isaacs/namespace-test', {
registry,
cache,
verifySignatures: true,
[`//localhost:${port}/:_keys`]: [{
keyid: 'someotherid',
}],
})
return t.rejects(
f.manifest(),
{
code: 'EMISSINGSIGNATUREKEY',
}
)
})

t.test('verifySignatures no registry keys at all', async t => {
const f = new RegistryFetcher('@isaacs/namespace-test', {
registry,
cache,
verifySignatures: true,
[`//localhost:${port}/:_keys`]: null,
})
return f.manifest().then(mani => {
t.notOk(mani._signatures)
})
})

t.test('404 fails with E404', t => {
const f = new RegistryFetcher('thing-is-not-here', { registry, cache })
return t.rejects(f.resolve(), { code: 'E404' }).then(() =>
Expand Down