fedora-selinux / selinux-policy

selinux-policy for Fedora is a large patch off the mainline
GNU General Public License v2.0
164 stars 165 forks source link

SELinux blocking DynamicUser cache dir creation #231

Open eclipseo opened 5 years ago

eclipseo commented 5 years ago

W/ selinux-policy-3.14.2-40.fc29

The issue is SELinux blocking DynamicUser cache dir creation:

type=AVC msg=audit(1541174465.681:2046): avc:  denied  { create } for  pid=2655 comm="(pt-proxy)" name="dnscrypt-proxy" scontext=system_u:system_r:init_t:s0 tcontext=system_u:object_r:var_t:s0 tclass=dir permissive=0

localhost systemd[13460]: dnscrypt-proxy.service: Failed to set up special execution directory in /var/cache: Permission denied
localhost systemd[13460]: dnscrypt-proxy.service: Failed at step CACHE_DIRECTORY 

@wrabcak , could you have a look on this, this is related to https://github.com/systemd/systemd/issues/9583#issuecomment-412567468 I didn't notice it before because the dir was already created on my system but users are reporting errors: https://bugzilla.redhat.com/show_bug.cgi?id=1645598

eclipseo commented 5 years ago

Any chance you have time to fix this?

wrabcak commented 5 years ago

Hi @eclipseo ,

Could you run following commands:

# semanage fcontext -a -t init_var_lib_t "/var/cache/private(/.*)?"
# restorecon -Rv /var/cache

and then try to reproduce the issue?

Thanks, Lukas.

eclipseo commented 5 years ago

Issue reproduced after running these commands:

févr. 21 17:24:29 Themisto systemd[15337]: dnscrypt-proxy.service: Failed to set up special execution directory in /var/cache: Permission denied
févr. 21 17:24:29 Themisto systemd[15337]: dnscrypt-proxy.service: Failed at step CACHE_DIRECTORY spawning /usr/bin/dnscrypt-proxy: Permission denied
ghost commented 5 years ago

CacheDIrectory=, LogDirectory=, StateDirectory=, and DynamicUsers= in general are explained here:

http://0pointer.net/blog/dynamic-users-with-systemd.html

https://lore.kernel.org/selinux-refpolicy/87sgxozucw.fsf@gmail.com/T/#m31a444e8db6ebb10e069f8c522ce1beb7d8c8d32

In a nutshell:

/var/log/private is equal to /var/log to the service unit that calls LogDirectory= /var//cache/private is equal to /var/cache to the service unit that calls CacheDirectory= /var/lib/private is equal to /var/lib to the service unit that calls StateDirectory=

One should probably add to file_contexts.subs_dist:

/var/cache/private /var/cache /var/lib/private /var/lib /var/log/private /var/log

Example: myservice

myservice has a service unit that calls: StateDirectory=myservice systemd creates /var/lib/private/myservice (it uses setfscreatecon to address the labeling automatically if it is allowed to) systemd creates a symlink /var/lib/myservice that points to /var/lib/private/myservice The myservice service needs to be able to read the /var/lib/myservice symlink to get to /var/lib/private/myservice

  1. systemd needs to be allowed to create myservice_var_lib_t type directories (/var/lib/private/myservice)
  2. systemd needs to be allowed to create generic var_lib_t lnk_files (/var/lib/myservice)
  3. myservice_t needs to be allowed to read generic var_lib_t lnk_files (var/lib/myservice)
  4. the fc specs for /var/lib/private/myservice must match that of /var/lib/myservice

I hope this clears things up a little

Edit: systemd also needs to be able to list myservice_var_lib_t directories and to set attributes of myservice_var_lib_t directories

ghost commented 5 years ago

@eclipseo see where semanage fcontext -a -e /var/cache /var/cache/private && systemctl daemon-reexec && restorecon -RvF /var/cache/private && systemctl start dnscrypt-proxy gets you

Best to first remove any /var/cache/private/dnscrypt-proxy dirs if they exist, so that it can be determined whether systemd is allowed to create it

(The systemctl daemon-reexec is essential in case youre wondering)

ghost commented 5 years ago

o and in case you havent already: semanage fcontext -d -t init_var_lib_t "/var/cache/private(/.*)?"

ghost commented 5 years ago

Also show any avc denials that you see when you try it please.

And try it in permissive mode please so that we get a clearer picture of what all permissions are needed

eclipseo commented 5 years ago

@eclipseo see where semanage fcontext -a -e /var/cache /var/cache/private && systemctl daemon-reexec && restorecon -RvF /var/cache/private && systemctl start dnscrypt-proxy gets you

Best to first remove any /var/cache/private/dnscrypt-proxy dirs if they exist, so that it can be determined whether systemd is allowed to create it

