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: add case statement for setting sssd capabilities based on Fedora version #9

Merged
merged 3 commits into from
Jan 4, 2025

Conversation

ABotelho23
Copy link
Contributor

To fix ublue-os/bazzite#2030 without breaking previous SSSD versions capabilities.

I'm not sure if there's a better place to put functions, but that could certainly be changed. I created a function in case the version comparison functionality might be useful for other packages in the future.

Tested locally on my machine, but tough for me to test running in the pipelines.

@antheas
Copy link
Collaborator

antheas commented Jan 2, 2025

check seems fragile. If there is a previous rc with 1 it will break. Can you do a fedora check instead and check for f40? Simpler and does the same

@alexth4ef9
Copy link

alexth4ef9 commented Jan 2, 2025

sssd is not depending on fedora version. (only the change from sssd 2.9.x 2.10.0 aligns with fedora version)

f40: sssd 2.9.x no capabilities (sssd running as root)
f41 till mid december: sssd 2.10.0 current capabilties (in rechunk) https://sssd.io/release-notes/sssd-2.10.0.html (sssd switched to non-root user)
f41 after mid december: sssd 2.10.1 new capabilties https://sssd.io/release-notes/sssd-2.10.1.html

rechunk has the rpm database available?

a possible way to get the required capabilties:

pull down all sssd rpms (dnf probably needs to point to the right database (chroot)?)

dnf download sssd*

for each downloaded rpm (sssd-krb5-common-2.10.1-1.fc41.x86_64.rpm is an example one of these) rpmrebuild package is required

rpmrebuild --package --notest-install -s - sssd-krb5-common-2.10.1-1.fc41.x86_64.rpm | grep '%caps(' >>sssd-caps.txt

this results in sssd-caps.txt

%attr(0750, root, sssd) %caps(cap_dac_read_search,cap_setgid,cap_setuid=p) "/usr/libexec/sssd/krb5_child"
%attr(0750, root, sssd) %caps(cap_dac_read_search=p) "/usr/libexec/sssd/ldap_child"

this can parsed and applied to the files that exists

@antheas
Copy link
Collaborator

antheas commented Jan 2, 2025

(only the change from sssd 2.9.x 2.10.0 aligns with fedora version)

Then yes. Only apply the workaround if not at F40. Its ok, we can retain increased capabilities for a while even as the new version comes out

@ABotelho23
Copy link
Contributor Author

I'm not sure what you mean. You want to only apply the new capabilities and pretend 2.10.0 doesn't exist?

@antheas
Copy link
Collaborator

antheas commented Jan 2, 2025

Pretend 2.10.1 does not exist and apply 2.10.0 if we are not on f40. Very simple very robust.

We dont care much about older images

@ABotelho23
Copy link
Contributor Author

ABotelho23 commented Jan 2, 2025

sssd is failing at the moment. The capabilities need to be updated to match what upstream is asking for. The current images have the capabilities required for 2.10.0, so something needs to change. I'm also not sure it's wise to have more capabilities than necessary.

We could apply the 2.10.1 capabilities if F41+, but I don't really get how applying 2.10.0 capabilities fixes anything.

@antheas
Copy link
Collaborator

antheas commented Jan 3, 2025

If 2.10.1 is now released we could do that but as the article explains 2.10.0 is a superset, which means it catches both 2.10.0 and 2.10.1 and I already have those capabilities

So do that check and we will fix to 2.10.1 later.

@ABotelho23
Copy link
Contributor Author

Did you go over ublue-os/bazzite#2030 ? That's why sssd is broken. I currently cannot sign into FreeIPA accounts on my Bluefin laptop at the moment because the stable images have received 2.10.1.

I was hoping for a more generic solution (not one linked to specific Fedora versions), but I can update the PR to read from the tree's /etc/os-release and apply capabilities assuming the latest sssd package for each major release (40 vs 41). Give me a few minutes to update the PR.

@ABotelho23
Copy link
Contributor Author

There we go, I've updated my PR.

How does that look? It should allow the addition of more Fedora versions and their corresponding sssd capabilities in the future.

@ABotelho23 ABotelho23 changed the title fix: check SSSD version and apply the correct set of capabilities fix: add case statement for setting sssd capabilities based on Fedora version Jan 3, 2025
@tbelway
Copy link

tbelway commented Jan 3, 2025

Just piping in my two cents, I feel like checking the sssd version is more flexible given that the current change occurred within the same Fedora version. If sssd has another version release that once again changes the capabilities within the same Fedora version, how is compatibility across sssd updates retained otherwise?

In it's current state, freeipa login is broken for myself as well and I have to boot up the prior snapshot just to log in... It would be nice if the sssd version capabilities were specified instead of distro version iteration in order to capture any future potential changes. I would argue this gives a far more flexible and robust solution...

