Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Commit

Permalink
Merge dashpay#831: Safegcd inverses, drop Jacobi symbols, remove libgmp
Browse files Browse the repository at this point in the history
24ad04f Make scalar_inverse{,_var} benchmark scale with SECP256K1_BENCH_ITERS (Pieter Wuille)
ebc1af7 Optimization: track f,g limb count and pass to new variable-time update_fg_var (Peter Dettman)
b306935 Optimization: use formulas instead of lookup tables for cancelling g bits (Peter Dettman)
9164a1b Optimization: special-case zero modulus limbs in modinv64 (Pieter Wuille)
1f233b3 Remove num/gmp support (Pieter Wuille)
20448b8 Remove unused Jacobi symbol support (Pieter Wuille)
5437e7b Remove unused scalar_sqr (Pieter Wuille)
aa9cc52 Improve field/scalar inverse tests (Pieter Wuille)
1e0e885 Make field/scalar code use the new modinv modules for inverses (Pieter Wuille)
436281a Move secp256k1_fe_inverse{_var} to per-impl files (Pieter Wuille)
aa404d5 Move secp256k1_scalar_{inverse{_var},is_even} to per-impl files (Pieter Wuille)
08d5496 Improve bounds checks in modinv modules (Pieter Wuille)
151aac0 Add tests for modinv modules (Pieter Wuille)
d8a92fc Add extensive comments on the safegcd algorithm and implementation (Pieter Wuille)
8e415ac Add safegcd based modular inverse modules (Peter Dettman)
de0a643 Add secp256k1_ctz{32,64}_var functions (Pieter Wuille)

