| Message ID | 20200110142038.21602-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [RFC] selinux: add policy capability for systemd overhaul | expand |
On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche <cgzones@googlemail.com> wrote: > Support a SELinux overhaul of systemd by adding a policy capability. > > The systemd patch can be found at > https://github.com/systemd/systemd/pull/10023 > and has NOT yet been accepted. > > This is just a rfc to test the water. > --- > security/selinux/include/security.h | 1 + > security/selinux/ss/services.c | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) Generally the SELinux policy capabilities are reserved for *kernel* changes that potentially break compatibility with existing SELinux policies. I'm probably not the best person to talk about tricks/conventions used to do similar things in userspace, but you've come to the right place :) > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index ecdd610e6449..2853e462977f 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -79,6 +79,7 @@ enum { > POLICYDB_CAPABILITY_ALWAYSNETWORK, > POLICYDB_CAPABILITY_CGROUPSECLABEL, > POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, > + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, > __POLICYDB_CAPABILITY_MAX > }; > #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 55cf42945cba..cb50e187b181 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { > "extended_socket_class", > "always_check_network", > "cgroup_seclabel", > - "nnp_nosuid_transition" > + "nnp_nosuid_transition", > + "systemd_overhaul" > }; > > static struct selinux_ss selinux_ss; > -- > 2.24.1
On 1/10/20 11:55 AM, Paul Moore wrote: > On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche > <cgzones@googlemail.com> wrote: >> Support a SELinux overhaul of systemd by adding a policy capability. >> >> The systemd patch can be found at >> https://github.com/systemd/systemd/pull/10023 >> and has NOT yet been accepted. >> >> This is just a rfc to test the water. >> --- >> security/selinux/include/security.h | 1 + >> security/selinux/ss/services.c | 3 ++- >> 2 files changed, 3 insertions(+), 1 deletion(-) > > Generally the SELinux policy capabilities are reserved for *kernel* > changes that potentially break compatibility with existing SELinux > policies. I'm probably not the best person to talk about > tricks/conventions used to do similar things in userspace, but you've > come to the right place :) It was my suggestion to use policy capabilities for this. There is no separate mechanism for supporting major changes to userspace SELinux permission checks in a backward-compatible manner. Userspace already relies upon /sys/fs/selinux/{deny_unknown,reject_unknown} to get the handle_unknown setting from the kernel policy to decide how to handle unknown userspace classes/permissions. That however is insufficient for these changes to systemd's permission check because they go beyond introducing new classes and permissions and overhaul the existing checks. Policy capability seemed like the best way to do it, and getting it from the kernel is consistent with the fact that we are also getting the userspace classes/perms from the kernel via /sys/fs/selinux/class and the userspace access decisions from the kernel via /sys/fs/selinux/access (through the libselinux AVC, typically). > >> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h >> index ecdd610e6449..2853e462977f 100644 >> --- a/security/selinux/include/security.h >> +++ b/security/selinux/include/security.h >> @@ -79,6 +79,7 @@ enum { >> POLICYDB_CAPABILITY_ALWAYSNETWORK, >> POLICYDB_CAPABILITY_CGROUPSECLABEL, >> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, >> + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, >> __POLICYDB_CAPABILITY_MAX >> }; >> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) >> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c >> index 55cf42945cba..cb50e187b181 100644 >> --- a/security/selinux/ss/services.c >> +++ b/security/selinux/ss/services.c >> @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { >> "extended_socket_class", >> "always_check_network", >> "cgroup_seclabel", >> - "nnp_nosuid_transition" >> + "nnp_nosuid_transition", >> + "systemd_overhaul" >> }; >> >> static struct selinux_ss selinux_ss; >> -- >> 2.24.1 >
On 1/10/20 12:41 PM, Stephen Smalley wrote: > On 1/10/20 11:55 AM, Paul Moore wrote: >> On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche >> <cgzones@googlemail.com> wrote: >>> Support a SELinux overhaul of systemd by adding a policy capability. >>> >>> The systemd patch can be found at >>> https://github.com/systemd/systemd/pull/10023 >>> and has NOT yet been accepted. >>> >>> This is just a rfc to test the water. >>> --- >>> security/selinux/include/security.h | 1 + >>> security/selinux/ss/services.c | 3 ++- >>> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> Generally the SELinux policy capabilities are reserved for *kernel* >> changes that potentially break compatibility with existing SELinux >> policies. I'm probably not the best person to talk about >> tricks/conventions used to do similar things in userspace, but you've >> come to the right place :) > > It was my suggestion to use policy capabilities for this. There is no > separate mechanism for supporting major changes to userspace SELinux > permission checks in a backward-compatible manner. Userspace already > relies upon /sys/fs/selinux/{deny_unknown,reject_unknown} to get the > handle_unknown setting from the kernel policy to decide how to handle > unknown userspace classes/permissions. That however is insufficient for > these changes to systemd's permission check because they go beyond > introducing new classes and permissions and overhaul the existing > checks. Policy capability seemed like the best way to do it, and > getting it from the kernel is consistent with the fact that we are also > getting the userspace classes/perms from the kernel via > /sys/fs/selinux/class and the userspace access decisions from the kernel > via /sys/fs/selinux/access (through the libselinux AVC, typically). As to why we keep the userspace policy as part of the kernel policy and not as a separate entity: - It allows us to provide an effective atomicity in policy changes that may span both kernel and userspace components, - There is significant overlap between the contexts used in the kernel and userspace policies, since most userspace policy enforcers are using contexts obtained from the kernel for the subject (e.g. SO_PEERSEC/getpeercon) or for the object (e.g. getfilecon), - Policy lookups via /sys/fs/selinux/access are more efficient than performing an IPC to a userspace security server. Of course, in both cases, we try to maximize use of the libselinux AVC first to avoid needing to perform the policy lookup at all. There were experiments done with introducing support for userspace security server(s) for things like XACE/XSELinux and it was found to be unsatisfying both performance and security-wise. There are still cases where we would recommend userspace security server(s), such as when the userspace component is implementing a policy entirely distinct from that of the kernel (e.g. a remote document server implementing RaDAC policies, as in one of our earlier experimental research projects), but not for things like systemd. > >> >>> diff --git a/security/selinux/include/security.h >>> b/security/selinux/include/security.h >>> index ecdd610e6449..2853e462977f 100644 >>> --- a/security/selinux/include/security.h >>> +++ b/security/selinux/include/security.h >>> @@ -79,6 +79,7 @@ enum { >>> POLICYDB_CAPABILITY_ALWAYSNETWORK, >>> POLICYDB_CAPABILITY_CGROUPSECLABEL, >>> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, >>> + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, >>> __POLICYDB_CAPABILITY_MAX >>> }; >>> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) >>> diff --git a/security/selinux/ss/services.c >>> b/security/selinux/ss/services.c >>> index 55cf42945cba..cb50e187b181 100644 >>> --- a/security/selinux/ss/services.c >>> +++ b/security/selinux/ss/services.c >>> @@ -73,7 +73,8 @@ const char >>> *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { >>> "extended_socket_class", >>> "always_check_network", >>> "cgroup_seclabel", >>> - "nnp_nosuid_transition" >>> + "nnp_nosuid_transition", >>> + "systemd_overhaul" >>> }; >>> >>> static struct selinux_ss selinux_ss; >>> -- >>> 2.24.1 >> >
On 1/10/20 1:03 PM, Stephen Smalley wrote: > On 1/10/20 12:41 PM, Stephen Smalley wrote: >> On 1/10/20 11:55 AM, Paul Moore wrote: >>> On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche >>> <cgzones@googlemail.com> wrote: >>>> Support a SELinux overhaul of systemd by adding a policy capability. >>>> >>>> The systemd patch can be found at >>>> https://github.com/systemd/systemd/pull/10023 >>>> and has NOT yet been accepted. >>>> >>>> This is just a rfc to test the water. >>>> --- >>>> security/selinux/include/security.h | 1 + >>>> security/selinux/ss/services.c | 3 ++- >>>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> Generally the SELinux policy capabilities are reserved for *kernel* >>> changes that potentially break compatibility with existing SELinux >>> policies. I'm probably not the best person to talk about >>> tricks/conventions used to do similar things in userspace, but you've >>> come to the right place :) >> >> It was my suggestion to use policy capabilities for this. There is no >> separate mechanism for supporting major changes to userspace SELinux >> permission checks in a backward-compatible manner. Userspace already >> relies upon /sys/fs/selinux/{deny_unknown,reject_unknown} to get the >> handle_unknown setting from the kernel policy to decide how to handle >> unknown userspace classes/permissions. That however is insufficient >> for these changes to systemd's permission check because they go beyond >> introducing new classes and permissions and overhaul the existing >> checks. Policy capability seemed like the best way to do it, and >> getting it from the kernel is consistent with the fact that we are >> also getting the userspace classes/perms from the kernel via >> /sys/fs/selinux/class and the userspace access decisions from the >> kernel via /sys/fs/selinux/access (through the libselinux AVC, >> typically). > > As to why we keep the userspace policy as part of the kernel policy and > not as a separate entity: > > - It allows us to provide an effective atomicity in policy changes that > may span both kernel and userspace components, > > - There is significant overlap between the contexts used in the kernel > and userspace policies, since most userspace policy enforcers are using > contexts obtained from the kernel for the subject (e.g. > SO_PEERSEC/getpeercon) or for the object (e.g. getfilecon), > > - Policy lookups via /sys/fs/selinux/access are more efficient than > performing an IPC to a userspace security server. Of course, in both > cases, we try to maximize use of the libselinux AVC first to avoid > needing to perform the policy lookup at all. > > There were experiments done with introducing support for userspace > security server(s) for things like XACE/XSELinux and it was found to be > unsatisfying both performance and security-wise. > > There are still cases where we would recommend userspace security > server(s), such as when the userspace component is implementing a policy > entirely distinct from that of the kernel (e.g. a remote document server > implementing RaDAC policies, as in one of our earlier experimental > research projects), but not for things like systemd. All that said, I can see that we probably don't want a hardcoded reference to systemd in the kernel, since not everyone uses systemd ;) Perhaps what we need is for some range of policy capabilities to be user-defined, with generic names in the kernel and then userspace can choose to associate meaning with them. This would be a bit easier if we implemented a solution to the 2nd part of https://github.com/SELinuxProject/selinux/issues/55, i.e. pass capabilities to the kernel as a list of uninterpreted string names rather than a bitmap. Then the kernel only needs to recognize its own capability names and create selinuxfs nodes for all of them reflecting their policy values, but no hard-coded references to systemd required. > >> >>> >>>> diff --git a/security/selinux/include/security.h >>>> b/security/selinux/include/security.h >>>> index ecdd610e6449..2853e462977f 100644 >>>> --- a/security/selinux/include/security.h >>>> +++ b/security/selinux/include/security.h >>>> @@ -79,6 +79,7 @@ enum { >>>> POLICYDB_CAPABILITY_ALWAYSNETWORK, >>>> POLICYDB_CAPABILITY_CGROUPSECLABEL, >>>> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, >>>> + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, >>>> __POLICYDB_CAPABILITY_MAX >>>> }; >>>> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) >>>> diff --git a/security/selinux/ss/services.c >>>> b/security/selinux/ss/services.c >>>> index 55cf42945cba..cb50e187b181 100644 >>>> --- a/security/selinux/ss/services.c >>>> +++ b/security/selinux/ss/services.c >>>> @@ -73,7 +73,8 @@ const char >>>> *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { >>>> "extended_socket_class", >>>> "always_check_network", >>>> "cgroup_seclabel", >>>> - "nnp_nosuid_transition" >>>> + "nnp_nosuid_transition", >>>> + "systemd_overhaul" >>>> }; >>>> >>>> static struct selinux_ss selinux_ss; >>>> -- >>>> 2.24.1 >>> >> >
On 1/10/20 1:12 PM, Stephen Smalley wrote: > On 1/10/20 1:03 PM, Stephen Smalley wrote: >> On 1/10/20 12:41 PM, Stephen Smalley wrote: >>> On 1/10/20 11:55 AM, Paul Moore wrote: >>>> On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche >>>> <cgzones@googlemail.com> wrote: >>>>> Support a SELinux overhaul of systemd by adding a policy capability. >>>>> >>>>> The systemd patch can be found at >>>>> https://github.com/systemd/systemd/pull/10023 >>>>> and has NOT yet been accepted. >>>>> >>>>> This is just a rfc to test the water. >>>>> --- >>>>> security/selinux/include/security.h | 1 + >>>>> security/selinux/ss/services.c | 3 ++- >>>>> 2 files changed, 3 insertions(+), 1 deletion(-) >>>> >>>> Generally the SELinux policy capabilities are reserved for *kernel* >>>> changes that potentially break compatibility with existing SELinux >>>> policies. I'm probably not the best person to talk about >>>> tricks/conventions used to do similar things in userspace, but you've >>>> come to the right place :) >>> >>> It was my suggestion to use policy capabilities for this. There is >>> no separate mechanism for supporting major changes to userspace >>> SELinux permission checks in a backward-compatible manner. Userspace >>> already relies upon /sys/fs/selinux/{deny_unknown,reject_unknown} to >>> get the handle_unknown setting from the kernel policy to decide how >>> to handle unknown userspace classes/permissions. That however is >>> insufficient for these changes to systemd's permission check because >>> they go beyond introducing new classes and permissions and overhaul >>> the existing checks. Policy capability seemed like the best way to >>> do it, and getting it from the kernel is consistent with the fact >>> that we are also getting the userspace classes/perms from the kernel >>> via /sys/fs/selinux/class and the userspace access decisions from the >>> kernel via /sys/fs/selinux/access (through the libselinux AVC, >>> typically). >> >> As to why we keep the userspace policy as part of the kernel policy >> and not as a separate entity: >> >> - It allows us to provide an effective atomicity in policy changes >> that may span both kernel and userspace components, >> >> - There is significant overlap between the contexts used in the kernel >> and userspace policies, since most userspace policy enforcers are >> using contexts obtained from the kernel for the subject (e.g. >> SO_PEERSEC/getpeercon) or for the object (e.g. getfilecon), >> >> - Policy lookups via /sys/fs/selinux/access are more efficient than >> performing an IPC to a userspace security server. Of course, in both >> cases, we try to maximize use of the libselinux AVC first to avoid >> needing to perform the policy lookup at all. >> >> There were experiments done with introducing support for userspace >> security server(s) for things like XACE/XSELinux and it was found to >> be unsatisfying both performance and security-wise. >> >> There are still cases where we would recommend userspace security >> server(s), such as when the userspace component is implementing a >> policy entirely distinct from that of the kernel (e.g. a remote >> document server implementing RaDAC policies, as in one of our earlier >> experimental research projects), but not for things like systemd. > > All that said, I can see that we probably don't want a hardcoded > reference to systemd in the kernel, since not everyone uses systemd ;) > Perhaps what we need is for some range of policy capabilities to be > user-defined, with generic names in the kernel and then userspace can > choose to associate meaning with them. > > This would be a bit easier if we implemented a solution to the 2nd part > of https://github.com/SELinuxProject/selinux/issues/55, i.e. pass > capabilities to the kernel as a list of uninterpreted string names > rather than a bitmap. Then the kernel only needs to recognize its own > capability names and create selinuxfs nodes for all of them reflecting > their policy values, but no hard-coded references to systemd required. Actually, given that Christian is proposing defining entirely new classes for the new checks in systemd, maybe we don't need a policy capability at all? systemd can just check whether its new classes are defined and use that as the indicator of whether the policy support the new checks? > >> >>> >>>> >>>>> diff --git a/security/selinux/include/security.h >>>>> b/security/selinux/include/security.h >>>>> index ecdd610e6449..2853e462977f 100644 >>>>> --- a/security/selinux/include/security.h >>>>> +++ b/security/selinux/include/security.h >>>>> @@ -79,6 +79,7 @@ enum { >>>>> POLICYDB_CAPABILITY_ALWAYSNETWORK, >>>>> POLICYDB_CAPABILITY_CGROUPSECLABEL, >>>>> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, >>>>> + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, >>>>> __POLICYDB_CAPABILITY_MAX >>>>> }; >>>>> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) >>>>> diff --git a/security/selinux/ss/services.c >>>>> b/security/selinux/ss/services.c >>>>> index 55cf42945cba..cb50e187b181 100644 >>>>> --- a/security/selinux/ss/services.c >>>>> +++ b/security/selinux/ss/services.c >>>>> @@ -73,7 +73,8 @@ const char >>>>> *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { >>>>> "extended_socket_class", >>>>> "always_check_network", >>>>> "cgroup_seclabel", >>>>> - "nnp_nosuid_transition" >>>>> + "nnp_nosuid_transition", >>>>> + "systemd_overhaul" >>>>> }; >>>>> >>>>> static struct selinux_ss selinux_ss; >>>>> -- >>>>> 2.24.1 >>>> >>> >> >
On 1/10/20 1:54 PM, Stephen Smalley wrote: > On 1/10/20 1:12 PM, Stephen Smalley wrote: >> On 1/10/20 1:03 PM, Stephen Smalley wrote: >>> On 1/10/20 12:41 PM, Stephen Smalley wrote: >>>> On 1/10/20 11:55 AM, Paul Moore wrote: >>>>> On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche >>>>> <cgzones@googlemail.com> wrote: >>>>>> Support a SELinux overhaul of systemd by adding a policy capability. >>>>>> >>>>>> The systemd patch can be found at >>>>>> https://github.com/systemd/systemd/pull/10023 >>>>>> and has NOT yet been accepted. >>>>>> >>>>>> This is just a rfc to test the water. >>>>>> --- >>>>>> security/selinux/include/security.h | 1 + >>>>>> security/selinux/ss/services.c | 3 ++- >>>>>> 2 files changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> Generally the SELinux policy capabilities are reserved for *kernel* >>>>> changes that potentially break compatibility with existing SELinux >>>>> policies. I'm probably not the best person to talk about >>>>> tricks/conventions used to do similar things in userspace, but you've >>>>> come to the right place :) >>>> >>>> It was my suggestion to use policy capabilities for this. There is >>>> no separate mechanism for supporting major changes to userspace >>>> SELinux permission checks in a backward-compatible manner. >>>> Userspace already relies upon >>>> /sys/fs/selinux/{deny_unknown,reject_unknown} to get the >>>> handle_unknown setting from the kernel policy to decide how to >>>> handle unknown userspace classes/permissions. That however is >>>> insufficient for these changes to systemd's permission check because >>>> they go beyond introducing new classes and permissions and overhaul >>>> the existing checks. Policy capability seemed like the best way to >>>> do it, and getting it from the kernel is consistent with the fact >>>> that we are also getting the userspace classes/perms from the kernel >>>> via /sys/fs/selinux/class and the userspace access decisions from >>>> the kernel via /sys/fs/selinux/access (through the libselinux AVC, >>>> typically). >>> >>> As to why we keep the userspace policy as part of the kernel policy >>> and not as a separate entity: >>> >>> - It allows us to provide an effective atomicity in policy changes >>> that may span both kernel and userspace components, >>> >>> - There is significant overlap between the contexts used in the >>> kernel and userspace policies, since most userspace policy enforcers >>> are using contexts obtained from the kernel for the subject (e.g. >>> SO_PEERSEC/getpeercon) or for the object (e.g. getfilecon), >>> >>> - Policy lookups via /sys/fs/selinux/access are more efficient than >>> performing an IPC to a userspace security server. Of course, in both >>> cases, we try to maximize use of the libselinux AVC first to avoid >>> needing to perform the policy lookup at all. >>> >>> There were experiments done with introducing support for userspace >>> security server(s) for things like XACE/XSELinux and it was found to >>> be unsatisfying both performance and security-wise. >>> >>> There are still cases where we would recommend userspace security >>> server(s), such as when the userspace component is implementing a >>> policy entirely distinct from that of the kernel (e.g. a remote >>> document server implementing RaDAC policies, as in one of our earlier >>> experimental research projects), but not for things like systemd. >> >> All that said, I can see that we probably don't want a hardcoded >> reference to systemd in the kernel, since not everyone uses systemd ;) >> Perhaps what we need is for some range of policy capabilities to be >> user-defined, with generic names in the kernel and then userspace can >> choose to associate meaning with them. >> >> This would be a bit easier if we implemented a solution to the 2nd >> part of https://github.com/SELinuxProject/selinux/issues/55, i.e. pass >> capabilities to the kernel as a list of uninterpreted string names >> rather than a bitmap. Then the kernel only needs to recognize its own >> capability names and create selinuxfs nodes for all of them reflecting >> their policy values, but no hard-coded references to systemd required. > > Actually, given that Christian is proposing defining entirely new > classes for the new checks in systemd, maybe we don't need a policy > capability at all? systemd can just check whether its new classes are > defined and use that as the indicator of whether the policy support the > new checks? And this can be done via the existing libselinux string_to_security_class() interface, already used by systemd elsewhere. Just test for a zero return value to indicate not-defined. NB On a policy update/reload, the class might become defined without necessarily restarting systemd. So you may need to set a SELINUX_CB_POLICYLOAD callback and recheck whether it has become defined there. > >> >>> >>>> >>>>> >>>>>> diff --git a/security/selinux/include/security.h >>>>>> b/security/selinux/include/security.h >>>>>> index ecdd610e6449..2853e462977f 100644 >>>>>> --- a/security/selinux/include/security.h >>>>>> +++ b/security/selinux/include/security.h >>>>>> @@ -79,6 +79,7 @@ enum { >>>>>> POLICYDB_CAPABILITY_ALWAYSNETWORK, >>>>>> POLICYDB_CAPABILITY_CGROUPSECLABEL, >>>>>> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, >>>>>> + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, >>>>>> __POLICYDB_CAPABILITY_MAX >>>>>> }; >>>>>> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) >>>>>> diff --git a/security/selinux/ss/services.c >>>>>> b/security/selinux/ss/services.c >>>>>> index 55cf42945cba..cb50e187b181 100644 >>>>>> --- a/security/selinux/ss/services.c >>>>>> +++ b/security/selinux/ss/services.c >>>>>> @@ -73,7 +73,8 @@ const char >>>>>> *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { >>>>>> "extended_socket_class", >>>>>> "always_check_network", >>>>>> "cgroup_seclabel", >>>>>> - "nnp_nosuid_transition" >>>>>> + "nnp_nosuid_transition", >>>>>> + "systemd_overhaul" >>>>>> }; >>>>>> >>>>>> static struct selinux_ss selinux_ss; >>>>>> -- >>>>>> 2.24.1 >>>>> >>>> >>> >> >
On Fri, Jan 10, 2020 at 1:12 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 1/10/20 1:03 PM, Stephen Smalley wrote: > > On 1/10/20 12:41 PM, Stephen Smalley wrote: > >> On 1/10/20 11:55 AM, Paul Moore wrote: > >>> On Fri, Jan 10, 2020 at 9:20 AM Christian Göttsche > >>> <cgzones@googlemail.com> wrote: > >>>> Support a SELinux overhaul of systemd by adding a policy capability. > >>>> > >>>> The systemd patch can be found at > >>>> https://github.com/systemd/systemd/pull/10023 > >>>> and has NOT yet been accepted. > >>>> > >>>> This is just a rfc to test the water. > >>>> --- > >>>> security/selinux/include/security.h | 1 + > >>>> security/selinux/ss/services.c | 3 ++- > >>>> 2 files changed, 3 insertions(+), 1 deletion(-) > >>> > >>> Generally the SELinux policy capabilities are reserved for *kernel* > >>> changes that potentially break compatibility with existing SELinux > >>> policies. I'm probably not the best person to talk about > >>> tricks/conventions used to do similar things in userspace, but you've > >>> come to the right place :) > >> > >> It was my suggestion to use policy capabilities for this. There is no > >> separate mechanism for supporting major changes to userspace SELinux > >> permission checks in a backward-compatible manner. Userspace already > >> relies upon /sys/fs/selinux/{deny_unknown,reject_unknown} to get the > >> handle_unknown setting from the kernel policy to decide how to handle > >> unknown userspace classes/permissions. That however is insufficient > >> for these changes to systemd's permission check because they go beyond > >> introducing new classes and permissions and overhaul the existing > >> checks. Policy capability seemed like the best way to do it, and > >> getting it from the kernel is consistent with the fact that we are > >> also getting the userspace classes/perms from the kernel via > >> /sys/fs/selinux/class and the userspace access decisions from the > >> kernel via /sys/fs/selinux/access (through the libselinux AVC, > >> typically). I would argue that the examples you mention above (/sys/fs/selinux/class, etc.) are different enough from the policy capabilities that it isn't a fair comparison. Although given your other responses (below), it's probably not worth our time digging into this argument further. > > As to why we keep the userspace policy as part of the kernel policy and > > not as a separate entity: > > > > - It allows us to provide an effective atomicity in policy changes that > > may span both kernel and userspace components, > > > > - There is significant overlap between the contexts used in the kernel > > and userspace policies, since most userspace policy enforcers are using > > contexts obtained from the kernel for the subject (e.g. > > SO_PEERSEC/getpeercon) or for the object (e.g. getfilecon), > > > > - Policy lookups via /sys/fs/selinux/access are more efficient than > > performing an IPC to a userspace security server. Of course, in both > > cases, we try to maximize use of the libselinux AVC first to avoid > > needing to perform the policy lookup at all. > > > > There were experiments done with introducing support for userspace > > security server(s) for things like XACE/XSELinux and it was found to be > > unsatisfying both performance and security-wise. > > > > There are still cases where we would recommend userspace security > > server(s), such as when the userspace component is implementing a policy > > entirely distinct from that of the kernel (e.g. a remote document server > > implementing RaDAC policies, as in one of our earlier experimental > > research projects), but not for things like systemd. > > All that said, I can see that we probably don't want a hardcoded > reference to systemd in the kernel, since not everyone uses systemd ;) That is definitely one of my concerns; the last thing I want to do is start a Debian-esque argument about init systems :) > Perhaps what we need is for some range of policy capabilities to be > user-defined, with generic names in the kernel and then userspace can > choose to associate meaning with them. It sounds like we may not need this after all, but if we do have to go this route, I would be okay with establishing a range/subset/namespace/etc. of policy capabilities for use by userspace so long as we somehow ensure that there is a clear separation between kernel and userspace policy capabilities. The last thing I want is to add a new policy capability to the kernel only to find out that the capability name already conflicts with someone's policy, that would be very bad. > This would be a bit easier if we implemented a solution to the 2nd part > of https://github.com/SELinuxProject/selinux/issues/55, i.e. pass > capabilities to the kernel as a list of uninterpreted string names > rather than a bitmap. Then the kernel only needs to recognize its own > capability names and create selinuxfs nodes for all of them reflecting > their policy values, but no hard-coded references to systemd required. We would probably want to enforce some sort of prefix string on the userspace policy capabilities during policy load to ensure that we don't have accidental overlap between kernel and userspace capabilities.
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index ecdd610e6449..2853e462977f 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -79,6 +79,7 @@ enum { POLICYDB_CAPABILITY_ALWAYSNETWORK, POLICYDB_CAPABILITY_CGROUPSECLABEL, POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, + POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL, __POLICYDB_CAPABILITY_MAX }; #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 55cf42945cba..cb50e187b181 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { "extended_socket_class", "always_check_network", "cgroup_seclabel", - "nnp_nosuid_transition" + "nnp_nosuid_transition", + "systemd_overhaul" }; static struct selinux_ss selinux_ss;