URL: https://github.com/SSSD/sssd/pull/577 Author: fidencio Title: #577: ipa: Use fqname on selinux_child_setup Action: opened
PR body: """ Although there was a comment saying that pam_selinux needs the username in the same format getpwnam() would return it, it doesn't seem to be the case anymore.
Just using fqname from selinux_child_setup allows us to have the expected results.
One difference that I've spotted while doing this patch (which may or may not be an issue) is that without this patch the output of `semanage login --list` was always (with or without domain_resolution_order set): [root@client1 x86_64]# semanage login --list
Login Name SELinux User MLS/MCS Range Service
__default__ unconfined_u s0-s0:c0.c1023 * admin staff_u s0-s0:c0.c1023 * root unconfined_u s0-s0:c0.c1023 *
While now I can see: [root@client1 x86_64]# semanage login --list
Login Name SELinux User MLS/MCS Range Service
__default__ unconfined_u s0-s0:c0.c1023 * admin staff_u s0-s0:c0.c1023 * admin@ipa.example staff_u s0-s0:c0.c1023 * root unconfined_u s0-s0:c0.c1023 *
Resolves: https://pagure.io/SSSD/sssd/issue/3740
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
NOTE: While I still have to run downstream tests to be sure it won't introduce regressions (although, it didn't in the very simple test that I've performed) ... I'd appreciate a review (even before the results of the tests) in order to have an early understanding of whether this is a valid approach or not. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/577/head:pr577 git checkout pr577
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
fidencio commented: """ With the patch: J:2496100
Without the patch: J:2495991
At least according to those tests no regression has been introduced. And, considering they're reliable, I've gotten even more green with the patch than without it. """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-391008722
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
fidencio commented: """ CI: http://vm-031.$%7Babc%7D/logs/job/89/36/summary.html """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-391008923
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
jhrozek commented: """ So it makes sense that the context is set correctly by pam_selinux after you log in, but are you sure that it gets set by the selinux child the way you want? Does everything still work if you do: ``` semanage login -d admin semanage login -d admin@ipa.example ``` ?
If you then log in, what is the output of `semanage login -l` ?
btw the reason I'm asking is that even after the patch the context is set for me only for the short name. Because `sss_output_fqname` does pretty much the same as the previous code (so, I agree that we should use it), but neither of the conditions to qualify the name match for me. The IPA domain has both `domain->fqnames` and `domain->output_fqnames` set to false. IIRC only the responders currently set `domain->output_fqnames`.
Wouldn't the correct fix be to also initialize `domain->output_fqnames` for the instances of `sss_domain_info` that providers use? """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-391379772
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
fidencio commented: """
So it makes sense that the context is set correctly by pam_selinux after you log in, but are you sure that it gets set by the selinux child the way you want? Does everything still work if you do:
semanage login -d admin semanage login -d admin@ipa.example?
If you then log in, what is the output of semanage login -l ? btw the reason I'm asking is that even after the patch the context is set for me only for the short name.
You're damn right here! The patch doesn't actually solve the situation :-(
Because sss_output_fqname does pretty much the same as the previous code (so, I agree that we should use it), but neither of the conditions to qualify the name match for me. The IPA domain has both `domain->fqnames` and `domain->output_fqnames` set to false.
Yep, I've noticed that as well. And it's not that easy to solve ...
IIRC only the responders currently set domain->output_fqnames.
Exactly.
Wouldn't the correct fix be to also initialize domain->output_fqnames for the instances of sss_domain_info that providers use?
I'll cook something up here and re-submit the patches. I may end up need to talk to you about the approach, in any case.
Thanks for the review. """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-391399115
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/577 Author: fidencio Title: #577: ipa: Use fqname on selinux_child_setup Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/577/head:pr577 git checkout pr577
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: ipa: Use fqname on selinux_child_setup
fidencio commented: """ patch set has been updated. """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-392532045
URL: https://github.com/SSSD/sssd/pull/577 Author: fidencio Title: #577: selinux_child: workaround fqnames when using DRO Action: edited
Changed field: title Original value: """ ipa: Use fqname on selinux_child_setup """
URL: https://github.com/SSSD/sssd/pull/577 Author: fidencio Title: #577: selinux_child: workaround fqnames when using DRO Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/577/head:pr577 git checkout pr577
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: selinux_child: workaround fqnames when using DRO
jhrozek commented: """ Works fine both with and without domain resolution order, ACK """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-393195559
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: selinux_child: workaround fqnames when using DRO
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: selinux_child: workaround fqnames when using DRO
fidencio commented: """ master: f9b42e3 """
See the full comment at https://github.com/SSSD/sssd/pull/577#issuecomment-393682489
URL: https://github.com/SSSD/sssd/pull/577 Author: fidencio Title: #577: selinux_child: workaround fqnames when using DRO Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/577/head:pr577 git checkout pr577
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: selinux_child: workaround fqnames when using DRO
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/577 Title: #577: selinux_child: workaround fqnames when using DRO
Label: -Accepted
sssd-devel@lists.fedorahosted.org
-
fidencio -
jhrozek