fixes/refactoring for building against LibreSSL

[jonasschnelli]

Alternative solution for #7585. Re-Enables LibreSSL compatibility.
Commit count and credit goes to @AliceWonderMiscreations

[jonasschnelli]

Added a commit that fixes a logical text-issue when compiling with LibreSSL.
Current masters shows: Using OpenSSL Version: OpenSSL 1.0.2d 9 Jul 2015 (or similar).
After this PR: Using SSL Version: OpenSSL 1.0.2d 9 Jul 2015 (or similar).

[laanwj]

In main.cpp we explicitly cede for LibreSSL, do we want a similar construction here? (or even: factor it out somehow so it exists only in one place, so that we don't have this inconsistency next time)

#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
    LogPrintf("Using OpenSSL version %s\n", SSLeay_version(SSLEAY_VERSION));
#elif defined OPENSSL_VERSION
    LogPrintf("Using OpenSSL version %s\n", OpenSSL_version(OPENSSL_VERSION));
#elif defined LIBRESSL_VERSION_TEXT
    LogPrintf("Using %s\n", LIBRESSL_VERSION_TEXT);
#endif

[jonasschnelli]

Yes. Let me factor it out.

But not sure about extra treating LibreSSL, IMO we should just use SSLeay_version() (which will result in returning LIBRESSL_VERSION_TEXT in the background when compiling against LibreSSL). Any OpenSSL compatible layer should probably implement SSLeay_version().

I could imaging allow compiling against different OpenSSL alternatives (Apple also has it's own IIRC).

[laanwj]

IIRC support for SSLeay_version() was dropped in recent OpenSSL, that's why the version check was added. But apparently LibreSSL didn't follow that convention, even though they report as an OpenSSL version that would no longer have SSLeay_version(). So it is, sort of, their mistake, as they aren't a perfect drop-in replacement for OpenSSL anymore.

(or we have the OpenSSL_version versus SSLeay_version threshold configured wrongly in the first place... but I think that was checked after I remarked that before on "OpenSSL 1.1 - SSLEAY_VERSION Not Declared In This Scope #7080")

[AliceWonderMiscreations]

It's not LibreSSL's mistake. They forked at a specific version of OpenSSL that they guarantee API compatibility with. That version uses SSLEAY_VERSION. So LibreSSL has to keep that in order to keep the API compatibility promise.

The change in OpenSSL is after the fork.

[jonasschnelli]

Added a commit that refactors the SSL detection.
Needs testing on CentOS (or other LibreSSL distribution)

[laanwj]

It's not LibreSSL's mistake. They forked at a specific version of OpenSSL that they guarantee API compatibility with. That version uses SSLEAY_VERSION. So LibreSSL has to keep that in order to keep the API compatibility promise.

That's true but they do report a newer OpenSSL version in the OPENSSL_VERSION_NUMBER macro, which, in case of OpenSSL means that there is no longer a SSLeay_version.
So they forked from an older release, then bumped the version, without changing the API to be the same as that newer release of OpenSSL.

[AliceWonderMiscreations]

@jonasschnelli just a note, CentOS is not a LibreSSL distribution. I built and run LibreSSL on it, as the OpenSSL it has is old and has poor ECC support.

[laanwj]

as the OpenSSL it has is old and has poor ECC support.

Aside, as it is no reason to have worse support for LibreSSL, but as of 0.12, ECC support in OpenSSL is no longer a requirement for Bitcoin Core.

[AliceWonderMiscreations]

@laanwj yeah, but while I can have both openssl and libressl installed at the same time I can't have the header files to both installed at the same time, at least not with both installed in /usr/include

[MarcoFalke]

Maybe add library to make clear it's not the protocol?

[MarcoFalke]

Concept ACK

[paveljanik]

what about returning _("unknown") and further simplify the code?

[jonasschnelli]

Soon, there could be an option to run without SSL (after rand and aes implementation).
Then we might want to skip/hide the SSL log info?

[paveljanik]

RET please

[paveljanik]

Debug log:

before:

2016-02-24 16:42:18 Using OpenSSL version OpenSSL 1.0.x 9 Apr 20xx

after:

2016-02-24 16:44:02 Using SSL version: OpenSSL 1.0.x 9 Apr 20xx

I propose to change the text to "Using SSL library"

[paveljanik]

Looks like this file is completely documented, can you please extend the comments? E.g. like this:

/**
  * Return the product name with the version number of the SSL library being used.
  * @note Bitcoin Core supports OpenSSL and LibreSSL.
  */

[jonasschnelli]

Thanks. Will use you comment.

[gmaxwell]

Since its no longer part of consensus does it make sense to log it? Bitcoind doesn't log the boost version or the QT version.

[MarcoFalke]

Agree with @gmaxwell. This way we could get rid of the hacky logic and also tidy up the GUI a bit, keeping in mind there are other ways to detect the library version, if this is ever desired.

[paveljanik]

I think it could be useful in the debug log. But I agree it can be removed in the UI.

[laanwj]

Closing (#7605 merged)