Re: [PATCH] xfrm_policy delete security check misplaced
From: Eric Paris [EMAIL PROTECTED] Date: Fri, 02 Mar 2007 13:29:50 -0500 The security hooks to check permissions to remove an xfrm_policy were actually done after the policy was removed. Since the unlinking and deletion are done in xfrm_policy_by* functions this moves the hooks inside those 2 functions. There we have all the information needed to do the security check and it can be done before the deletion. Since auditing requires the result of that security check err has to be passed back and forth from the xfrm_policy_by* functions. This patch also fixes a bug where a deletion that failed the security check could cause improper accounting on the xfrm_policy (xfrm_get_policy didn't have a put on the exit path for the hold taken by xfrm_policy_by*) It also fixes the return code when no policy is found in xfrm_add_pol_expire. In old code (at least back in the 2.6.18 days) err wasn't used before the return when no policy is found and so the initialization would cause err to be ENOENT. But since err has since been used above when we don't get a policy back from the xfrm_policy_by* function we would always return 0 instead of the intended ENOENT. Also fixed some white space damage in the same area. Signed-off-by: Eric Paris [EMAIL PROTECTED] Applied. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xfrm_policy delete security check misplaced
@@ -2552,7 +2550,7 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h return -EINVAL; xp = xfrm_policy_byid(XFRM_POLICY_TYPE_MAIN, dir, pol-sadb_x_policy_id, - hdr-sadb_msg_type == SADB_X_SPDDELETE2); + hdr-sadb_msg_type == SADB_X_SPDDELETE2, err); if (xp == NULL) return -ENOENT; I guess you meant to do this here? else if (err) return err; Also, [Joy cc'd] deletions here needn't be audited? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xfrm_policy delete security check misplaced
Also, [Joy cc'd] deletions here needn't be audited? OK, I see the next patch addressed this :) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xfrm_policy delete security check misplaced
Signed-off-by: Eric Paris [EMAIL PROTECTED] Acked-by: Venkat Yekkirala [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xfrm_policy delete security check misplaced
On Mon, 5 Mar 2007, Venkat Yekkirala wrote: Signed-off-by: Eric Paris [EMAIL PROTECTED] Acked-by: Venkat Yekkirala [EMAIL PROTECTED] What about your previous comment: I guess you meant to do this here? else if (err) return err; -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xfrm_policy delete security check misplaced
Signed-off-by: Eric Paris [EMAIL PROTECTED] Acked-by: Venkat Yekkirala [EMAIL PROTECTED] What about your previous comment: I guess you meant to do this here? else if (err) return err; I saw that this was taken care of in patch-2 for the delete case, but while err isn't currently applicable to the non-delete case, it would be proper/complete for err to still be handled for the non-delete case. Thanks for asking. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xfrm_policy delete security check misplaced
On Mon, 2007-03-05 at 11:39 -0500, James Morris wrote: On Mon, 5 Mar 2007, Venkat Yekkirala wrote: Signed-off-by: Eric Paris [EMAIL PROTECTED] Acked-by: Venkat Yekkirala [EMAIL PROTECTED] What about your previous comment: I guess you meant to do this here? else if (err) return err; That also gets taken care of in the pfkey_spdget cleanup in a later patch. The return isn't in that same place venkat suggested it instead happens inside the new if (delete) block. (err is only non-zero on delete operations so there is no need to check it otherwise) -Eric - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfrm_policy delete security check misplaced
On Fri, 2 Mar 2007, Eric Paris wrote: Signed-off-by: Eric Paris [EMAIL PROTECTED] Acked-by: James Morris [EMAIL PROTECTED] -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
