Merge pull request #38 from coderofstuff/path-for-change-2

Implement passing path type and index for change address

fuzzing/fuzz_tx_parser.cc

@@ -14,13 +14,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     buffer_t buf = {.ptr = data, .size = size, .offset = 0};
     transaction_t tx;
     parser_status_e status;
+    uint32_t bip32_path[5];
     char version[3] = {0};
     char tx_input_len[3] = {0};
     char tx_output_len[3] = {0};
 
     memset(&tx, 0, sizeof(tx));
 
-    status = transaction_deserialize(&buf, &tx);
+    status = transaction_deserialize(&buf, &tx, bip32_path);
 
     if (status == PARSING_OK) {
         format_u64(version, sizeof(version), tx.version);

src/handler/sign_tx.c

@@ -38,7 +38,8 @@ int handler_sign_tx(buffer_t *cdata, uint8_t type, bool more) {
         G_context.req_type = CONFIRM_TRANSACTION;
         G_context.state = STATE_NONE;
 
-        parser_status_e status = transaction_deserialize(cdata, &G_context.tx_info.transaction);
+        parser_status_e status =
+            transaction_deserialize(cdata, &G_context.tx_info.transaction, G_context.bip32_path);
 
         PRINTF("Header Parsing status: %d.\n", status);
 

src/transaction/deserialize.c

@@ -74,13 +74,17 @@ parser_status_e transaction_input_deserialize(buffer_t *buf, transaction_input_t
     return buf->size - buf->offset == 0 ? PARSING_OK : INPUT_PARSING_ERROR;
 }
 
-parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) {
+parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx, uint32_t *bip32_path) {
+    uint8_t n_output = 0;
+    uint8_t n_input = 0;
+    uint8_t change_address_type = 0;
+    uint32_t change_address_index = 0;
+
     // Version, 2 bytes
     if (!buffer_read_u16(buf, &tx->version, BE)) {
         return VERSION_PARSING_ERROR;
     }
 
-    uint8_t n_output = 0;
     if (!buffer_read_u8(buf, &n_output)) {
         return OUTPUTS_LENGTH_PARSING_ERROR;
     }
@@ -92,7 +96,6 @@ parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) {
         return OUTPUTS_LENGTH_PARSING_ERROR;
     }
 
-    uint8_t n_input = 0;
     if (!buffer_read_u8(buf, &n_input)) {
         return INPUTS_LENGTH_PARSING_ERROR;
     }
@@ -104,5 +107,23 @@ parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) {
         return INPUTS_LENGTH_PARSING_ERROR;
     }
 
+    if (!buffer_read_u8(buf, &change_address_type)) {
+        return HEADER_PARSING_ERROR;
+    }
+
+    if (change_address_type != 0 && change_address_type != 1) {
+        return HEADER_PARSING_ERROR;
+    }
+
+    if (!buffer_read_u32(buf, &change_address_index, BE)) {
+        return HEADER_PARSING_ERROR;
+    }
+
+    bip32_path[0] = 0x8000002C;
+    bip32_path[1] = 0x8001b207;
+    bip32_path[2] = 0x80000000;
+    bip32_path[3] = (uint32_t)(change_address_type);
+    bip32_path[4] = change_address_index;
+
     return buf->size - buf->offset == 0 ? PARSING_OK : HEADER_PARSING_ERROR;
 }

src/transaction/deserialize.h

@@ -10,11 +10,13 @@
  *   Pointer to buffer with serialized transaction.
  * @param[out]     tx
  *   Pointer to transaction structure.
+ * @param[out]     bip32_path
+ *   Pointer to bip32_path array where change address info will be set
  *
  * @return PARSING_OK if success, error status otherwise.
  *
  */
-parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx);
+parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx, uint32_t *bip32_path);
 
 parser_status_e transaction_output_deserialize(buffer_t *buf, transaction_output_t *txout);
 

src/transaction/serialize.c

@@ -7,7 +7,7 @@
 #include "../common/write.h"
 #include "../common/varint.h"
 
-int transaction_serialize(const transaction_t *tx, uint8_t *out, size_t out_len) {
+int transaction_serialize(const transaction_t *tx, uint32_t *path, uint8_t *out, size_t out_len) {
     size_t offset = 0;
 
     if (out_len < 4) {
@@ -19,6 +19,11 @@ int transaction_serialize(const transaction_t *tx, uint8_t *out, size_t out_len)
     out[offset++] = tx->tx_output_len;
     out[offset++] = tx->tx_input_len;
 
+    out[offset++] = (uint8_t) path[3];
+
+    write_u32_be(out, offset, path[4]);
+    offset += 4;
+
     return (int) offset;
 }
 

src/transaction/serialize.h

@@ -11,6 +11,8 @@
  *
  * @param[in]  tx
  *   Pointer to input transaction structure.
+ * @param[in]  bip32_path
+ *   Pointer to bip32_path array where change address data will be read
  * @param[out] out
  *   Pointer to output byte buffer.
  * @param[in]  out_len
@@ -19,7 +21,7 @@
  * @return number of bytes written if success, -1 otherwise.
  *
  */
-int transaction_serialize(const transaction_t *tx, uint8_t *out, size_t out_len);
+int transaction_serialize(const transaction_t *tx, uint32_t *path, uint8_t *out, size_t out_len);
 
 /**
  * Serialize transaction output in byte buffer.

src/transaction/tx_validate.c

@@ -18,7 +18,6 @@ bool tx_validate_parsed_transaction(transaction_t* tx) {
     // Change address will always be output[1] if it exists
     if (tx->tx_output_len == 2) {
         transaction_output_t change_output = tx->tx_outputs[1];
-        transaction_input_t first_utxo = tx->tx_inputs[0];
 
         if (change_output.script_public_key[0] != 0x20) {
             // Change address can only be SCHNORR address and it's not
@@ -31,11 +30,11 @@ bool tx_validate_parsed_transaction(transaction_t* tx) {
         // was validated when we deserialized the data.
         memmove(change_address_pubkey, change_output.script_public_key + 1, 32);
 
+        // Forcing these values. path[3] and path[4]
+        // would've been set by transaction_deserialize
         G_context.bip32_path[0] = 0x8000002C;
         G_context.bip32_path[1] = 0x8001b207;
         G_context.bip32_path[2] = 0x80000000;
-        G_context.bip32_path[3] = (uint32_t)(first_utxo.address_type);
-        G_context.bip32_path[4] = first_utxo.address_index;
 
         G_context.bip32_path_len = 5;
 

tests/application_client/kaspa_transaction.py

@@ -81,10 +81,14 @@ def __init__(self,
                  version: int,
                  inputs: list[TransactionInput],
                  outputs: list[TransactionOutput],
+                 change_address_type: int = 0,
+                 change_address_index: int = 0,
                  do_check: bool = True) -> None:
         self.version: int = version
         self.inputs: list[TransactionInput] = inputs
         self.outputs: list[TransactionOutput] = outputs
+        self.change_address_type: int = change_address_type
+        self.change_address_index: int = change_address_index
 
         if do_check:
             if not 0 <= self.version <= 1:
@@ -94,7 +98,9 @@ def serialize_first_chunk(self) -> bytes:
         return b"".join([
             self.version.to_bytes(2, byteorder="big"),
             len(self.outputs).to_bytes(1, byteorder="big"),
-            len(self.inputs).to_bytes(1, byteorder="big")
+            len(self.inputs).to_bytes(1, byteorder="big"),
+            self.change_address_type.to_bytes(1, byteorder="big"),
+            self.change_address_index.to_bytes(4, byteorder="big"),
         ])
 
     def serialize(self) -> bytes:

tests/test_sign_cmd.py

@@ -66,6 +66,75 @@ def test_sign_tx_simple(firmware, backend, navigator, test_name):
     assert transaction.get_sighash(0) == sighash
     assert check_signature_validity(public_key, der_sig, sighash, transaction)
 
+def test_sign_tx_simple_change_idx1(firmware, backend, navigator, test_name):
+    # Use the app interface instead of raw interface
+    client = KaspaCommandSender(backend)
+    # The path used for this entire test
+    path: str = "m/44'/111111'/0'/0/0"
+
+    # First we need to get the public key of the device in order to build the transaction
+    rapdu = client.get_public_key(path=path)
+    _, public_key, _, _ = unpack_get_public_key_response(rapdu.data)
+
+    change_path: str = "m/44'/111111'/0'/1/1"
+
+    # First we need to get the public key of the device in order to build the transaction
+    change_rapdu = client.get_public_key(path=change_path)
+    _, change_public_key, _, _ = unpack_get_public_key_response(change_rapdu.data)
+
+    valid_change_script_public_key = b"".join([
+        0x20.to_bytes(1, 'big'),
+        change_public_key[1:33],
+        0xac.to_bytes(1, 'big')]).hex()
+
+    # Create the transaction that will be sent to the device for signing
+    transaction = Transaction(
+        version=0,
+        change_address_index=1,
+        change_address_type=1,
+        inputs=[
+            TransactionInput(
+                value=1100000,
+                tx_id="40b022362f1a303518e2b49f86f87a317c87b514ca0f3d08ad2e7cf49d08cc70",
+                address_type=0,
+                address_index=0,
+                index=0,
+                public_key=public_key[1:33]
+            )
+        ],
+        outputs=[
+            TransactionOutput(
+                value=1090000,
+                script_public_key=valid_change_script_public_key
+            )
+        ]
+    )
+
+    # Send the sign device instruction.
+    # As it requires on-screen validation, the function is asynchronous.
+    # It will yield the result when the navigation is done
+    with client.sign_tx(transaction=transaction):
+        # Validate the on-screen request by performing the navigation appropriate for this device
+        if firmware.device.startswith("nano"):
+            navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
+                                                      [NavInsID.BOTH_CLICK],
+                                                      "Approve",
+                                                      ROOT_SCREENSHOT_PATH,
+                                                      test_name)
+        else:
+            navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
+                                                      [NavInsID.USE_CASE_REVIEW_CONFIRM,
+                                                       NavInsID.USE_CASE_STATUS_DISMISS],
+                                                      "Hold to sign",
+                                                      ROOT_SCREENSHOT_PATH,
+                                                      test_name)
+
+    # The device as yielded the result, parse it and ensure that the signature is correct
+    response = client.get_async_response().data
+    _, _, _, der_sig, _, sighash = unpack_sign_tx_response(response)
+    assert transaction.get_sighash(0) == sighash
+    assert check_signature_validity(public_key, der_sig, sighash, transaction)
+
 def test_sign_tx_with_change(firmware, backend, navigator, test_name):
     # Use the app interface instead of raw interface
     client = KaspaCommandSender(backend)

unit-tests/test_tx_parser.c

@@ -17,22 +17,26 @@ static void test_tx_serialization(void **state) {
     transaction_t tx;
     tx.tx_output_len = 2;
     tx.tx_input_len = 3;
+
+    uint32_t path[5] = {0};
 
     // clang-format off
     uint8_t raw_tx[] = {
         // header
-        0x00, 0x01, 0x02, 0x03
+        0x00, 0x01, 0x02, 0x03,
+        0x01,
+        0x04, 0x05, 0x06, 0xFF
     };
 
     buffer_t buf = {.ptr = raw_tx, .size = sizeof(raw_tx), .offset = 0};
 
-    parser_status_e status = transaction_deserialize(&buf, &tx);
+    parser_status_e status = transaction_deserialize(&buf, &tx, path);
 
     assert_int_equal(status, PARSING_OK);
     assert_int_equal(tx.version, 1);
 
     uint8_t output[350];
-    int length = transaction_serialize(&tx, output, sizeof(output));
+    int length = transaction_serialize(&tx, path, output, sizeof(output));
     assert_int_equal(length, sizeof(raw_tx));
     assert_memory_equal(raw_tx, output, sizeof(raw_tx));
 }
@@ -42,7 +46,9 @@ static int run_test_tx_serialize(uint8_t* raw_tx, size_t raw_tx_len) {
 
     buffer_t buf = {.ptr = raw_tx, .size = raw_tx_len, .offset = 0};
 
-    return transaction_deserialize(&buf, &tx);
+    uint32_t path[5] = {0};
+
+    return transaction_deserialize(&buf, &tx, path);
 }
 
 static void test_tx_deserialization_fail(void **state) {
@@ -72,11 +78,16 @@ static void test_tx_deserialization_fail(void **state) {
         0x00, 0x01, 0x02, 0x00
     };
 
+    uint8_t invalid_change_type[] = {
+        0x00, 0x01, 0x02, 0x03, 0x02
+    };
+
     assert_int_equal(run_test_tx_serialize(invalid_version, sizeof(invalid_version)), VERSION_PARSING_ERROR);
     assert_int_equal(run_test_tx_serialize(missing_outlen, sizeof(missing_outlen)), OUTPUTS_LENGTH_PARSING_ERROR);
     assert_int_equal(run_test_tx_serialize(invalid_outlen, sizeof(invalid_outlen)), OUTPUTS_LENGTH_PARSING_ERROR);
     assert_int_equal(run_test_tx_serialize(missing_inlen, sizeof(missing_inlen)), INPUTS_LENGTH_PARSING_ERROR);
     assert_int_equal(run_test_tx_serialize(invalid_inlen, sizeof(invalid_inlen)), INPUTS_LENGTH_PARSING_ERROR);
+    assert_int_equal(run_test_tx_serialize(invalid_change_type, sizeof(invalid_change_type)), HEADER_PARSING_ERROR);
 }
 
 static void test_tx_input_serialization(void **state) {
@@ -309,8 +320,9 @@ static void test_serialization_fail(void **state) {
     transaction_input_t txin;
 
     uint8_t buffer[1] = {0};
+    uint32_t path[5] = {0};
 
-    assert_int_equal(transaction_serialize(&tx, buffer, sizeof(buffer)), -1);
+    assert_int_equal(transaction_serialize(&tx, path, buffer, sizeof(buffer)), -1);
     assert_int_equal(transaction_output_serialize(&txout, buffer, sizeof(buffer)), -1);
     assert_int_equal(transaction_input_serialize(&txin, buffer, sizeof(buffer)), -1);
 }