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

Removed adding to attribute unpriv_userdomain from userdom_unpriv_type template #427

Merged
merged 2 commits into from
Sep 30, 2020

Conversation

Koncpa
Copy link
Contributor

@Koncpa Koncpa commented Sep 4, 2020

When is secure_mode boolean enabled the attribute unpriv_userdomain allow transition
only between unprivileged users. But one member this attribute was unconfined_t
domain, which had allow privilege operations. Solution was that from userdom_unpriv_type
template was remove adding domains to attribute unpriv_userdomain. This template is used only
for unconfined_t, so affected only uncofined domain.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1840851

@zpytela
Copy link
Contributor

zpytela commented Sep 4, 2020

I can confirm the userdom_unpriv_type interface is used only for unconfined_t.

@Koncpa, did you create a build just to confirm unconfined_u user work as expected, and the attribute is not set by some other call?

@wrabcak
Copy link
Member

wrabcak commented Sep 4, 2020

Let's properly test this before we merge it, because we're removing rules this could cause troubles.

…e template

When is secure_mode boolean enabled the attribute unpriv_userdomain allow transition
only between unprivileged users. But one member this attribute was unconfined_t
domain, which had allow privilege operations. Solution was that from userdom_unpriv_type
template was remove adding domains to attribute unpriv_userdomain. This template is used only
for unconfined_t, so affected only uncofined domain.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1840851
Removing attribute in previous commit affected connecting via ssh to unconfined user.
Missed dyntransition from sshd domain to unconfined domain.
Added ssh_dyntransition_to() interface.
@Koncpa
Copy link
Contributor Author

Koncpa commented Sep 9, 2020

@zpytela, @wrabcak I tested this PR and I found one issue. When I removed domain from this attribute I cannot connect to unconfined user via ssh, because from sshd_t missed dyntransition to unconfined_t. So I add interface ssh_dyntransition_to(), which solved this issue. Also I tried solution from different point of view, where I made neverallow rule which deny transition from newrole_t to unconfined_t, but in building policy appeared issues with violated rules, so I finally choosed the first option.

@wrabcak
Copy link
Member

wrabcak commented Sep 9, 2020

@Koncpa this should be explained and discussed with ssh maintainer.

cc @Jakuje

@Jakuje
Copy link

Jakuje commented Sep 9, 2020

The transition from sshd_t to unconfined_t is needed for ssh to allow ... well ... normal login of unconfined users. The process tree looks somehow like this for contexts used by sshd:

$ ps axZf | grep [s]shd
system_u:system_r:sshd_t:s0-s0:c0.c1023 2037143 ? Ss    0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
system_u:system_r:sshd_t:s0-s0:c0.c1023 755776 ? Ss     0:00  \_ sshd: jjelen [priv]
unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 755833 ? S   0:00      \_ sshd: jjelen@pts/12

That is how far I understand what sshd does with selinux and what is being discussed in this issue. But as long as this works, we should be fine from ssh point of view.

@Koncpa
Copy link
Contributor Author

Koncpa commented Sep 10, 2020

@wrabcak Also when I looked to the policy before the fix I found same rule with one difference. As you can see in the rule is the attribute where the member was unconfined_t domain before removed.

$ sesearch -A -s sshd_t -t unconfined_t -c process -p dyntransition
allow sshd_t unpriv_userdomain:process { dyntransition signal transition }

Policy with patch:

$ sesearch -A -s sshd_t -t unconfined_t -c process -p dyntransition
allow sshd_t unconfined_t:process { dyntransition getattr signal sigstop };

Thanks @Jakuje

@zpytela
Copy link
Contributor

zpytela commented Sep 10, 2020

@Koncpa, I see a lot of references to unpriv_userdomain: have you tried some other common scenarios for unconfined_u, like logging in in xdm or console, start some service, execute some commands, su/sudo, etc.?

@Koncpa
Copy link
Contributor Author

Koncpa commented Sep 23, 2020

@zpytela Yes, I tried a some other common scenarios for this type of user. I also reboot this machine with this new policy and I didn't found any issues with removing unconfined_t from unpriv_userdomain.

@Koncpa
Copy link
Contributor Author

Koncpa commented Sep 29, 2020

I specific tried to start|restart|stop httpd.service, logging user in console, executing basic command as ls, cd, cat, pwd and etc. Also I tested privileged command liked chown, chmod.

@zpytela
Copy link
Contributor

zpytela commented Sep 29, 2020

Thank you, merging then.

@zpytela zpytela merged commit 3bdcea7 into fedora-selinux:rawhide Sep 30, 2020
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.

4 participants