Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test failure when building against OpenSSL 3 on macOS #984

Closed
fanquake opened this issue Sep 30, 2021 · 3 comments · Fixed by #983
Closed

Test failure when building against OpenSSL 3 on macOS #984

fanquake opened this issue Sep 30, 2021 · 3 comments · Fixed by #983

Comments

@fanquake
Copy link
Member

Building 2a3a97c:

./autogen.sh
./configure
Build Options:
  with ecmult precomp     = yes
  with external callbacks = no
  with benchmarks         = yes
  with tests              = yes
  with openssl tests      = yes
  with coverage           = no
  module ecdh             = no
  module recovery         = no
  module extrakeys        = no
  module schnorrsig       = no

  asm                     = x86_64
  ecmult window size      = 15
  ecmult gen prec. bits   = 4

  valgrind                = no
  CC                      = gcc
  CPPFLAGS                = -I/usr/local/opt/valgrind/include 
  SECP_CFLAGS             = -O2  -std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-overlength-strings -Wall -Wno-unused-function -Wextra -Wcast-align -Wconditional-uninitialized -fvisibility=hidden 
  CFLAGS                  = -g -O2
  LDFLAGS                 = 

  CC_FOR_BUILD            = gcc
  CPPFLAGS_FOR_BUILD      = -I/usr/local/opt/valgrind/include 
  SECP_CFLAGS_FOR_BUILD   = -O2  -std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-overlength-strings -Wall -Wno-unused-function -Wextra -Wcast-align -Wconditional-uninitialized -fvisibility=hidden 
  CFLAGS_FOR_BUILD        = -g -O2
  LDFLAGS_FOR_BUILD       = 
...
gmake check -j9
...
  CCLD     bench_internal
  CCLD     bench_ecmult
  CCLD     libsecp256k1.la
  CCLD     bench_verify
  CCLD     bench_sign
6 warnings generated.
  CCLD     tests
gmake  check-TESTS
gmake[1]: Entering directory '/Users/michael/github/secp256k1'
gmake[2]: Entering directory '/Users/michael/github/secp256k1'
PASS: exhaustive_tests
./build-aux/test-driver: line 112: 88313 Abort trap: 6           "$@" >> "$log_file" 2>&1
FAIL: tests
============================================================================
Testsuite summary for libsecp256k1 0.1
============================================================================
# TOTAL: 2
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See ./test-suite.log
============================================================================
gmake[2]: *** [Makefile:1307: test-suite.log] Error 1

test-suite.log:

========================================
   libsecp256k1 0.1: ./test-suite.log
========================================

# TOTAL: 2
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: tests
===========

test count = 64
random seed = 59e74cf6fdde17e225a9ade47f72f707
Failure 10 on 30 24 02 06 b1 c3 87 ff 07 00 02 1a 45 00 00 08 00 00 00 00 00 00 00 10 00 00 00 02 00 00 00 00 00 00 80 ff 07 80 
src/tests.c:6015: test condition failed: ret == 0
FAIL tests (exit status: 134)

config.log

If I build against an older version of OpenSSL, i.e 1.1, I don't see any issues:

./autogen.sh
CRYPTO_LIBS="-L/usr/local/opt/[email protected]/lib" CRYPTO_CFLAGS="-I/usr/local/opt/[email protected]/include" ./configure
gmake check -j9
....
gmake  check-TESTS
gmake[1]: Entering directory '/Users/michael/github/secp256k1'
gmake[2]: Entering directory '/Users/michael/github/secp256k1'
PASS: exhaustive_tests
PASS: tests
============================================================================
Testsuite summary for libsecp256k1 0.1
============================================================================
# TOTAL: 2
# PASS:  2
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
@real-or-random real-or-random linked a pull request Oct 1, 2021 that will close this issue
@real-or-random
Copy link
Contributor

This does mean that any non-bitcoin libsecp256k1 users that need to interoperate with openssl may now experience inconsistencies. ... and maybe it suggests that libsecp256k1 should increase the strictness of its standard ecdsa parser to match the bitcoin strictder, which should also make it consistent with openssl 3. (legacy signatures in the blockchain are supported by the 'lax' parser in contrib in any case).

This would be a breaking change, so I don't think we should do it. I'd prefer API stability over compatibility with OpenSSL 3. There are some (blockchain/encoding-critical) projects out there that rely on libsecp256k1 (without the lax parser).

@real-or-random
Copy link
Contributor

real-or-random commented Oct 3, 2021

As far as breaking, the obvious thing to do would be to just introduce the stricter parser under a different name and recommend it, and mark the old one as deprecated (and potentially move it to contrib with the other lax parsers).

Ok sure, we could do this but it's work and I don't think Bitcoin Core would benefit from this, so we should not spend time on this. And new applications shouldn't use ECDSA at all.

@andypost
Copy link

andypost commented Oct 5, 2021

The related issue bitcoin/bitcoin#23048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants