On Sep 9, 2024 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" <tweek@xxxxxxxxxx> wrote:
>
> Reuse the existing extended permissions infrastructure to support
> policies based on the netlink message types.
>
> A new policy capability "netlink_xperm" is introduced. When disabled,
> the previous behaviour is preserved. That is, netlink_send will rely on
> the permission mappings defined in nlmsgtab.c (e.g, nlmsg_read for
> RTM_GETADDR on NETLINK_ROUTE). When enabled, the mappings are ignored
> and the generic "nlmsg" permission is used instead.
>
> The new "nlmsg" permission is an extended permission. The 16 bits of the
> extended permission are mapped to the nlmsg_type field.
>
> Example policy on Android, preventing regular apps from accessing the
> device's MAC address and ARP table, but allowing this access to
> privileged apps, looks as follows:
>
> allow netdomain self:netlink_route_socket {
> create read getattr write setattr lock append connect getopt
> setopt shutdown nlmsg
> };
> allowxperm netdomain self:netlink_route_socket nlmsg ~{
> RTM_GETLINK RTM_GETNEIGH RTM_GETNEIGHTBL
> };
> allowxperm priv_app self:netlink_route_socket nlmsg {
> RTM_GETLINK RTM_GETNEIGH RTM_GETNEIGHTBL
> };
>
> The constants in the example above (e.g., RTM_GETLINK) are explicitly
> defined in the policy.
>
> It is possible to generate policies to support kernels that may or
> may not have the capability enabled by generating a rule for each
> scenario. For instance:
>
> allow domain self:netlink_audit_socket nlmsg_read;
> allow domain self:netlink_audit_socket nlmsg;
> allowxperm domain self:netlink_audit_socket nlmsg { AUDIT_GET };
>
> The approach of defining a new permission ("nlmsg") instead of relying
> on the existing permissions (e.g., "nlmsg_read", "nlmsg_readpriv" or
> "nlmsg_tty_audit") has been preferred because:
> 1. This is similar to the other extended permission ("ioctl");
> 2. With the new extended permission, the coarse-grained mapping is not
> necessary anymore. It could eventually be removed, which would be
> impossible if the extended permission was defined below these.
> 3. Having a single extra extended permission considerably simplifies
> the implementation here and in libselinux.
>
> The class NETLINK_GENERIC is excluded from using this extended
> permission because the nlmsg_type field is referencing the family id
> which is dynamically associated.
>
> Signed-off-by: Thiébaud Weksteen <tweek@xxxxxxxxxx>
> Signed-off-by: Bram Bonné <brambonne@xxxxxxxxxx>
> Acked-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
> Tested-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
> ---
> v2: Reorder classmap.h to keep the new permission "nlmsg" at the end.
>
> security/selinux/hooks.c | 56 +++++++++++++---
> security/selinux/include/classmap.h | 8 +--
> security/selinux/include/policycap.h | 1 +
> security/selinux/include/policycap_names.h | 1 +
> security/selinux/include/security.h | 6 ++
> security/selinux/nlmsgtab.c | 21 ++++++
> security/selinux/ss/avtab.h | 5 +-
> security/selinux/ss/services.c | 78 ++++++++++++----------
> 8 files changed, 125 insertions(+), 51 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 400eca4ad0fb..d1ef898a8481 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5964,7 +5992,17 @@ static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
>
> rc = selinux_nlmsg_lookup(sclass, nlh->nlmsg_type, &perm);
> if (rc == 0) {
> - rc = sock_has_perm(sk, perm);
> + /* For Generic Netlink, nlmsg_type is mapped to the
> + * family id which is dynamically assigned.
> + * Ignore extended permissions for this type.
> + */
> + if (selinux_policycap_netlink_xperm() &&
> + (sclass != SECCLASS_NETLINK_GENERIC_SOCKET)) {
> + rc = nlmsg_sock_has_extended_perms(
> + sk, perm, nlh->nlmsg_type);
> + } else {
> + rc = sock_has_perm(sk, perm);
> + }
I agree with your approach of ignoring xperms on generic netlink sockets,
it seems like the only sane thing we can do, but aren't we always going
to fail a SECCLASS_NETLINK_GENERIC_SOCKET check here? It looks like
selinux_nlmsg_lookup() is going to return -ENOENT in the case of
SECCLASS_NETLINK_GENERIC_SOCKET which means we never hit this chunk of
code if we are checking a genetlink socket. If selinux_nlmsg_lookup()
returns zero, I believe we only need to check if the policy capability
is enabled before doing the xperm processing.
... or am I missing something?
Of course if selinux_nlmsg_lookup() were to gain generic netlink support
then the check would be necessary, but I don't see how we could ever
properly support generic netlink using our current mechanisms so I doubt
this is something we really need to worry about.
> if (rc)
> return rc;
> } else if (rc == -EINVAL) {
--
paul-moore.com
