From 0f345139b7a5e58d135214ad97a50fd3706cfb16 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Wed, 2 Sep 2026 10:56:04 +0530 Subject: [PATCH] fix(ed25519): reject public keys outside the prime-order subgroup An all-zero public key with an all-zero signature verifies against any message, and it is not the only pair that does. Ed25519 has eight low-order points; used as the public key, the verification equation can hold regardless of the message, so the signature is not weak but absent. eos_ed25519_verify() is what stands between eos_image_verify_signature() and a booted image. The verifier rejected a non-canonical S and a key off the curve, and stopped there. Being on the curve says nothing about order. Adds the subgroup test: [L]A must be the identity, and A must not itself be the identity -- the identity's order is 1, which divides L, so the multiply alone admits it. A arrives negated from unpackneg(); [L](-A) = -[L]A and the identity is its own negation, so neither condition is affected by the sign. The formulation is taken from #57 by @muhammadburhandevv-hub, which reached this first: it derives the byte scalar from the existing ORDER_L instead of writing L out a second time, and states both conditions in one expression. A mistyped duplicate constant would reject valid keys and only in the field, so having no second copy is worth more than it looks. Tests sweep all 64 combinations of the eight low-order encodings as the public key and as R, with S = 0, across eight messages. The breadth is not thoroughness for its own sake: which pair forges depends on k = SHA-512(R || A || M) mod L, so for the order-4 and order-8 points it depends on the message. Against master, 16 of those 64 pairs are accepted for at least one of these messages; with the fix, 0. That is also why the suite's existing test_ed25519_zero_pubkey_rejected and test_ed25519_zero_signature_rejected both passed while the bypass was open -- each holds one input legitimate, and the forgery needs both. My own first attempt at an identity case used R = 0, which is not one of the 16, so it passed against unfixed code and proved nothing. RFC 8032 section 7.1 vectors 1-3 still verify and tampering is still rejected, checked before and after: a wrong subgroup test rejects valid keys silently. 12/12 in this suite, 20/20 repo-wide with EBLDR_SANITIZE=ON. Against the unfixed verifier the sweep fails on its first accepted pair. Co-authored-by: muhammadburhandevv-hub Co-Authored-By: Claude Opus 5 (1M context) --- core/ed25519_verify.c | 53 ++++++++++++++++ tests/unit/test_ed25519.c | 123 +++++++++++++++++++++++++++++++++++++- 2 files changed, 175 insertions(+), 1 deletion(-) diff --git a/core/ed25519_verify.c b/core/ed25519_verify.c index 022b679..5040122 100644 --- a/core/ed25519_verify.c +++ b/core/ed25519_verify.c @@ -276,6 +276,55 @@ static void scalarmult(gf r[4], gf q[4], const uint8_t *s) } } +/* The identity encodes as y = 1 with the sign bit clear. Takes a point rather + * than an encoding so both callers below can pass one directly. */ +static int point_is_identity(gf p[4]) +{ + uint8_t encoded[32]; + point_pack(encoded, p); + + uint8_t diff = (uint8_t)(encoded[0] ^ 1U); + for (int i = 1; i < 32; i++) + diff |= encoded[i]; + return diff == 0; +} + +/* Reject a public key outside the prime-order subgroup. + * + * Decoding a point is not enough. Ed25519 has eight points of low order, and + * for any of them the verification equation can hold regardless of the + * message: an all-zero key with an all-zero signature verified against any + * content at all, which is not a weak signature but no signature. + * + * Two conditions, because neither alone is sufficient. [L]A = identity holds + * for every point whose order divides L -- including the identity itself, + * whose order is 1 -- so the identity must also be excluded explicitly. + * + * The scalar is derived from ORDER_L rather than written out a second time, + * so there is no separate constant to transcribe wrongly: a mistyped L would + * reject valid keys, and only in the field. + * + * A arrives negated from unpackneg(). [L](-A) = -[L]A and the identity is its + * own negation, so neither condition is affected by the sign. + * + * Formulation taken from eBoot#57 by @muhammadburhandevv-hub, which reached + * this before I did and states both conditions in one expression. + */ +static int key_has_prime_order(gf A[4]) +{ + uint8_t order_l[32]; + gf q[4], multiple[4]; + int i; + + for (i = 0; i < 32; i++) + order_l[i] = (uint8_t)ORDER_L[i]; + for (i = 0; i < 4; i++) + fe_copy16(q[i], A[i]); + + scalarmult(multiple, q, order_l); + return point_is_identity(multiple) && !point_is_identity(A); +} + static void scalarbase(gf r[4], const uint8_t *s) { gf q[4]; @@ -417,6 +466,10 @@ int eos_ed25519_verify(const uint8_t signature[64], if (unpackneg(A, public_key) != EOS_OK) return EOS_ERR_SIGNATURE; + /* On the curve is not enough: a low-order key verifies anything. */ + if (!key_has_prime_order(A)) + return EOS_ERR_SIGNATURE; + /* k = SHA-512(R || A || M) mod L */ eos_sha512_ctx_t ctx; uint8_t k[64]; diff --git a/tests/unit/test_ed25519.c b/tests/unit/test_ed25519.c index f9933fe..e2722a4 100644 --- a/tests/unit/test_ed25519.c +++ b/tests/unit/test_ed25519.c @@ -219,8 +219,126 @@ TEST(test_ed25519_zero_signature_rejected) ASSERT(eos_ed25519_verify(sig, pk, msg, 1) != EOS_OK); } +/* The eight low-order point encodings. A public key must be none of them, and + * so must the R half of a signature. */ +/* Messages the low-order sweeps run each candidate pair against. Which pair + * forges depends on k = SHA-512(R || A || M), so the message matters: the + * count below is for exactly this list. */ +static const char *const messages[] = { + "untrusted firmware", "malicious package payload", + "AB", "ABC", "boot this image", "", "A", "0123456789", +}; + +static const uint8_t k_low_order[8][32] = { + /* y = 0, order 4 */ + {0}, + /* the identity, order 1 */ + {1}, + /* order 8 */ + {0x26,0xe8,0x95,0x8f,0xc2,0xb2,0x27,0xb0,0x45,0xc3,0xf4,0x89,0xf2,0xef,0x98,0xf0, + 0xd5,0xdf,0xac,0x05,0xd3,0xc6,0x33,0x39,0xb1,0x38,0x02,0x88,0x6d,0x53,0xfc,0x05}, + /* order 8 */ + {0xc7,0x17,0x6a,0x70,0x3d,0x4d,0xd8,0x4f,0xba,0x3c,0x0b,0x76,0x0d,0x10,0x67,0x0f, + 0x2a,0x20,0x53,0xfa,0x2c,0x39,0xcc,0xc6,0x4e,0xc7,0xfd,0x77,0x92,0xac,0x03,0x7a}, + /* p - 1 */ + {0xec,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x7f}, + /* p, which reduces to y = 0 */ + {0xed,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x7f}, + /* p + 1, which reduces to the identity */ + {0xee,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x7f}, + /* non-canonical, above p */ + {0xd9,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff}, +}; + +TEST(test_ed25519_low_order_forgeries_rejected) +{ + /* All 64 combinations of the eight encodings as the public key and as R, + * with S = 0, against several messages. + * + * The sweep is not thoroughness for its own sake. Which combination + * forges depends on k = SHA-512(R || A || M) mod L, so for the order-4 + * and order-8 points it depends on the *message*: roughly one message in + * four takes for an order-4 key. A test pinned to one pair and one + * message can therefore pass against unfixed code and look like a + * regression test without being one. + * + * That is not hypothetical. Measured against this file's parent commit, + * 16 of these 64 pairs are accepted for at least one of the messages + * below -- and the pair (identity key, R = 0) is not among them, so an + * identity test written with an all-zero signature passes on the unfixed + * code and proves nothing. The pair that does forge with an identity key + * is R = identity, not R = 0. + */ + unsigned k, r, m; + + for (k = 0; k < 8; k++) { + for (r = 0; r < 8; r++) { + uint8_t sig[64]; + memset(sig, 0, sizeof(sig)); + memcpy(sig, k_low_order[r], 32); /* R = low order, S = 0 */ + + for (m = 0; m < sizeof(messages) / sizeof(messages[0]); m++) { + ASSERT(eos_ed25519_verify(sig, k_low_order[k], + (const uint8_t *)messages[m], + strlen(messages[m])) != EOS_OK); + } + } + } +} + +TEST(test_ed25519_low_order_key_with_real_signature_rejected) +{ + /* A low-order key paired with a genuine signature over a real message, + * rather than with S = 0. The key alone is disqualifying: the subgroup + * test must not depend on the signature being degenerate too. */ + uint8_t sig[64], msg[1]; + unsigned k; + + hex2bin(k_vectors[1].sig_hex, sig, 64); + hex2bin(k_vectors[1].msg_hex, msg, 1); + + for (k = 0; k < 8; k++) + ASSERT(eos_ed25519_verify(sig, k_low_order[k], msg, 1) != EOS_OK); +} + /* ---- SHA-512, the hash Ed25519 is defined over (FIPS 180-4) ---- */ +TEST(test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery) +{ + /* The subgroup check guards the public key, not R, and that is + * deliberate rather than an omission. Verification computes + * [S]B = R + [k]A: a low-order A collapses the equation for any message, + * which is the bypass. A low-order R with a genuine A does not, because + * k = SHA-512(R || A || M) depends on R, so an attacker choosing R would + * have to solve for it. + * + * Measured rather than argued: all eight low-order encodings as R with + * S = 0, against RFC 8032 test 2's real public key, are rejected on the + * unfixed verifier too -- 0 of 8, before and after. Pinned here so that + * a later "R should be checked as well" change is recognised as scope + * creep, and so that removing the key check because "R is the untrusted + * half" fails loudly. + */ + uint8_t pk[32], sig[64]; + size_t r, m; + + hex2bin(k_vectors[1].pubkey_hex, pk, 32); + + for (r = 0; r < sizeof(k_low_order) / sizeof(k_low_order[0]); r++) { + for (m = 0; m < sizeof(messages) / sizeof(messages[0]); m++) { + memset(sig, 0, sizeof(sig)); + memcpy(sig, k_low_order[r], 32); + ASSERT(eos_ed25519_verify(sig, pk, + (const uint8_t *)messages[m], + strlen(messages[m])) != EOS_OK); + } + } +} + TEST(test_sha512_known_answers) { struct { const char *msg; const char *digest; } kat[] = { @@ -281,10 +399,13 @@ int main(void) run_test_ed25519_null_args(); run_test_ed25519_zero_pubkey_rejected(); run_test_ed25519_zero_signature_rejected(); + run_test_ed25519_low_order_forgeries_rejected(); + run_test_ed25519_low_order_key_with_real_signature_rejected(); + run_test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery(); run_test_sha512_known_answers(); run_test_sha512_streaming_matches_one_shot(); - tests_run = 10; + tests_run = 13; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; }