Deleting xfrms

Deleting xfrms

[Joy Latten]

I was looking at a patch D.Miller posted for xfrm_audit_log()
and could not help but notice that in pfkey_spddelete() and
xfrm_get_policy() we delete policy first and then check to see if we
have permissions to.  Am I missing the original intentions or 
is this incorrect?  Shouldn't it be check the permissions first and then
call xfrm_policy_bysel_ctx()? 

pfkey_spddelete() in af_key.c:

        xp = xfrm_policy_bysel_ctx(XFRM_POLICY_TYPE_MAIN,
pol->sadb_x_policy_dir-1,
                                   &sel, tmp.security, 1);
        security_xfrm_policy_free(&tmp);

        xfrm_audit_log(audit_get_loginuid(current->audit_context), 0,
                       AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL);

        if (xp == NULL)
                return -ENOENT;

        err = 0;

        if ((err = security_xfrm_policy_delete(xp)))
                goto out;
        c.seq = hdr->sadb_msg_seq;
        c.pid = hdr->sadb_msg_pid;
        c.event = XFRM_MSG_DELPOLICY;
        km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);


xfrm_get_policy() in xfrm_user.c is very similar.

Regards,
Joy

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Deleting xfrms

[Stephen Smalley]

On Mon, 2007-02-12 at 17:39 -0600, Joy Latten wrote:
> I was looking at a patch D.Miller posted for xfrm_audit_log()
> and could not help but notice that in pfkey_spddelete() and
> xfrm_get_policy() we delete policy first and then check to see if we
> have permissions to.  Am I missing the original intentions or 
> is this incorrect?  Shouldn't it be check the permissions first and then
> call xfrm_policy_bysel_ctx()?

IIUC, the security_xfrm_policy_free call is just freeing the temporary
object created from the user context in order to perform the lookup of
the xp.  The permission check occurs upon security_xfrm_policy_delete,
and the actual deletion of the policy occurs upon xfrm_pol_put ->
__xfrm_policy_destroy.  pfkey_spddelete() does look wrong, since it
always calls xfrm_pol_put on the out path, whereas xfrm_get_policy()
jumps over the xfrm_pol_put() call upon an error from
security_xfrm_policy_delete().

> 
> pfkey_spddelete() in af_key.c:
> 
>         xp = xfrm_policy_bysel_ctx(XFRM_POLICY_TYPE_MAIN,
> pol->sadb_x_policy_dir-1,
>                                    &sel, tmp.security, 1);
>         security_xfrm_policy_free(&tmp);
> 
>         xfrm_audit_log(audit_get_loginuid(current->audit_context), 0,
>                        AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL);
> 
>         if (xp == NULL)
>                 return -ENOENT;
> 
>         err = 0;
> 
>         if ((err = security_xfrm_policy_delete(xp)))
>                 goto out;
>         c.seq = hdr->sadb_msg_seq;
>         c.pid = hdr->sadb_msg_pid;
>         c.event = XFRM_MSG_DELPOLICY;
>         km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
> 
> 
> xfrm_get_policy() in xfrm_user.c is very similar.

 
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Deleting xfrms

[Stephen Smalley]

On Tue, 2007-02-13 at 07:39 -0500, Stephen Smalley wrote:
> On Mon, 2007-02-12 at 17:39 -0600, Joy Latten wrote:
> > I was looking at a patch D.Miller posted for xfrm_audit_log()
> > and could not help but notice that in pfkey_spddelete() and
> > xfrm_get_policy() we delete policy first and then check to see if we
> > have permissions to.  Am I missing the original intentions or 
> > is this incorrect?  Shouldn't it be check the permissions first and then
> > call xfrm_policy_bysel_ctx()?
> 
> IIUC, the security_xfrm_policy_free call is just freeing the temporary
> object created from the user context in order to perform the lookup of
> the xp.  The permission check occurs upon security_xfrm_policy_delete,
> and the actual deletion of the policy occurs upon xfrm_pol_put ->
> __xfrm_policy_destroy.  pfkey_spddelete() does look wrong, since it
> always calls xfrm_pol_put on the out path, whereas xfrm_get_policy()
> jumps over the xfrm_pol_put() call upon an error from
> security_xfrm_policy_delete().

Ah, sorry - I see what you mean now.  xfrm_policy_bysel_ctx() does
appear to unlink the policy and kill it, so it looks like you are
correct - the security_xfrm_policy_delete() hook is being called too
late.  

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: Deleting xfrms

[Venkat Yekkirala]

I see this bug crept in here:

http://marc.theaimsgroup.com/?l=linux-netdev&m=114956850915839&w=2

Are you planning to fix this or did you want me to?

> -----Original Message-----
> From: Joy Latten [mailto:latten@austin.ibm.com]
> Sent: Monday, February 12, 2007 5:40 PM
> To: jmorris@namei.org; vyekkirala@TrustedCS.com
> Cc: selinux@tycho.nsa.gov; redhat-lspp@redhat.com
> Subject: Deleting xfrms
> 
> 
> I was looking at a patch D.Miller posted for xfrm_audit_log()
> and could not help but notice that in pfkey_spddelete() and
> xfrm_get_policy() we delete policy first and then check to see if we
> have permissions to.  Am I missing the original intentions or 
> is this incorrect?  Shouldn't it be check the permissions 
> first and then
> call xfrm_policy_bysel_ctx()? 
> 
> pfkey_spddelete() in af_key.c:
> 
>         xp = xfrm_policy_bysel_ctx(XFRM_POLICY_TYPE_MAIN,
> pol->sadb_x_policy_dir-1,
>                                    &sel, tmp.security, 1);
>         security_xfrm_policy_free(&tmp);
> 
>         xfrm_audit_log(audit_get_loginuid(current->audit_context), 0,
>                        AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, 
> xp, NULL);
> 
>         if (xp == NULL)
>                 return -ENOENT;
> 
>         err = 0;
> 
>         if ((err = security_xfrm_policy_delete(xp)))
>                 goto out;
>         c.seq = hdr->sadb_msg_seq;
>         c.pid = hdr->sadb_msg_pid;
>         c.event = XFRM_MSG_DELPOLICY;
>         km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
> 
> 
> xfrm_get_policy() in xfrm_user.c is very similar.
> 
> Regards,
> Joy
> 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [redhat-lspp] RE: Deleting xfrms

[Eric Paris]

On Mon, 2007-02-19 at 11:37 -0600, Venkat Yekkirala wrote:
> I see this bug crept in here:
> 
> http://marc.theaimsgroup.com/?l=linux-netdev&m=114956850915839&w=2
> 
> Are you planning to fix this or did you want me to?

Actually I was going to take a stab at it this afternoon.  But I won't
complain if you beat me to it!

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [redhat-lspp] RE: Deleting xfrms

[Joy Latten]

On Mon, 2007-02-19 at 12:47 -0500, Eric Paris wrote:
> On Mon, 2007-02-19 at 11:37 -0600, Venkat Yekkirala wrote:
> > I see this bug crept in here:
> > 
> > http://marc.theaimsgroup.com/?l=linux-netdev&m=114956850915839&w=2
> > 
> > Are you planning to fix this or did you want me to?
> 
> Actually I was going to take a stab at it this afternoon.  But I won't
> complain if you beat me to it!
> 
 If either of you fix it, it would be great to me. I am currently
working on the loopback patch. So far things are looking ok... if 
my stress tests continue with no errors, I will send the patches
to ipsec-tools tomorrow for review.

Next is the "ipsec dropping first packet", see
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225328 
I encountered problems when I tried to patch the 64 kernel a few weeks
ago. I could not get labeled ipsec to work... hopefully there will be
more success in lspp 65 kernel. Will try the lspp.65 patch next.
Also, when I tried the patch in upstream kernel, my kernel panic'd
because of https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228409
so I have not been able to make much progress with this patch in either
upstream or lspp kernel. Also, I only saw panic in upstream kernel, not
lspp 64 kernel at the time.

Thanks!!!!!

Joy 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.