diff --git a/src/node/sign.ts b/src/node/sign.ts index bd4c8c4..57668e6 100644 --- a/src/node/sign.ts +++ b/src/node/sign.ts @@ -41,69 +41,39 @@ export function signMcpbFile( readFileSync(path, "utf-8"), ); - // Create PKCS#7 signed data - const p7 = forge.pkcs7.createSignedData(); - p7.content = forge.util.createBuffer(mcpbContent); - - // Parse and add certificates - const signingCert = forge.pki.certificateFromPem(certificatePem); - const privateKey = forge.pki.privateKeyFromPem(privateKeyPem); + const initialSignature = createPkcs7Signature( + mcpbContent, + certificatePem, + privateKeyPem, + intermediatePems, + ); + let signatureBlockLength = createSignatureBlock(initialSignature).length; - p7.addCertificate(signingCert); + for (let attempts = 0; attempts < 3; attempts++) { + const updatedContent = updateEocdCommentLength( + mcpbContent, + signatureBlockLength, + ); + const signature = createPkcs7Signature( + updatedContent, + certificatePem, + privateKeyPem, + intermediatePems, + ); + const signatureBlock = createSignatureBlock(signature); - // Add intermediate certificates - if (intermediatePems) { - for (const pem of intermediatePems) { - p7.addCertificate(forge.pki.certificateFromPem(pem)); + if (signatureBlock.length !== signatureBlockLength) { + signatureBlockLength = signatureBlock.length; + continue; } - } - - // Add signer - p7.addSigner({ - key: privateKey, - certificate: signingCert, - digestAlgorithm: forge.pki.oids.sha256, - authenticatedAttributes: [ - { - type: forge.pki.oids.contentType, - value: forge.pki.oids.data, - }, - { - type: forge.pki.oids.messageDigest, - // Value will be auto-populated - }, - { - type: forge.pki.oids.signingTime, - // Value will be auto-populated with current time - }, - ], - }); - - // Sign with detached signature - p7.sign({ detached: true }); - // Convert to DER format - const asn1 = forge.asn1.toDer(p7.toAsn1()); - const pkcs7Signature = Buffer.from(asn1.getBytes(), "binary"); - - // Create signature block with PKCS#7 data - const signatureBlock = createSignatureBlock(pkcs7Signature); - - // Update ZIP EOCD comment_length to include signature block - // This ensures strict ZIP parsers accept the signed file - const updatedContent = Buffer.from(mcpbContent); - const eocdOffset = findEocdOffset(updatedContent); - if (eocdOffset !== -1) { - const currentCommentLength = updatedContent.readUInt16LE(eocdOffset + 20); - updatedContent.writeUInt16LE( - currentCommentLength + signatureBlock.length, - eocdOffset + 20, - ); + // Append signature block to MCPB file + const signedContent = Buffer.concat([updatedContent, signatureBlock]); + writeFileSync(mcpbPath, signedContent); + return; } - // Append signature block to MCPB file - const signedContent = Buffer.concat([updatedContent, signatureBlock]); - writeFileSync(mcpbPath, signedContent); + throw new Error("Failed to stabilize MCPB signature block length"); } /** @@ -137,17 +107,7 @@ export async function verifyMcpbFile( return { status: "unsigned" }; } - // Now we know it's PkcsSignedData. The types are incorrect, so we'll - // fix them there - const p7 = p7Message as unknown as forge.pkcs7.PkcsSignedData & { - signerInfos: Array<{ - authenticatedAttributes: Array<{ - type: string; - value: unknown; - }>; - }>; - verify: (options?: { authenticatedAttributes?: boolean }) => boolean; - }; + const p7 = p7Message as forge.pkcs7.PkcsSignedData; // Extract certificates from PKCS#7 const certificates = p7.certificates || []; @@ -158,34 +118,11 @@ export async function verifyMcpbFile( // Get the signing certificate (first one) const signingCert = certificates[0]; - // Verify PKCS#7 signature - const contentBuf = forge.util.createBuffer(originalContent); - - 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" }; - } - } - } catch (error) { + const signatureValid = await verifyPkcs7Signature( + pkcs7Signature, + originalContent, + ); + if (!signatureValid) { return { status: "unsigned" }; } @@ -195,22 +132,24 @@ export async function verifyMcpbFile( .slice(1) .map((cert) => Buffer.from(forge.pki.certificateToPem(cert))); - // Verify certificate chain against OS trust store - const chainValid = await verifyCertificateChain( - Buffer.from(certPem), - intermediatePems, - ); - - if (!chainValid) { - // Signature is valid but certificate is not trusted - return { status: "unsigned" }; - } - // Extract certificate info const isSelfSigned = signingCert.issuer.getField("CN")?.value === signingCert.subject.getField("CN")?.value; + if (!isSelfSigned) { + // Verify certificate chain against OS trust store + const chainValid = await verifyCertificateChain( + Buffer.from(certPem), + intermediatePems, + ); + + if (!chainValid) { + // Signature is valid but certificate is not trusted + return { status: "unsigned" }; + } + } + return { status: isSelfSigned ? "self-signed" : "signed", publisher: signingCert.subject.getField("CN")?.value || "Unknown", @@ -230,6 +169,138 @@ export async function verifyMcpbFile( } } +function createPkcs7Signature( + content: Buffer, + certificatePem: string, + privateKeyPem: string, + intermediatePems?: string[], +): Buffer { + const p7 = forge.pkcs7.createSignedData(); + p7.content = forge.util.createBuffer(content); + + const signingCert = forge.pki.certificateFromPem(certificatePem); + const privateKey = forge.pki.privateKeyFromPem(privateKeyPem); + + p7.addCertificate(signingCert); + + if (intermediatePems) { + for (const pem of intermediatePems) { + p7.addCertificate(forge.pki.certificateFromPem(pem)); + } + } + + p7.addSigner({ + key: privateKey, + certificate: signingCert, + digestAlgorithm: forge.pki.oids.sha256, + authenticatedAttributes: [ + { + type: forge.pki.oids.contentType, + value: forge.pki.oids.data, + }, + { + type: forge.pki.oids.messageDigest, + // Value will be auto-populated + }, + { + type: forge.pki.oids.signingTime, + // Value will be auto-populated with current time + }, + ], + }); + + p7.sign({ detached: true }); + + const asn1 = forge.asn1.toDer(p7.toAsn1()); + return Buffer.from(asn1.getBytes(), "binary"); +} + +function updateEocdCommentLength( + content: Buffer, + signatureBlockLength: number, +): Buffer { + const updatedContent = Buffer.from(content); + const eocdOffset = findEocdOffset(updatedContent); + if (eocdOffset === -1) { + return updatedContent; + } + + const currentCommentLength = updatedContent.readUInt16LE(eocdOffset + 20); + const updatedCommentLength = currentCommentLength + signatureBlockLength; + if (updatedCommentLength > 0xffff) { + throw new Error("Signature block exceeds ZIP comment length limit"); + } + + updatedContent.writeUInt16LE(updatedCommentLength, eocdOffset + 20); + return updatedContent; +} + +async function verifyPkcs7Signature( + pkcs7Signature: Buffer, + originalContent: Buffer, +): Promise { + let tempDir: string | null = null; + + try { + tempDir = await mkdtemp(join(tmpdir(), "mcpb-pkcs7-")); + const signaturePath = join(tempDir, "signature.der"); + const contentPath = join(tempDir, "content.bin"); + const outputPath = join(tempDir, "verified-content.bin"); + + await writeFile(signaturePath, pkcs7Signature); + await writeFile(contentPath, originalContent); + + if (process.platform === "win32") { + const psCommand = ` + $ErrorActionPreference = 'Stop' + $content = [System.IO.File]::ReadAllBytes('${escapePowerShellString(contentPath)}') + $signature = [System.IO.File]::ReadAllBytes('${escapePowerShellString(signaturePath)}') + $contentInfo = New-Object System.Security.Cryptography.Pkcs.ContentInfo -ArgumentList @(,$content) + $signedCms = New-Object System.Security.Cryptography.Pkcs.SignedCms -ArgumentList $contentInfo, $true + $signedCms.Decode($signature) + $signedCms.CheckSignature($true) + `.trim(); + + await execFileAsync("powershell.exe", [ + "-NoProfile", + "-NonInteractive", + "-Command", + psCommand, + ]); + } else { + await execFileAsync("openssl", [ + "cms", + "-verify", + "-inform", + "DER", + "-in", + signaturePath, + "-content", + contentPath, + "-noverify", + "-binary", + "-out", + outputPath, + ]); + } + return true; + } catch { + return false; + } finally { + if (tempDir) { + try { + await rm(tempDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } + } + } +} + +function escapePowerShellString(value: string): string { + return value.replace(/'/g, "''"); +} + /** * Finds the offset of the ZIP End of Central Directory record * by scanning backwards for the EOCD magic bytes (0x06054b50) diff --git a/test/sign.e2e.test.ts b/test/sign.e2e.test.ts index 9af52a0..3ffd900 100755 --- a/test/sign.e2e.test.ts +++ b/test/sign.e2e.test.ts @@ -341,8 +341,7 @@ async function testSelfSignedSigning() { // Verify the signature const result = await verifyMcpbFile(testFile); - // Self-signed certs may not be trusted by OS, so we accept either status - expect(["self-signed", "unsigned"]).toContain(result.status); + expect(result.status).toBe("self-signed"); // Clean up fs.unlinkSync(testFile);