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

Optimize bls #122

Open
wants to merge 6 commits into
base: main
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
36 changes: 28 additions & 8 deletions src/apdu.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,41 @@
#include <stdint.h>
#include <string.h>

int provide_pubkey(bip32_path_with_curve_t const* const path_with_curve) {
tz_exc read_path_with_curve(derivation_type_t derivation_type,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The derivation type is selected at the time of setup. It can not be changed during signing. so it makes sense to derivce the public key when read_path_with_curve is called from handle_setup. Rather create a new function setup_path_with_curve and derive public key there along with a call to read_path_with_curve. Call setup_path_with_curve from handle_setup and read_path_with_curve from other places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SETUP instruction is not required to sign:

  • AUTHORIZE_BAKING will setup the authorized key too.
  • If the authorized key was present in the RAM, SIGN with index 0x00 will tell which public key to sign with

The aims of this function is too make sure that, every time the stored Bip32-path-with-curve is updated, the stored PK is updated to.

buffer_t* buf,
bip32_path_with_curve_t* path_with_curve,
cx_ecfp_public_key_t* pubkey) {
tz_exc exc = SW_OK;
cx_err_t error = CX_OK;
bip32_path_with_curve_t tmp_path_with_curve = {0};

TZ_ASSERT_NOT_NULL(buf);
TZ_ASSERT_NOT_NULL(path_with_curve);

uint8_t resp[1u + MAX_SIGNATURE_SIZE] = {0};
size_t offset = 0;
tmp_path_with_curve.derivation_type = derivation_type;
TZ_ASSERT(read_bip32_path(buf, &tmp_path_with_curve.bip32_path), EXC_WRONG_VALUES);

// Do not derive the public key if the two path_with_curve are equal
if (!bip32_path_with_curve_eq(path_with_curve, &tmp_path_with_curve)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems my previous comment is addressed here. you can still think if a setup_path_with_curve makes sense.

memmove(path_with_curve, &tmp_path_with_curve, sizeof(bip32_path_with_curve_t));
if (pubkey != NULL) {
CX_CHECK(generate_public_key(pubkey, path_with_curve));
}
}

// Application could be PIN-locked, and pubkey->W_len would then be 0,
// so throwing an error rather than returning an empty key
TZ_ASSERT(os_global_pin_is_validated() == BOLOS_UX_OK, EXC_SECURITY);
end:
TZ_CONVERT_CX();
return exc;
}

cx_ecfp_public_key_t* pubkey = (cx_ecfp_public_key_t*) &(tz_ecfp_public_key_t){0};
CX_CHECK(generate_public_key(pubkey, path_with_curve));
int provide_pubkey(cx_ecfp_public_key_t const* const pubkey) {
tz_exc exc = SW_OK;
cx_err_t error = CX_OK;

TZ_ASSERT_NOT_NULL(pubkey);

uint8_t resp[1u + MAX_SIGNATURE_SIZE] = {0};
size_t offset = 0;

resp[offset] = pubkey->W_len;
offset++;
Expand Down
19 changes: 18 additions & 1 deletion src/apdu.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ static inline int io_send_apdu_err(uint16_t sw) {
return io_send_sw(sw);
}

/**
* @brief Reads a path with curve and derive the public key.
* Set [pubkey] to NULL to not deriving the public key.
* Will not derive the public key if the path with curve read
* is the same as the one provided.
*
* @param[in] derivation_type: Derivation type of the key.
* @param[in] buf: Buffer that should contains a bip32 path.
* @param[in/out] path_with_curve: Buffer to store the path with curve.
* @param[out] pubkey: Buffer to store the pubkey. Can be NULL
* @return tz_exc: exception, SW_OK if none
*/
tz_exc read_path_with_curve(derivation_type_t derivation_type,
buffer_t* buf,
bip32_path_with_curve_t* path_with_curve,
cx_ecfp_public_key_t* pubkey);

/**
* @brief Provides the public key in the apdu response
*
Expand All @@ -95,4 +112,4 @@ static inline int io_send_apdu_err(uint16_t sw) {
* @param path_with_curve: bip32 path and curve of the key
* @return int: zero or positive integer if success, negative integer otherwise.
*/
int provide_pubkey(bip32_path_with_curve_t const* const path_with_curve);
int provide_pubkey(cx_ecfp_public_key_t const* const pubkey);
1 change: 1 addition & 0 deletions src/apdu_hmac.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ static inline tz_exc hmac(uint8_t *const out,
CX_CHECK(sign(state->signed_hmac_key,
&signed_hmac_key_size,
path_with_curve,
NULL,
key_sha256,
sizeof(key_sha256)));

Expand Down
21 changes: 15 additions & 6 deletions src/apdu_pubkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* @return true
*/
static bool pubkey_ok(void) {
provide_pubkey(&global.path_with_curve);
provide_pubkey((cx_ecfp_public_key_t *) &global.public_key);
return true;
}

Expand Down Expand Up @@ -69,21 +69,29 @@ int handle_get_public_key(buffer_t *cdata,
bool authorize,
bool prompt) {
tz_exc exc = SW_OK;
cx_err_t error = CX_OK;

TZ_ASSERT_NOT_NULL(cdata);

global.path_with_curve.derivation_type = derivation_type;
if ((cdata->size == 0u) && authorize) {
TZ_ASSERT(copy_bip32_path_with_curve(&global.path_with_curve, &(g_hwm.baking_key)),
EXC_MEMORY_ERROR);
// Do not derive the public key if the two path_with_curve are equal
if (!bip32_path_with_curve_eq(&global.path_with_curve, &g_hwm.baking_key)) {
TZ_ASSERT(copy_bip32_path_with_curve(&global.path_with_curve, &g_hwm.baking_key),
EXC_MEMORY_ERROR);
CX_CHECK(generate_public_key((cx_ecfp_public_key_t *) &global.public_key,
&global.path_with_curve));
}
} else {
TZ_ASSERT(read_bip32_path(cdata, &global.path_with_curve.bip32_path), EXC_WRONG_VALUES);
TZ_CHECK(read_path_with_curve(derivation_type,
cdata,
&global.path_with_curve,
(cx_ecfp_public_key_t *) &global.public_key));
}

TZ_ASSERT(cdata->size == cdata->offset, EXC_WRONG_LENGTH);

if (!prompt) {
return provide_pubkey(&global.path_with_curve);
return provide_pubkey((cx_ecfp_public_key_t *) &global.public_key);
} else {
// INS_PROMPT_PUBLIC_KEY || INS_AUTHORIZE_BAKING
ui_callback_t cb;
Expand All @@ -100,5 +108,6 @@ int handle_get_public_key(buffer_t *cdata,
}

end:
TZ_CONVERT_CX();
return io_send_apdu_err(exc);
}
12 changes: 7 additions & 5 deletions src/apdu_setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ static bool ok(void) {

UPDATE_NVRAM;

provide_pubkey(&global.path_with_curve);
provide_pubkey((cx_ecfp_public_key_t *) &global.public_key);

return true;
}
Expand All @@ -72,14 +72,16 @@ int handle_setup(buffer_t *cdata, derivation_type_t derivation_type) {

TZ_ASSERT_NOT_NULL(cdata);

global.path_with_curve.derivation_type = derivation_type;

TZ_ASSERT(buffer_read_u32(cdata, &G.main_chain_id.v, BE) && // chain id
buffer_read_u32(cdata, &G.hwm.main, BE) && // main hwm level
buffer_read_u32(cdata, &G.hwm.test, BE) && // test hwm level
read_bip32_path(cdata, &global.path_with_curve.bip32_path),
buffer_read_u32(cdata, &G.hwm.test, BE), // test hwm level
EXC_WRONG_VALUES);

TZ_CHECK(read_path_with_curve(derivation_type,
cdata,
&global.path_with_curve,
(cx_ecfp_public_key_t *) &global.public_key));

TZ_ASSERT(cdata->size == cdata->offset, EXC_WRONG_LENGTH);

return prompt_setup(ok, reject);
Expand Down
54 changes: 41 additions & 13 deletions src/apdu_sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,13 @@ int select_signing_key(buffer_t *cdata, derivation_type_t derivation_type) {

clear_data();

TZ_ASSERT(read_bip32_path(cdata, &global.path_with_curve.bip32_path), EXC_WRONG_VALUES);
TZ_CHECK(read_path_with_curve(derivation_type,
cdata,
&global.path_with_curve,
(cx_ecfp_public_key_t *) &global.public_key));

TZ_ASSERT(cdata->size == cdata->offset, EXC_WRONG_LENGTH);

global.path_with_curve.derivation_type = derivation_type;

return io_send_sw(SW_OK);

end:
Expand Down Expand Up @@ -214,27 +215,49 @@ int handle_sign(buffer_t *cdata, const bool last, const bool with_hash) {
break;
case MAGIC_BYTE_UNSAFE_OP:
// Parse the operation. It will be verified in `baking_sign_complete`.
TZ_CHECK(parse_operations(cdata, &G.maybe_ops.v, &global.path_with_curve));
TZ_CHECK(parse_operations(cdata,
&G.maybe_ops.v,
(cx_ecfp_public_key_t *) &global.public_key));
break;
default:
TZ_FAIL(EXC_PARSE_ERROR);
}

CX_CHECK(
cx_hash_no_throw((cx_hash_t *) &G.hash_state.state, 0, cdata->ptr, cdata->size, NULL, 0));
#ifndef TARGET_NANOS
// There is no need to hash the message if it is not used for signing or if it is not sent at
// the end.
if (with_hash || (global.path_with_curve.derivation_type != DERIVATION_TYPE_BLS12_381)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of with_hash here. Is it used for BLS.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean if the derivation type is BLS dont hash at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The with_hash comes from the request SIGN_WITH_HASH.
We have to compute/send it if the sender requests it.

#endif
CX_CHECK(cx_hash_no_throw((cx_hash_t *) &G.hash_state.state,
0,
cdata->ptr,
cdata->size,
NULL,
0));
#ifndef TARGET_NANOS
}
#endif

#ifndef TARGET_NANOS
memmove(G.message, cdata->ptr, cdata->size);
G.message_len = cdata->size;
#endif

if (last) {
CX_CHECK(cx_hash_no_throw((cx_hash_t *) &G.hash_state.state,
CX_LAST,
NULL,
0,
G.final_hash,
sizeof(G.final_hash)));
#ifndef TARGET_NANOS
// There is no need to hash the message if it is not used for signing or if it is not sent
// at the end.
if (with_hash || (global.path_with_curve.derivation_type != DERIVATION_TYPE_BLS12_381)) {
#endif
CX_CHECK(cx_hash_no_throw((cx_hash_t *) &G.hash_state.state,
CX_LAST,
NULL,
0,
G.final_hash,
sizeof(G.final_hash)));
#ifndef TARGET_NANOS
}
#endif

G.maybe_ops.is_valid = parse_operations_final(&G.parse_state, &G.maybe_ops.v);

Expand Down Expand Up @@ -288,7 +311,12 @@ static int perform_signature(bool const send_hash) {

size_t signature_size = MAX_SIGNATURE_SIZE;

CX_CHECK(sign(resp + offset, &signature_size, &global.path_with_curve, message, message_len));
CX_CHECK(sign(resp + offset,
&signature_size,
&global.path_with_curve,
(cx_ecfp_public_key_t *) &global.public_key,
message,
message_len));

offset += signature_size;

Expand Down
27 changes: 19 additions & 8 deletions src/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,14 @@ WARN_UNUSED_RESULT cx_err_t bip32_derive_get_pubkey_bls(const uint32_t *path,
// https://gitlab.com/tezos/tezos/-/blob/master/src/lib_bls12_381_signature/bls12_381_signature.ml?ref_type=heads#L351
static const uint8_t CIPHERSUITE[] = "BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_AUG_";

WARN_UNUSED_RESULT cx_err_t bip32_derive_with_seed_bls_sign_hash(const uint32_t *path,
size_t path_len,
uint8_t const *msg,
size_t msg_len,
uint8_t *sig,
size_t *sig_len) {
WARN_UNUSED_RESULT cx_err_t
bip32_derive_with_seed_bls_sign_hash(const uint32_t *path,
size_t path_len,
cx_ecfp_384_public_key_t *public_key,
uint8_t const *msg,
size_t msg_len,
uint8_t *sig,
size_t *sig_len) {
cx_err_t error = CX_OK;
cx_ecfp_384_private_key_t privkey = {0};
uint8_t hash[CX_BLS_BLS12381_PARAM_LEN * 4] = {0};
Expand All @@ -156,8 +158,17 @@ WARN_UNUSED_RESULT cx_err_t bip32_derive_with_seed_bls_sign_hash(const uint32_t
// Derive private key according to BIP32 path
CX_CHECK(bip32_derive_init_privkey_bls(path, path_len, &privkey));

CX_CHECK(bip32_derive_get_pubkey_bls(path, path_len, raw_pubkey));
memmove(tmp, raw_pubkey + 1, BLS_COMPRESSED_PK_LEN);
if (public_key == NULL) {
CX_CHECK(bip32_derive_get_pubkey_bls(path, path_len, raw_pubkey));
memmove(tmp, raw_pubkey + 1, BLS_COMPRESSED_PK_LEN);
} else {
if ((public_key->curve != CX_CURVE_BLS12_381_G1) ||
(public_key->W_len < (BLS_COMPRESSED_PK_LEN + 1u))) {
error = CX_INVALID_PARAMETER_VALUE;
goto end;
}
memmove(tmp, public_key->W + 1, BLS_COMPRESSED_PK_LEN);
}
memmove(tmp + BLS_COMPRESSED_PK_LEN, msg, msg_len);

CX_CHECK(cx_hash_to_field(tmp,
Expand Down
17 changes: 11 additions & 6 deletions src/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ WARN_UNUSED_RESULT cx_err_t bip32_derive_get_pubkey_bls(const uint32_t *path,
*
* @param[in] path_len Bip32 path length.
*
* @param[in] public_key Must be the BLS key associated with the path and the inner seed.
* If NULL, it will be computed using the path and the inner seed.
*
* @param[in] msg Digest of the message to be signed.
* The length of *message* must be shorter than the group order size.
* Otherwise it is truncated.
Expand All @@ -73,10 +76,12 @@ WARN_UNUSED_RESULT cx_err_t bip32_derive_get_pubkey_bls(const uint32_t *path,
* - CX_EC_INVALID_CURVE
* - CX_INTERNAL_ERROR
*/
WARN_UNUSED_RESULT cx_err_t bip32_derive_with_seed_bls_sign_hash(const uint32_t *path,
size_t path_len,
uint8_t const *msg,
size_t msg_len,
uint8_t *sig,
size_t *sig_len);
WARN_UNUSED_RESULT cx_err_t
bip32_derive_with_seed_bls_sign_hash(const uint32_t *path,
size_t path_len,
cx_ecfp_384_public_key_t *public_key,
uint8_t const *msg,
size_t msg_len,
uint8_t *sig,
size_t *sig_len);
#endif
1 change: 1 addition & 0 deletions src/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ typedef struct {
} dynamic_display;

bip32_path_with_curve_t path_with_curve; ///< holds the bip32 path and curve of the current key
tz_ecfp_public_key_t public_key; ///< holds the current public key

/// apdu handling state
struct {
Expand Down
Loading
Loading