build, crypto, script: remove most of OpenSSL usage #2705

div72 · 2023-10-08T21:12:29Z

Removes all non-trivial usages of OpenSSL except in src/crypter.*.

Mostly sourced from crypto/fips/module/md5/ at `6d3db84c47643271cb553593ee67362be3820874`.

jamescowens · 2023-10-10T16:43:23Z

I like where this is going. On the disallowed ops, is this technically a mandatory to deprecate script code? How do we know those op codes are not in use?

div72 · 2023-10-10T17:28:54Z

The disallowed ops are unconditionally rejected here, so all nodes already reject scripts with any of them present at any block height already:

Gridcoin-Research/src/script.cpp

Lines 364 to 379 in 7548e6c

    
           if (opcode == OP_CAT || 
        
               opcode == OP_SUBSTR || 
        
               opcode == OP_LEFT || 
        
               opcode == OP_RIGHT || 
        
               opcode == OP_INVERT || 
        
               opcode == OP_AND || 
        
               opcode == OP_OR || 
        
               opcode == OP_XOR || 
        
               opcode == OP_2MUL || 
        
               opcode == OP_2DIV || 
        
               opcode == OP_MUL || 
        
               opcode == OP_DIV || 
        
               opcode == OP_MOD || 
        
               opcode == OP_LSHIFT || 
        
               opcode == OP_RSHIFT) 
        
               return false;

The main issue with this PR currently is that the CBigNum -> arith_uint256 changes to staking calculations break sync for PoS blocks at some point. I thought everything was limited to 2^256 but I'll re-check to see what broke.

While a non-weighted stake target hash can fit 236-bit integers[0], the multiplication by the 64-bit stake can cause the weighted target hash to go up to 300-bits. While I think this would have been better fixed by lowering the stake target hash limit to be 192-bit so that the weighted target hash fits 256-bit, this would break consensus and there are already blocks that require weighted target hashes higher than 2**256 to verify. The existing blob_int class requires bits which are multiples of 32, so 320 is used. [0] - src/gridcoin/staking/difficulty.cpp:24 # The PROOF_OF_STAKE_LIMIT is set to uint256 maximum right shifted by 20 resulting in a 236-bit limit. That's enforced by the GRC::GetNextTargetRequired function which is used to generate the nBits of blocks.

jamescowens

Hmm... how did this work before? I don't think we used 300+ bit integers in the original implementation...

div72 · 2023-10-22T07:00:55Z

Hmm... how did this work before? I don't think we used 300+ bit integers in the original implementation...

@jamescowens Pre f7c13be, CBigNum which used OpenSSL's unbounded BIGNUM was used.

jamescowens · 2023-10-22T15:06:05Z

Ah yes...

jamescowens · 2023-10-22T16:21:49Z

Hmm...

`
Build Options:
with external callbacks = no
with benchmarks = yes
with tests = yes
with ctime tests = yes
with coverage = no
with examples = no
module ecdh = no
module recovery = yes
module extrakeys = yes
module schnorrsig = yes

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

valgrind = yes
CC = gcc-12
CPPFLAGS =
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 -Wcast-align=strict -fvisibility=hidden
CFLAGS = -g -O2
LDFLAGS =
Fixing libtool for -rpath problems.

Options used to compile and link:
with wallet = yes
with gui / qt = yes
with qr = yes
with zmq =
with test = yes
with bench = yes
with upnp = yes
debug enabled = no
werror = no

target os = linux
build os =

CC = /usr/bin/ccache gcc-12
CFLAGS = -g -O2
CPPFLAGS = -DBOOST_SPIRIT_THREADSAFE -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DQT_GUI -DUSE_QRCODE
CXX = /usr/bin/ccache g++-12 -std=c++17
CXXFLAGS = -g -O2 -Wall -Wextra -Wformat -Wformat-security -Wvla -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wimplicit-fallthrough -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-deprecated-copy
LDFLAGS =
AS = gcc-12
ASFLAGS = -g -O2
`

CXX test/test_test_gridcoin-serialize_tests.o
CXX test/test_test_gridcoin-sigopcount_tests.o
CXX test/test_test_gridcoin-sync_tests.o
CXX test/test_test_gridcoin-test_gridcoin.o
CXX test/test_test_gridcoin-transaction_tests.o
CXX test/test_test_gridcoin-uint256_tests.o
CXX test/test_test_gridcoin-wallet_tests.o
CXX test/test_test_gridcoin-util_tests.o
test/gridcoin/project_tests.cpp:20:15: warning: ‘GRC::Contract {anonymous}::contract(std::string, std::string)’ defined but not used [-Wunused-function]
20 | GRC::Contract contract(std::string key, std::string value)
| ^~~~~~~~
CCLD libsecp256k1_precomputed.la
CCLD libsecp256k1.la
make[3]: Leaving directory '/home/jco/builds/Gridcoin-Research/src/secp256k1'
CXXLD gridcoinresearchd
CXXLD test/test_gridcoin
AR qt/libgridcoinqt.a
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: crypto/libgridcoin_crypto_base.a(crypto_libgridcoin_crypto_base_a-md5.o): relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIE
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: failed to set dynamic section sizes: bad value
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:4435: gridcoinresearchd] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory '/home/jco/builds/Gridcoin-Research/src'
make[1]: *** [Makefile:12371: all-recursive] Error 1
make[1]: Leaving directory '/home/jco/builds/Gridcoin-Research/src'
make: *** [Makefile:785: all-recursive] Error 1
jco@jco-monster-ng:~/builds/Gridcoin-Research>

jamescowens · 2023-10-22T16:25:06Z

Looks like the c source file for md5 is not being compiled with PIE flag.

jamescowens · 2023-10-22T20:34:48Z

I have successfully synced from zero on testnet. The CPID detection is broken with a researcher node. With this PR coming up as investor. This probably has something to do with the MD5. Need to track that down.

jamescowens · 2023-10-22T23:16:45Z

Also successfully synced from zero on mainnet.

jamescowens · 2023-10-23T20:16:26Z

Confirmed the Cpid::Hash is not working correctly.

Current testnet branch...
2023-10-23T19:58:10Z Loading BOINC CPIDs...
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23
2023-10-23T19:58:10Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = bc0621a4ac4610ffa400a0d298c02e23

This PR:
2023-10-23T20:02:21Z Loading BOINC CPIDs...
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000
2023-10-23T20:02:21Z INFO: Hash: internal = fa89b5db83c284b90f57a1fb65a137b5, email = ***, cpid = b00000002a000000ec000000e8000000

The lack of bit shifts caused the upper bytes to be interpreted as zero, after the implicit cast to uint8_t.

Since the system OpenSSL is also linked currently, that can take precedence over the vendored MD5.

Avoiding conflicts with OpenSSL.

jamescowens

tACK

@div72

Added - contrib: add nix file for compilation environment gridcoin-community#2660 (@div72) - gui: Make main Gridcoin window geometry save unique to datadir location gridcoin-community#2661 (@jamescowens) - build: Initial CMake support gridcoin-community#2676 (@CyberTailor) - util: Add `-shutdownnotify` and `startupnotify` options from upstream gridcoin-community#2688 (@barton2526) - gui, staking: Implement facilities for mandatory sidestakes and sidestake GUI gridcoin-community#2704 (@jamescowens) - gui, voting: Implement poll result caching and poll stale indicator gridcoin-community#2709 (@jamescowens) - gui, projects: Implement greylist state for projects in GUI projects table gridcoin-community#2715 (@jamescowens) - gui, poll: Implement poll expiration reminders gridcoin-community#2716 (@jamescowens) - serialize: allow variants to be serialized gridcoin-community#2729 (@div72) - gui: Implement poll field length limiters in GUI forms gridcoin-community#2742 (@jamescowens) Changed - consensus, contract, scraper, protocol, project, beacon, rpc: Replace remaining appcache sections with native structures gridcoin-community#2639 (@jamescowens) - build: update libsecp256k1 to v0.3.0 gridcoin-community#2655 (@div72) - build: Replace $(AT) with .SILENT gridcoin-community#2674 (@barton2526) - build: allow system bdb gridcoin-community#2675 (@div72) - Resize Header Column with Additional Text gridcoin-community#2683 (@PrestackI) - rpc: use RPCErrorCode everywhere gridcoin-community#2687 (@Pythonix) - wallet: SecureString to allow null characters gridcoin-community#2690 (@barton2526) - random: port some upstream changes gridcoin-community#2696 (@div72) - depends: Bump dependencies gridcoin-community#2692 (@barton2526) - doc: Update link to Discord server gridcoin-community#2693 (@adriaanjoubert) - rpc: Change capitalization, remove whitespace of rpc keys gridcoin-community#2711 (@Pythonix) - ci: bump MacOS version to 12 gridcoin-community#2713 (@div72) - depends: no-longer nuke libc++abi.so* in native_clang package gridcoin-community#2719 (@div72) - doc: update windows `-fstack-clash-protection` doc gridcoin-community#2720 (@div72) - Silence `-Wcast-function-type` warning gridcoin-community#2721 (@div72) - build: Use newest `config.{guess,sub}` available gridcoin-community#2722 (@div72) - refactor: use the try_lock result in TryEnter gridcoin-community#2723 (@div72) - Updates for file src/qt/locale/bitcoin_en.ts in pt_PT gridcoin-community#2726 (@gridcoin-community) - ci: do not silently fail gridcoin-community#2727 (@div72) - Properly include Boost Array header gridcoin-community#2730 (@theMarix) - build: Update depends zlib to 1.3.1 gridcoin-community#2734 (@jamescowens) - util: Enhance Fraction class overflow resistance gridcoin-community#2735 (@jamescowens) - refactor: Fix compilation warnings gridcoin-community#2737 (@jamescowens) - gui, util: Improve upgrade dialog gridcoin-community#2738 (@jamescowens) - util: Improve allocation class gridcoin-community#2740 (@jamescowens) - translations: translation updates for Miss Piggy release gridcoin-community#2745 (@jamescowens) Removed - gui: Disable snapshot GUI action gridcoin-community#2700 (@jamescowens) - build, crypto, script: remove most of OpenSSL usage gridcoin-community#2705 (@div72) - util: remove WSL 1 workaround in fs gridcoin-community#2717 (@div72) Fixed - diagnostics: fix missing arg in ETTS warning gridcoin-community#2684 (@div72) - misc: fix include guard in netaddress.h gridcoin-community#2695 (@div72) - gui: Fix expired pending beacon display gridcoin-community#2698 (@jamescowens) - consensus: Fix 20230904 testnet forking issue gridcoin-community#2703 (@jamescowens) - gui: Fix filter by type in Transaction View gridcoin-community#2708 (@jamescowens) - depends: make fontconfig build under clang-16 gridcoin-community#2718 (@div72) - diag: fix researcher mode check gridcoin-community#2725 (@div72) - gui: Add missing switch cases for ALREADY_IN_MEMPOOL gridcoin-community#2728 (@jamescowens) - beacon, registry: Fix beacon history stall gridcoin-community#2731 (@jamescowens) - build: Implement comp_double comparison function in certain tests gridcoin-community#2741 (@jamescowens) - ci: change Qt path in CMake CI gridcoin-community#2743 (@div72) - net: Correct -tor argument handling gridcoin-community#2744 (@jamescowens)

div72 added 7 commits October 9, 2023 00:22

crypto: add MD5 implementation from BoringSSL

084f313

Mostly sourced from crypto/fips/module/md5/ at `6d3db84c47643271cb553593ee67362be3820874`.

crypto: use vendored MD5 implementation

f2d993e

upgrade: use vendored SHA256 implementation

0cf35b7

crypto: use vendored HMAC-SHA256 implementation for PBKDF2

979ca56

script: remove code for disallowed ops

f8b4afb

script: use CScriptNum instead of CBigNum

4bf9686

refactor: remove usages of CBigNum

f7c13be

div72 force-pushed the closessl branch from 7a06e44 to f7c13be Compare October 8, 2023 21:23

jamescowens self-requested a review October 10, 2023 03:00

jamescowens assigned div72 Oct 10, 2023

jamescowens added compatibility deprecation Removed deprecated functionality labels Oct 10, 2023

jamescowens added this to the Miss Piggy milestone Oct 10, 2023

div72 force-pushed the closessl branch from 8c05aa3 to 53400d5 Compare October 19, 2023 21:15

div72 marked this pull request as ready for review October 19, 2023 21:22

jamescowens reviewed Oct 21, 2023

View reviewed changes

build: add PIE_FLAGS to CFLAGS for crypto lib

7cf37ca

div72 added 3 commits October 24, 2023 00:25

crypto: fix integer stores

9afa044

The lack of bit shifts caused the upper bytes to be interpreted as zero, after the implicit cast to uint8_t.

crypto: rename MD5 function to GRC__MD5

d5e2b8d

Since the system OpenSSL is also linked currently, that can take precedence over the vendored MD5.

crypto: mark internal MD5 functions as static

e7bc015

Avoiding conflicts with OpenSSL.

jamescowens approved these changes Oct 23, 2023

View reviewed changes

jamescowens merged commit d609b9e into gridcoin-community:development Oct 23, 2023
17 checks passed

jamescowens linked an issue Oct 28, 2023 that may be closed by this pull request

OpenSSL 1.1.1 has reached EOL #2702

Closed

jamescowens mentioned this pull request Oct 28, 2023

OpenSSL 1.1.1 has reached EOL #2702

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build, crypto, script: remove most of OpenSSL usage #2705

build, crypto, script: remove most of OpenSSL usage #2705

div72 commented Oct 8, 2023

jamescowens commented Oct 10, 2023

div72 commented Oct 10, 2023

jamescowens left a comment

div72 commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 23, 2023 •

edited

Loading

jamescowens left a comment

build, crypto, script: remove most of OpenSSL usage #2705

build, crypto, script: remove most of OpenSSL usage #2705

Conversation

div72 commented Oct 8, 2023

jamescowens commented Oct 10, 2023

div72 commented Oct 10, 2023

jamescowens left a comment

Choose a reason for hiding this comment

div72 commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 22, 2023

jamescowens commented Oct 23, 2023 • edited Loading

jamescowens left a comment

Choose a reason for hiding this comment

jamescowens commented Oct 23, 2023 •

edited

Loading