Skip to content
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.

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

Set OpenSSL 1.1.1 as the minimum version #3935

Closed
wants to merge 1 commit into from

Conversation

ximinez
Copy link
Collaborator

@ximinez ximinez commented Sep 27, 2021

High Level Overview of Change

CMake config defined OpenSSL 1.0.2 as the minimum, even though building rippled would fail with that version for about 6 months. Update it to the newest commonly available version, 1.1.1.

3.0.0 was released a few weeks ago, but I'm not making that the minimum until it has had more time to be used and "shake out" issues, and to be included in more linux distros. However, I updated the Windows build instructions to recommend 3.0.0, which seems to work well there.

Edit: Narrator: 3.0.0 didn't work well there.
I changed the Visual Studio README.md to recommend 1.1.1L.

Context of Change

Builds fail with older versions of OpenSSL. Because OpenSSL is security sensitive, we should keep up our requirements to match the latest version.

OpenSSL is referenced in two places in the CMake config. I don't know why that is, and am leaving that question as outside the scope of this PR.

@ximinez ximinez added Build System Dependencies Issues associated with 3rd party dependencies (RocksDB, SQLite, etc) Documentation README changes, code comments, etc. labels Sep 27, 2021
Copy link
Collaborator

@seelabs seelabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I don't have a window's system to tests this with, but the code changes look good. Thanks for doing this.

@ximinez
Copy link
Collaborator Author

ximinez commented Sep 27, 2021

@seelabs Sorry, I was premature. My quick checks with 3.0.0 looked good, but the longer checks dug up some deprecated functions in 3.0.0 that prevent rippled from building, so I changed the VS README back to 1.1.1L. I'm double-checking with that version now, but since I had previously been using 1.1.1K, I don't anticipate any problems.

@ximinez ximinez changed the title Set OpenSSL 1.1.1 as the minimum version, recommend 3.0.0 for Windows Set OpenSSL 1.1.1 as the minimum version Sep 27, 2021
Copy link
Collaborator

@legleux legleux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 Tested on windows too

@ximinez ximinez added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Oct 1, 2021
@manojsdoshi manojsdoshi mentioned this pull request Oct 6, 2021
* Also clean up some formatting in the Windows instructions
@manojsdoshi
Copy link
Contributor

"Merged as part of #3948"

@manojsdoshi manojsdoshi closed this Oct 7, 2021
@ximinez ximinez deleted the openssl1.1.1 branch October 7, 2021 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build System Dependencies Issues associated with 3rd party dependencies (RocksDB, SQLite, etc) Documentation README changes, code comments, etc. Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants