build: make libsodium an optional build dependency

[tobtoht]

This PR is a step towards removing libsodium as a release build dependency.

We don't need to pull in libsodium for one evergreen function (crypto_verify_32). This PR vendors that function in src/crypto. Libsodium is now only required when building with Trezor support.

The code was copied from: src/libsodium/include/crypto_verify_32.h and src/libsodium/crypto_verify/verify.c in the libsodium repo checked out at 1.0.20-RELEASE, with minor (non-functional) changes.

Trezor uses crypto_aead_chacha20poly1305_ietf_decrypt from libsodium in src/device_trezor/protocol.cpp which is a bit more complicated to vendor. We may want to consider using the upstream implementation or OpenSSL rather than libsodium's.

[iamamyth]

There were earlier discussions/PRs expanding the use of libsodium, for example, #4159. It's meant to be a fairly small and concise library so I'm not sure how much benefit eliminating it, versus replacing existing in-tree code with it, provides.

[jeffro256]

NACK. We can also use libsoduim to replace our custom memory zeroizing functions, as well as our mlocked/wiped storage for secret keys. I'm of the opinion that we should be unvendoring common crypto functions like these since it's just extra code that we will have to maintain in the future, and other well-established crypto libraries like libsodium probably do it better anyways. And let's say that there is a security vulnerability with one these crypto functions. If we vendor this code, we don't get the benefit of upstream security patches.

[iamamyth]

Worth noting: This very much requires strict aliasing disabled (which sodium does, quite explicitly, and by design; whereas this project's CMakeLists.txt suggest an intent to go the other direction):

  # With GCC 6.1.1 the compiled binary malfunctions due to aliasing. Until that
  # is fixed in the code (Issue #847), force compiler to be conservative.
  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-strict-aliasing")
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-strict-aliasing")

The more creative the compiler vendors get, the more problematic even a single vendored function like this becomes.

[iamamyth]

It happens that this PR crosses paths with a few other issues that bothered me recently, all to do with memory layout/alignment issues. Per the line comment I just posted, precisely because of those issues, I really dislike vendoring crypto functions. If you want to eliminate crypto_verify_32, you could use the OpenSSL equivalent, CRYPTO_memcmp, which takes a very different approach from libsodium's version (both rely on the same xor construction, but OpenSSL does the most straightforward thing possible, whereas libsodium tries to optimize the implementation and fight compiler optimizers). OpenSSL also has its own approach to the "keys in memory" problem (OpenSSL_cleanse, as well as, to a degree, the OpenSSL_secure_ family of functions). So, if you're looking to drop libsodium, I think "use OpenSSL" would be an option, because it's already used in the project, and, while not without historical problems in certain contexts, supports a very wide range of ciphers.

[tobtoht]

To further motivate why I submitted this PR and respond to some of the comments here:

I think we should eliminate chain split risk as much as possible. That means that we should avoid using library functions for our consensus. Otherwise, we trust that the semantics of these functions are never changed. If there is a change, even non-malicious, or through miscompilation, it could result in a chain split (inadvertently or through an introduced exploit).

This risk exists even if we don't update the library in our release builds. Many economic participants (exchanges, services, etc) run custom builds in production. These builds use library versions that don't necessarily correspond with our 'official' release builds. Library authors therefore have the power to change the consensus under which a significant number of nodes operate without our approval by simply releasing a new library version.

Monero doesn't have a consensus engine library like libbitcoinkernel, so it's unclear precisely which library functions underpin our consensus. Of our build dependencies, various boost components and this one function from libsodium are certainly used: crypto_verify_32 is used throughout bulletproofs and other crypto code. OpenSSL and unbound (unlikely) might touch consensus, but I can't conclusively say without further investigation.

A consensus library is something I would like to move towards in the long-term, but it necessitates that we 'own' our consensus code. If we must use third-party code, then vendoring ensures that we are in control when it changes (preferably never). This way we can benefit from 'better', higher quality, audited code that can be thoroughly tested within our builds without introducing chain split risk.

If implemented correctly, cryptographic functions require very little, if any, maintenance. Whereas library dependencies have an ongoing maintenance burden through package updates. Consider also that our consensus code must be maintained indefinitely and is likely to survive longer than any third-party library, thus vendoring will occur eventually. If not now, then years or decades from now.

There is also a desire from downstream projects for lower build complexity. Even if I do not find support in vendoring this particular function, replacing it with CRYPTO_memcmp from OpenSSL as @iamamyth suggested in order to drop the libsodium dependency might be worthwhile for this purpose alone.

The code in this PR isn't worked out (hence drafted), I was mostly looking for concept approvals - which seems unlikely now, unless what I have written here happens to strike a chord.

[iamamyth]

If implemented correctly, cryptographic functions require very little, if any, maintenance

There are two problems with this premise as the justification for vendoring:

1. It applies equally to the cryptographic libraries (for example, "constant time comparison" won't suddenly change meaning tomorrow, nor will the blake2b algorithm).
2. The behavior of cryptographic code may change without changes to the code itself: crypto_verify_32 provides a great case study in this exact phenemonon. As mentioned in my earlier comment, the function includes a ton of defensive techniques to try to prevent compiler optimizations, but they're inherently imperfect, and the build environment (both compiler flags, as well as the compiler used) greatly impacts them. The teams maintaining these libraries take care to address portability problems. Vendoring such code only expands the scope of unwanted, unintended compiler optimizations (the compiler can't apply certain global optimizations, like inlining the call and realizing its a no-op, to a dynamically linked third-party library function). If we want to move forward with flexibility in changing c++ versions, upgrading compilers, and possibly introducing safety-oriented build flags, the most likely cause of chain splits would be the project itself, not an external library like sodium or OpenSSL. While I can recall many OpenSSL bugs, they have not fallen along the lines of, say, "incorrectly computes hash function outputs".

I may agree with your proposal to limit dependencies:

Even if I do not find support in vendoring this particular function, replacing it with CRYPTO_memcmp from OpenSSL as @iamamyth suggested in order to drop the libsodium dependency might be worthwhile for this purpose alone.

We're not getting rid of OpenSSL any time soon. Many cryptography and server projects depend on it somehow, meaning it's not a difficult to obtain or build external library. There's an important discussion to be had about whether to ditch libsodium, or expand its usage, but I don't see a compelling case for keeping it just for a pair of functions we can replace with well-known alternatives.