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

[WIP][Build] Libsodium upgrade & sanity checks #2358

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

furszy
Copy link

@furszy furszy commented May 1, 2021

Conclusion of a time consuming gitian investigation..

Focused on the following points:

  1. Upgrading the current libsodium version that has been marked as deprecated by the maintainers.

  2. Adding sanity checks and unit test coverage for newer libsodium versions which are breaking the network consensus rules for the Ed25519 pubkey and signature validation.

  • The node now will abort at startup if it is running a consensus breaking version of the libsodium library.
    libsodium <= 1.0.15 accepts valid signatures for a non-zero pubkey with small order; this is currently part of our consensus rules.
    libsodium >= 1.0.16 rejects all pubkeys with small order.
    Investigation coming from zcash#4359, adapted to our sources as we cannot run libsodium 1.0.18 for glibc compatibility issues.
    This dangerous situation at the moment does not affect the released binaries as gitian/depends build is hardcoded to use libsodium 1.0.15 but can happen during local builds. For example, in macOS, brew by default installs the latest supported libsodium library, which at the time being is 1.0.18, and without the pubkey and signature validation patches, it breaks our consensus rules (same occurs in xenial with the libsodium-dev package).

       Plus, added another good to have init sanity check for libsodium that verifies that signatures are canonical (s < L)
       (coming from zcash#2902ac7c)

  1. And finally, it is solving the last remaining gitian build problem that we are currently having in master.

TO DO:
Create script to build and patch libsodium 1.0.17. So the library can easily be built and linked locally fixing GA issues.
Document this properly inside each of the build_**.md files.

@furszy furszy self-assigned this May 1, 2021
@furszy furszy added this to the 6.0.0 milestone May 1, 2021
@furszy furszy force-pushed the 2021_update_libsodium_plus_solve_gitian branch from 09e0059 to 3c7e965 Compare May 1, 2021 15:37
@furszy furszy changed the title [Build] Libsodium upgrade + broken gitian fix [WIP][Build] Libsodium upgrade + broken gitian fix May 1, 2021
@furszy furszy changed the title [WIP][Build] Libsodium upgrade + broken gitian fix [WIP][Build] Libsodium upgrade + sanity checks + broken gitian fix May 1, 2021
@furszy furszy changed the title [WIP][Build] Libsodium upgrade + sanity checks + broken gitian fix [WIP][Build] Libsodium upgrade & sanity checks + broken gitian fix May 1, 2021
@furszy furszy force-pushed the 2021_update_libsodium_plus_solve_gitian branch from 3c7e965 to 0b49eda Compare May 1, 2021 16:18
@furszy furszy force-pushed the 2021_update_libsodium_plus_solve_gitian branch 3 times, most recently from 826245e to 6aeb599 Compare May 5, 2021 03:46
@random-zebra
Copy link

Gitian looking good

Compiling pull-2358 Linux
Generating report
5eb60dea4810c8b2f9908b33020428b59c03fc4b2f0af513e7fa4920db4f4b19  pivx-5.1.99-aarch64-linux-gnu-debug.tar.gz
4cc5e6e06e03451ef51790ddedacda6f32b4ec55546779c7875b6b9b91e8f4a7  pivx-5.1.99-aarch64-linux-gnu.tar.gz
c7f11c8ad9fa1c46b5614a2afa4a0cf1767297faacc6822b7f256f59d5d367d5  pivx-5.1.99-arm-linux-gnueabihf-debug.tar.gz
7b7c2c929c454df40f380a98ca908769d3d6889e0b24ca80c5b4b947ccc19289  pivx-5.1.99-arm-linux-gnueabihf.tar.gz
29a56c5ec7ff153533067ecf43208ba0802161796539d08868f74be8b4b0289e  pivx-5.1.99-i686-pc-linux-gnu-debug.tar.gz
0bf33cc8e2b9b4a5aac8c27d28b6ac43b699ed69435b584bf641245bd1a08f21  pivx-5.1.99-i686-pc-linux-gnu.tar.gz
005229e8a35da94cc0b7daeb96bde805488e668e1d255ee14ce3759604f8caa8  pivx-5.1.99-x86_64-linux-gnu-debug.tar.gz
6c9a4ee332a9828f7b21dffa989629d8be628db5b4daf0f439df4fea0488ecc5  pivx-5.1.99-x86_64-linux-gnu.tar.gz
64fcbb51d2bb9e7fdd5d17036f8a89c36bdabb95268ff8f876d916e18cb3dd47  src/pivx-5.1.99.tar.gz
2c8b3aad44846b143c90973d21426b322caac35a6c5d3f0d79dce2a522985cfd  pivx-linux-5.1-res.yml
Done.

Compiling pull-2358 Windows
Generating report
e65dc8c47209f3dc365e116618b053217bab4d68dcf86cf6ce198620477386c8  pivx-5.1.99-win-unsigned.tar.gz
5a37a9ff4953e62f99ff1bfcecbecdbe469d329abd45ddbceca465ecbc15a37c  pivx-5.1.99-win64-debug.zip
e05029a945efd7fe1165e0b3448af75a36ba1a452abb0a952ad19823e3cdf8d1  pivx-5.1.99-win64-setup-unsigned.exe
0ceccc1c80690cbffbb18ed47b2bebbe68c5599e892b1f0d4bbfe5a49b833bc5  pivx-5.1.99-win64.zip
64fcbb51d2bb9e7fdd5d17036f8a89c36bdabb95268ff8f876d916e18cb3dd47  src/pivx-5.1.99.tar.gz
dd57b85ed3b1b0b57eaddb4590cf52427ca93db0075ac565c42a35c911728608  pivx-win-5.1-res.yml
Done.

Compiling pull-2358 MacOS
Generating report
0f9b7d278efd85d8289bf5e28d7540fd909a444139aa1c91fc4bd325d9d1dd3b  pivx-5.1.99-osx-unsigned.dmg
c2921dafa6fb68cfab05a8264cea4e1c654004a07792cc26322c728355d5ade9  pivx-5.1.99-osx-unsigned.tar.gz
2a3c23a0125a3c19803748b90f2ffb2418b50eb67c1aa68f02a1d94d604a589b  pivx-5.1.99-osx64.tar.gz
64fcbb51d2bb9e7fdd5d17036f8a89c36bdabb95268ff8f876d916e18cb3dd47  src/pivx-5.1.99.tar.gz
88199a3e92f5d97ab8562f596000ee13ff2f2aa740b0d24642703f4935428836  pivx-osx-5.1-res.yml
Done.

@Fuzzbawls
Copy link
Collaborator

So, i've spent the past couple days diving into this. I didn't like the idea of relying solely on a runtime assertion to determine if a linked 3rd party library dependency was compatible or not...

Ended up adding an additional patch to the 1.0.17 source tarball that sets a fixed SODIUM_LIBRARY_VERSION_MAJOR and SODIUM_LIBRARY_VERSION_MINOR combination, which was unique to v1.0.15. This then allowed me to do configure time checking for library compatibility that would disallow compilation if the detected library version was not compatible and error out.

This, of course, lead to issues with the PPA nightly builds. PPA builds don't allow network access during build-time, so they cannot use the install-sodium.sh script... so I setup a new PPA specifically for the purpose of providing the patched 1.0.17 version to the nightly PPA wallet builds. Similar to how we have a PPA that provides libdb4.8, this new PPA could also be used by end users who wish to self-compile on ubuntu, but who also do not want to use the install script.

The branch i've been working on is https://github.com/Fuzzbawls/PIVX/tree/pr-2358, which was rebased on master to resolve the minor conflicts currently present here.

Still todo:

  • test/fix nightly SNAP builds
  • add a new GA job for Ubuntu Focal that installs the patched libsodium via PPA (mimicking a syslib; this is to test the dependency conflict coming from ZMQ, which also requires libsodium23)
  • update documentation to reference the new PPA and build dependency requirements

Note: While all this makes the runtime assertion rather redundant, I have left it in place so as to not revert anything in the branch.

@furszy
Copy link
Author

furszy commented Jun 29, 2021

here again finally, so much stuff happening..
Ok, nice stuff 👍. The own libsodium PPA repo sounds good, was thinking on doing the same.
Will check it and rebase this work.

@furszy furszy force-pushed the 2021_update_libsodium_plus_solve_gitian branch from 6aeb599 to 6ca1355 Compare June 29, 2021 14:05
@furszy
Copy link
Author

furszy commented Jun 29, 2021

Updated + removed libsodium from the recently added native macOS-10.15 job. Let's see if GA likes it now.

str4d and others added 13 commits July 6, 2021 21:07
Not adding test vectors for non-canonical pubkeys, as that would
require grinding to find a private key corresponding to one of the 19
pubkeys that can be non-canonical.

Adapted for PIVX unit test framework.
Abort at node startup if it is running a non-patched version of the libsodium library.

libsodium <= 1.0.15 accepts valid signatures for a non-zero pubkey with small order; this is currently part of our consensus rules.
libsodium >= 1.0.16 rejects all pubkeys with small order.
…hat.

Adaptation coming from zcash@2902ac7ce8e754d09a5137cba82d8af10c172977
…ency, to avoid updating config scripts over the network.
This allows us to do some compile time checking to see if we are using a
compatible version
Otherwise it is already supplied by the previous depends stage
GA builds that intend to use syslibs instead of depends (CMake and
no_depends jobs) end up installing libsodium syslib, so remove that
prior to installing our patched version.

This also moves the installation of the patched version of sodium to the
 environment setup stage.
We require the use of libsodium <= 1.0.15, or a patched version of
1.0.17, which is supplied by depends or the `install_sodium.sh` shell
script.

This change will look at the libsodium version header's
`SODIUM_LIBRARY_VERSION_MAJOR` and `SODIUM_LIBRARY_VERSION_MINOR` values
to determine if it is a valid version. For reference, Sodium 1.0.15
used `10` and `0`, respectively. Earlier versions used a different major
value, and later versions used a different minor value, so this
combination of values is unique to v1.0.15 (and our patched version of
v1.0.17)
@furszy furszy force-pushed the 2021_update_libsodium_plus_solve_gitian branch from fbe871a to 80e8493 Compare July 7, 2021 00:08
@furszy furszy changed the title [WIP][Build] Libsodium upgrade & sanity checks + broken gitian fix [WIP][Build] Libsodium upgrade & sanity checks Jul 7, 2021
@furszy
Copy link
Author

furszy commented Aug 6, 2021

Moved to v6.0 milestone.

@furszy furszy modified the milestones: 5.3.0, 6.0.0 Aug 6, 2021
@random-zebra random-zebra modified the milestones: 6.0.0, v5.4.0 Aug 13, 2021
@furszy furszy modified the milestones: 5.4.0, 6.0.0 Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants