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.

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

Jump to bottom

fix: bug to support rotated keys in signature/attestation audit #338

Merged
merged 1 commit into from
Dec 1, 2023
Merged

fix: bug to support rotated keys in signature/attestation audit #338

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
32 changes: 28 additions & 4 deletions lib/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const sigstore = require('sigstore')
const corgiDoc = 'application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*'
const fullDoc = 'application/json'

// Some really old packages have no time field in their packument so we need a
// cutoff date.
const MISSING_TIME_CUTOFF = '2015-01-01T00:00:00.000Z'

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

const _headers = Symbol('_headers')
Expand Down Expand Up @@ -115,6 +119,13 @@ class RegistryFetcher extends Fetcher {
return this.package
}

// When verifying signatures, we need to fetch the full/uncompressed
// packument to get publish time as this is not included in the
// corgi/compressed packument.
if (this.opts.verifySignatures) {
this.fullMetadata = true
}

const packument = await this.packument()
let mani = await pickManifest(packument, this.spec.fetchSpec, {
...this.opts,
Expand All @@ -124,6 +135,12 @@ class RegistryFetcher extends Fetcher {
mani = rpj.normalize(mani)
/* XXX add ETARGET and E403 revalidation of cached packuments here */

// add _time from packument if fetched with fullMetadata
const time = packument.time?.[mani.version]
if (time) {
mani._time = time
}

// add _resolved and _integrity from dist object
const { dist } = mani
if (dist) {
Expand Down Expand Up @@ -171,8 +188,10 @@ class RegistryFetcher extends Fetcher {
'but no corresponding public key can be found'
), { code: 'EMISSINGSIGNATUREKEY' })
}
const validPublicKey =
!publicKey.expires || (Date.parse(publicKey.expires) > Date.now())

const publishedTime = Date.parse(mani._time || MISSING_TIME_CUTOFF)
const validPublicKey = !publicKey.expires ||
publishedTime < Date.parse(publicKey.expires)
if (!validPublicKey) {
throw Object.assign(new Error(
`${mani._id} has a registry signature with keyid: ${signature.keyid} ` +
Expand Down Expand Up @@ -254,8 +273,13 @@ class RegistryFetcher extends Fetcher {
), { code: 'EMISSINGSIGNATUREKEY' })
}

const validPublicKey =
!publicKey.expires || (Date.parse(publicKey.expires) > Date.now())
const integratedTime = new Date(
Number(
bundle.verificationMaterial.tlogEntries[0].integratedTime
) * 1000
)
const validPublicKey = !publicKey.expires ||
(integratedTime < Date.parse(publicKey.expires))
if (!validPublicKey) {
throw Object.assign(new Error(
`${mani._id} has attestations with keyid: ${keyid} ` +
Expand Down
74 changes: 72 additions & 2 deletions test/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,13 @@ t.test('verifySignatures valid signature', async t => {
t.ok(mani._integrity)
})

t.test('verifySignatures expired signature', async t => {
t.test('verifySignatures expired key', async t => {
const f = new RegistryFetcher('@isaacs/namespace-test', {
registry,
cache,
verifySignatures: true,
[`//localhost:${port}/:_keys`]: [{
expires: '2010-01-01',
expires: '2010-01-01T00:00:00.000Z',
keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
keytype: 'ecdsa-sha2-nistp256',
scheme: 'ecdsa-sha2-nistp256',
Expand All @@ -210,6 +210,45 @@ t.test('verifySignatures expired signature', async t => {
)
})

t.test('verifySignatures rotated keys', async t => {
const f = new RegistryFetcher('@isaacs/namespace-test', {
registry,
cache,
verifySignatures: true,
[`//localhost:${port}/:_keys`]: [{
expires: '2020-06-28T18:46:27.981Z', // Expired AFTER publish time: 2019-06-28T18:46:27.981Z
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-----',
}, {
expires: null,
keyid: 'SHA256:123',
keytype: 'ecdsa-sha2-nistp256',
scheme: 'ecdsa-sha2-nistp256',
// eslint-disable-next-line max-len
key: '123',
// eslint-disable-next-line max-len
pemkey: '-----BEGIN PUBLIC KEY-----\n123\n-----END PUBLIC KEY-----',
}],
})
const mani = await f.manifest()
t.match(mani, {
// eslint-disable-next-line max-len
_integrity: 'sha512-5ZYe1LgwHIaag0p9loMwsf5N/wJ4XAuHVNhSO+qulQOXWnyJVuco6IZjo+5u4ZLF/GimdHJcX+QK892ONfOCqQ==',
_signatures: [
{
keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
// eslint-disable-next-line max-len
sig: 'MEQCIHXwKYe70+xcDOvFhM1etZQFUKEwz9VarppUbp5/Ie1+AiAM7aZcT1a2JR0oF/XwjNb13YEHwiagnDapLgYbklRvtA==',
},
],
})
})

t.test('verifySignatures invalid signature', async t => {
tnock(t, 'https://registry.npmjs.org')
.get('/abbrev')
Expand Down Expand Up @@ -837,6 +876,37 @@ t.test('verifyAttestations no valid key', async t => {
)
})

t.test('verifyAttestations rotated key', async t => {
const fixture = fs.readFileSync(
path.join(__dirname, 'fixtures', 'sigstore/valid-attestations.json'),
'utf8'
)

tnock(t, 'https://registry.npmjs.org')
.get('/-/npm/v1/attestations/sigstore@0.4.0')
.reply(200, JSON.parse(fixture))

const f = new MockedRegistryFetcher('sigstore@0.4.0', {
registry: 'https://registry.npmjs.org',
cache,
verifyAttestations: true,
[`//registry.npmjs.org/:_keys`]: [{
expires: '2023-04-01T00:00:00.000Z', // Rotated AFTER integratedTime 2023-01-11T17:31:54.000Z
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-----',
}],
})

const mani = await f.manifest()
t.ok(mani._attestations)
t.ok(mani._integrity)
})

t.test('verifyAttestations no registry keys at all', async t => {
tnock(t, 'https://registry.npmjs.org')
.get('/sigstore')
Expand Down
Loading