Re: sysfs symlinks in genfscon
On Tue, Aug 29, 2017 at 5:50 AM Stephen Smalley wrote: > On Mon, 2017-08-28 at 14:58 -0700, Jeffrey Vander Stoep via Selinux > wrote: > > Genfs_contexts does not label symlinks in sysfs, instead it leaves > > them with the default “sysfs” label. Is this a bug? > > We excluded symlinks from genfscon labeling back when only proc was > using genfscon for per-file labeling and we encountered a compatibility > problem when /proc/net was moved to /proc/self/net for network > namespaces (see commit cited below). Suddenly applications were getting > proc_net_t:lnk_file read denials when accessing /proc/net because > existing policies only allowed proc_t:lnk_file read and > proc_net_t:dir/file read, so new kernel + old policy broke userspace, > which is not allowed by Linus. At the time, I couldn't see a use case > where we would need to support per-file labeling of symlinks in proc, > since the only meaningful operation on a proc symlink is read/getattr. > The real access controls are to the individual directories/files, not > the symlinks. > > We previously allowed read on all symlinks with "sysfs" label. I've added getattr here: https://android-review.googlesource.com/#/c/platform/system/sepolicy/+/470500/ Sounds like that should be sufficient to avoid breaking anything, at least for sysfs. > Do you truly need per-file labeling of symlinks in sysfs? What's the > use case? I guess it is an inconsistency between the support for sysfs > labeling via setxattr vs genfscon, but there are likely other more > significant inconsistencies due to the support for pathname regexes in > file_contexts versus pathname prefixes in genfscon, e.g. see > https://github.com/SELinuxProject/selinux-kernel/issues/29 > > That said, I agree it is an ugly hack and should likely be removed if > it doesn't cause compatibility problems (we'd need to test on RHEL 6/7 > at least). > > Have you measured to see the impact of switching from setxattr to > genfscon for sysfs labeling? > Yes, a couple of data points 1. Moving sysfs and debugfs to genfscon resulted in ~200 ms faster boot time on Marlin. 2. Also on Marlin, coming out of suspend with screen off (which happens somewhat regularly) takes about 500 ms. We were spending ~120 ms of that in restorecon of /sys/devices/system/cpu. Moving to genfscon reduced the restorecon time from 120 ms to 3 ms, resulting in a significant improvement in screen-off battery life. > > commit ea6b184f7d521a503ecab71feca6e4057562252b > Author: Stephen Smalley > Date: Mon Sep 22 15:41:19 2008 -0400 > > selinux: use default proc sid on symlinks > > As we are not concerned with fine-grained control over reading of > symlinks in proc, always use the default proc SID for all proc > symlinks. > This should help avoid permission issues upon changes to the proc > tree > as in the /proc/net -> /proc/self/net example. > This does not alter labeling of symlinks within /proc/pid > directories. > ls -Zd /proc/net output before and after the patch should show the > differenc > e. > > Signed-off-by: Stephen D. Smalley > Signed-off-by: James Morris > >
Re: sysfs symlinks in genfscon
On Tue, Aug 29, 2017 at 08:56:47AM -0400, Stephen Smalley wrote: > On Tue, 2017-08-29 at 08:54 -0400, Stephen Smalley wrote: > > On Mon, 2017-08-28 at 14:58 -0700, Jeffrey Vander Stoep via Selinux > > wrote: > > > Genfs_contexts does not label symlinks in sysfs, instead it leaves > > > them with the default “sysfs” label. Is this a bug? > > > > We excluded symlinks from genfscon labeling back when only proc was > > using genfscon for per-file labeling and we encountered a > > compatibility > > problem when /proc/net was moved to /proc/self/net for network > > namespaces (see commit cited below). Suddenly applications were > > getting > > proc_net_t:lnk_file read denials when accessing /proc/net because > > existing policies only allowed proc_t:lnk_file read and > > proc_net_t:dir/file read, so new kernel + old policy broke userspace, > > which is not allowed by Linus. At the time, I couldn't see a use > > case > > where we would need to support per-file labeling of symlinks in proc, > > since the only meaningful operation on a proc symlink is > > read/getattr. > > The real access controls are to the individual directories/files, not > > the symlinks. > > > > Do you truly need per-file labeling of symlinks in sysfs? What's the > > use case? I guess it is an inconsistency between the support for > > sysfs > > labeling via setxattr vs genfscon, but there are likely other more > > significant inconsistencies due to the support for pathname regexes > > in > > file_contexts versus pathname prefixes in genfscon, e.g. see > > https://github.com/SELinuxProject/selinux-kernel/issues/29 > > > > That said, I agree it is an ugly hack and should likely be removed if > > it doesn't cause compatibility problems (we'd need to test on RHEL > > 6/7 > > at least). > > I guess we should probably wrap the change with a policy capability so > that it can be conditionally enabled by new policies and then we don't > have to worry about any compatibility problems with old policies. A policy capability would be nice. I would be interested in reading more about the use-case for this as well. > > > > > Have you measured to see the impact of switching from setxattr to > > genfscon for sysfs labeling? > > > > commit ea6b184f7d521a503ecab71feca6e4057562252b > > Author: Stephen Smalley > > Date: Mon Sep 22 15:41:19 2008 -0400 > > > > selinux: use default proc sid on symlinks > > > > As we are not concerned with fine-grained control over reading of > > symlinks in proc, always use the default proc SID for all proc > > symlinks. > > This should help avoid permission issues upon changes to the proc > > tree > > as in the /proc/net -> /proc/self/net example. > > This does not alter labeling of symlinks within /proc/pid > > directories. > > ls -Zd /proc/net output before and after the patch should show > > the > > differenc > > e. > > > > Signed-off-by: Stephen D. Smalley > > Signed-off-by: James Morris -- Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift signature.asc Description: PGP signature
Re: sysfs symlinks in genfscon
On Tue, 2017-08-29 at 08:54 -0400, Stephen Smalley wrote: > On Mon, 2017-08-28 at 14:58 -0700, Jeffrey Vander Stoep via Selinux > wrote: > > Genfs_contexts does not label symlinks in sysfs, instead it leaves > > them with the default “sysfs” label. Is this a bug? > > We excluded symlinks from genfscon labeling back when only proc was > using genfscon for per-file labeling and we encountered a > compatibility > problem when /proc/net was moved to /proc/self/net for network > namespaces (see commit cited below). Suddenly applications were > getting > proc_net_t:lnk_file read denials when accessing /proc/net because > existing policies only allowed proc_t:lnk_file read and > proc_net_t:dir/file read, so new kernel + old policy broke userspace, > which is not allowed by Linus. At the time, I couldn't see a use > case > where we would need to support per-file labeling of symlinks in proc, > since the only meaningful operation on a proc symlink is > read/getattr. > The real access controls are to the individual directories/files, not > the symlinks. > > Do you truly need per-file labeling of symlinks in sysfs? What's the > use case? I guess it is an inconsistency between the support for > sysfs > labeling via setxattr vs genfscon, but there are likely other more > significant inconsistencies due to the support for pathname regexes > in > file_contexts versus pathname prefixes in genfscon, e.g. see > https://github.com/SELinuxProject/selinux-kernel/issues/29 > > That said, I agree it is an ugly hack and should likely be removed if > it doesn't cause compatibility problems (we'd need to test on RHEL > 6/7 > at least). I guess we should probably wrap the change with a policy capability so that it can be conditionally enabled by new policies and then we don't have to worry about any compatibility problems with old policies. > > Have you measured to see the impact of switching from setxattr to > genfscon for sysfs labeling? > > commit ea6b184f7d521a503ecab71feca6e4057562252b > Author: Stephen Smalley > Date: Mon Sep 22 15:41:19 2008 -0400 > > selinux: use default proc sid on symlinks > > As we are not concerned with fine-grained control over reading of > symlinks in proc, always use the default proc SID for all proc > symlinks. > This should help avoid permission issues upon changes to the proc > tree > as in the /proc/net -> /proc/self/net example. > This does not alter labeling of symlinks within /proc/pid > directories. > ls -Zd /proc/net output before and after the patch should show > the > differenc > e. > > Signed-off-by: Stephen D. Smalley > Signed-off-by: James Morris
Re: sysfs symlinks in genfscon
On Mon, 2017-08-28 at 14:58 -0700, Jeffrey Vander Stoep via Selinux wrote: > Genfs_contexts does not label symlinks in sysfs, instead it leaves > them with the default “sysfs” label. Is this a bug? We excluded symlinks from genfscon labeling back when only proc was using genfscon for per-file labeling and we encountered a compatibility problem when /proc/net was moved to /proc/self/net for network namespaces (see commit cited below). Suddenly applications were getting proc_net_t:lnk_file read denials when accessing /proc/net because existing policies only allowed proc_t:lnk_file read and proc_net_t:dir/file read, so new kernel + old policy broke userspace, which is not allowed by Linus. At the time, I couldn't see a use case where we would need to support per-file labeling of symlinks in proc, since the only meaningful operation on a proc symlink is read/getattr. The real access controls are to the individual directories/files, not the symlinks. Do you truly need per-file labeling of symlinks in sysfs? What's the use case? I guess it is an inconsistency between the support for sysfs labeling via setxattr vs genfscon, but there are likely other more significant inconsistencies due to the support for pathname regexes in file_contexts versus pathname prefixes in genfscon, e.g. see https://github.com/SELinuxProject/selinux-kernel/issues/29 That said, I agree it is an ugly hack and should likely be removed if it doesn't cause compatibility problems (we'd need to test on RHEL 6/7 at least). Have you measured to see the impact of switching from setxattr to genfscon for sysfs labeling? commit ea6b184f7d521a503ecab71feca6e4057562252b Author: Stephen Smalley Date: Mon Sep 22 15:41:19 2008 -0400 selinux: use default proc sid on symlinks As we are not concerned with fine-grained control over reading of symlinks in proc, always use the default proc SID for all proc symlinks. This should help avoid permission issues upon changes to the proc tree as in the /proc/net -> /proc/self/net example. This does not alter labeling of symlinks within /proc/pid directories. ls -Zd /proc/net output before and after the patch should show the differenc e. Signed-off-by: Stephen D. Smalley Signed-off-by: James Morris
