Summary
verifyMcpbFile() in src/node/sign.ts can never report a .mcpb as signed or self-signed — it always returns { status: "unsigned" },
even for a file that was just signed with mcpb sign.
The function delegates signature verification to node-forge's PKCS#7 signed-data verify(). In node-forge that method is an unimplemented stub
that throws:
// node_modules/node-forge/lib/pkcs7.js
verify: function() {
throw new Error('PKCS#7 signature verification not yet implemented.');
}
This is present in every published node-forge 1.x, including 1.4.0 (the version the current "node-forge": "^1.3.2" range resolves to
today) — confirmed at lib/pkcs7.js:381.
Impact
mcpb is fail-closed here (nothing ever verifies as trusted, so this is not a signature-forgery/bypass), but the signing feature is
effectively non-functional: a signed publisher identity is indistinguishable from no signature at all on macOS, Windows, and Linux. The
certificate-chain / OS-trust verification (sign.ts:192-227) is unreachable dead code.
Steps to reproduce
mcpb sign extension.mcpb --cert cert.pem --key key.pem # succeeds
mcpb verify extension.mcpb # => unsigned
Control flow:
src/node/sign.ts:165 p7.verify({ authenticatedAttributes: true }) throws.
- caught at
src/node/sign.ts:188-190 → return { status: "unsigned" }.
Root cause
Reliance on forge.pkcs7.PkcsSignedData.verify(), which is not implemented in node-forge. The cast at sign.ts:142-150 adds a verify member to
the type, but the runtime method still throws.
Note: on a parsed message, p7.signers is also empty, so the signerInfos[0]-shaped access the current code assumes would not work even if
verify() were implemented. The parsed signer fields live on p7.rawCapture (signature, authenticatedAttributes).
Suggested fix
Verify the signer manually with node-forge's lower-level primitives instead of the stub: (1) confirm the messageDigest authenticated attribute
equals SHA-256(content), then (2) verify the signer's RSA signature over the DER of the authenticated attributes re-tagged as a SET
(PKCS#7/CMS rule).
const rc = (p7 as any).rawCapture as {
signature: string;
authenticatedAttributes: forge.asn1.Asn1[];
};
if (!rc?.signature || !rc?.authenticatedAttributes) {
return { status: "unsigned" };
}
// 1) messageDigest attribute must equal SHA-256(content)
const md = forge.md.sha256.create();
md.update(contentBuf.getBytes());
const contentDigest = md.digest().getBytes();
let messageDigest: string | null = null;
for (const attrSeq of rc.authenticatedAttributes) {
const oid = forge.asn1.derToOid((attrSeq as any).value[0].value);
if (oid === forge.pki.oids.messageDigest) {
messageDigest = (attrSeq as any).value[1].value[0].value;
break;
}
}
if (messageDigest === null || messageDigest !== contentDigest) {
return { status: "unsigned" };
}
// 2) RSA-verify the signature over the authenticatedAttributes SET
const attrsSet = forge.asn1.create(
forge.asn1.Class.UNIVERSAL,
forge.asn1.Type.SET,
true,
rc.authenticatedAttributes,
);
const attrsMd = forge.md.sha256.create();
attrsMd.update(forge.asn1.toDer(attrsSet).getBytes());
let signatureValid = false;
try {
signatureValid = (signingCert.publicKey as forge.pki.rsa.PublicKey)
.verify(attrsMd.digest().getBytes(), rc.signature);
} catch {
signatureValid = false;
}
if (!signatureValid) {
return { status: "unsigned" };
}
// ...existing cert-chain / OS-trust verification then runs as before.
I verified this approach locally against node-forge 1.4.0 with positive and negative test vectors:
- Current stubbed
verify() on a valid signature → throws, forcing unsigned (bug reproduced)
- Good signed content → verifies ✅
- Tampered content → rejected (messageDigest mismatch) ✅
- Flipped signature byte → rejected (RSA verify fails) ✅
- Attacker-signed blob for different content → rejected ✅
Caveats / for maintainer decision
- The snippet handles RSA signers only — which matches what
signMcpbFile currently produces. ECDSA signing certs would need a different
publicKey.verify path.
- Hand-rolled PKCS#7/CMS verification is delicate; please pair any fix with committed positive + negative test vectors. An alternative worth
considering is verifying via Node's built-in crypto (X509Certificate / crypto.verify) or the OS toolchain already used in
verifyCertificateChain, whichever you can best test against the signing pipeline.
Happy to open a PR with the fix + tests if you'd like.
Summary
verifyMcpbFile()insrc/node/sign.tscan never report a.mcpbassignedorself-signed— it always returns{ status: "unsigned" },even for a file that was just signed with
mcpb sign.The function delegates signature verification to node-forge's PKCS#7 signed-data
verify(). In node-forge that method is an unimplemented stubthat throws:
This is present in every published
node-forge1.x, including 1.4.0 (the version the current"node-forge": "^1.3.2"range resolves totoday) — confirmed at
lib/pkcs7.js:381.Impact
mcpbis fail-closed here (nothing ever verifies as trusted, so this is not a signature-forgery/bypass), but the signing feature iseffectively non-functional: a signed publisher identity is indistinguishable from no signature at all on macOS, Windows, and Linux. The
certificate-chain / OS-trust verification (
sign.ts:192-227) is unreachable dead code.Steps to reproduce
Control flow:
src/node/sign.ts:165p7.verify({ authenticatedAttributes: true })throws.src/node/sign.ts:188-190→return { status: "unsigned" }.Root cause
Reliance on
forge.pkcs7.PkcsSignedData.verify(), which is not implemented in node-forge. The cast atsign.ts:142-150adds averifymember tothe type, but the runtime method still throws.
Note: on a parsed message,
p7.signersis also empty, so thesignerInfos[0]-shaped access the current code assumes would not work even ifverify()were implemented. The parsed signer fields live onp7.rawCapture(signature,authenticatedAttributes).Suggested fix
Verify the signer manually with node-forge's lower-level primitives instead of the stub: (1) confirm the
messageDigestauthenticated attributeequals
SHA-256(content), then (2) verify the signer's RSA signature over the DER of the authenticated attributes re-tagged as aSET(PKCS#7/CMS rule).
I verified this approach locally against node-forge 1.4.0 with positive and negative test vectors:
verify()on a valid signature → throws, forcingunsigned(bug reproduced)Caveats / for maintainer decision
signMcpbFilecurrently produces. ECDSA signing certs would need a differentpublicKey.verifypath.considering is verifying via Node's built-in
crypto(X509Certificate/crypto.verify) or the OS toolchain already used inverifyCertificateChain, whichever you can best test against the signing pipeline.Happy to open a PR with the fix + tests if you'd like.