[RFC] refining systemd mountpoints

[RFC] refining systemd mountpoints

[Chris PeBenito]

I'd like to refine how the policy handles systemd's mounton so that it 
works similar to how we manage mountpoints for mount_t. Since systemd 
can be made to mount over just about anything, I'm looking at adding a 
new conditional that would allow init_t to mounton 
non_security_file_type, and then an interface like files_mountpoint().

The question is for the implementation of the interface; I see two 
options, either the interface allows mounton for all file-like classes, 
or the classes are specified as a parameter:

--------
init.te:
attribute init_mountpoint_type;
allow init_t init_mountpoint_type:dir_file_class_set mounton;

init.if:
interface(`init_mountpoint',`
typeattribute $1 init_mountpoint_type;
')
--------

or

--------
init.if:
interface(`init_mountpoint',`
allow init_t $1:$2 mounton;
')
--------

I like the first option because it is clearer since you can see the 
mounton in init.te, but that is excessive access.  The second option 
could be made to look like the first option, but it would need several 
attributes and interfaces, e.g. init_dir_mountpoint_type, 
init_file_mountpoint_type, etc. which isn't so desirable.

Any thoughts on this?

-- 
Chris PeBenito

Re: [RFC] refining systemd mountpoints

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

On Thu, Jan 09, 2020 at 04:06:38PM -0500, Chris PeBenito wrote:
> I'd like to refine how the policy handles systemd's mounton so that it works
> similar to how we manage mountpoints for mount_t. Since systemd can be made
> to mount over just about anything, I'm looking at adding a new conditional
> that would allow init_t to mounton non_security_file_type, and then an
> interface like files_mountpoint().
> 
> The question is for the implementation of the interface; I see two options,
> either the interface allows mounton for all file-like classes, or the
> classes are specified as a parameter:
> 
> --------
> init.te:
> attribute init_mountpoint_type;
> allow init_t init_mountpoint_type:dir_file_class_set mounton;
> 
> init.if:
> interface(`init_mountpoint',`
> typeattribute $1 init_mountpoint_type;
> ')
> --------
> 
> or
> 
> --------
> init.if:
> interface(`init_mountpoint',`
> allow init_t $1:$2 mounton;
> ')
> --------
> 
> I like the first option because it is clearer since you can see the mounton
> in init.te, but that is excessive access.  The second option could be made
> to look like the first option, but it would need several attributes and
> interfaces, e.g. init_dir_mountpoint_type, init_file_mountpoint_type, etc.
> which isn't so desirable.
> 
> Any thoughts on this?

I implemented the former in my policy. ie the dir_file_class_set equiv..

4163               (allow subj bind_path_obj_type_attribute (dirs (create)))
4164               (allow subj bind_path_obj_type_attribute list_dir_perms)
4165               (allow subj bind_path_obj_type_attribute (dir (mounton)))
4166               (allow subj bind_path_obj_type_attribute create_file_perms)
4167               (allow subj bind_path_obj_type_attribute (file (mounton)))

As you can see i even allow systemd to create the mountpoint in case it does not exist. For example if /etc/machine-id does not exist and I have a BindReadOnlyPath=/etc/machine-id then systemd will touch /etc/machine-id and mount it ro

It also generally buggy. Systemd does not (alway's) use setfscreatecon to create the mountpoints. And sometimes it does use setfscreatecon where it shouldnt.

https://github.com/systemd/systemd/issues/13762

> 
> -- 
> Chris PeBenito

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC] refining systemd mountpoints

[Nicolas Iooss]

On Thu, Jan 9, 2020 at 10:42 PM Dominick Grift <dac.override@gmail.com> wrote:
>
> On Thu, Jan 09, 2020 at 04:06:38PM -0500, Chris PeBenito wrote:
> > I'd like to refine how the policy handles systemd's mounton so that it works
> > similar to how we manage mountpoints for mount_t. Since systemd can be made
> > to mount over just about anything, I'm looking at adding a new conditional
> > that would allow init_t to mounton non_security_file_type, and then an
> > interface like files_mountpoint().
> >
> > The question is for the implementation of the interface; I see two options,
> > either the interface allows mounton for all file-like classes, or the
> > classes are specified as a parameter:
> >
> > --------
> > init.te:
> > attribute init_mountpoint_type;
> > allow init_t init_mountpoint_type:dir_file_class_set mounton;
> >
> > init.if:
> > interface(`init_mountpoint',`
> > typeattribute $1 init_mountpoint_type;
> > ')
> > --------
> >
> > or
> >
> > --------
> > init.if:
> > interface(`init_mountpoint',`
> > allow init_t $1:$2 mounton;
> > ')
> > --------
> >
> > I like the first option because it is clearer since you can see the mounton
> > in init.te, but that is excessive access.  The second option could be made
> > to look like the first option, but it would need several attributes and
> > interfaces, e.g. init_dir_mountpoint_type, init_file_mountpoint_type, etc.
> > which isn't so desirable.
> >
> > Any thoughts on this?
>
> I implemented the former in my policy. ie the dir_file_class_set equiv..
>
> 4163               (allow subj bind_path_obj_type_attribute (dirs (create)))
> 4164               (allow subj bind_path_obj_type_attribute list_dir_perms)
> 4165               (allow subj bind_path_obj_type_attribute (dir (mounton)))
> 4166               (allow subj bind_path_obj_type_attribute create_file_perms)
> 4167               (allow subj bind_path_obj_type_attribute (file (mounton)))
>
> As you can see i even allow systemd to create the mountpoint in case it does not exist. For example if /etc/machine-id does not exist and I have a BindReadOnlyPath=/etc/machine-id then systemd will touch /etc/machine-id and mount it ro
>
> It also generally buggy. Systemd does not (alway's) use setfscreatecon to create the mountpoints. And sometimes it does use setfscreatecon where it shouldnt.
>
> https://github.com/systemd/systemd/issues/13762
>

Using dir_file_class_set in the first option seems excessive. For
example systemd mounts /dev/kmsg as chr_file, as explained in
https://github.com/SELinuxProject/refpolicy/pull/144, and allowing
mounting on kmsg_device_t:dir would not make sense. Nevertheless, as
systemd seems to be the only things I know that mounts over /dev/kmsg,
it seems that dev_mounton_kmsg(init_t) could be replaced by
init_mountpoint(kmsg_device_t, chr_file) or
init_chr_mountpoint(kmsg_device_t).

That being said, I personally prefer the first option (with an
attribute) for types that are regular files and directories, as having
files labeled like directories is quite common.

In short, what about this?
--------
init.te:
attribute init_mountpoint_type;
allow init_t init_mountpoint_type:{dir, file} mounton;

init.if:
interface(`init_mountpoint',`
typeattribute $1 init_mountpoint_type;
')
interface(`init_mounton_chr',` # Or "init_chr_mountpoint"?
allow init_t $1:chr_file mounton; # An attribute could also be used here
')
--------

Cheers,
Nicolas

Re: [RFC] refining systemd mountpoints

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

On Thu, Jan 09, 2020 at 10:42:40PM +0100, Dominick Grift wrote:
> On Thu, Jan 09, 2020 at 04:06:38PM -0500, Chris PeBenito wrote:
> > I'd like to refine how the policy handles systemd's mounton so that it works
> > similar to how we manage mountpoints for mount_t. Since systemd can be made
> > to mount over just about anything, I'm looking at adding a new conditional
> > that would allow init_t to mounton non_security_file_type, and then an
> > interface like files_mountpoint().
> > 
> > The question is for the implementation of the interface; I see two options,
> > either the interface allows mounton for all file-like classes, or the
> > classes are specified as a parameter:
> > 
> > --------
> > init.te:
> > attribute init_mountpoint_type;
> > allow init_t init_mountpoint_type:dir_file_class_set mounton;
> > 
> > init.if:
> > interface(`init_mountpoint',`
> > typeattribute $1 init_mountpoint_type;
> > ')
> > --------
> > 
> > or
> > 
> > --------
> > init.if:
> > interface(`init_mountpoint',`
> > allow init_t $1:$2 mounton;
> > ')
> > --------
> > 
> > I like the first option because it is clearer since you can see the mounton
> > in init.te, but that is excessive access.  The second option could be made
> > to look like the first option, but it would need several attributes and
> > interfaces, e.g. init_dir_mountpoint_type, init_file_mountpoint_type, etc.
> > which isn't so desirable.
> > 
> > Any thoughts on this?
> 
> I implemented the former in my policy. ie the dir_file_class_set equiv..
> 
> 4163               (allow subj bind_path_obj_type_attribute (dirs (create)))
> 4164               (allow subj bind_path_obj_type_attribute list_dir_perms)
> 4165               (allow subj bind_path_obj_type_attribute (dir (mounton)))
> 4166               (allow subj bind_path_obj_type_attribute create_file_perms)
> 4167               (allow subj bind_path_obj_type_attribute (file (mounton)))
> 
> As you can see i even allow systemd to create the mountpoint in case it does not exist. For example if /etc/machine-id does not exist and I have a BindReadOnlyPath=/etc/machine-id then systemd will touch /etc/machine-id and mount it ro


Okay, I think I am wrong. It will not create the bind_path if it does not exist. Not sure how I got to this...

> 
> It also generally buggy. Systemd does not (alway's) use setfscreatecon to create the mountpoints. And sometimes it does use setfscreatecon where it shouldnt.
> 
> https://github.com/systemd/systemd/issues/13762
> 
> > 
> > -- 
> > Chris PeBenito
> 
> -- 
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift



-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC] refining systemd mountpoints

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 3673 bytes --]

On Mon, Jan 13, 2020 at 10:42:10AM +0100, Dominick Grift wrote:
> On Thu, Jan 09, 2020 at 10:42:40PM +0100, Dominick Grift wrote:
> > On Thu, Jan 09, 2020 at 04:06:38PM -0500, Chris PeBenito wrote:
> > > I'd like to refine how the policy handles systemd's mounton so that it works
> > > similar to how we manage mountpoints for mount_t. Since systemd can be made
> > > to mount over just about anything, I'm looking at adding a new conditional
> > > that would allow init_t to mounton non_security_file_type, and then an
> > > interface like files_mountpoint().
> > > 
> > > The question is for the implementation of the interface; I see two options,
> > > either the interface allows mounton for all file-like classes, or the
> > > classes are specified as a parameter:
> > > 
> > > --------
> > > init.te:
> > > attribute init_mountpoint_type;
> > > allow init_t init_mountpoint_type:dir_file_class_set mounton;
> > > 
> > > init.if:
> > > interface(`init_mountpoint',`
> > > typeattribute $1 init_mountpoint_type;
> > > ')

To be clear: I like this option:

1. You can BindPath/BindReadOnlyPath/BindReadWritePath/InaccessiblePath *any* file in theory. So dir_file_class_set seems appropriate.
2. You might wat to extend it just a little though:

allow init_t init_mountpoint_type:dir_file_class_set { getattr mounton };
allow init_t init_mountpoint_type:dir search_dir_perms;


> > > --------
> > > 
> > > or
> > > 
> > > --------
> > > init.if:
> > > interface(`init_mountpoint',`
> > > allow init_t $1:$2 mounton;
> > > ')
> > > --------
> > > 
> > > I like the first option because it is clearer since you can see the mounton
> > > in init.te, but that is excessive access.  The second option could be made
> > > to look like the first option, but it would need several attributes and
> > > interfaces, e.g. init_dir_mountpoint_type, init_file_mountpoint_type, etc.
> > > which isn't so desirable.
> > > 
> > > Any thoughts on this?
> > 
> > I implemented the former in my policy. ie the dir_file_class_set equiv..
> > 
> > 4163               (allow subj bind_path_obj_type_attribute (dirs (create)))
> > 4164               (allow subj bind_path_obj_type_attribute list_dir_perms)
> > 4165               (allow subj bind_path_obj_type_attribute (dir (mounton)))
> > 4166               (allow subj bind_path_obj_type_attribute create_file_perms)
> > 4167               (allow subj bind_path_obj_type_attribute (file (mounton)))
> > 
> > As you can see i even allow systemd to create the mountpoint in case it does not exist. For example if /etc/machine-id does not exist and I have a BindReadOnlyPath=/etc/machine-id then systemd will touch /etc/machine-id and mount it ro
> 
> 
> Okay, I think I am wrong. It will not create the bind_path if it does not exist. Not sure how I got to this...
> 
> > 
> > It also generally buggy. Systemd does not (alway's) use setfscreatecon to create the mountpoints. And sometimes it does use setfscreatecon where it shouldnt.
> > 
> > https://github.com/systemd/systemd/issues/13762
> > 
> > > 
> > > -- 
> > > Chris PeBenito
> > 
> > -- 
> > Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> > https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> > Dominick Grift
> 
> 
> 
> -- 
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift



-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]