feat: verify integrity signature when downloading from npm registry (#…

…432) When the user has not provided any hash (so when running `corepack up`/`corepack use …`), and the package manager is downloaded from the npm registry, we can verify the signature. BREAKING CHANGE: attempting to download a version from the npm registry (or a mirror) that was published using the now deprecated PGP signature without providing a hash will trigger an error. Users can disable the signature verification using a environment variable.
nodejs · Apr 12, 2024 · e561dd0 · e561dd0
1 parent 2d63536
commit e561dd0
Show file tree

Hide file tree

Showing 12 changed files with 336 additions and 20 deletions.
diff --git a/README.md b/README.md
@@ -294,6 +294,9 @@ same major line. Should you need to upgrade to a new major, use an explicit
 - `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` are supported through
   [`node-proxy-agent`](https://github.com/TooTallNate/node-proxy-agent).
 
+- `COREPACK_INTEGRITY_KEYS` can be set to an empty string to instruct Corepack
+  to skip integrity checks, or a JSON string containing custom keys.
+
 ## Troubleshooting
 
 ### Networking

diff --git a/config.json b/config.json
@@ -161,5 +161,16 @@
         }
       }
     }
+  },
+  "keys": {
+    "npm": [
+      {
+        "expires": null,
+        "keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
+        "keytype": "ecdsa-sha2-nistp256",
+        "scheme": "ecdsa-sha2-nistp256",
+        "key": "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg=="
+      }
+    ]
   }
 }
