fix(connect): fix signatureValue of x509certificate

(cherry picked from commit cd33e9d)

packages/connect/src/api/firmware/__tests__/verifyAuthenticityProof.test.ts

@@ -1,4 +1,5 @@
 import { verifyAuthenticityProof } from '../verifyAuthenticityProof';
+import { fixSignature } from '../x509certificate';
 
 const CA_CERT =
     '308201df30820184a00302010202040a001ab3300a06082a8648ce3d0403023054310b300906035504061302435a311e301c060355040a0c155472657a6f7220436f6d70616e7920732e722e6f2e3125302306035504030c1c5472657a6f72204d616e75666163747572696e6720526f6f742043413020170d3233303130313030303030305a180f32303533303130313030303030305a304f310b300906035504061302435a311e301c060355040a0c155472657a6f7220436f6d70616e7920732e722e6f2e3120301e06035504030c175472657a6f72204d616e75666163747572696e672043413059301306072a8648ce3d020106082a8648ce3d030107034200041b36cc98d5e3d1a20677aaf26254ef3756f27c9d63080c93ad3e7d39d3ad23bf00497b924789bc8e3f87834994e16780ad4eae7e75db1f03835ca64363e980b4a3473045300e0603551d0f0101ff04040302020430120603551d130101ff040830060101ff020100301f0603551d2304183016801428b202f8f9c78a74e8c152bbfb433d99d0ca03ef300a06082a8648ce3d0403020349003046022100dfe2f837f3644c1f0250d37cd0f7d1e4e9b8cfc4820d7f5a623a8cb69df99f6c02210089148848c5fc597df4b8545d9b19d1cc15abe0e1252fa2938a4cf01ae835c563';
@@ -64,8 +65,8 @@ describe('firmware/verifyAuthenticityProof', () => {
         const verify = await verifyAuthenticityProof({
             certificates: [DEVICE_CERT, CA_CERT],
             signature:
-                // invalid 1st byte of signature
-                '0045022100c01793ffbe4f16d4efc84a4533d9bbfbbf1baa5349346678e07fdb6d848cca7902200df11b9d2850173d9c93993fca983c6d2a3f31ea69a0e19b69e18cc3b78424fe',
+                // invalid 2nd byte of signature
+                '3044022100c01793ffbe4f16d4efc84a4533d9bbfbbf1baa5349346678e07fdb6d848cca7902200df11b9d2850173d9c93993fca983c6d2a3f31ea69a0e19b69e18cc3b78424fe',
             challenge: Buffer.from(CHALLENGE, 'hex'),
             deviceModel: 'T2B1',
             config: CONFIG,
@@ -181,3 +182,40 @@ describe('firmware/verifyAuthenticityProof', () => {
         expect(verify.error).toBe('CA_PUBKEY_NOT_FOUND');
     });
 });
+
+// Optiga may produce a malformed signature with probability 1 in 256.
+// These test vectors verify that we are able to correctly reencode the malfomed signature into a valid DER encoding.
+describe('firmware/x509certificate', () => {
+    [
+        {
+            signature:
+                '3048022200007f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f02220000800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
+            result: '304502207f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f022100800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
+        },
+        {
+            signature:
+                '3045022100007f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e0220800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
+            result: '3044021f7f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e022100800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
+        },
+        {
+            signature:
+                '30330220007f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e020f000000000000008001020304050607',
+            result: '302c021f7f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e0209008001020304050607',
+        },
+        {
+            signature:
+                '303402210000800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e020f000000000000007f01020304050607',
+            result: '302c022000800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e02087f01020304050607',
+        },
+        {
+            signature:
+                '30440221007f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f021f800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e',
+            result: '304402207f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f022000800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e',
+        },
+    ].forEach(f => {
+        it(`fixSignature: ${f.signature.length} to ${f.result.length}`, () => {
+            const signature = fixSignature(new Uint8Array(Buffer.from(f.signature, 'hex')));
+            expect(Buffer.from(signature).toString('hex')).toEqual(f.result);
+        });
+    });
+});

packages/connect/src/api/firmware/verifyAuthenticityProof.ts

@@ -4,7 +4,7 @@ import { bufferUtils } from '@trezor/utils';
 import { PROTO } from '../../constants';
 import { DeviceAuthenticityConfig } from '../../data/deviceAuthenticityConfig';
 import { AuthenticateDeviceResult } from '../../types/api/authenticateDevice';
-import { parseCertificate } from './x509certificate';
+import { parseCertificate, fixSignature } from './x509certificate';
 
 // There is incomparability in results between nodejs and window SubtleCrypto api.
 // window.crypto.subtle.importKey (CryptoKey) cannot be used by `crypto-browserify`.Verify
@@ -142,7 +142,7 @@ export const verifyAuthenticityProof = async ({
     const isSignatureValid = await verifySignature(
         Buffer.from(deviceCert.tbsCertificate.subjectPublicKeyInfo.bits.bytes),
         prefixedChallenge,
-        Buffer.from(signature, 'hex'),
+        fixSignature(Buffer.from(signature, 'hex')),
     );
 
     if (rootPubKey && isDeviceCertValid && isSignatureValid) {

packages/connect/src/api/firmware/x509certificate.ts

@@ -98,13 +98,61 @@ const derBitStringValue = (byteArray: Uint8Array) => ({
     bytes: byteArray.subarray(1),
 });
 
+// Optiga may produce a malformed signature with probability 1 in 256.
+// https://github.com/trezor/trezor-firmware/issues/3411
+export const fixSignature = (byteArray: Uint8Array) => {
+    const asn1 = derToAsn1(byteArray);
+
+    if (asn1.cls !== 0 || asn1.tag !== 16 || !asn1.structured) {
+        throw new Error('Bad signature. Not a SEQUENCE.');
+    }
+
+    const items = derToAsn1List(asn1.contents);
+    let newLength = 0;
+    const fixedItems = items.map(chunk => {
+        // find first significant byte
+        const index = chunk.contents.findIndex(value => value > 0x00);
+        const data = chunk.contents.subarray(index);
+        // According to the DER-encoding rules, the integers are supposed to be prefixed with a 0x00 byte
+        // if **and only if** the most significant byte is >= 0x80
+        const offset = data[0] >= 0x80 ? 1 : 0;
+        // create replacement for chunk
+        const chunkLength = data.length + offset;
+        const newChunk = new Uint8Array(chunkLength + 2);
+        // set first two bytes: original value and new length of the chunk
+        newChunk.set([chunk.raw[0], chunkLength]);
+        // optionally add 0
+        if (offset > 0) {
+            newChunk.set([0], 2);
+        }
+        // fill new chunk with data
+        newChunk.set(data, 2 + offset);
+        newLength += newChunk.length;
+        return newChunk;
+    });
+
+    // create replacement for sequence object
+    const signature = new Uint8Array(newLength + 2);
+    // set two first bytes: original value and new length of all chunks
+    signature.set([byteArray[0], newLength]);
+    // fill new sequence with fixed items
+    let signatureOffset = 2;
+    fixedItems.forEach(item => {
+        signature.set(item, signatureOffset);
+        signatureOffset += item.length;
+    });
+
+    return signature;
+};
+
 const parseSignatureValue = (asn1: Asn1) => {
     if (asn1.cls !== 0 || asn1.tag !== 3 || asn1.structured) {
         throw new Error('Bad signature value. Not a BIT STRING.');
     }
+    const { unusedBits, bytes } = derBitStringValue(asn1.contents);
     return {
         asn1,
-        bits: derBitStringValue(asn1.contents),
+        bits: { unusedBits, bytes: fixSignature(bytes) },
     };
 };