archlinuxhardened / selinux

PKGBUILDs to build SELinux enabled packages for Arch Linux
146 stars 25 forks source link

Fix on pam and pambase dependency chain and build script order #61

Closed tqre closed 3 years ago

tqre commented 3 years ago

I stumbled upon this while developing a PKGBUILD which is based on the idea of using base-selinux package instead of base when installing the system. The package allows to install Arch Linux right from the start with SELinux support, provided that there is a repository that serves the packages.

The PKGBUILD I'm working on plus a public repository address: https://github.com/tqre/selinux/blob/base-selinux/base-selinux/PKGBUILD http://selinux.tqre.fi/selinux-testing

If pam-selinux is installed without pambase-selinux, login is not possible due to missing files needed with pambase package. There was some dependency and build order shuffling earlier with issue #46 . The case of upgrading pambase before pam should not occur normally, as these packages are always coupled together. I can't think of a reasonable use case where this would actually happen.

Pam should have the dependency on pambase, and when a user updates the system, both packages are updated. It used to be optional dependency, which broke my setup and made me notice this.

This is also in line with core packages: pam has a dependency on pambase, while pambase itself has no dependencies. The whole dependency chain: base -> shadow -> pam -> pambase

https://github.com/archlinux/svntogit-packages/blob/packages/pambase/trunk/PKGBUILD
https://github.com/archlinux/svntogit-packages/blob/packages/pam/trunk/PKGBUILD

The build order should also be restored in build scripts. Effectively pambase contains only text-files, which can be safely built before pam itself.

NOTE: I have only tested build_and_install_all.sh -script, as I have no use for the others atm. They are reverted to their original form as they were before pam-upgrade.

I would like to see the base-selinux adopted here, but I'll make a separate post about it.

tqre commented 3 years ago

A bit messy description I made, but the main point is that partial upgrades are not supported by Arch Linux. Hence the situation that something breaks when updating only pambase should not occur.

https://wiki.archlinux.org/index.php/System_maintenance#Partial_upgrades_are_unsupported

fishilico commented 3 years ago

Hello, I am trying to understand your issue, but am having some difficulties. So I am replying to some of your points and if I did not understand something that you said, please tell so.

A bit messy description I made, but the main point is that partial upgrades are not supported by Arch Linux. Hence the situation that something breaks when updating only pambase should not occur.

I agree for official Arch Linux packages, but not for AUR packages because "not supporting partial upgrades at all" is not compatible with makepkg -i (used to build and install packages from source), neither with most (if not all?) AUR helpers. If pambase-selinux is installed before pam-selinux 1.4.0, the system is broken because sudo, su, login... no longer work due to PAM configuration trying to use pam_faillock.so which is not installed (yet). And then, the user has to boot a rescue shell (booting with init=/bin/sh on the kernel command line, or from a LiveUSB/LiveCD...) in order to recover a working system.

Using packages built by developers fixes this (because pambase and pam are installed together), and if one day, pambase and pam gets adopted in official Arch Linux repositories, it would make sense to no longer consider partial upgrades. Until then, I consider that breaking the upgrade path due to packages being built and installed one after another is something that should be avoided.

If pam-selinux is installed without pambase-selinux, login is not possible due to missing files needed with pambase package.

This is a serous bug. Could you please report it, with logs, description of missing files, etc.?

This is also in line with core packages: pam has a dependency on pambase, while pambase itself has no dependencies.

In fact, this is misleading: pambase only contains configuration files, but requires pam>=1.4.0 in order to work properly (otherwise, libraries such as pam_faillock.so are missing). To my mind, this is like packages containing Python scripts that require some C libraries in order to work: these Python packages could be built and installed without the C libraries, but running them requires the libraries to be installed too. In fact, I do not understand why pam depends on pambase and not the other way round.

tqre commented 3 years ago

I guess I'm thinking already in terms of official support for SELinux, but as we are in AUR, you are correct: partial upgrades may happen. This is however a case where they really should be avoided at all costs, as with pam and pambase, partial upgrades can break the system, and hence they should to be coupled tightly together and reviewed good in the update process.

As I see it, the reason pam depends on pambase and not the other way around in the official repos, is that it ensures that the configuration files are always present and up to date. Also, if pam itself is updated, the dependency makes the package manager to update pambase too.

The configuration files pambase provides are essential for the whole system to function properly. I did some reading from ArchWiki: pambase is the basic Arch Linux -specific stack for PAM configuration, and it also provides configurations relating to shadow and util-linux packages, both of which we have SELinux -versions here. https://wiki.archlinux.org/index.php/PAM#PAM_base-stack

I was bit off with the dependency chain: the actual dependency comes from base -> util-linux -> pam -> pambase. And a basic Arch Linux setup should have these packages always installed, as they come as dependecies from the base package.

If pam-selinux is installed without pambase-selinux, login is not possible due to missing files needed with pambase package.

