On Tue, Jul 19, 2022 at 4:28 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Tue, Jul 19, 2022 at 7:58 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > On Tue, Jul 19, 2022 at 12:31 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > Rework the SCTP tests slightly to remove two assumptions which are > > > not always guaranteed to be true (below). This should have not any > > > affect on the current test suite or released kernels, but it will > > > help ensure that the test suite continues to work with upcoming > > > kernel releases. > > > > > > * Do not rely on IP options attached to a socket. Depending on the > > > kernel configuration, the on-the-wire packet labels may be > > > generated on a per-packet basis as opposed to a per-socket basis. > > > > Could you expand a bit on why this would be a problem? It's not clear > > to me how switching to per-packet would break the tests. (Maybe I'm > > just not thinking about it hard enough, but ideally the commit message > > would explain the problem to me so I don't have to :) > > NetLabel can either attach on-the-wire packet labels (IP options) > directly to the packet or to the socket, in the latter case the > network stack handles writing the on-the-wire labels to the packet > when it is generated. Deciding on when to attach IP options > (on-the-wire labels) to the socket versus the packet is an > implementation detail and depends on the specific configuration of the > system and the protocols involved. It is my opinion that going into > the level of detail necessary to explain the differences would involve > a discussion about how the Linux network stacks works, the design of > the NetLabel subsystem, and how the different network protocols work. > The important takeaway is that one can not safely rely on IP options > attached to a socket as a means of determining the labeling behavior > of a socket/connection/association/etc., this is why we have APIs such > as getpeercon() and the LSM specific socket options. Oh wait... I looked closer at what the test is actually doing with the -i options and I get it now. On the client side it extracts the per-socket IP option and expects to receive that from the server as its reported peercon. And on the server side, it also doesn't actually send the peercon, but rather sends whatever is set on its socket's IP options... That is indeed wrong and your patch fixes it correctly. However, I'm pondering whether in the seq server case we shouldn't change the approach a bit... Currently (after your patch) we are basically testing the "unified" socket-level peercon and that forces us to restart the server. But we could also ignore the socket-level peercon in case of SOCK_SEQPACKET and instead extract the per-packet peercon via IP_PASSSEC and SCM_SECURITY control messages, like we do in tests/inet_socket/server.c in the SOCK_DGRAM case. I think this should be the main way for users to get peercon when using SOCK_SEQPACKET SCTP sockets with multiple peers and we should test it with higher priority than the socket-level peercon. This means we would have to use plain recvmsg() instead of sctp_recvmsg() in sctp_server.c, but that shouldn't be a problem as we don't need to extract sctp_sndrcvinfo nor msg_flags there. If you don't want to mess with it, I can take this patch as-is (maybe with improved commit message) and propose the above in a separate patch. -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
