diff --git a/.doxygen/Doxyfile b/.doxygen/Doxyfile index 766774c8..92f2734b 100644 --- a/.doxygen/Doxyfile +++ b/.doxygen/Doxyfile @@ -734,7 +734,7 @@ STRICT_PROTO_MATCHING = NO # list. This list is created by putting \todo commands in the documentation. # The default value is: YES. -GENERATE_TODOLIST = YES +GENERATE_TODOLIST = NO # The GENERATE_TESTLIST tag can be used to enable (YES) or disable (NO) the test # list. This list is created by putting \test commands in the documentation. diff --git a/README.md b/README.md index b02866eb..95d3f38a 100644 --- a/README.md +++ b/README.md @@ -142,26 +142,23 @@ BOLOS_SDK=$NANOS_SDK make DEBUG=1 You can replace `NANOS` with `NANOSP`, `NANOX`, `STAX` for the other devices in BOLOS_SDK environmental variable. ### Testing -To test the application you need to enable python virtual environment and some dependencies. On any operating system, create a python virtual environment and activate it. +The application tests are run using same docker container used for building. Inside the docker container run following script, ``` -$ sudo apt-get update && sudo apt-get install -y qemu-user-static tesseract-ocr libtesseract-dev python3-pip libgmp-dev libsodium-dev git -$ python3 -m venv env -$ source env/bin/activate -``` -Inside the virtualenv, load the requirements.txt file. -``` -(env)$ pip install -r test/requirements.txt +$ apk add gmp-dev libsodium-dev; +$ python3 -m venv tezos_test_env --system-site-package; +$ source ./tezos_test_env/bin/activate; +(tezos_test_env)$ python3 -m pip install -r test/requirements.txt -q ; ``` Now you can run ragger tests for any perticular ledger device. Please make sure you have built the app.elf files for that perticular device first. Then run following command: ``` -(env)$ pytest test --device nanosp +(tezos_test_env)$ python3 -m pytest test --device nanosp ``` Replace nanosp with any of the following for respective device: nanos, nanosp, nanox , stax. These tests are run on Ledger emulator called speculos which emulates the actual ledger device. To run theese test on actual device you have to choose a backend. Run following commands to run these test on device: ``` -(env)$ pip install ragger[all_backends] -(env)$ pytest test --device nanosp --backend ledgercomm -s +(tezos_test_env)$ python3 -m pip install ragger[all_backends] +(tezos_test_env)$ python3 -m pytest test --device nanosp --backend ledgercomm -s ``` Note the `-s` flag which is required when running interactive tests with pytest. You can also choose `ledgerwallet` backend to run tests on device. diff --git a/src/apdu_query.c b/src/apdu_query.c index 5bba2735..e1518cec 100644 --- a/src/apdu_query.c +++ b/src/apdu_query.c @@ -69,10 +69,12 @@ int handle_query_main_hwm(void) { } int handle_query_auth_key(void) { + tz_exc exc = SW_OK; uint8_t resp[1u + (MAX_BIP32_PATH * sizeof(uint32_t))] = {0}; size_t offset = 0; uint8_t const length = g_hwm.baking_key.bip32_path.length; + TZ_ASSERT(length <= NUM_ELEMENTS(g_hwm.baking_key.bip32_path.components), EXC_WRONG_LENGTH); resp[offset] = length; offset++; @@ -83,21 +85,22 @@ int handle_query_auth_key(void) { } return io_send_response_pointer(resp, offset, SW_OK); +end: + return io_send_apdu_err(exc); } int handle_query_auth_key_with_curve(void) { + tz_exc exc = SW_OK; uint8_t resp[2u + (MAX_BIP32_PATH * sizeof(uint32_t))] = {0}; size_t offset = 0; uint8_t const length = g_hwm.baking_key.bip32_path.length; + TZ_ASSERT(length <= NUM_ELEMENTS(g_hwm.baking_key.bip32_path.components), EXC_WRONG_LENGTH); int derivation_type = unparse_derivation_type(g_hwm.baking_key.derivation_type); + TZ_ASSERT(derivation_type >= 0, EXC_REFERENCED_DATA_NOT_FOUND); - if (derivation_type < 0) { - return io_send_apdu_err(EXC_REFERENCED_DATA_NOT_FOUND); - } - - resp[offset] = unparse_derivation_type(g_hwm.baking_key.derivation_type); + resp[offset] = derivation_type; offset++; resp[offset] = length; @@ -109,4 +112,6 @@ int handle_query_auth_key_with_curve(void) { } return io_send_response_pointer(resp, offset, SW_OK); +end: + return io_send_apdu_err(exc); } diff --git a/src/apdu_sign.c b/src/apdu_sign.c index 13f13867..00af1963 100644 --- a/src/apdu_sign.c +++ b/src/apdu_sign.c @@ -97,7 +97,6 @@ static tz_exc blake2b_incremental_hash(uint8_t *const buff, *buff_length -= B2B_BLOCKBYTES; current += B2B_BLOCKBYTES; } - // TODO use circular buffer at some point memmove(buff, current, *buff_length); end: @@ -289,7 +288,7 @@ int select_signing_key(buffer_t *cdata, derivation_type_t derivation_type) { * Cdata: * + (max-size) uint8 *: message */ -int handle_sign(buffer_t *cdata, bool last, bool with_hash) { +int handle_sign(buffer_t *cdata, const bool last, const bool with_hash) { tz_exc exc = SW_OK; TZ_ASSERT_NOT_NULL(cdata); @@ -375,7 +374,7 @@ static int perform_signature(bool const send_hash) { TZ_ASSERT(os_global_pin_is_validated() == BOLOS_UX_OK, EXC_SECURITY); - TZ_ASSERT(write_high_water_mark(&G.parsed_baking_data), EXC_MEMORY_ERROR); + TZ_CHECK(write_high_water_mark(&G.parsed_baking_data)); uint8_t resp[SIGN_HASH_SIZE + MAX_SIGNATURE_SIZE] = {0}; size_t offset = 0; diff --git a/src/globals.h b/src/globals.h index 79ecb598..0ff19f72 100644 --- a/src/globals.h +++ b/src/globals.h @@ -58,8 +58,10 @@ void toggle_hwm(void); /// Our buffer must accommodate any remainder from hashing and the next message at once. #define TEZOS_BUFSIZE (BLAKE2B_BLOCKBYTES + MAX_APDU_SIZE) -#define PRIVATE_KEY_DATA_SIZE 64u -#define MAX_SIGNATURE_SIZE 100u +#define PRIVATE_KEY_DATA_SIZE 64u +#define MAX_SIGNATURE_SIZE 100u +#define ELLIPTIC_CURVE_PUB_KEY_LENGTH 65u +#define PUB_KEY_COMPPRESSED_LENGTH 33u /** * @brief This structure represents the state needed to handle HMAC diff --git a/src/keys.c b/src/keys.c index a06c4995..ffbfee04 100644 --- a/src/keys.c +++ b/src/keys.c @@ -87,7 +87,7 @@ cx_err_t generate_public_key(cx_ecfp_public_key_t *public_key, signature_type_t signature_type = derivation_type_to_signature_type(derivation_type); cx_curve_t cx_curve = signature_type_to_cx_curve(signature_type); - public_key->W_len = 65; + public_key->W_len = ELLIPTIC_CURVE_PUB_KEY_LENGTH; public_key->curve = cx_curve; // generate corresponding public key @@ -105,7 +105,7 @@ cx_err_t generate_public_key(cx_ecfp_public_key_t *public_key, if (cx_curve == CX_CURVE_Ed25519) { CX_CHECK( cx_edwards_compress_point_no_throw(CX_CURVE_Ed25519, public_key->W, public_key->W_len)); - public_key->W_len = 33; + public_key->W_len = PUB_KEY_COMPPRESSED_LENGTH; } end: @@ -134,7 +134,7 @@ static cx_err_t public_key_hash(uint8_t *const hash_out, return CX_INVALID_PARAMETER; } - if (hash_out_size < HASH_SIZE) { + if (hash_out_size < KEY_HASH_SIZE) { return CX_INVALID_PARAMETER_SIZE; } @@ -151,7 +151,7 @@ static cx_err_t public_key_hash(uint8_t *const hash_out, case SIGNATURE_TYPE_SECP256R1: { memcpy(compressed.W, public_key->W, public_key->W_len); compressed.W[0] = 0x02 + (public_key->W[64] & 0x01); - compressed.W_len = 33; + compressed.W_len = PUB_KEY_COMPPRESSED_LENGTH; break; } default: @@ -161,14 +161,14 @@ static cx_err_t public_key_hash(uint8_t *const hash_out, cx_err_t error = CX_OK; cx_blake2b_t hash_state; // cx_blake2b_init takes size in bits. - CX_CHECK(cx_blake2b_init_no_throw(&hash_state, HASH_SIZE * 8u)); + CX_CHECK(cx_blake2b_init_no_throw(&hash_state, KEY_HASH_SIZE * 8u)); CX_CHECK(cx_hash_no_throw((cx_hash_t *) &hash_state, CX_LAST, compressed.W, compressed.W_len, hash_out, - HASH_SIZE)); + KEY_HASH_SIZE)); if (compressed_out != NULL) { memmove(compressed_out, &compressed, sizeof(*compressed_out)); diff --git a/src/operations.c b/src/operations.c index c944d24b..0b371552 100644 --- a/src/operations.c +++ b/src/operations.c @@ -147,7 +147,7 @@ static inline tz_exc compute_pkh(cx_ecfp_public_key_t *const compressed_pubkey_o static tz_parser_result parse_implicit( parsed_contract_t *const out, raw_tezos_header_signature_type_t const *const raw_signature_type, - uint8_t const hash[HASH_SIZE]) { + uint8_t const hash[KEY_HASH_SIZE]) { tz_parser_result res = PARSER_CONTINUE; out->originated = 0; @@ -172,7 +172,6 @@ static tz_parser_result parse_implicit( #define NEXT_BYTE (byte) -// TODO: this function cannot parse z values than would not fit in a uint64 /** * @brief Parses a Z number * @@ -285,7 +284,6 @@ static tz_exc parse_operations_init(struct parsed_operation_group *const out, TZ_CHECK(compute_pkh(&out->public_key, &out->signing, path_with_curve)); // Start out with source = signing, for reveals - // TODO: This is slightly hackish memcpy(&out->operation.source, &out->signing, sizeof(out->signing)); state->op_step = 0; @@ -304,6 +302,9 @@ static tz_exc parse_operations_init(struct parsed_operation_group *const out, bool parse_operations_final(struct parse_state *const state, struct parsed_operation_group *const out) { + if ((state == NULL) || (out == NULL)) { + return false; + } if ((out->operation.tag == OPERATION_TAG_NONE) && !out->has_reveal) { return false; } diff --git a/src/operations.h b/src/operations.h index 764e0735..a13bf888 100644 --- a/src/operations.h +++ b/src/operations.h @@ -53,7 +53,7 @@ struct operation_group_header { */ struct implicit_contract { raw_tezos_header_signature_type_t signature_type; ///< type of the contract signature - uint8_t pkh[HASH_SIZE]; ///< raw public key hash + uint8_t pkh[KEY_HASH_SIZE]; ///< raw public key hash } __attribute__((packed)); /** @@ -72,7 +72,7 @@ union public_key { */ struct delegation_contents { raw_tezos_header_signature_type_t signature_type; ///< type of the delegate signature - uint8_t hash[HASH_SIZE]; ///< raw delegate + uint8_t hash[KEY_HASH_SIZE]; ///< raw delegate } __attribute__((packed)); /** diff --git a/src/to_string.c b/src/to_string.c index 7d440d0d..5dccde8a 100644 --- a/src/to_string.c +++ b/src/to_string.c @@ -34,14 +34,14 @@ static int pkh_to_string(char *const dest, size_t const dest_size, signature_type_t const signature_type, - uint8_t const hash[HASH_SIZE]); + uint8_t const hash[KEY_HASH_SIZE]); tz_exc bip32_path_with_curve_to_pkh_string(char *const out, size_t const out_size, bip32_path_with_curve_t const *const key) { tz_exc exc = SW_OK; cx_err_t error = CX_OK; - uint8_t hash[HASH_SIZE]; + uint8_t hash[KEY_HASH_SIZE]; TZ_ASSERT_NOT_NULL(out); TZ_ASSERT_NOT_NULL(key); @@ -87,7 +87,7 @@ static void compute_hash_checksum(uint8_t out[TEZOS_HASH_CHECKSUM_SIZE], static int pkh_to_string(char *const dest, size_t const dest_size, signature_type_t const signature_type, - uint8_t const hash[HASH_SIZE]) { + uint8_t const hash[KEY_HASH_SIZE]) { if ((dest == NULL) || (hash == NULL)) { return -1; } @@ -95,7 +95,7 @@ static int pkh_to_string(char *const dest, // Data to encode struct __attribute__((packed)) { uint8_t prefix[3]; - uint8_t hash[HASH_SIZE]; + uint8_t hash[KEY_HASH_SIZE]; uint8_t checksum[TEZOS_HASH_CHECKSUM_SIZE]; } data; @@ -161,13 +161,14 @@ static int chain_id_to_string(char *const dest, return base58_encode((const uint8_t *) &data, sizeof(data), dest, dest_size); } -#define SAFE_STRCPY(dest, size, x) \ - ({ \ - if (size < sizeof(x)) { \ - return -1; \ - } \ - strlcpy(dest, x, size); \ - return sizeof(x); \ +#define SAFE_STRCPY(dest, dest_size, in) \ + ({ \ + size_t in_size = strlen(in); \ + if (dest_size < in_size) { \ + return -1; \ + } \ + strlcpy(dest, in, dest_size); \ + return in_size; \ }) int chain_id_to_string_with_aliases(char *const dest, @@ -302,7 +303,7 @@ int microtez_to_string(char *const dest, size_t dest_size, uint64_t number) { } int hwm_to_string(char *dest, size_t dest_size, high_watermark_t const *const hwm) { - if (dest == NULL) { + if ((dest == NULL) || (hwm == NULL)) { return -1; } int result = number_to_string(dest, dest_size, hwm->highest_level); @@ -323,7 +324,7 @@ int hwm_to_string(char *dest, size_t dest_size, high_watermark_t const *const hw } int hwm_status_to_string(char *dest, size_t dest_size, volatile bool const *hwm_disabled) { - if ((dest == NULL) || (dest_size < 9u)) { + if ((dest == NULL) || (dest_size < 9u) || (hwm_disabled == NULL)) { return -1; } memcpy(dest, *hwm_disabled ? "Disabled" : "Enabled", dest_size); @@ -332,15 +333,8 @@ int hwm_status_to_string(char *dest, size_t dest_size, volatile bool const *hwm_ int copy_string(char *const dest, size_t const dest_size, char const *const src) { if ((dest == NULL) || (src == NULL)) { - return false; - } - - char const *const src_in = (char const *) PIC(src); - // I don't care that we will loop through the string twice, latency is not an issue - size_t src_size = strlen(src_in); - if (src_size >= dest_size) { return -1; } - strlcpy(dest, src_in, dest_size); - return src_size; + char const *const src_in = (char const *) PIC(src); + SAFE_STRCPY(dest, dest_size, src_in); } diff --git a/src/types.h b/src/types.h index 98322eff..522c85a4 100644 --- a/src/types.h +++ b/src/types.h @@ -204,15 +204,14 @@ typedef struct { no need to track HWM using Ledger.*/ } baking_data; -#define SIGN_HASH_SIZE 32u // TODO: Rename or use a different constant. +#define SIGN_HASH_SIZE 32u -#define PKH_STRING_SIZE 40u // includes null byte // TODO: use sizeof for this. +#define PKH_STRING_SIZE 40u // includes null byte #define HWM_STATUS_SIZE 9u // HWM status takes values Enabled and Disabled. #define PROTOCOL_HASH_BASE58_STRING_SIZE \ sizeof("ProtoBetaBetaBetaBetaBetaBetaBetaBetaBet11111a5ug96") -// TODO: Rename to KEY_HASH_SIZE -#define HASH_SIZE 20u +#define KEY_HASH_SIZE 20u /** * @brief This structure represents the content of a parsed baking data @@ -235,7 +234,7 @@ typedef struct parsed_contract { signature_type_t signature_type; /**< 0 in originated case An implicit contract with signature_type of 0 means not present*/ - uint8_t hash[HASH_SIZE]; ///< hash of the contract + uint8_t hash[KEY_HASH_SIZE]; ///< hash of the contract } parsed_contract_t; /** diff --git a/test/requirements.txt b/test/requirements.txt index 5841c6a6..74735960 100644 --- a/test/requirements.txt +++ b/test/requirements.txt @@ -1,5 +1,5 @@ base58 bip32 GitPython -pytezos -ragger[tests,speculos] +pytezos>=3.11.3 +ragger>=1.18.1 diff --git a/test/setup_script.sh b/test/setup_script.sh index d6656e8b..7f02b000 100755 --- a/test/setup_script.sh +++ b/test/setup_script.sh @@ -16,3 +16,4 @@ # limitations under the License. echo "No setup" +pip install ragger[tests,speculos]