Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .changeset/21524-plugin-signature-ed25519-key-type.md
Original file line number Diff line number Diff line change
@@ -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)

<!-- adr-0087: not-required (no-migration-prescription) a refusal of non-Ed25519 signing and verifying keys by the plugin artifact signature functions in @objectstack/core. No authorable key, spelling, export or stored metadata shape moves: the change is which cryptographic keys signPayload and verifyPayload accept, and a key is an operational secret that no ledger entry or os migrate meta run can rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers the signature contract (not already-registered); and the changed exports are functions whose behaviour narrows, not an interface or type declaration (not runtime-interface-only or type-surface-only). -->
50 changes: 49 additions & 1 deletion packages/cli/test/plugin-sign.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
}
});
99 changes: 98 additions & 1 deletion packages/core/src/security/plugin-artifact-signature.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
});
});
78 changes: 71 additions & 7 deletions packages/core/src/security/plugin-artifact-signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*/
Expand All @@ -30,19 +37,45 @@ import {
createPublicKey,
createPrivateKey,
generateKeyPairSync,
type KeyObject,
KeyObject,
} from 'node:crypto';

export const SIGNATURE_ALG = 'ed25519';
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;
Expand All @@ -60,14 +93,19 @@ export function generateEd25519KeyPair(): { publicKeyPem: string; privateKeyPem:
/**
* Sign `payload` with an Ed25519 private key, returning the formatted
* signature string `ed25519:<keyId>:<base64url(sig)>`.
*
* 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,
privateKey: KeyInput,
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')}`;
}

Expand All @@ -93,16 +131,32 @@ 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,
publicKey: KeyInput,
): 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;
}
Expand Down Expand Up @@ -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 },
Expand All @@ -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;
Expand Down Expand Up @@ -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: {
Expand Down
Loading