Re: [PATCH 1/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
I see.
Yes, I need to do pfkey_sadb2xfrm_user_ctx there as well.
Regards,
Trent.
Trent Jaeger
IBM T.J. Watson Research Center
19 Skyline Drive, Hawthorne, NY 10532
(914) 784-7225, FAX (914) 784-7225
Herbert Xu [EMAIL PROTECTED]
08/09/2005 07:13 PM
To: Trent Jaeger/Watson/[EMAIL PROTECTED]
cc: [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED],
[EMAIL PROTECTED], netdev@vger.kernel.org, [EMAIL PROTECTED], Serge E
Hallyn/Austin/[EMAIL PROTECTED], [EMAIL PROTECTED]
Subject:Re: [PATCH 1/2] LSM-IPSec Networking Hooks --
revised flow cache [resend]
On Tue, Aug 09, 2005 at 02:20:45PM -0400, Trent Jaeger wrote:
What makes spddelete different from spdadd?
spddelete takes a context string as input and we need to retrieve the
policy that matches the selector (xfrm_policy_bysel) and the security
context. The additional code checks the latter. I think that the
conversion of the context string to a 'normalized' context struct must
be
done by the LSM before we can do this check as done above.
What I meant is why does spdadd do pfkey_sadb2xfrm_user_ctx while
spddelete doesn't?
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
I have a few questions on your comments. The ones for which I do not have
questions, I will modify as suggested.
diff -puN include/net/xfrm.h~lsm-xfrm-nethooks include/net/xfrm.h
--- linux-2.6.13-rc4-xfrm/include/net/xfrm.h~lsm-xfrm-nethooks
2005-08-01 16:11:22.0 -0400
+++ linux-2.6.13-rc4-xfrm-root/include/net/xfrm.h 2005-08-01
16:11:22.0 -0400
@@ -510,6 +514,27 @@ xfrm_selector_match(struct xfrm_selector
return 0;
}
+/* If neither has a context -- match
+ Otherwise, both must have a context and the sids, doi, alg must
match */
+static inline int xfrm_sec_ctx_match(struct xfrm_sec_ctx *s1, struct
xfrm_sec_ctx *s2)
+{
+ return ((!s1 !s2) ||
+ (s1 s2
+ (s1-ctx_sid == s2-ctx_sid)
+ (s1-ctx_doi == s2-ctx_doi)
+ (s1-ctx_alg == s2-ctx_alg)));
+}
Would it be possible to make this conditional on
CONFIG_SECURITY_NETWORK?
This is specific to CONFIG_SECURITY_NETWORK_XFRM as contexts will only be
used in that case. I will make it conditional on that instead, if that's
OK.
Regards,
Trent.
Trent Jaeger
IBM T.J. Watson Research Center
19 Skyline Drive, Hawthorne, NY 10532
(914) 784-7225, FAX (914) 784-7225
-
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/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
@@ -2108,7 +2230,18 @@ static int pfkey_spddelete(struct sock *
if (sel.dport)
sel.dport_mask = ~0;
-xp = xfrm_policy_bysel(pol-sadb_x_policy_dir-1, sel,
1);
+sec_ctx = (struct sadb_x_sec_ctx *)
ext_hdrs[SADB_X_EXT_SEC_CTX-1];
+memset(tmp, 0, sizeof(struct xfrm_policy));
+
+if (sec_ctx != NULL) {
+err = security_xfrm_policy_alloc(
+tmp, (struct
xfrm_user_sec_ctx *)sec_ctx);
What makes spddelete different from spdadd?
spddelete takes a context string as input and we need to retrieve the
policy that matches the selector (xfrm_policy_bysel) and the security
context. The additional code checks the latter. I think that the
conversion of the context string to a 'normalized' context struct must be
done by the LSM before we can do this check as done above.
I could hide this computation a bit better (it is also done for xfrm_user)
to clean up the code.
Regards,
Trent.
Trent Jaeger
IBM T.J. Watson Research Center
19 Skyline Drive, Hawthorne, NY 10532
(914) 784-7225, FAX (914) 784-7225
-
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/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
@@ -2703,10 +2837,22 @@ static struct xfrm_policy *pfkey_compile
(*dir = parse_ipsecrequests(xp, pol)) 0)
goto out;
+/* security context too */
+if (len = (pol-sadb_x_policy_len*8 +
+sizeof(struct sadb_x_sec_ctx))) {
+char *p = (char *) pol;
+p += pol-sadb_x_policy_len*8;
+sec_ctx = (struct sadb_x_sec_ctx *) p;
+if (security_xfrm_policy_alloc(
+xp, (struct
xfrm_user_sec_ctx *)sec_ctx))
+goto out;
+}
+
Do we really need socket-specific policies with security context?
Security context information is being used by some user-level appls, such
as XWindows, so I can see that applications may want to set security
contexts for their sockets based on the principal for whom the code is
being run. For example, we may want to prevent leakage of data from a
window in X to a remote client by setting the security context for a
socket which limits the receivers of such data.
Regards,
Trent.
PS -- This is all the questions/comments.
Trent Jaeger
IBM T.J. Watson Research Center
19 Skyline Drive, Hawthorne, NY 10532
(914) 784-7225, FAX (914) 784-7225
-
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/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
Trent Jaeger [EMAIL PROTECTED] wrote:
This is specific to CONFIG_SECURITY_NETWORK_XFRM as contexts will only be
used in that case. I will make it conditional on that instead, if that's
OK.
That sounds good.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
OK. Thanks for the comments. I'll get back soon.
Regards,
Trent.
Trent Jaeger
IBM T.J. Watson Research Center
19 Skyline Drive, Hawthorne, NY 10532
(914) 784-7225, FAX (914) 784-7225
Herbert Xu [EMAIL PROTECTED]
08/06/2005 03:45 AM
To: Trent Jaeger/Watson/[EMAIL PROTECTED]
cc: [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED],
netdev@vger.kernel.org, [EMAIL PROTECTED], Serge E
Hallyn/Austin/[EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED]
Subject:Re: [PATCH 1/2] LSM-IPSec Networking Hooks --
revised flow cache [resend]
On Tue, Aug 02, 2005 at 02:04:41PM -0400, jaegert wrote:
Resend of 20 July patch that repaired the flow_cache_lookup
authorization (now for 2.6.13-rc4-git4).
Thanks Trent. I'm happy with the flow cache stuff now.
However, there are still some technical details to take
care of.
diff -puN include/linux/xfrm.h~lsm-xfrm-nethooks include/linux/xfrm.h
--- linux-2.6.13-rc4-xfrm/include/linux/xfrm.h~lsm-xfrm-nethooks
2005-08-01 16:11:22.0 -0400
+++ linux-2.6.13-rc4-xfrm-root/include/linux/xfrm.h 2005-08-01
16:11:22.0 -0400
@@ -173,6 +201,7 @@ enum xfrm_attr_type_t {
XFRMA_ALG_CRYPT,/* struct xfrm_algo */
XFRMA_ALG_COMP, /* struct
xfrm_algo */
XFRMA_ENCAP,/* struct
xfrm_algo + struct xfrm_encap_tmpl */
+ XFRMA_SEC_CTX, /* struct xfrm_sec_ctx */
XFRMA_TMPL, /* 1 or more
struct xfrm_user_tmpl */
XFRMA_SA,
XFRMA_POLICY,
Please add it at the end of the enum as otherwise you may break
existing user-space applications. In this particular case the
breakage isn't serious since those three XFRMA types are fairly
recent but still it's better to be safe than sorry :)
diff -puN include/net/xfrm.h~lsm-xfrm-nethooks include/net/xfrm.h
--- linux-2.6.13-rc4-xfrm/include/net/xfrm.h~lsm-xfrm-nethooks
2005-08-01 16:11:22.0 -0400
+++ linux-2.6.13-rc4-xfrm-root/include/net/xfrm.h 2005-08-01
16:11:22.0 -0400
@@ -510,6 +514,27 @@ xfrm_selector_match(struct xfrm_selector
return 0;
}
+/* If neither has a context -- match
+ Otherwise, both must have a context and the sids, doi, alg must
match */
+static inline int xfrm_sec_ctx_match(struct xfrm_sec_ctx *s1, struct
xfrm_sec_ctx *s2)
+{
+ return ((!s1 !s2) ||
+ (s1 s2
+ (s1-ctx_sid == s2-ctx_sid)
+ (s1-ctx_doi == s2-ctx_doi)
+ (s1-ctx_alg == s2-ctx_alg)));
+}
Would it be possible to make this conditional on CONFIG_SECURITY_NETWORK?
+static inline struct xfrm_sec_ctx *xfrm_policy_security(struct
xfrm_policy *xp)
+{
+ return (xp ? xp-security : NULL);
+}
+
+static inline struct xfrm_sec_ctx *xfrm_state_security(struct
xfrm_state *x)
+{
+ return (x ? x-security : NULL);
+}
+
Do you really need these NULL checks? If not I'd suggest getting rid
of these altogether.
A quick glance at all the users of xfrm_policy_security in Patch 1
seems to indicate that none of those places can have xp being NULL.
diff -puN net/core/flow.c~lsm-xfrm-nethooks net/core/flow.c
--- linux-2.6.13-rc4-xfrm/net/core/flow.c~lsm-xfrm-nethooks 2005-08-01
16:11:22.0 -0400
+++ linux-2.6.13-rc4-xfrm-root/net/core/flow.c 2005-08-01
16:12:03.0 -0400
@@ -23,6 +23,7 @@
#include net/flow.h
#include asm/atomic.h
#include asm/semaphore.h
+#include linux/security.h
This appears to be unnecessary.
diff -puN net/ipv4/xfrm4_policy.c~lsm-xfrm-nethooks
net/ipv4/xfrm4_policy.c
--- linux-2.6.13-rc4-xfrm/net/ipv4/xfrm4_policy.c~lsm-xfrm-nethooks
2005-08-01 16:11:22.0 -0400
+++ linux-2.6.13-rc4-xfrm-root/net/ipv4/xfrm4_policy.c 2005-08-01
16:11:22.0 -0400
@@ -36,6 +36,8 @@ __xfrm4_find_bundle(struct flowi *fl, st
if (xdst-u.rt.fl.oif == fl-oif
/*XXX*/
xdst-u.rt.fl.fl4_dst == fl-fl4_dst
xdst-u.rt.fl.fl4_src == fl-fl4_src
+ xfrm_sec_ctx_match(xfrm_policy_security(policy),
+ xfrm_state_security(dst-xfrm))
Is this necessary? The policy's context must've matched the state's
context at its creation time. AFAIK there is no way for the security
context to change during their life-cycle.
diff -puN net/ipv6/xfrm6_policy.c~lsm-xfrm-nethooks
net/ipv6/xfrm6_policy.c
--- linux-2.6.13-rc4-xfrm/net/ipv6/xfrm6_policy.c~lsm-xfrm-nethooks
2005-08-01 16:11:22.0 -0400
+++ linux-2.6.13-rc4-xfrm-root/net/ipv6/xfrm6_policy.c 2005-08-01
16:11:22.0 -0400
@@ -54,6 +54,8 @@ __xfrm6_find_bundle(struct flowi *fl, st
xdst-u.rt6.rt6i_src.plen
Re: [PATCH 1/2] LSM-IPSec Networking Hooks -- revised flow cache [resend]
On Tue, Aug 02, 2005 at 02:04:41PM -0400, jaegert wrote:
Resend of 20 July patch that repaired the flow_cache_lookup
authorization (now for 2.6.13-rc4-git4).
Thanks for the resend. I'll try to get back to you soon.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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
