On Tue, Jun 7, 2022 at 8:28 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > On Fri, Jun 3, 2022 at 12:45 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Thu, Jun 2, 2022 at 10:49 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > > > > > On Wed, Jun 1, 2022 at 3:32 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > > On Tue, May 31, 2022 at 7:53 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > > > On Tue, May 31, 2022 at 1:05 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: ... > > > > From a quick skim of the SCTP RFCs, the ASCONF chunk is sent from a > > > > remote endpoint to update the SCTP parameters so in this case I > > > > believe the subject should be the remote peer (the association/sock's > > > > peer_secid) and the object should be the local association/socket. > > > > It's important to note that any access control checks using the remote > > > > peer label should be gated by the selinux_peerlbl_enabled() function, > > > > see selinux_socket_sock_rcv_skb() for an example. > > > > > > I don't like the idea of using peer_secid as the subject for > > > socket::connect, which normally has a task sid as the subject. > > > > While I have concerns about reusing the socket:connect permission for > > the ASCONF updates, it isn't because of the peer_secid. The > > peer_secid represents the security label of the remote peer and it is > > the subject label for ASCONF operations. > > It does make sense to use it as a subject label, but not for the > connect permission, where the convention is that the subject context > is a process context. While we don't have any hard rule against mixing > different "kinds" of contexts in the subject/target of a given > permission, it makes both figuring out AVC denials and reasoning about > policy harder and I want to avoid it wherever possible. The network peer label is a process label, it is intended to represent the security attributes of the remote process. However, as I mentioned above, the socket:connect permission is likely not the proper permission to use in the ASCONF case. > > * We should list out all of the current ASCONF parameter changes and > > have a discussion about which ones we want to protect with access > > controls, and of those remaining controls which should be grouped (if > > any). > > I agree that we should think this through before making changes. I > likely won't have enough time to implement the fixes myself, but at > least I will try to provide some food for thought for whoever will > attempt to tackle this :) That's disappointing. I'll drop what I'm doing and work on this, because someone needs to fix this. > > > Also we might want to have another check over just the socket context > > > to allow/disallow ADD_IP/SET_PRIMARY regardless of the peer so that > > > there is some level of control also when peer labeling is disabled. > > > > Are you talking about something like this: "allow socket_t > > self:sctp_socket some_asconf_perm"? I'd need to hear a good > > explanation for this and why we would need this control, because it > > doesn't make a lot of sense to me. If you are allowing an application > > to create a SCTP socket, you are granting it permission to make use of > > SCTP, including ASCONF. > > My idea is to provide policy writers/users (particularly those who > don't want to enable peer labeling) a simple way to control the usage > of ASCONF on the machine (either at global or socket level). ASCONF is > quite a niche feature of SCTP and probably won't be used by most > applications (and the SCTP RFC mandates that it be used only in > conjunction with auth chunks, which is itself another niche feature). > Given that ASCONF has security implications, I see it as a natural > requirement to be able to allow a domain to use SCTP itself, but not > to process ASCONF commands from any peer. I'll reserve final judgement because I need to go read the ASCONF related RFCs instead of just skimming them, but I'm not overly swayed by the argument above. SELinux should be about providing mechanisms which allow policy writers to control what entities on a system have access to a specific blob of data, restricting the details of the protocols which are used to access that data is getting a bit out of scope unless those details directly affect the ability to access the data. I think we should keep the name_connect check, although likely with a different name in the ASCONF case, however, that checks the socket/sock's label against the connection endpoint's addr/port and is something different from the network peer labeling controls we've been talking about. Although I believe the ASCONF/name_connect check should address the security concerns of an ASCONF induced change affecting the endpoint of an SCTP connection and effectively bypassing the earlier name_connect check done when the connection was first established. -- paul-moore.com
