On Tuesday, February 22, 2011 7:11:43 AM Steffen Klassert wrote: > On Wed, Feb 16, 2011 at 03:11:25PM -0500, Paul Moore wrote: > > On Mon, 2011-02-14 at 14:20 +0100, Steffen Klassert wrote: > > > selinux_xfrm_decode_session is used to decode the security identifyer > > > of the originating flow. So return the sid of the socket instead of > > > SECSID_NULL, if we have socket context but no secpath. This is > > > needed because ip_xfrm_me_harder (and selinux_xfrm_decode_session) > > > is invoked on outbound packets and before the xfrm transformations, > > > therefore we have no secpath and the sid of a labeled IPsec SA usually > > > does not match SECSID_NULL. > > > > > > Signed-off-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx> > > > > Granted, it has been some time since I've looked at the labeled IPsec > > code in some detail so I might be a little off here, but is it possible > > to move the xfrm_decode LSM hook to an area on the outbound processing > > where we do have a valid secpath? > > No, we don't have a secpath for outbound packets. The secpath is used to > do a inbound policy check, check the used SA against their selectors etc. > With these checks we ensure that the packet is transformed back to it's > native form in the right way. That's not needed on outbound packets, > so we don't record a secpath in this case. The lack of a outbound > secpath was also the problem I mentioned in the introduction mail > of the patchset. Be warned: most of this reply is just me tossing out ideas ... Okay, so basically selinux_xfrm_decode_session() is a total waste of time in the output path, yes? I'm looking at the current code and it does nothing if the sec_path is NULL, so I'm wondering why we even keep it around for the outbound path. > > If not, I'd rather see us split this > > hook so that we preserve the existing xfrm_decode_session() hook for > > input (I believe it is also used for input, yes?) and create a new hook > > which only grabs the sksec's label on output (preferably named so that > > it is clear this is the socket's label and not the SA's label). > > I think that's not possible too. The security_xfrm_decode_session() > hook is used from within xfrm_decode_session(). This function > is used in codepaths that are used for both, inbound and outbound > processing (xfrm_lookup, xfrm_policy_check etc.). This makes me wonder if the LSM hook is even in the right place. > The xfrm_decode_session() function simply fills a 'struct flowi' with > the informations of the originating flow whenever these informations > are needed, selinux_xfrm_decode_session just adds the sid the > that struct. > > In the case we are constructing a 'struct flowi' within the outbound > packet path, we use selinux_sk_getsecid() which just adds the > sksec label from the socket if we have socket context. > > So with this patch we would use the secpath's sid on inbound > packets and the sksec label from the socket if we are on > oubound packet processing with socket context, when we construct > a 'struct flowi' from xfrm_decode_session(). I _really_ think we need to split the inbound and outbound handling for xfrm. I suppose we can always fall back to selecting the right path based on the presence of a sec_path pointer, but I would rather see us get rid of an unnecessary conditional. The inbound handling can stick with the logic in selinux_xfrm_decode_session() but the outbound handling should follow similar logic to how we determine the network peer label in selinux_ip_postroute(). 1. If we have a valid sk_security_stuct, use it. 2. If we are forwarding the packet, call selinux_skb_peerlbl_sid(). 3. If all else fails, use SECINITSID_KERNEL. -- paul moore linux @ hp -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.
