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

ipa-client-install: enable SELinux for SSSD #6978

Closed
wants to merge 1 commit into from

Conversation

abbra
Copy link
Contributor

@abbra abbra commented Aug 29, 2023

For passkeys (FIDO2) support, SSSD uses libfido2 library which needs access to USB devices. Add SELinux booleans handling to ipa-client-install so that correct SELinux booleans can be enabled and disabled during install and uninstall. Ignore and record a warning when SELinux policy does not support the boolean.

Fixes: https://pagure.io/freeipa/issue/9434

@abbra abbra added needs review Pull Request is waiting for a review ipa-4-11 Mark for backport to ipa 4.11 labels Aug 30, 2023
@f-trivino f-trivino self-requested a review August 30, 2023 13:06
Copy link
Contributor

@f-trivino f-trivino left a comment

Choose a reason for hiding this comment

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

I just have one concern, added a comment, otherwise LGTM.
This addresses well the passkeys on client side. Since this is the client we don't have issues with upgrade path.


try:
tasks.set_selinux_booleans(constants.SELINUX_BOOLEAN_SSSD,
backup_state)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be statestore.backup_state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. The task expects a function that is provided by the installation service which calls into statestore.backup_state(). See ipaserver/install/service.py:Service.backup_state():

   def backup_state(self, key, value):
        self.sstore.backup_state(self.service_name, key, value)

@rcritten
Copy link
Contributor

rcritten commented Sep 1, 2023

The selinux state is not restored on uninstall. Since the new SSSD boolean wasn't available I picked a random one that was off, in this case xdm_write_home.

Uninstall fails because the [selinux] section remains in sysrestore.state:

[selinux]
xdm_write_home = off

@abbra
Copy link
Contributor Author

abbra commented Sep 1, 2023

thanks. I added statestore.delete_state() after resetting SELinux booleans.

@rcritten
Copy link
Contributor

rcritten commented Sep 1, 2023

What we do in other places that SELinux is restored is this:

this example is from ipa_client_samba.py.

    # Restore the state of affected selinux booleans
    boolean_states = {}
    for usecase in constants.SELINUX_BOOLEAN_SMBSERVICE:
        for name in usecase:
            boolean_states[name] = statestore.restore_state("selinux", name)

    if boolean_states:
        set_selinux_booleans(boolean_states, statestore, backup=False)

or similar in httpinstance.py

        # Restore SELinux boolean states
        boolean_states = {name: self.restore_state(name)
                          for name in constants.SELINUX_BOOLEAN_HTTPD}
        try:
            tasks.set_selinux_booleans(boolean_states)
        except ipapython.errors.SetseboolError as e:
            self.print_msg('WARNING: ' + str(e))

@abbra
Copy link
Contributor Author

abbra commented Sep 1, 2023

That's exactly what my original code did.

@abbra
Copy link
Contributor Author

abbra commented Sep 1, 2023

I pushed the original code. restore_state() calls delete_state() if what was restored is not None.

tasks.set_selinux_booleans(boolean_states)
except SetseboolError as e:
logger.warning("Unable to reset SELinux variable: %s", str(e))

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I got the original failure reason wrong, in a way. This code needs to be moved (I moved it to 3627 in my testing). Right now it is conditioned on the state of whether SSSD was pre-configured. I think this SELinux code is independent of that.
With moving the code the original boolean values are restored and sysrestore.state removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it out and conditioned on was_sssd_installed and selinux_works becase that's what we did when enabling the booleans (if sssd: ... if selinux_works: ..).

For passkeys (FIDO2) support, SSSD uses libfido2 library which needs
access to USB devices. Add SELinux booleans handling to ipa-client-install
so that correct SELinux booleans can be enabled and disabled during
install and uninstall. Ignore and record a warning when SELinux policy
does not support the boolean.

Fixes: https://pagure.io/freeipa/issue/9434

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
@rcritten
Copy link
Contributor

rcritten commented Sep 1, 2023

Thanks, changes look good.

@rcritten rcritten added ack Pull Request approved, can be merged re-run Trigger a new run of PR-CI and removed needs review Pull Request is waiting for a review labels Sep 1, 2023
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Sep 1, 2023
@abbra abbra added the re-run Trigger a new run of PR-CI label Sep 4, 2023
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Sep 4, 2023
@flo-renaud flo-renaud added the pushed Pull Request has already been pushed label Sep 11, 2023
@flo-renaud
Copy link
Contributor

master:

  • d355761 ipa-client-install: enable SELinux for SSSD

@flo-renaud flo-renaud closed this Sep 11, 2023
t-woerner added a commit to t-woerner/ansible-freeipa that referenced this pull request Feb 6, 2024
This is "ipa-client-install: enable SELinux for SSSD"
freeipa/freeipa#6978 for ansible-freeipa:

For passkeys (FIDO2) support, SSSD uses libfido2 library which needs
access to USB devices. Add SELinux booleans handling to ipa-client-install
so that correct SELinux booleans can be enabled and disabled during
install and uninstall. Ignore and record a warning when SELinux policy
does not support the boolean.

Fixes: https://pagure.io/freeipa/issue/9434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-11 Mark for backport to ipa 4.11 pushed Pull Request has already been pushed
Projects
None yet
5 participants