From f80ff7b770cccdb8a9bbd47eb7a599ef8a74f9c9 Mon Sep 17 00:00:00 2001 From: Bryan Thompson Date: Thu, 12 Mar 2026 09:10:05 -0500 Subject: [PATCH] fix: implement manual PKCS#7 signature verification (#21) Replace non-functional node-forge p7.verify() with manual PKCS#7 verification. node-forge throws "not yet implemented" on verify(), causing mcpb verify to always report unsigned status. The fix manually verifies the message digest in authenticated attributes matches the content hash, then verifies the RSA signature over the authenticated attributes using the certificate's public key. Also fixes self-signed certificate detection to correctly report "self-signed" status even when chain validation fails. Original fix by Joan Xie. Co-Authored-By: Joan Xie Co-Authored-By: Claude Opus 4.6 (1M context) --- src/node/sign.ts | 146 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 110 insertions(+), 36 deletions(-) diff --git a/src/node/sign.ts b/src/node/sign.ts index e0ed5ce..84535ad 100644 --- a/src/node/sign.ts +++ b/src/node/sign.ts @@ -127,13 +127,18 @@ export async function verifyMcpbFile( // Now we know it's PkcsSignedData. The types are incorrect, so we'll // fix them there + interface AuthenticatedAttribute { + type: string; + value: string | forge.asn1.Asn1; + } + + interface SignerInfo { + authenticatedAttributes: AuthenticatedAttribute[]; + signature: string; + } + const p7 = p7Message as unknown as forge.pkcs7.PkcsSignedData & { - signerInfos: Array<{ - authenticatedAttributes: Array<{ - type: string; - value: unknown; - }>; - }>; + signerInfos: SignerInfo[]; verify: (options?: { authenticatedAttributes?: boolean }) => boolean; }; @@ -146,37 +151,104 @@ export async function verifyMcpbFile( // Get the signing certificate (first one) const signingCert = certificates[0]; - // Verify PKCS#7 signature - const contentBuf = forge.util.createBuffer(originalContent); - + // Manually verify PKCS#7 signature (node-forge's verify() is not implemented) try { - p7.verify({ authenticatedAttributes: true }); - - // Also verify the content matches const signerInfos = p7.signerInfos; const signerInfo = signerInfos?.[0]; - if (signerInfo) { - const md = forge.md.sha256.create(); - md.update(contentBuf.getBytes()); - const digest = md.digest().getBytes(); - - // Find the message digest attribute - let messageDigest = null; - for (const attr of signerInfo.authenticatedAttributes) { - if (attr.type === forge.pki.oids.messageDigest) { - messageDigest = attr.value; - break; - } - } - if (!messageDigest || messageDigest !== digest) { - return { status: "unsigned" }; + if (!signerInfo) { + return { status: "unsigned" }; + } + + // Step 1: Verify the message digest in authenticated attributes matches the content + const md = forge.md.sha256.create(); + md.update(forge.util.createBuffer(originalContent).getBytes()); + const contentDigest = md.digest().getBytes(); + + // Find and verify the message digest attribute + let messageDigestAttr = null; + for (const attr of signerInfo.authenticatedAttributes) { + if (attr.type === forge.pki.oids.messageDigest) { + messageDigestAttr = attr.value; + break; } } + + if (!messageDigestAttr || messageDigestAttr !== contentDigest) { + return { status: "unsigned" }; + } + + // Step 2: Verify the signature over the authenticated attributes + // Create a DER encoding of the authenticated attributes for signature verification + const authenticatedAttributesAsn1 = forge.asn1.create( + forge.asn1.Class.UNIVERSAL, + forge.asn1.Type.SET, + true, + signerInfo.authenticatedAttributes.map((attr: AuthenticatedAttribute) => + forge.asn1.create( + forge.asn1.Class.UNIVERSAL, + forge.asn1.Type.SEQUENCE, + true, + [ + forge.asn1.create( + forge.asn1.Class.UNIVERSAL, + forge.asn1.Type.OID, + false, + forge.asn1.oidToDer(attr.type).getBytes(), + ), + forge.asn1.create( + forge.asn1.Class.UNIVERSAL, + forge.asn1.Type.SET, + true, + [ + typeof attr.value === "string" + ? forge.asn1.create( + forge.asn1.Class.UNIVERSAL, + forge.asn1.Type.OCTETSTRING, + false, + attr.value, + ) + : attr.value, + ], + ), + ], + ), + ), + ); + + const bytes = forge.asn1.toDer(authenticatedAttributesAsn1).getBytes(); + + // Hash the authenticated attributes + const attrMd = forge.md.sha256.create(); + attrMd.update(bytes); + + // Verify the signature using the certificate's public key + // Cast to rsa.PublicKey since PKCS#7 typically uses RSA + const publicKey = signingCert.publicKey as forge.pki.rsa.PublicKey; + if ( + !publicKey || + (typeof publicKey === "object" && Buffer.isBuffer(publicKey)) + ) { + return { status: "unsigned" }; + } + + const verified = publicKey.verify( + attrMd.digest().getBytes(), + signerInfo.signature, + ); + + if (!verified) { + return { status: "unsigned" }; + } } catch (error) { return { status: "unsigned" }; } + // Check if certificate is self-signed + const isSelfSigned = + signingCert.issuer.getField("CN")?.value === + signingCert.subject.getField("CN")?.value; + // Convert forge certificate to PEM for OS verification const certPem = forge.pki.certificateToPem(signingCert); const intermediatePems = certificates @@ -189,18 +261,20 @@ export async function verifyMcpbFile( intermediatePems, ); - if (!chainValid) { - // Signature is valid but certificate is not trusted - return { status: "unsigned" }; + // Determine status based on trust validation + let status: "signed" | "self-signed" | "unsigned"; + if (chainValid) { + // Certificate is trusted by OS + status = isSelfSigned ? "self-signed" : "signed"; + } else { + // Signature is cryptographically valid but certificate is not trusted + // For self-signed certificates, still report as self-signed (not unsigned) + // For other certificates, report as unsigned (untrusted) + status = isSelfSigned ? "self-signed" : "unsigned"; } - // Extract certificate info - const isSelfSigned = - signingCert.issuer.getField("CN")?.value === - signingCert.subject.getField("CN")?.value; - return { - status: isSelfSigned ? "self-signed" : "signed", + status, publisher: signingCert.subject.getField("CN")?.value || "Unknown", issuer: signingCert.issuer.getField("CN")?.value || "Unknown", valid_from: signingCert.validity.notBefore.toISOString(),