lbryio / lbrycrd

The blockchain that provides the digital content namespace for the LBRY protocol
https://lbry.com
MIT License
2.57k stars 178 forks source link

fixed incompatibility with newer libssl #221

Closed BrannonKing closed 6 years ago

BrannonKing commented 6 years ago

I was going to wait for the upstream merge, but I don't want to have to explain this problem in my introductory video.

BrannonKing commented 6 years ago

@lbrynaut , @kaykurokawa to finish our conversation from yesterday, the upstream bitcoin was changed to use a completely different library in 0.13: https://github.com/bitcoin/bitcoin/commit/9049cde4d962862f507f9ddf1c0dbd49ea04be51

BrannonKing commented 6 years ago

The change to openssl from 1.0.2 to 1.1 is quite long: https://www.openssl.org/news/changelog.html#x12 There are latest versions of 1.0.2 and 1.1.x -- it looks like many of the same security fixes go into both: https://www.openssl.org/news/vulnerabilities.html I can't make any argument that 1.1.latest is more/less vulnerable than 1.0.latest.

BrannonKing commented 6 years ago

Our current hard-coded dependency is openssl-1.0.1p. This is far behind the current version, and it was patched by all the major OSs ages ago. It's a strong argument against using the well-known version. https://www.openssl.org/news/vulnerabilities.html

lbrynaut commented 6 years ago

It sounds clearly like the way to go is to also update to some other library, and not to continue using OpenSSL, which has many known issues and has never been thoroughly reviewed. I've actually never been involved with a project that actively wanted to work with OpenSSL, rather than making best efforts to move away from it. Enabling users to continue on using it adds more harm than good, IMO.

lbrynaut commented 6 years ago

I was going to wait for the upstream merge, but I don't want to have to explain this problem in my introductory video.

If reproducible_build.sh isn't working, that's a much higher priority issue to resolve than this kind of hack for openssl. It should be the "go to" for installation, since getting the dependencies right are difficult, and may affect how the code operates with system version differences. I'm not saying our script is the best script for this, but there's a very good reason bitcoin related code strives for reproducible builds with fixed dependencies.

BrannonKing commented 6 years ago

[reproducable_build.sh] should be the "go to" for installation,

I assume that you mean compilation, not installation. (Our curated release builds are not in question here; we must have static builds for official releases.) And I fully disagree with this stated ideal. The "go to" should be the common build tools that developers across the planet use. In C++ land that means autotools or CMake. We don't need an additional learning curve for compiling our software.

since getting the dependencies right are difficult...

It may be difficult on Windows or OSX; I've never tried it on one of those. But on Linux, this is a common and well-handled scenario. The bitcoin build instructions are very clear on how to get and install dependencies using common paradigms. There is no "deps, build, and test" script in the bitcoin codebase that I can see. Gitian seems to be the modern equivalent of their reproducible deployment, but that's not what developers are iterating with. Current instructions: https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md

and may affect how the code operates with system version differences.

We have three primary dependencies:

  1. BerkleyDB -- used only by the built-in wallet, something our customers are generally not using.
  2. OpenSSL -- release with our old version of this is a significant mistake, and if it's good enough for the user's system it's probably good enough for us, since all modern OSs keep this up to date. We rely on consensus for security anyway, not what's on a minority of users' machines. This goes away with the upstream merge. (This whole PR is a temporary solution.)
  3. Boost -- it's used heavily in the 0.12 code, but these are core boost pieces, not the more esoteric parts of Boost. Very little on these pieces has changed in recent years. In addition, it's a stated goal of bitcoin to eliminate Boost dependencies. Upstream has already made significant progress on this with more PRs in the wings. Whatever is available on any modern machine will be fine.

bitcoin-related code strives for reproducible builds with fixed dependencies.

I could find no proof of this in the bitcoin docs. However, I do think this is a common need. And the "go to" solution for this is Docker, which completely eliminates the dependency build time. I've been debating on where to host the Dockerfile for compiling lbrycrd; this repo or the lbry-docker repo? The Dockerfile is trivial. I have one that I use already.

It sounds clearly like the way to go is to also update to some other library...

Yes, that's in progress, no? But I need something this week, something for the interim.

bvbfan commented 6 years ago

Our current hard-coded dependency is openssl-1.0.1p. This is far behind the current version, and it was patched by all the major OSs ages ago.

I'm on 1.0.2p so we can increase it. Versions 1.1 and 1.0 goes in parallel, i don't see drama here. OpenSSL is far away from insecure, so moving on other one is not required.

@BrannonKing, changes are sane you can go for them. @lbrynaut, when you merge new bitcoin, on demand, depend on OpenSSL can be removed.

lbrynaut commented 6 years ago

[reproducable_build.sh] should be the "go to" for installation,

I assume that you mean compilation, not installation. (Our curated release builds are not in question here; we must have static builds for official releases.) And I fully disagree with this stated ideal. The "go to" should be the common build tools that developers across the planet use. In C++ land that means autotools or CMake. We don't need an additional learning curve for compiling our software.

It sounds like you agree that the provided releases builds are the way to go, but you don't think there's a difference between running supported software linked with correct dependencies and compiling (unsupported) software linked with incorrect dependencies. Bugs can happen either way, but there's a chance also that they may be mismatched, no?

It sounds clearly like the way to go is to also update to some other library...

Yes, that's in progress, no? But I need something this week, something for the interim.

If you need a quick fix this week, it sounds like you know what you want to support.

I think that reproducible_build.sh should work (not just on travis), and once installed, it's trivial to modify and build the code subsequently with supported dependencies. It'll never improve if we don't know there are issues. Maybe that's something others can help with too. It is also specifically designed to not have the kind of issue that this kind of 'solution' attempts to address.

@bvbfan This isn't drama -- I just don't agree that it's necessary or a good idea. There's no reason we all have to agree on something like this. If the goal is to get more people building unsupported builds, we just have differing goals.