Re: [PATCH] sctp: initialize endpoint LSM labels also on the client side
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: 2021-10-21 15:52:34
Also in:
linux-sctp, linux-security-module, lkml, selinux
On Thu, Oct 21, 2021 at 5:38 PM Ondrej Mosnacek [off-list ref] wrote:
The secid* fields in struct sctp_endpoint are used to initialize the
labels of a peeloff socket created from the given association. Currently
they are initialized properly when a new association is created on the
server side (upon receiving an INIT packet), but not on the client side.
As a result, when the client obtains a peeloff socket via
sctp_peeloff(3) under SELinux, it ends up unlabeled, leading to
unexpected denials.
Fix this by calling the security_sctp_assoc_request() hook also upon
receiving a valid INIT-ACK response from the server, so that the
endpoint labels are properly initialized also on the client side.
Fixes: 2277c7cd75e3 ("sctp: Add LSM hooks")
Cc: Richard Haines <redacted>
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
include/net/sctp/structs.h | 11 ++++++-----
net/sctp/sm_statefuns.c | 5 +++++
2 files changed, 11 insertions(+), 5 deletions(-)See also the selinux-testsuite [1] patch [2] that verifies this fix (i.e. the new tests fail without this patch and pass with it). Not being very familiar with SCTP, I'm not 100% sure if this fix is correct or complete, so reviews are very much welcome. [1] https://github.com/SELinuxProject/selinux-testsuite/ [2] https://patchwork.kernel.org/project/selinux/patch/20211021144543.740762-1-omosnace@redhat.com/
quoted hunk ↗ jump to hunk
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index 651bba654d77..033a955592dd 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h@@ -1356,11 +1356,12 @@ struct sctp_endpoint { __u8 strreset_enable; - /* Security identifiers from incoming (INIT). These are set by - * security_sctp_assoc_request(). These will only be used by - * SCTP TCP type sockets and peeled off connections as they - * cause a new socket to be generated. security_sctp_sk_clone() - * will then plug these into the new socket. + /* Security identifiers from incoming (INIT/INIT-ACK). These + * are set by security_sctp_assoc_request(). These will only + * be used by SCTP TCP type sockets and peeled off connections + * as they cause a new socket to be generated. + * security_sctp_sk_clone() will then plug these into the new + * socket. */ u32 secid;diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 32df65f68c12..cb291c7f5fb7 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c@@ -521,6 +521,11 @@ enum sctp_disposition sctp_sf_do_5_1C_ack(struct net *net, if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + /* Update socket peer label if first association. */ + if (security_sctp_assoc_request((struct sctp_endpoint *)ep, + chunk->skb)) + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + /* 6.10 Bundling * An endpoint MUST NOT bundle INIT, INIT ACK or * SHUTDOWN COMPLETE with any other chunks. --2.31.1
-- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.