(The systemctl daemon-reexec is essential in case youre wondering)

o and in case you havent already: semanage fcontext -d -t init_var_lib_t "/var/cache/private(/.*)?"

No change:

SELinux is preventing (pt-proxy) from create access on the directory dnscrypt-proxy.

*****  Plugin catchall (100. confidence) suggests   **************************

If you believe that (pt-proxy) should be allowed create access on the dnscrypt-proxy directory by default.
Then you should report this as a bug.
You can generate a local policy module to allow this access.
Do
allow this access for now by executing:
# ausearch -c '(pt-proxy)' --raw | audit2allow -M my-ptproxy
# semodule -X 300 -i my-ptproxy.pp

Additional Information:
Source Context                system_u:system_r:init_t:s0
Target Context                system_u:object_r:var_t:s0
Target Objects                dnscrypt-proxy [ dir ]
Source                        (pt-proxy)
Source Path                   (pt-proxy)
Port                          <Unknown>
Host                          Themisto
Source RPM Packages           
Target RPM Packages           
Policy RPM                    selinux-policy-3.14.2-49.fc29.noarch
Selinux Enabled               True
Policy Type                   targeted
Enforcing Mode                Enforcing
Host Name                     Themisto
Platform                      Linux Themisto 4.20.6-200.fc29.x86_64 #1 SMP Thu
                              Jan 31 15:50:43 UTC 2019 x86_64 x86_64
Alert Count                   5
First Seen                    2019-02-21 18:32:58 CET
Last Seen                     2019-02-21 18:32:58 CET
Local ID                      b6c66070-7ee3-471f-b862-10675a28256e

Raw Audit Messages
type=AVC msg=audit(1550770378.134:55381): avc:  denied  { create } for  pid=20662 comm="(pt-proxy)" name="dnscrypt-proxy" scontext=system_u:system_r:init_t:s0 tcontext=system_u:object_r:var_t:s0 tclass=dir permissive=0

Hash: (pt-proxy),init_t,var_t,dir,create
eclipseo commented 5 years ago

Same message popping up in permissive (but it works)

ghost commented 5 years ago

That is in enforcing mode. Its better to test it in permissive mode and then show all avc denials. Regardless:

  1. The dnscrypt-proxy service should ideally be targeted by the policy.
  2. However one might argue that systemd should be able to set these directories up for unconfined_service_t as well, and so one might want to allow init_t to create generic var_t dirs for this (thats optional i suppse)
  3. A bit late but the policy should ideally really have implemented that var_cache_t type for /var/cache

Anyways for now you should be able to use audit2allow to allow systemd to create /var/cache/private/dnscrypt-proxy, and hopfully @wrabcak will address this issue

eclipseo commented 5 years ago

That is in enforcing mode. Its better to test it in permissive mode and then show all avc denials.

I don't see any other AVC denials in permissive.

Anyways for now you should be able to use audit2allow to allow systemd to create /var/cache/private/dnscrypt-proxy, and hopfully @wrabcak will address this issue

I've added a policy to my package as a workaround: https://src.fedoraproject.org/rpms/dnscrypt-proxy/c/472a2814f1db781641fb022581517f63ff2a0f5a?branch=master

ghost commented 5 years ago

This might do it:

