Merge bitcoin-core/secp256k1#983: [RFC] Remove OpenSSL testing support

bc08599 Remove OpenSSL testing support (Pieter Wuille) Pull request description: This removes the ability to test against OpenSSL, as well as the OpenSSL verification benchmark. The motivation is that OpenSSL 3 is deprecating part of the API used here (see bitcoin#869), and I'm not sure it's worth maintaining. We do lose the fact that this is the only test that verifies randomly-generated cases against an independent implementation. On the other hand, there are tons of existing fixed tests now that test all kinds of edge cases already. ACKs for top commit: elichai: tACK bc08599 real-or-random: ACK bc08599 jonasnick: ACK bc08599 Tree-SHA512: 632e6d3cf7bbc5828f5ca1f0f2a92c80bcb681bbcd4320c352b4a86fd521e410c852ccebcfc30fadc8fbf86649267a9e521f53e0f78072a8cd74d8726da28973
sipa · Oct 16, 2021 · f34b5ca · f34b5ca
2 parents 297ce82 + bc08599
commit f34b5ca
Show file tree

Hide file tree

Showing 6 changed files with 0 additions and 275 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -285,7 +285,6 @@ task:
     EXPERIMENTAL: yes
     SCHNORRSIG: yes
     CTIMETEST: no
-    EXTRAFLAGS: "--disable-openssl-tests"
   matrix:
     - name: "Valgrind (memcheck)"
       env:

diff --git a/Makefile.am b/Makefile.am
@@ -84,8 +84,6 @@ if USE_BENCHMARK
 noinst_PROGRAMS += bench_verify bench_sign bench_internal bench_ecmult
 bench_verify_SOURCES = src/bench_verify.c
 bench_verify_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
-# SECP_TEST_INCLUDES are only used here for CRYPTO_CPPFLAGS
-bench_verify_CPPFLAGS = $(SECP_TEST_INCLUDES)
 bench_sign_SOURCES = src/bench_sign.c
 bench_sign_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
 bench_internal_SOURCES = src/bench_internal.c

diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4
@@ -9,72 +9,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 AC_MSG_RESULT([$has_64bit_asm])
 ])
 
