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.

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

RIPEMD160 is now available by default since OpenSSL 3.0.7. #2094

Merged
merged 2 commits into from Jun 28, 2023
Merged

RIPEMD160 is now available by default since OpenSSL 3.0.7. #2094

merged 2 commits into from Jun 28, 2023

Conversation

antonsviridenko
Copy link
Contributor

Closes #1980

@codecov
Copy link

codecov bot commented Jun 16, 2023 •
edited

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (32149be) 83.81% compared to head (e36be3e) 83.81%.

❗ Current head e36be3e differs from pull request most recent head 3a48626. Consider uploading reports for the commit 3a48626 to get more accurate results

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2094   +/-   ##
=======================================
  Coverage   83.81%   83.81%           
=======================================
  Files         161      161           
  Lines       32395    32405   +10     
=======================================
+ Hits        27151    27161   +10     
  Misses       5244     5244
Impacted Files Coverage Δ
src/lib/crypto/backend_version.cpp 73.91% <ø> (ø)
src/tests/utils-rnpcfg.cpp 100.00% <ø> (ø)
src/rnpkeys/rnpkeys.cpp 91.03% <100.00%> (+0.25%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ronaldtse
Copy link
Contributor

@antonsviridenko don't we need a specific check to the OpenSSL engine to see if RIPEMD160 is included / available?

@antonsviridenko
Copy link
Contributor Author

@ronaldtse these checks are already implemented in #1883

ronaldtse
ronaldtse approved these changes Jun 17, 2023
View reviewed changes
Copy link
Contributor

@ronaldtse ronaldtse left a comment

Choose a reason for hiding this comment

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

Thank you @antonsviridenko !

@antonsviridenko antonsviridenko requested a review from ni4 June 17, 2023 17:45
@ni4
Copy link
Contributor

ni4 commented Jun 19, 2023

@ronaldtse these checks are already implemented in #1883

@antonsviridenko Changes from #1883 doesn't check whether specific algo is included into the default or legacy crypto provider. So for this case we should check in code for OpenSSL version >= 3.0.7, using the OPENSSL_VERSION_NUMBER define.

ni4
ni4 requested changes Jun 19, 2023
View reviewed changes
Copy link
Contributor

@ni4 ni4 left a comment

Choose a reason for hiding this comment

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

We should check for OpenSSL version, please see my comment.

@antonsviridenko antonsviridenko force-pushed the antonsviridenko-1980-ripemd160-def-prov-openssl3 branch from 46b66c6 to b42cff8 Compare June 19, 2023 16:44
@antonsviridenko antonsviridenko requested a review from ni4 June 19, 2023 16:46
ni4
ni4 approved these changes Jun 19, 2023
View reviewed changes
Copy link
Contributor

@ni4 ni4 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ni4
Copy link
Contributor

ni4 commented Jun 19, 2023

@antonsviridenko Feel free to merge once CI is all green.

@antonsviridenko
Copy link
Contributor Author

Don't know what's wrong with nix.

@ribose-jeffreylau
Copy link
Contributor

Looks like we need to bump the action version to v22: cachix/install-nix-action#183 (comment)

@ribose-jeffreylau
Copy link
Contributor

@antonsviridenko CI is all green now.

@ni4 ni4 merged commit a692155 into main Jun 28, 2023
108 checks passed
@ni4 ni4 deleted the antonsviridenko-1980-ripemd160-def-prov-openssl3 branch June 28, 2023 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ronaldtse ronaldtse ronaldtse approved these changes

@ni4 ni4 ni4 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OpenSSL 3.0.7 adds RIPEMD160 to the default crypto provider
4 participants
@antonsviridenko @ronaldtse @ni4 @ribose-jeffreylau