RE: [PATCH 3/3] mlsxfrm: Various fixes
On Wed, 2006-11-08 at 13:17 -0600, Venkat Yekkirala wrote: > > > > Not sure > > > > though when > > > > that would apply here, > > > > > > It could apply to xfrms if they happen to be using the context > > > represented by any of the initial SIDs. > > > > Which would happen when? > > If one were attempting to use a context pertaining to the unlabeled init > sid in the SPD and/or the SAD. But would I be correct in assuming that the > same sid (unlabeled init sid in all likelyhood) would end up being returned > when the context is turned into a sid, resulting in the SPD and the SAD > using > the same init sid, thus making a full-context compare unnecessary? Yes. > > What's the harm from just using the SID comparison and > > allowing for the > > possibility that there might be a few duplicates in rare > > circumstances? > > Does it break any assumptions in the rest of the logic? > > The best I can think of is if the SA's sid doesn't match the > socket's SID, IKE would come into play, if it's configured. > > I also wanted to conversely ask what harm exists if we did > a full-context compare in the event the sids didn't match? > > Are we just trying to generally avoid extra code? More complexity and overhead for no real gain. -- Stephen Smalley National Security Agency - 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 3/3] mlsxfrm: Various fixes
> > > Not sure > > > though when > > > that would apply here, > > > > It could apply to xfrms if they happen to be using the context > > represented by any of the initial SIDs. > > Which would happen when? If one were attempting to use a context pertaining to the unlabeled init sid in the SPD and/or the SAD. But would I be correct in assuming that the same sid (unlabeled init sid in all likelyhood) would end up being returned when the context is turned into a sid, resulting in the SPD and the SAD using the same init sid, thus making a full-context compare unnecessary? > > > > and it would only apply if both SIDs > > > were initial > > > SIDs. > > > > OK. Will narrow the full context comparison to just this case. > > What's the harm from just using the SID comparison and > allowing for the > possibility that there might be a few duplicates in rare > circumstances? > Does it break any assumptions in the rest of the logic? The best I can think of is if the SA's sid doesn't match the socket's SID, IKE would come into play, if it's configured. I also wanted to conversely ask what harm exists if we did a full-context compare in the event the sids didn't match? Are we just trying to generally avoid extra code? - 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 3/3] mlsxfrm: Various fixes
On Wed, 2006-11-08 at 08:34 -0600, Venkat Yekkirala wrote: > > Such duplication can occur among the initial SIDs. > > For some reason I thought that could happen between an initial SID > and a non-initial SID. I think only in the case where a non-initial SID has been invalidated by policy reload that renders its context illegal and is thus remapped to the unlabeled SID. > > Not sure > > though when > > that would apply here, > > It could apply to xfrms if they happen to be using the context > represented by any of the initial SIDs. Which would happen when? > > and it would only apply if both SIDs > > were initial > > SIDs. > > OK. Will narrow the full context comparison to just this case. What's the harm from just using the SID comparison and allowing for the possibility that there might be a few duplicates in rare circumstances? Does it break any assumptions in the rest of the logic? -- Stephen Smalley National Security Agency - 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 3/3] mlsxfrm: Various fixes
> Such duplication can occur among the initial SIDs. For some reason I thought that could happen between an initial SID and a non-initial SID. > Not sure > though when > that would apply here, It could apply to xfrms if they happen to be using the context represented by any of the initial SIDs. > and it would only apply if both SIDs > were initial > SIDs. OK. Will narrow the full context comparison to just this case. - 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 3/3] mlsxfrm: Various fixes
On Tue, 2006-11-07 at 15:29 -0500, Paul Moore wrote:
> Venkat Yekkirala wrote:
> > +/*
> > + * security_sid_compare() - compares two given sid contexts.
> > + * Returns 1 if they are equal, 0 otherwise.
> > + */
> > +int security_sid_compare(u32 sid1, u32 sid2)
> > +{
> > + struct context *context1;
> > + struct context *context2;
> > + int rc;
> > +
> > + if (!ss_initialized)
> > + return 1;
> > +
> > + if (sid1 == sid2)
> > + return 1;
> > + else if (sid1 > SECINITSID_NUM && sid2 > SECINITSID_NUM)
> > + return 0;
> > +
> > + /* explicit comparison in order */
> > +
> > + POLICY_RDLOCK;
> > + context1 = sidtab_search(&sidtab, sid1);
> > + if (!context1) {
> > + printk(KERN_ERR "security_sid_compare: unrecognized SID "
> > + "%u\n", sid1);
> > + rc = 0;
> > + goto out_unlock;
> > + }
> > +
> > + context2 = sidtab_search(&sidtab, sid2);
> > + if (!context2) {
> > + printk(KERN_ERR "security_sid_compare: unrecognized SID "
> > + "%u\n", sid2);
> > + rc = 0;
> > + goto out_unlock;
> > + }
> > +
> > + rc = context_cmp(context1, context2);
> > +
> > +out_unlock:
> > + POLICY_RDUNLOCK;
> > + return rc;
> > +}
>
> I understand wanting a generic LSM interface to do secid token comparisons,
> but
> in the SELinux implementation of this function I think we can get away with
> only
> a simple "sid1 == sid2" since the security server shouldn't be creating
> duplicate SID/secid values for identical contexts, I think. Did you run into
> something in testing that would indicate otherwise?
Such duplication can occur among the initial SIDs. Not sure though when
that would apply here, and it would only apply if both SIDs were initial
SIDs.
--
Stephen Smalley
National Security Agency
-
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 3/3] mlsxfrm: Various fixes
Venkat Yekkirala wrote:
> +/*
> + * security_sid_compare() - compares two given sid contexts.
> + * Returns 1 if they are equal, 0 otherwise.
> + */
> +int security_sid_compare(u32 sid1, u32 sid2)
> +{
> + struct context *context1;
> + struct context *context2;
> + int rc;
> +
> + if (!ss_initialized)
> + return 1;
> +
> + if (sid1 == sid2)
> + return 1;
> + else if (sid1 > SECINITSID_NUM && sid2 > SECINITSID_NUM)
> + return 0;
> +
> + /* explicit comparison in order */
> +
> + POLICY_RDLOCK;
> + context1 = sidtab_search(&sidtab, sid1);
> + if (!context1) {
> + printk(KERN_ERR "security_sid_compare: unrecognized SID "
> +"%u\n", sid1);
> + rc = 0;
> + goto out_unlock;
> + }
> +
> + context2 = sidtab_search(&sidtab, sid2);
> + if (!context2) {
> + printk(KERN_ERR "security_sid_compare: unrecognized SID "
> +"%u\n", sid2);
> + rc = 0;
> + goto out_unlock;
> + }
> +
> + rc = context_cmp(context1, context2);
> +
> +out_unlock:
> + POLICY_RDUNLOCK;
> + return rc;
> +}
I understand wanting a generic LSM interface to do secid token comparisons, but
in the SELinux implementation of this function I think we can get away with only
a simple "sid1 == sid2" since the security server shouldn't be creating
duplicate SID/secid values for identical contexts, I think. Did you run into
something in testing that would indicate otherwise?
--
paul moore
linux security @ hp
-
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
[PATCH 3/3] mlsxfrm: Various fixes
Fix the selection of an SA for an outgoing packet to be at the same
context as the originating socket/flow. This eliminates the SELinux
policy's ability to use/sendto SAs with contexts other than the socket's.
With this patch applied, the SELinux policy will require one or more of the
following for a socket to be able to communicate with/without SAs:
1. To enable a socket to communicate without using labeled-IPSec SAs:
allow socket_t unlabeled_t:association { sendto recvfrom }
2. To enable a socket to communicate with labeled-IPSec SAs:
allow socket_t self:association { sendto };
allow socket_t peer_sa_t:association { recvfrom };
Signed-off-by: Venkat Yekkirala <[EMAIL PROTECTED]>
---
include/linux/security.h| 19 -
net/xfrm/xfrm_policy.c |3
security/dummy.c|7 -
security/selinux/hooks.c| 26 --
security/selinux/include/security.h |2
security/selinux/include/xfrm.h |7 -
security/selinux/ss/services.c | 44 +++
security/selinux/xfrm.c | 97 --
8 files changed, 112 insertions(+), 93 deletions(-)
--- net-2.6.xfrm2/include/linux/security.h 2006-10-25 12:26:20.0
-0500
+++ net-2.6/include/linux/security.h2006-11-01 11:22:17.0 -0600
@@ -886,11 +886,6 @@ struct request_sock;
* @xp contains the policy to check for a match.
* @fl contains the flow to check for a match.
* Return 1 if there is a match.
- * @xfrm_flow_state_match:
- * @fl contains the flow key to match.
- * @xfrm points to the xfrm_state to match.
- * @xp points to the xfrm_policy to match.
- * Return 1 if there is a match.
* @xfrm_decode_session:
* @skb points to skb to decode.
* @secid points to the flow key secid to set.
@@ -1388,8 +1383,6 @@ struct security_operations {
int (*xfrm_policy_lookup)(struct xfrm_policy *xp, u32 fl_secid, u8 dir);
int (*xfrm_state_pol_flow_match)(struct xfrm_state *x,
struct xfrm_policy *xp, struct flowi *fl);
- int (*xfrm_flow_state_match)(struct flowi *fl, struct xfrm_state *xfrm,
- struct xfrm_policy *xp);
int (*xfrm_decode_session)(struct sk_buff *skb, u32 *secid, int ckall);
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
@@ -3186,12 +3179,6 @@ static inline int security_xfrm_state_po
return security_ops->xfrm_state_pol_flow_match(x, xp, fl);
}
-static inline int security_xfrm_flow_state_match(struct flowi *fl,
- struct xfrm_state *xfrm, struct xfrm_policy *xp)
-{
- return security_ops->xfrm_flow_state_match(fl, xfrm, xp);
-}
-
static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
{
return security_ops->xfrm_decode_session(skb, secid, 1);
@@ -3255,12 +3242,6 @@ static inline int security_xfrm_state_po
return 1;
}
-static inline int security_xfrm_flow_state_match(struct flowi *fl,
- struct xfrm_state *xfrm, struct xfrm_policy *xp)
-{
- return 1;
-}
-
static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
{
return 0;
--- net-2.6.xfrm2/net/xfrm/xfrm_policy.c2006-11-01 11:25:39.0
-0600
+++ net-2.6/net/xfrm/xfrm_policy.c 2006-11-01 12:10:23.0 -0600
@@ -1894,7 +1894,8 @@ int xfrm_bundle_ok(struct xfrm_policy *p
if (fl && !xfrm_selector_match(&dst->xfrm->sel, fl, family))
return 0;
- if (fl && !security_xfrm_flow_state_match(fl, dst->xfrm, pol))
+ if (fl && pol &&
+ !security_xfrm_state_pol_flow_match(dst->xfrm, pol, fl))
return 0;
if (dst->xfrm->km.state != XFRM_STATE_VALID)
return 0;
--- net-2.6.xfrm2/security/dummy.c 2006-10-25 12:23:47.0 -0500
+++ net-2.6/security/dummy.c2006-11-01 11:22:34.0 -0600
@@ -886,12 +886,6 @@ static int dummy_xfrm_state_pol_flow_mat
return 1;
}
-static int dummy_xfrm_flow_state_match(struct flowi *fl, struct xfrm_state
*xfrm,
- struct xfrm_policy *xp)
-{
- return 1;
-}
-
static int dummy_xfrm_decode_session(struct sk_buff *skb, u32 *fl, int ckall)
{
return 0;
@@ -1126,7 +1120,6 @@ void security_fixup_ops (struct security
set_to_dummy_if_null(ops, xfrm_state_delete_security);
set_to_dummy_if_null(ops, xfrm_policy_lookup);
set_to_dummy_if_null(ops, xfrm_state_pol_flow_match);
- set_to_dummy_if_null(ops, xfrm_flow_state_match);
set_to_dummy_if_null(ops, xfrm_decode_session);
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
#ifdef CONFIG_KEYS
--- net-2.6.xfrm2/security/selinux/include/xfrm.h 2006-11-07
09:49:24.0 -0600
+++ net-2.6/security/selinux/include/xfrm.h 2006-11-07 10:03:20.0
-0600
@@ -19,9 +19,6 @@ int selinux_xfrm_state_dele
