Merge pull request #5952

437ada3 Switch test case signing to RFC6979 extra entropy (Pieter Wuille)
9d09322 Squashed 'src/secp256k1/' changes from 50cc6ab..1897b8e (Pieter Wuille)

Author: laanwj

src/key.cpp

@@ -5,6 +5,7 @@
 #include "key.h"
 
 #include "arith_uint256.h"
+#include "crypto/common.h"
 #include "crypto/hmac_sha512.h"
 #include "eccryptoverify.h"
 #include "pubkey.h"
@@ -73,25 +74,14 @@ CPubKey CKey::GetPubKey() const {
     return result;
 }
 
-extern "C"
-{
-static int secp256k1_nonce_function_test_case(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, unsigned int attempt, const void *data)
-{
-    const uint32_t *test_case = static_cast<const uint32_t*>(data);
-    uint256 nonce;
-    secp256k1_nonce_function_rfc6979(nonce.begin(), msg32, key32, attempt, NULL);
-    nonce = ArithToUint256(UintToArith256(nonce) + *test_case);
-    memcpy(nonce32, nonce.begin(), 32);
-    return 1;
-}
-}
-
 bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, uint32_t test_case) const {
     if (!fValid)
         return false;
     vchSig.resize(72);
     int nSigLen = 72;
-    int ret = secp256k1_ecdsa_sign(hash.begin(), (unsigned char*)&vchSig[0], &nSigLen, begin(), test_case == 0 ? secp256k1_nonce_function_rfc6979 : secp256k1_nonce_function_test_case, test_case == 0 ? NULL : &test_case);
+    unsigned char extra_entropy[32] = {0};
+    WriteLE32(extra_entropy, test_case);
+    int ret = secp256k1_ecdsa_sign(hash.begin(), (unsigned char*)&vchSig[0], &nSigLen, begin(), secp256k1_nonce_function_rfc6979, test_case ? extra_entropy : NULL);
     assert(ret);
     vchSig.resize(nSigLen);
     return true;

src/key.h

@@ -124,8 +124,7 @@ class CKey
 
     /**
      * Create a DER-serialized signature.
-     * The test_case parameter tweaks the deterministic nonce, and is only for
-     * testing. It should be zero for normal use.
+     * The test_case parameter tweaks the deterministic nonce.
      */
     bool Sign(const uint256& hash, std::vector<unsigned char>& vchSig, uint32_t test_case = 0) const;
 

src/secp256k1/Makefile.am

@@ -49,7 +49,7 @@ libsecp256k1_la_LIBADD = $(SECP_LIBS)
 
 noinst_PROGRAMS =
 if USE_BENCHMARK
-noinst_PROGRAMS += bench_verify bench_recover bench_sign bench_inv
+noinst_PROGRAMS += bench_verify bench_recover bench_sign bench_internal
 bench_verify_SOURCES = src/bench_verify.c
 bench_verify_LDADD = libsecp256k1.la $(SECP_LIBS)
 bench_verify_LDFLAGS = -static
@@ -59,10 +59,10 @@ bench_recover_LDFLAGS = -static
 bench_sign_SOURCES = src/bench_sign.c
 bench_sign_LDADD = libsecp256k1.la $(SECP_LIBS)
 bench_sign_LDFLAGS = -static
-bench_inv_SOURCES = src/bench_inv.c
-bench_inv_LDADD = $(SECP_LIBS)
-bench_inv_LDFLAGS = -static
-bench_inv_CPPFLAGS = $(SECP_INCLUDES)
+bench_internal_SOURCES = src/bench_internal.c
+bench_internal_LDADD = $(SECP_LIBS)
+bench_internal_LDFLAGS = -static
+bench_internal_CPPFLAGS = $(SECP_INCLUDES)
 endif
 
 if USE_TESTS

src/secp256k1/README.md

@@ -5,25 +5,29 @@ libsecp256k1
 
 Optimized C library for EC operations on curve secp256k1.
 
-This library is experimental, so use at your own risk.
+This library is a work in progress and is being used to research best practices. Use at your own risk.
 
 Features:
-* Low-level field and group operations on secp256k1.
-* ECDSA signing/verification and key generation.
+* secp256k1 ECDSA signing/verification and key generation.
 * Adding/multiplying private/public keys.
 * Serialization/parsing of private keys, public keys, signatures.
+* Constant time, constant memory access signing and pubkey generation.
+* Derandomized DSA (via RFC6979 or with a caller provided function.)
 * Very efficient implementation.
 
 Implementation details
 ----------------------
 
 * General
-  * Avoid dynamic memory usage almost everywhere.
+  * No runtime heap allocation.
+  * Extensive testing infrastructure.
+  * Structured to facilitate review and analysis.
+  * Intended to be portable to any system with a C89 compiler and uint64_t support.
+  * Expose only higher level interfaces to minimize the API surface and improve application security. ("Be difficult to use insecurely.")
 * Field operations
   * Optimized implementation of arithmetic modulo the curve's field size (2^256 - 0x1000003D1).
     * Using 5 52-bit limbs (including hand-optimized assembly for x86_64, by Diederik Huys).
     * Using 10 26-bit limbs.
-    * Using GMP.
   * Field inverses and square roots using a sliding window over blocks of 1s (by Peter Dettman).
 * Scalar operations
   * Optimized implementation without data-dependent branches of arithmetic modulo the curve's order.
@@ -33,14 +37,15 @@ Implementation details
   * Point addition formula specifically simplified for the curve equation (y^2 = x^3 + 7).
   * Use addition between points in Jacobian and affine coordinates where possible.
   * Use a unified addition/doubling formula where necessary to avoid data-dependent branches.
+  * Point/x comparison without a field inversion by comparison in the Jacobian coordinate space.
 * Point multiplication for verification (a*P + b*G).
   * Use wNAF notation for point multiplicands.
   * Use a much larger window for multiples of G, using precomputed multiples.
   * Use Shamir's trick to do the multiplication with the public key and the generator simultaneously.
-  * Optionally use secp256k1's efficiently-computable endomorphism to split the multiplicands into 4 half-sized ones first.
+  * Optionally (off by default) use secp256k1's efficiently-computable endomorphism to split the P multiplicand into 2 half-sized ones.
 * Point multiplication for signing
   * Use a precomputed table of multiples of powers of 16 multiplied with the generator, so general multiplication becomes a series of additions.
-  * Slice the precomputed table in memory per byte, so memory access to the table becomes uniform.
+  * Access the table with branch-free conditional moves so memory access is uniform.
   * No data-dependent branches
   * The precomputed tables add and eventually subtract points for which no known scalar (private key) is known, preventing even an attacker with control over the private key used to control the data internally.
 
@@ -52,4 +57,5 @@ libsecp256k1 is built using autotools:
     $ ./autogen.sh
     $ ./configure
     $ make
+    $ ./tests
     $ sudo make install  # optional

src/secp256k1/configure.ac

@@ -5,7 +5,7 @@ AC_CONFIG_MACRO_DIR([build-aux/m4])
 AC_CANONICAL_HOST
 AH_TOP([#ifndef LIBSECP256K1_CONFIG_H])
 AH_TOP([#define LIBSECP256K1_CONFIG_H])
-AH_BOTTOM([#endif //LIBSECP256K1_CONFIG_H])
+AH_BOTTOM([#endif /*LIBSECP256K1_CONFIG_H*/])
 AM_INIT_AUTOMAKE([foreign subdir-objects])
 LT_INIT
 
@@ -22,9 +22,9 @@ if test "x$CFLAGS" = "x"; then
   CFLAGS="-O3 -g"
 fi
 
-AC_PROG_CC_C99
-if test x"$ac_cv_prog_cc_c99" = x"no"; then
-  AC_MSG_ERROR([c99 compiler support required])
+AC_PROG_CC_C89
+if test x"$ac_cv_prog_cc_c89" = x"no"; then
+  AC_MSG_ERROR([c89 compiler support required])
 fi
 
 case $host in
@@ -70,7 +70,7 @@ esac
 
 CFLAGS="$CFLAGS -W"
 
-warn_CFLAGS="-Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function"
+warn_CFLAGS="-std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings"
 saved_CFLAGS="$CFLAGS"
 CFLAGS="$CFLAGS $warn_CFLAGS"
 AC_MSG_CHECKING([if ${CC} supports ${warn_CFLAGS}])
@@ -305,6 +305,8 @@ if test x"$use_endomorphism" = x"yes"; then
   AC_DEFINE(USE_ENDOMORPHISM, 1, [Define this symbol to use endomorphism optimization])
 fi
 
+AC_C_BIGENDIAN()
+
 AC_MSG_NOTICE([Using assembly optimizations: $set_asm])
 AC_MSG_NOTICE([Using field implementation: $set_field])
 AC_MSG_NOTICE([Using bignum implementation: $set_bignum])

src/secp256k1/include/secp256k1.h

@@ -78,7 +78,7 @@ SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify(
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(4);
 
 /** A pointer to a function to deterministically generate a nonce.
- * Returns: 1 if a nonce was succesfully generated. 0 will cause signing to fail.
+ * Returns: 1 if a nonce was successfully generated. 0 will cause signing to fail.
  * In:      msg32:     the 32-byte message hash being verified (will not be NULL)
  *          key32:     pointer to a 32-byte secret key (will not be NULL)
  *          attempt:   how many iterations we have tried to find a nonce.
@@ -97,7 +97,10 @@ typedef int (*secp256k1_nonce_function_t)(
   const void *data
 );
 
-/** An implementation of RFC6979 (using HMAC-SHA256) as nonce generation function. */
+/** An implementation of RFC6979 (using HMAC-SHA256) as nonce generation function.
+ * If a data pointer is passed, it is assumed to be a pointer to 32 bytes of
+ * extra entropy.
+ */
 extern const secp256k1_nonce_function_t secp256k1_nonce_function_rfc6979;
 
 /** A default safe nonce generation function (currently equal to secp256k1_nonce_function_rfc6979). */
@@ -106,15 +109,43 @@ extern const secp256k1_nonce_function_t secp256k1_nonce_function_default;
 
 /** Create an ECDSA signature.
  *  Returns: 1: signature created
- *           0: the nonce generation function failed
+ *           0: the nonce generation function failed, the private key was invalid, or there is not
+ *              enough space in the signature (as indicated by siglen).
  *  In:      msg32:  the 32-byte message hash being signed (cannot be NULL)
- *           seckey: pointer to a 32-byte secret key (cannot be NULL, assumed to be valid)
+ *           seckey: pointer to a 32-byte secret key (cannot be NULL)
  *           noncefp:pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used
  *           ndata:  pointer to arbitrary data used by the nonce generation function (can be NULL)
  *  Out:     sig:    pointer to an array where the signature will be placed (cannot be NULL)
  *  In/Out:  siglen: pointer to an int with the length of sig, which will be updated
- *                   to contain the actual signature length (<=72).
+ *                   to contain the actual signature length (<=72). If 0 is returned, this will be
+ *                   set to zero.
  * Requires starting using SECP256K1_START_SIGN.
+ *
+ * The sig always has an s value in the lower half of the range (From 0x1
+ * to 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0,
+ * inclusive), unlike many other implementations.
+ * With ECDSA a third-party can can forge a second distinct signature
+ * of the same message given a single initial signature without knowing
+ * the key by setting s to its additive inverse mod-order, 'flipping' the
+ * sign of the random point R which is not included in the signature.
+ * Since the forgery is of the same message this isn't universally
+ * problematic, but in systems where message malleability or uniqueness
+ * of signatures is important this can cause issues.  This forgery can be
+ * blocked by all verifiers forcing signers to use a canonical form. The
+ * lower-S form reduces the size of signatures slightly on average when
+ * variable length encodings (such as DER) are used and is cheap to
+ * verify, making it a good choice. Security of always using lower-S is
+ * assured because anyone can trivially modify a signature after the
+ * fact to enforce this property.  Adjusting it inside the signing
+ * function avoids the need to re-serialize or have curve specific
+ * constants outside of the library.  By always using a canonical form
+ * even in applications where it isn't needed it becomes possible to
+ * impose a requirement later if a need is discovered.
+ * No other forms of ECDSA malleability are known and none seem likely,
+ * but there is no formal proof that ECDSA, even with this additional
+ * restriction, is free of other malleability.  Commonly used serialization
+ * schemes will also accept various non-unique encodings, so care should
+ * be taken when this property is required for an application.
  */
 int secp256k1_ecdsa_sign(
   const unsigned char *msg32,
@@ -127,12 +158,13 @@ int secp256k1_ecdsa_sign(
 
 /** Create a compact ECDSA signature (64 byte + recovery id).
  *  Returns: 1: signature created
- *           0: the nonce generation function failed
+ *           0: the nonce generation function failed, or the secret key was invalid.
  *  In:      msg32:  the 32-byte message hash being signed (cannot be NULL)
- *           seckey: pointer to a 32-byte secret key (cannot be NULL, assumed to be valid)
+ *           seckey: pointer to a 32-byte secret key (cannot be NULL)
  *           noncefp:pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used
  *           ndata:  pointer to arbitrary data used by the nonce generation function (can be NULL)
  *  Out:     sig:    pointer to a 64-byte array where the signature will be placed (cannot be NULL)
+ *                   In case 0 is returned, the returned signature length will be zero.
  *           recid:  pointer to an int, which will be updated to contain the recovery id (can be NULL)
  * Requires starting using SECP256K1_START_SIGN.
  */

src/secp256k1/src/bench.h

@@ -17,21 +17,40 @@ static double gettimedouble(void) {
     return tv.tv_usec * 0.000001 + tv.tv_sec;
 }
 
-void run_benchmark(void (*benchmark)(void*), void (*setup)(void*), void (*teardown)(void*), void* data, int count, int iter) {
+void print_number(double x) {
+    double y = x;
+    int c = 0;
+    if (y < 0.0) y = -y;
+    while (y < 100.0) {
+        y *= 10.0;
+        c++;
+    }
+    printf("%.*f", c, x);
+}
+
+void run_benchmark(char *name, void (*benchmark)(void*), void (*setup)(void*), void (*teardown)(void*), void* data, int count, int iter) {
+    int i;
     double min = HUGE_VAL;
     double sum = 0.0;
     double max = 0.0;
-    for (int i = 0; i < count; i++) {
+    for (i = 0; i < count; i++) {
+        double begin, total;
         if (setup) setup(data);
-        double begin = gettimedouble();
+        begin = gettimedouble();
         benchmark(data);
-        double total = gettimedouble() - begin;
+        total = gettimedouble() - begin;
         if (teardown) teardown(data);
         if (total < min) min = total;
         if (total > max) max = total;
         sum += total;
     }
-    printf("min %.3fus / avg %.3fus / max %.3fus\n", min * 1000000.0 / iter, (sum / count) * 1000000.0 / iter, max * 1000000.0 / iter);
+    printf("%s: min ", name);
+    print_number(min * 1000000.0 / iter);
+    printf("us / avg ");
+    print_number((sum / count) * 1000000.0 / iter);
+    printf("us / avg ");
+    print_number(max * 1000000.0 / iter);
+    printf("us\n");
 }
 
 #endif