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

Fix build with rpm >= 4.19 #1601

Merged

Conversation

pmatilai
Copy link
Member

Remove uses of long obsoleted APIs which have been removed in rpm 4.19. Besides obsolete, this is just redundant fluff as well because the surrounding code already takes care of it all.

These APIs have been obsolete for years and now removed in rpm 4.19.
Luckily the usage here is entirely redundant, rpmKeyringAddKey() checks
for duplicates and that case is even handled in this code.
…le()

These APIs have been obsolete for years and now removed in rpm 4.19.
This was always a rather strange way to go at verifying a package,
but now also wholly redundant because the signature are already verified
by rpmcliVerifySignatures().
@pmatilai
Copy link
Member Author

Hmm, actually the rpmReadPackageFile() call is also totally redundant here, because it's now only used for checking digests. Which could be just as well left for rpmcliVerifySignatures() (but currently isn't because this is supposed to be about checking signatures only).

@pmatilai
Copy link
Member Author

The last two commits can be merged if you prefer that, but left them separate for now as that may make it easier to see what's going on.

@jan-kolarik jan-kolarik self-assigned this May 11, 2023
@pmatilai
Copy link
Member Author

pmatilai commented May 11, 2023

Just FWIW, we're building the rpm 4.19 alpha in Fedora initially with these APIs brought back, so take your time reviewing this. I get that this is a somewhat touchy area.

@@ -271,7 +270,7 @@ dnf_keyring_check_untrusted_file(rpmKeyring keyring,
g_set_error_literal(error, DNF_ERROR, DNF_ERROR_INTERNAL_ERROR, "failed to set keyring");
goto out;
}
rpmtsSetVfyLevel(ts, RPMSIG_SIGNATURE_TYPE);
rpmtsSetVfyLevel(ts, RPMSIG_SIGNATURE_TYPE|RPMSIG_DIGEST_TYPE);
Copy link
Member

@jan-kolarik jan-kolarik May 15, 2023

Choose a reason for hiding this comment

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

I was reading the related PR (this comment specifically) where this change was originally proposed. So can we assume that there are already no packages with MD5 digest present on the supported systems, or should we consider them untrusted?

@jan-kolarik
Copy link
Member

I've dropped the last commit related to the discussed change for now and we could deliver it as a separate PR then.

@jan-kolarik jan-kolarik merged commit 4572ee7 into rpm-software-management:dnf-4-master May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants