On Aug 26, 2024 Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > sctp_sf_do_5_2_4_dupcook() currently calls security_sctp_assoc_request() > on new_asoc, but as it turns out, this association is always discarded > and the LSM labels never get into the final association (asoc). > > This can be reproduced by having two SCTP endpoints try to initiate an > association with each other at approximately the same time and then peel > off the association into a new socket, which exposes the unitialized > labels and triggers SELinux denials. > > Fix it by calling security_sctp_assoc_request() on asoc instead of > new_asoc. Xin Long also suggested limit calling the hook only to cases > A, B, and D, since in cases C and E the COOKIE ECHO chunk is discarded > and the association doesn't enter the ESTABLISHED state, so rectify that > as well. > > One related caveat with SELinux and peer labeling: When an SCTP > connection is set up simultaneously in this way, we will end up with an > association that is initialized with security_sctp_assoc_request() on > both sides, so the MLS component of the security context of the > association will get swapped between the peers, instead of just one side > setting it to the other's MLS component. However, at that point > security_sctp_assoc_request() had already been called on both sides in > sctp_sf_do_unexpected_init() (on a temporary association) and thus if > the exchange didn't fail before due to MLS, it won't fail now either > (most likely both endpoints have the same MLS range). > > Tested by: > - reproducer from https://src.fedoraproject.org/tests/selinux/pull-request/530 > - selinux-testsuite (https://github.com/SELinuxProject/selinux-testsuite/) > - sctp-tests (https://github.com/sctp/sctp-tests) - no tests failed > that wouldn't fail also without the patch applied > > Fixes: c081d53f97a1 ("security: pass asoc to sctp_assoc_request and sctp_sk_clone") > Suggested-by: Xin Long <lucien.xin@xxxxxxxxx> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > Acked-by: Xin Long <lucien.xin@xxxxxxxxx> > --- > net/sctp/sm_statefuns.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx> (LSM/SELinux) -- paul-moore.com
