diff --git a/src/domain/apkSignature/apkV1SignatureBlock.test.ts b/src/domain/apkSignature/apkV1SignatureBlock.test.ts index f4aa41a1a9..02b676c152 100644 --- a/src/domain/apkSignature/apkV1SignatureBlock.test.ts +++ b/src/domain/apkSignature/apkV1SignatureBlock.test.ts @@ -6,6 +6,7 @@ import { indefinite, oid, withIssuer, + withSerial, LEAF_CERTIFICATE, LEAF_SHA256, OID_ECDSA_SHA256, @@ -307,6 +308,57 @@ it("matches supported issuer strings across encodings and RDN attribute order, p ).toBeNull(); }); +it("matches v1 signer serials by signed INTEGER value, preserving raw certificates", () => { + // apksig tolerates redundant sign padding, although DER requires minimal + // encoding. A sign octet remains significant when the next high bit differs. + const identify = (certificate: Uint8Array, serial: Uint8Array) => { + const block = decoded( + signedDataBlock({ + certificates: [certificate], + signerInfos: [signerInfo({ names: certificate, serial })], + }), + ); + expect(block.certificates.map((value) => Buffer.from(value))).toEqual([ + Buffer.from(certificate), + ]); + return block.signerInfos[0]?.certificateIndex; + }; + + expect(identify(LEAF_CERTIFICATE, Uint8Array.of(0x00, 0x12, 0x34))).toBe(0); + expect( + identify( + withSerial(LEAF_CERTIFICATE, tlv(0x02, Uint8Array.of(0xff))), + Uint8Array.of(0xff, 0xff), + ), + ).toBe(0); + for (const [canonical, padded] of [ + [Uint8Array.of(0x12, 0x34), Uint8Array.of(0x00, 0x00, 0x12, 0x34)], + [Uint8Array.of(0xff), Uint8Array.of(0xff, 0xff, 0xff)], + ] as const) { + const canonicalCertificate = withSerial( + LEAF_CERTIFICATE, + tlv(0x02, canonical), + ); + const paddedCertificate = withSerial(LEAF_CERTIFICATE, tlv(0x02, padded)); + expect(identify(canonicalCertificate, padded)).toBe(0); + expect(identify(paddedCertificate, canonical)).toBe(0); + expect(identify(paddedCertificate, padded)).toBe(0); + } + // 0x00 0x80 is +128; a bare 0x80 is -128. 0xff 0x7f is -129, not 0x7f. + expect( + identify( + withSerial(LEAF_CERTIFICATE, tlv(0x02, Uint8Array.of(0x00, 0x80))), + Uint8Array.of(0x80), + ), + ).toBeNull(); + expect( + identify( + withSerial(LEAF_CERTIFICATE, tlv(0x02, Uint8Array.of(0xff, 0x7f))), + Uint8Array.of(0x7f), + ), + ).toBeNull(); +}); + it("keeps exact issuer identity when its comparison key cannot be interpreted", () => { const unsupported = tlv( 0x30, diff --git a/src/domain/apkSignature/apkV1SignatureBlock.ts b/src/domain/apkSignature/apkV1SignatureBlock.ts index b43859a0f4..0ec4ff4542 100644 --- a/src/domain/apkSignature/apkV1SignatureBlock.ts +++ b/src/domain/apkSignature/apkV1SignatureBlock.ts @@ -172,6 +172,22 @@ const algorithmOid = (tlv: Asn1BerNode | undefined, what: string) => const equalBytes = (a: Uint8Array, b: Uint8Array): boolean => a.length === b.length && a.every((byte, i) => byte === b[i]); +/** + * apksig compares serials as signed INTEGER values, tolerating redundant sign + * octets even though DER requires minimal encoding. Normalize each parsed + * identity once; retain the original certificate encoding independently. + */ +const integerIdentityContents = (bytes: Uint8Array): Uint8Array => { + let start = 0; + while (start < bytes.length - 1) { + const first = bytes[start]; + const nextHigh = ((bytes[start + 1] ?? 0) & 0x80) !== 0; + if (!((first === 0x00 && !nextHigh) || (first === 0xff && nextHigh))) break; + start += 1; + } + return start === 0 ? bytes : bytes.subarray(start); +}; + /** * RFC 5280 ยง7.1: RDN order matters; attributes in one SET do not. This * comparison profile admits ASCII PrintableString/UTF8String for common @@ -231,7 +247,7 @@ const directoryNameKey = (name: Asn1BerNode): string | null => { interface CertificateIdentity { readonly issuer: Uint8Array; readonly issuerKey: string | null; - readonly serial: Uint8Array; + readonly serialIdentity: Uint8Array; readonly subjectKeyIdentifier: Uint8Array | null; } @@ -278,7 +294,7 @@ const certificateIdentity = ( return { issuer: reader.rawEncodedSpan(issuer), issuerKey: directoryNameKey(issuer), - serial: serial.content, + serialIdentity: integerIdentityContents(serial.content), subjectKeyIdentifier, }; } catch (cause: unknown) { @@ -305,18 +321,16 @@ const signerInfo = ( const issuerNode = expectTag(parts[0], TAG.sequence, `${what} issuer`); const issuer = reader.rawEncodedSpan(issuerNode); const issuerKey = directoryNameKey(issuerNode); - const serial = expectTag( - parts[1], - TAG.integer, - `${what} serialNumber`, - ).content; + const serialIdentity = integerIdentityContents( + expectTag(parts[1], TAG.integer, `${what} serialNumber`).content, + ); identifiedBy = "issuer-and-serial"; certificateIndex = identities.findIndex( (identity) => identity !== null && (equalBytes(identity.issuer, issuer) || (issuerKey !== null && identity.issuerKey === issuerKey)) && - equalBytes(identity.serial, serial), + equalBytes(identity.serialIdentity, serialIdentity), ); } else if (sid?.tag === TAG.context0Primitive) { const keyId = sid.content; diff --git a/tests/fixtures/apkSignature/v1SignatureBlock.ts b/tests/fixtures/apkSignature/v1SignatureBlock.ts index 0a2562adc1..6a72af4bf5 100644 --- a/tests/fixtures/apkSignature/v1SignatureBlock.ts +++ b/tests/fixtures/apkSignature/v1SignatureBlock.ts @@ -117,15 +117,18 @@ const certificateParts = (certificate: Uint8Array) => { }; /** tbsCertificate fields a SignerInfo names: issuer Name TLV and serial content. */ -const issuerAndSerial = (certificate: Uint8Array): Uint8Array => { +const issuerAndSerial = ( + certificate: Uint8Array, + serial?: Uint8Array, +): Uint8Array => { // Fixture certificates are v3 with short-form serials: walk the known layout. const { fields } = certificateParts(certificate); - const serial = fields[1]!; + const serialField = fields[1]!; const issuer = fields[3]!; return tlv( 0x30, certificate.subarray(issuer.start, issuer.end), - certificate.subarray(serial.start, serial.end), + serial ?? certificate.subarray(serialField.start, serialField.end), ); }; @@ -162,6 +165,23 @@ export const withSubject = ( ); }; +/** Replace a fixture serial INTEGER (the full TLV) without asserting a valid signature. */ +export const withSerial = ( + certificate: Uint8Array, + serial: Uint8Array, +): Uint8Array => { + const { fields, outer } = certificateParts(certificate); + const part = ({ start, end }: DerSpan) => certificate.subarray(start, end); + return tlv( + 0x30, + tlv( + 0x30, + ...fields.map((field, index) => (index === 1 ? serial : part(field))), + ), + ...outer.map(part), + ); +}; + /** Replace a fixture's issuer or extensions without asserting a valid signature. */ export const withIssuer = ( certificate: Uint8Array, @@ -205,13 +225,25 @@ export interface SignerInfoSpec { readonly names: Uint8Array; /** Replace the required signature TLV, including omission for malformed fixtures. */ readonly signature?: Uint8Array; + /** + * INTEGER content of issuerAndSerialNumber.serialNumber when it must differ + * from the certificate named by `names`. + */ + readonly serial?: Uint8Array; } -export const signerInfo = ({ names, signature }: SignerInfoSpec): Uint8Array => +export const signerInfo = ({ + names, + signature, + serial, +}: SignerInfoSpec): Uint8Array => tlv( 0x30, integer(1), - issuerAndSerial(names), + issuerAndSerial( + names, + serial === undefined ? undefined : tlv(0x02, serial), + ), algorithm(OID_SHA256), algorithm(OID_ECDSA_SHA256), signature ?? tlv(0x04, Uint8Array.from([1, 2, 3])),