feat: audit signatures verifies attestations

Update `audit signatures` to also verify Sigstore attestations. Additional changes: - Adding error message to json error output as there are a lot of different failure cases with signature verification that would be hard to debug without this - Adding predicateType to json error output for attestations to diffentiate between provenance and publish attestations References: - Pacote changes: npm/pacote#259 - RFC: npm/rfcs#626 Signed-off-by: Philip Harrison <[email protected]>
npm · Feb 14, 2023 · 79bfd03 · 79bfd03
1 parent d20ee2a
commit 79bfd03
Show file tree

Hide file tree

Showing 6 changed files with 610 additions and 21 deletions.
diff --git a/lib/commands/audit.js b/lib/commands/audit.js
@@ -24,7 +24,8 @@ class VerifySignatures {
     this.missing = []
     this.checkedPackages = new Set()
     this.auditedWithKeysCount = 0
-    this.verifiedCount = 0
+    this.verifiedSignatureCount = 0
+    this.verifiedAttestationCount = 0
     this.exitCode = 0
   }
 
@@ -78,12 +79,25 @@ class VerifySignatures {
     this.npm.output(timing)
     this.npm.output('')
 
-    if (this.verifiedCount) {
-      const verifiedBold = this.npm.chalk.bold('verified')
-      if (this.verifiedCount === 1) {
-        this.npm.output(`${this.verifiedCount} package has a ${verifiedBold} registry signature`)
+    const verifiedBold = this.npm.chalk.bold('verified')
+    if (this.verifiedSignatureCount) {
+      if (this.verifiedSignatureCount === 1) {
+        /* eslint-disable-next-line max-len */
+        this.npm.output(`${this.verifiedSignatureCount} package has a ${verifiedBold} registry signature`)
+      } else {
+        /* eslint-disable-next-line max-len */
+        this.npm.output(`${this.verifiedSignatureCount} packages have ${verifiedBold} registry signatures`)
+      }
+      this.npm.output('')
+    }
+
+    if (this.verifiedAttestationCount) {
+      if (this.verifiedAttestationCount === 1) {
+        /* eslint-disable-next-line max-len */
+        this.npm.output(`${this.verifiedAttestationCount} package has a ${verifiedBold} attestation`)
       } else {
-        this.npm.output(`${this.verifiedCount} packages have ${verifiedBold} registry signatures`)
+        /* eslint-disable-next-line max-len */
+        this.npm.output(`${this.verifiedAttestationCount} packages have ${verifiedBold} attestations`)
       }
       this.npm.output('')
     }
@@ -110,19 +124,35 @@ class VerifySignatures {
       const invalidClr = this.npm.chalk.bold(this.npm.chalk.red('invalid'))
       // We can have either invalid signatures or invalid provenance
       const invalidSignatures = this.invalid.filter(i => i.code === 'EINTEGRITYSIGNATURE')
-      if (invalidSignatures.length === 1) {
-        this.npm.output(`1 package has an ${invalidClr} registry signature:`)
-      // } else if (invalidSignatures.length > 1) {
-      } else {
-        // TODO move this back to an else if once provenance attestation audit is added
-        /* eslint-disable-next-line max-len */
-        this.npm.output(`${invalidSignatures.length} packages have ${invalidClr} registry signatures:`)
+      if (invalidSignatures.length) {
+        if (invalidSignatures.length === 1) {
+          this.npm.output(`1 package has an ${invalidClr} registry signature:`)
+        } else {
+          /* eslint-disable-next-line max-len */
+          this.npm.output(`${invalidSignatures.length} packages have ${invalidClr} registry signatures:`)
+        }
+        this.npm.output('')
+        invalidSignatures.map(i =>
+          this.npm.output(`${this.npm.chalk.red(`${i.name}@${i.version}`)} (${i.registry})`)
+        )
+        this.npm.output('')
       }
-      this.npm.output('')
-      invalidSignatures.map(i =>
-        this.npm.output(`${this.npm.chalk.red(`${i.name}@${i.version}`)} (${i.registry})`)
-      )
-      this.npm.output('')
+
+      const invalidAttestations = this.invalid.filter(i => i.code === 'EATTESTATIONVERIFY')
+      if (invalidAttestations.length) {
+        if (invalidAttestations.length === 1) {
+          this.npm.output(`1 package has an ${invalidClr} attestation:`)
+        } else {
+          /* eslint-disable-next-line max-len */
+          this.npm.output(`${invalidAttestations.length} packages have ${invalidClr} attestations:`)
+        }
+        this.npm.output('')
+        invalidAttestations.map(i =>
+          this.npm.output(`${this.npm.chalk.red(`${i.name}@${i.version}`)} (${i.registry})`)
+        )
+        this.npm.output('')
+      }
+
       if (invalid.length === 1) {
         /* eslint-disable-next-line max-len */
         this.npm.output(`Someone might have tampered with this package since it was published on the registry!`)
@@ -252,16 +282,19 @@ class VerifySignatures {
     const {
       _integrity: integrity,
       _signatures,
+      _attestations,
       _resolved: resolved,
     } = await pacote.manifest(`${name}@${version}`, {
       verifySignatures: true,
+      verifyAttestations: true,
       ...this.buildRegistryConfig(registry),
       ...this.npm.flatOptions,
     })
     const signatures = _signatures || []
     const result = {
       integrity,
       signatures,
+      attestations: _attestations,
       resolved,
     }
     return result
@@ -287,14 +320,14 @@ class VerifySignatures {
     }
 
     try {
-      const { integrity, signatures, resolved } = await this.verifySignatures(
+      const { integrity, signatures, attestations, resolved } = await this.verifySignatures(
         name, version, registry
       )
 
       // Currently we only care about missing signatures on registries that provide a public key
       // We could make this configurable in the future with a strict/paranoid mode
       if (signatures.length) {
-        this.verifiedCount += 1
+        this.verifiedSignatureCount += 1
       } else if (keys.length) {
         this.missing.push({
           integrity,
@@ -305,17 +338,26 @@ class VerifySignatures {
           version,
         })
       }
+
+      // Track verified attestations separately to registry signatures, as all
+      // packages on registries with signing keys are expected to have registry
+      // signatures, but not all packages have provenance and publish attestations.
+      if (attestations) {
+        this.verifiedAttestationCount += 1
+      }
     } catch (e) {
-      if (e.code === 'EINTEGRITYSIGNATURE') {
+      if (e.code === 'EINTEGRITYSIGNATURE' || e.code === 'EATTESTATIONVERIFY') {
         this.invalid.push({
           code: e.code,
+          message: e.message,
           integrity: e.integrity,
           keyid: e.keyid,
           location,
           name,
           registry,
           resolved: e.resolved,
           signature: e.signature,
+          predicateType: e.predicateType,
           type,
           version,
         })

diff --git a/package-lock.json b/package-lock.json
@@ -11143,6 +11143,7 @@
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/sigstore/-/sigstore-1.0.0.tgz",
       "integrity": "sha512-e+qfbn/zf1+rCza/BhIA//Awmf0v1pa5HQS8Xk8iXrn9bgytytVLqYD0P7NSqZ6IELTgq+tcDvLPkQjNHyWLNg==",
+      "inBundle": true,
       "dependencies": {
         "make-fetch-happen": "^11.0.1",
         "tuf-js": "^1.0.0"
@@ -14066,6 +14067,7 @@
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/tuf-js/-/tuf-js-1.0.0.tgz",
       "integrity": "sha512-1dxsQwESDzACJjTdYHQ4wJ1f/of7jALWKfJEHSBWUQB/5UTJUx9SW6GHXp4mZ1KvdBRJCpGjssoPFGi4hvw8/A==",
+      "inBundle": true,
       "dependencies": {
         "make-fetch-happen": "^11.0.1",
         "minimatch": "^6.1.0"

diff --git a/tap-snapshots/test/lib/commands/audit.js.test.cjs b/tap-snapshots/test/lib/commands/audit.js.test.cjs
@@ -53,6 +53,7 @@ exports[`test/lib/commands/audit.js TAP audit signatures json output with invali
   "invalid": [
     {
       "code": "EINTEGRITYSIGNATURE",
+      "message": "[email protected] has an invalid registry signature with keyid: SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA and signature: bogus",
       "integrity": "sha512-QqZ7VJ/8xPkS9s2IWB7Shj3qTJdcRyeXKbPQnsZjsPEwvutGv0EGeVchPcauoiDFJlGbZMFq5GDCurAGNSghJQ==",
       "keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
       "location": "node_modules/kms-demo",
@@ -76,11 +77,34 @@ exports[`test/lib/commands/audit.js TAP audit signatures json output with invali
 }
 `
 
+exports[`test/lib/commands/audit.js TAP audit signatures json output with invalid attestations > must match snapshot 1`] = `
+{
+  "invalid": [
+    {
+      "code": "EATTESTATIONVERIFY",
+      "message": "[email protected] failed to verify attestation: artifact signature verification failed",
+      "integrity": "sha512-e+qfbn/zf1+rCza/BhIA//Awmf0v1pa5HQS8Xk8iXrn9bgytytVLqYD0P7NSqZ6IELTgq+tcDvLPkQjNHyWLNg==",
+      "keyid": "",
+      "location": "node_modules/sigstore",
+      "name": "sigstore",
+      "registry": "https://registry.npmjs.org/",
+      "resolved": "https://registry.npmjs.org/sigstore/-/sigstore-1.0.0.tgz",
+      "signature": "MEYCIQD10kAn3lC/1rJvXBtSDckbqkKEmz369gPDKb4lG4zMKQIhAP1+RhbMcASsfXhxpXKNCAjJb+3Av3Br95eKD7VL/BEB",
+      "predicateType": "https://slsa.dev/provenance/v0.2",
+      "type": "dependencies",
+      "version": "1.0.0"
+    }
+  ],
+  "missing": []
+}
+`
+
 exports[`test/lib/commands/audit.js TAP audit signatures json output with invalid signatures > must match snapshot 1`] = `
 {
   "invalid": [
     {
       "code": "EINTEGRITYSIGNATURE",
+      "message": "[email protected] has an invalid registry signature with keyid: SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA and signature: bogus",
       "integrity": "sha512-QqZ7VJ/8xPkS9s2IWB7Shj3qTJdcRyeXKbPQnsZjsPEwvutGv0EGeVchPcauoiDFJlGbZMFq5GDCurAGNSghJQ==",
       "keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
       "location": "node_modules/kms-demo",
@@ -173,6 +197,17 @@ audited 1 package in xxx
 
 `
 
+exports[`test/lib/commands/audit.js TAP audit signatures with invalid attestations > must match snapshot 1`] = `
+audited 1 package in xxx
+
+1 package has an invalid attestation:
+
+[email protected] (https://registry.npmjs.org/)
+
+Someone might have tampered with this package since it was published on the registry!
+
+`
+
 exports[`test/lib/commands/audit.js TAP audit signatures with invalid signatures > must match snapshot 1`] = `
 audited 1 package in xxx
 
@@ -203,6 +238,18 @@ audited 1 package in xxx
 [email protected] (https://registry.npmjs.org/)
 `
 
+exports[`test/lib/commands/audit.js TAP audit signatures with multiple invalid attestations > must match snapshot 1`] = `
+audited 2 packages in xxx
+
+2 packages have invalid attestations:
+
+[email protected] (https://registry.npmjs.org/)
+[email protected] (https://registry.npmjs.org/)
+
+Someone might have tampered with these packages since they were published on the registry!
+
+`
+
 exports[`test/lib/commands/audit.js TAP audit signatures with multiple invalid signatures > must match snapshot 1`] = `
 audited 2 packages in xxx
 
@@ -247,6 +294,15 @@ audited 2 packages in xxx
 [email protected] (https://registry.npmjs.org/)
 `
 
+exports[`test/lib/commands/audit.js TAP audit signatures with valid attestations > must match snapshot 1`] = `
+audited 1 package in xxx
+
+1 package has a verified registry signature
+
+1 package has a verified attestation
+
+`
+
 exports[`test/lib/commands/audit.js TAP audit signatures with valid signatures > must match snapshot 1`] = `
 audited 1 package in xxx