| Message ID | 20220125145931.56831-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [RFC] selinux: split no transition execve check | expand |
On Tue, Jan 25, 2022 at 9:59 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > In case a setuid or setgid binary is mislabeled with a generic context, > either via a policy mistake or a move by the distribution package, > executing it will be checked by the file permission execute_no_trans on > the generic file context (e.g. bin_t). The setuid(2)/setgid(2) syscall > within will then be checked against the unchanged caller process > context, which might have been granted the capability permission setuid/ > setgid to initially drop privileges. To avoid that scenario split the > execute_no_trans permission in case of a setuid/setgid binary into a new > permission execute_sxid_no_trans. > > For backward compatibility this behavior is contained in a new policy > capability. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > security/selinux/hooks.c | 9 ++++++++- > security/selinux/include/classmap.h | 2 +- > security/selinux/include/policycap.h | 1 + > security/selinux/include/policycap_names.h | 3 ++- > security/selinux/include/security.h | 8 ++++++++ > 5 files changed, 20 insertions(+), 3 deletions(-) Adding the refpolicy list to this thread as their opinion seems particularly relevant to this discussion. FWIW, this looks reasonable to me but I would like to hear what others have to say. > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 5b6895e4fc29..b825fee39a70 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2348,9 +2348,16 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) > ad.u.file = bprm->file; > > if (new_tsec->sid == old_tsec->sid) { > + u32 perm; > + > + if (selinux_policycap_execute_sxid_no_trans() && is_sxid(inode->i_mode)) > + perm = FILE__EXECUTE_SXID_NO_TRANS; > + else > + perm = FILE__EXECUTE_NO_TRANS; > + > rc = avc_has_perm(&selinux_state, > old_tsec->sid, isec->sid, > - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad); > + SECCLASS_FILE, perm, &ad); > if (rc) > return rc; > } else { > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h > index 35aac62a662e..53a1eeeb86fb 100644 > --- a/security/selinux/include/classmap.h > +++ b/security/selinux/include/classmap.h > @@ -65,7 +65,7 @@ struct security_class_mapping secclass_map[] = { > "quotaget", "watch", NULL } }, > { "file", > { COMMON_FILE_PERMS, > - "execute_no_trans", "entrypoint", NULL } }, > + "execute_no_trans", "entrypoint", "execute_sxid_no_trans", NULL } }, > { "dir", > { COMMON_FILE_PERMS, "add_name", "remove_name", > "reparent", "search", "rmdir", NULL } }, > diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h > index 2ec038efbb03..23929dc3e1db 100644 > --- a/security/selinux/include/policycap.h > +++ b/security/selinux/include/policycap.h > @@ -11,6 +11,7 @@ enum { > POLICYDB_CAPABILITY_CGROUPSECLABEL, > POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, > POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, > + POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS, > __POLICYDB_CAPABILITY_MAX > }; > #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h > index b89289f092c9..4c014c2cf352 100644 > --- a/security/selinux/include/policycap_names.h > +++ b/security/selinux/include/policycap_names.h > @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { > "always_check_network", > "cgroup_seclabel", > "nnp_nosuid_transition", > - "genfs_seclabel_symlinks" > + "genfs_seclabel_symlinks", > + "execute_sxid_no_trans", > }; > > #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index ac0ece01305a..ab95241b6b7b 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -219,6 +219,14 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) > return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); > } > > +static inline bool selinux_policycap_execute_sxid_no_trans(void) > +{ > + struct selinux_state *state = &selinux_state; > + > + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS]); > +} > + > + > struct selinux_policy_convert_data; > > struct selinux_load_state { > -- > 2.34.1 >
On 1/26/22 17:51, Paul Moore wrote: > On Tue, Jan 25, 2022 at 9:59 AM Christian Göttsche > <cgzones@googlemail.com> wrote: >> >> In case a setuid or setgid binary is mislabeled with a generic context, >> either via a policy mistake or a move by the distribution package, >> executing it will be checked by the file permission execute_no_trans on >> the generic file context (e.g. bin_t). The setuid(2)/setgid(2) syscall >> within will then be checked against the unchanged caller process >> context, which might have been granted the capability permission setuid/ >> setgid to initially drop privileges. To avoid that scenario split the >> execute_no_trans permission in case of a setuid/setgid binary into a new >> permission execute_sxid_no_trans. >> >> For backward compatibility this behavior is contained in a new policy >> capability. >> >> Signed-off-by: Christian Göttsche <cgzones@googlemail.com> >> --- >> security/selinux/hooks.c | 9 ++++++++- >> security/selinux/include/classmap.h | 2 +- >> security/selinux/include/policycap.h | 1 + >> security/selinux/include/policycap_names.h | 3 ++- >> security/selinux/include/security.h | 8 ++++++++ >> 5 files changed, 20 insertions(+), 3 deletions(-) > > Adding the refpolicy list to this thread as their opinion seems > particularly relevant to this discussion. > > FWIW, this looks reasonable to me but I would like to hear what others > have to say. I think this a band-aid to cover up the real problem, which is the mislabeled files. >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 5b6895e4fc29..b825fee39a70 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -2348,9 +2348,16 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) >> ad.u.file = bprm->file; >> >> if (new_tsec->sid == old_tsec->sid) { >> + u32 perm; >> + >> + if (selinux_policycap_execute_sxid_no_trans() && is_sxid(inode->i_mode)) >> + perm = FILE__EXECUTE_SXID_NO_TRANS; >> + else >> + perm = FILE__EXECUTE_NO_TRANS; >> + >> rc = avc_has_perm(&selinux_state, >> old_tsec->sid, isec->sid, >> - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad); >> + SECCLASS_FILE, perm, &ad); >> if (rc) >> return rc; >> } else { >> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h >> index 35aac62a662e..53a1eeeb86fb 100644 >> --- a/security/selinux/include/classmap.h >> +++ b/security/selinux/include/classmap.h >> @@ -65,7 +65,7 @@ struct security_class_mapping secclass_map[] = { >> "quotaget", "watch", NULL } }, >> { "file", >> { COMMON_FILE_PERMS, >> - "execute_no_trans", "entrypoint", NULL } }, >> + "execute_no_trans", "entrypoint", "execute_sxid_no_trans", NULL } }, >> { "dir", >> { COMMON_FILE_PERMS, "add_name", "remove_name", >> "reparent", "search", "rmdir", NULL } }, >> diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h >> index 2ec038efbb03..23929dc3e1db 100644 >> --- a/security/selinux/include/policycap.h >> +++ b/security/selinux/include/policycap.h >> @@ -11,6 +11,7 @@ enum { >> POLICYDB_CAPABILITY_CGROUPSECLABEL, >> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, >> POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, >> + POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS, >> __POLICYDB_CAPABILITY_MAX >> }; >> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) >> diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h >> index b89289f092c9..4c014c2cf352 100644 >> --- a/security/selinux/include/policycap_names.h >> +++ b/security/selinux/include/policycap_names.h >> @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { >> "always_check_network", >> "cgroup_seclabel", >> "nnp_nosuid_transition", >> - "genfs_seclabel_symlinks" >> + "genfs_seclabel_symlinks", >> + "execute_sxid_no_trans", >> }; >> >> #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ >> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h >> index ac0ece01305a..ab95241b6b7b 100644 >> --- a/security/selinux/include/security.h >> +++ b/security/selinux/include/security.h >> @@ -219,6 +219,14 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) >> return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); >> } >> >> +static inline bool selinux_policycap_execute_sxid_no_trans(void) >> +{ >> + struct selinux_state *state = &selinux_state; >> + >> + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS]); >> +} >> + >> + >> struct selinux_policy_convert_data; >> >> struct selinux_load_state { >> -- >> 2.34.1 >> > >
On Thu, Jan 27, 2022 at 8:42 AM Chris PeBenito <pebenito@ieee.org> wrote: > On 1/26/22 17:51, Paul Moore wrote: > > On Tue, Jan 25, 2022 at 9:59 AM Christian Göttsche > > <cgzones@googlemail.com> wrote: > >> > >> In case a setuid or setgid binary is mislabeled with a generic context, > >> either via a policy mistake or a move by the distribution package, > >> executing it will be checked by the file permission execute_no_trans on > >> the generic file context (e.g. bin_t). The setuid(2)/setgid(2) syscall > >> within will then be checked against the unchanged caller process > >> context, which might have been granted the capability permission setuid/ > >> setgid to initially drop privileges. To avoid that scenario split the > >> execute_no_trans permission in case of a setuid/setgid binary into a new > >> permission execute_sxid_no_trans. > >> > >> For backward compatibility this behavior is contained in a new policy > >> capability. > >> > >> Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > >> --- > >> security/selinux/hooks.c | 9 ++++++++- > >> security/selinux/include/classmap.h | 2 +- > >> security/selinux/include/policycap.h | 1 + > >> security/selinux/include/policycap_names.h | 3 ++- > >> security/selinux/include/security.h | 8 ++++++++ > >> 5 files changed, 20 insertions(+), 3 deletions(-) > > > > Adding the refpolicy list to this thread as their opinion seems > > particularly relevant to this discussion. > > > > FWIW, this looks reasonable to me but I would like to hear what others > > have to say. > > I think this a band-aid to cover up the real problem, which is the mislabeled files. It's hard to disagree with that, and the kernel is probably the wrong place to apply a band-aid unless it is the only option left. > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > >> index 5b6895e4fc29..b825fee39a70 100644 > >> --- a/security/selinux/hooks.c > >> +++ b/security/selinux/hooks.c > >> @@ -2348,9 +2348,16 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) > >> ad.u.file = bprm->file; > >> > >> if (new_tsec->sid == old_tsec->sid) { > >> + u32 perm; > >> + > >> + if (selinux_policycap_execute_sxid_no_trans() && is_sxid(inode->i_mode)) > >> + perm = FILE__EXECUTE_SXID_NO_TRANS; > >> + else > >> + perm = FILE__EXECUTE_NO_TRANS; > >> + > >> rc = avc_has_perm(&selinux_state, > >> old_tsec->sid, isec->sid, > >> - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad); > >> + SECCLASS_FILE, perm, &ad); > >> if (rc) > >> return rc; > >> } else { > >> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h > >> index 35aac62a662e..53a1eeeb86fb 100644 > >> --- a/security/selinux/include/classmap.h > >> +++ b/security/selinux/include/classmap.h > >> @@ -65,7 +65,7 @@ struct security_class_mapping secclass_map[] = { > >> "quotaget", "watch", NULL } }, > >> { "file", > >> { COMMON_FILE_PERMS, > >> - "execute_no_trans", "entrypoint", NULL } }, > >> + "execute_no_trans", "entrypoint", "execute_sxid_no_trans", NULL } }, > >> { "dir", > >> { COMMON_FILE_PERMS, "add_name", "remove_name", > >> "reparent", "search", "rmdir", NULL } }, > >> diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h > >> index 2ec038efbb03..23929dc3e1db 100644 > >> --- a/security/selinux/include/policycap.h > >> +++ b/security/selinux/include/policycap.h > >> @@ -11,6 +11,7 @@ enum { > >> POLICYDB_CAPABILITY_CGROUPSECLABEL, > >> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, > >> POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, > >> + POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS, > >> __POLICYDB_CAPABILITY_MAX > >> }; > >> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > >> diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h > >> index b89289f092c9..4c014c2cf352 100644 > >> --- a/security/selinux/include/policycap_names.h > >> +++ b/security/selinux/include/policycap_names.h > >> @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { > >> "always_check_network", > >> "cgroup_seclabel", > >> "nnp_nosuid_transition", > >> - "genfs_seclabel_symlinks" > >> + "genfs_seclabel_symlinks", > >> + "execute_sxid_no_trans", > >> }; > >> > >> #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ > >> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > >> index ac0ece01305a..ab95241b6b7b 100644 > >> --- a/security/selinux/include/security.h > >> +++ b/security/selinux/include/security.h > >> @@ -219,6 +219,14 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) > >> return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); > >> } > >> > >> +static inline bool selinux_policycap_execute_sxid_no_trans(void) > >> +{ > >> + struct selinux_state *state = &selinux_state; > >> + > >> + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS]); > >> +} > >> + > >> + > >> struct selinux_policy_convert_data; > >> > >> struct selinux_load_state { > >> -- > >> 2.34.1 > >> > > > > > > > -- > Chris PeBenito
On Fri, 28 Jan 2022 at 02:47, Paul Moore <paul@paul-moore.com> wrote: > > On Thu, Jan 27, 2022 at 8:42 AM Chris PeBenito <pebenito@ieee.org> wrote: > > On 1/26/22 17:51, Paul Moore wrote: > > > On Tue, Jan 25, 2022 at 9:59 AM Christian Göttsche > > > <cgzones@googlemail.com> wrote: > > >> > > >> In case a setuid or setgid binary is mislabeled with a generic context, > > >> either via a policy mistake or a move by the distribution package, > > >> executing it will be checked by the file permission execute_no_trans on > > >> the generic file context (e.g. bin_t). The setuid(2)/setgid(2) syscall > > >> within will then be checked against the unchanged caller process > > >> context, which might have been granted the capability permission setuid/ > > >> setgid to initially drop privileges. To avoid that scenario split the > > >> execute_no_trans permission in case of a setuid/setgid binary into a new > > >> permission execute_sxid_no_trans. > > >> > > >> For backward compatibility this behavior is contained in a new policy > > >> capability. > > >> > > >> Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > >> --- > > >> security/selinux/hooks.c | 9 ++++++++- > > >> security/selinux/include/classmap.h | 2 +- > > >> security/selinux/include/policycap.h | 1 + > > >> security/selinux/include/policycap_names.h | 3 ++- > > >> security/selinux/include/security.h | 8 ++++++++ > > >> 5 files changed, 20 insertions(+), 3 deletions(-) > > > > > > Adding the refpolicy list to this thread as their opinion seems > > > particularly relevant to this discussion. > > > > > > FWIW, this looks reasonable to me but I would like to hear what others > > > have to say. > > > > I think this a band-aid to cover up the real problem, which is the mislabeled files. > > It's hard to disagree with that, and the kernel is probably the wrong > place to apply a band-aid unless it is the only option left. > Adding a new datapoint to this RFC: An unprivileged user can via the setuid binary newgrp(1) write arbitrary content to files he can open in write mode but not actually write to, e.g. /proc/sys/kernel/ns_last_pid [1]. This also is reproducible on Fedora where /usr/bin/newgrp has the generic context bin_t, so the write (and capable check) happens in the caller context of unconfined_t. With the proposed permission split applied and instead of granting unconfined_t the new permission execute_sxid_no_trans but relying on an intermediate template domain $1_newgrp_t the access would have been denied, due to lack of permissions of the templated newgrp domain. With a generic file type of bin_t newgrp would not be executable for callers otherwise. Most setuid binaries are already labeled with a private type, newgrp and pkexec are the only ones left on a head-less Fedora system. One could still argue this is a policy neglect, but normally policy neglects result in missing permissions and reduced functionality, not in unintended privilege escalation. [1]: https://github.com/shadow-maint/shadow/pull/758 > > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > >> index 5b6895e4fc29..b825fee39a70 100644 > > >> --- a/security/selinux/hooks.c > > >> +++ b/security/selinux/hooks.c > > >> @@ -2348,9 +2348,16 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) > > >> ad.u.file = bprm->file; > > >> > > >> if (new_tsec->sid == old_tsec->sid) { > > >> + u32 perm; > > >> + > > >> + if (selinux_policycap_execute_sxid_no_trans() && is_sxid(inode->i_mode)) > > >> + perm = FILE__EXECUTE_SXID_NO_TRANS; > > >> + else > > >> + perm = FILE__EXECUTE_NO_TRANS; > > >> + > > >> rc = avc_has_perm(&selinux_state, > > >> old_tsec->sid, isec->sid, > > >> - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad); > > >> + SECCLASS_FILE, perm, &ad); > > >> if (rc) > > >> return rc; > > >> } else { > > >> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h > > >> index 35aac62a662e..53a1eeeb86fb 100644 > > >> --- a/security/selinux/include/classmap.h > > >> +++ b/security/selinux/include/classmap.h > > >> @@ -65,7 +65,7 @@ struct security_class_mapping secclass_map[] = { > > >> "quotaget", "watch", NULL } }, > > >> { "file", > > >> { COMMON_FILE_PERMS, > > >> - "execute_no_trans", "entrypoint", NULL } }, > > >> + "execute_no_trans", "entrypoint", "execute_sxid_no_trans", NULL } }, > > >> { "dir", > > >> { COMMON_FILE_PERMS, "add_name", "remove_name", > > >> "reparent", "search", "rmdir", NULL } }, > > >> diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h > > >> index 2ec038efbb03..23929dc3e1db 100644 > > >> --- a/security/selinux/include/policycap.h > > >> +++ b/security/selinux/include/policycap.h > > >> @@ -11,6 +11,7 @@ enum { > > >> POLICYDB_CAPABILITY_CGROUPSECLABEL, > > >> POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, > > >> POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, > > >> + POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS, > > >> __POLICYDB_CAPABILITY_MAX > > >> }; > > >> #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > > >> diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h > > >> index b89289f092c9..4c014c2cf352 100644 > > >> --- a/security/selinux/include/policycap_names.h > > >> +++ b/security/selinux/include/policycap_names.h > > >> @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { > > >> "always_check_network", > > >> "cgroup_seclabel", > > >> "nnp_nosuid_transition", > > >> - "genfs_seclabel_symlinks" > > >> + "genfs_seclabel_symlinks", > > >> + "execute_sxid_no_trans", > > >> }; > > >> > > >> #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ > > >> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > > >> index ac0ece01305a..ab95241b6b7b 100644 > > >> --- a/security/selinux/include/security.h > > >> +++ b/security/selinux/include/security.h > > >> @@ -219,6 +219,14 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) > > >> return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); > > >> } > > >> > > >> +static inline bool selinux_policycap_execute_sxid_no_trans(void) > > >> +{ > > >> + struct selinux_state *state = &selinux_state; > > >> + > > >> + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS]); > > >> +} > > >> + > > >> + > > >> struct selinux_policy_convert_data; > > >> > > >> struct selinux_load_state { > > >> -- > > >> 2.34.1 > > >> > > > > > > > > > > > > -- > > Chris PeBenito > > > > -- > paul-moore.com
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5b6895e4fc29..b825fee39a70 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2348,9 +2348,16 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) ad.u.file = bprm->file; if (new_tsec->sid == old_tsec->sid) { + u32 perm; + + if (selinux_policycap_execute_sxid_no_trans() && is_sxid(inode->i_mode)) + perm = FILE__EXECUTE_SXID_NO_TRANS; + else + perm = FILE__EXECUTE_NO_TRANS; + rc = avc_has_perm(&selinux_state, old_tsec->sid, isec->sid, - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad); + SECCLASS_FILE, perm, &ad); if (rc) return rc; } else { diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 35aac62a662e..53a1eeeb86fb 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -65,7 +65,7 @@ struct security_class_mapping secclass_map[] = { "quotaget", "watch", NULL } }, { "file", { COMMON_FILE_PERMS, - "execute_no_trans", "entrypoint", NULL } }, + "execute_no_trans", "entrypoint", "execute_sxid_no_trans", NULL } }, { "dir", { COMMON_FILE_PERMS, "add_name", "remove_name", "reparent", "search", "rmdir", NULL } }, diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h index 2ec038efbb03..23929dc3e1db 100644 --- a/security/selinux/include/policycap.h +++ b/security/selinux/include/policycap.h @@ -11,6 +11,7 @@ enum { POLICYDB_CAPABILITY_CGROUPSECLABEL, POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, + POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS, __POLICYDB_CAPABILITY_MAX }; #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h index b89289f092c9..4c014c2cf352 100644 --- a/security/selinux/include/policycap_names.h +++ b/security/selinux/include/policycap_names.h @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { "always_check_network", "cgroup_seclabel", "nnp_nosuid_transition", - "genfs_seclabel_symlinks" + "genfs_seclabel_symlinks", + "execute_sxid_no_trans", }; #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index ac0ece01305a..ab95241b6b7b 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -219,6 +219,14 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); } +static inline bool selinux_policycap_execute_sxid_no_trans(void) +{ + struct selinux_state *state = &selinux_state; + + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXECUTE_SXID_NO_TRANS]); +} + + struct selinux_policy_convert_data; struct selinux_load_state {
In case a setuid or setgid binary is mislabeled with a generic context, either via a policy mistake or a move by the distribution package, executing it will be checked by the file permission execute_no_trans on the generic file context (e.g. bin_t). The setuid(2)/setgid(2) syscall within will then be checked against the unchanged caller process context, which might have been granted the capability permission setuid/ setgid to initially drop privileges. To avoid that scenario split the execute_no_trans permission in case of a setuid/setgid binary into a new permission execute_sxid_no_trans. For backward compatibility this behavior is contained in a new policy capability. Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- security/selinux/hooks.c | 9 ++++++++- security/selinux/include/classmap.h | 2 +- security/selinux/include/policycap.h | 1 + security/selinux/include/policycap_names.h | 3 ++- security/selinux/include/security.h | 8 ++++++++ 5 files changed, 20 insertions(+), 3 deletions(-)