Use libsodium's s < L check, instead checking that libsodium checks that.

src/crypto/common.h

@@ -10,9 +10,15 @@
 #endif
 
 #include <stdint.h>
+#include <assert.h>
 
+#include "sodium.h"
 #include "compat/endian.h"
 
+#if defined(NDEBUG)
+# error "Bitcoin cannot be compiled without assertions."
+#endif
+
 uint16_t static inline ReadLE16(const unsigned char* ptr)
 {
     return le16toh(*((uint16_t*)ptr));
@@ -63,4 +69,42 @@ void static inline WriteBE64(unsigned char* ptr, uint64_t x)
     *((uint64_t*)ptr) = htobe64(x);
 }
 
+int inline init_and_check_sodium()
+{
+    if (sodium_init() == -1) {
+        return -1;
+    }
+
+    // What follows is a runtime test that ensures the version of libsodium
+    // we're linked against checks that signatures are canonical (s < L).
+    const unsigned char message[1] = { 0 };
+
+    unsigned char pk[crypto_sign_PUBLICKEYBYTES];
+    unsigned char sk[crypto_sign_SECRETKEYBYTES];
+    unsigned char sig[crypto_sign_BYTES];
+
+    crypto_sign_keypair(pk, sk);
+    crypto_sign_detached(sig, NULL, message, sizeof(message), sk);
+
+    assert(crypto_sign_verify_detached(sig, message, sizeof(message), pk) == 0);
+
+    // Copied from libsodium/crypto_sign/ed25519/ref10/open.c
+    static const unsigned char L[32] =
+      { 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+        0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10 };
+
+    // Add L to S, which starts at sig[32].
+    unsigned int s = 0;
+    for (size_t i = 0; i < 32; i++) {
+        s = sig[32 + i] + L[i] + (s >> 8);
+        sig[32 + i] = s & 0xff;
+    }
+
+    assert(crypto_sign_verify_detached(sig, message, sizeof(message), pk) != 0);
+
+    return 0;
+}
+
 #endif // BITCOIN_CRYPTO_COMMON_H

src/gtest/main.cpp

@@ -1,8 +1,8 @@
 #include "gtest/gtest.h"
-#include "sodium.h"
+#include "crypto/common.h"
 
 int main(int argc, char **argv) {
-  assert(sodium_init() != -1);
+  assert(init_and_check_sodium() != -1);
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }

src/gtest/test_checktransaction.cpp

@@ -329,6 +329,13 @@ TEST(checktransaction_tests, bad_txns_invalid_joinsplit_signature) {
 TEST(checktransaction_tests, non_canonical_ed25519_signature) {
     CMutableTransaction mtx = GetValidTransaction();
 
+    // Check that the signature is valid before we add L
+    {
+        CTransaction tx(mtx);
+        MockCValidationState state;
+        EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state));
+    }
+
     // Copied from libsodium/crypto_sign/ed25519/ref10/open.c
     static const unsigned char L[32] =
       { 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
@@ -346,6 +353,6 @@ TEST(checktransaction_tests, non_canonical_ed25519_signature) {
     CTransaction tx(mtx);
 
     MockCValidationState state;
-    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "non-canonical-ed25519-signature", false)).Times(1);
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1);
     CheckTransactionWithoutProofVerification(tx, state);
 }

src/init.cpp

@@ -8,8 +8,7 @@
 #endif
 
 #include "init.h"
-#include "sodium.h"
-
+#include "crypto/common.h"
 #include "addrman.h"
 #include "amount.h"
 #include "checkpoints.h"
@@ -907,7 +906,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
     // ********************************************************* Step 4: application initialization: dir lock, daemonize, pidfile, debug log
 
     // Initialize libsodium
-    if (sodium_init() == -1) {
+    if (init_and_check_sodium() == -1) {
         return false;
     }
 

src/main.cpp

@@ -841,35 +841,6 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in
     return nSigOps;
 }
 
-
-
-
-// Taken from
-//   https://github.com/jedisct1/libsodium/commit/4099618de2cce5099ac2ec5ce8f2d80f4585606e
-// which was removed to maintain backwards compatibility in
-//   https://github.com/jedisct1/libsodium/commit/cb07df046f19ee0d5ad600c579df97aaa4295cc3
-static int
-crypto_sign_check_S_lt_l(const unsigned char *S)
-{
-    static const unsigned char l[32] =
-      { 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
-        0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10 };
-    unsigned char c = 0;
-    unsigned char n = 1;
-    unsigned int  i = 32;
-
-    do {
-        i--;
-        c |= ((S[i] - l[i]) >> 8) & n;
-        n &= ((S[i] ^ l[i]) - 1) >> 8;
-    } while (i != 0);
-
-    return -(c == 0);
-}
-
-
 bool CheckTransaction(const CTransaction& tx, CValidationState &state)
 {
     if (!CheckTransactionWithoutProofVerification(tx, state)) {
@@ -1011,6 +982,8 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
 
             BOOST_STATIC_ASSERT(crypto_sign_PUBLICKEYBYTES == 32);
 
+            // We rely on libsodium to check that the signature is canonical.
+            // https://github.com/jedisct1/libsodium/commit/62911edb7ff2275cccd74bf1c8aefcc4d76924e0
             if (crypto_sign_verify_detached(&tx.joinSplitSig[0],
                                             dataToBeSigned.begin(), 32,
                                             tx.joinSplitPubKey.begin()
@@ -1018,11 +991,6 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
                 return state.DoS(100, error("CheckTransaction(): invalid joinsplit signature"),
                                  REJECT_INVALID, "bad-txns-invalid-joinsplit-signature");
             }
-
-            if (crypto_sign_check_S_lt_l(&tx.joinSplitSig[32]) != 0) {
-                return state.DoS(100, error("CheckTransaction(): non-canonical ed25519 signature"),
-                                 REJECT_INVALID, "non-canonical-ed25519-signature");
-            }
         }
     }
 

src/test/test_bitcoin.cpp

@@ -6,7 +6,7 @@
 
 #include "test_bitcoin.h"
 
-#include "sodium.h"
+#include "crypto/common.h"
 
 #include "key.h"
 #include "main.h"
@@ -32,7 +32,7 @@ extern void noui_connect();
 
 BasicTestingSetup::BasicTestingSetup()
 {
-        assert(sodium_init() != -1);
+        assert(init_and_check_sodium() != -1);
         ECC_Start();
         SetupEnvironment();
         fPrintToDebugLog = false; // don't want to write to debug.log file

src/zcash/GenerateParams.cpp

@@ -1,11 +1,11 @@
 #include "zcash/JoinSplit.hpp"
 
 #include <iostream>
-#include "sodium.h"
+#include "crypto/common.h"
 
 int main(int argc, char **argv)
 {
-    if (sodium_init() == -1) {
+    if (init_and_check_sodium() == -1) {
         return 1;
     }