| Message ID | 20250108231554.3634987-1-tweek@google.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: map RTM_DELNSID to nlmsg_write | expand |
Le 09/01/2025 à 00:15, Thiébaud Weksteen a écrit : > The mapping for RTM_DELNSID was added in commit 387f989a60db > ("selinux/nlmsg: add RTM_GETNSID"). While this message type is not > expected from userspace, other RTM_DEL* types are mapped to the more > restrictive nlmsg_write permission. Move RTM_DELNSID to nlmsg_write in > case the implementation is changed in the future. Frankly, I don't think this will ever change. It's not a problem of implementing the delete command, it's conceptually no sense. I don't see why the DEL should be restricted in any way. Regards, Nicolas > > Fixes: 387f989a60db ("selinux/nlmsg: add RTM_GETNSID") > Signed-off-by: Thiébaud Weksteen <tweek@google.com> > --- > security/selinux/nlmsgtab.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c > index 3a95986b134f..f97e75301018 100644 > --- a/security/selinux/nlmsgtab.c > +++ b/security/selinux/nlmsgtab.c > @@ -71,7 +71,7 @@ static const struct nlmsg_perm nlmsg_route_perms[] = { > { RTM_DELMDB, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, > { RTM_GETMDB, NETLINK_ROUTE_SOCKET__NLMSG_READ }, > { RTM_NEWNSID, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, > - { RTM_DELNSID, NETLINK_ROUTE_SOCKET__NLMSG_READ }, > + { RTM_DELNSID, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, > { RTM_GETNSID, NETLINK_ROUTE_SOCKET__NLMSG_READ }, > { RTM_NEWSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ }, > { RTM_GETSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ },
On Thu, Jan 9, 2025 at 4:24 AM Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote: > Le 09/01/2025 à 00:15, Thiébaud Weksteen a écrit : > > > > The mapping for RTM_DELNSID was added in commit 387f989a60db > > ("selinux/nlmsg: add RTM_GETNSID"). While this message type is not > > expected from userspace, other RTM_DEL* types are mapped to the more > > restrictive nlmsg_write permission. Move RTM_DELNSID to nlmsg_write in > > case the implementation is changed in the future. > > Frankly, I don't think this will ever change. It's not a problem of implementing > the delete command, it's conceptually no sense. > > I don't see why the DEL should be restricted in any way. While the RTM_DELNSID messages are not generated from userspace, the presence of the SELinux access control point is visible to userspace and thus we have to worry about the backwards compatibility impact of changing a "read" operation to a "write" operation. We could likely have a discussion about which is a better permission mapping for RTM_DELNSID, read or write, but ultimately I think this should probably be treated as a read operation since the kernel is using this simply as a notification message. Sending, or receiving, a RTM_DELNSID message doesn't affect the state of the netns ID, or the netns itself; in other words, a RTM_DELNSID is not the cause of netns state change, it is a notification artifact of such a change. Leaving this mapped as a "read" operation seems correct to me. It is also worth noting that the SELinux netlink xperms support that will ship for the first time in v6.13 will allow policy developers to target RTM_DELNSID messages with much greater permissions granularity, largely solving this problem for those who care about it. Finally, looking at unhash_nsid(), the only place which seems to generate RTM_DELNSID notification messages, an access control denial on the netlink notification operation will have no impact on the removal of the netns or the netns ID, only the notification itself should be impacted.
diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c index 3a95986b134f..f97e75301018 100644 --- a/security/selinux/nlmsgtab.c +++ b/security/selinux/nlmsgtab.c @@ -71,7 +71,7 @@ static const struct nlmsg_perm nlmsg_route_perms[] = { { RTM_DELMDB, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, { RTM_GETMDB, NETLINK_ROUTE_SOCKET__NLMSG_READ }, { RTM_NEWNSID, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, - { RTM_DELNSID, NETLINK_ROUTE_SOCKET__NLMSG_READ }, + { RTM_DELNSID, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, { RTM_GETNSID, NETLINK_ROUTE_SOCKET__NLMSG_READ }, { RTM_NEWSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ }, { RTM_GETSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ },
The mapping for RTM_DELNSID was added in commit 387f989a60db ("selinux/nlmsg: add RTM_GETNSID"). While this message type is not expected from userspace, other RTM_DEL* types are mapped to the more restrictive nlmsg_write permission. Move RTM_DELNSID to nlmsg_write in case the implementation is changed in the future. Fixes: 387f989a60db ("selinux/nlmsg: add RTM_GETNSID") Signed-off-by: Thiébaud Weksteen <tweek@google.com> --- security/selinux/nlmsgtab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)