fix(pe): enforce DER extension critical encoding (#10212)

src/native/pe/PeCertificateAsn1.ts

@@ -442,6 +442,11 @@ const requireExtensions = (extensions: readonly Asn1BerNode[]): void => {
       return fail(
         "TBSCertificate extension is not an OID, optional BOOLEAN, and OCTET STRING.",
       );
+    // X.690 11.1 requires DER TRUE=0xff; 11.5 omits DEFAULT FALSE.
+    if (critical !== undefined && critical.content[0] !== 0xff)
+      return fail(
+        "TBSCertificate extension critical must be omitted for FALSE or encoded as DER TRUE (0xff).",
+      );
     const oid = decodeOid(extnId.content);
     if (seen.has(oid)) return fail(`TBSCertificate repeats extension ${oid}.`);
     seen.add(oid);

src/native/pe/PeCertificateParser.test.ts

@@ -1274,6 +1274,10 @@ it("rejects malformed and repeated certificate extensions", () => {
     derSeq(ski, derInt(1)),
     derSeq(ski, derInt(1), skiValue),
     derSeq(ski, critical(0xff, 0xff), skiValue),
+    // X.690 11.1 requires DER TRUE=0xff; 11.5 omits DEFAULT FALSE.
+    ...[0x00, 0x01, 0x7f, 0xfe].map((value) =>
+      derSeq(ski, critical(value), skiValue),
+    ),
     derSeq(ski, skiValue, critical(0xff)),
     derSeq(ski, skiValue, der(0x05, Buffer.alloc(0))),
   ])

src/native/pe/PeCertificates.fixture.ts

@@ -68,7 +68,9 @@ export const pkcs7SignedDataFixture = (
   organization = "REA Tests",
   signature = octet(Buffer.from("sig")),
   unsignedAttributes?: readonly (readonly number[])[],
-  names: Partial<Record<"issuer" | "subject" | "signerIssuer", Buffer>> = {},
+  fields: Partial<
+    Record<"issuer" | "subject" | "signerIssuer" | "extensions", Buffer>
+  > = {},
 ): Buffer => {
   const serial = Buffer.from([0x11, 0x22, 0x33, 0x44]);
   const subject = name(commonName, organization);
@@ -76,13 +78,14 @@ export const pkcs7SignedDataFixture = (
     context(0, integer(2)),
     integer(serial),
     algorithm(SHA256_RSA),
-    names.issuer ?? subject,
+    fields.issuer ?? subject,
     seq(utc("250101000000Z"), utc("260101000000Z")),
-    names.subject ?? subject,
+    fields.subject ?? subject,
     seq(algorithm(RSA), bitString(Buffer.from("pubkey"))),
     context(
       3,
-      seq(seq(oid([2, 5, 29, 14]), octet(Buffer.from([0x04, 0x01, 0xaa])))),
+      fields.extensions ??
+        seq(seq(oid([2, 5, 29, 14]), octet(Buffer.from([0x04, 0x01, 0xaa])))),
     ),
   );
   const certificate = seq(
@@ -92,7 +95,7 @@ export const pkcs7SignedDataFixture = (
   );
   const signer = seq(
     integer(1),
-    seq(names.signerIssuer ?? subject, integer(serial)),
+    seq(fields.signerIssuer ?? subject, integer(serial)),
     algorithm(SHA256),
     algorithm(SHA256_RSA),
     signature,

tests/acceptance/peCertificatesCli.test.ts

@@ -10,9 +10,79 @@ import {
   pkcs7SignedDataIndefiniteEnvelopeFixture,
 } from "../../src/native/pe/PeCertificates.fixture.js";
 import { repeatedCertificateSignedData } from "../fixtures/peCertificateAllowance.js";
+import { parseMcpToolError } from "../fixtures/mcpToolError.js";
 import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";
 import { cliTest, runFileCli } from "../support/cli/cliFixture.js";
 
+cliTest.for([0x00, 0x01, 0x7f, 0xfe, 0xff, null])(
+  "requires canonical extension critical encoding %s through CLI and MCP",
+  async (critical, { cli }) => {
+    const root = await createTestTempDirectory("rea-pe-cert-critical-");
+    const path = join(root, "signed.exe");
+    // Extension ::= SEQUENCE { extnID, critical BOOLEAN DEFAULT FALSE, extnValue }.
+    // The extension OID and opaque value stay identical in every control.
+    const fields = Buffer.concat([
+      Buffer.from([0x06, 0x03, 0x55, 0x1d, 0x0e]),
+      critical === null ? Buffer.alloc(0) : Buffer.from([0x01, 0x01, critical]),
+      Buffer.from([0x04, 0x03, 0x04, 0x01, 0xaa]),
+    ]);
+    const extension = Buffer.concat([
+      Buffer.from([0x30, fields.length]),
+      fields,
+    ]);
+    const extensions = Buffer.concat([
+      Buffer.from([0x30, extension.length]),
+      extension,
+    ]);
+    const bytes = peCertificateFixture(
+      false,
+      pkcs7SignedDataFixture(undefined, undefined, undefined, undefined, {
+        extensions,
+      }),
+    );
+    await writeFile(path, bytes);
+    const result = await runFileCli(cli, {
+      arguments: ["inspect-pe-certificates", path, "--json"],
+    });
+    const call = await cli.sharedMcpCall!();
+    const response = await call("inspect_pe_certificates", { path });
+    const valid = critical === null || critical === 0xff;
+    expect(result.exitCode, result.stdout).toBe(valid ? 0 : 1);
+    if (valid)
+      expect(response.isError, JSON.stringify(response)).not.toBe(true);
+    else expect(response.isError, JSON.stringify(response)).toBe(true);
+    for (const value of [
+      result.json,
+      valid ? response.structuredContent : parseMcpToolError(response).error,
+    ]) {
+      if (valid) {
+        const report = toolContract(
+          "inspect_pe_certificates",
+        ).outputSchema.parse(value).normalized_result;
+        expect(report.coverage.status).toBe("complete");
+        expect(report.entries[0]?.pkcs7?.certificates[0]?.serial_hex).toBe(
+          "11223344",
+        );
+      } else {
+        expect(value).toMatchObject({
+          code: "invalid_request",
+          details: {
+            operation: "inspect_pe_certificates",
+            issues: [
+              expect.objectContaining({
+                path: ["path"],
+                reason: "invalid_format",
+                message: expect.stringContaining("critical"),
+              }),
+            ],
+          },
+        });
+      }
+    }
+    expect((await readFile(path)).equals(bytes)).toBe(true);
+  },
+);
+
 cliTest.for(["extra-field", "empty-rdn"] as const)(
   "rejects a malformed certificate name %s through CLI and MCP",
   async (kind, { cli }) => {