Bug 1269072 - SELinux/PAM: Openssh chage related issue
SELinux/PAM: Openssh chage related issue
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: openssh (Show other bugs)
rawhide
Unspecified Unspecified
low Severity low
: ---
: ---
Assigned To: Jakub Jelen
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-10-06 04:45 EDT by dac.override
Modified: 2016-02-03 05:27 EST (History)
6 users (show)

See Also:
Fixed In Version: openssh-7.1p1-4.fc23 openssh-7.1p1-5.fc23
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-11-05 17:24:02 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
proposed patch (1.77 KB, text/plain)
2015-10-14 11:03 EDT, Jakub Jelen
no flags Details

  None (edit)
Description dac.override 2015-10-06 04:45:23 EDT
Description of problem:

Openssh changes to the login context of the user login very early. So early that Openssh-server briefly runs with the permissions associated with the login user context and not with the openssh context. This is generally not such a big problem because the only thing sshd does in that brief period is log (and that can be silently denied without too big problems)

However if you configure chage to force a user to change his password on the next boot then things get ugly:

openssh-server running with the context of the user that is about to log-in runs the passwd utility, but it insists that it runs the passwd utility with a manual type transition to the login user type (the openssh process is already running with that type at that point by the way)

The problem here is to make this work we would need to associate broad permissions with the login user type (we need to allow the login user type to run passwd with a manual type transition, and since sshd insist it run passwd with the user login type, we need to associate all the permisisons that passwd requires with the login user type. (thus associating excesssive permission with anyone login in with sshd)

These issues started a long time ago when it was decided to change the selinux/pam implementation in sshd, and it is very ugly.

the login program doest it right. So if you are looking for an example of how it should be done instead, i suggest you take a look at that
Comment 1 Petr Lautrbach 2015-10-13 05:00:19 EDT
Thanks for your input.

The difference is that in case of the PasswordAuthentication, the passwd utility is used to change a password. If you don't want to use 'passwd', you can use 'ChallengeResponseAuthentication yes' and 'PasswordAuthentication no' and it will work similar to login. It's still workaround though.

Since all Fedora and refpolicy SELinux users are allowed to change their password and there's the workaround, I set the priority to Low and I'm leaving this bug open to be considered in future.
Comment 2 dac.override 2015-10-13 05:42:59 EDT
SELinux users are allowed to change their password, yes, but they do so by running the passwd command with a domain transition to the passwd domain:

https://github.com/fedora-selinux/selinux-policy/blob/rawhide-base/policy/modules/system/userdomain.if#L1004

This is the difference

SSHD currently insists on running the passwd utility without a domain transition.

Have you tried it?

A simple way to try it is to create a user, and associate that user with guest_u

Then use chage to force the user to change his password on the next login:

chage -d 0 {user-name}

then login with ssh and see if you can change the password
Comment 3 Petr Lautrbach 2015-10-13 06:54:36 EDT
I tried it and I can change a password:

$ ssh guest@plautrba-rawhide.virt
guest@plautrba-rawhide.virt's password: 
You are required to change your password immediately (root enforced)
Last login: Tue Oct 13 12:45:53 2015 from 192.168.122.1
WARNING: Your password has expired.
You must change your password now and login again!
Changing password for user guest.
Changing password for guest.

>>>> In this moment I listed processes and passwd is run as passwd_t, see bellow the rest of login proces 

(current) UNIX password: 
New password: 
Retype new password: 
passwd: all authentication tokens updated successfully.
Connection to plautrba-rawhide.virt closed.

$ ssh guest@plautrba-rawhide.virt
guest@plautrba-rawhide.virt's password: <new password>
Last login: Tue Oct 13 12:50:27 2015 from 192.168.122.1
[guest@plautrba-rawhide ~]$ id -Z
guest_u:guest_r:guest_t:s0
[guest@plautrba-rawhide ~]$ getenforce 
getenforce:  getenforce() failed[guest@plautrba-rawhide ~]$


Process list:

system_u:system_r:sshd_t:s0-s0:c0.c1023 1333 ? Ss     0:00  \_ sshd: guest [priv]
guest_u:guest_r:guest_t:s0       1341 ?        S      0:00      \_ sshd: guest@pts/1
guest_u:guest_r:passwd_t:s0      1342 pts/1    Ss+    0:00          \_ passwd


It might be that I simply don't understand what you mean by: 
SSHD currently insists on running the passwd utility without a domain transition.

Would you please be more specific?
Comment 4 dac.override 2015-10-13 07:03:14 EDT
Ok this is strange. It does work for you.

In my tests the transition does not take place and instead sshd insists to run passwd with the context of the user (guest_u:guest_r:guest_t:s0) instead of the context of passwd (guest_u:guest_r:passwd_t:s0) (its using "setexec" to accomplish this)

I will have a closer look, and i suppose you can close this bug since it works fine for you.

Sorry for the noise
Comment 5 dac.override 2015-10-13 07:19:11 EDT
ok i think i figured this out. For this to work the user needs to be allowed to (process setexec)

So if you would remove the rule:

allow guest_t self:process setexec;

then you would hit the same issue that i am hitting

So i suppose the end result is that due to this "issue" i have to allow all my sshd login users to "setexec"
Comment 6 dac.override 2015-10-13 07:24:31 EDT
So to me this still seems to be a sshd "bug". because obviously it is not the "user" that needs "setexec" but it is sshd running with the users permissions requiring "setexec"
Comment 7 dac.override 2015-10-13 07:29:38 EDT
sshd does not need to run passwd with setexeccon() because sshd already runs with the users context. So the automatic domain transition rule in the policy is already enough

if you do not use setexeccon() to run passwd then the user also does not need "process setexec"

So i suspect the situation could be improved a great deal by not letting sshd use setexeccon to run passwd on behalf of the login user

I hope i made my concern clear. its a weird little issue but setexec is a permission i would preferably not be forced to grant my ssh login users
Comment 8 dac.override 2015-10-13 07:46:03 EDT
This is the change i had to make to my policy to make this chage functionality work:

https://github.com/DefenSec/dssp-contrib/commit/db3acff4f565a938b628e070da0152a4539b7af6

I have not tried this but i doubt that refpolicy grants its sshd login users the setexec permission, and that it would want to do that.

So this issue is still very much real. I suppose fedora allowed its ssh login users to setexec but i am not sure if anyone else would like that
Comment 9 Petr Lautrbach 2015-10-13 08:38:07 EDT
The Fedora policy change was introduced in https://github.com/fedora-selinux/selinux-policy/commit/fa7447f5bf74d8bccd2eb92173fc1d486110d16a

I'd think that setexeccon() is probably not needed any more since there's setcon() used to change a context but lets look into it closer.
Comment 10 Jakub Jelen 2015-10-14 06:54:18 EDT
Thanks for a report and for some explanation in previous comments. First of all sorry for a long post explaining internals and reasoning what is going on where. I also needed to sort the things out.

To explain what is going on under the hood based on exec, setcon and setexeccon calls we can put the program structure this way. This is simplified call tree:

sshd.c: main() authenticated part
  sshd_selinux_setup_exec_context() -> setexeccon(user_ctx)
  do_pam_session() -> pam_open_session() # should not require SELinux user context
  privsep_postauth() -> # forks child, continues as a monitor
  do_authenticated() -> # [child]   session stuff described later

session.c: do_setusercontext()
  platform_setusercontext_post_groups()
    -> sshd_selinux_setup_exec_context() -> setexeccon(user_ctx)
  if chrooting
    sshd_selinux_copy_context() -> setcon(getexeccon())
    chroot
    unser chroot variable
  if !chrooting (falls here also chroot condition, since the variable is unset)
    sshd_selinux_copy_context() -> setcon(getexeccon())
    #basically noop if chrooting, but can cause denials

session.c: do_child()
  if force_pwchange:
    do_setusercontext()
    do_pwchange()
      setexeccon(NULL)
      execl("passwd")
  do_setusercontext()
  ...
  execve(coomand/shell/)

sshd.c: privsep_postauth()
  (only for postauth child)
  do_setusercontext() # set user context
  sshd_selinux_copy_context() #once more for root???

Another chapter is privilege separation and its context for net child -- switch to "sshd_net_t", but it is not related to this problem.

sshd.c: privsep_preauth_child()
  sshd_selinux_change_privsep_preauth_context()
    -> ssh_selinux_change_context() -> setcon()



When we will look at our use case for postauth child and password change, we have more context changes than needed which can cause your SELinux denials:

sshd.c main() {sshd_t}
  sshd_selinux_setup_exec_context() -> setexeccon(user_ctx)
  privsep_postauth() -> do_setusercontext()
    platform_setusercontext_post_groups() -> sshd_selinux_setup_exec_context() -> setexeccon(user_ctx)
    sshd_selinux_copy_context() -> setcon(getexeccon()) {guest_t}
  do_authenticated() -> ... -> do_child()
    do_setusercontext()
      platform_setusercontext_post_groups() -> sshd_selinux_setup_exec_context() -> setexeccon(user_ctx) {denial ??}
      sshd_selinux_copy_context() -> setcon(getexeccon())
    setexeccon(NULL) {if not above, it will be denied here}
    execl("passwd")



Certainly we have things to fix here.

  * When chrooting we do setcon twice

  * Last "trace" shows, that we do setexeccon three times and and setcon twice which is basically not a thing we want.

I will elaborate on patches and test it soon.
Comment 11 dac.override 2015-10-14 07:29:16 EDT
Thank you, I appreciate that this is revisited thoroughly.

There is also another long standing issue i have related to this, but it is very hard to explain.

it boils down to this:

root sshd logins requires that we associate a lot of extra permissions with the context that is associated with root login user.

These are the permissions i need to associate with the context that is associated with root if i want root to be able to login with sshd:

			(allow ssh.priv_login_subj_type_attribute self (capability (setuid setgid sys_tty_config)))
			(allow ssh.priv_login_subj_type_attribute self (process (dyntransition setcurrent)))
			(allow ssh.priv_login_subj_type_attribute self r_netlink_route_socket_perms)
			(allow ssh.priv_login_subj_type_attribute self setattr_key_perms)

			(call sys.audit_set_params (ssh.priv_login_subj_type_attribute))

			(call sec.compute_relabel_context (ssh.priv_login_subj_type_attribute))
			(call sec.compute_user_contexts (ssh.priv_login_subj_type_attribute))
			(call sec.validate_context (ssh.priv_login_subj_type_attribute))

			(call usersubj.relabel_ptys (ssh.priv_login_subj_type_attribute))
			(call usersubj.setattr_ptys (ssh.priv_login_subj_type_attribute))

As you can see this is very excessive. This is sshd with the context associated with the root login user. So the root login user does not need this but because i want to root login user to be able to login with sshd. and since sshd needs these permissions when it already setcon'd to the context associated with the root login user, i am forced to grant these permission.

This only applies to root logins
Comment 12 Tomas Mraz 2015-10-14 08:37:08 EDT
To simplify/resolve the root logins issue perhaps the solution would be to drop all the special-casing for root login in case SELinux is enabled.
I believe the reasoning for the special-casing for root was that it is privileged anyway so the session privilege separation does not make much sense, but that is not the case for SELinux where the root login can be limited by it.
Comment 13 dac.override 2015-10-14 09:28:23 EDT
Yes I agree, In my (sshd) case root only needs to run systemd-run with a domain transition, nothing more


For example, on my home network i need to allow root ssh logins for systemd-run (-H). But all this does is just run the systemd-stdio-bridge on the remote, and all that does is some dbus chatting with various systemd components. Not to mention that systemd-stdio-bridge runs in a very confined domain that only has the privileges associated with it it that it needs.

from /root/.authorized_keys:

command="systemd-stdio-bridge",from="192.168.1.0/24,fe80::3602: ....

So i agree with you that in a selinux system "root" can have many meanings.
Comment 14 Jakub Jelen 2015-10-14 11:03 EDT
Created attachment 1082863 [details]
proposed patch

After running the code I found, that the part with setexeccon in function

  sshd_selinux_setup_exec_context()

is skipped, because UsePAM is yes by default (exec context is set up by PAM). This means that your denial is coming certainly from (and from here the strange behaviour: not fatal, but passwd executed with wrong context)

  setexeccon(NULL)

just before exec(passwd). This should overwrite execcon defined by pam_selinux module.

To solve this issue we can unset execcon before calling setcon (before we potentially lose this capability). The other execs should not matter since the process will be in users limited context and both shells are limited even without execcon (first testing looks fine, but we should think how to handle failures).

For the chroot problem we can use the variable doing_chroot, which is persistent over the function, unlike the option value.


Updated diagram with only important parts:

sshd.c main() {sshd_t, execcon: sshd_t}
  do_pam_session() {sshd_t, execcon: guest_t}
  privsep_postauth() -> do_setusercontext() -> sshd_selinux_copy_context()
    setcon(getexeccon()) {guest_t, execcon: guest_t}
  do_authenticated() -> ... -> do_child()
    do_setusercontext() -> sshd_selinux_copy_context()
      setcon(getexeccon()) {guest_t, execcon: guest_t} /* probably noop */
    setexeccon(NULL) {denied}
    execl("passwd")


My proposed fix would change it to this:

sshd.c main() {sshd_t, execcon: sshd_t}
  do_pam_session() {sshd_t, execcon: guest_t}
  privsep_postauth() -> do_setusercontext() -> sshd_selinux_copy_context()
    c=getexeccon()
    setexeccon(NULL) {sshd_t, execcon: NULL}
    setcon(c) {guest_t, execcon: NULL}
  do_authenticated() -> ... -> do_child()
    do_setusercontext() -> sshd_selinux_copy_context()
      c=getexeccon()
      setexeccon(NULL) {guest_t, execcon: NULL} /* probably noop */
      setcon(c) {guest_t, execcon: NULL} /* probably noop */
    execl("passwd")

This should not affect other code branches, but before pushing this out, it needs some testing and making sure that if setcon fails, the user is not given terminal with sshd_t type (probably should be fatal).

dac, can you review it and make sure it suits for your needs this way?
Comment 15 dac.override 2015-10-14 13:12:32 EDT
If you can provide me with a link to a scratch built (for rawhide) then i will be glad to try it out and see if i can do away with the setexec and any other things you want me to look at from a usability point of view
Comment 16 dac.override 2015-10-14 13:14:58 EDT
BTW looking that that snippit hurts my eyes but that is not your fault.

identifier such as sshd_t, guest_t are customisable and should not be hard-coded, instead the openssh_contexts file should be used to make those types configurable.
Comment 17 dac.override 2015-10-14 13:18:49 EDT
there is already an openssh_contexts libselinux file used in openssh, if you have a requirement to know selinux identifiers then you should extent the openssh_contexts file to query what types you should use.

You see not every policy has type sshd_t associated with the sshd processes so you cannot assume that this is there. instead you should make it so that you can lookup what types to use where using openssh_contexts
Comment 18 dac.override 2015-10-14 13:20:18 EDT
O never mind those last comments, that does not look like code but some other stuff. As long as no selinux identifiers are hard-coded I am fine with that and willing to try it out
Comment 19 dac.override 2015-10-15 02:27:00 EDT
I will look into doing it myself with fedpkg (I need to further scrutinise my fedpkg and rpmbuild policies anyways and this seems like an good occasion to do so)
Comment 20 Jakub Jelen 2015-10-15 03:56:14 EDT
Yes, the SELinux types were just examples for this run. Currently there is nothing hard-coded except for the fallback type for net child.

 * sshd_t is based on the transition from sshd_exec_t
 * guest_t is based on the execcon which is set by pam_selinux (which depends on real type set by semanage login)

Scratch build with my patch is available here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=11455426
Comment 21 dac.override 2015-10-15 04:46:07 EDT
Thanks, this is tested. It solves my "setexec" issue and your has my ACK

It would really be nice if the root issue would also be "solved" now that we have the momentum to do it
Comment 22 dac.override 2015-10-15 05:47:44 EDT
dunno why i am posting this here but i recorded my tests (profanity warning though)

part1
https://www.youtube.com/watch?v=mv25E0yI0X8
part2
https://www.youtube.com/watch?v=QPns7hWYY14
part3
https://www.youtube.com/watch?v=NBXPhYn58jk
Comment 23 Jakub Jelen 2015-10-15 12:40:41 EDT
About the root login, it is more complicated.
If uid=0, after authentication, privilege separation is turned off, which is the reason why we now require these capabilities. This is visible from process traces:

# parent process
system_u:system_r:sshd_t:s0-s0:c0.c1023 25708 ? Ss    0:00 /usr/sbin/sshd -D

# Example user login:
system_u:system_r:sshd_t:s0-s0:c0.c1023 25710 ? Ss    0:00  \_ sshd: user [priv]
guest_u:guest_r:guest_t:s0      25719 ?        S      0:00  |   \_ sshd: user@pts/3
guest_u:guest_r:guest_t:s0      25720 pts/3    Ss+    0:00  |       \_ -bash

# Example root login:
unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 25747 ? Ss   0:00  \_ sshd: root@pts/4
unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 25750 pts/4 Ss   0:00  |   \_ -bash


This decision is probably based on the expectation, that root is just unconfined by definition. When using SELinux we can set some constraints even on root accounts (with uid=0) so I guess this upstream expectation does not apply in our use case.

Changing this one condition makes our root accounts behave the same way as other users (with privileged parent handling the thing you described). I tested with our Fedora policy and it worked fine. Can you test with your? There is no reason why it should behave different way. Here is another scratch build with this patch:
http://koji.fedoraproject.org/koji/taskinfo?taskID=11461917

Patch:
diff --git a/sshd.c b/sshd.c
index 7b7b24e..f69f7a5 100644
--- a/sshd.c
+++ b/sshd.c
@@ -796,9 +796,9 @@ privsep_postauth(Authctxt *authctxt)
 #ifdef DISABLE_FD_PASSING
 	if (1) {
 #else
-	if (authctxt->pw->pw_uid == 0 || options.use_login) {
+	if (options.use_login) {
 #endif
-		/* File descriptor passing is broken or root login */
+		/* File descriptor passing is broken */
 		use_privsep = 0;
 		goto skip;
 	}
Comment 24 dac.override 2015-10-15 13:23:10 EDT
Tested OK. works good enough for me.

It does try to log to audit, which i blocked in policy, and log a big-fat "error" but it does not seem to break anything except from an audit POV i might be missing some important records.

But at least it allows root to log in. and that is what i was after.

recorded test for reference:

https://www.youtube.com/watch?v=6_I_6OAqPw4

So both patches successfully address my concerns. Would be nice if these would get applied. Thanks
Comment 25 Jakub Jelen 2015-10-16 03:39:58 EDT
Thanks for testing and confirmation. I will try to review and maybe refactor the whole bunch of selinux patches once more, before posting to Fedora, so give me some time.

About the audit_open() denial you mention in your screencast, I didn't have such error in my testing environment. From the message it looks like the PAM doesn't have rights to open audit log, but sshd itself logs audit messages without problems. This message is not present when logging as normal user? It is quite strange.
Comment 26 dac.override 2015-10-16 03:50:55 EDT
Yes, but Fedora policy is coarse grained compared to my policy.

unpriv users do not have this audit socket issue, however instead unpriv users need to log to the journal instead. (which i also deny because as i do not want my unpriv users to be able to insert bad stuff into my log directly from a shell, programs like logger are be confined to their own domain, and select unpriv users are allowed to run logger with a automatic domain transition)
Comment 27 dac.override 2015-10-16 04:03:03 EDT
actually, the context associated with priv user login shells is allowed to log to the journal as opposed to the context associated with unpriv user login shells. So it may be that PAM or whatever does this , just bails out when it determines that it can't log to journal (in other words it may take a different code path) So the two can't really be compared.

Testing this in permissive mode with my policy would show more info about what the two actually need. But yes , these users are restricted so an avc denial here and there is to be expected.
Comment 28 dac.override 2015-10-16 04:11:01 EDT
I will have another look from a permissive perspective to see what priv and unpriv users really *want* , instead of *need*
Comment 29 dac.override 2015-10-16 04:34:46 EDT
Ok well i tested it in permissive mode just to confirm and it is what i thought it was. Basically root related logins want to log to audit, but unpriv related logins do not have this requirement.

So the audit issue is root login specific


Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { create } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysad
m.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_audit_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { create } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysad
m.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_route_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { bind } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.
subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_route_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { getattr } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_route_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { write } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_route_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { nlmsg_read } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_route_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { read } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_route_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { write } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_audit_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { nlmsg_relay } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_audit_socket permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { audit_write } for  pid=21639 comm="sshd" capability=29  scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=capability permissive=1
Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { read } for  pid=21639 comm="sshd" scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=netlink_audit_socket permissive=1

unpriv users want to log to journal

Oct 16 10:23:59 fedoratest4 audit[21672]: AVC avc:  denied  { search } for  pid=21672 comm="sshd" name="systemd" dev="tmpfs" ino=9285 scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sd.runtime_file:s0 tclass=dir permissive=1
Oct 16 10:23:59 fedoratest4 audit[21672]: AVC avc:  denied  { search } for  pid=21672 comm="sshd" name="journal" dev="tmpfs" ino=9487 scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sd_journald.runtime_file:s0 tclass=dir permissive=1
Oct 16 10:23:59 fedoratest4 audit[21672]: AVC avc:  denied  { write } for  pid=21672 comm="sshd" name="dev-log" dev="tmpfs" ino=9488 scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sd_journald.runtime_file:s0 tclass=sock_file permissive=1
Oct 16 10:23:59 fedoratest4 audit[21672]: AVC avc:  denied  { sendto } for  pid=21672 comm="sshd" path="/run/systemd/journal/dev-log" scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sys.subj:s0 tclass=unix_dgram_socket permissive=1

So for me this is just fine. It is my personal policy design decision to block this. Fedora probably does not mind these accesses.

My policy follows a different filosofy. I am saying: shells do not log to journal/audit, so contexts associated with shells should be allowed this. Programs log. Like for example "logger" so the context associated with logger should be allowed to log, and contexts associated with shells should, potentially, be allowed to run programs with a context transition.

So if i want my unpriv user to be able to log to the journal then i will allow the context associated with the unpriv shell to run programs associated with for example the logger executable file type, and then transition from the shell context to the logger context. the logger context can log , and the shell context is not permitted to log
Comment 30 dac.override 2015-10-16 04:51:59 EDT
O and root specific logins also want to read the routing table, that might be related to the audit requirement, not sure

So root logins:

1.log to audit
2.read routing table

unpriv users:

1.log to journal
Comment 31 Miroslav Grepl 2015-10-16 05:23:32 EDT
Thank you guys for your great work here. I believe it is really great to make SELinux + openssh code much more sufficient. 

Also how Dominick mentioned he uses a different concept of his personal policy which is sometimes so different against Fedora policy.

For example we have

allow syslog_client_type devlog_t : sock_file { write getattr append open } ; 

seinfo -xasyslog_client_type

      guest_t
      staff_t
      ..
      ..

So I believe we can say it is "OK" and we can wait for another patches/builds for testing.
Comment 32 Fedora Update System 2015-10-22 12:40:55 EDT
openssh-7.1p1-4.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-1ff6efd876
Comment 33 dac.override 2015-10-22 13:21:08 EDT
Works. unfortunately the audit/journal thing is still there but as i mentioned above this is not a "blocker" for me. Thanks
Comment 34 Jakub Jelen 2015-10-23 05:25:21 EDT
Actually I didn't fully get the problem you were trying to describe in the last comments about the audit/logging. I believe the logging/auditing *should* be done by privileged process with sshd_t context, not by the process with user context, as your logs are trying to show. Anyway I am not sure how I should track down the source of these denials without any context.

If I am right, it works fine on Fedora with our policy, which is now important for pushing the update.
Comment 35 dac.override 2015-10-23 05:32:47 EDT
I does not work fine with fedora policy. Fedora policy just hides the issue.

The simple fact is that sshd logs to audit with the context associated with the root login user (thus not *always* with "sshd_t"), when root logs in with sshd.

similarly when unpriv users log in with sshd , then sshd logs to journal with the context associated with the unpriv login user and not with always "sshd_t"

The proof is here:

Oct 16 10:21:27 fedoratest4 audit[21639]: AVC avc:  denied  { audit_write } for  pid=21639 comm="sshd" capability=29  scontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tcontext=sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 tclass=capability permissive=1


The comm= has "sshd" so the sshd command uses cap_audit_write, the context associated with sshd is sysadm.id:sysadm.role:sysadm.subj:s0-s0:c0.c1023 this is the context associated with the root login user and not the context associated with sshd (that would be: sys.id:sys.role:sshd.subj:s0)

But again for me, this isnt terrible. its not perfect but its not terrible either. I am already glad that the most pressing issues were resolved
Comment 36 dac.override 2015-10-23 05:55:52 EDT
but yes this is not a blocker.
Comment 37 Fedora Update System 2015-10-24 08:06:58 EDT
openssh-7.1p1-4.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update openssh'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-1ff6efd876
Comment 38 Jakub Jelen 2015-10-26 11:45:44 EDT
As I noted, the denials are coming from PAM. Can you (just in curiosity) try login without PAM (UsePAM no), if these denials persist (but the login will probably not work correctly)?

On the second sight, it is obvious. OpenSSH first sets SELinux exec context and then calls PAM functions (pam_setcred), which do exec/forks and then gains target user context. If I am right, these functions should have user context (although APM documentation is not too verbose in this point).

Tomas? Do pam_open_session() and pam_setcred() need target user SELinux context? If not, we can avoid this problem, otherwise we require this audit privilege for confined users context.
Comment 39 dac.override 2015-10-26 13:38:43 EDT
Okay, but comparing this to logins with /bin/login (which also uses PAM for auth), this is different in that local login does not have this "issue".

So i suspect it is openssh related/specific
Comment 40 Tomas Mraz 2015-10-27 05:22:20 EDT
I believe the exec context setting for the user context should be done in pam_selinux or am I wrong?

If /bin/login does not have this issue I am really curious what is the difference in the order of the selinux and pam calls in login and in sshd.
Comment 41 Jakub Jelen 2015-10-29 10:40:50 EDT
Yes. This seems like some code part setting up context if it is not set up by PAM.

The discussed line was introduced in commit [1] as part of MLS patch (bug #220487) by Tomas. Few ideas can be observed from bugzilla. The setup_selinux_exec_context function was moved from _after_ PAM to _before_ PAM calls (pam_setcred, pam_open_session). The effect is not only "overwrite execcon by PAM", but also "run PAM with this execcon", which is probably not intentional (can you confirm?).

I believe we should change this part of sshd.c

#ifdef WITH_SELINUX
    sshd_selinux_setup_exec_context(authctxt->pw->pw_name);
#endif
#ifdef USE_PAM
    if (options.use_pam) {
        do_pam_setcred(1);
        do_pam_session();
    }
#endif

to something like this

#ifdef USE_PAM
    if (options.use_pam) {
        do_pam_setcred(1);
        do_pam_session();
    } else 
#else
    if (1)
#endif
#ifdef WITH_SELINUX
        sshd_selinux_setup_exec_context(authctxt->pw->pw_name);
#else
        ;
#endif

to entertain this call only when we know that PAM will not do execcon for us (yes, it looks really awkward).

There is still the theoretical case when we will not have pam_selinux in PAM stack, which would not set any execcon. So probably most defensive solution would be checking the execcon after pam returns and if it was not set, fall back to native setting:

#ifdef USE_PAM
    if (options.use_pam) {
        do_pam_setcred(1);
        do_pam_session();
    }
#endif
#ifdef WITH_SELINUX
    char *context;
    if (getexeccon(&context) != 0 || context == NULL)
        sshd_selinux_setup_exec_context(authctxt->pw->pw_name);
    freecon(context);
#endif


Thomas, what do you think?

I also built testing package with this change for Dominick to rawhide test package [2] if it makes any difference.


[1] pkgs.fedoraproject.org/cgit/openssh.git/commit/?id=c2b35d09c06efb3c78d76dd47ebfbbb23c5430a4
[2] http://koji.fedoraproject.org/koji/taskinfo?taskID=11627004
Comment 42 dac.override 2015-10-29 11:19:31 EDT
I just tested the above with root login and it did not change anything visible. sshd is still trying to log to audit with root login context

Next i will try unpriv login users to see if it still wants to log to journal
Comment 43 dac.override 2015-10-29 11:33:02 EDT
hmm with unpriv users i notice a slight difference

When i log in, then ssh does not try to log to journal anymore. However now when i log out it logs to journal instead.

So it *seems* the event moved from login to logout

Oct 29 16:30:06 fedoratest1 sshd[5279]: Received disconnect from 172.20.0.1: 11: disconnected by user
Oct 29 16:30:06 fedoratest1 kernel: audit: type=1400 audit(1446132606.717:1083): avc:  denied  { search } for  pid=5279 comm="sshd" name="journal" dev="tmpfs" ino=8941 scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sd_journald.runtime_file:s0 tclass=dir permissive=1
Oct 29 16:30:06 fedoratest1 kernel: audit: type=1400 audit(1446132606.717:1083): avc:  denied  { write } for  pid=5279 comm="sshd" name="dev-log" dev="tmpfs" ino=8948 scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sd_journald.runtime_file:s0 tclass=sock_file permissive=1
Oct 29 16:30:06 fedoratest1 kernel: audit: type=1400 audit(1446132606.717:1083): avc:  denied  { sendto } for  pid=5279 comm="sshd" path="/run/systemd/journal/dev-log" scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:sys.subj:s0 tclass=unix_dgram_socket permissive=1
Comment 44 Fedora Update System 2015-10-31 22:29:28 EDT
openssh-7.1p1-4.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
Comment 45 Tomas Mraz 2015-11-02 04:39:46 EST
(In reply to Jakub Jelen from comment #41)
> There is still the theoretical case when we will not have pam_selinux in PAM
> stack, which would not set any execcon. So probably most defensive solution
> would be checking the execcon after pam returns and if it was not set, fall
> back to native setting:
> 
> #ifdef USE_PAM
>     if (options.use_pam) {
>         do_pam_setcred(1);
>         do_pam_session();
>     }
> #endif
> #ifdef WITH_SELINUX
>     char *context;
>     if (getexeccon(&context) != 0 || context == NULL)
>         sshd_selinux_setup_exec_context(authctxt->pw->pw_name);
>     freecon(context);
> #endif
> 
> 
> Thomas, what do you think?

This should be correct change I believe. I think I originally completely disabled setting up the exec context by sshd but that might get lost during the later rebases.
Comment 46 Jakub Jelen 2015-11-03 10:33:13 EST
Tomas, you are right. I wrote too fast before properly trying out. The problem Dominick is pointing out is about the one "unconfined_t" (target context user) event, which is in latest version when you are connection as a root:

> Oct 29 16:40:46 rawhide audit[15183]: CRED_ACQ pid=15183 uid=0 auid=0 ses=8 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='op=PAM:setcred grantors=pam_unix acct="root" exe="/usr/sbin/sshd" hostname=127.0.0.1 addr=127.0.0.1 terminal=ssh res=success'

After root authentication, the pam_setcred() function is called once more, which results in three CRED_ACQ audit messages for root logins, which creates different audit trail in latest version. This needs to be certainly fixed.


This is caused by some more checks for root privileges, that were not dropped and therefore we reinitialize pam credentials and several more things once more.


This check in do_setusercontext (session.c):

	if (platform_privileged_uidswap()) {

does not check if we want root login, but only if we have capability (based on getuid(), geteuid()), which does not necessarily apply for SELinux logins. We should rather check if we already did all the transition or we are in the target child (is_child = 1 variable differentiate the process which will further exec bash or command and we don't want to do setting contexts again, since the context should be set up from parent process). Changing to 

	if (platform_privileged_uidswap() && !is_child) {

should do the magic and remote the difference between root/non-root login.


But for normal users there are some writes into journal if I am right, but I don't seem to be able to get the AVC even when I install local auditallow policy:

    echo '(auditallow guest_t devlog_t (unix_dgram_socket (sendto)))' > s.cil && semodule -i s.cil

which should be corresponding to your last AVC, if I am right.


Anyway adding scratch build with fixed root logins:
http://koji.fedoraproject.org/koji/taskinfo?taskID=11688095
Comment 47 dac.override 2015-11-03 10:41:05 EST
That rule is wrong, devlog_t is a object type and unix_dgram_socket is checked on the subject.

You would need instead:

auditallow guest_t kernel_t (unix_dgram_socket (sendto)))

I will try out the latest scratch build to see if the root login audit logging is fixed.
Comment 48 Jakub Jelen 2015-11-03 11:09:45 EST
(In reply to dac.override from comment #47)
> That rule is wrong, devlog_t is a object type and unix_dgram_socket is
> checked on the subject.

Thanks! It works this way. I am able to separate the problem to these two messages issued from the context of the target user. But if I turn more verbose logging, I have got journal full of other log other messages.

Nov 03 16:57:25 rawhide sshd[24282]: Received disconnect from 127.0.0.1: 11: disconnected by user
Nov 03 16:57:25 rawhide sshd[24282]: Disconnected from 127.0.0.1

I think this will not be fixed, because it would require some ugly hack, similar to chroot logging (preserving FD from parent and logging). Our policy doesn't block this so it seems to me cleaner.
Comment 49 dac.override 2015-11-03 11:17:01 EST
The root login audit logging/routing table reading issue is fixed in scratch build above

Now we only have to deal with the unpriv journal logging issue:

last time i checked it *seemed* to no longer happen on log-in, but only on log-out

So make sure to also look what sshd does when the unpriv user logs out.
Comment 50 dac.override 2015-11-03 11:18:27 EST
ok thats fine with me. I am already happy that the root logging audit issue is resolved.
Comment 51 dac.override 2015-11-03 14:45:46 EST
By the way. I started seeing these:

AVC avc:  denied  { search } for  pid=2230 comm="sshd" name="ssh" dev="dm-1" ino=1179891 scontext=wheel.id:wheel.role:wheel.subj:s0-s0:c0.c1023 tcontext=sys.id:sys.role:ssh_client.config_file:s0 tclass=dir permissive=0

so sshd (running with the context of the unpriv login user) wants to traverse /etc/ssh when the unpriv user logs in. (this event *seems* relatively new as i did not see this before)
Comment 52 Jakub Jelen 2015-11-04 04:00:41 EST
I see this line in strace, which should correspond to your AVC:

[pid 25565] stat("/etc/ssh/sshrc", 0x7fff988de440) = -1 ENOENT (No such file or directory)

Based on manual, it is documented behaviour if the users file is not available in ~/.ssh/rc (see man sshd). This file is stat just before executing bash or command and it is obvious it is in target user context.
Comment 53 dac.override 2015-11-04 04:06:08 EST
Okay i suppose i will allow all login shells to read system-wide ssh_client config files.
Comment 55 Fedora Update System 2015-11-04 05:16:29 EST
openssh-7.1p1-5.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-74d5902b06
Comment 56 Fedora Update System 2015-11-04 20:28:18 EST
openssh-7.1p1-5.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update openssh'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-74d5902b06
Comment 57 Fedora Update System 2015-11-05 17:23:59 EST
openssh-7.1p1-5.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.