diff --git a/docs/apk-signature.md b/docs/apk-signature.md index ece167d0b1..b25482022e 100644 --- a/docs/apk-signature.md +++ b/docs/apk-signature.md @@ -99,10 +99,13 @@ share the application workflow, projection, retention and publication owners. certificate (`identified_by: subject-key-identifier`), adds a limitation, and treats that signer as unknown in `v1_signers_match_block_signers`. `certificates` lists every certificate the block carries (SHA-256 and subject) and `signer_infos` gives each - `SignerInfo`'s certificate and digest/signature algorithm OIDs. + `SignerInfo`'s certificate and digest/signature algorithm OIDs. The PKCS#7 + `SignedData` envelope is read as BER, including constructed signature OCTET + STRINGs; each embedded certificate is still interpreted as DER by the X.509 + owner. `certificate` is `null` when the block has more than one `SignerInfo` (with nothing verified there is no single signer) or names a certificate it does - not carry. A block that cannot be read (malformed DER, an unsupported + not carry. A block that cannot be read (malformed ASN.1, an unsupported compression method, Zip64 sizes, a local header that disagrees with the central directory on the name, data-descriptor flag, CRC-32 or a size, as apksig refuses it, or over a bound) keeps its names and carries @@ -219,9 +222,11 @@ instances; pending scans retain no APK bytes and can be cancelled with their original reason. Further requests receive a capacity error. The active slot is held through parsing and cleanup. Signers per scheme, v1 signers and lineage nodes are capped at 32 each as resource admission limits, not format -maxima (apksig separately accepts at most 10 v1 signers). Each v1 block is inflated to at most 1 MiB, and its DER -is read within 32 levels of nesting and 65,536 elements, with at most 32 -certificates and 32 `SignerInfo`s; BER indefinite lengths are accepted. The report's size is +maxima (apksig separately accepts at most 10 v1 signers). Each v1 block is +inflated to at most 1 MiB, and its BER is read within 32 levels of nesting and +65,536 elements, with at most 32 certificates and 32 `SignerInfo`s; BER +indefinite lengths and constructed OCTET STRING fields are accepted. Embedded +X.509 certificates retain their DER requirement. The report's size is bounded from above by those counts before anything is materialised, and the exact serialized size of the report together with its limitations (which repeat archive names) is then checked against an 8 MiB output budget (the exact check diff --git a/src/domain/apkSignature/apkV1SignatureBlock.test.ts b/src/domain/apkSignature/apkV1SignatureBlock.test.ts index dbbbaa87ca..14d091845e 100644 --- a/src/domain/apkSignature/apkV1SignatureBlock.test.ts +++ b/src/domain/apkSignature/apkV1SignatureBlock.test.ts @@ -18,7 +18,7 @@ import { } from "../../../tests/fixtures/apkSignature/v1SignatureBlock.js"; import { decodeV1SignatureBlock, - MAX_DER_ELEMENTS, + MAX_ASN1_ELEMENTS, MAX_V1_BLOCK_CERTIFICATES, MAX_V1_SIGNER_INFOS, } from "./apkV1SignatureBlock.js"; @@ -181,7 +181,7 @@ describe("v1 signature block (PKCS#7 SignedData)", () => { }), ).kind, ).toBe("budget-exceeded"); - const flood = new Uint8Array(2 * (MAX_DER_ELEMENTS + 1)).fill(0); + const flood = new Uint8Array(2 * (MAX_ASN1_ELEMENTS + 1)).fill(0); for (let i = 0; i < flood.length; i += 2) flood[i] = 0x05; expect( errorOf( @@ -192,15 +192,55 @@ describe("v1 signature block (PKCS#7 SignedData)", () => { ), ).toEqual({ kind: "budget-exceeded", - resource: "der-elements", - limit: MAX_DER_ELEMENTS, + resource: "asn1-elements", + limit: MAX_ASN1_ELEMENTS, }); // Deep nesting is bounded before recursion can exhaust the stack. let nested: Uint8Array = Uint8Array.from([0x05, 0x00]); for (let i = 0; i < 64; i += 1) nested = indefinite(0x30, nested); expect(errorOf(nested)).toEqual({ kind: "malformed-pkcs7", - detail: "nesting deeper than 32", + detail: "ASN.1 nesting exceeds 32 levels.", }); }); }); + +it.each([ + [ + "definite", + tlv(0x24, tlv(0x04, Uint8Array.of(1)), tlv(0x04, Uint8Array.of(2, 3))), + ], + [ + "indefinite", + indefinite( + 0x24, + tlv(0x04, Uint8Array.of(1)), + tlv(0x04, Uint8Array.of(2, 3)), + ), + ], +])("accepts %s constructed BER signature OCTET STRINGs", (_form, signature) => { + const block = decoded( + signedDataBlock({ + certificates: [LEAF_CERTIFICATE], + signerInfos: [signerInfo({ names: LEAF_CERTIFICATE, signature })], + }), + ); + expect(block.certificates.map(sha256)).toEqual([LEAF_SHA256]); + expect(block.signerInfos[0]?.certificateIndex).toBe(0); +}); + +it("rejects non-OCTET children inside a constructed signature", () => { + expect( + errorOf( + signedDataBlock({ + certificates: [LEAF_CERTIFICATE], + signerInfos: [ + signerInfo({ + names: LEAF_CERTIFICATE, + signature: tlv(0x24, tlv(0x02, Uint8Array.of(1))), + }), + ], + }), + ).kind, + ).toBe("malformed-pkcs7"); +}); diff --git a/src/domain/apkSignature/apkV1SignatureBlock.ts b/src/domain/apkSignature/apkV1SignatureBlock.ts index a36bf157c2..c3a84edc3e 100644 --- a/src/domain/apkSignature/apkV1SignatureBlock.ts +++ b/src/domain/apkSignature/apkV1SignatureBlock.ts @@ -17,6 +17,7 @@ * conflated with malformed input. */ import { type Result, err, ok } from "../result.js"; +import { Asn1BerFailure, Asn1BerReader, type Asn1BerNode } from "../asn1Ber.js"; export type ApkV1BlockError = | { readonly kind: "malformed-pkcs7"; readonly detail: string } @@ -25,7 +26,7 @@ export type ApkV1BlockError = readonly resource: | "v1-block-certificates" | "v1-signer-infos" - | "der-elements"; + | "asn1-elements"; readonly limit: number; }; @@ -34,9 +35,9 @@ export const MAX_V1_BLOCK_CERTIFICATES = 32; /** SignerInfos in one block; apksig verifies one, more is pathological. */ export const MAX_V1_SIGNER_INFOS = 32; /** Nesting depth; SignedData → X.509 extensions is under 12 levels deep. */ -const MAX_DER_DEPTH = 32; +const MAX_ASN1_DEPTH = 32; /** Elements read while decoding one block; bounds work for any byte length. */ -export const MAX_DER_ELEMENTS = 65_536; +export const MAX_ASN1_ELEMENTS = 65_536; export interface ApkV1SignerInfo { readonly identifiedBy: "issuer-and-serial" | "subject-key-identifier"; @@ -73,14 +74,7 @@ const TAG = { context0Primitive: 0x80, } as const; -interface Tlv { - readonly tag: number; - readonly start: number; - readonly contentStart: number; - readonly contentEnd: number; - readonly end: number; - readonly depth: number; -} +type Tlv = Asn1BerNode; /** Expected failures inside one decode; converted to a Result at the boundary. */ class BlockFailure { @@ -89,82 +83,39 @@ class BlockFailure { const malformed = (detail: string): BlockFailure => new BlockFailure({ kind: "malformed-pkcs7", detail }); -class DerReader { - #elements = 0; - constructor(readonly bytes: Uint8Array) {} +class ApkBerReader { + readonly #reader: Asn1BerReader; + readonly #bytes: Buffer; + + constructor(readonly bytes: Uint8Array) { + this.#bytes = Buffer.isBuffer(bytes) + ? bytes + : Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength); + this.#reader = new Asn1BerReader( + this.#bytes, + MAX_ASN1_ELEMENTS, + MAX_ASN1_DEPTH, + ); + } /** One TLV starting at `at`, which must end at or before `limit`. */ read(at: number, limit: number, depth: number): Tlv { - if (depth > MAX_DER_DEPTH) - throw malformed(`nesting deeper than ${String(MAX_DER_DEPTH)}`); - this.#elements += 1; - if (this.#elements > MAX_DER_ELEMENTS) - throw new BlockFailure({ - kind: "budget-exceeded", - resource: "der-elements", - limit: MAX_DER_ELEMENTS, - }); - if (at + 2 > limit) - throw malformed(`element at byte ${String(at)} is truncated`); - const tag = this.bytes[at] ?? 0; - if ((tag & 0x1f) === 0x1f) - throw malformed(`high-tag-number form at byte ${String(at)}`); - const first = this.bytes[at + 1] ?? 0; - let contentStart = at + 2; - if (first === 0x80) { - if ((tag & 0x20) === 0) - throw malformed( - `indefinite length on a primitive at byte ${String(at)}`, - ); - // BER indefinite length: children run to an end-of-contents marker. - let child = contentStart; - for (;;) { - if (child + 2 > limit) - throw malformed( - `indefinite element at byte ${String(at)} has no end-of-contents`, - ); - if (this.bytes[child] === 0 && this.bytes[child + 1] === 0) - return { - tag, - start: at, - contentStart, - contentEnd: child, - end: child + 2, - depth, - }; - child = this.read(child, limit, depth + 1).end; - } + try { + return this.#reader.read(at, limit, depth); + } catch (cause: unknown) { + if (!(cause instanceof Asn1BerFailure)) throw cause; + if (cause.kind === "budget-exceeded") + throw new BlockFailure({ + kind: "budget-exceeded", + resource: "asn1-elements", + limit: MAX_ASN1_ELEMENTS, + }); + throw malformed(cause.message); } - let length = first; - if (first > 0x80) { - const count = first & 0x7f; - if (count > 4) - throw malformed( - `length of ${String(count)} bytes at byte ${String(at)}`, - ); - if (contentStart + count > limit) - throw malformed(`length at byte ${String(at)} is truncated`); - length = 0; - for (let i = 0; i < count; i += 1) - length = length * 256 + (this.bytes[contentStart + i] ?? 0); - contentStart += count; - } - const end = contentStart + length; - if (end > limit) - throw malformed( - `element at byte ${String(at)} runs past its enclosing element`, - ); - return { tag, start: at, contentStart, contentEnd: end, end, depth }; } children(parent: Tlv): Tlv[] { - const out: Tlv[] = []; - for (let at = parent.contentStart; at < parent.contentEnd;) { - const child = this.read(at, parent.contentEnd, parent.depth + 1); - out.push(child); - at = child.end; - } - return out; + return [...parent.children]; } content(tlv: Tlv): Uint8Array { @@ -185,6 +136,28 @@ const expectTag = (tlv: Tlv | undefined, tag: number, what: string): Tlv => { return tlv; }; +/** BER permits an OCTET STRING to use constructed form with string fragments. */ +const expectOctetString = (tlv: Tlv | undefined, what: string): Tlv => { + if (tlv === undefined) throw malformed(`${what} is missing`); + if (tlv.tag === TAG.octetString) return tlv; + if (tlv.tag !== 0x24) + throw malformed( + `${what} has tag 0x${tlv.tag.toString(16)}, expected 0x4 or 0x24`, + ); + const validateFragments = (node: Tlv): void => { + for (const child of node.children) { + if (child.tag === TAG.octetString) continue; + if (child.tag === 0x24) validateFragments(child); + else + throw malformed( + `${what} has non-OCTET STRING fragment tag 0x${child.tag.toString(16)}`, + ); + } + }; + validateFragments(tlv); + return tlv; +}; + /** Dotted OID text; arcs are accumulated as BigInt so no arc overflows. */ const oidText = (content: Uint8Array): string => { if (content.length === 0 || content.length > 64) @@ -207,7 +180,11 @@ const oidText = (content: Uint8Array): string => { return arcs.join("."); }; -const algorithmOid = (reader: DerReader, tlv: Tlv | undefined, what: string) => +const algorithmOid = ( + reader: ApkBerReader, + tlv: Tlv | undefined, + what: string, +) => oidText( reader.content( expectTag( @@ -234,7 +211,7 @@ interface CertificateIdentity { * by a SignerInfo (null) but is still reported by the adapter's X.509 owner. */ const certificateIdentity = ( - reader: DerReader, + reader: ApkBerReader, certificate: Tlv, ): CertificateIdentity | null => { try { @@ -280,7 +257,7 @@ const certificateIdentity = ( }; const signerInfo = ( - reader: DerReader, + reader: ApkBerReader, tlv: Tlv, identities: readonly (CertificateIdentity | null)[], index: number, @@ -327,7 +304,7 @@ const signerInfo = ( fields[next], `${what} signatureAlgorithm`, ); - expectTag(fields[next + 1], TAG.octetString, `${what} signature`); + expectOctetString(fields[next + 1], `${what} signature`); return { identifiedBy, certificateIndex: certificateIndex < 0 ? null : certificateIndex, @@ -337,7 +314,7 @@ const signerInfo = ( }; const decode = (bytes: Uint8Array): ApkV1SignedData => { - const reader = new DerReader(bytes); + const reader = new ApkBerReader(bytes); const contentInfo = expectTag( reader.read(0, bytes.length, 0), TAG.sequence, diff --git a/src/domain/asn1Ber.ts b/src/domain/asn1Ber.ts new file mode 100644 index 0000000000..764865f754 --- /dev/null +++ b/src/domain/asn1Ber.ts @@ -0,0 +1,142 @@ +/** A bounded BER value tree shared by binary format owners. */ +export interface Asn1BerNode { + readonly tag: number; + readonly constructed: boolean; + readonly lengthForm: "definite" | "indefinite"; + readonly start: number; + readonly contentStart: number; + readonly contentEnd: number; + readonly end: number; + readonly content: Buffer; + readonly children: readonly Asn1BerNode[]; +} + +export class Asn1BerFailure extends Error { + constructor( + message: string, + readonly kind: "malformed" | "budget-exceeded" = "malformed", + readonly limit: number | null = null, + ) { + super(message); + this.name = "Asn1BerFailure"; + } +} + +interface Budget { + remaining: number; +} + +const fail = ( + message: string, + kind: Asn1BerFailure["kind"] = "malformed", + limit: number | null = null, +): never => { + throw new Asn1BerFailure(message, kind, limit); +}; + +/** + * Reads one BER TLV and its constructed children. A reader shares its node + * budget across separately encoded nested values, such as X.509 extnValue. + */ +export class Asn1BerReader { + readonly #budget: Budget; + + constructor( + readonly bytes: Buffer, + readonly maxNodes: number, + readonly maxDepth = 32, + ) { + this.#budget = { remaining: maxNodes }; + } + + read(offset = 0, end = this.bytes.length, depth = 0): Asn1BerNode { + if (depth > this.maxDepth) + return fail(`ASN.1 nesting exceeds ${String(this.maxDepth)} levels.`); + if (this.#budget.remaining <= 0) + return fail( + `ASN.1 value count exceeds the ${String(this.maxNodes)}-node budget.`, + "budget-exceeded", + this.maxNodes, + ); + this.#budget.remaining -= 1; + if (offset >= end || offset + 2 > end) + return fail("Truncated ASN.1 tag or length."); + const tag = this.bytes[offset]; + if (tag === undefined) return fail("Truncated ASN.1 tag."); + if ((tag & 0x1f) === 0x1f) + return fail( + `High-tag-number form at byte ${String(offset)} is unsupported.`, + ); + if (tag === 0) + return fail( + "Unexpected end-of-contents marker outside an indefinite value.", + ); + const constructed = (tag & 0x20) !== 0; + let cursor = offset + 1; + const lengthByte = this.bytes[cursor]; + if (lengthByte === undefined) return fail("Truncated ASN.1 length."); + cursor += 1; + let length = 0; + const indefinite = lengthByte === 0x80; + if (indefinite) { + if (!constructed) + return fail("Primitive ASN.1 values cannot use indefinite length."); + } else if ((lengthByte & 0x80) === 0) length = lengthByte; + else { + const width = lengthByte & 0x7f; + if (width > 4) return fail("Unsupported ASN.1 long-form length width."); + if (cursor + width > end) return fail("Truncated ASN.1 long length."); + length = 0; + for (let index = 0; index < width; index += 1) { + const byte = this.bytes[cursor]; + if (byte === undefined) return fail("Truncated ASN.1 length."); + length = (length << 8) | byte; + cursor += 1; + } + } + const contentStart = cursor; + const children: Asn1BerNode[] = []; + let contentEnd = cursor; + let encodedEnd = cursor; + if (indefinite) { + let at = cursor; + let terminated = false; + while (at < end) { + if (at + 1 < end && this.bytes[at] === 0 && this.bytes[at + 1] === 0) { + contentEnd = at; + encodedEnd = at + 2; + terminated = true; + break; + } + const child = this.read(at, end, depth + 1); + children.push(child); + at = child.end; + } + if (!terminated) + return fail("Indefinite ASN.1 value has no end-of-contents marker."); + } else { + contentEnd = cursor + length; + if (contentEnd > end) return fail("ASN.1 value exceeds its parent."); + encodedEnd = contentEnd; + } + if (constructed && !indefinite) { + let at = cursor; + while (at < contentEnd) { + const child = this.read(at, contentEnd, depth + 1); + children.push(child); + at = child.end; + } + } + return { + tag, + constructed, + lengthForm: indefinite ? "indefinite" : "definite", + start: offset, + contentStart, + contentEnd, + end: encodedEnd, + content: this.bytes.subarray(contentStart, contentEnd), + children, + }; + } +} diff --git a/src/native/pe/PeCertificateAsn1.ts b/src/native/pe/PeCertificateAsn1.ts index 4e67643378..43c78446a8 100644 --- a/src/native/pe/PeCertificateAsn1.ts +++ b/src/native/pe/PeCertificateAsn1.ts @@ -1,3 +1,9 @@ +import { + Asn1BerFailure, + Asn1BerReader, + type Asn1BerNode, +} from "../../domain/asn1Ber.js"; + const OID_NAMES: Readonly> = { "1.2.840.113549.1.7.1": "data", "1.2.840.113549.1.7.2": "signedData", @@ -19,16 +25,7 @@ const OID_NAMES: Readonly> = { "2.16.840.1.101.3.4.2.3": "sha512", }; -export interface Asn1Node { - readonly tag: number; - readonly constructed: boolean; - readonly lengthForm: "definite" | "indefinite"; - readonly start: number; - readonly contentEnd: number; - readonly end: number; - readonly content: Buffer; - readonly children: readonly Asn1Node[]; -} +export type Asn1Node = Asn1BerNode; export class PeCertificateAsn1Failure extends Error { constructor(message: string) { @@ -48,96 +45,14 @@ const required = (value: T | undefined, message: string): T => // few thousand nodes; the budget bounds sibling fan-out, which depth does not. export const ASN1_NODE_BUDGET = 16_384; -interface Asn1Budget { - remaining: number; -} - /** Decode one bounded ASN.1 TLV, including indefinite constructed BER values. */ -export const readAsn1 = ( - bytes: Buffer, - offset = 0, - end = bytes.length, - depth = 0, - budget: Asn1Budget = { remaining: ASN1_NODE_BUDGET }, -): Asn1Node => { - if (depth > 32) return fail("ASN.1 nesting exceeds 32 levels."); - if (budget.remaining <= 0) - return fail( - `ASN.1 value count exceeds the ${String(ASN1_NODE_BUDGET)}-node budget.`, - ); - budget.remaining -= 1; - if (offset >= end || offset + 2 > end) - return fail("Truncated ASN.1 tag or length."); - const tag = required(bytes[offset], "Truncated ASN.1 tag."); - if (tag === 0) - return fail( - "Unexpected end-of-contents marker outside an indefinite value.", - ); - const constructed = (tag & 0x20) !== 0; - let cursor = offset + 1; - const lengthByte = required(bytes[cursor], "Truncated ASN.1 length."); - cursor += 1; - let length = 0; - const indefinite = lengthByte === 0x80; - if (indefinite) { - if (!constructed) - return fail("Primitive ASN.1 values cannot use indefinite length."); - } else if ((lengthByte & 0x80) === 0) length = lengthByte; - else { - const width = lengthByte & 0x7f; - if (width > 4) return fail("Unsupported ASN.1 long-form length width."); - if (cursor + width > end) return fail("Truncated ASN.1 long length."); - length = 0; - for (let index = 0; index < width; index += 1) { - length = - (length << 8) | required(bytes[cursor], "Truncated ASN.1 length."); - cursor += 1; - } +export const readAsn1 = (bytes: Buffer): Asn1Node => { + try { + return new Asn1BerReader(bytes, ASN1_NODE_BUDGET).read(); + } catch (cause: unknown) { + if (cause instanceof Asn1BerFailure) return fail(cause.message); + throw cause; } - const children: Asn1Node[] = []; - let contentEnd = cursor; - let encodedEnd = cursor; - if (indefinite) { - let at = cursor; - let terminated = false; - while (at < end) { - if (at + 1 < end && bytes[at] === 0 && bytes[at + 1] === 0) { - contentEnd = at; - encodedEnd = at + 2; - terminated = true; - break; - } - const child = readAsn1(bytes, at, end, depth + 1, budget); - children.push(child); - at = child.end; - } - if (!terminated) - return fail("Indefinite ASN.1 value has no end-of-contents marker."); - } else { - contentEnd = cursor + length; - if (length < 0 || contentEnd > end) - return fail("ASN.1 value exceeds its parent."); - encodedEnd = contentEnd; - } - if (constructed && !indefinite) { - let at = cursor; - while (at < contentEnd) { - const child = readAsn1(bytes, at, contentEnd, depth + 1, budget); - children.push(child); - at = child.end; - } - } - const content = bytes.subarray(cursor, contentEnd); - return { - tag, - constructed, - lengthForm: indefinite ? "indefinite" : "definite", - start: offset, - contentEnd, - end: encodedEnd, - content, - children, - }; }; const requireDefiniteEncoding = (node: Asn1Node, label: string): void => { diff --git a/tests/acceptance/apkSignatureV1Cli.test.ts b/tests/acceptance/apkSignatureV1Cli.test.ts index 8c18537e26..fd8f82ce9a 100644 --- a/tests/acceptance/apkSignatureV1Cli.test.ts +++ b/tests/acceptance/apkSignatureV1Cli.test.ts @@ -13,12 +13,16 @@ import { createServer } from "../../src/server/createServer.js"; import { APK_SIGNATURE_V2_ID, APK_SIGNATURE_V3_ID, + indefinite, LEAF_CERTIFICATE, LEAF_SHA256, LEAF_SUBJECT, OPENSSL_BLOCK_ISSUER_SERIAL, OPENSSL_BLOCK_KEY_ID, ROOT_CERTIFICATE, + signedDataBlock, + signerInfo, + tlv, v2SchemeValue, v3SchemeValue, withSigningBlock, @@ -175,6 +179,58 @@ cliTest( }, ); +cliTest( + "retains v1 signer identity when the CMS signature OCTET STRING is constructed BER", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-apk-v1-ber-signature-"); + const encodings = [ + [ + "definite", + tlv(0x24, tlv(0x04, Uint8Array.of(1)), tlv(0x04, Uint8Array.of(2, 3))), + ], + [ + "indefinite", + indefinite( + 0x24, + tlv(0x04, Uint8Array.of(1)), + tlv(0x04, Uint8Array.of(2, 3)), + ), + ], + ] as const; + for (const [form, signature] of encodings) { + const path = join(root, `${form}.apk`); + await writeFile( + path, + await zipWith([ + { name: "META-INF/MANIFEST.MF", data: "Manifest-Version: 1.0" }, + { name: "META-INF/CERT.SF", data: "Signature-Version: 1.0" }, + { + name: "META-INF/CERT.RSA", + data: signedDataBlock({ + certificates: [LEAF_CERTIFICATE], + signerInfos: [signerInfo({ names: LEAF_CERTIFICATE, signature })], + }), + }, + ]), + ); + const result = await cli.run({ + arguments: ["inspect-apk-signature", path, "--json"], + }); + expect(result.exitCode, result.stderr).toBe(0); + const evidence = completeEvidence(result.json); + expect(evidence.normalized_result.schemes.v1.signers).toMatchObject([ + { + certificate: { + sha256_fingerprint: LEAF_SHA256, + subject: LEAF_SUBJECT, + }, + decode_error: null, + }, + ]); + } + }, +); + cliTest( "keeps a subjectKeyIdentifier v1 signer out of the posture match, as apksig cannot resolve it", async ({ cli }) => {