From ce2260afab60c6ef1cda5c7571aef1f69019c7d9 Mon Sep 17 00:00:00 2001
From: Isis Lovecruft <isis@patternsinthevoid.net>
Date: Thu, 3 Oct 2019 23:42:16 +0000
Subject: [PATCH] Implement stricter scalar malleability checking for
 signatures.

Previously, we were checking that the highest 3 bits were unset, which still
leaves 2^253 - 2^252 + 27742317777372353535851937790883648493 potential scalars
for the `s` component of a signature which are not strictly mod \ell.

This change fixes that.

Note: This change makes ed25519-dalek incompatible with ed25519-donna in that
some signatures produced by donna will be verifiable by donna but NOT VERIFIABLE
by dalek.  On the other hand, libsodium exports a -DED25519_COMPAT feature,
which when enabled, means it is compatible with dalek with the
`legacy_compatibility` feature disabled.  Otherwise, libsodium's behaviour is
identical to the behaviour enabled by default in this patch.
---
 Cargo.toml       |  2 ++
 src/signature.rs | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 453d72e..28411f1 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -69,6 +69,8 @@ alloc = ["curve25519-dalek/alloc", "rand_os"]
 nightly = ["curve25519-dalek/nightly", "clear_on_drop/nightly"]
 batch = ["rand"]
 asm = ["sha2/asm"]
+# This features turns off stricter checking for scalar malleability in signatures
+legacy_compatibility = []
 yolocrypto = ["curve25519-dalek/yolocrypto"]
 u64_backend = ["curve25519-dalek/u64_backend"]
 u32_backend = ["curve25519-dalek/u32_backend"]
diff --git a/src/signature.rs b/src/signature.rs
index d5079fd..653155d 100644
--- a/src/signature.rs
+++ b/src/signature.rs
@@ -71,6 +71,31 @@ impl Debug for Signature {
     }
 }
 
+#[cfg(feature = "legacy_compatibility")]
+#[inline(always)]
+fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
+    // The highest 3 bits must not be set.  No other checking for the
+    // remaining 2^253 - 2^252 + 27742317777372353535851937790883648493
+    // potential non-reduced scalars is performed.
+    //
+    // This is compatible with ed25519-donna and libsodium when
+    // -DED25519_COMPAT is NOT specified.
+    if bytes[31] & 224 != 0 {
+        return Err(SignatureError(InternalError::ScalarFormatError));
+    }
+
+    Ok(Scalar::from_bits(bytes))
+}
+
+#[cfg(not(feature = "legacy_compatibility"))]
+#[inline(always)]
+fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
+    match Scalar::from_canonical_bytes(bytes) {
+        None => return Err(SignatureError(InternalError::ScalarFormatError)),
+        Some(x) => return Ok(x),
+    };
+}
+
 impl Signature {
     /// Convert this `Signature` to a byte array.
     #[inline]
@@ -97,13 +122,16 @@ impl Signature {
         lower.copy_from_slice(&bytes[..32]);
         upper.copy_from_slice(&bytes[32..]);
 
-        if upper[31] & 224 != 0 {
-            return Err(SignatureError(InternalError::ScalarFormatError));
+        let s: Scalar;
+
+        match check_scalar(upper) {
+            Ok(x)  => s = x,
+            Err(x) => return Err(x),
         }
 
         Ok(Signature {
             R: CompressedEdwardsY(lower),
-            s: Scalar::from_bits(upper),
+            s: s,
         })
     }
 }