cat > allow_sd_to_create_cache_dirs_for_unconfined_services.te <<EOF
policy_module(allow_sd_to_create_cache_dirs_for_unconfined_services,1.0.0)
gen_require(\` type init_t, var_t; ')
allow init_t var_t:dir create_dir_perms;
EOF
make -f /usr/share/selinux//devel/Makefile allow_sd_to_create_cache_dirs_for_unconfined_services.pp
sudo semodule -i allow_sd_to_create_cache_dirs_for_unconfined_services.pp
ghost commented 5 years ago

That is in enforcing mode. Its better to test it in permissive mode and then show all avc denials.

I don't see any other AVC denials in permissive.

Anyways for now you should be able to use audit2allow to allow systemd to create /var/cache/private/dnscrypt-proxy, and hopfully @wrabcak will address this issue

I've added a policy to my package as a workaround: https://src.fedoraproject.org/rpms/dnscrypt-proxy/c/472a2814f1db781641fb022581517f63ff2a0f5a?branch=master

Right, that looks good but this shoulld really be fixed upstream and it really shouldnt be part of you package. (so remove it as soon as @wrabcak addressed it please)

ghost commented 5 years ago

Also you or upstream selinux-policy should ideally work on an proper selinux policy for dnscrypt-proxy

eclipseo commented 5 years ago

I'm no expert on SELinux, so ideally upstream selinux-policywould do it.

ghost commented 5 years ago

I (personally) agree

ghost commented 5 years ago

Althought I think there might be two things there:

1.The SELinux expert might argue similarly that he is no expert on dnscrypt-proxy and that ideally dnscrypt-proxy packager should do it. (I personally still prefer selinux-policy upstream address it)

  1. I think Fedora as a whole decided that packagers should include policy for their packages instead of upstream selinux-policy ( i don't agree with that but no one ever asked me.) i do agree that policy needs to be modularized, but it should be maintained centrally and selinux "experts" should have oversight
ghost commented 5 years ago

Also something to consider:

make sure that /var/cache/dnscrypt-proxy symlink and /var/cache/private/dnscrypr-proxy dirs gets removed when the package is uninstalled. Because systemd wont do it and neither will rpm (unless you tell it to)

ghost commented 5 years ago

I would also probably consider adding: AmbientCapabilities=cap_net_bind_service so that your service can bind socket to port 53 if the user decides to not enable the dnscrypt-proxy socket unit

ghost commented 5 years ago

Oh nevermind , i see that you do not support that scenario.

bachradsusi commented 5 years ago

That is in enforcing mode. Its better to test it in permissive mode and then show all avc denials.

I don't see any other AVC denials in permissive.

Anyways for now you should be able to use audit2allow to allow systemd to create /var/cache/private/dnscrypt-proxy, and hopfully @wrabcak will address this issue

I've added a policy to my package as a workaround: https://src.fedoraproject.org/rpms/dnscrypt-proxy/c/472a2814f1db781641fb022581517f63ff2a0f5a?branch=master

There's ongoing review of new packaging guidelines related to SELinux modules - https://pagure.io/packaging-committee/pull-request/814#

It would be great if you could follow the proposed guidelines - https://pagure.io/fork/vmojzis/packaging-committee/blob/8261bb88bc6226c40398a5ac58730cd35c0d0f98/f/guidelines/modules/ROOT/pages/SELinuxIndependentPolicy.adoc

@vmojzis ^^ can you provide a better link please?

ghost commented 5 years ago

I have to admit that it looks pretty well thought through, and i also admit that my concerns are not addressed when the policy would be maintained centrally. Because regardless of any scenario packagers are alway's able to include policy in non-conventional way's (like @eclipseo did above) that might compromise the base.

That is my biggest concern regardless. that a packager compromises the whole policy by injecting faulty policy, and the policy maintainer would'nt know about it until after the damage is done

ghost commented 5 years ago

Would be nice if we could have a (temporary) mirror of SELinuxIndependentPolicy.adoc on github, so that it's is easier to discuss for people that may not necessarily have a fas account.

ghost commented 5 years ago

One way I envision how this could be implemented (there are alternatives, but this one might be the least intrusive):

  1. Add equivalent rules for the 3 (see earlier comment)
  2. create 3 interfaces systemd_log_directory_template() systemd_state_directory_template() systemd_cache_dirrectory_template()
  3. Expect two arguments: caller domain, directory object type (example systemd_log_directory_template(myservice_t, myservice_log_t)
  4. Interface would look like this (simplified example for brevity) interface`systemd_log_directory_template',` gen_require(` attribute systemd_log_directory_type; ') logging_read_generic_var_log_symlinks($1) typeattribute $2 systemd_log_directory_type; ')
  5. in the systemd policy there would be the following: attribute systemd_log_directory_type;

    read and create generic symlinks in /var/log

    logging_create_generic_var_log_symlinks(init_t) logging_read_generic_var_log_symlinks(init)

    allow systemd to setup LogDirectory=

    allow init_t systemd_log_directory_type:dir create_dir_perms) allow init_t systemd_log_directory_type:dir list_dir_perms) allow init_t systemd_log_directory_type:dir setattr;

    for unconffined_service_t:

    logging_create_generic_var_log_dirs(init_t) logging_list_generic_var_log_dirs(init_t) logging_setattr_generic_var_log_dirs(init_t)

Same idea for Cache and StateDirectory.

ghost commented 5 years ago

There is atleast one potential (label) issue with the solution above though

For example i will take "isns":

Lets say the above is implemented and the isns packager decides to add: StateDirectory=isns to the isnsd.service unit. If the policy is extended with: systemd_state_directory_template(isnsd_t, isnsd_var_lib_t), then in theory things should just work ...

... until you relabel /var/lib:

the /var/lib/isns(/.*)? gen_context(system_u:object_r:isnsd_var_lib_t,s0) might cause setfiles to relabel the /var/lib/isns symlink created by systemd from var_lib_t to isnsd_var_lib_t, and this could cause things to break down.

so fs specs for these clients would need to be adjusted as well to address this:

/var/lib/isns -d gen_context(system_u:object_r:isnsd_var_lib_t,s0)
/var/lib/isns/.* gen_context(system_u:object_r:isnsd_var_lib_t,s0)