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

Fbe/swap spl token #242

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
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: 1 addition & 1 deletion .github/workflows/codeql-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
name: Analyse
strategy:
matrix:
sdk: [ "$NANOS_SDK", "$NANOX_SDK", "$NANOSP_SDK" ]
sdk: [ "$NANOX_SDK", "$NANOSP_SDK", "$STAX_SDK", "$FLEX_SDK" ]
#'cpp' covers C and C++
language: [ 'cpp' ]
runs-on: ubuntu-latest
Expand Down
16 changes: 14 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ APPNAME = "Exchange"

# Application version
APPVERSION_M = 4
APPVERSION_N = 0
APPVERSION_N = 1
APPVERSION_P = 0
APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)-pkiv2"

# Application source files
APP_SOURCE_PATH += src
Expand Down Expand Up @@ -131,6 +131,18 @@ ifdef TEST_PUBLIC_KEY
DEFINES += TEST_PUBLIC_KEY
endif

ifdef TRUSTED_NAME_TEST_KEY
$(info [INFO] TRUSTED_NAME_TEST_KEY enabled)
DEFINES += TRUSTED_NAME_TEST_KEY
endif

ifdef FIXED_TLV_CHALLENGE
$(info [INFO] FIXED_TLV_CHALLENGE enabled)
DEFINES += FIXED_TLV_CHALLENGE
endif

DEFINES += SDK_TLV_PARSER


########################################
# Protobuf files regeneration #
Expand Down
7 changes: 4 additions & 3 deletions ledger_app.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ sdk = "C"
devices = ["nanox", "nanos+", "stax", "flex"]

[use_cases]
use_test_keys = "TEST_PUBLIC_KEY=1"
dbg_use_test_keys = "DEBUG=1 TEST_PUBLIC_KEY=1"
testing_dbg_use_test_keys = "DEBUG=1 TESTING=1 TEST_PUBLIC_KEY=1"
use_test_keys = "TEST_PUBLIC_KEY=1 TRUSTED_NAME_TEST_KEY=1"
dbg_use_test_keys = "DEBUG=1 TEST_PUBLIC_KEY=1 TRUSTED_NAME_TEST_KEY=1"
full_replay = "DEBUG=1 TESTING=1 TEST_PUBLIC_KEY=1 TRUSTED_NAME_TEST_KEY=1 FIXED_TLV_CHALLENGE=1"
dbg_full_replay = "DEBUG=1 TESTING=1 TEST_PUBLIC_KEY=1 TRUSTED_NAME_TEST_KEY=1 FIXED_TLV_CHALLENGE=1"

[tests]
pytest_directory = "./test/python/"
47 changes: 28 additions & 19 deletions src/apdu_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,7 @@ typedef struct apdu_s {
uint8_t rate;
uint8_t subcommand;
uint16_t data_length;
// We could have put the reconstructed apdu buffer here but it would increase the RAM usage by
// 512 bytes which is a lot on NANOS
// Instead the reconstructed apdu buffer is G_swap_ctx.raw_transaction
// It is unionized with the decoded protobuf transaction requests
// Pro: less memory usage
// Cons: use cautiously and only during the command PROCESS_TRANSACTION_RESPONSE_COMMAND
// The split reception is only made for this command anyway
uint8_t raw_transaction[256 * 2];
} apdu_t;
apdu_t G_received_apdu;

Expand All @@ -66,6 +60,17 @@ static uint16_t check_instruction(uint8_t instruction, uint8_t subcommand) {
return INVALID_INSTRUCTION;
}

if (instruction == GET_CHALLENGE && (subcommand != SWAP && subcommand != SWAP_NG)) {
PRINTF("Instruction GET_CHALLENGE is only for SWAP based flows\n");
return INVALID_INSTRUCTION;
}

if (instruction == SEND_TRUSTED_NAME_DESCRIPTOR &&
(subcommand != SWAP && subcommand != SWAP_NG)) {
PRINTF("Instruction SEND_TRUSTED_NAME_DESCRIPTOR is only for SWAP based flows\n");
return INVALID_INSTRUCTION;
}

