* [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid
@ 2020-03-28 22:09 Topi Miettinen
2020-03-29 9:27 ` Dominick Grift
0 siblings, 1 reply; 6+ messages in thread
From: Topi Miettinen @ 2020-03-28 22:09 UTC (permalink / raw)
To: selinux
Mount selinuxfs with mount flags nodev,noexec and nosuid. It's not
likely that this has any effect, but it's visually more pleasing.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
---
libselinux/src/load_policy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
index fa1a3bf1..3e4020a9 100644
--- a/libselinux/src/load_policy.c
+++ b/libselinux/src/load_policy.c
@@ -279,7 +279,7 @@ int selinux_init_load_policy(int *enforce)
const char *mntpoint = NULL;
/* First make sure /sys is mounted */
if (mount("sysfs", "/sys", "sysfs", 0, 0) == 0 || errno == EBUSY) {
- if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, 0, 0) == 0
|| errno == EBUSY) {
+ if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, MS_NODEV |
MS_NOEXEC | MS_NOSUID, 0) == 0 || errno == EBUSY) {
mntpoint = SELINUXMNT;
} else {
/* check old mountpoint */
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid
2020-03-28 22:09 [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid Topi Miettinen
@ 2020-03-29 9:27 ` Dominick Grift
2020-03-29 11:29 ` Topi Miettinen
0 siblings, 1 reply; 6+ messages in thread
From: Dominick Grift @ 2020-03-29 9:27 UTC (permalink / raw)
To: Topi Miettinen; +Cc: selinux
Topi Miettinen <toiwoton@gmail.com> writes:
> Mount selinuxfs with mount flags nodev,noexec and nosuid. It's not
> likely that this has any effect, but it's visually more pleasing.
will nodev interfere with this?
File: /sys/fs/selinux/null
Size: 0 Blocks: 0 IO Block: 4096 character special file
Device: 15h/21d Inode: 23 Links: 1 Device type: 1,3
Access: (0666/crw-rw-rw-) Uid: ( 0/ root) Gid: ( 0/ root)
Context: sys.id:sys.role:null.isid:s0
Access: 2020-03-28 13:04:05.578999988 +0100
Modify: 2020-03-28 13:04:05.578999988 +0100
Change: 2020-03-28 13:04:05.578999988 +0100
Birth: -
/sys/fs/selinux/null: character special (1/3)
>
> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
> ---
> libselinux/src/load_policy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
> index fa1a3bf1..3e4020a9 100644
> --- a/libselinux/src/load_policy.c
> +++ b/libselinux/src/load_policy.c
> @@ -279,7 +279,7 @@ int selinux_init_load_policy(int *enforce)
> const char *mntpoint = NULL;
> /* First make sure /sys is mounted */
> if (mount("sysfs", "/sys", "sysfs", 0, 0) == 0 || errno == EBUSY) {
> - if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, 0, 0) == 0
> || errno == EBUSY) {
> + if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, MS_NODEV |
> MS_NOEXEC | MS_NOSUID, 0) == 0 || errno == EBUSY) {
> mntpoint = SELINUXMNT;
> } else {
> /* check old mountpoint */
--
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6 E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid
2020-03-29 9:27 ` Dominick Grift
@ 2020-03-29 11:29 ` Topi Miettinen
2020-03-29 20:44 ` Stephen Smalley
0 siblings, 1 reply; 6+ messages in thread
From: Topi Miettinen @ 2020-03-29 11:29 UTC (permalink / raw)
To: Dominick Grift; +Cc: selinux
On 29.3.2020 12.27, Dominick Grift wrote:
> Topi Miettinen <toiwoton@gmail.com> writes:
>
>> Mount selinuxfs with mount flags nodev,noexec and nosuid. It's not
>> likely that this has any effect, but it's visually more pleasing.
>
> will nodev interfere with this?
>
> File: /sys/fs/selinux/null
> Size: 0 Blocks: 0 IO Block: 4096 character special file
> Device: 15h/21d Inode: 23 Links: 1 Device type: 1,3
> Access: (0666/crw-rw-rw-) Uid: ( 0/ root) Gid: ( 0/ root)
> Context: sys.id:sys.role:null.isid:s0
> Access: 2020-03-28 13:04:05.578999988 +0100
> Modify: 2020-03-28 13:04:05.578999988 +0100
> Change: 2020-03-28 13:04:05.578999988 +0100
> Birth: -
>
> /sys/fs/selinux/null: character special (1/3)
That device does not give me joy. Yes, the patch prevents it from being
used. But I didn't see any problems in the logs, even with something
else mounted over it (adding InaccessiblePaths=/sys/fs/selinux/null to
systemd unit files). The device file was added pretty early to Linux,
perhaps it was needed then, but not anymore?
Judging from internet searches, maybe it's only used by Android. They
seem to use a forked version of libselinux anyway.
-Topi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid
2020-03-29 11:29 ` Topi Miettinen
@ 2020-03-29 20:44 ` Stephen Smalley
2020-03-30 6:13 ` Topi Miettinen
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2020-03-29 20:44 UTC (permalink / raw)
To: Topi Miettinen; +Cc: Dominick Grift, selinux
On Sun, Mar 29, 2020 at 7:30 AM Topi Miettinen <toiwoton@gmail.com> wrote:
>
> On 29.3.2020 12.27, Dominick Grift wrote:
> > Topi Miettinen <toiwoton@gmail.com> writes:
> >
> >> Mount selinuxfs with mount flags nodev,noexec and nosuid. It's not
> >> likely that this has any effect, but it's visually more pleasing.
> >
> > will nodev interfere with this?
> >
> > File: /sys/fs/selinux/null
> > Size: 0 Blocks: 0 IO Block: 4096 character special file
> > Device: 15h/21d Inode: 23 Links: 1 Device type: 1,3
> > Access: (0666/crw-rw-rw-) Uid: ( 0/ root) Gid: ( 0/ root)
> > Context: sys.id:sys.role:null.isid:s0
> > Access: 2020-03-28 13:04:05.578999988 +0100
> > Modify: 2020-03-28 13:04:05.578999988 +0100
> > Change: 2020-03-28 13:04:05.578999988 +0100
> > Birth: -
> >
> > /sys/fs/selinux/null: character special (1/3)
>
> That device does not give me joy. Yes, the patch prevents it from being
> used. But I didn't see any problems in the logs, even with something
> else mounted over it (adding InaccessiblePaths=/sys/fs/selinux/null to
> systemd unit files). The device file was added pretty early to Linux,
> perhaps it was needed then, but not anymore?
>
> Judging from internet searches, maybe it's only used by Android. They
> seem to use a forked version of libselinux anyway.
/sys/fs/selinux/null is used by the kernel; SELinux closes any open
file descriptors not authorized for the new process context upon a
context-changing exec, and replaces them with a reference to
/sys/fs/selinux/null. This was introduced because /dev/null couldn't
be guaranteed to exist or be available at all times. nodev likely has
no effect on this usage because it is probably only checked when a
userspace process tries to open it.
That said, I don't really understand what you think you are gaining by
adding these mount options to selinuxfs. What threat are you
mitigating? It is a kernel pseudo filesystem that doesn't support
dynamic file creation by userspace and whose contents are entirely
determined by the kernel.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid
2020-03-29 20:44 ` Stephen Smalley
@ 2020-03-30 6:13 ` Topi Miettinen
2020-04-27 17:22 ` Stephen Smalley
0 siblings, 1 reply; 6+ messages in thread
From: Topi Miettinen @ 2020-03-30 6:13 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Dominick Grift, selinux
On 29.3.2020 23.44, Stephen Smalley wrote:
> On Sun, Mar 29, 2020 at 7:30 AM Topi Miettinen <toiwoton@gmail.com> wrote:
>>
>> On 29.3.2020 12.27, Dominick Grift wrote:
>>> Topi Miettinen <toiwoton@gmail.com> writes:
>>>
>>>> Mount selinuxfs with mount flags nodev,noexec and nosuid. It's not
>>>> likely that this has any effect, but it's visually more pleasing.
>>>
>>> will nodev interfere with this?
>>>
>>> File: /sys/fs/selinux/null
>>> Size: 0 Blocks: 0 IO Block: 4096 character special file
>>> Device: 15h/21d Inode: 23 Links: 1 Device type: 1,3
>>> Access: (0666/crw-rw-rw-) Uid: ( 0/ root) Gid: ( 0/ root)
>>> Context: sys.id:sys.role:null.isid:s0
>>> Access: 2020-03-28 13:04:05.578999988 +0100
>>> Modify: 2020-03-28 13:04:05.578999988 +0100
>>> Change: 2020-03-28 13:04:05.578999988 +0100
>>> Birth: -
>>>
>>> /sys/fs/selinux/null: character special (1/3)
>>
>> That device does not give me joy. Yes, the patch prevents it from being
>> used. But I didn't see any problems in the logs, even with something
>> else mounted over it (adding InaccessiblePaths=/sys/fs/selinux/null to
>> systemd unit files). The device file was added pretty early to Linux,
>> perhaps it was needed then, but not anymore?
>>
>> Judging from internet searches, maybe it's only used by Android. They
>> seem to use a forked version of libselinux anyway.
>
> /sys/fs/selinux/null is used by the kernel; SELinux closes any open
> file descriptors not authorized for the new process context upon a
> context-changing exec, and replaces them with a reference to
> /sys/fs/selinux/null. This was introduced because /dev/null couldn't
> be guaranteed to exist or be available at all times. nodev likely has
> no effect on this usage because it is probably only checked when a
> userspace process tries to open it.
Perhaps then the device should not be visible to user space at all, or
at least not usable (which is the effect of MS_NODEV)? The file
descriptor replacement thing seems to work also when /sys{,/fs/selinux}
is not mounted in the mount namespace of the process, at least I haven't
seen any problems.
> That said, I don't really understand what you think you are gaining by
> adding these mount options to selinuxfs. What threat are you
> mitigating? It is a kernel pseudo filesystem that doesn't support
> dynamic file creation by userspace and whose contents are entirely
> determined by the kernel.
I don't think there's any change to threat situation (a process which
shouldn't have access to /dev/null, gains access by using
/sys/fs/selinux/null? Not very credible) or even any other effect from
this change, but I don't like it when selinuxfs always shows up when I
grep for filesystems without nodev/noexec/nosuid. So the gain is visual.
What's the purpose and gain of having the filesystem mounted with
dev,exec,suid, which for other filesystems than selinuxfs are the more
dangerous options?
-Topi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid
2020-03-30 6:13 ` Topi Miettinen
@ 2020-04-27 17:22 ` Stephen Smalley
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Smalley @ 2020-04-27 17:22 UTC (permalink / raw)
To: Topi Miettinen; +Cc: Stephen Smalley, Dominick Grift, SElinux list
On Mon, Mar 30, 2020 at 2:14 AM Topi Miettinen <toiwoton@gmail.com> wrote:
>
> On 29.3.2020 23.44, Stephen Smalley wrote:
> > On Sun, Mar 29, 2020 at 7:30 AM Topi Miettinen <toiwoton@gmail.com> wrote:
> >>
> >> On 29.3.2020 12.27, Dominick Grift wrote:
> >>> Topi Miettinen <toiwoton@gmail.com> writes:
> >>>
> >>>> Mount selinuxfs with mount flags nodev,noexec and nosuid. It's not
> >>>> likely that this has any effect, but it's visually more pleasing.
> >>>
> >>> will nodev interfere with this?
> >>>
> >>> File: /sys/fs/selinux/null
> >>> Size: 0 Blocks: 0 IO Block: 4096 character special file
> >>> Device: 15h/21d Inode: 23 Links: 1 Device type: 1,3
> >>> Access: (0666/crw-rw-rw-) Uid: ( 0/ root) Gid: ( 0/ root)
> >>> Context: sys.id:sys.role:null.isid:s0
> >>> Access: 2020-03-28 13:04:05.578999988 +0100
> >>> Modify: 2020-03-28 13:04:05.578999988 +0100
> >>> Change: 2020-03-28 13:04:05.578999988 +0100
> >>> Birth: -
> >>>
> >>> /sys/fs/selinux/null: character special (1/3)
> >>
> >> That device does not give me joy. Yes, the patch prevents it from being
> >> used. But I didn't see any problems in the logs, even with something
> >> else mounted over it (adding InaccessiblePaths=/sys/fs/selinux/null to
> >> systemd unit files). The device file was added pretty early to Linux,
> >> perhaps it was needed then, but not anymore?
> >>
> >> Judging from internet searches, maybe it's only used by Android. They
> >> seem to use a forked version of libselinux anyway.
> >
> > /sys/fs/selinux/null is used by the kernel; SELinux closes any open
> > file descriptors not authorized for the new process context upon a
> > context-changing exec, and replaces them with a reference to
> > /sys/fs/selinux/null. This was introduced because /dev/null couldn't
> > be guaranteed to exist or be available at all times. nodev likely has
> > no effect on this usage because it is probably only checked when a
> > userspace process tries to open it.
>
> Perhaps then the device should not be visible to user space at all, or
> at least not usable (which is the effect of MS_NODEV)? The file
> descriptor replacement thing seems to work also when /sys{,/fs/selinux}
> is not mounted in the mount namespace of the process, at least I haven't
> seen any problems.
>
> > That said, I don't really understand what you think you are gaining by
> > adding these mount options to selinuxfs. What threat are you
> > mitigating? It is a kernel pseudo filesystem that doesn't support
> > dynamic file creation by userspace and whose contents are entirely
> > determined by the kernel.
>
> I don't think there's any change to threat situation (a process which
> shouldn't have access to /dev/null, gains access by using
> /sys/fs/selinux/null? Not very credible) or even any other effect from
> this change, but I don't like it when selinuxfs always shows up when I
> grep for filesystems without nodev/noexec/nosuid. So the gain is visual.
>
> What's the purpose and gain of having the filesystem mounted with
> dev,exec,suid, which for other filesystems than selinuxfs are the more
> dangerous options?
I don't think we can switch to nodev since we have been exposing that
null device node forever and
we know of at least one user (Android). Android started with a
complete fork of libselinux but went
back to following upstream, although they still retain their own
custom additions. So I think at most we
could add noexec/nosuid here with no risk of userspace breakage.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-27 17:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-28 22:09 [PATCH] libselinux: mount selinuxfs nodev,noexec,nosuid Topi Miettinen
2020-03-29 9:27 ` Dominick Grift
2020-03-29 11:29 ` Topi Miettinen
2020-03-29 20:44 ` Stephen Smalley
2020-03-30 6:13 ` Topi Miettinen
2020-04-27 17:22 ` Stephen Smalley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).