From 8b88125e7e52692a253dbac354af4ab4234cea34 Mon Sep 17 00:00:00 2001 From: srpatcha Date: Tue, 1 Sep 2026 21:03:44 -0700 Subject: [PATCH] fix(crypto): reject Ed25519 public keys outside the prime-order subgroup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit eos_ed25519_verify() decoded the public key and never checked which subgroup it was in. Ed25519 has eight low-order points, and for any of them every term of the verification equation collapses regardless of the message, so a signature of all zeros verifies against arbitrary firmware: identity_pub[32] = {1} /* 01 00..00, the identity */ identity_sig[64] = {1} /* R = identity, S = 0 */ master: rc=0 -> ACCEPTED (forgery works) fixed: rc=-4 -> rejected A complete secure-boot bypass: anyone able to set the trusted key can boot arbitrary firmware. The implementation is muhammadburhandevv-hub's, from #57, carried across because that PR has been unmergeable for four days — it branched before test_secure_boot.c and test_stage0_reset_entry.py landed, so merging it now would delete them. Only core/ed25519_verify.c is taken; the build-repair half of #57 is already upstream. Credit is theirs. Their formulation is better than the one I wrote for the same defect in eos (#99 there): return point_is_identity(multiple) && !point_is_identity(public_key); One expression covering both required checks. The subgroup test alone is insufficient — the identity has order 1, which divides L, so [L]identity = identity and it passes. My eos version needed two separate guards to say this; theirs says it once. The regression tests are mine. test_ed25519_zero_pubkey_rejected already existed and looks like it covers this, but does not: the identity encodes as 01 00..00, not 00 00..00, and unlike the all-zero encoding it decodes to a valid curve point. Two tests are added — the identity case, and all sixteen combinations of four low-order encodings used as key and as R. Verified the tests fail against the unfixed code rather than merely passing with it: unfixed test_ed25519_identity_key_forgery_rejected [FAIL] at line 210 fixed 12/12 tests passed full 20/20 ctest, 0 build errors eos had the identical gap in an independent implementation and was fixed in embeddedos-org/eos#99, which has merged. This was the last one. Refs #73, #57 Co-authored-by: muhammadburhandevv-hub Co-Authored-By: Claude Opus 5 (1M context) --- core/ed25519_verify.c | 30 ++++++++++++++++++++++++++ tests/unit/test_ed25519.c | 45 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/core/ed25519_verify.c b/core/ed25519_verify.c index 022b679..0367fa1 100644 --- a/core/ed25519_verify.c +++ b/core/ed25519_verify.c @@ -286,6 +286,34 @@ static void scalarbase(gf r[4], const uint8_t *s) scalarmult(r, q, s); } +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; +} + +/* Public keys must be non-identity points in Ed25519's prime-order subgroup. + * Merely decoding a point is insufficient: an identity or torsion key can + * make the verification equation true without knowledge of a private key. */ +static int public_key_is_valid_subgroup(gf public_key[4]) +{ + uint8_t order_l[32]; + gf q[4], multiple[4]; + + for (int i = 0; i < 32; i++) + order_l[i] = (uint8_t)ORDER_L[i]; + for (int i = 0; i < 4; i++) + fe_copy16(q[i], public_key[i]); + + scalarmult(multiple, q, order_l); + return point_is_identity(multiple) && !point_is_identity(public_key); +} + /* Decode a compressed point into -P (the negation is what verification wants). */ static int unpackneg(gf r[4], const uint8_t p[32]) { @@ -416,6 +444,8 @@ int eos_ed25519_verify(const uint8_t signature[64], gf A[4]; if (unpackneg(A, public_key) != EOS_OK) return EOS_ERR_SIGNATURE; + if (!public_key_is_valid_subgroup(A)) + return EOS_ERR_SIGNATURE; /* k = SHA-512(R || A || M) mod L */ eos_sha512_ctx_t ctx; diff --git a/tests/unit/test_ed25519.c b/tests/unit/test_ed25519.c index f9933fe..8cba99a 100644 --- a/tests/unit/test_ed25519.c +++ b/tests/unit/test_ed25519.c @@ -197,6 +197,47 @@ TEST(test_ed25519_null_args) ASSERT(eos_ed25519_verify(sig, pk, NULL, sizeof(msg)) != EOS_OK); } +TEST(test_ed25519_identity_key_forgery_rejected) +{ + /* The identity point encodes as 01 00..00. With the public key and R both + * set to it and S zero, every term of the verification equation collapses + * to the identity, so the equation holds for any message at all. That is + * not a weakened signature; it is no signature. */ + uint8_t identity_pub[32] = {1}; + uint8_t identity_sig[64] = {1}; + const uint8_t msg[] = "untrusted firmware"; + + ASSERT(eos_ed25519_verify(identity_sig, identity_pub, + msg, sizeof(msg) - 1) != EOS_OK); +} + +TEST(test_ed25519_low_order_keys_rejected) +{ + /* zero_pubkey covers one encoding; Ed25519 has eight low-order points and + * the family is what matters. A subgroup test alone is not enough either: + * the identity has order 1, which divides L, so [L]identity = identity and + * it passes. Both checks are required. */ + static const uint8_t low_order[4][32] = { + {0}, + {1}, + {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}, + {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}, + }; + const uint8_t msg[] = "untrusted firmware"; + + for (int k = 0; k < 4; k++) { + for (int r = 0; r < 4; r++) { + uint8_t sig[64]; + memset(sig, 0, sizeof(sig)); + memcpy(sig, low_order[r], 32); + ASSERT(eos_ed25519_verify(sig, low_order[k], + msg, sizeof(msg) - 1) != EOS_OK); + } + } +} + TEST(test_ed25519_zero_pubkey_rejected) { uint8_t zero_pub[32], sig[64], msg[1]; @@ -279,12 +320,14 @@ int main(void) run_test_ed25519_wrong_public_key_rejected(); run_test_ed25519_malleated_signature_rejected(); run_test_ed25519_null_args(); + run_test_ed25519_identity_key_forgery_rejected(); + run_test_ed25519_low_order_keys_rejected(); run_test_ed25519_zero_pubkey_rejected(); run_test_ed25519_zero_signature_rejected(); run_test_sha512_known_answers(); run_test_sha512_streaming_matches_one_shot(); - tests_run = 10; + tests_run = 12; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; }