Remove OpenSSL #17265

pull fanquake wants to merge 8 commits into bitcoin:master from fanquake:remove_openssl changing 28 files +22 −209
  1. fanquake commented at 2:14 PM on October 26, 2019: member

    Now that #17165 has been merged, removing our remaining OpenSSL usage is possible.

    That remaining usage was a call to RAND_bytes during the ::SLOW path of ProcRand. As well as feeding output from our RNG back into OpenSSL via RAND_add during the ::SLOW and ::SLEEP paths.

    Optimistically tagged for 0.20.0. Needs discussion, potentially in an upcoming weekly meeting?

    Closes #12530.

  2. fanquake added the label Needs release note on Oct 26, 2019
  3. fanquake added the label Needs Conceptual Review on Oct 26, 2019
  4. fanquake added this to the milestone 0.20.0 on Oct 26, 2019
  5. fanquake requested review from sipa on Oct 26, 2019
  6. jnewbery commented at 3:00 PM on October 26, 2019: member

    Concept ACK!

  7. in src/random.cpp:15 in f56a128579 outdated 
      10 | @@ -11,7 +11,7 @@
  11 |  #include <compat.h> // for Windows API
  12 |  #include <wincrypt.h>
  13 |  #endif
  14 | -#include <logging.h>  // for LogPrint()
  15 | +#include <logging.h>  // for LogPrintf()
  16 |  #include <sync.h>     // for WAIT_LOCK
    fanquake commented at 3:11 PM on October 26, 2019:

    note: Will update 5cc4f20c7064fee90a78a827a9e30b2934ce4ca7 to change WAIT_LOCK to Mutex.

  8. DrahtBot commented at 3:31 PM on October 26, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16834 (Fetch Headers over DNS by TheBlueMatt)
    • #16762 (Rust-based Backup over-REST block downloader by TheBlueMatt)
    • #15382 (util: add runCommandParseJSON by Sjors)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  9. sipa commented at 7:46 PM on October 26, 2019: member

    I think we wanted to include some of the environmental entropy sources (statistics, pid, ...) that OpenSSL uses in our own RNG state first. See #10299. I'll PR something soon.

  10. promag commented at 7:48 PM on October 26, 2019: member

    Concept ACK.

  11. BlockMechanic commented at 7:22 AM on October 27, 2019: contributor

    Concept ACK.

    I recently ran into openssl issues here #17123, this is awesome !

  12. laanwj commented at 9:55 AM on October 27, 2019: member

    Concept and code review ACK, agree that we should ideally get #17270 in first.

  13. practicalswift commented at 10:02 PM on October 27, 2019: contributor

    Concept ACK

    Very pleased to see OpenSSL go :)

  14. fanquake force-pushed on Oct 28, 2019
  15. fanquake renamed this:
    [WIP] Remove OpenSSL
    Remove OpenSSL
    on Oct 28, 2019
  16. fanquake removed the label Needs Conceptual Review on Oct 28, 2019
  17. fanquake commented at 1:10 PM on October 28, 2019: member

    Fixed doc nit above and squashed some commits together. This is waiting on #17270.

  18. fanquake cross-referenced this on Oct 28, 2019 from issue Feed environment data into RNG initializers by sipa
  19. Sjors commented at 3:47 PM on October 29, 2019: member

    Concept ACK after #17270. This fixes #12530.

  20. jamesob commented at 4:03 PM on October 29, 2019: member

    big Concept ACK

  21. fanquake force-pushed on Nov 2, 2019
  22. random: stop feeding RNG output back into OpenSSL
    On the ::SLOW or ::SLEEP paths, we would feed our RNG output back into
OpenSSL using RAND_add. This commit removes that functionality.

RAND_add(): https://www.openssl.org/docs/manmaster/man3/RAND_add.html

RAND_add() mixes the num bytes at buf into the internal state of the
random generator. This function will not normally be needed, as
mentioned above. The randomness argument is an estimate of how much
randomness is contained in buf, in bytes, and should be a number
between zero and num.
    5624ab0b4f
  23. random: stop retrieving random bytes from OpenSSL
    On the ::SLOW path we would use OpenSSL as an additional source of
random bytes. This commit removes that functionality. Note that this was
always only an additional source, and that we never checked the return
value

RAND_bytes(): https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html