-dnl
-AC_DEFUN([SECP_OPENSSL_CHECK],[
-  has_libcrypto=no
-  m4_ifdef([PKG_CHECK_MODULES],[
-    PKG_CHECK_MODULES([CRYPTO], [libcrypto], [has_libcrypto=yes],[has_libcrypto=no])
-    if test x"$has_libcrypto" = x"yes"; then
-      TEMP_LIBS="$LIBS"
-      LIBS="$LIBS $CRYPTO_LIBS"
-      AC_CHECK_LIB(crypto, main,[AC_DEFINE(HAVE_LIBCRYPTO,1,[Define this symbol if libcrypto is installed])],[has_libcrypto=no])
-      LIBS="$TEMP_LIBS"
-    fi
-  ])
-  if test x$has_libcrypto = xno; then
-    AC_CHECK_HEADER(openssl/crypto.h,[
-      AC_CHECK_LIB(crypto, main,[
-        has_libcrypto=yes
-        CRYPTO_LIBS=-lcrypto
-        AC_DEFINE(HAVE_LIBCRYPTO,1,[Define this symbol if libcrypto is installed])
-      ])
-    ])
-    LIBS=
-  fi
-if test x"$has_libcrypto" = x"yes" && test x"$has_openssl_ec" = x; then
-  AC_MSG_CHECKING(for EC functions in libcrypto)
-  CPPFLAGS_TEMP="$CPPFLAGS"
-  CPPFLAGS="$CRYPTO_CPPFLAGS $CPPFLAGS"
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
-    #include <openssl/bn.h>
-    #include <openssl/ec.h>
-    #include <openssl/ecdsa.h>
-    #include <openssl/obj_mac.h>]],[[
-    # if OPENSSL_VERSION_NUMBER < 0x10100000L
-    void ECDSA_SIG_get0(const ECDSA_SIG *sig, const BIGNUM **pr, const BIGNUM **ps) {(void)sig->r; (void)sig->s;}
-    # endif
-
-    unsigned int zero = 0;
-    const unsigned char *zero_ptr = (unsigned char*)&zero;
-    EC_KEY_free(EC_KEY_new_by_curve_name(NID_secp256k1));
-    EC_KEY *eckey = EC_KEY_new();
-    EC_GROUP *group = EC_GROUP_new_by_curve_name(NID_secp256k1);
-    EC_KEY_set_group(eckey, group);
-    ECDSA_sign(0, NULL, 0, NULL, &zero, eckey);
-    ECDSA_verify(0, NULL, 0, NULL, 0, eckey);
-    o2i_ECPublicKey(&eckey, &zero_ptr, 0);
-    d2i_ECPrivateKey(&eckey, &zero_ptr, 0);
-    EC_KEY_check_key(eckey);
-    EC_KEY_free(eckey);
-    EC_GROUP_free(group);
-    ECDSA_SIG *sig_openssl;
-    sig_openssl = ECDSA_SIG_new();
-    d2i_ECDSA_SIG(&sig_openssl, &zero_ptr, 0);
-    i2d_ECDSA_SIG(sig_openssl, NULL);
-    ECDSA_SIG_get0(sig_openssl, NULL, NULL);
-    ECDSA_SIG_free(sig_openssl);
-    const BIGNUM *bignum = BN_value_one();
-    BN_is_negative(bignum);
-    BN_num_bits(bignum);
-    if (sizeof(zero) >= BN_num_bytes(bignum)) {
-        BN_bn2bin(bignum, (unsigned char*)&zero);
-    }
-  ]])],[has_openssl_ec=yes],[has_openssl_ec=no])
-  AC_MSG_RESULT([$has_openssl_ec])
-  CPPFLAGS="$CPPFLAGS_TEMP"
-fi
-])
-
 AC_DEFUN([SECP_VALGRIND_CHECK],[
 if test x"$has_valgrind" != x"yes"; then
   CPPFLAGS_TEMP="$CPPFLAGS"

diff --git a/configure.ac b/configure.ac
@@ -43,13 +43,7 @@ case $host_os in
          # These Homebrew packages may be keg-only, meaning that they won't be found
          # in expected paths because they may conflict with system files. Ask
          # Homebrew where each one is located, then adjust paths accordingly.
-         openssl_prefix=`$BREW --prefix openssl 2>/dev/null`
          valgrind_prefix=`$BREW --prefix valgrind 2>/dev/null`
-         if test x$openssl_prefix != x; then
-           PKG_CONFIG_PATH="$openssl_prefix/lib/pkgconfig:$PKG_CONFIG_PATH"
-           export PKG_CONFIG_PATH
-           CRYPTO_CPPFLAGS="-I$openssl_prefix/include"
-         fi
          if test x$valgrind_prefix != x; then
            VALGRIND_CPPFLAGS="-I$valgrind_prefix/include"
          fi
@@ -121,11 +115,6 @@ AC_ARG_ENABLE(tests,
     [use_tests=$enableval],
     [use_tests=yes])
 
-AC_ARG_ENABLE(openssl_tests,
-    AS_HELP_STRING([--enable-openssl-tests],[enable OpenSSL tests [default=auto]]),
-    [enable_openssl_tests=$enableval],
-    [enable_openssl_tests=auto])
-
 AC_ARG_ENABLE(experimental,
     AS_HELP_STRING([--enable-experimental],[allow experimental configure options [default=no]]),
     [use_experimental=$enableval],
@@ -329,32 +318,6 @@ case $set_ecmult_gen_precision in
   ;;
 esac
 
-if test x"$use_tests" = x"yes"; then
-  SECP_OPENSSL_CHECK
-  if test x"$enable_openssl_tests" != x"no" && test x"$has_openssl_ec" = x"yes"; then
-      enable_openssl_tests=yes
-      AC_DEFINE(ENABLE_OPENSSL_TESTS, 1, [Define this symbol if OpenSSL EC functions are available])
-      SECP_TEST_INCLUDES="$SSL_CFLAGS $CRYPTO_CFLAGS $CRYPTO_CPPFLAGS"
-      SECP_TEST_LIBS="$CRYPTO_LIBS"
-
-      case $host in
-      *mingw*)
-        SECP_TEST_LIBS="$SECP_TEST_LIBS -lgdi32"
-        ;;
-      esac
-  else
-    if test x"$enable_openssl_tests" = x"yes"; then
-      AC_MSG_ERROR([OpenSSL tests requested but OpenSSL with EC support is not available])
-    fi
-    enable_openssl_tests=no
-  fi
-else
-  if test x"$enable_openssl_tests" = x"yes"; then
-    AC_MSG_ERROR([OpenSSL tests requested but tests are not enabled])
-  fi
-  enable_openssl_tests=no
-fi
-
 if test x"$enable_valgrind" = x"yes"; then
   SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
 fi
@@ -519,7 +482,6 @@ echo "  with ecmult precomp     = $set_precomp"
 echo "  with external callbacks = $use_external_default_callbacks"
 echo "  with benchmarks         = $use_benchmark"
 echo "  with tests              = $use_tests"
-echo "  with openssl tests      = $enable_openssl_tests"
 echo "  with coverage           = $enable_coverage"
 echo "  module ecdh             = $enable_module_ecdh"
 echo "  module recovery         = $enable_module_recovery"

diff --git a/src/bench_verify.c b/src/bench_verify.c
@@ -11,13 +11,6 @@
 #include "util.h"
 #include "bench.h"
 
-#ifdef ENABLE_OPENSSL_TESTS
-#include <openssl/bn.h>
-#include <openssl/ecdsa.h>
-#include <openssl/obj_mac.h>
-#endif
-
-
 typedef struct {
     secp256k1_context *ctx;
     unsigned char msg[32];
@@ -26,9 +19,6 @@ typedef struct {
     size_t siglen;
     unsigned char pubkey[33];
     size_t pubkeylen;
-#ifdef ENABLE_OPENSSL_TESTS
-    EC_GROUP* ec_group;
-#endif
 } bench_verify_data;
 
 static void bench_verify(void* arg, int iters) {
@@ -50,36 +40,6 @@ static void bench_verify(void* arg, int iters) {
     }
 }
 
-#ifdef ENABLE_OPENSSL_TESTS
-static void bench_verify_openssl(void* arg, int iters) {
-    int i;
-    bench_verify_data* data = (bench_verify_data*)arg;
-
-    for (i = 0; i < iters; i++) {
-        data->sig[data->siglen - 1] ^= (i & 0xFF);
-        data->sig[data->siglen - 2] ^= ((i >> 8) & 0xFF);
-        data->sig[data->siglen - 3] ^= ((i >> 16) & 0xFF);
-        {
-            EC_KEY *pkey = EC_KEY_new();
-            const unsigned char *pubkey = &data->pubkey[0];
-            int result;
-
-            CHECK(pkey != NULL);
-            result = EC_KEY_set_group(pkey, data->ec_group);
-            CHECK(result);
-            result = (o2i_ECPublicKey(&pkey, &pubkey, data->pubkeylen)) != NULL;
-            CHECK(result);
-            result = ECDSA_verify(0, &data->msg[0], sizeof(data->msg), &data->sig[0], data->siglen, pkey) == (i == 0);
-            CHECK(result);
-            EC_KEY_free(pkey);
-        }
-        data->sig[data->siglen - 1] ^= (i & 0xFF);
-        data->sig[data->siglen - 2] ^= ((i >> 8) & 0xFF);
-        data->sig[data->siglen - 3] ^= ((i >> 16) & 0xFF);
-    }
-}
-#endif
-
 int main(void) {
     int i;
     secp256k1_pubkey pubkey;
@@ -104,11 +64,6 @@ int main(void) {
     CHECK(secp256k1_ec_pubkey_serialize(data.ctx, data.pubkey, &data.pubkeylen, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
 
     run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters);
-#ifdef ENABLE_OPENSSL_TESTS
-    data.ec_group = EC_GROUP_new_by_curve_name(NID_secp256k1);
-    run_benchmark("ecdsa_verify_openssl", bench_verify_openssl, NULL, NULL, &data, 10, iters);
-    EC_GROUP_free(data.ec_group);
-#endif
 
     secp256k1_context_destroy(data.ctx);
     return 0;

diff --git a/src/tests.c b/src/tests.c
@@ -20,16 +20,6 @@
 #include "testrand_impl.h"
 #include "util.h"
 
-#ifdef ENABLE_OPENSSL_TESTS
-#include <openssl/bn.h>
-#include <openssl/ec.h>
-#include <openssl/ecdsa.h>
-#include <openssl/obj_mac.h>
-# if OPENSSL_VERSION_NUMBER < 0x10100000L
-void ECDSA_SIG_get0(const ECDSA_SIG *sig, const BIGNUM **pr, const BIGNUM **ps) {*pr = sig->r; *ps = sig->s;}
-# endif
-#endif
-
 #include "../contrib/lax_der_parsing.c"
 #include "../contrib/lax_der_privatekey_parsing.c"
 
@@ -5685,14 +5675,6 @@ void run_ecdsa_end_to_end(void) {
 
 int test_ecdsa_der_parse(const unsigned char *sig, size_t siglen, int certainly_der, int certainly_not_der) {
     static const unsigned char zeroes[32] = {0};
-#ifdef ENABLE_OPENSSL_TESTS
-    static const unsigned char max_scalar[32] = {
-        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe,
-        0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b,
-        0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x40
-    };
-#endif
 
     int ret = 0;
 
@@ -5708,15 +5690,6 @@ int test_ecdsa_der_parse(const unsigned char *sig, size_t siglen, int certainly_
     size_t len_der_lax = 2048;
     int parsed_der_lax = 0, valid_der_lax = 0, roundtrips_der_lax = 0;
 
-#ifdef ENABLE_OPENSSL_TESTS
-    ECDSA_SIG *sig_openssl;
-    const BIGNUM *r = NULL, *s = NULL;
-    const unsigned char *sigptr;
-    unsigned char roundtrip_openssl[2048];
-    int len_openssl = 2048;
-    int parsed_openssl, valid_openssl = 0, roundtrips_openssl = 0;
-#endif
-
     parsed_der = secp256k1_ecdsa_signature_parse_der(ctx, &sig_der, sig, siglen);
     if (parsed_der) {
         ret |= (!secp256k1_ecdsa_signature_serialize_compact(ctx, compact_der, &sig_der)) << 0;
@@ -5757,43 +5730,6 @@ int test_ecdsa_der_parse(const unsigned char *sig, size_t siglen, int certainly_
         ret |= (!parsed_der_lax) << 16;
     }
 
-#ifdef ENABLE_OPENSSL_TESTS
-    sig_openssl = ECDSA_SIG_new();
-    sigptr = sig;
-    parsed_openssl = (d2i_ECDSA_SIG(&sig_openssl, &sigptr, siglen) != NULL);
-    if (parsed_openssl) {
-        ECDSA_SIG_get0(sig_openssl, &r, &s);
-        valid_openssl = !BN_is_negative(r) && !BN_is_negative(s) && BN_num_bits(r) > 0 && BN_num_bits(r) <= 256 && BN_num_bits(s) > 0 && BN_num_bits(s) <= 256;
-        if (valid_openssl) {
-            unsigned char tmp[32] = {0};
-            BN_bn2bin(r, tmp + 32 - BN_num_bytes(r));
-            valid_openssl = secp256k1_memcmp_var(tmp, max_scalar, 32) < 0;
-        }
-        if (valid_openssl) {
-            unsigned char tmp[32] = {0};
-            BN_bn2bin(s, tmp + 32 - BN_num_bytes(s));
-            valid_openssl = secp256k1_memcmp_var(tmp, max_scalar, 32) < 0;
-        }
-    }
-    len_openssl = i2d_ECDSA_SIG(sig_openssl, NULL);
-    if (len_openssl <= 2048) {
-        unsigned char *ptr = roundtrip_openssl;
-        CHECK(i2d_ECDSA_SIG(sig_openssl, &ptr) == len_openssl);
-        roundtrips_openssl = valid_openssl && ((size_t)len_openssl == siglen) && (secp256k1_memcmp_var(roundtrip_openssl, sig, siglen) == 0);
-    } else {
-        len_openssl = 0;
-    }
-    ECDSA_SIG_free(sig_openssl);
-
-    ret |= (parsed_der && !parsed_openssl) << 4;
-    ret |= (valid_der && !valid_openssl) << 5;
-    ret |= (roundtrips_openssl && !parsed_der) << 6;
-    ret |= (roundtrips_der != roundtrips_openssl) << 7;
-    if (roundtrips_openssl) {
-        ret |= (len_der != (size_t)len_openssl) << 8;
-        ret |= ((len_der != (size_t)len_openssl) || (secp256k1_memcmp_var(roundtrip_der, roundtrip_openssl, len_der) != 0)) << 9;
-    }
-#endif
     return ret;
 }
 
@@ -6387,62 +6323,6 @@ void run_ecdsa_edge_cases(void) {
     test_ecdsa_edge_cases();
 }
 
-#ifdef ENABLE_OPENSSL_TESTS
-EC_KEY *get_openssl_key(const unsigned char *key32) {
-    unsigned char privkey[300];
-    size_t privkeylen;
-    const unsigned char* pbegin = privkey;
-    int compr = secp256k1_testrand_bits(1);
-    EC_KEY *ec_key = EC_KEY_new_by_curve_name(NID_secp256k1);
-    CHECK(ec_privkey_export_der(ctx, privkey, &privkeylen, key32, compr));
-    CHECK(d2i_ECPrivateKey(&ec_key, &pbegin, privkeylen));
-    CHECK(EC_KEY_check_key(ec_key));
-    return ec_key;
-}
-
-void test_ecdsa_openssl(void) {
-    secp256k1_gej qj;
-    secp256k1_ge q;
-    secp256k1_scalar sigr, sigs;
-    secp256k1_scalar one;
-    secp256k1_scalar msg2;
-    secp256k1_scalar key, msg;
-    EC_KEY *ec_key;
-    unsigned int sigsize = 80;
-    size_t secp_sigsize = 80;
-    unsigned char message[32];
-    unsigned char signature[80];
-    unsigned char key32[32];
-    secp256k1_testrand256_test(message);
-    secp256k1_scalar_set_b32(&msg, message, NULL);
-    random_scalar_order_test(&key);
-    secp256k1_scalar_get_b32(key32, &key);
-    secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &qj, &key);
-    secp256k1_ge_set_gej(&q, &qj);
-    ec_key = get_openssl_key(key32);
-    CHECK(ec_key != NULL);
-    CHECK(ECDSA_sign(0, message, sizeof(message), signature, &sigsize, ec_key));
-    CHECK(secp256k1_ecdsa_sig_parse(&sigr, &sigs, signature, sigsize));
-    CHECK(secp256k1_ecdsa_sig_verify(&sigr, &sigs, &q, &msg));
-    secp256k1_scalar_set_int(&one, 1);
-    secp256k1_scalar_add(&msg2, &msg, &one);
-    CHECK(!secp256k1_ecdsa_sig_verify(&sigr, &sigs, &q, &msg2));
-
-    random_sign(&sigr, &sigs, &key, &msg, NULL);
-    CHECK(secp256k1_ecdsa_sig_serialize(signature, &secp_sigsize, &sigr, &sigs));
-    CHECK(ECDSA_verify(0, message, sizeof(message), signature, secp_sigsize, ec_key) == 1);
-
-    EC_KEY_free(ec_key);
-}
-
-void run_ecdsa_openssl(void) {
-    int i;
-    for (i = 0; i < 10*count; i++) {
-        test_ecdsa_openssl();
-    }
-}
-#endif
-
 #ifdef ENABLE_MODULE_ECDH
 # include "modules/ecdh/tests_impl.h"
 #endif
@@ -6729,9 +6609,6 @@ int main(int argc, char **argv) {
     run_ecdsa_sign_verify();
     run_ecdsa_end_to_end();
     run_ecdsa_edge_cases();
-#ifdef ENABLE_OPENSSL_TESTS
-    run_ecdsa_openssl();
-#endif
 
 #ifdef ENABLE_MODULE_RECOVERY
     /* ECDSA pubkey recovery tests */