Re: [PATCH] selinux: vsock: Set SID for socket returned by accept()
From: David Brazdil <hidden>
Date: 2021-03-19 12:58:22
Also in:
linux-security-module, lkml, selinux
Hi Paul, I'll post a v2 shortly but will address your comments here.
quoted
include/linux/lsm_hooks.h | 7 +++++++ include/linux/security.h | 5 +++++ net/vmw_vsock/af_vsock.c | 1 + security/security.c | 5 +++++ security/selinux/hooks.c | 10 ++++++++++ 6 files changed, 29 insertions(+)Additional comments below, but I think it would be a good idea for you to test your patches on a more traditional Linux distribution as well as Android.
No problem, I was going to add a test case into selinux-testsuite anyway. Done now (link in v2) and tested on Fedora 33 with v5.12-rc3.
quoted
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 5546710d8ac1..a9bf3b90cb2f 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c@@ -755,6 +755,7 @@ static struct sock *__vsock_create(struct net *net, vsk->buffer_size = psk->buffer_size; vsk->buffer_min_size = psk->buffer_min_size; vsk->buffer_max_size = psk->buffer_max_size; + security_vsock_sk_clone(parent, sk);Did you try calling the existing security_sk_clone() hook here? I would be curious to hear why it doesn't work in this case. Feel free to educate me on AF_VSOCK, it's entirely possible I'm misunderstanding something here :)
No, you're completely right. security_sk_clone does what's needed here. Adding a new hook was me trying to mimic other socket families going via selinux_conn_sid. Happy to reuse the existing hook - makes this a nice oneliner. :) Please note that I'm marking v2 with 'Fixes' for backporting. This does feel to me like a bug, an integration that was never considered. Please shout if you disagree. -David