Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Allow extension feature on legacy flows" #238

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ Changing the subcommand after an exchange flow is started will result in an erro

### EXTENSION:

This feature is only available in a PROCESS_TRANSACTION_RESPONSE command in a SWAP_NEW, SELL_NEW, or FUND_NEW flow.\
In Legacy flows the extension must be P2_NONE. \
The extension is sent to the app as the upper 4 bits of the P2 byte of the APDU.

The maximum DATA in a single APDU is 255 bytes, in case it is not sufficient for a command,
Expand Down
6 changes: 5 additions & 1 deletion src/apdu_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,11 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
bool is_first_data_chunk = !(extension & P2_EXTEND);
bool is_last_data_chunk = !(extension & P2_MORE);
bool is_whole_apdu = is_first_data_chunk && is_last_data_chunk;

// Split reception is only for NG flows
if (subcommand != SWAP_NG && subcommand != SELL_NG && subcommand != FUND_NG && !is_whole_apdu) {
PRINTF("Extension %d refused, only allowed for unified flows\n", extension);
return WRONG_P2_EXTENSION;
}
// Split reception is only for PROCESS_TRANSACTION_RESPONSE_COMMAND
if (instruction != PROCESS_TRANSACTION_RESPONSE_COMMAND && !is_whole_apdu) {
PRINTF("Extension %d refused, only allowed for PROCESS_TRANSACTION_RESPONSE instruction\n",
Expand Down
21 changes: 19 additions & 2 deletions test/python/test_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@

class TestExtension:

@pytest.mark.parametrize("subcommand", LEGACY_SUBCOMMANDS)
def test_extension_forbidden_for_subcommand(self, backend, subcommand):
ex = ExchangeClient(backend, Rate.FIXED, subcommand)
partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")
transaction_id = ex.init_transaction().data
credentials = get_credentials(subcommand, partner)
ex.set_partner_key(credentials)
ex.check_partner_key(LEDGER_SIGNER.sign(credentials))
tx, _ = craft_and_sign_tx(subcommand, TX_INFOS[subcommand], transaction_id, FEES, partner)
# Send the exchange transaction proposal and its signature

for ext in [P2_MORE, P2_EXTEND, (P2_EXTEND | P2_MORE)]:
with pytest.raises(ExceptionRAPDU) as e:
backend.exchange(ex.CLA, Command.PROCESS_TRANSACTION_RESPONSE, p1=Rate.FIXED, p2=(subcommand | ext), data=tx)
assert e.value.status == Errors.WRONG_P2_EXTENSION


@pytest.mark.parametrize("subcommand", ALL_SUBCOMMANDS)
def test_extension_forbidden_for_command(self, backend, subcommand):
for ext in [P2_MORE, P2_EXTEND, (P2_EXTEND | P2_MORE)]:
Expand All @@ -60,7 +77,7 @@ def test_extension_forbidden_for_command(self, backend, subcommand):
assert e.value.status == Errors.WRONG_P2_EXTENSION


@pytest.mark.parametrize("subcommand", ALL_SUBCOMMANDS)
@pytest.mark.parametrize("subcommand", NEW_SUBCOMMANDS)
def test_extension_many_chunks(self, backend, subcommand):
ex = ExchangeClient(backend, Rate.FIXED, subcommand)
partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")
Expand All @@ -82,7 +99,7 @@ def test_extension_many_chunks(self, backend, subcommand):
ex.check_transaction_signature(tx_signature)


@pytest.mark.parametrize("subcommand", ALL_SUBCOMMANDS)
@pytest.mark.parametrize("subcommand", NEW_SUBCOMMANDS)
def test_extension_advanced_usage(self, backend, subcommand):
ex = ExchangeClient(backend, Rate.FIXED, subcommand)
partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")
Expand Down
Loading