if ((instruction == CHECK_REFUND_ADDRESS_AND_DISPLAY ||
instruction == CHECK_REFUND_ADDRESS_NO_DISPLAY) &&
(subcommand != SWAP && subcommand != SWAP_NG)) {
Expand Down Expand Up @@ -103,6 +108,14 @@ static uint16_t check_instruction(uint8_t instruction, uint8_t subcommand) {
check_current_state = TRANSACTION_RECEIVED;
check_subcommand_context = true;
break;
case GET_CHALLENGE:
check_current_state = SIGNATURE_CHECKED;
check_subcommand_context = true;
break;
case SEND_TRUSTED_NAME_DESCRIPTOR:
check_current_state = SIGNATURE_CHECKED;
check_subcommand_context = true;
break;
case CHECK_PAYOUT_ADDRESS:
case CHECK_ASSET_IN_AND_DISPLAY:
case CHECK_ASSET_IN_NO_DISPLAY:
Expand Down Expand Up @@ -221,9 +234,9 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
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",
extension);
if (instruction != PROCESS_TRANSACTION_RESPONSE_COMMAND &&
instruction != SEND_TRUSTED_NAME_DESCRIPTOR && !is_whole_apdu) {
PRINTF("Extension %d refused for instruction %d\n", extension, instruction);
return WRONG_P2_EXTENSION;
}

Expand All @@ -233,11 +246,7 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
G_received_apdu.subcommand = subcommand;
G_received_apdu.data_length = data_length;
if (!is_last_data_chunk) {
// Use the raw_transaction buffer as temporary storage.
// It's unionized with the decoded transaction but we are currently handling
// PROCESS_TRANSACTION_RESPONSE_COMMAND instruction (checked previously).
// After this command is done it will contain the decoded PB data, don't use it anymore
memcpy(G_swap_ctx.raw_transaction, apdu + OFFSET_CDATA, data_length);
memcpy(G_received_apdu.raw_transaction, apdu + OFFSET_CDATA, data_length);
G_received_apdu.expecting_more = true;
}
} else {
Expand All @@ -258,15 +267,15 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
subcommand);
return INVALID_P2_EXTENSION;
}
if (G_received_apdu.data_length + data_length > sizeof(G_swap_ctx.raw_transaction)) {
if (G_received_apdu.data_length + data_length > sizeof(G_received_apdu.raw_transaction)) {
PRINTF("Reception buffer size %d is not sufficient to receive more data (%d + %d)\n",
sizeof(G_swap_ctx.raw_transaction),
sizeof(G_received_apdu.raw_transaction),
G_received_apdu.data_length,
data_length);
return INVALID_P2_EXTENSION;
}
// Extend the already received buffer
memcpy(G_swap_ctx.raw_transaction + G_received_apdu.data_length,
memcpy(G_received_apdu.raw_transaction + G_received_apdu.data_length,
apdu + OFFSET_CDATA,
data_length);
G_received_apdu.data_length += data_length;
Expand All @@ -292,7 +301,7 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
} else {
// Split has taken place, data is in the split buffer
PRINTF("Split APDU successfully recreated, size %d\n", G_received_apdu.data_length);
command->data.bytes = G_swap_ctx.raw_transaction;
command->data.bytes = G_received_apdu.raw_transaction;
}
return 0;
}
Expand Down
5 changes: 5 additions & 0 deletions src/buf.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ typedef struct buf_s {
uint16_t size;
} buf_t;

typedef struct cbuf_s {
const uint8_t *bytes;
uint16_t size;
} cbuf_t;

