diff --git a/src/crypto/common.h b/src/crypto/common.h index 580c72f5a..5d5027ada 100644 --- a/src/crypto/common.h +++ b/src/crypto/common.h @@ -10,9 +10,15 @@ #endif #include +#include +#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 diff --git a/src/gtest/main.cpp b/src/gtest/main.cpp index 456d59e1f..0fdc2b775 100644 --- a/src/gtest/main.cpp +++ b/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(); } diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index be45eb97d..c48e5ab30 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/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); } diff --git a/src/init.cpp b/src/init.cpp index 6b3d9a1df..43aaae288 100644 --- a/src/init.cpp +++ b/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; } diff --git a/src/main.cpp b/src/main.cpp index b5d4f3d94..742550613 100644 --- a/src/main.cpp +++ b/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"); - } } } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index eb5ba6b92..aed9514fb 100644 --- a/src/test/test_bitcoin.cpp +++ b/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 diff --git a/src/zcash/GenerateParams.cpp b/src/zcash/GenerateParams.cpp index 8433345bb..5ac3b8e6f 100644 --- a/src/zcash/GenerateParams.cpp +++ b/src/zcash/GenerateParams.cpp @@ -1,11 +1,11 @@ #include "zcash/JoinSplit.hpp" #include -#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; }