fixes/refactoring for building against LibreSSL #7586

pull jonasschnelli wants to merge 3 commits into bitcoin:master from jonasschnelli:2016/02/fix_openssl_libressl changing 5 files +18 −14

jonasschnelli commented at 9:32 AM on February 24, 2016: contributor

Alternative solution for #7585. Re-Enables LibreSSL compatibility. Commit count and credit goes to @AliceWonderMiscreations
fixes for building against LibreSSL 16605c6fb7
jonasschnelli added the label GUI on Feb 24, 2016
[Qt] change "Using OpenSSL version" to "Using SSL version" 12c8802c46
jonasschnelli commented at 9:48 AM on February 24, 2016: contributor

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 commented at 10:08 AM on February 24, 2016: member

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 commented at 10:12 AM on February 24, 2016: contributor

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 commented at 10:18 AM on February 24, 2016: member

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 commented at 10:21 AM on February 24, 2016: contributor

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.
Refactor SSL product and version detection and reporting 99c99d3763
jonasschnelli commented at 10:29 AM on February 24, 2016: contributor

Added a commit that refactors the SSL detection. Needs testing on CentOS (or other LibreSSL distribution)
laanwj commented at 10:30 AM on February 24, 2016: member

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 commented at 10:34 AM on February 24, 2016: contributor

@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.
jonasschnelli added the label Refactoring on Feb 24, 2016
jonasschnelli removed the label GUI on Feb 24, 2016
jonasschnelli renamed this:
~~[Qt] fix for building against LibreSSL~~
fixes/refactoring for building against LibreSSL
on Feb 24, 2016
laanwj commented at 11:25 AM on February 24, 2016: member

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 commented at 12:50 PM on February 24, 2016: contributor

@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

in src/qt/forms/debugwindow.ui:None in 99c99d3763

 114 | @@ -115,7 +115,7 @@
 115 |         <item row="4" column="0">
 116 |          <widget class="QLabel" name="label_14">
 117 |           <property name="text">
 118 | -          <string>Using OpenSSL version</string>
 119 | +          <string>Using SSL version</string>

MarcoFalke commented at 1:41 PM on February 24, 2016:

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

MarcoFalke commented at 1:41 PM on February 24, 2016: member

Concept ACK
in src/util.cpp:None in 99c99d3763
```
 853 | +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
 854 | +    return OpenSSL_version(OPENSSL_VERSION);
 855 | +#elif defined(SSLEAY_VERSION_NUMBER) || defined(OPENSSL_VERSION_NUMBER)
 856 | +    return SSLeay_version(SSLEAY_VERSION);
 857 | +#else
 858 | +    return NULL;
```
paveljanik commented at 4:35 PM on February 24, 2016:

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

jonasschnelli commented at 6:53 PM on February 24, 2016:

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?

in src/util.cpp:None in 99c99d3763

 855 | +#elif defined(SSLEAY_VERSION_NUMBER) || defined(OPENSSL_VERSION_NUMBER)
 856 | +    return SSLeay_version(SSLEAY_VERSION);
 857 | +#else
 858 | +    return NULL;
 859 | +#endif
 860 | +}

paveljanik commented at 4:38 PM on February 24, 2016:

RET please

paveljanik commented at 4:50 PM on February 24, 2016: contributor
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"

in src/util.h:None in 99c99d3763

 248 | @@ -249,4 +249,6 @@ template <typename Callable> void TraceThread(const char* name,  Callable func)
 249 |  
 250 |  std::string CopyrightHolders(const std::string& strPrefix);
 251 |  
 252 | +/* returns the current SSL product and version info */

paveljanik commented at 6:28 PM on February 24, 2016:

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-bitcoin/contributor/note/) Bitcoin Core supports OpenSSL and LibreSSL.
  */

jonasschnelli commented at 6:53 PM on February 24, 2016:

Thanks. Will use you comment.

gmaxwell commented at 6:54 PM on February 24, 2016: contributor

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.
luke-jr referenced this in commit 94d0135a68 on Feb 25, 2016
MarcoFalke commented at 2:22 PM on February 25, 2016: member

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 commented at 2:35 PM on February 25, 2016: contributor

I think it could be useful in the debug log. But I agree it can be removed in the UI.
laanwj commented at 12:56 PM on March 3, 2016: member

Closing (#7605 merged)
laanwj closed this on Mar 3, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

jonasschnelli

laanwj

AliceWonderMiscreations

Labels

Linked (view graph)

#7585 Update rpcconsole.cpp #7605 Remove openssl info from init/log and from Qt debug window