On Mon, Aug 19, 2024 at 8:27 PM Thiébaud Weksteen <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>
Thank you for reviving this patch.
Do you have a corresponding userspace patch? And for extra credit, a
selinux-testsuite patch?
A minor comment below.
> ---
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 7229c9bf6c27..c95bf89c9ce5 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -96,17 +96,17 @@ const struct security_class_mapping secclass_map[] = {
> { "shm", { COMMON_IPC_PERMS, "lock", NULL } },
> { "ipc", { COMMON_IPC_PERMS, NULL } },
> { "netlink_route_socket",
> - { COMMON_SOCK_PERMS, "nlmsg_read", "nlmsg_write", NULL } },
> + { COMMON_SOCK_PERMS, "nlmsg", "nlmsg_read", "nlmsg_write", NULL } },
I would just add the "nlmsg" permission to the end of the list before
the NULL for each class.
Doesn't matter as much anymore since the dynamic class/perm mapping
support was added but generally we avoid perturbing the
class/permission bit assignments unless there is a good reason to do
so. Feel free to wait to see if Paul agrees since your code will work
as is.
