RE: [PATCH 1/3] Fix for IPsec leakage with SELinux enabled - V.03
On Thu, 5 Oct 2006, Venkat Yekkirala wrote:
> > > - if (xfrm_policy_match(pol, fl, type, family, dir)) {
> > > + err = xfrm_policy_match(pol, fl, type, family, dir);
> > > + if (err) {
> > > + if (err == -ESRCH)
> > > + continue;
> > > + else {
> > > + ret = ERR_PTR(err);
> > > + goto fail;
> > > + }
> > > + } else {
> >
> > Semantics issue: if the exact policy match fails with
> > -EACCESS, should we
> > then try an inexact match before failing?
>
> I wonder what you mean by an inexact match here.
There are two policy chains in the kernel: 'bydst'for non-prefixed
addresses (e.g. /32 or /128 nodes) and 'byidx' for prefixed addresses.
xfrm_policy_lookup_bytype() first checks for a non-prefixed match and then
for a prefixed match with a higher priority mark.
So, I wonder if it makes sense to continue on to the prefixed lookup if we
get an -EACCESS on the non-prefixed lookup.
Perhaps some further discussion on this with MLS folk is needed, but in
any case, I don't think it should block this patchset going in as a
bugfix, as the behavior can be changed in a further patch if required.
- James
--
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 1/3] Fix for IPsec leakage with SELinux enabled - V.03
On Thu, 5 Oct 2006, Venkat Yekkirala wrote: > > We're trying to fill the flow cache here. In the case where we'd > > have a match in both the sub-policy and main table, I think the > > sub-policy is supposed to take precedence, and if you fail to get > > this sub-policy you should fail the entire lookup. > > Which is what's happening here correct? Yes, the patch is correct for this. The way sub-policy is used is during Mobile IP, where you have multiple policies composed, so it wouldn't make sense for one of the policies to be rejected and the other allowed and for packets to flow. - James -- 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 1/3] Fix for IPsec leakage with SELinux enabled - V.03
From: Venkat Yekkirala <[EMAIL PROTECTED]> Date: Thu, 5 Oct 2006 17:27:03 -0400 > May be James can help me understand this; when exactly would a sub-policy > be "notice"d here? What does "put the whole thing together" mean? The code in xfrm_lookup() which does a flow cache lookup, and then if it finds it has obtained a sub-policy it tries to do an explicit main table policy lookup. The sub-policy and the main table policy thus found are "put together" to form the full route. - 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 1/3] Fix for IPsec leakage with SELinux enabled - V.03
> From: James Morris <[EMAIL PROTECTED]>
> Date: Thu, 5 Oct 2006 16:54:38 -0400 (EDT)
>
> > > #ifdef CONFIG_XFRM_SUB_POLICY
> > > pol = xfrm_policy_lookup_bytype(XFRM_POLICY_TYPE_SUB,
> fl, family, dir);
> > > - if (pol)
> > > + if (IS_ERR(pol)) {
> > > + err = PTR_ERR(pol);
> > > + pol = NULL;
> > > + }
> > > + if (pol || err)
> > > goto end;
> >
> > Similarly, if the sub-policy lookup returns -EACCESS,
> should we then try a
> > main policy lookup before failing?
>
> We're trying to fill the flow cache here. In the case where we'd
> have a match in both the sub-policy and main table, I think the
> sub-policy is supposed to take precedence, and if you fail to get
> this sub-policy you should fail the entire lookup.
Which is what's happening here correct?
>
> The way the sub-policied entries work is that you find the sub-policy
> as the primary object in the flow cache, and once you notice you have
> a sub-policy you do an explicit lookup in the main table to put the
> whole thing together.
May be James can help me understand this; when exactly would a sub-policy
be "notice"d here? What does "put the whole thing together" mean?
-
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 1/3] Fix for IPsec leakage with SELinux enabled - V.03
> > - if (xfrm_policy_match(pol, fl, type, family, dir)) {
> > + err = xfrm_policy_match(pol, fl, type, family, dir);
> > + if (err) {
> > + if (err == -ESRCH)
> > + continue;
> > + else {
> > + ret = ERR_PTR(err);
> > + goto fail;
> > + }
> > + } else {
>
> Semantics issue: if the exact policy match fails with
> -EACCESS, should we
> then try an inexact match before failing?
I wonder what you mean by an inexact match here.
>
> > #ifdef CONFIG_XFRM_SUB_POLICY
> > pol = xfrm_policy_lookup_bytype(XFRM_POLICY_TYPE_SUB,
> fl, family, dir);
> > - if (pol)
> > + if (IS_ERR(pol)) {
> > + err = PTR_ERR(pol);
> > + pol = NULL;
> > + }
> > + if (pol || err)
> > goto end;
>
> Similarly, if the sub-policy lookup returns -EACCESS, should
> we then try a
> main policy lookup before failing?
I would think not since we are already handling the more usual
"failure" of EACCES properly, and any other error would usually
have to be a near-fatal error concerning the whole LSM policy or
temporary memory pressure, for example. Usually the latter is auto
handled when the callers reattempt the llokup.
While it is theoretically possible
that the LSM might generate an error for the sub but not for the main,
we would have to first redefine the LSM hook to communicate this
differentiation. And at least in the case of the current user of LSM
(SELinux)
I don't currently see the need for this differentiation.
>
> I would think yes to both.
>
> Opinions?
>
>
> - James
> --
> 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 1/3] Fix for IPsec leakage with SELinux enabled - V.03
From: James Morris <[EMAIL PROTECTED]>
Date: Thu, 5 Oct 2006 16:54:38 -0400 (EDT)
> > #ifdef CONFIG_XFRM_SUB_POLICY
> > pol = xfrm_policy_lookup_bytype(XFRM_POLICY_TYPE_SUB, fl, family, dir);
> > - if (pol)
> > + if (IS_ERR(pol)) {
> > + err = PTR_ERR(pol);
> > + pol = NULL;
> > + }
> > + if (pol || err)
> > goto end;
>
> Similarly, if the sub-policy lookup returns -EACCESS, should we then try a
> main policy lookup before failing?
We're trying to fill the flow cache here. In the case where we'd
have a match in both the sub-policy and main table, I think the
sub-policy is supposed to take precedence, and if you fail to get
this sub-policy you should fail the entire lookup.
The way the sub-policied entries work is that you find the sub-policy
as the primary object in the flow cache, and once you notice you have
a sub-policy you do an explicit lookup in the main table to put the
whole thing together.
-
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 1/3] Fix for IPsec leakage with SELinux enabled - V.03
On Thu, 5 Oct 2006, Venkat Yekkirala wrote:
> - if (xfrm_policy_match(pol, fl, type, family, dir)) {
> + err = xfrm_policy_match(pol, fl, type, family, dir);
> + if (err) {
> + if (err == -ESRCH)
> + continue;
> + else {
> + ret = ERR_PTR(err);
> + goto fail;
> + }
> + } else {
Semantics issue: if the exact policy match fails with -EACCESS, should we
then try an inexact match before failing?
> #ifdef CONFIG_XFRM_SUB_POLICY
> pol = xfrm_policy_lookup_bytype(XFRM_POLICY_TYPE_SUB, fl, family, dir);
> - if (pol)
> + if (IS_ERR(pol)) {
> + err = PTR_ERR(pol);
> + pol = NULL;
> + }
> + if (pol || err)
> goto end;
Similarly, if the sub-policy lookup returns -EACCESS, should we then try a
main policy lookup before failing?
I would think yes to both.
Opinions?
- James
--
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