RAND_bytes() puts num cryptographically strong pseudo-random bytes into buf.
    4fcfcc294e
  24. random: Remove remaining OpenSSL calls and locking infrastructure b49b6b0f70
  25. build: remove OpenSSL detection and libs 8983ee3e6d
  26. depends: remove OpenSSL package 648b2e3c32
  27. doc: remove OpenSSL from build instructions and licensing info a4eb839619
  28. ci: remove OpenSSL installation 397dbae070
  29. doc: add OpenSSL removal to release-notes.md e5a0bece6e
  30. fanquake force-pushed on Nov 18, 2019
  31. fanquake marked this as ready for review on Nov 18, 2019
  32. fanquake removed the label Needs release note on Nov 18, 2019
  33. fanquake added the label Build system on Nov 18, 2019
  34. fanquake added the label Utils/log/libs on Nov 18, 2019
  35. MarcoFalke added the label Needs gitian build on Nov 18, 2019
  36. MarcoFalke commented at 3:45 PM on November 18, 2019: member

    ACK e5a0bece6e84402fcb1fe4f25fd24da1d21ec077

  37. DrahtBot commented at 12:32 AM on November 19, 2019: contributor

    <!--a722867cd34abeea1fadc8d60700f111-->

    Gitian builds

    File commit 397c6d32c8f8a20a3605ef0d51d159adc21fd125<br>(master) commit e585117b8cbf55a5a24e69daa46213eb85e3ca97<br>(master and this pull)
    bitcoin-0.19.99-osx-unsigned.dmg bd2b4ba1fdf14cc3... 82e6d97a073a58b0...
    bitcoin-0.19.99-osx64.tar.gz 808ee6836d30ef71... 548abcca6668abd8...
    bitcoin-0.19.99-win64-debug.zip f2198cd29b50b270... ec8fa1926b836175...
    bitcoin-0.19.99-win64-setup-unsigned.exe 4d888664e10e48b3... 4f5824c20e1e76ff...
    bitcoin-0.19.99-win64.zip a5b95db2cb16eead... cfe5011add847980...
    bitcoin-0.19.99.tar.gz a53e2d332d78c83f... 3161156d91852540...
    bitcoin-core-osx-0.20-res.yml 59bb5b378df58a9a... 187795b725ae0939...
    bitcoin-core-win-0.20-res.yml 36d0611ac8b849e8... 7d527c4abd073ab3...
    linux-build.log 1899b85b14c6efa8... f9d24a4a5d96e59d...
    osx-build.log 6ebc79b0eba7f721... f661d147073efb70...
    win-build.log 6e158ddc9a88ac61... 15bf1d25b468ba3a...
    bitcoin-core-osx-0.20-res.yml.diff ab1d17c25c6dbb47...
    bitcoin-core-win-0.20-res.yml.diff 9d8445f792e214f8...
    linux-build.log.diff f316a7c935756c1e...
    osx-build.log.diff 646c8ab4a694daf2...
    win-build.log.diff 20f1212c6b9d9bfb...
  38. DrahtBot removed the label Needs gitian build on Nov 19, 2019
  39. laanwj commented at 8:25 AM on November 19, 2019: member

    ACK e5a0bece6e84402fcb1fe4f25fd24da1d21ec077

  40. laanwj referenced this in commit 2065ef66ee on Nov 19, 2019
  41. laanwj merged this on Nov 19, 2019
  42. laanwj closed this on Nov 19, 2019
  43. Sjors cross-referenced this on Nov 19, 2019 from issue [WIP] 64 bit iOS device support by Sjors
  44. laanwj cross-referenced this on Nov 19, 2019 from issue openssl version 1.1.1 and Qt 5.12.4/5 by BlockMechanic
  45. laanwj cross-referenced this on Nov 19, 2019 from issue Remove straggling OpenSSL references from doc and build by laanwj
  46. fanquake deleted the branch on Nov 19, 2019
  47. sidhujag commented at 3:18 PM on November 19, 2019: none

    concept ACK, is libsecp256k1 going to remove OpenSSL as well? It uses $CRYPTO_CFLAGS

  48. sidhujag referenced this in commit 517bf47683 on Nov 19, 2019
  49. fanquake referenced this in commit b4a1da9ef8 on Nov 19, 2019
  50. sipa commented at 4:41 PM on November 19, 2019: member

    @sidhujag libsecp256k1 only uses OpenSSL as an optional dependency in tests. This may change in the future, but the same reasons really don't apply.

  51. sidhujag referenced this in commit 8acd6757e7 on Nov 19, 2019
  52. laanwj commented at 10:11 AM on November 20, 2019: member

    it doesn't affect any of the bitcoin core binaries, so it's off topic here. Please take your question upstream. (That said, cross-checking against other libs is generally a good idea for cryptographic libraries.)

  53. fanquake cross-referenced this on Dec 2, 2019 from issue Fixing ubuntu installation description by dr-orlovsky
  54. fanquake cross-referenced this on Dec 7, 2019 from issue Update build-openbsd.md by rusticbison
  55. emilengler cross-referenced this on Dec 15, 2019 from issue SSL/TLS support for the HTTP(S) server by emilengler
  56. daira cross-referenced this on Jan 9, 2020 from issue Remove OpenSSL dependency. Ref: NCC-2016-007 by nathan-at-least
  57. fanquake referenced this in commit b07b2a9887 on Jan 10, 2020
  58. fanquake referenced this in commit 6fe72fef8a on Jan 10, 2020
  59. fanquake cross-referenced this on Jan 10, 2020 from issue doc: openSSL is no longer required to build bitcoind by fanquake
  60. fanquake referenced this in commit 0ee3961ccd on Jan 11, 2020
  61. cdecker referenced this in commit 46f879d73f on Jan 12, 2020
  62. azuchi cross-referenced this on Mar 27, 2020 from issue Remove BIP70 payment server protocol from tapyrus by Naviabheeman
  63. Naviabheeman cross-referenced this on Apr 1, 2020 from issue Remove Openssl dependency from Tapyrus by Naviabheeman
  64. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
  65. azuchi cross-referenced this on Apr 8, 2020 from issue Remove openssl and Improve rand by Naviabheeman
  66. deadalnix referenced this in commit 8d6b4a37ad on May 25, 2020
  67. deadalnix referenced this in commit 1092598e82 on May 25, 2020
  68. deadalnix referenced this in commit ea9e88b8c6 on May 25, 2020
  69. RdeWilde commented at 4:05 AM on July 26, 2020: none

    Great 👌🏼

  70. ftrader referenced this in commit 7e5daea9ee on Aug 17, 2020
  71. str4d cross-referenced this on Sep 23, 2020 from issue Remove OpenSSL by str4d
  72. zkbot referenced this in commit 5bea6d806f on Sep 23, 2020
  73. zkbot referenced this in commit 8777d2884e on Sep 29, 2020
  74. zkbot referenced this in commit b5fa52b701 on Oct 1, 2020
  75. sidhujag referenced this in commit 8d83aefe28 on Nov 10, 2020
  76. sidhujag referenced this in commit c1d569169a on Nov 10, 2020
  77. Fuzzbawls cross-referenced this on Apr 29, 2021 from issue [Refactor] Remove OpenSSL by Fuzzbawls
  78. furszy referenced this in commit 086fc30d75 on May 12, 2021
  79. Sjors cross-referenced this on Oct 4, 2021 from issue Edits to 2.7 by deutschbitte
  80. UdjinM6 cross-referenced this on Feb 11, 2022 from issue build: allow building without openssl, enables native m1 development builds by PastaPastaPasta
  81. bitcoin locked this on Feb 15, 2022
Contributors
fanquakejnewberyDrahtBotsipapromagBlockMechaniclaanwjpracticalswiftSjorsjamesobMarcoFalkesidhujagRdeWilde


sipa

Labels
Build systemUtils/log/libs

Milestone
0.20.0

Linked (view graph)
#10299 Remove OpenSSL#12530 Remove OpenSSL for non-QT builds#12557 [WIP] 64 bit iOS device support #17123 openssl version 1.1.1 and Qt 5.12.4/5#17165 Remove BIP70 support#17270 Feed environment data into RNG initializers#17309 doc: update MSVC instructions to remove Qt OpenSSL linking#17515 Remove straggling OpenSSL references from doc and build#17651 Fixing ubuntu installation description#17689 Update build-openbsd.md#17747 SSL/TLS support for the HTTP(S) server
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC