sshd_net_t should not be hard coded into the app.  We need to use a context file which a policy writer would provide and sshd could read to get the proper context.

Dominick,
how did you start sshd? I believe you ran it directly.

# ps -eZ |grep sshd

and re-test.

Miroslav, believe you me, this bug was triaged. sshd_net_t is hard coded. Hard coding identifiers is bad practice as i have explained almost a year ago here:

http://bachradsusi.livejournal.com/2239.html?thread=447#t447

Yes, I don't tell hard coding is OK.

I know, but that is the reason for this bugzilla (hard coded types), it wouldnt matter how i ran SSHD.

We want to have sshd which changes the process label and it is allowed only if it runs with the correct labeling. Of course, it should not be done by hard coding.

# grep -r sshd_net openssh-6.2p2
openssh-6.2p2/sshd.c:	ssh_selinux_change_context("sshd_net_t");
openssh-6.2p2/sshd.c.gsskex:	ssh_selinux_change_context("sshd_net_t");
openssh-6.2p2/sshd.c.log-usepam-no:	ssh_selinux_change_context("sshd_net_t");
openssh-6.2p2/sshd.c.vendor:	ssh_selinux_change_context("sshd_net_t");
openssh-6.2p2/sshd.c.fips:	ssh_selinux_change_context("sshd_net_t");

This was fixed in:

libselinux-2.1.13-19.fc20.x86_64
libselinux-utils-2.1.13-19.fc20.x86_64
libselinux-python-2.1.13-19.fc20.x86_64

Although you might want to get rid of the verbose output:

Security Context base_u:base_r:base_t Assigned
Key Creation Context base_u:base_r:base_t Assigned



However: Do get rid of the hard-coding issue please

(In reply to Dominick Grift from comment #7)

> 
> Although you might want to get rid of the verbose output:
> 
> Security Context base_u:base_r:base_t Assigned
> Key Creation Context base_u:base_r:base_t Assigned
> 

Forget the above my pam_selinux was still configured with verbose debug

(In reply to Miroslav Grepl from comment #6)
> We want to have sshd which changes the process label and it is allowed only
> if it runs with the correct labeling. Of course, it should not be done by
> hard coding.

I don' t really understand that reasoning though (i might just be ignorant). Whether sshd is allowed to change identifiers is defined in security policy.

All it needs to do is determine its current context and then calculate whether its allowed (by security policy) to change to the requested context.

If the policy allows it then: do it, else exit 1

Again, i might be oversimplifying this

(In reply to Dominick Grift from comment #9)
> (In reply to Miroslav Grepl from comment #6)
> > We want to have sshd which changes the process label and it is allowed only
> > if it runs with the correct labeling. Of course, it should not be done by
> > hard coding.
> 
> I don' t really understand that reasoning though (i might just be ignorant).
> Whether sshd is allowed to change identifiers is defined in security policy.
> 

Sure, it is. I'm not saying the opposite.

"dyntransition" if setcon() is used.

We just need to add a context file called sshd_context, then add name value pairs for anything sshd wants to do, if nothing is specified the setcon() does not happen, if a policy writer wants to add a context he needs to populate this file.

Yes, and now i think i know why that is. To prevent that login users end up in domains like mkhomedir_t or whatever.

(In reply to Dominick Grift from comment #12)
> Yes, and now i think i know why that is. To prevent that login users end up
> in domains like mkhomedir_t or whatever.

Grrr... no that's not it, You probably do this for the setcon() for sshd_t to sshd_net_t

(In reply to Daniel Walsh from comment #11)
> We just need to add a context file called sshd_context, then add name value
> pairs for anything sshd wants to do, if nothing is specified the setcon()
> does not happen, if a policy writer wants to add a context he needs to
> populate this file.

So there should be a check which tells me what the current sshd context is and I know I want to pick up sshd_net_t in the specific part of the code.

So I will have the following pair for example

sshd_net_t <-> sshd_t

Yes we could put something like that in default context or have a config file that says..

sshd_net = sshd_net_t

Currently we have for lxc domains.

# more /etc/selinux/targeted/contexts/lxc_contexts 
process = "system_u:system_r:svirt_lxc_net_t:s0"
file = "system_u:object_r:svirt_lxc_file_t:s0"
content = "system_u:object_r:virt_var_lib_t:s0"

Those context files, do they have some common format? I can see several formats used in files in /etc/selinux/targeted/contexts/ 

Is there any API in libselinux which can be used either to get a context file name or better to get a value? Or is it like a project defines it's own format and data and selinux-policy just provides a context file? 

If sshd read the context, should it read it every time before setcon() or is it enough to read it once during an initialization?

It is up to the application to define the format, although I now believe that name value pairs are the best format for this.

It should read the file every time or at least stat the file to see if has been updated.  I would doubt it takes long to read it.  Since the policy could have changed.

Libselinux would just return the path to the file.

It is up to the application to define the format, although I now believe that name value pairs are the best format for this.

It should read the file every time or at least stat the file to see if has been updated.  I would doubt it takes long to read it.  Since the policy could have changed.

Libselinux would just return the path to the file.

Created attachment 946481 [details]
use privsep_preauth context from selinux-policy

This patch changes how sshd choose an SELinux context for preauth privsep processes. It depends on selinux_openssh_contexts_path() from libselinux-2.2.2-8.

How it works:
- sshd gets the contexts filename path - /etc/selinux/targeted/contexts/openssh_contexts and looks for "privsep_preauth" key
- if it doesn't exist, sshd will use "sshd_net_t"
- if it's empty (privsep_preauth= ), sshd won't change the context
- otherwise, sshd will try to change the context to the read value. However, this operation is not fatal so when setcon() fails, sshd just logs a problem.


You can try it using a copr build [1] and /etc/selinux/targeted/contexts file containing:
privsep_preauth=sshd_net_t

[1] http://copr.fedoraproject.org/coprs/plautrba/openssh_testing/build/52674/

The above is not sufficient in my view:

1. change the location/name of the sshd selinux config file to for example:
 /etc/selinux/$SELINUXTYPE/contexts/openssh_contexts

2. implement a selinux awereness option in sshd_config that allow one to enable/disable sshd selinux awareness on runtime (example SELINUX=(true|false))

   - if openssh selinux awareness is enabled in sshd config then openssh requires /etc/selinux/$SELINUXTYPE/contexts/openssh_contexts to be present and valid

     if it does not meet the requirements then sshd should refuse to start with a message like for example: file /etc/selinux/$SELINUXTYPE/openssh_contexts not present or invalid

   - if openssh selinux awareness is disabled in sshd config then openssh should not look for /etc/selinux/$SELINUXTYPE/openssh_contexts and should disable all SELinux code

This implementation should also take into account all other existing SELinux related code in open-ssh

In the past some bad SELinux code managed to make it into upstream openssh, this bad SELinux code should, while were at it, also be fixed.

Actually, and this is up for debate, the file should probably be called something like

/etc/selinux/$SELINUXTYPE/contexts/openssh_privsep_preauth_context

Its content should probably instead be the full security context that openssh should use for "privsep_preauth"

Example of the content of the /etc/selinux/$SELINUXTYPE/contexts/openssh_privsep_context:

system_u:system_r:sshd_net_t:s0

If any other functionality requires customizable selinux security identifiers then each should have their own respective selinux context file using the same conventions

For an example look at the libvirt code and its accompanying /etc/selinux/$SELINUXTYPE/contexts/virtual_domain_context, and virtual_image_context

Actually no. We want to have a pair in these config files.

What we have for libvirt is wrong and we want to change it also.

Thanks for your input, dac.override.

(In reply to dac.override from comment #20)
> The above is not sufficient in my view:
> 
> 1. change the location/name of the sshd selinux config file to for example:
>  /etc/selinux/$SELINUXTYPE/contexts/openssh_contexts

In fact, this is already part the proposed patch. The path is returned by selinux_openssh_contexts_path() from libselinux. It's '/etc/selinux/targeted/contexts/openssh_contexts' by default  but it depends on SELinux policy type. 

> 2. implement a selinux awereness option in sshd_config that allow one to
> enable/disable sshd selinux awareness on runtime (example
> SELINUX=(true|false))

I don't think this is something worth to have. The Fedora openssh works with SELinux contexts in two cases:

1. it changes SELinux context for an unprivileged pre-authentication process which is responsible only for combination via network and gets sshd_net_t type. The type is hardcoded now, but the proposed patch changes it to read the type from the contexts type.
If you wanted to run 'unprivileged pre-authentication process' with 'sshd_t' type, you could set empty privsep_auth= value in the contexts file.

2. it changes SELinux context for an unprivileged post-authentication process which is run with user's credentials and user's type. It allows to have different port-forwarding rules for different user based on their SELinux user type.

I don't see why we should support running "unconfined" sshd. And it would mean that openssh need to aware about SELinux mode - is it enforcing, permissive?

>    - if openssh selinux awareness is enabled in sshd config then openssh
> requires /etc/selinux/$SELINUXTYPE/contexts/openssh_contexts to be present
> and valid
> 
>      if it does not meet the requirements then sshd should refuse to start
> with a message like for example: file
> /etc/selinux/$SELINUXTYPE/openssh_contexts not present or invalid
> 
>    - if openssh selinux awareness is disabled in sshd config then openssh
> should not look for /etc/selinux/$SELINUXTYPE/openssh_contexts and should
> disable all SELinux code
 
It would break sshd even in permissive mode.

> This implementation should also take into account all other existing SELinux
> related code in open-ssh

I'm not sure what you mean here.

(In reply to Petr Lautrbach from comment #23)

> > 1. change the location/name of the sshd selinux config file to for example:
> >  /etc/selinux/$SELINUXTYPE/contexts/openssh_contexts
> 
> In fact, this is already part the proposed patch. The path is returned by
> selinux_openssh_contexts_path() from libselinux. It's
> '/etc/selinux/targeted/contexts/openssh_contexts' by default  but it depends
> on SELinux policy type. 

Oh, right. I read https://bugzilla.redhat.com/show_bug.cgi?id=1008580#c19 wrong

> 
> > 2. implement a selinux awereness option in sshd_config that allow one to
> > enable/disable sshd selinux awareness on runtime (example
> > SELINUX=(true|false))
> 
> I don't think this is something worth to have. The Fedora openssh works with
> SELinux contexts in two cases:
> 
> 1. it changes SELinux context for an unprivileged pre-authentication process
> which is responsible only for combination via network and gets sshd_net_t
> type. The type is hardcoded now, but the proposed patch changes it to read
> the type from the contexts type.
> If you wanted to run 'unprivileged pre-authentication process' with 'sshd_t'
> type, you could set empty privsep_auth= value in the contexts file.
> 
> 2. it changes SELinux context for an unprivileged post-authentication
> process which is run with user's credentials and user's type. It allows to
> have different port-forwarding rules for different user based on their
> SELinux user type.
> 
> I don't see why we should support running "unconfined" sshd. And it would
> mean that openssh need to aware about SELinux mode - is it enforcing,
> permissive?
> 

Okay, i think it is not optimal. I stated an alternative for the record. It is up to the professionals to make the ultimate decision.

I think that the solution i suggested may provide the best experience, as it provides the end-user to disable OpenSSHd SELinux awareness on runtime altogether.

> >    - if openssh selinux awareness is enabled in sshd config then openssh
> > requires /etc/selinux/$SELINUXTYPE/contexts/openssh_contexts to be present
> > and valid
> > 
> >      if it does not meet the requirements then sshd should refuse to start
> > with a message like for example: file
> > /etc/selinux/$SELINUXTYPE/openssh_contexts not present or invalid
> > 
> >    - if openssh selinux awareness is disabled in sshd config then openssh
> > should not look for /etc/selinux/$SELINUXTYPE/openssh_contexts and should
> > disable all SELinux code
>  
> It would break sshd even in permissive mode.

I fail to see how, but i trust that you know what you are talking about.

I have stated, what i suspect, is an optimal alternative. For your consideration. It is up to you, the professionals, to make the ultimate decision.

I think hardcoding customizable security identifiers for whatever reason, be it fallback/failover or whatever is sub-optimal.
 
> 
> > This implementation should also take into account all other existing SELinux
> > related code in open-ssh
> 
> I'm not sure what you mean here.

Do a "grep -r unconfined_t" in the upstream openssh tree. It should return some. Those are other hardcoded security identifiers that made it upstream by accident.

Welp, I tested it (against better judgement) and it does not even work

undefined symbol: openssh_selinux_contexts_path (connection refused)

Have you even done some basic testing?

Screen recording of test procedure on youtube for reference purposes:

https://www.youtube.com/watch?v=a0NhKIcv_6M&feature=youtube_gdata

> It depends on selinux_openssh_contexts_path() from libselinux-2.2.2-8.

It should have been libselinux-2.3-5, I used wrong libselinux changelog entry.

A temporary scratch build with fixed dependency for Rawhide - http://koji.fedoraproject.org/koji/taskinfo?taskID=7913038 

src.rpm - https://plautrba.fedorapeople.org/openssh/testing/openssh-6.6.1p1-5.testing.3.fc22.src.rpm

Yes this does fix it for me. I would not call the solution elegant but it does deal with the issue

You can close this as far as I am concerned (I am the original OP under a different nick)

openssh-6.6.1p1-6.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/openssh-6.6.1p1-6.fc21

Lukas,
could you add

'/etc/selinux/targeted/contexts/openssh_contexts'

to selinux-policy?

Package openssh-6.6.1p1-6.fc21:
* should fix your issue,
* was pushed to the Fedora 21 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing openssh-6.6.1p1-6.fc21'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-14298/openssh-6.6.1p1-6.fc21
then log in and leave karma (feedback).

Package openssh-6.6.1p1-7.fc21:
* should fix your issue,
* was pushed to the Fedora 21 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing openssh-6.6.1p1-7.fc21'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-14298/openssh-6.6.1p1-7.fc21
then log in and leave karma (feedback).

openssh-6.6.1p1-7.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.

I seem to be hitting some bug that *may or may* not be related to this. My system configuration has diverged from stock Fedora to the point where i can't tell for sure without trying it in stock config

The problem i see is when (confined) root logs in via ssh, for some reason "a" sshd process runs with sysadm's selinux credentials instead of sshd's credentials

Things seem to work fine with ordinary users

Again, this may not be related to this patch. Because besides this patch i am also using a modified /etc/pam.d/systemd-user (one with pam_selinux added to the stack to make the systemd session daemons run with the users credentials instead of the systems credentials)

A way to test this would be to map root to sysadm_u, and then to try logging in with ssh. AVC denials may occur where comm=sshd and scontext=sysadm_u:sysadm_r:sysadm_t:s0-s0:c0.c1023. if that is the case then it's more likely due to this patch

I can speculate

Does sshd re-exec it self? The debug log seems to imply that it does?

could it be that this is where sshd gets confused? it re-execs itself but uses the context that pam tells it to use for uid root?

I would assume that it knows not to consult pam for the execution context if it concerns a re-exec?

I think sshd re-execs itself but since pam would tell it the context for root is sysadm_u:sysadm_r:sysadm_t, that if it would listen to pam it would re-exec itself with that content. which would cause sshd to run with sysadm_u:sysadm_r:sysadm_t (at least temporary)

The question would then be: why does this only happen when root logs in? and not when unpriv users log in?

Yes i think i confirmed this..

I know sshd (re-)execs itself:

avc:  granted  { execute } for  pid=1446 comm="sshd" name="sshd" dev="dm-0" ino=1184435 scontext=sys.u:sys.r:sshd.common_subject tcontext=sys.u:sys.r:sshd.bin_object tclass=file

And i know that, since sshd runs as root, it wants to (re-)exec itself with the credentials pam tells it to use for user root.

But pam_selinux is called in /etc/pam.d/sshd, and so if sshd asks pam_selinux: "hey, what context do i needed to associate with root?", then pam_selinux will say: "sysadm.u:sysadm.r:sysadm.common_subject"

And then sshd end ups (re-)execing itself with "sysadm.u:sysadm.r:sysadm.common_subject" instead of the proper "sys.u:sys.r:sshd.common_subject"

Here is the proof:

avc:  granted  { dyntransition } for  pid=1446 comm="sshd" scontext=sys.u:sys.r:sshd.common_subject tcontext=sysadm.u:sysadm.r:sysadm.common_subject tclass=process

There is a difference between opening a ssh session on behalf of a user, and (re-)execing.

We can;t have sshd running with user SELinux credentials

(In reply to dac.override from comment #37)
> Yes i think i confirmed this..
> 
...
> 
> We can;t have sshd running with user SELinux credentials


Please see https://bugzilla.redhat.com/show_bug.cgi?id=1169149#c1

I saw it and i disagree.

The way i see it this is just a bug, and this argument:

"Yes, to change the context for the user session, the sshd process must be unrestricted."

To me is just utterly wrong. It works fine as long as the user log-in is not root. What makes root any different from other users in this perspective?

Anyhow, I said what i think needs to be said. If you all cannot or will not acknowledge the issue then i suppose it is case closed.