| Message ID | 20240103163415.304358-1-mic@digikod.net (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v3] selinux: Fix error priority for bind with AF_UNSPEC on PF_INET6 socket | expand |
On 1/3/24 9:34 PM, Mickaël Salaün 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> Thank you Mickaël for the patch. Tested patch on v6.7-rc8. Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > > Changes since v2: > https://lore.kernel.org/r/20231229171922.106190-1-mic@digikod.net > * Add !PF_INET6 check and comments (suggested by Paul). > * s/AF_INET/PF_INET/g (cosmetic change). > > 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 | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index feda711c6b7b..8b1429eb2db5 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -4667,6 +4667,13 @@ 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 == PF_INET6) { > + /* Length check from inet6_bind_sk() */ > + if (addrlen < SIN6_LEN_RFC2133) > + return -EINVAL; > + /* Family check from __inet6_bind() */ > + goto err_af; > + } > /* see __inet_bind(), we only want to allow > * AF_UNSPEC if the address is INADDR_ANY > */
On Thu, Jan 4, 2024 at 6:40 AM Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > On 1/3/24 9:34 PM, Mickaël Salaün 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> > Thank you Mickaël for the patch. Tested patch on v6.7-rc8. > > Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com> This looks good to me, and since it is rather trivial I'm going to go ahead and merge this into selinux/next so it should go up to Linus during the upcoming merge window. Thanks everyone!
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index feda711c6b7b..8b1429eb2db5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4667,6 +4667,13 @@ 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 == PF_INET6) { + /* Length check from inet6_bind_sk() */ + if (addrlen < SIN6_LEN_RFC2133) + return -EINVAL; + /* Family check from __inet6_bind() */ + goto err_af; + } /* 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 v2: https://lore.kernel.org/r/20231229171922.106190-1-mic@digikod.net * Add !PF_INET6 check and comments (suggested by Paul). * s/AF_INET/PF_INET/g (cosmetic change). 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 | 7 +++++++ 1 file changed, 7 insertions(+)