diff --git a/.changeset/21524-plugin-signature-ed25519-key-type.md b/.changeset/21524-plugin-signature-ed25519-key-type.md new file mode 100644 index 0000000000..439403a8f1 --- /dev/null +++ b/.changeset/21524-plugin-signature-ed25519-key-type.md @@ -0,0 +1,24 @@ +--- +'@objectstack/core': minor +--- + +fix(core)!: the plugin artifact signature contract refuses any key that is not Ed25519, so its `ed25519` label now holds (#21524) + +**BREAKING**: `signPayload` and `verifyPayload` (the plugin artifact signature contract in `@objectstack/core`) now refuse a key whose type is not Ed25519. Until now they accepted any asymmetric key. node's `sign(null, …)` and `verify(null, …)` follow the key they are handed, so an RSA, EC or Ed448 key signed under the `ed25519:KEYID:SIG` label and verified against its own public half. `os plugin sign --key` with an RSA private key exited 0, printed `Plugin signed`, and wrote an `ed25519:`-labelled sidecar over an RSA signature. + +What is refused now: + +- **`signPayload`** throws when the private key is not Ed25519. The error names the key type found (`rsa`, `ec`, `ed448`, and `secret` for a symmetric key). +- **`verifyPayload`** throws when the verifying key's type is not the algorithm the signature's label names. The label is checked against the key, not trusted, and the only label the contract parses is `ed25519`. The error names the key type found. +- **`verifyPublisherSignature`, `verifyPlatformSignature` and `verifyPluginArtifact`** verify through `verifyPayload`. So a publisher key registry entry or a platform key that is not Ed25519 makes them throw, or reject, with that same error. It is not folded into a `false` or an `ok: false` result, because a wrong key is the verifier's own configuration, not a verdict on the artifact. +- **`os plugin sign`** prints one `✗ Signing failed: signPayload: …` line naming the key type, exits 1, and writes no sidecar. + +Each refusal is a plain `Error`, the error style the module already used. + +**The fix:** sign with an Ed25519 key, generated with `openssl genpkey -algorithm ed25519` or `generateEd25519KeyPair()`. Configure Ed25519 public keys for the publisher key registry and the platform key. A signature made earlier with a non-Ed25519 key cannot be verified any more. Sign the artifact again with an Ed25519 key. + +**Unchanged:** an Ed25519 key signs and verifies exactly as before, with the same deterministic signature bytes. That holds for a PEM string, a `KeyObject`, and the PEM buffer, DER and JWK inputs node also accepts. A malformed signature string, a signature that does not verify, and a key that cannot be read still answer `false`. The signature string format and every export are unchanged. + +Clause-②: no (narrowing) + + diff --git a/packages/cli/test/plugin-sign.test.ts b/packages/cli/test/plugin-sign.test.ts index a5c49d4e6c..5a9e21e2fd 100644 --- a/packages/cli/test/plugin-sign.test.ts +++ b/packages/cli/test/plugin-sign.test.ts @@ -1,6 +1,8 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { describe, it, expect, afterAll } from 'vitest'; +import { describe, it, expect, afterAll, afterEach, vi } from 'vitest'; +import { generateKeyPairSync } from 'node:crypto'; +import { existsSync } from 'node:fs'; import { gunzipSync } from 'node:zlib'; import { mkdtemp, rm, mkdir, writeFile, readFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -59,3 +61,49 @@ describe('os plugin sign (end-to-end, build → sign → verify)', () => { expect(() => gunzipSync(Buffer.from(artifactBytes))).not.toThrow(); }); }); + +describe('os plugin sign refuses a publisher key that is not Ed25519', () => { + // Without the key-type check an RSA key signed, printed `Plugin signed`, + // exited 0 and wrote an `ed25519:`-labelled sidecar over an RSA signature. + const dirs: string[] = []; + afterEach(() => vi.restoreAllMocks()); + afterAll(async () => { + for (const d of dirs) await rm(d, { recursive: true, force: true }); + }); + + const foreign = [ + { type: 'rsa', pair: () => generateKeyPairSync('rsa', { modulusLength: 2048 }) }, + { type: 'ec', pair: () => generateKeyPairSync('ec', { namedCurve: 'P-256' }) }, + ] as const; + + for (const { type, pair } of foreign) { + it(`refuses an ${type} private key: exit 1, the refusal names '${type}', no sidecar`, async () => { + const dir = await mkdtemp(join(tmpdir(), 'osplugin-sign-keytype-')); + dirs.push(dir); + const artifactPath = join(dir, 'com.acme.keytype-1.0.0.osplugin'); + await writeFile(artifactPath, Buffer.from('artifact bytes')); + const keyPath = join(dir, `${type}.key.pem`); + await writeFile(keyPath, pair().privateKey.export({ type: 'pkcs8', format: 'pem' }).toString()); + + const printed: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => void printed.push(a.join(' '))); + + let exit: number | undefined; + try { + await PluginSign.run([artifactPath, '--key', keyPath]); + } catch (err) { + exit = (err as { oclif?: { exit?: number } }).oclif?.exit; + if (typeof exit !== 'number') throw err; + } + + expect(exit).toBe(1); + const errors = printed.filter((line) => line.includes('✗')); + expect(errors).toHaveLength(1); + // The refusal is signPayload's: the key is refused before anything is + // signed, not caught afterwards by the self-verification step. + expect(errors[0]).toMatch(new RegExp(`signPayload: the private key is of type '${type}'`)); + expect(printed.join('\n')).not.toContain('Plugin signed'); + expect(existsSync(`${artifactPath}.sig`)).toBe(false); + }); + } +}); diff --git a/packages/core/src/security/plugin-artifact-signature.test.ts b/packages/core/src/security/plugin-artifact-signature.test.ts index d81714abbd..d1f65844d7 100644 --- a/packages/core/src/security/plugin-artifact-signature.test.ts +++ b/packages/core/src/security/plugin-artifact-signature.test.ts @@ -1,7 +1,15 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { createPublicKey, createPrivateKey } from 'node:crypto'; +import { + createPublicKey, + createPrivateKey, + createSecretKey, + generateKeyPairSync, + sign as cryptoSign, + verify as cryptoVerify, + type KeyObject, +} from 'node:crypto'; import { counterSignPayload, generateEd25519KeyPair, @@ -145,3 +153,92 @@ describe('plugin-artifact-signature: KeyObject inputs', () => { expect(verifyPayload(artifact, sig, pub)).toBe(true); }); }); + +describe('plugin-artifact-signature: the Ed25519 algorithm is enforced by key type', () => { + // node's sign(null, …) / verify(null, …) follow whatever key they are + // handed, so without a key-type check an RSA or EC key signs under the + // `ed25519` label and verifies against its own public half. + const foreign = [ + { type: 'rsa', pair: generateKeyPairSync('rsa', { modulusLength: 2048 }) }, + { type: 'ec', pair: generateKeyPairSync('ec', { namedCurve: 'P-256' }) }, + ] as const; + + /** A signature the unchecked contract used to emit: a foreign algorithm under the ed25519 label. */ + function mislabelled(priv: KeyObject): string { + return `ed25519:k:${cryptoSign(null, artifact, priv).toString('base64url')}`; + } + + for (const { type, pair } of foreign) { + const privPem = pair.privateKey.export({ type: 'pkcs8', format: 'pem' }).toString(); + const pubPem = pair.publicKey.export({ type: 'spki', format: 'pem' }).toString(); + const named = new RegExp(`of type '${type}'`); + + it(`signPayload refuses an ${type} private key, naming its type (PEM and KeyObject)`, () => { + expect(() => signPayload(artifact, privPem, 'k')).toThrow(named); + expect(() => signPayload(artifact, pair.privateKey, 'k')).toThrow(named); + }); + + it(`verifyPayload refuses an ${type} public key, naming its type (PEM and KeyObject)`, () => { + const sig = mislabelled(pair.privateKey); + // The signature is cryptographically valid for this key: only the key-type + // check stands between it and `true`. + expect(cryptoVerify(null, artifact, pair.publicKey, parseSignature(sig)!.signature)).toBe(true); + expect(() => verifyPayload(artifact, sig, pubPem)).toThrow(named); + expect(() => verifyPayload(artifact, sig, pair.publicKey)).toThrow(named); + }); + + it(`every trust path of verifyPluginArtifact refuses an ${type} key`, async () => { + const sig = mislabelled(pair.privateKey); + const v = { package_id: 'com.acme.p', version: '1.0.0', signature: sig }; + // Publisher key registry hands back an rsa/ec key. + await expect( + verifyPluginArtifact( + { artifact, version: v }, + { getPublisherPublicKey: () => pubPem, requirePlatform: false }, + ), + ).rejects.toThrow(named); + // Platform key is an rsa/ec key. + const pubSig = signPayload(artifact, privateKeyPem, 'pub'); + const counter = `ed25519:platform:${cryptoSign(null, Buffer.from(counterSignPayload({ ...v, signature: pubSig })), pair.privateKey).toString('base64url')}`; + await expect( + verifyPluginArtifact( + { artifact, version: { ...v, signature: pubSig, platform_signature: counter } }, + { platformPublicKey: pubPem, getPublisherPublicKey: () => publicKeyPem }, + ), + ).rejects.toThrow(named); + }); + } + + it('an Ed25519 key signs and verifies as before, as PEM and as KeyObject', () => { + const pemSig = signPayload(artifact, privateKeyPem, 'k'); + const objSig = signPayload(artifact, createPrivateKey(privateKeyPem), 'k'); + expect(objSig).toBe(pemSig); + expect(verifyPayload(artifact, pemSig, publicKeyPem)).toBe(true); + expect(verifyPayload(artifact, pemSig, createPublicKey(publicKeyPem))).toBe(true); + // A private key still verifies through its public half, as node allows. + expect(verifyPayload(artifact, pemSig, privateKeyPem)).toBe(true); + }); + + it('a label that disagrees with the key type is refused, in both directions', () => { + const [{ pair: rsa }] = foreign; + const rsaPub = rsa.publicKey.export({ type: 'spki', format: 'pem' }).toString(); + const rsaSig = cryptoSign(null, artifact, rsa.privateKey).toString('base64url'); + // `ed25519` label over an rsa key: the label is checked against the key, not trusted. + expect(() => verifyPayload(artifact, `ed25519:k:${rsaSig}`, rsaPub)).toThrow(/of type 'rsa'/); + // A truthful `rsa` label is not a label this contract accepts at all. + expect(verifyPayload(artifact, `rsa:k:${rsaSig}`, rsaPub)).toBe(false); + // A valid Ed25519 signature relabelled as anything else does not verify. + const edSig = signPayload(artifact, privateKeyPem, 'k').slice('ed25519:'.length); + expect(verifyPayload(artifact, `rsa:${edSig}`, publicKeyPem)).toBe(false); + }); + + it('a symmetric key is refused as `secret`', () => { + expect(() => signPayload(artifact, createSecretKey(Buffer.alloc(32)), 'k')).toThrow(/of type 'secret'/); + }); + + it('an unreadable key or a malformed signature still answers false, not a throw', () => { + const sig = signPayload(artifact, privateKeyPem, 'k'); + expect(verifyPayload(artifact, sig, 'not a pem')).toBe(false); + expect(verifyPayload(artifact, 'garbage', publicKeyPem)).toBe(false); + }); +}); diff --git a/packages/core/src/security/plugin-artifact-signature.ts b/packages/core/src/security/plugin-artifact-signature.ts index 57da0ca738..e80baaed5f 100644 --- a/packages/core/src/security/plugin-artifact-signature.ts +++ b/packages/core/src/security/plugin-artifact-signature.ts @@ -20,6 +20,13 @@ * short, deterministic, no padding ambiguity. The `keyId` is an opaque * rotation handle used to resolve the verifying public key. * + * The algorithm is ENFORCED, not just labelled. node's `sign(null, …)` and + * `verify(null, …)` follow whatever key they are handed, so an RSA or EC key + * would otherwise sign and verify under the `ed25519` label. Both + * {@link signPayload} and {@link verifyPayload} therefore refuse, by throwing, + * any key whose type is not the signature's algorithm, and the refusal names + * the key type found ({@link requireSignatureKeyType} is the one check). + * * The two trust chains the runtime checks before loading a third-party * plugin are combined in {@link verifyPluginArtifact}. */ @@ -30,7 +37,7 @@ import { createPublicKey, createPrivateKey, generateKeyPairSync, - type KeyObject, + KeyObject, } from 'node:crypto'; export const SIGNATURE_ALG = 'ed25519'; @@ -38,11 +45,37 @@ const SIG_PREFIX = 'ed25519:'; export type KeyInput = string | KeyObject; +// A KeyObject is used as given; every other input (a PEM string, and the PEM +// buffer / DER / JWK inputs node also accepts at runtime) is parsed into one, +// so its key type can be read before anything is signed or verified. function toPrivateKey(key: KeyInput): KeyObject { - return typeof key === 'string' ? createPrivateKey(key) : key; + return key instanceof KeyObject ? key : createPrivateKey(key); } function toPublicKey(key: KeyInput): KeyObject { - return typeof key === 'string' ? createPublicKey(key) : key; + return key instanceof KeyObject ? key : createPublicKey(key); +} + +/** + * The contract's one key rule: a key signs or verifies only when its type IS + * the signature's algorithm. On the signing side that algorithm is + * {@link SIGNATURE_ALG}; on the verifying side it is the label the signature + * string carries ({@link ParsedSignature.alg}), so the label is checked against + * the verifying key's type instead of being trusted. Throws, naming the key + * type found (`rsa`, `ec`, `ed448`, … or `secret` for a symmetric key). + */ +function requireSignatureKeyType( + key: KeyObject, + alg: ParsedSignature['alg'], + use: 'signPayload' | 'verifyPayload', +): void { + if (key.asymmetricKeyType === alg) return; + const found = key.type === 'secret' ? 'secret' : String(key.asymmetricKeyType); + const role = use === 'signPayload' ? 'private key' : 'public key'; + throw new Error( + `${use}: the ${role} is of type '${found}', but plugin artifact signatures are '${alg}' only;` + + ` a key of that type would sign or verify a non-${alg} signature under the '${alg}' label.` + + ` Use an Ed25519 key (generate one with \`openssl genpkey -algorithm ed25519\`).`, + ); } function toBytes(payload: string | Uint8Array): Uint8Array { return typeof payload === 'string' ? new TextEncoder().encode(payload) : payload; @@ -60,6 +93,9 @@ export function generateEd25519KeyPair(): { publicKeyPem: string; privateKeyPem: /** * Sign `payload` with an Ed25519 private key, returning the formatted * signature string `ed25519::`. + * + * Throws when the private key is not Ed25519 (an RSA, EC, Ed448, … key), and + * the error names the key type found. */ export function signPayload( payload: string | Uint8Array, @@ -67,7 +103,9 @@ export function signPayload( keyId = 'default', ): string { if (keyId.includes(':')) throw new Error('keyId must not contain ":"'); - const sig = cryptoSign(null, toBytes(payload), toPrivateKey(privateKey)); + const key = toPrivateKey(privateKey); + requireSignatureKeyType(key, SIGNATURE_ALG, 'signPayload'); + const sig = cryptoSign(null, toBytes(payload), key); return `${SIG_PREFIX}${keyId}:${sig.toString('base64url')}`; } @@ -93,7 +131,16 @@ export function parseSignature(s: string | undefined | null): ParsedSignature | } } -/** Verify a formatted signature string over `payload` with the given public key. */ +/** + * Verify a formatted signature string over `payload` with the given public key. + * + * Returns `false` for a malformed signature string, an unreadable key, or a + * signature that does not verify. Throws when the key's type is not the + * algorithm the signature's label names (any key that is not Ed25519), and the + * error names the key type found: such a key is the verifier's own trust + * configuration being wrong, never a verdict on the signed bytes, so it is not + * folded into `false`. + */ export function verifyPayload( payload: string | Uint8Array, signature: string, @@ -101,8 +148,15 @@ export function verifyPayload( ): boolean { const parsed = parseSignature(signature); if (!parsed) return false; + let key: KeyObject; + try { + key = toPublicKey(publicKey); + } catch { + return false; + } + requireSignatureKeyType(key, parsed.alg, 'verifyPayload'); try { - return cryptoVerify(null, toBytes(payload), toPublicKey(publicKey), parsed.signature); + return cryptoVerify(null, toBytes(payload), key, parsed.signature); } catch { return false; } @@ -144,6 +198,8 @@ export interface PublisherVerifyResult { * - no signature → ok, verified=false (caller decides via trust tier). * - malformed / fails verification → NOT ok. * - unknown keyId → NOT ok (never silently trust). + * - a resolved key that is not Ed25519 → throws (from {@link verifyPayload}): + * the key registry is misconfigured, which is not a verdict on the artifact. */ export async function verifyPublisherSignature( args: { artifact: Uint8Array; signature?: string | null }, @@ -167,7 +223,10 @@ export async function verifyPublisherSignature( : { ok: false, verified: false, reason: 'publisher signature does not match artifact' }; } -/** Verify a platform counter-signature against the version identity + platform public key. */ +/** + * Verify a platform counter-signature against the version identity + platform + * public key. Throws when the platform key is not Ed25519 (see {@link verifyPayload}). + */ export function verifyPlatformSignature( version: { package_id: string; @@ -196,6 +255,11 @@ export interface PluginArtifactVerifyResult { * marketplace attestation; the publisher signature additionally binds the * exact bytes. `requirePlatform` (default true) rejects artifacts that lack * a valid platform counter-sign — set false for first-party / local builds. + * + * Both verifying keys come from `keys` — the caller's platform key and its + * publisher key registry; the artifact contributes only the signature strings + * and the `keyId` that selects a registry entry. A configured key that is not + * Ed25519 makes this reject (see {@link verifyPayload}) rather than resolve. */ export async function verifyPluginArtifact( input: {