There is currently a very suspicious condition in sock_has_perm() that
basically skips any permission checking in sock_has_perm() if the target
socket is labeled with SECINITSID_KERNEL. This seems bad for several
reasons:
1. If a user-space process somehow gets its hands on such a socket, it
will be able to don any socket operation on it without any SELinux
restriction, even if it is not allowed to do such operations by the
policy.
2. The exception is inconsistent, since one can e.g. write to a stream
socket not only via send(2)/sendto(2)/..., but also via write(2),
which doesn't go through sock_has_perm() and isn't subject to the
SECINITSID_KERNEL exception.
3. The exception also allows operations on sockets that were created
before the SELinux policy was loaded (even by user space), since
these will always inherit the SECINITSID_KERNEL label.
Additionally, it's unclear what is the rationale behind this exception.
For sockets created by the kernel that are expected to be passed to
user space, it seems better to let them undergo normal access checks to
avoid misuse. A possible rationale is to skip checking on operations on
sockets created with kern=1 passed to __sock_create(), which can happen
under user-space credentials even thogh executed internally by the
kernel - notice that such sockets are always labeled with
SECINITSID_KERNEL. However, the operations on these sockets already
normally bypass LSM checks entirely, so arguably this not necessary. On
the contrary, it's better if actual user-space operations on these
sockets go through SELinux checks, since there may be a possibility that
such a socket accidentally leaks into user space and we definitely want
SELinux to detect that and prevent privilege escalation.
Since removing this condition could lead to regressions (notably due to
bullet point (3.) above), add a new policy capability that allows the
policy to opt-out from the condition. This allows policy writers or
distributors to test for impact, add any missing rules, and then enable
the capability.
I tested a kernel with the condition removed on my Fedora workstation
and noted only one new denial, related to a user-space socket created by
systemd-journald before the policy is loaded, which is then continued to
be used by systemd-journald while the system is running.
Also selinux-testsuite is passing without new denials when the check is
removed.
Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
---
security/selinux/hooks.c | 4 +++-
security/selinux/include/policycap.h | 1 +
security/selinux/include/policycap_names.h | 3 ++-
security/selinux/include/security.h | 7 +++++++
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3c5be76a91991..cf7418df3d4c0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4561,7 +4561,9 @@ static int sock_has_perm(struct sock *sk, u32 perms)
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
- if (sksec->sid == SECINITSID_KERNEL)
+ /* legacy behavior was to not perform checks on kernel sockets */
+ if (!selinux_policycap_check_kernel_sockets() &&
+ sksec->sid == SECINITSID_KERNEL)
return 0;
ad.type = LSM_AUDIT_DATA_NET;
diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
index f35d3458e71de..814db520b9d8b 100644
--- a/security/selinux/include/policycap.h
+++ b/security/selinux/include/policycap.h
@@ -12,6 +12,7 @@ enum {
POLICYDB_CAP_NNP_NOSUID_TRANSITION,
POLICYDB_CAP_GENFS_SECLABEL_SYMLINKS,
POLICYDB_CAP_IOCTL_SKIP_CLOEXEC,
+ POLICYDB_CAP_CHECK_KERNEL_SOCKETS,
__POLICYDB_CAP_MAX
};
#define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
index 2a87fc3702b81..62de8262f90fe 100644
--- a/security/selinux/include/policycap_names.h
+++ b/security/selinux/include/policycap_names.h
@@ -13,7 +13,8 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
"cgroup_seclabel",
"nnp_nosuid_transition",
"genfs_seclabel_symlinks",
- "ioctl_skip_cloexec"
+ "ioctl_skip_cloexec",
+ "check_kernel_sockets",
};
#endif /* _SELINUX_POLICYCAP_NAMES_H_ */
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 393aff41d3ef8..1e57c71d067fb 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -230,6 +230,13 @@ static inline bool selinux_policycap_ioctl_skip_cloexec(void)
return READ_ONCE(state->policycap[POLICYDB_CAP_IOCTL_SKIP_CLOEXEC]);
}
+static inline bool selinux_policycap_check_kernel_sockets(void)
+{
+ struct selinux_state *state = &selinux_state;
+
+ return READ_ONCE(state->policycap[POLICYDB_CAP_CHECK_KERNEL_SOCKETS]);
+}
+
struct selinux_policy_convert_data;
struct selinux_load_state {
--
2.39.1
