Skip to content

Commit

Permalink
feat: verify integrity signature when downloading from npm registry (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
aduh95 authored Apr 12, 2024
1 parent 2d63536 commit e561dd0
Show file tree
Hide file tree
Showing 12 changed files with 336 additions and 20 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -161,5 +161,16 @@
}
}
}
},
"keys": {
"npm": [
{
"expires": null,
"keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
"keytype": "ecdsa-sha2-nistp256",
"scheme": "ecdsa-sha2-nistp256",
"key": "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg=="
}
]
}
}
2 changes: 1 addition & 1 deletion sources/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
17 changes: 15 additions & 2 deletions sources/corepackUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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}`);

Expand Down
48 changes: 43 additions & 5 deletions sources/npmRegistryUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import {UsageError} from 'clipanion';
import {createVerify} from 'crypto';

import defaultConfig from '../config.json';

import * as httpUtils from './httpUtils';

Expand Down Expand Up @@ -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) {
Expand All @@ -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};
}
10 changes: 10 additions & 0 deletions sources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ export interface Config {
};
};
};

keys: {
[registry: string]: Array<{
expires: null;
keyid: string;
keytype: string;
scheme: string;
key: string;
}>;
};
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Up.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 2 additions & 2 deletions tests/Use.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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({
Expand Down
68 changes: 59 additions & 9 deletions tests/_registryServer.mjs
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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

Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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`:
Expand Down
14 changes: 14 additions & 0 deletions tests/config.test.ts
Original file line number Diff line number Diff line change
@@ -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});
});
});
Loading

0 comments on commit e561dd0

Please sign in to comment.