Pull request description:

  This is a rebased and squashed version of dashpay#767, adding safegcd-based implementations of constant-time and variable-time modular inverses for scalars and field elements, by Peter Dettman. The PR is organized as follows:
  * **Add secp256k1_ctz{32,64}_var functions** Introduction of ctz functions to util.h (which use `__builtin_ctz` on recent GCC and Clang, but fall back to using a software emulation using de Bruijn on other platforms). This isn't used anywhere in this commit, but does include tests.
  * **Add safegcd based modular inverse modules** Add Peter Dettman's safegcd code from dashpay#767 (without some of his optimizations, which are moved to later commits), turned into separate modules by me.
  * **Add extensive comments on the safegcd algorithm and implementation** Add a long description of the algorithm and optimizations to `doc/safegcd_implementation.md`, as well as additional comments to the code itself. It is probably best to review this together with the previous commit (they're separated to keep authorship).
  * **Add tests for modinv modules** Adds tests on the modinv interface directly, for arbitrary moduli.
  * **Improve bounds checks in modinv modules** Adds a lot of sanity checking to the modinv modules.
  * **Move secp256k1_scalar_{inverse{_var},is_even} to per-impl files** A pure refactor to prepare for switching the field and scalar code to modinv.
  * **Make field/scalar code use the new modinv modules for inverses** Actually switch over.
  * **Add extra modular inverse tests** This adds modular inverse tests through the field/scalar interface, now that those use modinv.
  * **Remove unused Jacobi symbol support** No longer needed.
  * **Remove num/gmp support** Bye-bye.
  * 3 commits with further optimizations.

ACKs for top commit:
  gmaxwell:
    ACK 24ad04f
  sanket1729:
    ACK 24ad04f
  real-or-random:
    ACK 24ad04f careful code review, some testing

Tree-SHA512: 732fe29315965e43ec9a10ee8c71eceeb983c43fe443da9dc5380a5a11b5e40b06e98d6abf67b773b1de74571fd2014973c6376f3a0caeac85e0cf163ba2144b
  • Loading branch information
sipa committed Mar 18, 2021
2 parents 4c3ba88 + 24ad04f commit 26de4df
Show file tree
Hide file tree
Showing 34 changed files with 3,049 additions and 1,650 deletions.
17 changes: 3 additions & 14 deletions .cirrus.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
env:
WIDEMUL: auto
BIGNUM: auto
STATICPRECOMPUTATION: yes
ECMULTGENPRECISION: auto
ASM: no
Expand Down Expand Up @@ -59,17 +58,15 @@ task:
- env: {WIDEMUL: int128, RECOVERY: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ECDH: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ASM: x86_64}
- env: {BIGNUM: no}
- env: {BIGNUM: no, RECOVERY: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: {BIGNUM: no, STATICPRECOMPUTATION: no}
- env: { RECOVERY: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: { STATICPRECOMPUTATION: no}
- env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
- env: {CPPFLAGS: -DDETERMINISTIC}
- env: {CFLAGS: -O0, CTIMETEST: no}
- env:
CFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer"
LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer"
UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
BIGNUM: no
ASM: x86_64
ECDH: yes
RECOVERY: yes
Expand All @@ -80,7 +77,6 @@ task:
- env: { ECMULTGENPRECISION: 8 }
- env:
RUN_VALGRIND: yes
BIGNUM: no
ASM: x86_64
ECDH: yes
RECOVERY: yes
Expand Down Expand Up @@ -115,12 +111,6 @@ task:
CC: i686-linux-gnu-gcc
- env:
CC: clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include
matrix:
- env:
BIGNUM: gmp
- env:
BIGNUM: no
<< : *MERGE_BASE
test_script:
- ./ci/cirrus.sh
<< : *CAT_LOGS
Expand Down Expand Up @@ -178,7 +168,7 @@ task:
# If we haven't restored from cached (and just run brew install), this is a no-op.
- brew link valgrind
brew_script:
- brew install automake libtool gmp gcc@9
- brew install automake libtool gcc@9
<< : *MERGE_BASE
test_script:
- ./ci/cirrus.sh
Expand All @@ -195,7 +185,6 @@ task:
HOST: s390x-linux-gnu
BUILD:
WITH_VALGRIND: no
BIGNUM: no
ECDH: yes
RECOVERY: yes
EXPERIMENTAL: yes
Expand Down
8 changes: 4 additions & 4 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ noinst_HEADERS += src/scalar_8x32_impl.h
noinst_HEADERS += src/scalar_low_impl.h
noinst_HEADERS += src/group.h
noinst_HEADERS += src/group_impl.h
noinst_HEADERS += src/num_gmp.h
noinst_HEADERS += src/num_gmp_impl.h
noinst_HEADERS += src/ecdsa.h
noinst_HEADERS += src/ecdsa_impl.h
noinst_HEADERS += src/eckey.h
Expand All @@ -26,14 +24,16 @@ noinst_HEADERS += src/ecmult_const.h
noinst_HEADERS += src/ecmult_const_impl.h
noinst_HEADERS += src/ecmult_gen.h
noinst_HEADERS += src/ecmult_gen_impl.h
noinst_HEADERS += src/num.h
noinst_HEADERS += src/num_impl.h
noinst_HEADERS += src/field_10x26.h
noinst_HEADERS += src/field_10x26_impl.h
noinst_HEADERS += src/field_5x52.h
noinst_HEADERS += src/field_5x52_impl.h
noinst_HEADERS += src/field_5x52_int128_impl.h
noinst_HEADERS += src/field_5x52_asm_impl.h
noinst_HEADERS += src/modinv32.h
noinst_HEADERS += src/modinv32_impl.h
noinst_HEADERS += src/modinv64.h
noinst_HEADERS += src/modinv64_impl.h
noinst_HEADERS += src/assumptions.h
noinst_HEADERS += src/util.h
noinst_HEADERS += src/scratch.h
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ Implementation details
* 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 (including hand-optimized assembly for 32-bit ARM, by Wladimir J. van der Laan).
* 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.
* Using 4 64-bit limbs (relying on __int128 support in the compiler).
* Using 8 32-bit limbs.
* Modular inverses (both field elements and scalars) based on [safegcd](https://gcd.cr.yp.to/index.html) with some modifications, and a variable-time variant (by Peter Dettman).
* Group operations
* 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.
Expand Down
13 changes: 0 additions & 13 deletions build-aux/m4/bitcoin_secp.m4
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,6 @@ if test x"$has_libcrypto" = x"yes" && test x"$has_openssl_ec" = x; then
fi
])

dnl
AC_DEFUN([SECP_GMP_CHECK],[
if test x"$has_gmp" != x"yes"; then
CPPFLAGS_TEMP="$CPPFLAGS"
CPPFLAGS="$GMP_CPPFLAGS $CPPFLAGS"
LIBS_TEMP="$LIBS"
LIBS="$GMP_LIBS $LIBS"
AC_CHECK_HEADER(gmp.h,[AC_CHECK_LIB(gmp, __gmpz_init,[has_gmp=yes; GMP_LIBS="$GMP_LIBS -lgmp"; AC_DEFINE(HAVE_LIBGMP,1,[Define this symbol if libgmp is installed])])])
CPPFLAGS="$CPPFLAGS_TEMP"
LIBS="$LIBS_TEMP"
fi
])

AC_DEFUN([SECP_VALGRIND_CHECK],[
if test x"$has_valgrind" != x"yes"; then
CPPFLAGS_TEMP="$CPPFLAGS"
Expand Down
2 changes: 1 addition & 1 deletion ci/cirrus.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ valgrind --version || true

./configure \
--enable-experimental="$EXPERIMENTAL" \
--with-test-override-wide-multiply="$WIDEMUL" --with-bignum="$BIGNUM" --with-asm="$ASM" \
--with-test-override-wide-multiply="$WIDEMUL" --with-asm="$ASM" \
--enable-ecmult-static-precomputation="$STATICPRECOMPUTATION" --with-ecmult-gen-precision="$ECMULTGENPRECISION" \
--enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \
--enable-module-schnorrsig="$SCHNORRSIG" \
Expand Down
4 changes: 2 additions & 2 deletions ci/linux-debian.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ RUN apt-get update
RUN apt-get install --no-install-recommends --no-upgrade -y \
git ca-certificates \
make automake libtool pkg-config dpkg-dev valgrind qemu-user \
gcc clang libc6-dbg libgmp-dev \
gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libgmp-dev:i386 \
gcc clang libc6-dbg \
gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \
gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x
58 changes: 0 additions & 58 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,12 @@ case $host_os in
# 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`
gmp_prefix=`$BREW --prefix gmp 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$gmp_prefix != x; then
GMP_CPPFLAGS="-I$gmp_prefix/include"
GMP_LIBS="-L$gmp_prefix/lib"
fi
if test x$valgrind_prefix != x; then
VALGRIND_CPPFLAGS="-I$valgrind_prefix/include"
fi
Expand Down Expand Up @@ -164,9 +159,6 @@ AC_ARG_ENABLE(external_default_callbacks,
# Legal values are int64 (for [u]int64_t), int128 (for [unsigned] __int128), and auto (the default).
AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=auto])

AC_ARG_WITH([bignum], [AS_HELP_STRING([--with-bignum=gmp|no|auto],
[bignum implementation to use [default=auto]])],[req_bignum=$withval], [req_bignum=auto])

AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto],
[assembly optimizations to use (experimental: arm) [default=auto]])],[req_asm=$withval], [req_asm=auto])

Expand Down Expand Up @@ -245,32 +237,6 @@ else
esac
fi

if test x"$req_bignum" = x"auto"; then
SECP_GMP_CHECK
if test x"$has_gmp" = x"yes"; then
set_bignum=gmp
fi

if test x"$set_bignum" = x; then
set_bignum=no
fi
else
set_bignum=$req_bignum
case $set_bignum in
gmp)
SECP_GMP_CHECK
if test x"$has_gmp" != x"yes"; then
AC_MSG_ERROR([gmp bignum explicitly requested but libgmp not available])
fi
;;
no)
;;
*)
AC_MSG_ERROR([invalid bignum implementation selection])
;;
esac
fi

# Select assembly optimization
use_external_asm=no

Expand Down Expand Up @@ -308,24 +274,6 @@ auto)
;;
esac

# Select bignum implementation
case $set_bignum in
gmp)
AC_DEFINE(HAVE_LIBGMP, 1, [Define this symbol if libgmp is installed])
AC_DEFINE(USE_NUM_GMP, 1, [Define this symbol to use the gmp implementation for num])
AC_DEFINE(USE_FIELD_INV_NUM, 1, [Define this symbol to use the num-based field inverse implementation])
AC_DEFINE(USE_SCALAR_INV_NUM, 1, [Define this symbol to use the num-based scalar inverse implementation])
;;
no)
AC_DEFINE(USE_NUM_NONE, 1, [Define this symbol to use no num implementation])
AC_DEFINE(USE_FIELD_INV_BUILTIN, 1, [Define this symbol to use the native field inverse implementation])
AC_DEFINE(USE_SCALAR_INV_BUILTIN, 1, [Define this symbol to use the native scalar inverse implementation])
;;
*)
AC_MSG_ERROR([invalid bignum implementation])
;;
esac

# Set ecmult window size
if test x"$req_ecmult_window" = x"auto"; then
set_ecmult_window=15
Expand Down Expand Up @@ -390,11 +338,6 @@ else
enable_openssl_tests=no
fi

if test x"$set_bignum" = x"gmp"; then
SECP_LIBS="$SECP_LIBS $GMP_LIBS"
SECP_INCLUDES="$SECP_INCLUDES $GMP_CPPFLAGS"
fi

if test x"$enable_valgrind" = x"yes"; then
SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
fi
Expand Down Expand Up @@ -571,7 +514,6 @@ echo " module extrakeys = $enable_module_extrakeys"
echo " module schnorrsig = $enable_module_schnorrsig"
echo
echo " asm = $set_asm"
echo " bignum = $set_bignum"
echo " ecmult window size = $set_ecmult_window"
echo " ecmult gen prec. bits = $set_ecmult_gen_precision"
# Hide test-only options unless they're used.
Expand Down
Loading

0 comments on commit 26de4df

Please sign in to comment.