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

Stop using sha1 (for dnf_sack_get_rpmdb_version) #1373

Closed
cgwalters opened this issue Nov 2, 2021 · 5 comments
Closed

Stop using sha1 (for dnf_sack_get_rpmdb_version) #1373

cgwalters opened this issue Nov 2, 2021 · 5 comments

Comments

@cgwalters
Copy link
Collaborator

Moving this from https://bugzilla.redhat.com/show_bug.cgi?id=1936664

Basically https://github.com/rpm-software-management/libdnf/blob/dnf-4-master/libdnf/utils/crypto/sha1.hpp is (AFAICS) only used by this code but AFAICS we could switch to e.g. sha256 here, at just the cost of computing a different version.

(It seems like this code should really live in librpm)
@j-mracek
Copy link
Contributor

j-mracek commented Nov 8, 2021

We have to change the approach how to check rpmdb version. In DNF5 we will use direct information from RPM whether rpmdb changed or not. Therefore I recommend to skip movement from sha1 to sha256 to not use our own calculation at all.

@adelton
Copy link
Contributor

adelton commented Jan 21, 2022

Note that with latest OpenSSL 3 builds, the unconditional use of SHA-1 seems to cause segfault: https://bugzilla.redhat.com/show_bug.cgi?id=2043476.

@beldmit
Copy link

beldmit commented Jan 21, 2022

In fact, the presence of any algorithm obtained from OpenSSL should be checked in run-time.

cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Feb 8, 2022
@cgwalters
db version: Switch to using SHA-256
8dc1dad 
Motivated by a RHEL-initiated effort to change or drop code using
SHA-1.

This code is old, has no unit tests, and I am not aware of anything
actually using it.  We could probably just delete it.

That said, let's take a middle ground of changing it to use SHA-256.

I believe the higher level logic here duplicates that of libdnf;
xref rpm-software-management/libdnf#1373

So we should be using that API anyways.
cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Feb 8, 2022
@cgwalters
db version: Switch to using SHA-256
c81a3f8 
Motivated by a RHEL-initiated effort to change or drop code using
SHA-1.

This code is old, has no unit tests, and I am not aware of anything
actually using it.  We could probably just delete it.

That said, let's take a middle ground of changing it to use SHA-256.

I believe the higher level logic here duplicates that of libdnf;
xref rpm-software-management/libdnf#1373
We should be using that API anyways; but that's a larger change,
I'm just trying to get SHA-1 out for now.
@cgwalters cgwalters mentioned this issue Feb 8, 2022
Merged
@pmatilai
Copy link
Member

I think this can be closed, as the "offending" code has already been removed and replaced (6fd718b and 94ab063) and rpm itself has switched to SHA256 for the cookie value.

@j-mracek
Copy link
Contributor

Closing as resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@cgwalters @adelton @pmatilai @j-mracek @beldmit