Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: verify integrity signature when downloading from npm registry #432

Merged
merged 28 commits into from Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
81842da
feat: verify integrity signature when downloading from npm registry w…
aduh95 Mar 17, 2024
0088a0e
f
aduh95 Mar 22, 2024
cdb8d45
u
aduh95 Mar 22, 2024
0bc914e
Update config.json
aduh95 Mar 22, 2024
cd24483
Update sources/types.ts
aduh95 Mar 28, 2024
582f894
Merge branch 'main' of https://github.com/nodejs/corepack
aduh95 Mar 28, 2024
fc62607
chore: fix tests on custom registry
aduh95 Mar 29, 2024
c42cfda
add tests for integrity verifications
aduh95 Mar 29, 2024
a5b3516
fixup! chore: fix tests on custom registry
aduh95 Mar 29, 2024
7812de1
fixup! add tests for integrity verifications
aduh95 Mar 29, 2024
c893071
Fix failing test (Yarn 3.0.0 is not signed on npm registry)
aduh95 Mar 29, 2024
715e642
verify signature for default version as well
aduh95 Mar 29, 2024
cb443d1
skip signature verification when hash is provided
aduh95 Mar 29, 2024
3404456
add tests for integrity
aduh95 Mar 30, 2024
86e9322
fix failing test when integrity mismatch
aduh95 Mar 30, 2024
193a83c
mark test as failing
aduh95 Mar 30, 2024
300a105
fixup
aduh95 Mar 30, 2024
6eb2ea8
Update sources/corepackUtils.ts
aduh95 Mar 30, 2024
f61e984
Merge branch 'main' of https://github.com/nodejs/corepack
aduh95 Apr 1, 2024
45ad4ce
add more tests for valid signature
aduh95 Apr 2, 2024
734ae46
fixup! add more tests for valid signature
aduh95 Apr 2, 2024
57a9b1e
fixup! add more tests for valid signature
aduh95 Apr 2, 2024
9f3d002
fixup! add more tests for valid signature
aduh95 Apr 2, 2024
9c62dc8
Increase timeout with the hope to get green on Windows
aduh95 Apr 12, 2024
e159667
try to parallelize more
aduh95 Apr 12, 2024
48aaff6
try something else
aduh95 Apr 12, 2024
7dcc648
remove longer timeout
aduh95 Apr 12, 2024
4b4d016
Merge branch 'main' of https://github.com/nodejs/corepack
aduh95 Apr 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
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
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
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
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`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why switching to sha512?

Copy link
Contributor Author

@aduh95 aduh95 Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that’s what npm registry uses for the signature: they sign a combination of the package name & version and the SHA512 of the package archive. See https://docs.npmjs.com/verifying-registry-signatures for reference.

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
@@ -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
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
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
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
@@ -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
@@ -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});
});
});