If 2.10.1 is now released we could do that but as the article explains 2.10.0 is a superset, which means it catches both 2.10.0 and 2.10.1 and I already have those capabilities

So do that check and we will fix to 2.10.1 later.

I don't really understand the idea of "it's broken now, so we won't fix it until later", unless I am misunderstanding something?

@ABotelho23
Copy link
Contributor Author

Hey @antheas, would it be possible to merge this? sssd remains broken on uBlue images on Fedora 41 until this can be merged and new images built.

@antheas
Copy link
Collaborator

antheas commented Jan 4, 2025

There, that wasnt that hard

@antheas antheas merged commit bde873c into hhd-dev:master Jan 4, 2025
@ABotelho23
Copy link
Contributor Author

No offense, but what you've committed isn't great in my opinion. It'll have to be completely reworked for Fedora 42, while what I had would have just required a new case in the case statement. It also has a pointless extra grep (you could have just grepped for VERSION_ID=40), and checks if a string is set to determine that the Fedora release is 40. It's pretty discouraging that you felt the need to replace a forward-thinking PR for something that will have to be replaced entirely in less than 6 months.

@antheas
Copy link
Collaborator

antheas commented Jan 4, 2025

The whole script will be removed in the next refactor in a few months as upstream changes should mean its no longer necessary

I just don't have time to get to it atm

It just needs to work for now and do so reliably. Your script had some failure points that I wanted to avoid such as assuming os release exists and is filled in

Sorry for wasting your time

@antheas
Copy link
Collaborator

antheas commented Jan 4, 2025

Also, only f40 has no capabilities and f39 is not supported. F42 will have them so this script is evergreen as it only checks for f40, which is the only release that will ever have the issue at the point of release

@tbelway
Copy link

tbelway commented Jan 6, 2025

Also, only f40 has no capabilities and f39 is not supported. F42 will have them so this script is evergreen as it only checks for f40, which is the only release that will ever have the issue at the point of release

Hey @antheas , sorry to bother but it looks like it didn't quite resolve the issue, the capabilities did not get updated: ublue-os/bazzite#2030

Summarized here:
Rechunk https://github.com/hhd-dev/rechunk/releases/tag/v1.0.2 should have the changes, and https://github.com/ublue-os/bazzite/releases/tag/41.20250106 shows rechunk is now at latest:

# SSSD fails to start
Jan 06 10:49:41 bdt.ipa.belway.me systemd[1]: Starting sssd.service - System Security Services Daemon...
Jan 06 10:49:41 bdt.ipa.belway.me sssd[2841]: Starting up
Jan 06 10:49:41 bdt.ipa.belway.me sssd_be[2871]: Starting up
Jan 06 10:49:41 bdt.ipa.belway.me sssd_be[2874]: Starting up
Jan 06 10:49:43 bdt.ipa.belway.me sssd_be[3052]: Starting up
Jan 06 10:49:47 bdt.ipa.belway.me sssd_be[3072]: Starting up
Jan 06 10:49:47 bdt.ipa.belway.me sssd[2841]: Exiting the SSSD. Could not restart critical service [ipa.belway.me].
Jan 06 10:49:47 bdt.ipa.belway.me systemd[1]: sssd.service: Main process exited, code=exited, status=1/FAILURE
Jan 06 10:49:47 bdt.ipa.belway.me systemd[1]: sssd.service: Failed with result 'exit-code'.
Jan 06 10:49:47 bdt.ipa.belway.me systemd[1]: Failed to start sssd.service - System Security Services Daemon.

# Current build
OSTREE_VERSION='41.20250106.0'
BUILD_ID="Stable (F41.20250106)"
BOOTLOADER_NAME="Bazzite Stable (F41.20250106)"

# Capabilities
getcap /usr/libexec/sssd/*
/usr/libexec/sssd/krb5_child cap_chown,cap_dac_override,cap_setgid,cap_setuid=ep
/usr/libexec/sssd/ldap_child cap_chown,cap_dac_override,cap_setgid,cap_setuid=ep
/usr/libexec/sssd/selinux_child cap_setgid,cap_setuid=p
/usr/libexec/sssd/sssd_pam cap_dac_read_search=p

@antheas
Copy link
Collaborator

antheas commented Jan 6, 2025

They're using a bot to do the update

And I think it didn't update the container.

@tbelway
Copy link

tbelway commented Jan 7, 2025

They're using a bot to do the update

And I think it didn't update the container.

Is this something we should open a new issue on bazzite, or given that you are a contributor there you can take a quick look?

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.

SSSD binaries capabilities have changed
4 participants