fix(crypto): remove undefined behaviour from Ed25519 ref10 arithmetic

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 ### Security
+- **The Ed25519 arithmetic that verifies package signatures has no undefined behaviour left on its tested paths.** The ref10 field and scalar code in `services/crypto/src` reduced limbs with `carry << 25` (and `<< 26`, `<< 21`) on signed values that are negative about half the time, packed bytes in `fe_tobytes()` with `int32_t` shifts that overflow, and computed `babs` in `ge.c`'s `select()` by shifting a negative `signed char`. All three are undefined in C, so a compiler is entitled to miscompile the verifier, and under `-fsanitize=undefined` five suites aborted (`test_crypto_ed25519_loworder`, `test_ed25519_contract`, `test_pkg_anchor_usable`, `test_pkg_trust_anchor`, `test_pkg_install_paths`). Each shift is now a multiplication by the same power of two, or a shift of the value cast to `uint32_t`, which is what two's-complement hardware computed anyway, so the results do not change. No output change was observed: the new `tests/test_ed25519_rfc8032_kat.c` pins key generation and signing to RFC 8032 section 7.1 byte for byte, and passes against both the old and new code at `-O3`. Before this change nothing pinned the signer's output; the package tests only round-tripped their own signatures.
 - **Build backends no longer hand configuration to a shell unchecked, and never run a truncated command.** All eleven backends under `backends/` built their `system()` lines with `snprintf` from the project's `src_dir`, `build_dir`, `install_dir`, toolchain and option values, quoted but unfiltered: a value carrying `"; touch pwned; "` ran as shell (reproduced against the CMake backend's `clean`), and `freertos_install`, `cargo_install` and `nuttx_install` formatted three `EOS_MAX_PATH` paths into a 1024-byte buffer, so a long path ran a cut-off line. Commands are now assembled through `EosShellCmd` (`core/include/eos/shell_cmd.h`): program text written in the tree is appended as text, every configuration value goes through `eos_shell_cmd_arg()` (double-quoted, refused if it carries a shell metacharacter or control character) or `eos_shell_cmd_word()` (unquoted, an allowlist of one word), and a line that does not fit or carries a refused value is refused with `EOS_ERR_INVALID` before anything runs. Option values are quoted, so a value with a space is one argument. The two rules are the ones `linux_security` reviewed and shipped, moved to `core` and used by both; `linux_security`'s own predicates delegate to them. `eos_shell_cmd_run()` reports a shell that could not be started, or a command the shell could not run at all (exit 127), as `EOS_ERR_SYSTEM` rather than as the `EOS_ERR_BUILD` a genuine build failure produces. `tests/test_shell_cmd.c` covers the rules, the builder, and every backend against a hostile directory.
 
 ### Changed

services/crypto/src/ed25519_fe.c

