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

spec: Adjust selinux requirement for stable upstream releases #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tinez
Copy link
Member

@tinez tinez commented Aug 30, 2022

We make official upstream releases from packages built either in CBS or
COPR. Neither of these environments offers any of the upstream clones of
RHEL (AlmaLinux, Rocky Linux) as chroot environments - only CentOS
Stream is available. This fact, combined with how %{?selinux_requires}
macro works, makes our lives complicated.

As part of selinux-policy installation we get
a /usr/lib/rpm/macros.d/macros.selinux-policy file that has among
others:

%_selinux_policy_version 36.14-1.fc36
...
%selinux_requires \
Requires: selinux-policy >= %{_selinux_policy_version} \
...

The %_selinux_policy_version expands to the version of the
selinux-policy package that is installed in the environment where
the RPM is built
. On CentOS Stream these RPMs are usually slightly
ahead than their counterparts on the stable releases.
Since we only have CentOS Stream chroots available for building RPMs,
when we try to install such a package on a stable release, dnf
resolves the conflicts by simply picking a package version that's old
enough to meet the selinux-policy requirement.

Ideally, packages for stable releases should be built in chroots based
on stable releases. Since we don't have that option for now, let's define
the %_selinux_policy_version by ourselves to a value that matches
the version from the latest stable release.

This patch should not affect deployments on CentOS Stream 9 in a any
way. It may affect deployments on stable distros, as the SELinux policy
compiled with a newer version of 'selinux-policy' package may not
be backwards compatible with an older version. In current tests however
everything worked fine.

An alternative for this approach would be to force-install an older
version of 'selinux-policy' package in the environment where
'ovirt-console' is built. This might not be possible due to some
'ovirt-vmconsole's build dependencies requiring a newer version.
Even if it would work today, it could still be broken by an update
of the dependent packages.

Signed-off-by: Marcin Sobczyk [email protected]

@tinez tinez requested a review from mz-pdm as a code owner August 30, 2022 10:18
@tinez tinez requested a review from michalskrivanek August 30, 2022 10:19
@tinez tinez force-pushed the adjust-selinux-requirement branch from c0bf711 to 652ca76 Compare August 30, 2022 10:44
@mz-pdm
Copy link
Member

mz-pdm commented Aug 31, 2022

What does the build command

make -f /usr/share/selinux/devel/Makefile -C selinux-ovirt_vmconsole

do? Can it happen that something gets processed/created and put into the rpm that is compatible only with the new policy and then it would break when installed on a system with an older policy (not permitting something that should be permitted, permitting something that shouldn't be permitted, or not working at all)?

@tinez
Copy link
Member Author

tinez commented Aug 31, 2022

What does the build command

make -f /usr/share/selinux/devel/Makefile -C selinux-ovirt_vmconsole

do?

Not sure if this is the answer you expected, but it compiles the selinux policy.

Can it happen that something gets processed/created and put into the rpm that is compatible only with the new policy and then it would break when installed on a system with an older policy (not permitting something that should be permitted, permitting something that shouldn't be permitted, or not working at all)?

This is a bigger problem unfortunately and the PR is (hopefully) a temporary workaround until we have chroots with stable releases or a broader solution. Any binary blob is prone to this issue - using newer packages for building and then installing on a release with older, stable packages may not work as expected.

@tinez
Copy link
Member Author

tinez commented Aug 31, 2022

Maybe another comment here is needed - this was one of two things that was blocking basic suite run from being successful on Alma 9 images, so things don't look so dire either ;)

@mz-pdm
Copy link
Member

mz-pdm commented Aug 31, 2022

Any binary blob is prone to this issue - using newer packages for building and then installing on a release with older, stable packages may not work as expected.

And this is what dependencies are for. Before intentionally breaking them for the purpose of compiling once and then running on different incompatible distributions:

  • The possible negative impact should be explained in the commit message.

  • As well as why we cannot provide the required versions of selinux-policy packages in upstream repos to satisfy the real dependencies.

  • A TODO should be added to the spec file, briefly explaining the broken dependency and the need to remove it once proper builds are available.

In practice, the overall impact of this change may be positive:

  • I assume it's likely to work on the incompatible distributions.

  • It should still work reliably on Stream as long as the whole installed OS is kept up-to-date.

So I'm not against it if there is no better solution at the moment.

@tinez tinez force-pushed the adjust-selinux-requirement branch from 652ca76 to 897251e Compare September 1, 2022 08:36
@tinez
Copy link
Member Author

tinez commented Sep 1, 2022

* The possible negative impact should be explained in the commit message.
* As well as why we cannot provide the required versions of selinux-policy packages in upstream repos to satisfy the real dependencies.
* A TODO should be added to the spec file, briefly explaining the broken dependency and the need to remove it once proper builds are available.

I've added a TODO and added 2 paragraphs in the commit message.

@mz-pdm
Copy link
Member

mz-pdm commented Sep 1, 2022

This patch should not affect deployments on CentOS Stream 9 in a any way

This is a bold statement, basically telling the dependency on the newer version is completely unnecessary. But if you stay behind it and are ready to handle the contingent problems... :-)

An alternative for this approach would be to force-install an older version of 'selinux-policy' package in the environment where 'ovirt-console' is built.

Oh, no, what I meant with

why we cannot provide the required versions of selinux-policy packages in upstream repos to satisfy the real dependencies.

was wondering whether there is an option to provide a newer selinux-policy for those stable systems in oVirt repos when needed. Not changing the build environment.

We make official upstream releases from packages built either in CBS or
COPR. Neither of these environments offers any of the upstream clones of
RHEL (AlmaLinux, Rocky Linux) as chroot environments - only CentOS
Stream is available. This fact, combined with how `%{?selinux_requires}`
macro works, makes our lives complicated.

As part of `selinux-policy` installation we get
a `/usr/lib/rpm/macros.d/macros.selinux-policy` file that has among
others i.e.:

```
%_selinux_policy_version 36.14-1.fc36
...
%selinux_requires \
Requires: selinux-policy >= %{_selinux_policy_version} \
...
```

The `%_selinux_policy_version` expands to the version of the
`selinux-policy` package *that is installed in the environment where
the RPM is built*. On CentOS Stream these RPMs are usually slightly
ahead than their counterparts on the stable releases.
Since we only have CentOS Stream chroots available for building RPMs,
when we try to install such a package on a stable release, 'dnf'
resolves the conflicts by simply picking a package version that's old
enough to meet the 'selinux-policy' requirement.

Ideally, packages for stable releases should be built in chroots based
on stable releases. Since we don't have that option for now, let's define
the `%_selinux_policy_version` by ourselves to a value that matches
the version from the latest stable release.

This patch should not affect deployments on CentOS Stream 9 in a any
way. It may affect deployments on stable distros, as the SELinux policy
compiled with a newer version of 'selinux-policy' package may not
be backwards compatible with an older version. In current tests however
everything worked fine.

An alternative for this approach would be to force-install an older
version of 'selinux-policy' package in the environment where
'ovirt-console' is built. This might not be possible due to some
'ovirt-vmconsole's build dependencies requiring a newer version.
Even if it would work today, it could still be broken by an update
of the dependent packages.

Signed-off-by: Marcin Sobczyk <[email protected]>
@sandrobonazzola sandrobonazzola force-pushed the adjust-selinux-requirement branch from 897251e to 3057637 Compare November 29, 2023 08:30
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.

2 participants