Improved readability of EdDSA verification

Basically, we separated the computation of R_check from the verification
that it is equal to R. The computation of R_check takes s, h_ram and the
public key as parameter, and output R_check.

The primary advantage is a better separation of concerns, which makes
the code more readable in my opinion.

A secondary advantage is that we could now test ge_r_check() separately,
with arbitrary values of s and h_ram.  This lets us test difficult to
trigger edge cases, like having s or h_ram exceeding 2^252, and is just
plain more general than only testing valid and invalid signatures.

I very much like this secondary advantage, because EdDSA is to this day
the part of Monocypher that makes me the most nervous.

diff --git a/src/monocypher.c b/src/monocypher.c
index e48f02bded3ef4e3ee92f5b368038c63733ee982..3e861ca0b65d10bb18ba471f3605dfdeafb5ade4 100644 (file)
--- a/src/monocypher.c
+++ b/src/monocypher.c
@@ -1817,6 +1817,23 @@ static void ge_double_scalarmult_vartime(ge *P, const u8 p[32], const u8 b[32])
     }
 }
 
+// R_check = s[B] - [h_ram](-pk), where B is the base point
+//
+// Variable time! Internal buffers are not wiped! Inputs must not be secret!
+// => Use only to *check* signatures.
+static int ge_r_check(u8 R_check[32], u8 s[32], u8 h_ram[32], u8 pk[32])
+{
+    ge A;
+    if (ge_frombytes_neg_vartime(&A, pk) || // A = -pk
+        is_above_L(s)) { // prevent s malleability
+        return -1;
+    }
+    ge_double_scalarmult_vartime(&A, h_ram, s); // A = [s]B + [h_ram]A
+    ge_tobytes(R_check, &A);                    // R_check = A
+    return 0;
+    // No secret, no wipe
+}
+
 // 5-bit signed comb in cached format (Niels coordinates, Z=1)
 static const ge_precomp b_comb[16] = {
     {{2615675,9989699,17617367,-13953520,-8802803,
@@ -2115,28 +2132,16 @@ void crypto_check_update(crypto_check_ctx_abstract *ctx,
 
 int crypto_check_final(crypto_check_ctx_abstract *ctx)
 {
-    ge  A;
-    u8 *h_ram   = ctx->pk; // save stack space
-    u8 *R_check = ctx->pk; // save stack space
-    u8 *R       = ctx->buf;                      // R
-    u8 *s       = ctx->buf + 32;                 // s
-    ge *diff    = &A;                            // -A is overwritten...
-    if (ge_frombytes_neg_vartime(&A, ctx->pk) ||
-        is_above_L(s)) { // prevent s malleability
+    u8 h_ram[64];
+    ctx->hash->final(ctx, h_ram);
+    reduce(h_ram);
+    u8 *R       = ctx->buf;      // R
+    u8 *s       = ctx->buf + 32; // s
+    u8 *R_check = ctx->pk;       // overwrite ctx->pk to save stack space
+    if (ge_r_check(R_check, s, h_ram, ctx->pk)) {
         return -1;
     }
-    {
-        u8 tmp[64];
-        ctx->hash->final(ctx, tmp);
-        reduce(tmp);
-        FOR (i, 0, 32) { // the extra copy saves 32 bytes of stack
-            h_ram[i] = tmp[i];
-        }
-    }
-    ge_double_scalarmult_vartime(&A, h_ram, s);  // ...here
-    ge_tobytes(R_check, diff);                   // R_check = s*B - h_ram*A
-    return crypto_verify32(R, R_check);          // R == R_check ? OK : fail
-    // No secret, no wipe
+    return crypto_verify32(R, R_check); // R == R_check ? OK : fail
 }
 
 int crypto_check(const u8  signature[64],