@@ -318,34 +318,34 @@ void fe_frombytes(fe h, const unsigned char *s) {
 
     carry9 = (h9 + (int64_t) (1 << 24)) >> 25;
     h0 += carry9 * 19;
-    h9 -= carry9 << 25;
+    h9 -= carry9 * ((int64_t) 1 << 25);
     carry1 = (h1 + (int64_t) (1 << 24)) >> 25;
     h2 += carry1;
-    h1 -= carry1 << 25;
+    h1 -= carry1 * ((int64_t) 1 << 25);
     carry3 = (h3 + (int64_t) (1 << 24)) >> 25;
     h4 += carry3;
-    h3 -= carry3 << 25;
+    h3 -= carry3 * ((int64_t) 1 << 25);
     carry5 = (h5 + (int64_t) (1 << 24)) >> 25;
     h6 += carry5;
-    h5 -= carry5 << 25;
+    h5 -= carry5 * ((int64_t) 1 << 25);
     carry7 = (h7 + (int64_t) (1 << 24)) >> 25;
     h8 += carry7;
-    h7 -= carry7 << 25;
+    h7 -= carry7 * ((int64_t) 1 << 25);
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
     carry2 = (h2 + (int64_t) (1 << 25)) >> 26;
     h3 += carry2;
-    h2 -= carry2 << 26;
+    h2 -= carry2 * ((int64_t) 1 << 26);
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
     carry6 = (h6 + (int64_t) (1 << 25)) >> 26;
     h7 += carry6;
-    h6 -= carry6 << 26;
+    h6 -= carry6 * ((int64_t) 1 << 26);
     carry8 = (h8 + (int64_t) (1 << 25)) >> 26;
     h9 += carry8;
-    h8 -= carry8 << 26;
+    h8 -= carry8 * ((int64_t) 1 << 26);
 
     h[0] = (int32_t) h0;
     h[1] = (int32_t) h1;
@@ -711,46 +711,46 @@ void fe_mul(fe h, const fe f, const fe g) {
 
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
 
     carry1 = (h1 + (int64_t) (1 << 24)) >> 25;
     h2 += carry1;
-    h1 -= carry1 << 25;
+    h1 -= carry1 * ((int64_t) 1 << 25);
     carry5 = (h5 + (int64_t) (1 << 24)) >> 25;
     h6 += carry5;
-    h5 -= carry5 << 25;
+    h5 -= carry5 * ((int64_t) 1 << 25);
 
     carry2 = (h2 + (int64_t) (1 << 25)) >> 26;
     h3 += carry2;
-    h2 -= carry2 << 26;
+    h2 -= carry2 * ((int64_t) 1 << 26);
     carry6 = (h6 + (int64_t) (1 << 25)) >> 26;
     h7 += carry6;
-    h6 -= carry6 << 26;
+    h6 -= carry6 * ((int64_t) 1 << 26);
 
     carry3 = (h3 + (int64_t) (1 << 24)) >> 25;
     h4 += carry3;
-    h3 -= carry3 << 25;
+    h3 -= carry3 * ((int64_t) 1 << 25);
     carry7 = (h7 + (int64_t) (1 << 24)) >> 25;
     h8 += carry7;
-    h7 -= carry7 << 25;
+    h7 -= carry7 * ((int64_t) 1 << 25);
 
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
     carry8 = (h8 + (int64_t) (1 << 25)) >> 26;
     h9 += carry8;
-    h8 -= carry8 << 26;
+    h8 -= carry8 * ((int64_t) 1 << 26);
 
     carry9 = (h9 + (int64_t) (1 << 24)) >> 25;
     h0 += carry9 * 19;
-    h9 -= carry9 << 25;
+    h9 -= carry9 * ((int64_t) 1 << 25);
 
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
 
     h[0] = (int32_t) h0;
     h[1] = (int32_t) h1;
@@ -808,17 +808,17 @@ void fe_mul121666(fe h, fe f) {
     int64_t carry8;
     int64_t carry9;
 
-    carry9 = (h9 + (int64_t) (1<<24)) >> 25; h0 += carry9 * 19; h9 -= carry9 << 25;
-    carry1 = (h1 + (int64_t) (1<<24)) >> 25; h2 += carry1; h1 -= carry1 << 25;
-    carry3 = (h3 + (int64_t) (1<<24)) >> 25; h4 += carry3; h3 -= carry3 << 25;
-    carry5 = (h5 + (int64_t) (1<<24)) >> 25; h6 += carry5; h5 -= carry5 << 25;
-    carry7 = (h7 + (int64_t) (1<<24)) >> 25; h8 += carry7; h7 -= carry7 << 25;
+    carry9 = (h9 + (int64_t) (1<<24)) >> 25; h0 += carry9 * 19; h9 -= carry9 * ((int64_t) 1 << 25);
+    carry1 = (h1 + (int64_t) (1<<24)) >> 25; h2 += carry1; h1 -= carry1 * ((int64_t) 1 << 25);
+    carry3 = (h3 + (int64_t) (1<<24)) >> 25; h4 += carry3; h3 -= carry3 * ((int64_t) 1 << 25);
+    carry5 = (h5 + (int64_t) (1<<24)) >> 25; h6 += carry5; h5 -= carry5 * ((int64_t) 1 << 25);
+    carry7 = (h7 + (int64_t) (1<<24)) >> 25; h8 += carry7; h7 -= carry7 * ((int64_t) 1 << 25);
 
-    carry0 = (h0 + (int64_t) (1<<25)) >> 26; h1 += carry0; h0 -= carry0 << 26;
-    carry2 = (h2 + (int64_t) (1<<25)) >> 26; h3 += carry2; h2 -= carry2 << 26;
-    carry4 = (h4 + (int64_t) (1<<25)) >> 26; h5 += carry4; h4 -= carry4 << 26;
-    carry6 = (h6 + (int64_t) (1<<25)) >> 26; h7 += carry6; h6 -= carry6 << 26;
-    carry8 = (h8 + (int64_t) (1<<25)) >> 26; h9 += carry8; h8 -= carry8 << 26;
+    carry0 = (h0 + (int64_t) (1<<25)) >> 26; h1 += carry0; h0 -= carry0 * ((int64_t) 1 << 26);
+    carry2 = (h2 + (int64_t) (1<<25)) >> 26; h3 += carry2; h2 -= carry2 * ((int64_t) 1 << 26);
+    carry4 = (h4 + (int64_t) (1<<25)) >> 26; h5 += carry4; h4 -= carry4 * ((int64_t) 1 << 26);
+    carry6 = (h6 + (int64_t) (1<<25)) >> 26; h7 += carry6; h6 -= carry6 * ((int64_t) 1 << 26);
+    carry8 = (h8 + (int64_t) (1<<25)) >> 26; h9 += carry8; h8 -= carry8 * ((int64_t) 1 << 26);
 
     h[0] = (int32_t) h0;
     h[1] = (int32_t) h1;
@@ -1080,40 +1080,40 @@ void fe_sq(fe h, const fe f) {
     int64_t carry9;
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
     carry1 = (h1 + (int64_t) (1 << 24)) >> 25;
     h2 += carry1;
-    h1 -= carry1 << 25;
+    h1 -= carry1 * ((int64_t) 1 << 25);
     carry5 = (h5 + (int64_t) (1 << 24)) >> 25;
     h6 += carry5;
-    h5 -= carry5 << 25;
+    h5 -= carry5 * ((int64_t) 1 << 25);
     carry2 = (h2 + (int64_t) (1 << 25)) >> 26;
     h3 += carry2;
-    h2 -= carry2 << 26;
+    h2 -= carry2 * ((int64_t) 1 << 26);
     carry6 = (h6 + (int64_t) (1 << 25)) >> 26;
     h7 += carry6;
-    h6 -= carry6 << 26;
+    h6 -= carry6 * ((int64_t) 1 << 26);
     carry3 = (h3 + (int64_t) (1 << 24)) >> 25;
     h4 += carry3;
-    h3 -= carry3 << 25;
+    h3 -= carry3 * ((int64_t) 1 << 25);
     carry7 = (h7 + (int64_t) (1 << 24)) >> 25;
     h8 += carry7;
-    h7 -= carry7 << 25;
+    h7 -= carry7 * ((int64_t) 1 << 25);
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
     carry8 = (h8 + (int64_t) (1 << 25)) >> 26;
     h9 += carry8;
-    h8 -= carry8 << 26;
+    h8 -= carry8 * ((int64_t) 1 << 26);
     carry9 = (h9 + (int64_t) (1 << 24)) >> 25;
     h0 += carry9 * 19;
-    h9 -= carry9 << 25;
+    h9 -= carry9 * ((int64_t) 1 << 25);
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
     h[0] = (int32_t) h0;
     h[1] = (int32_t) h1;
     h[2] = (int32_t) h2;
@@ -1253,40 +1253,40 @@ void fe_sq2(fe h, const fe f) {
     h9 += h9;
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
     carry1 = (h1 + (int64_t) (1 << 24)) >> 25;
     h2 += carry1;
-    h1 -= carry1 << 25;
+    h1 -= carry1 * ((int64_t) 1 << 25);
     carry5 = (h5 + (int64_t) (1 << 24)) >> 25;
     h6 += carry5;
-    h5 -= carry5 << 25;
+    h5 -= carry5 * ((int64_t) 1 << 25);
     carry2 = (h2 + (int64_t) (1 << 25)) >> 26;
     h3 += carry2;
-    h2 -= carry2 << 26;
+    h2 -= carry2 * ((int64_t) 1 << 26);
     carry6 = (h6 + (int64_t) (1 << 25)) >> 26;
     h7 += carry6;
-    h6 -= carry6 << 26;
+    h6 -= carry6 * ((int64_t) 1 << 26);
     carry3 = (h3 + (int64_t) (1 << 24)) >> 25;
     h4 += carry3;
-    h3 -= carry3 << 25;
+    h3 -= carry3 * ((int64_t) 1 << 25);
     carry7 = (h7 + (int64_t) (1 << 24)) >> 25;
     h8 += carry7;
-    h7 -= carry7 << 25;
+    h7 -= carry7 * ((int64_t) 1 << 25);
     carry4 = (h4 + (int64_t) (1 << 25)) >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int64_t) 1 << 26);
     carry8 = (h8 + (int64_t) (1 << 25)) >> 26;
     h9 += carry8;
-    h8 -= carry8 << 26;
+    h8 -= carry8 * ((int64_t) 1 << 26);
     carry9 = (h9 + (int64_t) (1 << 24)) >> 25;
     h0 += carry9 * 19;
-    h9 -= carry9 << 25;
+    h9 -= carry9 * ((int64_t) 1 << 25);
     carry0 = (h0 + (int64_t) (1 << 25)) >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int64_t) 1 << 26);
     h[0] = (int32_t) h0;
     h[1] = (int32_t) h1;
     h[2] = (int32_t) h2;
@@ -1421,33 +1421,33 @@ void fe_tobytes(unsigned char *s, const fe h) {
     /* Goal: Output h-2^255 q, which is between 0 and 2^255-20. */
     carry0 = h0 >> 26;
     h1 += carry0;
-    h0 -= carry0 << 26;
+    h0 -= carry0 * ((int32_t) 1 << 26);
     carry1 = h1 >> 25;
     h2 += carry1;
-    h1 -= carry1 << 25;
+    h1 -= carry1 * ((int32_t) 1 << 25);
     carry2 = h2 >> 26;
     h3 += carry2;
-    h2 -= carry2 << 26;
+    h2 -= carry2 * ((int32_t) 1 << 26);
     carry3 = h3 >> 25;
     h4 += carry3;
-    h3 -= carry3 << 25;
+    h3 -= carry3 * ((int32_t) 1 << 25);
     carry4 = h4 >> 26;
     h5 += carry4;
-    h4 -= carry4 << 26;
+    h4 -= carry4 * ((int32_t) 1 << 26);
     carry5 = h5 >> 25;
     h6 += carry5;
-    h5 -= carry5 << 25;
+    h5 -= carry5 * ((int32_t) 1 << 25);
     carry6 = h6 >> 26;
     h7 += carry6;
-    h6 -= carry6 << 26;
+    h6 -= carry6 * ((int32_t) 1 << 26);
     carry7 = h7 >> 25;
     h8 += carry7;
-    h7 -= carry7 << 25;
+    h7 -= carry7 * ((int32_t) 1 << 25);
     carry8 = h8 >> 26;
     h9 += carry8;
-    h8 -= carry8 << 26;
+    h8 -= carry8 * ((int32_t) 1 << 26);
     carry9 = h9 >> 25;
-    h9 -= carry9 << 25;
+    h9 -= carry9 * ((int32_t) 1 << 25);
 
     /* h10 = carry9 */
     /*
@@ -1459,32 +1459,32 @@ void fe_tobytes(unsigned char *s, const fe h) {
     s[0] = (unsigned char) (h0 >> 0);
     s[1] = (unsigned char) (h0 >> 8);
     s[2] = (unsigned char) (h0 >> 16);
-    s[3] = (unsigned char) ((h0 >> 24) | (h1 << 2));
+    s[3] = (unsigned char) ((h0 >> 24) | ((uint32_t) h1 << 2));
     s[4] = (unsigned char) (h1 >> 6);
     s[5] = (unsigned char) (h1 >> 14);
-    s[6] = (unsigned char) ((h1 >> 22) | (h2 << 3));
+    s[6] = (unsigned char) ((h1 >> 22) | ((uint32_t) h2 << 3));
     s[7] = (unsigned char) (h2 >> 5);
     s[8] = (unsigned char) (h2 >> 13);
-    s[9] = (unsigned char) ((h2 >> 21) | (h3 << 5));
+    s[9] = (unsigned char) ((h2 >> 21) | ((uint32_t) h3 << 5));
     s[10] = (unsigned char) (h3 >> 3);
     s[11] = (unsigned char) (h3 >> 11);
-    s[12] = (unsigned char) ((h3 >> 19) | (h4 << 6));
+    s[12] = (unsigned char) ((h3 >> 19) | ((uint32_t) h4 << 6));
     s[13] = (unsigned char) (h4 >> 2);
     s[14] = (unsigned char) (h4 >> 10);
     s[15] = (unsigned char) (h4 >> 18);
     s[16] = (unsigned char) (h5 >> 0);
     s[17] = (unsigned char) (h5 >> 8);
     s[18] = (unsigned char) (h5 >> 16);
-    s[19] = (unsigned char) ((h5 >> 24) | (h6 << 1));
+    s[19] = (unsigned char) ((h5 >> 24) | ((uint32_t) h6 << 1));
     s[20] = (unsigned char) (h6 >> 7);
     s[21] = (unsigned char) (h6 >> 15);
-    s[22] = (unsigned char) ((h6 >> 23) | (h7 << 3));
+    s[22] = (unsigned char) ((h6 >> 23) | ((uint32_t) h7 << 3));
     s[23] = (unsigned char) (h7 >> 5);
     s[24] = (unsigned char) (h7 >> 13);
-    s[25] = (unsigned char) ((h7 >> 21) | (h8 << 4));
+    s[25] = (unsigned char) ((h7 >> 21) | ((uint32_t) h8 << 4));
     s[26] = (unsigned char) (h8 >> 4);
     s[27] = (unsigned char) (h8 >> 12);
-    s[28] = (unsigned char) ((h8 >> 20) | (h9 << 6));
+    s[28] = (unsigned char) ((h8 >> 20) | ((uint32_t) h9 << 6));
     s[29] = (unsigned char) (h9 >> 2);
     s[30] = (unsigned char) (h9 >> 10);
     s[31] = (unsigned char) (h9 >> 18);

services/crypto/src/ed25519_ge.c

@@ -356,7 +356,7 @@ static void cmov(ge_precomp *t, const ge_precomp *u, unsigned char b) {
 static void select(ge_precomp *t, int pos, signed char b) {
     ge_precomp minust;
     unsigned char bnegative = negative(b);
-    unsigned char babs = b - (((-bnegative) & b) << 1);
+    unsigned char babs = b - (((-bnegative) & b) * 2);
     fe_1(t->yplusx);
     fe_1(t->yminusx);
     fe_0(t->xy2d);