| Message ID | 20231229171922.106190-1-mic@digikod.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v2] selinux: Fix error priority for bind with AF_UNSPEC on AF_INET6 socket | expand |
On Fri, Dec 29, 2023 at 12:19 PM Mickaël Salaün <mic@digikod.net> wrote: > > The IPv6 network stack first checks the sockaddr length (-EINVAL error) > before checking the family (-EAFNOSUPPORT error). > > This was discovered thanks to commit a549d055a22e ("selftests/landlock: > Add network tests"). > > Cc: Eric Paris <eparis@parisplace.org> > Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <stephen.smalley.work@gmail.com> > Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > Closes: https://lore.kernel.org/r/0584f91c-537c-4188-9e4f-04f192565667@collabora.com > Fixes: 0f8db8cc73df ("selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()") > Signed-off-by: Mickaël Salaün <mic@digikod.net> > --- > > Changes since v1: > https://lore.kernel.org/r/20231228113917.62089-1-mic@digikod.net > * Use the "family" variable (suggested by Paul). > --- > security/selinux/hooks.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index feda711c6b7b..748baa98f623 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -4667,6 +4667,9 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in > return -EINVAL; > addr4 = (struct sockaddr_in *)address; > if (family_sa == AF_UNSPEC) { > + if (family == AF_INET6 && > + addrlen < SIN6_LEN_RFC2133) > + return -EINVAL; If we want to try and match the non-LSM PF_INET6 socket handling as much as possible, after the length check (above) we should fail any non AF_INET6 addresses on an INET6 sock, see __inet6_bind(). My guess is we want something like this: if (family == AF_INET6) { /* length check from inet6_bind_sk() */ if (addrlen < SIN6_LEN_RFC2133) return -EINVAL; /* !AF_INET6 check from __inet6_bind() */ goto err_af; } > /* see __inet_bind(), we only want to allow > * AF_UNSPEC if the address is INADDR_ANY > */ > -- > 2.43.0
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index feda711c6b7b..748baa98f623 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4667,6 +4667,9 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in return -EINVAL; addr4 = (struct sockaddr_in *)address; if (family_sa == AF_UNSPEC) { + if (family == AF_INET6 && + addrlen < SIN6_LEN_RFC2133) + return -EINVAL; /* see __inet_bind(), we only want to allow * AF_UNSPEC if the address is INADDR_ANY */
The IPv6 network stack first checks the sockaddr length (-EINVAL error) before checking the family (-EAFNOSUPPORT error). This was discovered thanks to commit a549d055a22e ("selftests/landlock: Add network tests"). Cc: Eric Paris <eparis@parisplace.org> Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> Cc: Paul Moore <paul@paul-moore.com> Cc: Stephen Smalley <stephen.smalley.work@gmail.com> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com> Closes: https://lore.kernel.org/r/0584f91c-537c-4188-9e4f-04f192565667@collabora.com Fixes: 0f8db8cc73df ("selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()") Signed-off-by: Mickaël Salaün <mic@digikod.net> --- Changes since v1: https://lore.kernel.org/r/20231228113917.62089-1-mic@digikod.net * Use the "family" variable (suggested by Paul). --- security/selinux/hooks.c | 3 +++ 1 file changed, 3 insertions(+)