-
Notifications
You must be signed in to change notification settings - Fork 45
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: bug to support rotated keys in signature/attestation audit
*Context* `npm audit signatures` performs the registry signature and sigstore attestation bundle verification in `pacote`. The current code checks if the public key from the tuf trust root keys target has expires set to in the past: https://github.com/npm/pacote/blob/main/lib/registry.js#L174-L175 If we decide to rotate signing keys and add expires to a old public key, verification will always fail saying the key for old packages has expired. This means we can't rotate signing keys for npm at the moment! *Solution* Check public key expiry against either `integratedTime` for attestations or the publish time for registry signatures. This allows us to rotate a key, setting expiry to after all packages that where signed with that key where published. Complication: some really old npm packages don't have `time` set so we need some kind of cutoff date for these packages. Having time in the packument also requires the npm/cli to fetch the full manifest, not the minified packument that does not contain time. This will restrict usage in the install loop. Signed-off-by: Philip Harrison <[email protected]>
- Loading branch information
Showing
2 changed files
with
100 additions
and
6 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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', | ||
|
@@ -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') | ||
|
@@ -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/[email protected]') | ||
.reply(200, JSON.parse(fixture)) | ||
|
||
const f = new MockedRegistryFetcher('[email protected]', { | ||
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') | ||
|