I'll try to clarify this a bit: it happened accidentally when I was testing my base-selinux -package. There is only util-linux-selinux listed, and I thought the dependency chain should ensure that all dependencies are pulled. The main reason for this to happen, is that in our repo is that pambase-selinux is currently listed as an optional dependency for pam-selinux. I can make a separate issue from this, but I have to think how to setup a proper test case without my custom base package.

Good questions, I hope you catch at least some of my reasoning. On a side note, I have no problems if you ditch my PR, as I can easily workaround this. The ultimate goal of course is to have an official support, and I'm learning SELinux policy stuff hoping to get involved on that front as well.

EDIT: typo fix

fishilico commented 3 years ago

As I see it, the reason pam depends on pambase and not the other way around in the official repos, is that it ensures that the configuration files are always present and up to date. Also, if pam itself is updated, the dependency makes the package manager to update pambase too.

Here I am learning something. I supposed that "partial upgrade is not supported" meant that ugrading pam without upgrading pambase was not supported. But if an update to both packages it available and I choose to update pam using pacman -Sy pam (not pacman -Syu), I thought that pambase was not updated: one really needs to use pacman -Syu to upgrade the packages and get a "supported system". The fact that pambase is a dependency to pam does not play a role there. This is a supposition (I did not take time to test this recently): did you test that thanks to the dependency, if pacman -Sy pam upgrades pam, pambase is also upgraded?

I'll try to clarify this a bit: it happened accidentally when I was testing my base-selinux -package. There is only util-linux-selinux listed, and I thought the dependency chain should ensure that all dependencies are pulled.

Like an Arch Linux developer stated on https://bugs.archlinux.org/task/39767 (about the cyclic dependency that have been present between util-linux and systemd since 2014): "It is called bootstraping a system.". Clearly, the reasoning between official packages and the AUR is different, and all the more when considering essential packages such as pambase and pam.

To my mind, if one day libselinux becomes part of the core repository and pam depends on it, Arch Linux developers will be free to choose whatever they want to help users bootstrap their systems. Until then, the reasoning behind packaging decisions of -selinux packages need to follow rules related to the AUR, like "not breaking the system during the upgrade process, where packages are built one after another".

For your base-selinux package, does something prevent you from listing pam-selinux and pambase-selinux as dependencies?

tqre commented 3 years ago

did you test that thanks to the dependency, if pacman -Sy pam upgrades pam, pambase is also upgraded?

I was assuming -Suy flags, so my statements are plainly wrong. The packages don't get updated automatically due to another package getting update.

For your base-selinux package, does something prevent you from listing pam-selinux and pambase-selinux as dependencies?

Nothing prevents this, and I'm fine with this.

Just an idea for the partial upgrade -problem: instead of messing with dependencies, could there be a simple conflicts=('pam-selinux>=1.4.0') entry? I still feel the dependencies with core packages are well formed, and we should follow them.

I'll see if I can make the conflicts entry work.

EDIT: reopened PR, as the conflicts entry just adds to this, and should solve the partial upgrade problem.

tqre commented 3 years ago

Digging into the past a bit: in #46 fish wrote:

Before upgrading, I wanted to solve a potential issue for users upgrading that install pambase-selinux 20200721.1-2 before pam-selinux 1.4.0-3, which breaks the system (and makes sudo and su no longer works).

To summarize this current PR: the conflicts -addition solves this particular problem, and basically the whole PR reverts the dependency chain manipulations that were made earlier.

@fishilico: sorry for stressing my point, but for me it makes sense to comply with the core packages all the way if we are to have official support one day. But of course as the maintainer you make the decisions as you see fit.

I'll can set up a VM that faces this issue if need be, but depending on how we proceed, I'll move on anyway to sneak in the base-selinux -package :)

fishilico commented 3 years ago

Indeed I haven't thought about using conflict rules to prevent from installing pambase-selinux before pam-selinux. But if I had done this, ./build_and_install_all.sh would have been broken to upgrade the system...

Anyway, I agree with merging your PR on the long term (in order to avoid diverging from the base packages too much), once enough users have had chance to upgrade smoothly. This means "at least some months"... which already occurred. So I am planning to merge your PR (with a fixed conflicts comparison if I misunderstood something) right before the next release of pambase-selinux or pam-selinux packages (when pambase or pam will be upgraded).

tqre commented 3 years ago

Sorry, didn't have the time to test this yet, just got a replacement keyboard for my laptop (again). And yes, it should be <1.4.0 as you said.

Nice to hear that you think alike. I'll start preparing to get the base packages ready and also ArchWiki instructions related to them. I'll let you know. 2 public repositories in the making...

tqre commented 3 years ago

I fixed the conflicts line and tested building with this PR. Everything seems to work. I'll squash the commits later today. EDIT: I'm doing something wrong with my squashes... and lost the .SRCINFO commit...

tqre commented 3 years ago

I have succesfully messed this up failing to squash and rebase the branch. I'll make a new branch along with a new PR.