8000 [Upstream] [Consensus] Allow for compilation with OpenSSL 1.1 by lyricidal · Pull Request #62 · PRCYCoin/PRCYCoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Upstream] [Consensus] Allow for compilation with OpenSSL 1.1 #62

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 22, 2021
Merged

[Upstream] [Consensus] Allow for compilation with OpenSSL 1.1 #62

merged 1 commit into from
Jun 22, 2021

Conversation

lyricidal
Copy link
@lyricidal lyricidal commented Jun 22, 2021 •
edited
Loading

What prompted me to address the situation is related to issue PIVX-Project/PIVX#149
Let's give a bit of context:
In bitcoin, openssl was used in consensus critical code. Following the infamous heartbleed vulnerability discovered in openssl code, libressl was created as a fork to refactor and improve ssl code. To this day, libressl is used as default instead of openssl in the os openBSD. As far as crypto are concerned: the openssl hotfix was integrated into bitcoin promptly. The change in consensus code triggered a fork. bip 66 was then decided, removing openssl code from consensus critical area and using custom implementation by core team instead. This bip was backported in dash commit 3d004ab which we inherited. So good so far.
The thing is, before this bip a limitation was put into the autoconf to avoid compiling consensus critical code with the new lib libressl. We directly inherited this limitation from bitcoin core 0.10. It was removed from bitcoin core in commit a3874c7. Too late for us. Since then, anyone wanting to compile PIVX with libressl will get the warning as seen in PIVX-Project/PIVX#149. And this has never been removed. But this warning is not cleanly looking for libressl, it is using a workaround, testing the absence of a specific function existing in openssl 1.0 but removed in both libressl and openssl 1.1 prompting you to get a libressl warning if you are using openssl 1.1. The simple workaround would be to altogether remove the warning, as done by bitcoin.
But the thing which leaves me wondering in this situation is that when implementing libzerocoin, we came to depend on openssl again. Depend so much that we can't compile with openssl 1.1. Even by ignoring the warning thanks to the --with-libressl argument. This pull request aim to fix this compilation problem. I am not fully aware of the internal workings of the zerocoin implementation but surely, since it's burning and creating coins, it is influencing consensus.

My advice would be: after merging this, either try to totally remove openssl dependency from any consensus critical code (especially related to zerocoin). If not possible, and anyway, as a short term fix: change the alert message and argument when compiling as to include openssl 1.1 beside libressl as potentially consensus breaking. This would allow to quickly close issue PIVX-Project/PIVX#149

About testing: I compiled and ran unit tests on debian with both openssl 1.0 and 1.1. I did not test libressl and I'd prefer if someone is able to try it on openbsd. I also minted and spent some zPIV on testnet without problem.

from PIVX-Project/PIVX#447

@lyricidal
[Consensus] Allow for compilation with OpenSSL 1.1
c937ef7 
Code is largely inspired from str4d/zcash@f3f600a

Squashing commits...
-Fix double declaration of operator< in bn.h
-Ensured compatibility with openssl pre 1.1
-Fix a depreciation with OpenSSL 1.1
-Compatible with OpenSSL below 1.1
-Fix compilation openssl 1.0 when using gui
-Fix typo for compilation
-[Compilation] Acknowledge ability to compile against OpenSSL 1.1wq
@lyricidal lyricidal requested review from lopeed and anprdev June 22, 2021 18:08
@lopeed lopeed merged commit ffe574c into develop Jun 22, 2021
@lopeed lopeed deleted the upstream-openssl_1.1 branch June 22, 2021 18:13
@lyricidal lyricidal mentioned this pull request Jul 22, 2021
Merged
@lyricidal lyricidal added the Upstream Upstream backports/fixes label Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lopeed lopeed Awaiting requested review from lopeed

@anprdev anprdev Awaiting requested review from anprdev

Assignees
No one assigned
Labels
Upstream Upstream backports/fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lyricidal @lopeed
0