From 892f3e348dacb6c7b9880bc697234a78aaf6d80a Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Thu, 23 Jun 2022 18:00:31 +0200 Subject: [PATCH] fix(crypto): Remove public key from the ed25519 signing API. [no changelog] --- .../modtrezorcrypto/modtrezorcrypto-ed25519.h | 13 ++------- crypto/bip32.c | 17 ++--------- crypto/ed25519-donna/README.md | 2 +- crypto/ed25519-donna/ed25519-keccak.h | 2 +- crypto/ed25519-donna/ed25519-sha3.h | 2 +- crypto/ed25519-donna/ed25519.c | 28 +++++++++++++++---- crypto/ed25519-donna/ed25519.h | 4 +-- crypto/fuzzer/fuzzer.c | 2 +- crypto/nem.c | 3 +- crypto/tests/test_check.c | 4 +-- crypto/tests/test_check_cardano.h | 3 +- crypto/tests/test_speed.c | 6 ++-- legacy/firmware/stellar.c | 3 +- 13 files changed, 41 insertions(+), 48 deletions(-) diff --git a/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ed25519.h b/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ed25519.h index d893d6f67..3aac1d9b3 100644 --- a/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ed25519.h +++ b/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ed25519.h @@ -77,7 +77,6 @@ STATIC mp_obj_t mod_trezorcrypto_ed25519_sign(size_t n_args, if (msg.len == 0) { mp_raise_ValueError("Empty data to sign"); } - ed25519_public_key pk = {0}; mp_buffer_info_t hash_func = {0}; vstr_t sig = {0}; vstr_init_len(&sig, sizeof(ed25519_signature)); @@ -86,16 +85,14 @@ STATIC mp_obj_t mod_trezorcrypto_ed25519_sign(size_t n_args, mp_get_buffer_raise(args[2], &hash_func, MP_BUFFER_READ); // if hash_func == 'keccak': if (memcmp(hash_func.buf, "keccak", sizeof("keccak")) == 0) { - ed25519_publickey_keccak(*(const ed25519_secret_key *)sk.buf, pk); ed25519_sign_keccak(msg.buf, msg.len, *(const ed25519_secret_key *)sk.buf, - pk, *(ed25519_signature *)sig.buf); + *(ed25519_signature *)sig.buf); } else { vstr_clear(&sig); mp_raise_ValueError("Unknown hash function"); } } else { - ed25519_publickey(*(const ed25519_secret_key *)sk.buf, pk); - ed25519_sign(msg.buf, msg.len, *(const ed25519_secret_key *)sk.buf, pk, + ed25519_sign(msg.buf, msg.len, *(const ed25519_secret_key *)sk.buf, *(ed25519_signature *)sig.buf); } @@ -128,14 +125,10 @@ STATIC mp_obj_t mod_trezorcrypto_ed25519_sign_ext(mp_obj_t secret_key, if (msg.len == 0) { mp_raise_ValueError("Empty data to sign"); } - ed25519_public_key pk = {0}; - - ed25519_publickey_ext(*(const ed25519_secret_key *)sk.buf, - *(const ed25519_secret_key *)skext.buf, pk); vstr_t sig = {0}; vstr_init_len(&sig, sizeof(ed25519_signature)); ed25519_sign_ext(msg.buf, msg.len, *(const ed25519_secret_key *)sk.buf, - *(const ed25519_secret_key *)skext.buf, pk, + *(const ed25519_secret_key *)skext.buf, *(ed25519_signature *)sig.buf); return mp_obj_new_str_from_vstr(&mp_type_bytes, &sig); } diff --git a/crypto/bip32.c b/crypto/bip32.c index c36ff9810..34130dbb0 100644 --- a/crypto/bip32.c +++ b/crypto/bip32.c @@ -646,23 +646,12 @@ int hdnode_sign(HDNode *node, const uint8_t *msg, uint32_t msg_len, return 1; // signatures are not supported } else { if (node->curve == &ed25519_info) { - if (hdnode_fill_public_key(node) != 0) { - return 1; - } - ed25519_sign(msg, msg_len, node->private_key, node->public_key + 1, sig); + ed25519_sign(msg, msg_len, node->private_key, sig); } else if (node->curve == &ed25519_sha3_info) { - if (hdnode_fill_public_key(node) != 0) { - return 1; - } - ed25519_sign_sha3(msg, msg_len, node->private_key, node->public_key + 1, - sig); + ed25519_sign_sha3(msg, msg_len, node->private_key, sig); #if USE_KECCAK } else if (node->curve == &ed25519_keccak_info) { - if (hdnode_fill_public_key(node) != 0) { - return 1; - } - ed25519_sign_keccak(msg, msg_len, node->private_key, node->public_key + 1, - sig); + ed25519_sign_keccak(msg, msg_len, node->private_key, sig); #endif } else { return 1; // unknown or unsupported curve diff --git a/crypto/ed25519-donna/README.md b/crypto/ed25519-donna/README.md index 73eadf8db..db40438ec 100644 --- a/crypto/ed25519-donna/README.md +++ b/crypto/ed25519-donna/README.md @@ -127,7 +127,7 @@ To generate a public key: To sign a message: ed25519_signature sig; - ed25519_sign(message, message_len, sk, pk, signature); + ed25519_sign(message, message_len, sk, signature); To verify a signature: diff --git a/crypto/ed25519-donna/ed25519-keccak.h b/crypto/ed25519-donna/ed25519-keccak.h index d5321800e..d40ce7797 100644 --- a/crypto/ed25519-donna/ed25519-keccak.h +++ b/crypto/ed25519-donna/ed25519-keccak.h @@ -10,7 +10,7 @@ extern "C" { void ed25519_publickey_keccak(const ed25519_secret_key sk, ed25519_public_key pk); int ed25519_sign_open_keccak(const unsigned char *m, size_t mlen, const ed25519_public_key pk, const ed25519_signature RS); -void ed25519_sign_keccak(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_public_key pk, ed25519_signature RS); +void ed25519_sign_keccak(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, ed25519_signature RS); int ed25519_scalarmult_keccak(ed25519_public_key res, const ed25519_secret_key sk, const ed25519_public_key pk); diff --git a/crypto/ed25519-donna/ed25519-sha3.h b/crypto/ed25519-donna/ed25519-sha3.h index 58748a555..51b764b03 100644 --- a/crypto/ed25519-donna/ed25519-sha3.h +++ b/crypto/ed25519-donna/ed25519-sha3.h @@ -10,7 +10,7 @@ extern "C" { void ed25519_publickey_sha3(const ed25519_secret_key sk, ed25519_public_key pk); int ed25519_sign_open_sha3(const unsigned char *m, size_t mlen, const ed25519_public_key pk, const ed25519_signature RS); -void ed25519_sign_sha3(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_public_key pk, ed25519_signature RS); +void ed25519_sign_sha3(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, ed25519_signature RS); int ed25519_scalarmult_sha3(ed25519_public_key res, const ed25519_secret_key sk, const ed25519_public_key pk); diff --git a/crypto/ed25519-donna/ed25519.c b/crypto/ed25519-donna/ed25519.c index 00bc89226..2a7c7c941 100644 --- a/crypto/ed25519-donna/ed25519.c +++ b/crypto/ed25519-donna/ed25519.c @@ -107,10 +107,12 @@ ED25519_FN(ed25519_cosi_sign) (const unsigned char *m, size_t mlen, const ed2551 } void -ED25519_FN(ed25519_sign) (const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_public_key pk, ed25519_signature RS) { +ED25519_FN(ed25519_sign) (const unsigned char *m, size_t mlen, const ed25519_secret_key sk, ed25519_signature RS) { ed25519_hash_context ctx; bignum256modm r = {0}, S = {0}, a = {0}; ge25519 ALIGN(16) R = {0}; + ge25519 ALIGN(16) A = {0}; + ed25519_public_key pk = {0}; hash_512bits extsk = {0}, hashr = {0}, hram = {0}; ed25519_extsk(extsk, sk); @@ -128,13 +130,19 @@ ED25519_FN(ed25519_sign) (const unsigned char *m, size_t mlen, const ed25519_sec ge25519_scalarmult_base_niels(&R, ge25519_niels_base_multiples, r); ge25519_pack(RS, &R); + /* a = aExt[0..31] */ + expand256_modm(a, extsk, 32); + memzero(&extsk, sizeof(extsk)); + + /* A = aB */ + ge25519_scalarmult_base_niels(&A, ge25519_niels_base_multiples, a); + ge25519_pack(pk, &A); + /* S = H(R,A,m).. */ ed25519_hram(hram, RS, pk, m, mlen); expand256_modm(S, hram, 64); /* S = H(R,A,m)a */ - expand256_modm(a, extsk, 32); - memzero(&extsk, sizeof(extsk)); mul256_modm(S, S, a); memzero(&a, sizeof(a)); @@ -148,10 +156,12 @@ ED25519_FN(ed25519_sign) (const unsigned char *m, size_t mlen, const ed25519_sec #if USE_CARDANO void -ED25519_FN(ed25519_sign_ext) (const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_secret_key skext, const ed25519_public_key pk, ed25519_signature RS) { +ED25519_FN(ed25519_sign_ext) (const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_secret_key skext, ed25519_signature RS) { ed25519_hash_context ctx; bignum256modm r = {0}, S = {0}, a = {0}; ge25519 ALIGN(16) R = {0}; + ge25519 ALIGN(16) A = {0}; + ed25519_public_key pk = {0}; hash_512bits extsk = {0}, hashr = {0}, hram = {0}; /* we don't stretch the key through hashing first since its already 64 bytes */ @@ -172,13 +182,19 @@ ED25519_FN(ed25519_sign_ext) (const unsigned char *m, size_t mlen, const ed25519 ge25519_scalarmult_base_niels(&R, ge25519_niels_base_multiples, r); ge25519_pack(RS, &R); + /* a = aExt[0..31] */ + expand256_modm(a, extsk, 32); + memzero(&extsk, sizeof(extsk)); + + /* A = aB */ + ge25519_scalarmult_base_niels(&A, ge25519_niels_base_multiples, a); + ge25519_pack(pk, &A); + /* S = H(R,A,m).. */ ed25519_hram(hram, RS, pk, m, mlen); expand256_modm(S, hram, 64); /* S = H(R,A,m)a */ - expand256_modm(a, extsk, 32); - memzero(&extsk, sizeof(extsk)); mul256_modm(S, S, a); memzero(&a, sizeof(a)); diff --git a/crypto/ed25519-donna/ed25519.h b/crypto/ed25519-donna/ed25519.h index 6c0b58f0f..43cde535c 100644 --- a/crypto/ed25519-donna/ed25519.h +++ b/crypto/ed25519-donna/ed25519.h @@ -21,9 +21,9 @@ void ed25519_publickey_ext(const ed25519_secret_key sk, const ed25519_secret_key #endif int ed25519_sign_open(const unsigned char *m, size_t mlen, const ed25519_public_key pk, const ed25519_signature RS); -void ed25519_sign(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_public_key pk, ed25519_signature RS); +void ed25519_sign(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, ed25519_signature RS); #if USE_CARDANO -void ed25519_sign_ext(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_secret_key skext, const ed25519_public_key pk, ed25519_signature RS); +void ed25519_sign_ext(const unsigned char *m, size_t mlen, const ed25519_secret_key sk, const ed25519_secret_key skext, ed25519_signature RS); #endif int ed25519_scalarmult(ed25519_public_key res, const ed25519_secret_key sk, const ed25519_public_key pk); diff --git a/crypto/fuzzer/fuzzer.c b/crypto/fuzzer/fuzzer.c index af8a2ae6e..767eea4f9 100644 --- a/crypto/fuzzer/fuzzer.c +++ b/crypto/fuzzer/fuzzer.c @@ -922,7 +922,7 @@ int fuzz_ed25519_sign_verify(void) { ed25519_publickey(secret_key, public_key); // sign message, this should always succeed - ed25519_sign(message, sizeof(message), secret_key, public_key, signature); + ed25519_sign(message, sizeof(message), secret_key, signature); // verify message, we expect this to work ret = ed25519_sign_open(message, sizeof(message), public_key, signature); diff --git a/crypto/nem.c b/crypto/nem.c index 7c37e8d5e..dd77241ce 100644 --- a/crypto/nem.c +++ b/crypto/nem.c @@ -205,8 +205,7 @@ size_t nem_transaction_end(nem_transaction_ctx *ctx, const ed25519_secret_key private_key, ed25519_signature signature) { if (private_key != NULL && signature != NULL) { - ed25519_sign_keccak(ctx->buffer, ctx->offset, private_key, ctx->public_key, - signature); + ed25519_sign_keccak(ctx->buffer, ctx->offset, private_key, signature); } return ctx->offset; diff --git a/crypto/tests/test_check.c b/crypto/tests/test_check.c index ef3dd85ab..a95c35b66 100644 --- a/crypto/tests/test_check.c +++ b/crypto/tests/test_check.c @@ -6595,7 +6595,7 @@ START_TEST(test_ed25519) { UNMARK_SECRET_DATA(pk, sizeof(pk)); ck_assert_mem_eq(pk, fromhex(*spk), 32); - ed25519_sign(pk, 32, sk, pk, sig); + ed25519_sign(pk, 32, sk, sig); UNMARK_SECRET_DATA(sig, sizeof(sig)); ck_assert_mem_eq(sig, fromhex(*ssig), 64); @@ -6813,7 +6813,7 @@ START_TEST(test_ed25519_keccak) { ck_assert_mem_eq(public_key, fromhex(tests[i].public_key), 32); ed25519_sign_keccak(fromhex(tests[i].data), tests[i].length, private_key, - public_key, signature); + signature); UNMARK_SECRET_DATA(signature, sizeof(signature)); ck_assert_mem_eq(signature, fromhex(tests[i].signature), 64); diff --git a/crypto/tests/test_check_cardano.h b/crypto/tests/test_check_cardano.h index 78dabe0f0..81b84aa0c 100644 --- a/crypto/tests/test_check_cardano.h +++ b/crypto/tests/test_check_cardano.h @@ -95,8 +95,7 @@ START_TEST(test_ed25519_cardano_sign_vectors) { ck_assert_mem_eq(public_key, fromhex(*(test_data + 2)), 32); const uint8_t *message = (const uint8_t *)"Hello World"; - ed25519_sign_ext(message, 11, secret_key, secret_key_extension, public_key, - signature); + ed25519_sign_ext(message, 11, secret_key, secret_key_extension, signature); UNMARK_SECRET_DATA(signature, sizeof(signature)); ck_assert_mem_eq(signature, fromhex(*(test_data + 3)), 64); diff --git a/crypto/tests/test_speed.c b/crypto/tests/test_speed.c index 2d0dc347e..e2371ee85 100644 --- a/crypto/tests/test_speed.c +++ b/crypto/tests/test_speed.c @@ -50,7 +50,6 @@ void bench_sign_nist256p1(int iterations) { } void bench_sign_ed25519(int iterations) { - ed25519_public_key pk; ed25519_secret_key sk; ed25519_signature sig; @@ -58,10 +57,9 @@ void bench_sign_ed25519(int iterations) { "\xc5\x5e\xce\x85\x8b\x0d\xdd\x52\x63\xf9\x68\x10\xfe\x14\x43\x7c\xd3" "\xb5\xe1\xfb\xd7\xc6\xa2\xec\x1e\x03\x1f\x05\xe8\x6d\x8b\xd5", 32); - ed25519_publickey(sk, pk); for (int i = 0; i < iterations; i++) { - ed25519_sign(msg, sizeof(msg), sk, pk, sig); + ed25519_sign(msg, sizeof(msg), sk, sig); } } @@ -143,7 +141,7 @@ void bench_verify_ed25519(int iterations) { "\xb5\xe1\xfb\xd7\xc6\xa2\xec\x1e\x03\x1f\x05\xe8\x6d\x8b\xd5", 32); ed25519_publickey(sk, pk); - ed25519_sign(msg, sizeof(msg), sk, pk, sig); + ed25519_sign(msg, sizeof(msg), sk, sig); for (int i = 0; i < iterations; i++) { ed25519_sign_open(msg, sizeof(msg), pk, sig); diff --git a/legacy/firmware/stellar.c b/legacy/firmware/stellar.c index 4d2ebaf92..e263bad36 100644 --- a/legacy/firmware/stellar.c +++ b/legacy/firmware/stellar.c @@ -1368,8 +1368,7 @@ void stellar_getSignatureForActiveTx(uint8_t *out_signature) { sha256_Final(&(stellar_activeTx.sha256_ctx), to_sign); uint8_t signature[64] = {0}; - ed25519_sign(to_sign, sizeof(to_sign), node->private_key, - node->public_key + 1, signature); + ed25519_sign(to_sign, sizeof(to_sign), node->private_key, signature); memcpy(out_signature, signature, sizeof(signature)); }