diff --git a/sources/Engine.ts b/sources/Engine.ts
@@ -296,7 +296,7 @@ export class Engine {
 
     let isTransparentCommand = false;
     if (packageManager != null) {
-      const defaultVersion = await this.getDefaultVersion(packageManager);
+      const defaultVersion = binaryVersion || await this.getDefaultVersion(packageManager);
       const definition = this.config.definitions[packageManager]!;
 
       // If all leading segments match one of the patterns defined in the `transparent`

diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts
@@ -219,13 +219,15 @@ export async function installVersion(installTarget: string, locator: Locator, {s
   }
 
   let url: string;
+  let signatures: Array<{keyid: string, sig: string}>;
+  let integrity: string;
   let binPath: string | null = null;
   if (locatorIsASupportedPackageManager) {
     url = spec.url.replace(`{}`, version);
     if (process.env.COREPACK_NPM_REGISTRY) {
       const registry = getRegistryFromPackageManagerSpec(spec);
       if (registry.type === `npm`) {
-        url = await npmRegistryUtils.fetchTarballUrl(registry.package, version);
+        ({tarball: url, signatures, integrity} = await npmRegistryUtils.fetchTarballURLAndSignature(registry.package, version));
         if (registry.bin) {
           binPath = registry.bin;
         }
@@ -247,7 +249,7 @@ export async function installVersion(installTarget: string, locator: Locator, {s
   }
 
   debugUtils.log(`Installing ${locator.name}@${version} from ${url}`);
-  const algo = build[0] ?? `sha256`;
+  const algo = build[0] ?? `sha512`;
   const {tmpFolder, outputFile, hash: actualHash} = await download(installTarget, url, algo, binPath);
 
   let bin: BinSpec | BinList;
@@ -280,6 +282,17 @@ export async function installVersion(installTarget: string, locator: Locator, {s
     }
   }
 
+  if (!build[1]) {
+    const registry = getRegistryFromPackageManagerSpec(spec);
+    if (registry.type === `npm` && !registry.bin && process.env.COREPACK_INTEGRITY_KEYS !== ``) {
+      if (signatures! == null || integrity! == null)
+        ({signatures, integrity} = (await npmRegistryUtils.fetchTarballURLAndSignature(registry.package, version)));
+
+      npmRegistryUtils.verifySignature({signatures, integrity, packageName: registry.package, version});
+      // @ts-expect-error ignore readonly
+      build[1] = Buffer.from(integrity.slice(`sha512-`.length), `base64`).toString(`hex`);
+    }
+  }
   if (build[1] && actualHash !== build[1])
     throw new Error(`Mismatch hashes. Expected ${build[1]}, got ${actualHash}`);
 

diff --git a/sources/npmRegistryUtils.ts b/sources/npmRegistryUtils.ts
@@ -1,4 +1,7 @@
 import {UsageError}   from 'clipanion';
+import {createVerify} from 'crypto';
+
+import defaultConfig  from '../config.json';
 
 import * as httpUtils from './httpUtils';
 
@@ -28,11 +31,46 @@ export async function fetchAsJson(packageName: string, version?: string) {
   return httpUtils.fetchAsJson(`${npmRegistryUrl}/${packageName}${version ? `/${version}` : ``}`, {headers});
 }
 
+export function verifySignature({signatures, integrity, packageName, version}: {
+  signatures: Array<{keyid: string, sig: string}>;
+  integrity: string;
+  packageName: string;
+  version: string;
+}) {
+  const {npm: keys} = process.env.COREPACK_INTEGRITY_KEYS ?
+    JSON.parse(process.env.COREPACK_INTEGRITY_KEYS) as typeof defaultConfig.keys :
+    defaultConfig.keys;
+
+  const key = keys.find(({keyid}) => signatures.some(s => s.keyid === keyid));
+  const signature = signatures.find(({keyid}) => keyid === key?.keyid);
+
+  if (key == null || signature == null) throw new Error(`Cannot find matching keyid: ${JSON.stringify({signatures, keys})}`);
+
+  const verifier = createVerify(`SHA256`);
+  verifier.end(`${packageName}@${version}:${integrity}`);
+  const valid = verifier.verify(
+    `-----BEGIN PUBLIC KEY-----\n${key.key}\n-----END PUBLIC KEY-----`,
+    signature.sig,
+    `base64`,
+  );
+  if (!valid) {
+    throw new Error(`Signature does not match`);
+  }
+}
+
 export async function fetchLatestStableVersion(packageName: string) {
   const metadata = await fetchAsJson(packageName, `latest`);
 
-  const {shasum} = metadata.dist;
-  return `${metadata.version}+sha1.${shasum}`;
+  const {version, dist: {integrity, signatures}} = metadata;
+
+  if (process.env.COREPACK_INTEGRITY_KEYS !== ``) {
+    verifySignature({
+      packageName, version,
+      integrity, signatures,
+    });
+  }
+
+  return `${version}+sha512.${Buffer.from(integrity.slice(7), `base64`).toString(`hex`)}`;
 }
 
 export async function fetchAvailableTags(packageName: string) {
@@ -45,11 +83,11 @@ export async function fetchAvailableVersions(packageName: string) {
   return Object.keys(metadata.versions);
 }
 
-export async function fetchTarballUrl(packageName: string, version: string) {
+export async function fetchTarballURLAndSignature(packageName: string, version: string) {
   const versionMetadata = await fetchAsJson(packageName, version);
-  const {tarball} = versionMetadata.dist;
+  const {tarball, signatures, integrity} = versionMetadata.dist;
   if (tarball === undefined || !tarball.startsWith(`http`))
     throw new Error(`${packageName}@${version} does not have a valid tarball.`);
 
-  return tarball;
+  return {tarball, signatures, integrity};
 }
diff --git a/sources/types.ts b/sources/types.ts
@@ -103,6 +103,16 @@ export interface Config {
       };
     };
   };
+
+  keys: {
+    [registry: string]: Array<{
+      expires: null;
+      keyid: string;
+      keytype: string;
+      scheme: string;
+      key: string;
+    }>;
+  };
 }
 
 /**

diff --git a/tests/Up.test.ts b/tests/Up.test.ts
@@ -23,7 +23,7 @@ describe(`UpCommand`, () => {
       });
 
       await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({
-        packageManager: `yarn@2.4.3+sha256.8c1575156cfa42112242cc5cfbbd1049da9448ffcdb5c55ce996883610ea983f`,
+        packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`,
       });
 
       await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({

diff --git a/tests/Use.test.ts b/tests/Use.test.ts
@@ -22,7 +22,7 @@ describe(`UseCommand`, () => {
       });
 
       await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({
-        packageManager: `yarn@1.22.4+sha256.bc5316aa110b2f564a71a3d6e235be55b98714660870c5b6b2d2d3f12587fb58`,
+        packageManager: `yarn@1.22.4+sha512.a1833b862fe52169bd6c2a033045a07df5bc6a23595c259e675fed1b2d035ab37abe6ce309720abb6636d68f03615054b6292dc0a70da31c8697fda228b50d18`,
       });
 
       await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
@@ -40,7 +40,7 @@ describe(`UseCommand`, () => {
       });
 
       await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({
-        packageManager: `yarn@1.22.4+sha256.bc5316aa110b2f564a71a3d6e235be55b98714660870c5b6b2d2d3f12587fb58`,
+        packageManager: `yarn@1.22.4+sha512.a1833b862fe52169bd6c2a033045a07df5bc6a23595c259e675fed1b2d035ab37abe6ce309720abb6636d68f03615054b6292dc0a70da31c8697fda228b50d18`,
       });
 
       await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({

diff --git a/tests/_registryServer.mjs b/tests/_registryServer.mjs
@@ -1,8 +1,40 @@
-import {createHash}   from 'node:crypto';
-import {once}         from 'node:events';
-import {createServer} from 'node:http';
-import {connect}      from 'node:net';
-import {gzipSync}     from 'node:zlib';
+import {createHash, createSign, generateKeyPairSync} from 'node:crypto';
+import {once}                                        from 'node:events';
+import {createServer}                                from 'node:http';
+import {connect}                                     from 'node:net';
+import {gzipSync}                                    from 'node:zlib';
+
+let privateKey, keyid;
+
+switch (process.env.TEST_INTEGRITY) {
+  case `invalid_signature`: {
+    ({privateKey} = generateKeyPairSync(`ec`, {
+      namedCurve: `sect239k1`,
+    }));
+  }
+  // eslint-disable-next-line no-fallthrough
+  case `invalid_integrity`:
+  case `valid`: {
+    const {privateKey: p, publicKey} = generateKeyPairSync(`ec`, {
+      namedCurve: `sect239k1`,
+      publicKeyEncoding: {
+        type: `spki`,
+        format: `pem`,
+      },
+    });
+    privateKey ??= p;
+    keyid = `SHA256:${createHash(`SHA256`).end(publicKey).digest(`base64`)}`;
+    process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({npm: [{
+      expires: null,
+      keyid,
+      keytype: `ecdsa-sha2-sect239k1`,
+      scheme: `ecdsa-sha2-sect239k1`,
+      key: publicKey.split(`\n`).slice(1, -2).join(``),
+    }]});
+    break;
+  }
+}
+
 
 function createSimpleTarArchive(fileName, fileContent, mode = 0o644) {
   const contentBuffer = Buffer.from(fileContent);
@@ -13,7 +45,7 @@ function createSimpleTarArchive(fileName, fileContent, mode = 0o644) {
   header.write(`0001750 `, 108, 8, `utf-8`); // Owner's numeric user ID (octal) followed by a space
   header.write(`0001750 `, 116, 8, `utf-8`); // Group's numeric user ID (octal) followed by a space
   header.write(`${contentBuffer.length.toString(8)} `, 124, 12, `utf-8`); // File size in bytes (octal) followed by a space
-  header.write(`${Math.floor(Date.now() / 1000).toString(8)} `, 136, 12, `utf-8`); // Last modification time in numeric Unix time format (octal) followed by a space
+  header.write(`${Math.floor(new Date(2000, 1, 1) / 1000).toString(8)} `, 136, 12, `utf-8`); // Last modification time in numeric Unix time format (octal) followed by a space
   header.fill(` `, 148, 156); // Fill checksum area with spaces for calculation
   header.write(`ustar  `, 257, 8, `utf-8`); // UStar indicator
 
@@ -37,7 +69,11 @@ const mockPackageTarGz = gzipSync(Buffer.concat([
   Buffer.alloc(1024),
 ]));
 const shasum = createHash(`sha1`).update(mockPackageTarGz).digest(`hex`);
-const integrity = `sha512-${createHash(`sha512`).update(mockPackageTarGz).digest(`base64`)}`;
+const integrity = `sha512-${createHash(`sha512`).update(
+  process.env.TEST_INTEGRITY === `invalid_integrity` ?
+    mockPackageTarGz.subarray(1) :
+    mockPackageTarGz,
+).digest(`base64`)}`;
 
 const registry = {
   __proto__: null,
@@ -48,6 +84,14 @@ const registry = {
   customPkgManager: [`1.0.0`],
 };
 
+function generateSignature(packageName, version) {
+  if (privateKey == null) return undefined;
+  const sign = createSign(`SHA256`).end(`${packageName}@${version}:${integrity}`);
+  return {signatures: [{
+    keyid,
+    sig: sign.sign(privateKey, `base64`),
+  }]};
+}
 function generateVersionMetadata(packageName, version) {
   return {
     name: packageName,
@@ -61,14 +105,20 @@ function generateVersionMetadata(packageName, version) {
       size: mockPackageTarGz.length,
       noattachment: false,
       tarball: `${process.env.COREPACK_NPM_REGISTRY}/${packageName}/-/${packageName}-${version}.tgz`,
+      ...generateSignature(packageName, version),
     },
   };
 }
 
+const TOKEN_MOCK = `SOME_DUMMY_VALUE`;
+
 const server = createServer((req, res) => {
   const auth = req.headers.authorization;
 
-  if (auth?.startsWith(`Basic `) && Buffer.from(auth.slice(`Basic `.length), `base64`).toString() !== `user:pass`) {
+  if (
+    (auth?.startsWith(`Bearer `) && auth.slice(`Bearer `.length) !== TOKEN_MOCK) ||
+    (auth?.startsWith(`Basic `) && Buffer.from(auth.slice(`Basic `.length), `base64`).toString() !== `user:pass`)
+  ) {
     res.writeHead(401).end(`Unauthorized`);
     return;
   }
@@ -159,7 +209,7 @@ switch (process.env.AUTH_TYPE) {
 
   case `COREPACK_NPM_TOKEN`:
     process.env.COREPACK_NPM_REGISTRY = `http://${address.includes(`:`) ? `[${address}]` : address}:${port}`;
-    process.env.COREPACK_NPM_TOKEN = Buffer.from(`user:pass`).toString(`base64`);
+    process.env.COREPACK_NPM_TOKEN = TOKEN_MOCK;
     break;
 
   case `COREPACK_NPM_PASSWORD`:

diff --git a/tests/config.test.ts b/tests/config.test.ts
@@ -0,0 +1,14 @@
+import {jest, describe, it, expect} from '@jest/globals';
+
+import defaultConfig                from '../config.json';
+import {DEFAULT_NPM_REGISTRY_URL}   from '../sources/npmRegistryUtils';
+
+jest.mock(`../sources/httpUtils`);
+
+describe(`key store should be up-to-date`, () => {
+  it(`should contain up-to-date npm keys`, async () => {
+    const r = await globalThis.fetch(new URL(`/-/npm/v1/keys`, DEFAULT_NPM_REGISTRY_URL));
+    expect(r.ok).toBe(true);
+    expect(r.json()).resolves.toMatchObject({keys: defaultConfig.keys.npm});
+  });
+});