Skip to content

apt-devel to BuildRequires; apt-libs to Requires #1932

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

Merged
merged 1 commit into from
May 23, 2023

Conversation

candrews
Copy link
Contributor

@candrews candrews commented Feb 14, 2023

  • Using openscap to scan a system that use apt (such as Debian or Ubuntu) returns incomplete results. This warning is logged: OVAL object 'dpkginfo_object' is not supported.
    The problem is that openscap is built without apt support.
    The fix, implemented in this commit, is to add apt as a build requirement.

@candrews candrews changed the title Add apt-libs as a build dependency Add apt-libs and opendbx as rpm dependencies Feb 14, 2023
@jan-cerny
Copy link
Member

@candrews Thank for opening this PR.

Can you help us understand the goal of this change?

AFAIU you want our CI to build OpenSCAP with dpkginfo and sql57 probe so that it's tested in the GitHub actions. Is my understanding correct? That has 2 problems: 1. It doesn't build on CentOS. 2. there are no tests for the dpkginfo probe, so it would only test whether it builds correctly.

@jan-cerny jan-cerny self-assigned this Feb 20, 2023
@candrews
Copy link
Contributor Author

AFAIU you want our CI to build OpenSCAP with dpkginfo and sql57 probe so that it's tested in the GitHub actions. Is my understanding correct?

Yes. My other motivation is to provide a better reference for packagers to use. For example, right now, Fedora's openscap package is missing the sql57 and dpkginfo dependencies, presumably because the packager used the openscap.spec from the project as a reference. By improving the reference, this kind of problem can hopefully be avoided going forward.

That has 2 problems: 1. It doesn't build on CentOS.

Why not? Is that perhaps something that could be fixed?

  1. there are no tests for the dpkginfo probe, so it would only test whether it builds correctly.

That's better than nothing :)

@jan-cerny
Copy link
Member

Thanks for the explanation!

presumably because the packager used the openscap.spec from the project as a reference. By improving the reference, this kind of problem can hopefully be avoided going forward.

Actually it was the opposite, we created the upstream openscap.spec based on the Fedora spec file. We haven't add these 2 probes to the Fedora package because we don't see any benefits for the users - dpkginfo is for Debian/ubuntu and sql57 doesn't have any use case in security hardening. Do you imagine any use case?

Why not? Is that perhaps something that could be fixed?

CentOS doesn't provide the apt-devel and opendbx-devel packages. You can try to fix the broken CI it by wrapping the Requires and BuildRequires entry in the spec file by a condition, eg.

%if 0%{?centos}
Requires:       apt-libs
Requires:       opendbx
%endif

@candrews
Copy link
Contributor Author

dpkginfo is for Debian/ubuntu and sql57 doesn't have any use case in security hardening. Do you imagine any use case?

These dependencies enable probes which will impact the results, as far as I understand. For example, using openscap built without apt support means it won't provide full results when scanning targets that use apt; this happens for example when running openscap on a Fedora system to scan an Ubuntu docker image.

You can try to fix the broken CI it by wrapping the Requires and BuildRequires entry in the spec file by a condition

That's great, I just updated this PR with a commit implementing that idea. Thank you!

@evgenyz
Copy link
Contributor

evgenyz commented Feb 28, 2023

The dpkg probe is being built within the Ubuntu gating job. I'd be better to actually write a probe test that would be executed there.

The sql probe does not have a test and does not have any meaningful use cases (lack of auth, compatibility problems etc), it is pretty much a dead-born OVAL test.

In my opinion this change would only increase CI overhead without any meaningful benefits. Unless, of course, we decide to ship Fedora package with dpkg probe enabled.

@candrews
Copy link
Contributor Author

Unless, of course, we decide to ship Fedora package with dpkg probe enabled.

I believe the Fedora package absolutely should be shipping with the dpkg probe enabled.

Right now, if you run oscap-podman (for example) on a Debian image from a Fedora system, you get misleading results due to the lack of the dpkg probe.

@candrews
Copy link
Contributor Author

candrews commented Mar 2, 2023

I submitted a PR against the Fedora packaging to add these dependencies: https://src.fedoraproject.org/rpms/openscap/pull-request/19

@evgenyz
Copy link
Contributor

evgenyz commented Mar 2, 2023

Okay, so, after internal meeting of Fedora packagers of openscap, the consensus is:

  1. It makes sense to package dpkg probe by default (scap-security-guide includes Ubuntu profiles, so it is absulutely logical to support dpkg probe in the scanner).

  2. It does not make any sense to package sql probe as it is basically dysfunctional. We'll take measures to deprecate it and maybe even remove it in a future API-breaking version of the scanner.

@candrews
Copy link
Contributor Author

candrews commented Mar 2, 2023

That sounds great!

Here's a PR to the fedora packaging: https://src.fedoraproject.org/rpms/openscap/pull-request/20

And I've updated this PR accordingly.

@candrews candrews changed the title Add apt-libs and opendbx as rpm dependencies apt-devel to BuildRequires; apt-libs to Requires Mar 2, 2023
@evgenyz
Copy link
Contributor

evgenyz commented Mar 3, 2023

I'll get back to it when GH CI will recover from its coma.

@candrews
Copy link
Contributor Author

Can this PR be merged? Is there assistance I could provide to help it progress?

@evgenyz
Copy link
Contributor

evgenyz commented May 23, 2023

Can this PR be merged? Is there assistance I could provide to help it progress?

The CentOS Stream 9 test is still failing. I think we should only enable it for Fedora.

@candrews
Copy link
Contributor Author

The CentOS Stream 9 test is still failing. I think we should only enable it for Fedora.

I thought I did that already, using the approach covered in #1932 (comment)

However, I think that advise was slightly flawed - I think it's backwards. Instead of:

%if 0%{?centos}
Requires:       apt-libs
%endif

I think it should be:

%if 0%{?fedora}
Requires:       apt-libs
%endif

I've updated this PR accordingly - how's it look now?

Using openscap to scan a system that use apt (such as Debian or Ubuntu) returns incomplete results. This warning is logged:
OVAL object 'dpkginfo_object' is not supported.

The problem is that openscap is built without apt support.

The fix, implemented in this commit, is to add apt as a build requirement.

Only Fedora provides the apt-devel package.
Therefore it is only included when building for Fedora.
@evgenyz
Copy link
Contributor

evgenyz commented May 23, 2023

The testing-farm:fedora-rawhide-x86_64 test failure is irrelevant. LGTM. Thanks!

@evgenyz evgenyz merged commit 94cd797 into OpenSCAP:maint-1.3 May 23, 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.

3 participants