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 3 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
13 changes: 12 additions & 1 deletion config.json
Expand Up @@ -160,5 +160,16 @@
}
}
}
},
"keys": {
"npm": [
{
"expires": null,
"keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
"keytype": "ecdsa-sha2-nistp256",
"scheme": "ecdsa-sha2-nistp256",
"key": "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1Olb3zMAFFxXKHiIkQO5cJ3Yhl5i6UPp+IhuteBJbuHcA5UogKo0EWtlWwW6KSaKoTNEYL7JlCQiVnkhBktUgg=="
}
]
}
}
}
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
42 changes: 38 additions & 4 deletions sources/corepackUtils.ts
@@ -1,4 +1,4 @@
import {createHash} from 'crypto';
import {createHash, createVerify} from 'crypto';
import {once} from 'events';
import {FileHandle} from 'fs/promises';
import fs from 'fs';
Expand All @@ -8,6 +8,8 @@ import path from 'path';
import semver from 'semver';
import {setTimeout as setTimeoutPromise} from 'timers/promises';

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

aduh95 marked this conversation as resolved.
Show resolved Hide resolved
import * as engine from './Engine';
import * as debugUtils from './debugUtils';
import * as folderUtils from './folderUtils';
Expand Down Expand Up @@ -159,12 +161,14 @@ export async function installVersion(installTarget: string, locator: Locator, {s
}

let url: string;
let signatures: Array<{keyid: string, sig: string}>;
let integrity: string;
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));
} else {
url = url.replace(
npmRegistryUtils.DEFAULT_NPM_REGISTRY_URL,
Expand Down Expand Up @@ -200,7 +204,7 @@ export async function installVersion(installTarget: string, locator: Locator, {s

stream.pipe(sendTo);

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 hash = stream.pipe(createHash(algo));
await once(sendTo, `finish`);

Expand Down Expand Up @@ -234,7 +238,37 @@ export async function installVersion(installTarget: string, locator: Locator, {s
}
}

const actualHash = hash.digest(`hex`);
const actualHash: string = hash.digest(`hex`);
if (!build[1]) {
const registry = getRegistryFromPackageManagerSpec(spec);
if (registry.type === `npm` && !process.env.COREPACK_NPM_REGISTRY) {
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
if (signatures! == null || integrity! == null)
({signatures, integrity} = (await npmRegistryUtils.fetchTarballURLAndSignature(registry.package, version)));
const {npm: keys} = defaultConfig.keys;
const key = keys.find(({keyid}) => signatures.some(s => s.keyid === keyid));
const signature = signatures.find(({keyid}) => keyid === key?.keyid);
switch (key?.keytype) {
case `ecdsa-sha2-nistp256`: {
const message = `${registry.package}@${version}:${integrity}`;
const verifier = createVerify(`SHA256`);
verifier.write(message);
verifier.end();
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`);
break;
}

default: throw new Error(`Unsupported signature key`, {cause: key});
}
// @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
6 changes: 3 additions & 3 deletions sources/npmRegistryUtils.ts
Expand Up @@ -49,15 +49,15 @@ 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 metadata = await fetchAsJson(packageName);
const versionMetadata = metadata.versions?.[version];
if (versionMetadata === undefined)
throw new Error(`${packageName}@${version} does not exist.`);

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};
}
4 changes: 4 additions & 0 deletions sources/types.ts
Expand Up @@ -96,6 +96,10 @@ export interface Config {
};
};
};

keys: {
[registry: string]: Array<{"expires": null, "keyid": string, "keytype": string, "scheme": string, "key": string}>;
};
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
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
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});
});
});
Binary file modified tests/nocks.db
Binary file not shown.