Fix audit signatures for expired keys #284
Conversation
Decorate the manifest with the publish time from the packument if this exists for the picked version. This is used in pacote to pick a valid signing key when verifying registry signatures and attestations: npm/pacote#284 Signed-off-by: Philip Harrison <philip@mailharrison.com>
Decorate the manifest with the publish time from the packument if this exists for the picked version. This is used in pacote to pick a valid signing key when verifying registry signatures and attestations: npm/pacote#284 Signed-off-by: Philip Harrison <philip@mailharrison.com>
Decorate the manifest with the publish time from the packument if this exists for the picked version. This is used in pacote to pick a valid signing key when verifying registry signatures and attestations: npm/pacote#284 Signed-off-by: Philip Harrison <philip@mailharrison.com>
Decorate the manifest with the publish time from the packument if this exists for the picked version. This is used in pacote to pick a valid signing key when verifying registry signatures and attestations: npm/pacote#284 Signed-off-by: Philip Harrison <philip@mailharrison.com>
The current implementation of verifySignatures and verifyAttestations assumes the verification keys never expire as its checking if expiry is in the past, this means we can never roll these keys and verify old signatures that where signed with a still valid signing key. Kudos to @bdehamer for spotting this bug. This changes the key selector to allow keys that where valid before the package was published so we can roll new keys in future. There's an accompanying change required in npm-pick-manifest to expose the publish time. If this is not set I opted to use a fallback date, this isn't pretty but seems better than ignoring packages that don't have created at time? Open to suggeststions here! TODO: - Introduce `_time` in https://github.com/npm/npm-pick-manifest - Add a test for very old packages that don't have `time` in packument Signed-off-by: Philip Harrison <philip@mailharrison.com>
| @@ -254,8 +266,10 @@ class RegistryFetcher extends Fetcher { | |||
| ), { code: 'EMISSINGSIGNATUREKEY' }) | |||
| } | |||
|
|
|||
| const publishTime = mani._time ? | |||
There was a problem hiding this comment.
We could pull a more reliably time from the bundle's integrated time when verifying attestations, which I believe is signed
cc @bdehamer thoughts on this?
There was a problem hiding this comment.
Yeah, assuming that the we want the timestamp that is as close as possible to the exact moment when the signature was generated, the bundle.verificationMaterial.tlogEntries[0].integratedTime is gonna be more accurate. I looked at the timestamps for a couple of published packages and the integratedTime was always slightly before the publish time in the package manfiest.
That said . . . when it comes time to do a key rotation I doubt that we're going to be able to identify the precise moment of key rotation (at least not down to the second/millisecond level of accuracy) so we're gonna want to give a wide margin of error to any expires value that we attach to any key when we rotate it.
Knowing that we're gonna have a bit of a buffer in the expiry time, the difference between the integratedTime and the mani._time is probably unimportant. I'd say we should use whichever value is easiest to access.
The current implementation of verifySignatures and verifyAttestations assumes the verification keys never expire as its checking if expiry is in the past, this means we can never roll these keys and verify old signatures that where signed with a still valid signing key.
Kudos to @bdehamer for spotting this bug.
This changes the key selector to allow keys that where valid before the package was published so we can roll new keys in future.
There's an accompanying change required in npm-pick-manifest to expose the publish time. If this is not set I opted to use a fallback date, this isn't pretty but seems better than ignoring packages that don't have created at time? Open to suggeststions here!
TODO:
_time: feat: decorate picked manifest with publish time npm-pick-manifest#87timein packument