On Wednesday, August 08, 2012 10:09:38 PM Eric Dumazet wrote: > On Wed, 2012-08-08 at 15:59 -0400, Eric Paris wrote: > > Seems wrong. We shouldn't ever need ifdef CONFIG_SECURITY in core > > code. > > Sure but it seems include file misses an accessor for this. > > We could add it on a future cleanup patch, as Paul mentioned.
Actually, the issue is that the shared socket doesn't have an init/alloc function to do the LSM allocation like we do with other sockets so Eric's patch does it as part of ip_send_unicast_reply(). If we look at the relevant part of Eric's patch: +#ifdef CONFIG_SECURITY + if (!sk->sk_security && security_sk_alloc(sk, PF_INET, GFP_ATOMIC)) + goto out; +#endif ... if we were to remove the CONFIG_SECURITY conditional we would end up calling security_sk_alloc() each time through in the CONFIG_SECURITY=n case as sk->sk_security would never be initialized to a non-NULL value. In the CONFIG_SECURITY=y case it should only be called once as security_sk_alloc() should set sk->sk_security to a LSM blob. > > Ifndef CONF_SECURITY then security_sk_alloc() is a static > > > > inline return 0; I guess the question is "Where did the sk come > > from"? Why wasn't security_sk_alloc() called when it was allocated? > > Should it have been updated at some time and that wasn't done either? > > Seems wrong to be putting packets on the queue for a socket where the > > security data was never allocated and was never set to its proper > > state. > > IMHO it seems wrong to even care about security for internal sockets. > > They are per cpu, shared for all users on the machine. The issue, from a security point of view, is that these sockets are sending network traffic; even if it is just resets and timewait ACKs, it is still network traffic and the LSMs need to be able to enforce security policy on this traffic. After all, what would you say if your firewall let these same packets pass without any filtering? The issue I'm struggling with at present is how should we handle this traffic from a LSM perspective. The label based LSMs, e.g. SELinux and Smack, use the LSM blob assigned to locally generated outbound traffic to identify the traffic and apply the security policy, so not only do we have to resolve the issue of ensuring the traffic is labeled correctly, we have to do it with a shared socket (although the patch didn't change the shared nature of the socket). For those who are interested, I think the reasonable labeling solution here is to go with SECINITSID_KERNEL/kernel_t for SELinux and likely the ambient label for Smack as in both the TCP reset and timewait ACK there shouldn't be any actual user data present. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