bool parse_to_sized_buffer(uint8_t *in_buffer,
uint16_t in_size,
uint8_t size_of_length_field,
Expand Down
8 changes: 8 additions & 0 deletions src/command_dispatcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "start_signing_transaction.h"
#include "check_addresses_and_amounts.h"
#include "prompt_ui_display.h"
#include "get_challenge_handler.h"
#include "trusted_name_descriptor_handler.h"

#include "io.h"

Expand Down Expand Up @@ -42,6 +44,12 @@ int dispatch_command(const command_t *cmd) {
case CHECK_TRANSACTION_SIGNATURE_COMMAND:
ret = check_tx_signature(cmd);
break;
case GET_CHALLENGE:
ret = get_challenge_handler();
break;
case SEND_TRUSTED_NAME_DESCRIPTOR:
ret = trusted_name_descriptor_handler(cmd);
break;
case CHECK_PAYOUT_ADDRESS:
case CHECK_ASSET_IN_AND_DISPLAY:
case CHECK_ASSET_IN_NO_DISPLAY:
Expand Down
2 changes: 2 additions & 0 deletions src/commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ typedef enum {
CHECK_PARTNER_COMMAND = 0x05,
PROCESS_TRANSACTION_RESPONSE_COMMAND = 0x06,
CHECK_TRANSACTION_SIGNATURE_COMMAND = 0x07,
GET_CHALLENGE = 0x10,
SEND_TRUSTED_NAME_DESCRIPTOR = 0x11,
CHECK_PAYOUT_ADDRESS = 0x08,
CHECK_ASSET_IN_LEGACY_AND_DISPLAY = 0x08, // Same ID as CHECK_PAYOUT_ADDRESS, deprecated
CHECK_ASSET_IN_AND_DISPLAY = 0x0B, // Do note the 0x0B
Expand Down
48 changes: 48 additions & 0 deletions src/get_challenge_handler.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#include <cx.h>
#include "get_challenge_handler.h"
#include "swap_errors.h"
#include "buffer.h"
#include "io.h"
#include "globals.h"

static uint32_t challenge;

/**
* Generate a new challenge from the Random Number Generator
*/
void roll_challenge(void) {
#ifdef FIXED_TLV_CHALLENGE
challenge = 0xdeadbeef;
#else
challenge = cx_rng_u32();
#endif
}

/**
* Get the current challenge
*
* @return challenge
*/
uint32_t get_challenge(void) {
return challenge;
}

/**
* Send back the current challenge
*/
int get_challenge_handler(void) {
PRINTF("New challenge -> %u\n", challenge);
uint8_t output_buffer[4];
U4BE_ENCODE(output_buffer, 0, challenge);

buffer_t output;
output.ptr = output_buffer;
output.size = 4;
output.offset = 0;

if (io_send_response_buffers(&output, 1, SUCCESS) < 0) {
return -1;
}

return 0;
}
5 changes: 5 additions & 0 deletions src/get_challenge_handler.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#pragma once

void roll_challenge(void);
uint32_t get_challenge(void);
int get_challenge_handler(void);
2 changes: 0 additions & 2 deletions src/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ typedef struct swap_app_context_s {

// SWAP, SELL, and FUND flows are unionized as they cannot be used in the same context
union {
// This is the raw received APDU
uint8_t raw_transaction[256 * 2];
ledger_swap_NewTransactionResponse swap_transaction;
struct {
ledger_swap_NewSellResponse sell_transaction;
Expand Down
4 changes: 4 additions & 0 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "swap_errors.h"
#include "apdu_parser.h"
#include "sign_result.h"
#include "get_challenge_handler.h"

#include "usbd_core.h"

Expand Down Expand Up @@ -155,6 +156,9 @@ __attribute__((section(".boot"))) int main(__attribute__((unused)) int arg0) {
BLE_power(1, NULL);
#endif

// to prevent it from having a fixed value at boot
roll_challenge();

app_main();
}
CATCH(SWO_SEC_APP_14) {
Expand Down
52 changes: 52 additions & 0 deletions src/pki.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#include "os.h"
#include "cx.h"
#include "pki.h"
#include "os_pki.h"

swap_error_e check_signature_with_pki(const uint8_t *buffer,
uint8_t buffer_length,
uint8_t expected_key_usage,
cx_curve_t expected_curve,
const cbuf_t *signature) {
uint8_t key_usage = 0;
size_t certificate_name_len = 0;
uint8_t certificate_name[CERTIFICATE_TRUSTED_NAME_MAXLEN] = {0};
cx_ecfp_384_public_key_t public_key = {0};

bolos_err_t bolos_err;
bolos_err = os_pki_get_info(&key_usage, certificate_name, &certificate_name_len, &public_key);
if (bolos_err != 0x0000) {
PRINTF("Error %x while getting PKI certificate info\n", bolos_err);
return NO_CERTIFICATE_LOADED;
}

if (key_usage != expected_key_usage) {
cedelavergne-ledger marked this conversation as resolved.
Show resolved Hide resolved
PRINTF("Wrong usage certificate %d, expected %d\n", key_usage, expected_key_usage);
return WRONG_CERTIFICATE_DATA;
}

if (public_key.curve != expected_curve) {
PRINTF("Wrong curve %d, expected %d\n", public_key.curve, expected_curve);
return WRONG_CERTIFICATE_DATA;
}

PRINTF("Certificate '%s' loaded with success\n", certificate_name);

// Checking the signature with PKI
if (!os_pki_verify((uint8_t *) buffer,
buffer_length,
(uint8_t *) signature->bytes,
signature->size)) {
PRINTF("Error, '%.*H' is not a signature of buffer '%.*H' by the PKI key '%.*H'\n",
signature->size,
signature->bytes,
buffer_length,
buffer,
sizeof(public_key),
&public_key);
return WRONG_TLV_SIGNATURE;
}

PRINTF("Signature verified successfully\n");
return SUCCESS;
}
16 changes: 16 additions & 0 deletions src/pki.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#pragma once

#include <cx.h>
#include <os.h>
#include <stddef.h>
#include <stdint.h>
#include <stdbool.h>

#include "buf.h"
#include "swap_errors.h"

swap_error_e check_signature_with_pki(const uint8_t *buffer,
uint8_t buffer_length,
uint8_t expected_key_usage,
cx_curve_t expected_curve,
const cbuf_t *signature);
15 changes: 1 addition & 14 deletions src/process_transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -336,23 +336,10 @@ static void save_fees(buf_t fees) {
}

int process_transaction(const command_t *cmd) {
uint8_t undecoded_transaction[sizeof(G_swap_ctx.raw_transaction)];
uint8_t *data;
// For memory optimization, the undecoded protobuf apdu may have been stored in an union
// with the decoded protobuf transaction.
if (cmd->data.bytes == G_swap_ctx.raw_transaction) {
// Copy locally the apdu to avoid problems during protobuf decode and fees extraction
PRINTF("Copying locally, the APDU has been received split\n");
memcpy(undecoded_transaction, G_swap_ctx.raw_transaction, cmd->data.size);
data = undecoded_transaction;
} else {
data = cmd->data.bytes;
}

buf_t fees;
buf_t payload;
bool needs_base64_decoding;
if (!parse_transaction(data,
if (!parse_transaction(cmd->data.bytes,
cmd->data.size,
cmd->subcommand,
&payload,
Expand Down
Loading
Loading