[PATCH 1/1]: Add support for aes-ctr to ipsec
Very sorry, re-posting as first patch was incomplete. The below patch allows IPsec to use CTR mode with AES encryption algorithm. Tested this using setkey in ipsec-tools. regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] -- diff -urpN net-2.6.25/include/linux/pfkeyv2.h net-2.6.25.patch/include/linux/pfkeyv2.h --- net-2.6.25/include/linux/pfkeyv2.h 2008-01-29 11:48:00.0 -0600 +++ net-2.6.25.patch/include/linux/pfkeyv2.h2008-01-29 13:43:59.0 -0600 @@ -298,6 +298,7 @@ struct sadb_x_sec_ctx { #define SADB_X_EALG_BLOWFISHCBC7 #define SADB_EALG_NULL 11 #define SADB_X_EALG_AESCBC 12 +#define SADB_X_EALG_AESCTR 13 #define SADB_X_EALG_CAMELLIACBC22 #define SADB_EALG_MAX 253 /* last EALG */ /* private allocations should use 249-255 (RFC2407) */ diff -urpN net-2.6.25/net/xfrm/xfrm_algo.c net-2.6.25.patch/net/xfrm/xfrm_algo.c --- net-2.6.25/net/xfrm/xfrm_algo.c 2008-01-29 11:48:03.0 -0600 +++ net-2.6.25.patch/net/xfrm/xfrm_algo.c 2008-01-29 13:42:43.0 -0600 @@ -300,6 +300,23 @@ static struct xfrm_algo_desc ealg_list[] .sadb_alg_maxbits = 256 } }, +{ + .name = rfc3686(ctr(aes)), + + .uinfo = { + .encr = { + .blockbits = 128, + .defkeybits = 160, /* 128-bit key + 32-bit nonce */ + } + }, + + .desc = { + .sadb_alg_id = SADB_X_EALG_AESCTR, + .sadb_alg_ivlen = 8, + .sadb_alg_minbits = 128, + .sadb_alg_maxbits = 256 + } +}, }; static struct xfrm_algo_desc calg_list[] = { -- 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[1/1]: Add ctr-aes support to ipsec
The below patch allows IPsec to use CTR mode with AES encryption algorithm. Tested this using setkey in ipsec-tools. regards, Joy diff -urpN net-2.6.25/include/linux/pfkeyv2.h net-2.6.25.patch/include/linux/pfkeyv2.h --- net-2.6.25/include/linux/pfkeyv2.h 2008-01-29 11:48:00.0 -0600 +++ net-2.6.25.patch/include/linux/pfkeyv2.h2008-01-29 13:43:59.0 -0600 @@ -298,6 +298,7 @@ struct sadb_x_sec_ctx { #define SADB_X_EALG_BLOWFISHCBC7 #define SADB_EALG_NULL 11 #define SADB_X_EALG_AESCBC 12 +#define SADB_X_EALG_AESCTR 13 #define SADB_X_EALG_CAMELLIACBC22 #define SADB_EALG_MAX 253 /* last EALG */ /* private allocations should use 249-255 (RFC2407) */ diff -urpN net-2.6.25/net/xfrm/xfrm_algo.c net-2.6.25.patch/net/xfrm/xfrm_algo.c --- net-2.6.25/net/xfrm/xfrm_algo.c 2008-01-29 11:48:03.0 -0600 +++ net-2.6.25.patch/net/xfrm/xfrm_algo.c 2008-01-29 13:42:43.0 -0600 @@ -300,6 +300,23 @@ static struct xfrm_algo_desc ealg_list[] .sadb_alg_maxbits = 256 } }, +{ + .name = rfc3686(ctr(aes)), + + .uinfo = { + .encr = { + .blockbits = 128, + .defkeybits = 160, /* 128-bit key + 32-bit nonce */ + } + }, + + .desc = { + .sadb_alg_id = SADB_X_EALG_AESCTR, + .sadb_alg_ivlen = 8, + .sadb_alg_minbits = 128, + .sadb_alg_maxbits = 256 + } +}, }; static struct xfrm_algo_desc calg_list[] = { -- 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: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
Rereading the thread it's unclear to me which solution was deemed correct. I'm not a big fan of fiddling/forcing SA lifetimes unless we have no other option; if someone is foolish enough to use manual keying with replay protection and no mechanism to catch rollover then they most likely have larger problems. It's the whole we'll provide you with the gun, but you have to shoot yourself argument as applied to SA lifetimes. Also, the ipsec rfc require auotmated SA management when using anti-replay service and that the option be disabled when SAs are manually setup. It may not stop anyone, but we can always point to rfc. :-) Joy -- 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: Labeled IPsec with NAT
I am working on setting up Labeled IPsec along with iptables nat rules. Once I insert nat related rules, the ipsec connection breaks and the system tries to re-negotiate and creates multiple SAs. I am using 2.6.19 kernel (with Venkat's MLSXFRM patches bugfixes). I guess those were incorporated into the 2.6.20 kernel. Are you using racoon in ipsec-tools to create labeled SAs? If so, I recall seeing duplicate SA pairs being created while using racoon for labeled ipsec. We determined userspace should manage this. I posted a patch to ipsec-tools, but I don't think they ever picked it up... I do not know if this is same problem, but you could give it a try. regards, Joy -- 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: RFC4303 compliant auditing
On Fri, 2007-12-07 at 16:06 -0500, Paul Moore wrote: On Friday 07 December 2007 3:52:31 pm Eric Paris wrote: On Fri, 2007-12-07 at 14:57 -0500, Paul Moore wrote: NOTE: This really is an RFC patch, it compiles and boots but that is pretty much all I can promise at this point. I'm posting this patch to gather feedback from the audit crowd about the continued overloading of the AUDIT_MAC_IPSEC_EVENT message type - continue to use it or create a new audit message type? Of course any other comments people may have are always welcome. I'm all for continuing to use it, but I feel like the op= strings should probably all get collected up in one place to ease maintenance in the future, might not matter but it's nice to be able to look only on place in the code to find all of the possible op= Agreed. I punted on doing anything here for two main reasons: 1. It makes sense to do this in the xfrm_audit_start() function which I couldn't use here without some overhaul ... 2. ... I didn't want to overhaul anything if I was going to end up using separate message types. If we decide to go with a single audit message type (kinda sounds like it) I'll fix this up in the next version. The one advantage to multiple messages is the ability to exclude and not audit certain things. How often will these extra messages actually pop out of a system? Enough that people would likely still care about some of them but decide they don't want others? I don't know this stuff, so tell me how often would any of these show up? Bingo, this is the whole reason why I was wondering about a different message type. Currently only SAD and SPD changes are audited and only because they effect the security labels that are assigned to packets as they are imported/exported out of the system (look at the LSPP requirements for auditing the import and export of data). These new audit messages apply to individual packets and/or a particular SA and have nothing to do with security labels, rather they indicate error conditions found during normal IPsec processing. It would be difficult to think of all of the particular cases where these error conditions but in general I would say that these audit messages should not be common. Yes, I agree. They should not happen often. Especially compared to LSPP requirements of auditing whenever SA or SPD entries were added or deleted, which are common events. The only reason for creating a separate audit message type for these packet/SA messages would be to meet a RFC requirement that states that the implementation MUST allow the administrator to enable and disable ESP auditing. Now, we can probably say we fulfill that requirement regardless, but more message types allow us greater granularity and flexibility ... Also, there is great possibility of additional messages. This is for RFC 4303, which is ESP. There are also audit messages listed for rfc 4301-IPsec architecture and rfc 4302-AH that may happen later. -- 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: SPD auditing fix to include the netmask/prefix-length
On Fri, 2007-11-30 at 09:51 -0500, Paul Moore wrote: On Thursday 29 November 2007 8:45:46 am Paul Moore wrote: On Thursday 29 November 2007 5:34:59 am Herbert Xu wrote: On Mon, Nov 26, 2007 at 07:55:12PM +, Paul Moore wrote: Currently the netmask/prefix-length of an IPsec SPD entry is not included in any of the SPD related audit messages. This can cause a problem when the audit log is examined as the netmask/prefix-length is vital in determining what network traffic is affected by a particular SPD entry. This patch fixes this problem by adding two additional fields, src_prefixlen and dst_prefixlen, to the SPD audit messages to indicate the source and destination netmasks. These new fields are only included in the audit message when the netmask/prefix-length is less than the address length, i.e. the SPD entry applies to a network address and not a host address. Any reason why we don't just always include them? The audit folks seem to be very sensitive to the size/length of the audit messages, they prefer they be as small as possible. I thought that one way to save space would be to only print the prefix length information when the address referred to a network and not a single host. Would you prefer it if the prefix length information was always included in the audit message? Joy? Audit folks? Steve and/or Joy, could we get a verdict on this issue? The lack of a netmask in the SPD audit messages is pretty serious so I'd like to see this fixed as soon as possible. I think Steve may be able to answer this much better than I can in regards to audit. In my opinion having the netmask is good. regards, Joy - 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 1/1]: SAs created although tmpl-optional set
Although tmpl-optional is set, kernel still attempts to create a set of SAs. In xfrm_tmpl_resolve_one(), xfrm_state_find() is called to find an SA. First time, there won't be an SA, so an ACQUIRE will be sent and code then returns to xfrm_tmpl_resolv_one() who then checks tmpl-optional. Since tmpl-optional is set, the xfrm code will then allow flow to pass not transformed. I may have misinterpreted semantics of tmpl-optional, (I thought it meant use an SA only if there is one, otherwise do not transform) but do we want to create an SA as well as send flow without xfrm'ing? regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.orig/net/xfrm/xfrm_state.c linux-2.6.spd/net/xfrm/xfrm_state.c --- linux-2.6.orig/net/xfrm/xfrm_state.c2007-11-18 16:53:16.0 -0600 +++ linux-2.6.spd/net/xfrm/xfrm_state.c 2007-11-18 23:38:08.0 -0600 @@ -814,6 +814,12 @@ xfrm_state_find(xfrm_address_t *daddr, x error = -EEXIST; goto out; } + + if (tmpl-optional) { + error = 0; + goto out; + } + x = xfrm_state_alloc(); if (x == NULL) { error = -ENOMEM; - 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/1]: SAs created although tmpl-optional set
Heh I made the same mistake when I first read this piece of code too :) The optional flag isn't saying that it doesn't need to be protected, but rather that the SA may not be present on input. It's only used for IPComp where we may skip the IPComp if the data is not compressible. In other words the optional flag is really only meaningful on inbuond policy checks. Thanks for clearing that up for me. :-) I think it is not documented clearly in ipsec-tools. Joy - 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 1/1]: Using ICMP type and code in xfrm selector
ICMP message type and/or code may be value 0 when used as selector. Currently, if you specify SPD entry with upper layer protocol set as icmp, specify message type is 0 and code is 0, then all icmp messages get mapped to this. It appears value 0 for port is interpreted to mean ANY, which is not entirely true for ICMP and MH message type values. Below patch fixes this so ICMP message types and codes as well as MH types having value 0 aren't interpreted as ANY. While fixing this I wondered why we put icmp message type in sport and code in dport? recent ipsec rfc 4301 says: If the Next Layer Protocol is a Mobility Header, then there is a selector for IPv6 Mobility Header message type (MH type) [Mobip]. This is an 8-bit value that identifies a particular mobility message. Note that the MH type may not be available in the case of receipt of a fragmented packet. (See Section 7, Handling Fragments.) For IKE, the IPv6 Mobility Header message type (MH type) is placed in the most significant eight bits of the 16-bit local port selector. If the Next Layer Protocol value is ICMP, then there is a 16-bit selector for the ICMP message type and code. The message type is a single 8-bit value, which defines the type of an ICMP message, or ANY. The ICMP code is a single 8-bit value that defines a specific subtype for an ICMP message. For IKE, the message type is placed in the most significant 8 bits of the 16-bit selector and the code is placed in the least significant 8 bits. Should I leave as is or put both type and code into sport and also copy into dport to be closer to rfc? Similar question for MH type... Seems ok as is, but I could be missing something. xfrm_user did not appear to require this change. I tested icmp with my patched ipsec-tools. Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.24-rc1-git11/include/linux/ipsec.h linux-2.6.24-rc1-git11.patch/include/linux/ipsec.h --- linux-2.6.24-rc1-git11/include/linux/ipsec.h2007-11-02 16:36:30.0 -0500 +++ linux-2.6.24-rc1-git11.patch/include/linux/ipsec.h 2007-11-02 16:52:57.0 -0500 @@ -8,6 +8,7 @@ #define IPSEC_PORT_ANY 0 #define IPSEC_ULPROTO_ANY 255 #define IPSEC_PROTO_ANY255 +#define IPSEC_ICMPMH_ANY 255 enum { IPSEC_MODE_ANY = 0,/* We do not support this for SA */ diff -urpN linux-2.6.24-rc1-git11/net/key/af_key.c linux-2.6.24-rc1-git11.patch/net/key/af_key.c --- linux-2.6.24-rc1-git11/net/key/af_key.c 2007-11-02 16:39:40.0 -0500 +++ linux-2.6.24-rc1-git11.patch/net/key/af_key.c 2007-11-02 16:44:17.0 -0500 @@ -568,6 +568,35 @@ static int pfkey_sadb_addr2xfrm_addr(str /* NOTREACHED */ } +static void pfkey_set_sportmask(struct xfrm_selector *sel) +{ + switch(sel-proto) { + case IPPROTO_ICMP: + case IPPROTO_ICMPV6: + case IPPROTO_MH: + if (sel-sport != IPSEC_ICMPMH_ANY) + sel-sport_mask = htons(0x); + break; + default: + if (sel-sport) + sel-sport_mask = htons(0x); + } +} + +static void pfkey_set_dportmask(struct xfrm_selector *sel) +{ + switch(sel-proto) { + case IPPROTO_ICMP: + case IPPROTO_ICMPV6: + if (sel-dport != IPSEC_ICMPMH_ANY) + sel-dport_mask = htons(0x); + break; + default: + if (sel-dport) + sel-dport_mask = htons(0x); + } +} + static struct xfrm_state *pfkey_xfrm_state_lookup(struct sadb_msg *hdr, void **ext_hdrs) { struct sadb_sa *sa; @@ -2218,8 +2247,7 @@ static int pfkey_spdadd(struct sock *sk, xp-selector.prefixlen_s = sa-sadb_address_prefixlen; xp-selector.proto = pfkey_proto_to_xfrm(sa-sadb_address_proto); xp-selector.sport = ((struct sockaddr_in *)(sa+1))-sin_port; - if (xp-selector.sport) - xp-selector.sport_mask = htons(0x); + pfkey_set_sportmask(xp-selector); sa = ext_hdrs[SADB_EXT_ADDRESS_DST-1], pfkey_sadb_addr2xfrm_addr(sa, xp-selector.daddr); @@ -2231,8 +2259,7 @@ static int pfkey_spdadd(struct sock *sk, xp-selector.proto = pfkey_proto_to_xfrm(sa-sadb_address_proto); xp-selector.dport = ((struct sockaddr_in *)(sa+1))-sin_port; - if (xp-selector.dport) - xp-selector.dport_mask = htons(0x); + pfkey_set_dportmask(xp-selector); sec_ctx = (struct sadb_x_sec_ctx *) ext_hdrs[SADB_X_EXT_SEC_CTX-1]; if (sec_ctx != NULL) { @@ -2324,16 +2351,14 @@ static int pfkey_spddelete(struct sock * sel.prefixlen_s = sa-sadb_address_prefixlen; sel.proto = pfkey_proto_to_xfrm(sa-sadb_address_proto
Re: net-2.6.24 build problem
On Wed, 2007-09-12 at 07:18 -0700, David Miller wrote: From: Stephen Hemminger [EMAIL PROTECTED] Date: Wed, 12 Sep 2007 16:08:33 +0200 ERROR: xfrm_audit_state_delete [net/key/af_key.ko] undefined! ERROR: xfrm_audit_state_add [net/key/af_key.ko] undefined! ERROR: xfrm_audit_policy_add [net/key/af_key.ko] undefined! ERROR: xfrm_audit_policy_delete [net/key/af_key.ko] undefined I just checked in the following fix for this: Thanks. I took note for future reference, so it won't happen again. Joy - 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 audit calls
On Wed, 2007-09-12 at 14:56 -0400, [EMAIL PROTECTED] wrote: On Tue, 11 Sep 2007 19:03:14 CDT, Joy Latten said: This patch modifies the current ipsec audit layer by breaking it up into purpose driven audit calls. So far, the only audit calls made are when add/delete an SA/policy. What other audit calls do you envision adding in the future? Those specified in updated RFCs for ipsec, mainly 4301, 4302 and 4303. Joy - 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]: xfrm audit calls
This patch modifies the current ipsec audit layer by breaking it up into purpose driven audit calls. So far, the only audit calls made are when add/delete an SA/policy. It had been discussed to give each key manager it's own calls to do this, but I found there to be much redundnacy since they did the exact same things, except for how they got auid and sid, so I combined them. The below audit calls can be made by any key manager. Hopefully, this is ok. I compiled and tested with CONFIG_AUDITSYSCALLS on and off. Regards, Joy Latten Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.22/include/linux/audit.h linux-2.6.22-rc6/include/linux/audit.h --- linux-2.6.22/include/linux/audit.h 2007-09-11 13:59:49.0 -0500 +++ linux-2.6.22-rc6/include/linux/audit.h 2007-09-11 14:10:57.0 -0500 @@ -108,10 +108,11 @@ #define AUDIT_MAC_CIPSOV4_DEL 1408/* NetLabel: del CIPSOv4 DOI entry */ #define AUDIT_MAC_MAP_ADD 1409/* NetLabel: add LSM domain mapping */ #define AUDIT_MAC_MAP_DEL 1410/* NetLabel: del LSM domain mapping */ -#define AUDIT_MAC_IPSEC_ADDSA 1411/* Add a XFRM state */ -#define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ -#define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ -#define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_ADDSA 1411/* Not used */ +#define AUDIT_MAC_IPSEC_DELSA 1412/* Not used */ +#define AUDIT_MAC_IPSEC_ADDSPD 1413/* Not used */ +#define AUDIT_MAC_IPSEC_DELSPD 1414/* Not used */ +#define AUDIT_MAC_IPSEC_EVENT 1415/* Audit an IPSec event */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 diff -urpN linux-2.6.22/include/net/xfrm.h linux-2.6.22-rc6/include/net/xfrm.h --- linux-2.6.22/include/net/xfrm.h 2007-09-11 13:59:49.0 -0500 +++ linux-2.6.22-rc6/include/net/xfrm.h 2007-09-11 14:10:57.0 -0500 @@ -12,6 +12,7 @@ #include linux/ipsec.h #include linux/in6.h #include linux/mutex.h +#include linux/audit.h #include net/sock.h #include net/dst.h @@ -421,15 +422,46 @@ extern unsigned int xfrm_policy_count[XF /* Audit Information */ struct xfrm_audit { - uid_t loginuid; + u32 loginuid; u32 secid; }; #ifdef CONFIG_AUDITSYSCALL -extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, - struct xfrm_policy *xp, struct xfrm_state *x); +static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid) +{ + struct audit_buffer *audit_buf = NULL; + char *secctx; + u32 secctx_len; + + audit_buf = audit_log_start(current-audit_context, GFP_ATOMIC, + AUDIT_MAC_IPSEC_EVENT); + if (audit_buf == NULL) + return NULL; + + audit_log_format(audit_buf, auid=%u, auid); + + if (sid != 0 + security_secid_to_secctx(sid, secctx, secctx_len) == 0) { + audit_log_format(audit_buf, subj=%s, secctx); + security_release_secctx(secctx, secctx_len); + } else + audit_log_task_context(audit_buf); + return audit_buf; +} + +extern void xfrm_audit_policy_add(struct xfrm_policy *xp, int result, + u32 auid, u32 sid); +extern void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result, + u32 auid, u32 sid); +extern void xfrm_audit_state_add(struct xfrm_state *x, int result, +u32 auid, u32 sid); +extern void xfrm_audit_state_delete(struct xfrm_state *x, int result, + u32 auid, u32 sid); #else -#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#define xfrm_audit_policy_add(x, r, a, s) do { ; } while (0) +#define xfrm_audit_policy_delete(x, r, a, s) do { ; } while (0) +#define xfrm_audit_state_add(x, r, a, s) do { ; } while (0) +#define xfrm_audit_state_delete(x, r, a, s)do { ; } while (0) #endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22-rc6/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-09-11 13:59:52.0 -0500 +++ linux-2.6.22-rc6/net/key/af_key.c 2007-09-11 14:10:58.0 -0500 @@ -27,7 +27,6 @@ #include linux/proc_fs.h #include linux/init.h #include net/xfrm.h -#include linux/audit.h #include net/sock.h @@ -1461,8 +1460,8 @@ static int pfkey_add(struct sock *sk, st else err = xfrm_state_update(x); - xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSA, err ? 0 : 1, NULL, x); + xfrm_audit_state_add(x, err ? 0 : 1, +audit_get_loginuid(current-audit_context), 0); if (err 0) { x-km.state = XFRM_STATE_DEAD; @@ -1515,8 +1514,8 @@ static int pfkey_delete
Re: [PATCH] improved xfrm_audit_log() patch
On Wed, 2007-08-22 at 20:05 -0700, David Miller wrote: I would suggest, at this point, to make purpose built situation specific interfaces that pass specific objects (the ones being operated upon) to the audit layer. Let the audit layer pick out the bits it actually wants in the format it likes. For example, if we're creating a template, pass the policy and the templace to the audit layer via a function called: xfrm_audit_template_add() or something like that. That function only needs two arguments. All of these call sites will rarely need more than 2 or 3 arguments in any given situation, and the on-stack audit thing will be gone too. This is the suggestion I made to you over a month ago, but you choose to do the on-stack thing. I misunderstood. My bad. For clarification, I plan on removing xfrm_audit_log() and replacing it with more specific ipsec audit interfaces. For example, when auditing the addition of a policy, either xfrm_user_audit_policy_add(xp, result, skb) or pfkey_audit_policy_add(xp, result) will get called. I need two because xfrm_user gets loginuid/secid from netlink/skb and pfkey gets it from audit_get_loginuid(). Each will setup and format audit buffer according to what they want. Also, for deleting, there will be pfkey_audit_policy_delete(xp, result) and xfrm_user_audit_policy_delete(xp, result, skb). You must make this cost absolutely nothing when it is either not configured, and have next to no cost when not enabled at run time. And it is very doable. The new ipsec audit functions can be ifdef'd with CONFIG_AUDITSYSCALL just as xfrm_audit_log() was so that there is no cost when audit is not configured. Let me know if this is better. Regards, Joy - 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] improved xfrm_audit_log() patch
On Wed, 2007-08-22 at 12:51 -0700, David Miller wrote: From: David Miller [EMAIL PROTECTED] Date: Tue, 21 Aug 2007 00:24:05 -0700 (PDT) Looks good, applied to net-2.6.24, thanks Joy. Something is still buggered up in this patch, you can't add this local audit_info variable unconditionally to these functions, and alternatively you also can't add a bunch of ifdefs to xfrm_user.c to cover it up either. I wonder if I am subconsciously trying to break a record or something! My apologies as time is valuable. I mean to get this right. My rationale for using audit_info was to reduce amount of arguments to xfrm_audit_log(). However, I now like it better when I just called xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid, ...). User determines where/how to get loginuid and secid and nothing happens when AUDIT not configured. But would make xfrm_audit_log() have 7 arguments instead of 6. My alternative is to remove xfrm_get_auditinfo() out of the #ifdef CONFIG_AUDITSYSCALL and always fill in audit_info regardless if AUDIT is configured or not. Less calls to xfrm_audit_log() but perhaps unnecessary info when AUDIT not configured. Would first solution be acceptable? Joy - 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] improved xfrm_audit_log() patch
On Tue, 2007-08-07 at 18:32 -0700, David Miller wrote: From: Joy Latten [EMAIL PROTECTED] Date: Thu, 2 Aug 2007 15:56:47 -0500 @@ -426,10 +426,15 @@ struct xfrm_audit }; #ifdef CONFIG_AUDITSYSCALL -extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, -struct xfrm_policy *xp, struct xfrm_state *x); +extern void xfrm_audit_log(struct xfrm_audit audit_info, int result, + __be32 flowid, struct xfrm_policy *xp, + struct xfrm_state *x, char *buf); Passing audit_info as an aggregate argument puts them into previous argument registers, or if they are not enough it goes either partially of wholly onto the stack, depending upon architecture. In fact you've made the argument register usage worse than in your previous revision. :-/ Perhaps you meant to pass struct xfrm_audit * instead? Revised patch to pass pointer to struct xfrm_audit. Sorry, I missed that. Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.22/include/linux/audit.h linux-2.6.22.patch/include/linux/audit.h --- linux-2.6.22/include/linux/audit.h 2007-08-14 18:13:53.0 -0500 +++ linux-2.6.22.patch/include/linux/audit.h2007-08-14 19:08:42.0 -0500 @@ -112,6 +112,7 @@ #define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ #define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ #define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_EVENT 1415/* Audit IPSec events */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 diff -urpN linux-2.6.22/include/net/xfrm.h linux-2.6.22.patch/include/net/xfrm.h --- linux-2.6.22/include/net/xfrm.h 2007-08-14 18:13:53.0 -0500 +++ linux-2.6.22.patch/include/net/xfrm.h 2007-08-14 19:08:42.0 -0500 @@ -426,10 +426,15 @@ struct xfrm_audit }; #ifdef CONFIG_AUDITSYSCALL -extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, - struct xfrm_policy *xp, struct xfrm_state *x); +extern void xfrm_audit_log(struct xfrm_audit *audit_info, int result, + __be32 flowid, struct xfrm_policy *xp, + struct xfrm_state *x, char *buf); + +extern void xfrm_get_auditinfo(struct sk_buff *skb, + struct xfrm_audit *audit_info); #else -#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#define xfrm_audit_log(a,r,f,p,s,b) do { ; } while (0) +#define xfrm_get_auditinfo(s, a) do { ; } while (0) #endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22.patch/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-08-14 18:13:53.0 -0500 +++ linux-2.6.22.patch/net/key/af_key.c 2007-08-14 19:08:42.0 -0500 @@ -1450,6 +1450,7 @@ static int pfkey_add(struct sock *sk, st struct xfrm_state *x; int err; struct km_event c; + struct xfrm_audit audit_info; x = pfkey_msg2xfrm_state(hdr, ext_hdrs); if (IS_ERR(x)) @@ -1461,8 +1462,8 @@ static int pfkey_add(struct sock *sk, st else err = xfrm_state_update(x); - xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSA, err ? 0 : 1, NULL, x); + xfrm_get_auditinfo(0, audit_info); + xfrm_audit_log(audit_info, err ? 0 : 1, 0, 0, x, SAD-add); if (err 0) { x-km.state = XFRM_STATE_DEAD; @@ -1487,6 +1488,7 @@ static int pfkey_delete(struct sock *sk, struct xfrm_state *x; struct km_event c; int err; + struct xfrm_audit audit_info; if (!ext_hdrs[SADB_EXT_SA-1] || !present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1], @@ -1515,8 +1517,9 @@ static int pfkey_delete(struct sock *sk, c.event = XFRM_MSG_DELSA; km_state_notify(x, c); out: - xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x); + xfrm_get_auditinfo(0, audit_info); + xfrm_audit_log(audit_info, err ? 0 : 1, 0, 0, x, SAD-delete); + xfrm_state_put(x); return err; @@ -1691,8 +1694,7 @@ static int pfkey_flush(struct sock *sk, if (proto == 0) return -EINVAL; - audit_info.loginuid = audit_get_loginuid(current-audit_context); - audit_info.secid = 0; + xfrm_get_auditinfo(0, audit_info); err = xfrm_state_flush(proto, audit_info); if (err) return err; @@ -2182,6 +2184,7 @@ static int pfkey_spdadd(struct sock *sk, struct xfrm_policy *xp; struct km_event c; struct sadb_x_sec_ctx *sec_ctx; + struct xfrm_audit audit_info; if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1], ext_hdrs
ipsec not working in 2.6.23-rc1-git10 when using pfkey
Although an ipsec SA was established, kernel couldn't seem to find it. I think since we are now using x-sel.family instead of family in the xfrm_selector_match() called in xfrm_state_find(), af_key needs to set this field too, just as xfrm_user. In af_key.c, x-sel.family only gets set when there's an ext_hdrs[SADB_EXT_ADDRESS_PROXY-1] which I think is for tunnel. I think pfkey needs to also set the x-sel.family field when it is 0. Tested with below patch, and ipsec worked when using pfkey. Let me know if this is correct approach or not. Regards, Joy diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22.fp/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-08-02 12:32:02.0 -0500 +++ linux-2.6.22.fp/net/key/af_key.c2007-08-02 12:40:57.0 -0500 @@ -1205,6 +1205,9 @@ static struct xfrm_state * pfkey_msg2xfr x-sel.family = pfkey_sadb_addr2xfrm_addr(addr, x-sel.saddr); x-sel.prefixlen_s = addr-sadb_address_prefixlen; } + + if (!x-sel.family) + x-sel.family = x-props.family; if (ext_hdrs[SADB_X_EXT_NAT_T_TYPE-1]) { struct sadb_x_nat_t_type* n_type; - 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] improved xfrm_audit_log() patch
Sorry for delay, here is xfrm_audit_log() modification with recommended changes. Let me know if this looks better. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.22/include/linux/audit.h linux-2.6.22.patch10/include/linux/audit.h --- linux-2.6.22/include/linux/audit.h 2007-08-01 11:49:23.0 -0500 +++ linux-2.6.22.patch10/include/linux/audit.h 2007-08-01 13:11:14.0 -0500 @@ -112,6 +112,7 @@ #define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ #define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ #define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_EVENT 1415/* Audit IPSec events */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 diff -urpN linux-2.6.22/include/net/xfrm.h linux-2.6.22.patch10/include/net/xfrm.h --- linux-2.6.22/include/net/xfrm.h 2007-08-01 11:49:24.0 -0500 +++ linux-2.6.22.patch10/include/net/xfrm.h 2007-08-01 13:11:14.0 -0500 @@ -426,10 +426,15 @@ struct xfrm_audit }; #ifdef CONFIG_AUDITSYSCALL -extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, - struct xfrm_policy *xp, struct xfrm_state *x); +extern void xfrm_audit_log(struct xfrm_audit audit_info, int result, + __be32 flowid, struct xfrm_policy *xp, + struct xfrm_state *x, char *buf); + +extern void xfrm_get_auditinfo(struct sk_buff *skb, + struct xfrm_audit *audit_info); #else -#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#define xfrm_audit_log(a,r,f,p,s,b) do { ; } while (0) +#define xfrm_get_auditinfo(s, a) do { ; } while (0) #endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) @@ -975,7 +980,7 @@ struct xfrmk_spdinfo { extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq); extern int xfrm_state_delete(struct xfrm_state *x); -extern int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); +extern int xfrm_state_flush(u8 proto, struct xfrm_audit audit_info); extern void xfrm_sad_getinfo(struct xfrmk_sadinfo *si); extern void xfrm_spd_getinfo(struct xfrmk_spdinfo *si); extern int xfrm_replay_check(struct xfrm_state *x, __be32 seq); @@ -1032,13 +1037,13 @@ struct xfrm_policy *xfrm_policy_bysel_ct struct xfrm_sec_ctx *ctx, int delete, int *err); struct xfrm_policy *xfrm_policy_byid(u8, int dir, u32 id, int delete, int *err); -int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +int xfrm_policy_flush(u8 type, struct xfrm_audit audit_info); u32 xfrm_get_acqseq(void); void xfrm_alloc_spi(struct xfrm_state *x, __be32 minspi, __be32 maxspi); struct xfrm_state * xfrm_find_acq(u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr, int create, unsigned short family); -extern int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +extern int xfrm_policy_flush(u8 type, struct xfrm_audit audit_info); extern int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol); extern int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *xdst, struct flowi *fl, int family, int strict); diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22.patch10/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-08-01 11:49:42.0 -0500 +++ linux-2.6.22.patch10/net/key/af_key.c 2007-08-01 13:14:01.0 -0500 @@ -1447,6 +1447,7 @@ static int pfkey_add(struct sock *sk, st struct xfrm_state *x; int err; struct km_event c; + struct xfrm_audit audit_info; x = pfkey_msg2xfrm_state(hdr, ext_hdrs); if (IS_ERR(x)) @@ -1458,8 +1459,8 @@ static int pfkey_add(struct sock *sk, st else err = xfrm_state_update(x); - xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSA, err ? 0 : 1, NULL, x); + xfrm_get_auditinfo(0, audit_info); + xfrm_audit_log(audit_info, err ? 0 : 1, 0, 0, x, SAD-add); if (err 0) { x-km.state = XFRM_STATE_DEAD; @@ -1484,6 +1485,7 @@ static int pfkey_delete(struct sock *sk, struct xfrm_state *x; struct km_event c; int err; + struct xfrm_audit audit_info; if (!ext_hdrs[SADB_EXT_SA-1] || !present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1], @@ -1512,8 +1514,9 @@ static int pfkey_delete(struct sock *sk, c.event = XFRM_MSG_DELSA; km_state_notify(x, c); out: - xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x); + xfrm_get_auditinfo(0, audit_info); + xfrm_audit_log(audit_info, err ? 0 : 1, 0
Re: [PATCH]: 2nd revision of make xfrm_audit_log more generic
On Wed, 2007-07-25 at 17:17 -0700, David Miller wrote: From: Joy Latten [EMAIL PROTECTED] Date: Wed, 25 Jul 2007 14:21:43 -0500 This is 2nd revision of patch to modify xfrm_audit_log() such that it can accomodate auditing other ipsec events besides add/delete of an SA or SPD entry. 2nd revision includes new define for all IPsec events in audit.h and introduces op= entry in logfile as well as add a hyphen in description for report parsing. This is a small change to accomodate updating ipsec protocol to RFCs 4301, 4302 and 4303 which require auditing some ipsec events if auditing is available. Please let me know if ok. Signed-off-by: Joy Latten [EMAIL PROTECTED] I like very much how the implementation of xfrm_audit_log() got simplified. But _TEN_ function call arguments, good grief! That's at least twice as many as most cpus can pass in registers. :-) :-) Sorry. Must have been in a fog or something now that I take a step back and look at it. Let's try an alternative where you have specialized xfrm_audit_log_foo() routines that take a user policy pointer, or whatever the main object is. If internally this just unpacks the needed bits and calls some do_xfrm_audit_log() thing inside of the auditing code that takes lots of arguments, that's fine, but let's not expand all of that argument setup code in the main IPSEC code paths. Fixing up right now. Will re-post when done. Thanks! Joy - 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]: 2nd revision of make xfrm_audit_log more generic
This is 2nd revision of patch to modify xfrm_audit_log() such that it can accomodate auditing other ipsec events besides add/delete of an SA or SPD entry. 2nd revision includes new define for all IPsec events in audit.h and introduces op= entry in logfile as well as add a hyphen in description for report parsing. This is a small change to accomodate updating ipsec protocol to RFCs 4301, 4302 and 4303 which require auditing some ipsec events if auditing is available. Please let me know if ok. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.22/include/linux/audit.h linux-2.6.22.patch/include/linux/audit.h --- linux-2.6.22/include/linux/audit.h 2007-07-23 14:35:28.0 -0500 +++ linux-2.6.22.patch/include/linux/audit.h2007-07-23 14:38:51.0 -0500 @@ -112,6 +112,7 @@ #define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ #define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ #define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_EVENT 1415/* Audit IPSec events */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 diff -urpN linux-2.6.22/include/net/xfrm.h linux-2.6.22.patch/include/net/xfrm.h --- linux-2.6.22/include/net/xfrm.h 2007-07-23 14:35:28.0 -0500 +++ linux-2.6.22.patch/include/net/xfrm.h 2007-07-23 14:38:51.0 -0500 @@ -427,9 +427,11 @@ struct xfrm_audit #ifdef CONFIG_AUDITSYSCALL extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, - struct xfrm_policy *xp, struct xfrm_state *x); + u16 family, xfrm_address_t saddr, + xfrm_address_t daddr, __be32 spi, __be32 flowid, + struct xfrm_sec_ctx *sctx, char *buf); #else -#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#define xfrm_audit_log(a,i,t,r,f,s,d,p,l,c,b) do { ; } while (0) #endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22.patch/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-07-08 18:32:17.0 -0500 +++ linux-2.6.22.patch/net/key/af_key.c 2007-07-24 11:50:35.0 -0500 @@ -1459,7 +1459,9 @@ static int pfkey_add(struct sock *sk, st err = xfrm_state_update(x); xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSA, err ? 0 : 1, NULL, x); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + x-props.family, x-props.saddr, x-id.daddr, + x-id.spi, 0, x-security, SAD-add); if (err 0) { x-km.state = XFRM_STATE_DEAD; @@ -1513,7 +1515,10 @@ static int pfkey_delete(struct sock *sk, km_state_notify(x, c); out: xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, x-props.family, + x-props.saddr, x-id.daddr, x-id.spi, 0, + x-security, SAD-delete); + xfrm_state_put(x); return err; @@ -2266,7 +2271,9 @@ static int pfkey_spdadd(struct sock *sk, hdr-sadb_msg_type != SADB_X_SPDUPDATE); xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, SPD-add); if (err) goto out; @@ -2350,7 +2357,9 @@ static int pfkey_spddelete(struct sock * return -ENOENT; xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, SPD-delete); if (err) goto out; @@ -2611,7 +2620,10 @@ static int pfkey_spdget(struct sock *sk, if (delete) { xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, + SPD-delete); if (err) goto out; diff -urpN linux-2.6.22/net/xfrm/xfrm_policy.c linux-2.6.22.patch/net/xfrm/xfrm_policy.c --- linux-2.6.22/net/xfrm/xfrm_policy.c 2007-07-23 14:35:29.0 -0500 +++ linux-2.6.22
Re: [PATCH]: revised make xfrm_audit_log more generic patch
On Tue, 2007-07-24 at 11:04 -0400, Steve Grubb wrote: + audit_log_format(audit_buf, %s: auid=%u, buf, auid); if (sid != 0 security_secid_to_secctx(sid, secctx, secctx_len) == 0) The operation in buf will not be parsed by the user space tools. Let's use op=%s where you have %s: above. Audit record fields are name=value and fields separated by spaces. op is what we are using in other places to mean operation. I know its a change from the records above, but we previously had some detail about what operation was being performed by the record type and this did not matter so much. Now that we only have one event type, the meaning of the event being recorded needs to be parsable and in a field. It also wouldn't hurt to change the text being sent to this function to have a hyphen instead of a space, so SPD delete becomes SPD-delete. This keeps the parser happy. This patch otherwise looks good. Sounds good. I will make the changes and resend. Thanks!! Joy - 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]: revised make xfrm_audit_log more generic patch
On Tue, 2007-07-24 at 11:04 -0400, Steve Grubb wrote: It also wouldn't hurt to change the text being sent to this function to have a hyphen instead of a space, so SPD delete becomes SPD-delete. This keeps the parser happy. Steve, more for my education, should all entries have this sort of syntax, that is, a hyphen in it? I imagine some entries might be a bit more wordy and so I was wondering ahead of time how to do it. Thanks!! Joy - 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] make xfrm_audit_log more generic
On Thu, 2007-07-19 at 21:45 -0400, James Morris wrote: On Thu, 19 Jul 2007, Joy Latten wrote: --- linux-2.6.22/include/linux/audit.h 2007-07-19 13:17:22.0 -0500 +++ linux-2.6.22.patch/include/linux/audit.h2007-07-19 13:21:29.0 -0500 @@ -108,10 +108,7 @@ #define AUDIT_MAC_CIPSOV4_DEL 1408/* NetLabel: del CIPSOv4 DOI entry */ #define AUDIT_MAC_MAP_ADD 1409/* NetLabel: add LSM domain mapping */ #define AUDIT_MAC_MAP_DEL 1410/* NetLabel: del LSM domain mapping */ -#define AUDIT_MAC_IPSEC_ADDSA 1411/* Add a XFRM state */ -#define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ -#define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ -#define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_EVENT 1411/* Audit IPSec events */ Will this cause existing applications to break? Perhaps someone in audit list could help answer this. During testing, because I changed the above defines, all IPSec events are listed as MAC_IPSEC_ADDSA for now without userspace change. Is this ok? Or is there a better way to migrate this change in? Perhaps leave previous IPsec defines and just add in a new one and use it? If that is better approach, let me know and I will change code to accomodate. Regards, Joy - 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]: revised make xfrm_audit_log more generic patch
Revised patch that modifies xfrm_audit_log() such that it can accomodate auditing other ipsec events besides add/delete of an SA or SPD entry. This patch differs from original in that it does not remove existing ipsec audit defines so as to not break existing audit apps. This is a small change to accomodate updating ipsec protocol to RFCs 4301, 4302 and 4303 which require auditing some ipsec events if auditing is available. Please let me know if ok. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.22/include/linux/audit.h linux-2.6.22.patch/include/linux/audit.h --- linux-2.6.22/include/linux/audit.h 2007-07-23 14:35:28.0 -0500 +++ linux-2.6.22.patch/include/linux/audit.h2007-07-23 14:38:51.0 -0500 @@ -112,6 +112,7 @@ #define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ #define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ #define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_EVENT 1415/* Audit IPSec events */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 diff -urpN linux-2.6.22/include/net/xfrm.h linux-2.6.22.patch/include/net/xfrm.h --- linux-2.6.22/include/net/xfrm.h 2007-07-23 14:35:28.0 -0500 +++ linux-2.6.22.patch/include/net/xfrm.h 2007-07-23 14:38:51.0 -0500 @@ -427,9 +427,11 @@ struct xfrm_audit #ifdef CONFIG_AUDITSYSCALL extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, - struct xfrm_policy *xp, struct xfrm_state *x); + u16 family, xfrm_address_t saddr, + xfrm_address_t daddr, __be32 spi, __be32 flowid, + struct xfrm_sec_ctx *sctx, char *buf); #else -#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#define xfrm_audit_log(a,i,t,r,f,s,d,p,l,c,b) do { ; } while (0) #endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22.patch/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-07-08 18:32:17.0 -0500 +++ linux-2.6.22.patch/net/key/af_key.c 2007-07-23 14:38:51.0 -0500 @@ -1459,7 +1459,9 @@ static int pfkey_add(struct sock *sk, st err = xfrm_state_update(x); xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSA, err ? 0 : 1, NULL, x); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + x-props.family, x-props.saddr, x-id.daddr, + x-id.spi, 0, x-security, SAD add); if (err 0) { x-km.state = XFRM_STATE_DEAD; @@ -1513,7 +1515,10 @@ static int pfkey_delete(struct sock *sk, km_state_notify(x, c); out: xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, x-props.family, + x-props.saddr, x-id.daddr, x-id.spi, 0, + x-security, SAD delete); + xfrm_state_put(x); return err; @@ -2266,7 +2271,9 @@ static int pfkey_spdadd(struct sock *sk, hdr-sadb_msg_type != SADB_X_SPDUPDATE); xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, SPD add); if (err) goto out; @@ -2350,7 +2357,9 @@ static int pfkey_spddelete(struct sock * return -ENOENT; xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, SPD delete); if (err) goto out; @@ -2611,7 +2620,10 @@ static int pfkey_spdget(struct sock *sk, if (delete) { xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, + SPD delete); if (err) goto out; diff -urpN linux-2.6.22/net/xfrm/xfrm_policy.c linux-2.6.22.patch/net/xfrm/xfrm_policy.c --- linux-2.6.22/net/xfrm/xfrm_policy.c 2007-07-23 14:35:29.0 -0500 +++ linux-2.6.22.patch/net/xfrm/xfrm_policy.c 2007-07-23
[PATCH] make xfrm_audit_log more generic
This patch modifies xfrm_audit_log() such that it can accomodate auditing other ipsec events besides add/delete of an SA or SPD entry. This is a small change to accomodate updating ipsec protocol to RFCs 4301, 4302 and 4303 which require auditing some ipsec events if auditing is available. Please let me know if ok. I tested with selinux/labeled-ipsec/plain-ipsec and plain ipsec without selinux. Also compiled and tested with auditing disabled. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.22/include/linux/audit.h linux-2.6.22.patch/include/linux/audit.h --- linux-2.6.22/include/linux/audit.h 2007-07-19 13:17:22.0 -0500 +++ linux-2.6.22.patch/include/linux/audit.h2007-07-19 13:21:29.0 -0500 @@ -108,10 +108,7 @@ #define AUDIT_MAC_CIPSOV4_DEL 1408/* NetLabel: del CIPSOv4 DOI entry */ #define AUDIT_MAC_MAP_ADD 1409/* NetLabel: add LSM domain mapping */ #define AUDIT_MAC_MAP_DEL 1410/* NetLabel: del LSM domain mapping */ -#define AUDIT_MAC_IPSEC_ADDSA 1411/* Add a XFRM state */ -#define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ -#define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ -#define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ +#define AUDIT_MAC_IPSEC_EVENT 1411/* Audit IPSec events */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 diff -urpN linux-2.6.22/include/net/xfrm.h linux-2.6.22.patch/include/net/xfrm.h --- linux-2.6.22/include/net/xfrm.h 2007-07-19 13:17:22.0 -0500 +++ linux-2.6.22.patch/include/net/xfrm.h 2007-07-19 13:21:29.0 -0500 @@ -427,9 +427,11 @@ struct xfrm_audit #ifdef CONFIG_AUDITSYSCALL extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, - struct xfrm_policy *xp, struct xfrm_state *x); + u16 family, xfrm_address_t saddr, + xfrm_address_t daddr, __be32 spi, __be32 flowid, + struct xfrm_sec_ctx *sctx, char *buf); #else -#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#define xfrm_audit_log(a,i,t,r,f,s,d,p,l,c,b) do { ; } while (0) #endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) diff -urpN linux-2.6.22/net/key/af_key.c linux-2.6.22.patch/net/key/af_key.c --- linux-2.6.22/net/key/af_key.c 2007-07-08 18:32:17.0 -0500 +++ linux-2.6.22.patch/net/key/af_key.c 2007-07-19 13:21:30.0 -0500 @@ -1459,7 +1459,9 @@ static int pfkey_add(struct sock *sk, st err = xfrm_state_update(x); xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSA, err ? 0 : 1, NULL, x); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + x-props.family, x-props.saddr, x-id.daddr, + x-id.spi, 0, x-security, SAD add); if (err 0) { x-km.state = XFRM_STATE_DEAD; @@ -1513,7 +1515,10 @@ static int pfkey_delete(struct sock *sk, km_state_notify(x, c); out: xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, x-props.family, + x-props.saddr, x-id.daddr, x-id.spi, 0, + x-security, SAD delete); + xfrm_state_put(x); return err; @@ -2266,7 +2271,9 @@ static int pfkey_spdadd(struct sock *sk, hdr-sadb_msg_type != SADB_X_SPDUPDATE); xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_ADDSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, SPD add); if (err) goto out; @@ -2350,7 +2357,9 @@ static int pfkey_spddelete(struct sock * return -ENOENT; xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security, SPD delete); if (err) goto out; @@ -2611,7 +2620,10 @@ static int pfkey_spdget(struct sock *sk, if (delete) { xfrm_audit_log(audit_get_loginuid(current-audit_context), 0, - AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); + AUDIT_MAC_IPSEC_EVENT, err ? 0 : 1, + xp-selector.family, xp-selector.saddr, + xp-selector.daddr, 0, 0, xp-security
Re: [PATCH]: Add security check before flushing SAD/SPD
On Mon, 2007-06-04 at 15:44 -0400, James Morris wrote: On Mon, 4 Jun 2007, Eric Paris wrote: Some time ago this thread bounced back and forth and seemed to come to rest with the patch below, I cleaned up the comments and put all the ACKs it received in one place below, but so much time has passed I doubt if they should still count for free. I also rediffed the patch against the latest miller tree. Is the idea or patch in any way flawed or unacceptable to people at the moment? Anyone willing to step up an re-ack the patch to get it moving into the tree? Looks good to me. Acked-by: James Morris [EMAIL PROTECTED] I have also tested with 2.6.22-rc3-git7 and all appears to be working as expected. Acked-by: Joy Latten [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
PATCH[1/1]: kernel panic when large security contexts in ACQUIRE
When sending a security context of 50+ characters in an ACQUIRE message, following kernel panic occurred. kernel BUG in xfrm_send_acquire at net/xfrm/xfrm_user.c:1781! cpu 0x3: Vector: 700 (Program Check) at [c000421bb2e0] pc: c033b074: .xfrm_send_acquire+0x240/0x2c8 lr: c033b014: .xfrm_send_acquire+0x1e0/0x2c8 sp: c000421bb560 msr: 80029032 current = 0xcfce8f00 paca= 0xc0464b00 pid = 2303, comm = ping kernel BUG in xfrm_send_acquire at net/xfrm/xfrm_user.c:1781! enter ? for help 3:mon t [c000421bb650] c033538c .km_query+0x6c/0xec [c000421bb6f0] c0337374 .xfrm_state_find+0x7f4/0xb88 [c000421bb7f0] c0332350 .xfrm_tmpl_resolve+0xc4/0x21c [c000421bb8d0] c03326e8 .xfrm_lookup+0x1a0/0x5b0 [c000421bba00] c02e6ea0 .ip_route_output_flow+0x88/0xb4 [c000421bbaa0] c03106d8 .ip4_datagram_connect+0x218/0x374 [c000421bbbd0] c031bc00 .inet_dgram_connect+0xac/0xd4 [c000421bbc60] c02b11ac .sys_connect+0xd8/0x120 [c000421bbd90] c02d38d0 .compat_sys_socketcall+0xdc/0x214 [c000421bbe30] c000869c syscall_exit+0x0/0x40 --- Exception: c00 (System Call) at 07f0ca9c SP (fc0ef8f0) is in userspace We are using size of security context from xfrm_policy to determine how much space to alloc skb and then putting security context from xfrm_state into skb. Should have been using size of security context from xfrm_state to alloc skb. Following fix does that Please let me know if this is acceptable. Patch was built and tested against 2.6.21-rc6-git5. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.20/net/xfrm/xfrm_user.c linux-2.6.20.patch/net/xfrm/xfrm_user.c --- linux-2.6.20/net/xfrm/xfrm_user.c 2007-04-12 15:12:27.0 -0500 +++ linux-2.6.20.patch/net/xfrm/xfrm_user.c 2007-04-13 09:32:35.0 -0500 @@ -272,9 +272,8 @@ static int attach_encap_tmpl(struct xfrm } -static inline int xfrm_user_sec_ctx_size(struct xfrm_policy *xp) +static inline int xfrm_user_sec_ctx_size(struct xfrm_sec_ctx *xfrm_ctx) { - struct xfrm_sec_ctx *xfrm_ctx = xp-security; int len = 0; if (xfrm_ctx) { @@ -2170,7 +2169,7 @@ static int xfrm_send_acquire(struct xfrm len = RTA_SPACE(sizeof(struct xfrm_user_tmpl) * xp-xfrm_nr); len += NLMSG_SPACE(sizeof(struct xfrm_user_acquire)); - len += RTA_SPACE(xfrm_user_sec_ctx_size(xp)); + len += RTA_SPACE(xfrm_user_sec_ctx_size(x-security)); #ifdef CONFIG_XFRM_SUB_POLICY len += RTA_SPACE(sizeof(struct xfrm_userpolicy_type)); #endif @@ -2280,7 +2279,7 @@ static int xfrm_exp_policy_notify(struct len = RTA_SPACE(sizeof(struct xfrm_user_tmpl) * xp-xfrm_nr); len += NLMSG_SPACE(sizeof(struct xfrm_user_polexpire)); - len += RTA_SPACE(xfrm_user_sec_ctx_size(xp)); + len += RTA_SPACE(xfrm_user_sec_ctx_size(xp-security)); #ifdef CONFIG_XFRM_SUB_POLICY len += RTA_SPACE(sizeof(struct xfrm_userpolicy_type)); #endif - 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]: SAD sometimes has double SAs.
Last night I browsed the ipsec-tools code and saw places in racoon where improvement would actually fix this problem. I am working on a patch and will pursue this on the ipsec-tools mailing list. I apologize for any inconvenience. Eric, sorry as I know you already patched lspp kernel for testing. I strongly think this should be fixed in userspace. The permission check before flushing does still need to be added to kernel. Regards, Joy On Mon, 2007-03-26 at 19:04 -0600, Joy Latten wrote: On Mon, 2007-03-26 at 14:48 -0700, David Miller wrote: From: Eric Paris [EMAIL PROTECTED] Date: Mon, 26 Mar 2007 17:34:59 -0400 I'm not at all able to speak on the correctness or validity of the solution, Neither am I yet :) but shouldn't the ipv6 case be a not an || like the ipv4 case? Isn't this going to match all sorts of things? Did you test this patch on ipv6 and see it to solve your problem? I'm also not enjoying the formatting in the ipv6 part where the first time you have the cast on the same time as the object but not the second part where x-props.saddr.a6 is on its own little line. Also, I want to understand what is going to tear down these other direction fake entries later on? I think I can review this patch better if I understand that. I am going to refer to the other-direction-placeholder as the fake entry. And the larval SA that gets created for the new spi as result of a GETSPI message as the real entry. The fake entry gets created when the real entry does and does not have an spi. It shares some of the same properties of a real larval SA (they are created using same code) and it's state is marked as XFRM_STATE_ACQ. The real entry has a timeout. So, should IKE negotiation fail, take too long, etc... it will eventually timeout and be deleted. So does the fake entry. It will timeout and should be eventually deleted. (I will test this part tomorrow for assurance.) When the IKE negotiations are successful, xfrm_state_add() and the xfrm_state_update() look for larval SAs in that they look for an SA with same src, dst, etc... and with state==XFRM_STATE_ACQUIRE. Any that are found are deleted and new SA added. This removes the real larval SA, and should also remove the fake entry too. Of course, this is all based on my assumption that IKE will install two SAs, one for incoming and one for outgoing. Hopefully this answers how fake entries will be removed. Admittedly, I may miss something or didn't understand something correctly as I learn the code, so please let me know. There may even be a better solution that I don't readily see. As it stands, this looks to me like a workaround for an improperly implemented IPSEC daemon. Joy states it as saying that the current code requires the keying daemon to manage it's SAs, and I wonder whether any other implementation is even valid. My big mouth. :-) But yes, I do think more SA management in userspace would be ideal. This fix will hopefully ensure kernel doesn't send any extra acquires regardless. Joy - 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 - 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]: Add security check before flushing SAD/SPD
On Thu, 2007-03-22 at 20:56 -0400, James Morris wrote: On Thu, 22 Mar 2007, Joy Latten wrote: Perhaps a better semantic would be to fail the entire flush operation if one of the security checks failed. e.g. loop through for permissions first, then if all ok, loop through for deletion. Ok, will code this up and test it if there are no objections. I'd suggest making the permission loop a noop if CONFIG_SECURITY=n, via a static inline function. Reworked patch with improved semantics as suggested. I used CONFIG_SECURITY_NETWORK_XFRM instead of CONFIG_SECURITY since this can be considered part of the labeled networking semantics. Let me know if my assumption sounds incorrect. Also, I have built and tested with and without CONFIG_SECURITY_NETWORK_XFRM. Regards, Joy Signed-off-by: Joy Latten[EMAIL PROTECTED] diff -urpN linux-2.6.20.orig/include/net/xfrm.h linux-2.6.20.patch/include/net/xfrm.h --- linux-2.6.20.orig/include/net/xfrm.h2007-03-23 11:01:48.0 -0500 +++ linux-2.6.20.patch/include/net/xfrm.h 2007-03-25 21:36:05.0 -0500 @@ -937,7 +937,7 @@ static inline int xfrm_state_sort(struct #endif extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq); extern int xfrm_state_delete(struct xfrm_state *x); -extern void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); +extern int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); extern int xfrm_replay_check(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_advance(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_notify(struct xfrm_state *x, int event); @@ -991,13 +991,13 @@ struct xfrm_policy *xfrm_policy_bysel_ct struct xfrm_sec_ctx *ctx, int delete, int *err); struct xfrm_policy *xfrm_policy_byid(u8, int dir, u32 id, int delete, int *err); -void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); u32 xfrm_get_acqseq(void); void xfrm_alloc_spi(struct xfrm_state *x, __be32 minspi, __be32 maxspi); struct xfrm_state * xfrm_find_acq(u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr, int create, unsigned short family); -extern void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +extern int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); extern int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol); extern int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *xdst, struct flowi *fl, int family, int strict); diff -urpN linux-2.6.20.orig/net/key/af_key.c linux-2.6.20.patch/net/key/af_key.c --- linux-2.6.20.orig/net/key/af_key.c 2007-03-23 11:02:49.0 -0500 +++ linux-2.6.20.patch/net/key/af_key.c 2007-03-25 21:34:35.0 -0500 @@ -1645,6 +1645,7 @@ static int pfkey_flush(struct sock *sk, unsigned proto; struct km_event c; struct xfrm_audit audit_info; + int err; proto = pfkey_satype2proto(hdr-sadb_msg_satype); if (proto == 0) @@ -1652,7 +1653,9 @@ static int pfkey_flush(struct sock *sk, audit_info.loginuid = audit_get_loginuid(current-audit_context); audit_info.secid = 0; - xfrm_state_flush(proto, audit_info); + err = xfrm_state_flush(proto, audit_info); + if (err) + return err; c.data.proto = proto; c.seq = hdr-sadb_msg_seq; c.pid = hdr-sadb_msg_pid; @@ -2628,10 +2631,13 @@ static int pfkey_spdflush(struct sock *s { struct km_event c; struct xfrm_audit audit_info; + int err; audit_info.loginuid = audit_get_loginuid(current-audit_context); audit_info.secid = 0; - xfrm_policy_flush(XFRM_POLICY_TYPE_MAIN, audit_info); + err = xfrm_policy_flush(XFRM_POLICY_TYPE_MAIN, audit_info); + if (err) + return err; c.data.type = XFRM_POLICY_TYPE_MAIN; c.event = XFRM_MSG_FLUSHPOLICY; c.pid = hdr-sadb_msg_pid; diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_policy.c linux-2.6.20.patch/net/xfrm/xfrm_policy.c --- linux-2.6.20.orig/net/xfrm/xfrm_policy.c2007-03-23 11:02:46.0 -0500 +++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-25 21:32:51.0 -0500 @@ -813,11 +813,65 @@ struct xfrm_policy *xfrm_policy_byid(u8 } EXPORT_SYMBOL(xfrm_policy_byid); -void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) +#ifdef CONFIG_SECURITY_NETWORK_XFRM +static inline int +xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info) { - int dir; + int dir, err = 0; + + for (dir = 0; dir XFRM_POLICY_MAX; dir++) { + struct xfrm_policy *pol; + struct hlist_node *entry; + int i; + + hlist_for_each_entry(pol, entry
Re: [PATCH]: Add security check before flushing SAD/SPD
I have made improvements based on James' and Eric's comments. Regards, Joy Signed-off-by: Joy Latten[EMAIL PROTECTED] diff -urpN linux-2.6.20.orig/include/net/xfrm.h linux-2.6.20.patch/include/net/xfrm.h --- linux-2.6.20.orig/include/net/xfrm.h2007-03-23 11:01:48.0 -0500 +++ linux-2.6.20.patch/include/net/xfrm.h 2007-03-25 21:36:05.0 -0500 @@ -937,7 +937,7 @@ static inline int xfrm_state_sort(struct #endif extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq); extern int xfrm_state_delete(struct xfrm_state *x); -extern void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); +extern int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); extern int xfrm_replay_check(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_advance(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_notify(struct xfrm_state *x, int event); @@ -991,13 +991,13 @@ struct xfrm_policy *xfrm_policy_bysel_ct struct xfrm_sec_ctx *ctx, int delete, int *err); struct xfrm_policy *xfrm_policy_byid(u8, int dir, u32 id, int delete, int *err); -void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); u32 xfrm_get_acqseq(void); void xfrm_alloc_spi(struct xfrm_state *x, __be32 minspi, __be32 maxspi); struct xfrm_state * xfrm_find_acq(u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr, int create, unsigned short family); -extern void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +extern int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); extern int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol); extern int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *xdst, struct flowi *fl, int family, int strict); diff -urpN linux-2.6.20.orig/net/key/af_key.c linux-2.6.20.patch/net/key/af_key.c --- linux-2.6.20.orig/net/key/af_key.c 2007-03-23 11:02:49.0 -0500 +++ linux-2.6.20.patch/net/key/af_key.c 2007-03-25 21:34:35.0 -0500 @@ -1645,6 +1645,7 @@ static int pfkey_flush(struct sock *sk, unsigned proto; struct km_event c; struct xfrm_audit audit_info; + int err; proto = pfkey_satype2proto(hdr-sadb_msg_satype); if (proto == 0) @@ -1652,7 +1653,9 @@ static int pfkey_flush(struct sock *sk, audit_info.loginuid = audit_get_loginuid(current-audit_context); audit_info.secid = 0; - xfrm_state_flush(proto, audit_info); + err = xfrm_state_flush(proto, audit_info); + if (err) + return err; c.data.proto = proto; c.seq = hdr-sadb_msg_seq; c.pid = hdr-sadb_msg_pid; @@ -2628,10 +2631,13 @@ static int pfkey_spdflush(struct sock *s { struct km_event c; struct xfrm_audit audit_info; + int err; audit_info.loginuid = audit_get_loginuid(current-audit_context); audit_info.secid = 0; - xfrm_policy_flush(XFRM_POLICY_TYPE_MAIN, audit_info); + err = xfrm_policy_flush(XFRM_POLICY_TYPE_MAIN, audit_info); + if (err) + return err; c.data.type = XFRM_POLICY_TYPE_MAIN; c.event = XFRM_MSG_FLUSHPOLICY; c.pid = hdr-sadb_msg_pid; diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_policy.c linux-2.6.20.patch/net/xfrm/xfrm_policy.c --- linux-2.6.20.orig/net/xfrm/xfrm_policy.c2007-03-23 11:02:46.0 -0500 +++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-26 17:19:26.0 -0500 @@ -813,11 +813,67 @@ struct xfrm_policy *xfrm_policy_byid(u8 } EXPORT_SYMBOL(xfrm_policy_byid); -void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) +#ifdef CONFIG_SECURITY_NETWORK_XFRM +static inline int +xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info) { - int dir; + int dir, err = 0; + + for (dir = 0; dir XFRM_POLICY_MAX; dir++) { + struct xfrm_policy *pol; + struct hlist_node *entry; + int i; + + hlist_for_each_entry(pol, entry, +xfrm_policy_inexact[dir], bydst) { + if (pol-type != type) + continue; + err = security_xfrm_policy_delete(pol); + if (err) { + xfrm_audit_log(audit_info-loginuid, + audit_info-secid, + AUDIT_MAC_IPSEC_DELSPD, 0, + pol, NULL); + return err; + } +} + for (i = xfrm_policy_bydst[dir].hmask; i = 0; i--) { + hlist_for_each_entry(pol, entry
Re: [PATCH]: Add security check before flushing SAD/SPD
Sending again since one of the email addresses was incorrect. Ok, I have made improvements based on James' and Eric's comments. Regards, Joy Signed-off-by: Joy Latten[EMAIL PROTECTED] diff -urpN linux-2.6.20.orig/include/net/xfrm.h linux-2.6.20.patch/include/net/xfrm.h --- linux-2.6.20.orig/include/net/xfrm.h2007-03-23 11:01:48.0 -0500 +++ linux-2.6.20.patch/include/net/xfrm.h 2007-03-25 21:36:05.0 -0500 @@ -937,7 +937,7 @@ static inline int xfrm_state_sort(struct #endif extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq); extern int xfrm_state_delete(struct xfrm_state *x); -extern void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); +extern int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); extern int xfrm_replay_check(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_advance(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_notify(struct xfrm_state *x, int event); @@ -991,13 +991,13 @@ struct xfrm_policy *xfrm_policy_bysel_ct struct xfrm_sec_ctx *ctx, int delete, int *err); struct xfrm_policy *xfrm_policy_byid(u8, int dir, u32 id, int delete, int *err); -void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); u32 xfrm_get_acqseq(void); void xfrm_alloc_spi(struct xfrm_state *x, __be32 minspi, __be32 maxspi); struct xfrm_state * xfrm_find_acq(u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr, int create, unsigned short family); -extern void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); +extern int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); extern int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol); extern int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *xdst, struct flowi *fl, int family, int strict); diff -urpN linux-2.6.20.orig/net/key/af_key.c linux-2.6.20.patch/net/key/af_key.c --- linux-2.6.20.orig/net/key/af_key.c 2007-03-23 11:02:49.0 -0500 +++ linux-2.6.20.patch/net/key/af_key.c 2007-03-25 21:34:35.0 -0500 @@ -1645,6 +1645,7 @@ static int pfkey_flush(struct sock *sk, unsigned proto; struct km_event c; struct xfrm_audit audit_info; + int err; proto = pfkey_satype2proto(hdr-sadb_msg_satype); if (proto == 0) @@ -1652,7 +1653,9 @@ static int pfkey_flush(struct sock *sk, audit_info.loginuid = audit_get_loginuid(current-audit_context); audit_info.secid = 0; - xfrm_state_flush(proto, audit_info); + err = xfrm_state_flush(proto, audit_info); + if (err) + return err; c.data.proto = proto; c.seq = hdr-sadb_msg_seq; c.pid = hdr-sadb_msg_pid; @@ -2628,10 +2631,13 @@ static int pfkey_spdflush(struct sock *s { struct km_event c; struct xfrm_audit audit_info; + int err; audit_info.loginuid = audit_get_loginuid(current-audit_context); audit_info.secid = 0; - xfrm_policy_flush(XFRM_POLICY_TYPE_MAIN, audit_info); + err = xfrm_policy_flush(XFRM_POLICY_TYPE_MAIN, audit_info); + if (err) + return err; c.data.type = XFRM_POLICY_TYPE_MAIN; c.event = XFRM_MSG_FLUSHPOLICY; c.pid = hdr-sadb_msg_pid; diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_policy.c linux-2.6.20.patch/net/xfrm/xfrm_policy.c --- linux-2.6.20.orig/net/xfrm/xfrm_policy.c2007-03-23 11:02:46.0 -0500 +++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-26 17:19:26.0 -0500 @@ -813,11 +813,67 @@ struct xfrm_policy *xfrm_policy_byid(u8 } EXPORT_SYMBOL(xfrm_policy_byid); -void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) +#ifdef CONFIG_SECURITY_NETWORK_XFRM +static inline int +xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info) { - int dir; + int dir, err = 0; + + for (dir = 0; dir XFRM_POLICY_MAX; dir++) { + struct xfrm_policy *pol; + struct hlist_node *entry; + int i; + + hlist_for_each_entry(pol, entry, +xfrm_policy_inexact[dir], bydst) { + if (pol-type != type) + continue; + err = security_xfrm_policy_delete(pol); + if (err) { + xfrm_audit_log(audit_info-loginuid, + audit_info-secid, + AUDIT_MAC_IPSEC_DELSPD, 0, + pol, NULL); + return err; + } +} + for (i
Re: [PATCH]: SAD sometimes has double SAs.
On Mon, 2007-03-26 at 17:34 -0400, Eric Paris wrote: On Fri, 2007-03-23 at 16:58 -0600, Joy Latten wrote: @@ -710,11 +713,20 @@ static struct xfrm_state *__find_acq_cor switch (family) { case AF_INET: + if (x-id.daddr.a4 == saddr-a4 + x-props.saddr.a4 == daddr-a4) + track_opposite = 1; if (x-id.daddr.a4!= daddr-a4 || x-props.saddr.a4 != saddr-a4) continue; break; case AF_INET6: + if (ipv6_addr_equal((struct in6_addr *)x-id.daddr.a6, +(struct in6_addr *)saddr) || + ipv6_addr_equal((struct in6_addr *) +x-props.saddr.a6, +(struct in6_addr *)daddr)) + track_opposite = 1; if (!ipv6_addr_equal((struct in6_addr *)x-id.daddr.a6, (struct in6_addr *)daddr) || !ipv6_addr_equal((struct in6_addr *) I'm not at all able to speak on the correctness or validity of the solution, but shouldn't the ipv6 case be a not an || like the ipv4 case? Isn't this going to match all sorts of things? Did you test this patch on ipv6 and see it to solve your problem? Will fix this and resend. Sorry, forgot about ipv6. My mistake! :-( I'm also not enjoying the formatting in the ipv6 part where the first time you have the cast on the same time as the object but not the second part where x-props.saddr.a6 is on its own little line. ok. Joy - 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]: SAD sometimes has double SAs.
On Mon, 2007-03-26 at 14:48 -0700, David Miller wrote: From: Eric Paris [EMAIL PROTECTED] Date: Mon, 26 Mar 2007 17:34:59 -0400 I'm not at all able to speak on the correctness or validity of the solution, Neither am I yet :) but shouldn't the ipv6 case be a not an || like the ipv4 case? Isn't this going to match all sorts of things? Did you test this patch on ipv6 and see it to solve your problem? I'm also not enjoying the formatting in the ipv6 part where the first time you have the cast on the same time as the object but not the second part where x-props.saddr.a6 is on its own little line. Also, I want to understand what is going to tear down these other direction fake entries later on? I think I can review this patch better if I understand that. I am going to refer to the other-direction-placeholder as the fake entry. And the larval SA that gets created for the new spi as result of a GETSPI message as the real entry. The fake entry gets created when the real entry does and does not have an spi. It shares some of the same properties of a real larval SA (they are created using same code) and it's state is marked as XFRM_STATE_ACQ. The real entry has a timeout. So, should IKE negotiation fail, take too long, etc... it will eventually timeout and be deleted. So does the fake entry. It will timeout and should be eventually deleted. (I will test this part tomorrow for assurance.) When the IKE negotiations are successful, xfrm_state_add() and the xfrm_state_update() look for larval SAs in that they look for an SA with same src, dst, etc... and with state==XFRM_STATE_ACQUIRE. Any that are found are deleted and new SA added. This removes the real larval SA, and should also remove the fake entry too. Of course, this is all based on my assumption that IKE will install two SAs, one for incoming and one for outgoing. Hopefully this answers how fake entries will be removed. Admittedly, I may miss something or didn't understand something correctly as I learn the code, so please let me know. There may even be a better solution that I don't readily see. As it stands, this looks to me like a workaround for an improperly implemented IPSEC daemon. Joy states it as saying that the current code requires the keying daemon to manage it's SAs, and I wonder whether any other implementation is even valid. My big mouth. :-) But yes, I do think more SA management in userspace would be ideal. This fix will hopefully ensure kernel doesn't send any extra acquires regardless. Joy - 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]: Add security check before flushing SAD/SPD
On Fri, 2007-03-23 at 01:39 -0400, Eric Paris wrote: In either case though proper auditing needs to be addressed. I see that the first patch from Joy wouldn't audit deletion failures. It appears to me if the check is done per policy then the security hook return code needs to be recorded and passed to xfrm_audit_log instead of the hard coded 1 result used now. Assuming we go with James's double loop what should we be auditing for a security hook denial? Just audit the first policy entry which we tried to remove but couldn't and then leave the rest of the auditing in those functions the way it is now in case there was no denial, calling xfrm_audit_log with a hard coded 1 for the result? Actually, I thought the original intent of the ipsec auditing was to just audit changes made to the SAD/SPD databases, not securiy hook denials, right? Joy - 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]: Add security check before flushing SAD/SPD
On Fri, 2007-03-23 at 12:59 -0400, Eric Paris wrote: On Fri, 2007-03-23 at 10:33 -0600, Joy Latten wrote: On Fri, 2007-03-23 at 01:39 -0400, Eric Paris wrote: In either case though proper auditing needs to be addressed. I see that the first patch from Joy wouldn't audit deletion failures. It appears to me if the check is done per policy then the security hook return code needs to be recorded and passed to xfrm_audit_log instead of the hard coded 1 result used now. Assuming we go with James's double loop what should we be auditing for a security hook denial? Just audit the first policy entry which we tried to remove but couldn't and then leave the rest of the auditing in those functions the way it is now in case there was no denial, calling xfrm_audit_log with a hard coded 1 for the result? Actually, I thought the original intent of the ipsec auditing was to just audit changes made to the SAD/SPD databases, not securiy hook denials, right? Then what is the point of the 'result' field that we capture and log in xfrm_audit_log if the only things you care to audit are successful changes to the databases? Yes, I think we do want to audit the security denial since it is the reason we could not change the policy. In the flush case it seem it will be the only reason. As you suggested, I will audit the first denial since this is the reason the flush will fail. But sometimes, in other cases, the delete or add could fail for other reasons too such as not being able to allocate memory, not finding the entry, etc... which is passed in the result field. Regards, Joy - 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]: SAD sometimes has double SAs.
Last Friday I proposed creating larval SAs to act as placeholders to prevent a second acquire resulting in double SAs being created. I tried this and so far I have not seen any double SAs being created. I also plan to run some stress tests over the weekend. Please let me know what improvements I can make to this patch or if there is a better way to do this. A while back I reported that I sometimes saw double and triple SAs being created. The patch to check for protocol when deleting larval SA removed one obstacle in that I no longer see triple SAs. Now, once in a while double SAs. I think I have figured out the second obstacle. The initiator installs his SAs into the kernel before the responder. As soon as they are installed, the blocked packet (which started the ACQUIRE) is sent. By this time the responder has installed his inbound SA(s) and so the newly arrived ipsec packet can be processed. In the case of tcp connections and a ping, a response may be warranted, and thus an outgoing packet results. From what I can tell of the log file below, sometimes, this might happen before the responder has completed installing the outbound SAs. In the log file, the outbound AH has been added, but not the outbound ESP, which is the one the outgoing packet looks for first. Thus resulting in a second acquire. I think this becomes more problematic when using both AH AND ESP, rather than just using ESP with authentication. With the latter, only one SA needed thus reducing the latency in installing the SAs before incoming packet arrives. So far, the only solution I can think of besides mandating all userspace key daemons do SA maintenance is to perhaps add larval SAs for both directions when adding the SPI. Currently, responder does GETSPI for one way and initiator for opposite. When GETSPI is called, larval SA is created containing the SPI, but it is only for one direction. Perhaps we can add a larval SA (no SPI) for opposite direction to act as a placeholder indicating ACQUIRE occurring, since SAs are created for both directions during an ACQUIRE. The initiator may have larval SA from GETSPI and larval SA from the ACQUIRE depending that GETSPI is in opposite direction of ACQUIRE. Calling __find_acq_core() should ensure we don't create duplicate larval SAs. Also, should IKE negotiations return error, larval SAs should expire. They also should be removed when we do the xfrm_state_add() and xfrm_state_update() to add the new SAs. Joy This patch is against linux-2.6.21-rc4-git5 Signed-off-by: Joy Latten[EMAIL PROTECTED] diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_state.c linux-2.6.20/net/xfrm/xfrm_state.c --- linux-2.6.20.orig/net/xfrm/xfrm_state.c 2007-03-20 22:39:15.0 -0500 +++ linux-2.6.20/net/xfrm/xfrm_state.c 2007-03-23 16:38:37.0 -0500 @@ -692,12 +692,15 @@ void xfrm_state_insert(struct xfrm_state } EXPORT_SYMBOL(xfrm_state_insert); +static struct xfrm_state *create_larval_sa(unsigned short family, u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr); + /* xfrm_state_lock is held */ static struct xfrm_state *__find_acq_core(unsigned short family, u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr, int create) { unsigned int h = xfrm_dst_hash(daddr, saddr, reqid, family); struct hlist_node *entry; - struct xfrm_state *x; + struct xfrm_state *x, *x1; + int track_opposite = 0; hlist_for_each_entry(x, entry, xfrm_state_bydst+h, bydst) { if (x-props.reqid != reqid || @@ -710,11 +713,20 @@ static struct xfrm_state *__find_acq_cor switch (family) { case AF_INET: + if (x-id.daddr.a4 == saddr-a4 + x-props.saddr.a4 == daddr-a4) + track_opposite = 1; if (x-id.daddr.a4!= daddr-a4 || x-props.saddr.a4 != saddr-a4) continue; break; case AF_INET6: + if (ipv6_addr_equal((struct in6_addr *)x-id.daddr.a6, +(struct in6_addr *)saddr) || + ipv6_addr_equal((struct in6_addr *) +x-props.saddr.a6, +(struct in6_addr *)daddr)) + track_opposite = 1; if (!ipv6_addr_equal((struct in6_addr *)x-id.daddr.a6, (struct in6_addr *)daddr) || !ipv6_addr_equal((struct in6_addr *) @@ -731,6 +743,27 @@ static struct xfrm_state *__find_acq_cor if (!create) return NULL; + x = create_larval_sa(family, mode, reqid, proto, daddr, saddr); + + /* create a larval
[PATCH]: Add security check before flushing SAD/SPD
Within selinux we check for authorization before deleting entries from SAD and SPD. We are not checking for authorization when flushing the SPD and the SAD. It was perhaps missed in original patch. This patch adds security check when flushing entries from SAD and SPD. Please let me know if this patch is ok. It was built against linux-2.6.21-rc4-git5. I have also tested it. Joy Signed-off-by: Joy Latten[EMAIL PROTECTED] diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_policy.c linux-2.6.20/net/xfrm/xfrm_policy.c --- linux-2.6.20.orig/net/xfrm/xfrm_policy.c2007-03-21 14:25:51.0 -0500 +++ linux-2.6.20/net/xfrm/xfrm_policy.c 2007-03-21 14:30:59.0 -0500 @@ -829,6 +829,8 @@ void xfrm_policy_flush(u8 type, struct x xfrm_policy_inexact[dir], bydst) { if (pol-type != type) continue; + if (security_xfrm_policy_delete(pol)) + continue; hlist_del(pol-bydst); hlist_del(pol-byidx); write_unlock_bh(xfrm_policy_lock); @@ -850,6 +852,8 @@ void xfrm_policy_flush(u8 type, struct x bydst) { if (pol-type != type) continue; + if (security_xfrm_policy_delete(pol)) + continue; hlist_del(pol-bydst); hlist_del(pol-byidx); write_unlock_bh(xfrm_policy_lock); diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_state.c linux-2.6.20/net/xfrm/xfrm_state.c --- linux-2.6.20.orig/net/xfrm/xfrm_state.c 2007-03-21 14:25:51.0 -0500 +++ linux-2.6.20/net/xfrm/xfrm_state.c 2007-03-21 14:27:48.0 -0500 @@ -400,7 +400,8 @@ void xfrm_state_flush(u8 proto, struct x restart: hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) { if (!xfrm_state_kern(x) - xfrm_id_proto_match(x-id.proto, proto)) { + xfrm_id_proto_match(x-id.proto, proto) + !security_xfrm_state_delete(x)) { xfrm_state_hold(x); spin_unlock_bh(xfrm_state_lock); - 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]: Add security check before flushing SAD/SPD
On Thu, 2007-03-22 at 12:01 -0700, David Miller wrote: From: Joy Latten [EMAIL PROTECTED] Date: Thu, 22 Mar 2007 12:35:39 -0600 Within selinux we check for authorization before deleting entries from SAD and SPD. We are not checking for authorization when flushing the SPD and the SAD. It was perhaps missed in original patch. This patch adds security check when flushing entries from SAD and SPD. Please let me know if this patch is ok. It was built against linux-2.6.21-rc4-git5. I have also tested it. Signed-off-by: Joy Latten[EMAIL PROTECTED] I don't understand this and it does not sit well with me. If we are flushing the policy database, we are flushing it regardless of what the security layer might or might not say. I would look at this patch differently if there were some security level key being checked for a match here, which is an input key to the flush, but that is not what is happening here as the object is being looked at by itself. Yes, I understand what you are saying. I was concerned about having to check each entry to flush database. I did this patch because we check for authorization when deleting single specified entries from the SAD/SPD. It seem like a hole to me that we check for this, but that same user/process can delete the entire database with no checks. Unfortunately, each policy entry or SA can have a different security label. And that is why I would have to check each entry's security label before deleting. To see if the user/process has authorization to delete an entry with that security label. Including selinux list for suggestions. Joy - 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
Possible solution to those doubly created SAs in ipsec
A while back I reported that I sometimes saw double and triple SAs being created. The patch to check for protocol when deleting larval SA removed one obstacle in that I no longer see triple SAs. Now, once in a while double SAs. I think I have figured out the second obstacle. The initiator installs his SAs into the kernel before the responder. As soon as they are installed, the blocked packet (which started the ACQUIRE) is sent. By this time the responder has installed his inbound SA(s) and so the newly arrived ipsec packet can be processed. In the case of tcp connections and a ping, a response may be warranted, and thus an outgoing packet results. From what I can tell of the log file below, sometimes, this might happen before the responder has completed installing the outbound SAs. In the log file, the outbound AH has been added, but not the outbound ESP, which is the one the outgoing packet looks for first. Thus resulting in a second acquire. I think this becomes more problematic when using both AH AND ESP, rather than just using ESP with authentication. With the latter, only one SA needed thus reducing the latency in installing the SAs before incoming packet arrives. So far, the only solution I can think of besides mandating all userspace key daemons do SA maintenance is to perhaps add larval SAs for both directions when adding the SPI. Currently, responder does GETSPI for one way and initiator for opposite. When GETSPI is called, larval SA is created containing the SPI, but it is only for one direction. Perhaps we can add a larval SA (no SPI) for opposite direction to act as a placeholder indicating ACQUIRE occurring, since SAs are created for both directions during an ACQUIRE. The initiator may have larval SA from GETSPI and larval SA from the ACQUIRE depending that GETSPI is in opposite direction of ACQUIRE. Calling __find_acq_core() should ensure we don't create duplicate larval SAs. Also, should IKE negotiations return error, larval SAs should expire. They also should be removed when we do the xfrm_state_add() and xfrm_state_update() to add the new SAs. If this is not a good idea or there is a better way to do this, please let me know. I wanted to ask before coding and testing in case there is a better solution and I won't have wasted time. Regards, Joy mylogfile (it has been cut and pasted, so irrelevant sections left out) 2007-03-20 13:39:04: DEBUG: pfkey UPDATE succeeded: AH/Transport x.x.x.210[0]-x.x.x.55[0] spi=203125341(0xc1b725d) 2007-03-20 13:39:04: INFO: IPsec-SA established: AH/Transport x.x.x.210[0]-x.x.x.55[0] spi=203125341(0xc1b725d) 2007-03-20 13:39:04: DEBUG: pfkey UPDATE succeeded: ESP/Transport x.x.x.210[0]-x.x.x.55[0] spi=157349023(0x960f49f) 2007-03-20 13:39:04: INFO: IPsec-SA established: ESP/Transport x.x.x.210[0]-x.x.x.55[0] spi=157349023(0x960f49f) 2007-03-20 13:39:04: DEBUG: === 2007-03-20 13:39:04: DEBUG: get pfkey ADD message 2007-03-20 13:39:04: INFO: IPsec-SA established: AH/Transport x.x.x.55[0]-x.x.x.x.210[0] spi=196161037(0xbb12e0d) 2007-03-20 13:39:04: DEBUG: === 2007-03-20 13:39:04: DEBUG: get pfkey ACQUIRE message 2007-03-20 13:39:04: DEBUG: call pfkey_send_getspi 2007-03-20 13:39:04: DEBUG: pfkey GETSPI sent: AH/Transport x.x.x.210[0]-x.x.x.55[0] 2007-03-20 13:39:04: DEBUG: call pfkey_send_getspi 2007-03-20 13:39:04: DEBUG: pfkey GETSPI sent: ESP/Transport x.x.x.210[0]-x.x.x.55[0] 2007-03-20 13:39:04: DEBUG: pfkey getspi sent. 2007-03-20 13:39:04: DEBUG: get pfkey ADD message 2007-03-20 13:39:04: INFO: IPsec-SA established: ESP/Transport x.x.x.55[0]-x.x.x.210[0] spi=241180149(0xe601df5) - 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]: ipsecv6 needs a space when printing audit record.
This patch adds a space between printing of the src and dst ipv6 addresses. Otherwise, audit or other test tools may fail to process the audit record properly because they cannot find the dst address. Please let me know if this patch is ok. It is minor fix, but I have tested it and printing ipsecv6 audit record is much better. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] Patch is against linux-2.6.20-rc4. diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_policy.c linux-2.6.20.patch/net/xfrm/xfrm_policy.c --- linux-2.6.20.orig/net/xfrm/xfrm_policy.c2007-03-16 17:21:27.0 -0500 +++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-19 18:39:09.0 -0500 @@ -2089,7 +2089,7 @@ void xfrm_audit_log(uid_t auid, u32 sid, sizeof(struct in6_addr)); } audit_log_format(audit_buf, - src= NIP6_FMT dst= NIP6_FMT, + src= NIP6_FMT dst= NIP6_FMT, NIP6(saddr6), NIP6(daddr6)); } break; - 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]: double SAs are created when using AH and ESP together
On Tue, 2007-03-06 at 14:40 -0500, James Morris wrote: On Tue, 6 Mar 2007, Joy Latten wrote: I saw something similar to this some time ago when testing various failure modes, and discused it with Herbert. IIRC, there's a larval SA which is not torn down properly by Racoon once the full SA is established, and the larval SA keeps resending until it times out. Ok, good to know. I thought a bit more about this last night but am not sure best way to fix it. Perhaps a way to keep larval SA around until all SAs resulting from xfrm_vec[xfrm_nr] are established... oh well, just thinking out loud... :-) I think the solution, if this actually the problem, is for the userland code to maintain the SAs. Gotta agree. :-) I noticed that in xfrm_state_add we look for the larval SA in a few places without checking for protocol match. So when using both AH and ESP, whichever one gets added first, deletes the larval SA. It seems AH always gets added first and ESP is always the larval SA's protocol since the xfrm-tmpl has it first. Thus causing the additional km_query() Adding the check eliminates the double SA creation. I know this may not seem like a complete solution and I will continue to test and be on the lookout, but isn't having the check a good thing? So far I have tested SAs with just ESP, just AH and with both and all seems ok. Please let me know if this patch is ok. My kernel was 2.6.20-rc3-git3. Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_state.c linux-2.6.20.patch/net/xfrm/xfrm_state.c --- linux-2.6.20.orig/net/xfrm/xfrm_state.c 2007-03-08 17:39:14.0 -0600 +++ linux-2.6.20.patch/net/xfrm/xfrm_state.c2007-03-09 11:03:25.0 -0600 @@ -704,7 +704,8 @@ static struct xfrm_state *__find_acq_cor x-props.mode != mode || x-props.family != family || x-km.state != XFRM_STATE_ACQ || - x-id.spi != 0) + x-id.spi != 0 || + x-id.proto != proto) continue; switch (family) { @@ -801,7 +802,8 @@ int xfrm_state_add(struct xfrm_state *x) if (use_spi x-km.seq) { x1 = __xfrm_find_acq_byseq(x-km.seq); - if (x1 xfrm_addr_cmp(x1-id.daddr, x-id.daddr, family)) { + if (x1 ((x1-id.proto != x-id.proto) || + xfrm_addr_cmp(x1-id.daddr, x-id.daddr, family))) { xfrm_state_put(x1); x1 = NULL; } - 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]: double SAs are created when using AH and ESP together
On Fri, 2007-03-09 at 16:20 -0800, David Miller wrote: From: Joy Latten [EMAIL PROTECTED] Date: Fri, 9 Mar 2007 17:14:54 -0600 I noticed that in xfrm_state_add we look for the larval SA in a few places without checking for protocol match. So when using both AH and ESP, whichever one gets added first, deletes the larval SA. It seems AH always gets added first and ESP is always the larval SA's protocol since the xfrm-tmpl has it first. Thus causing the additional km_query() Adding the check eliminates the double SA creation. I know this may not seem like a complete solution and I will continue to test and be on the lookout, but isn't having the check a good thing? So far I have tested SAs with just ESP, just AH and with both and all seems ok. Please let me know if this patch is ok. My kernel was 2.6.20-rc3-git3. Signed-off-by: Joy Latten [EMAIL PROTECTED] Generally it looks OK, but I'm going to let this one sit for a while before I apply it so that other folks can review it too and spot any unintended consequences. In particular, I find it strance that we didn't check the protocol field all this time and I wonder whether that might be on purpose for some reason. Ok. And I'll continue to test, though most of my testing is done on lspp kernel. lspp kernel is an earlier version of upstream kernel and I noticed the old __xfrm4_find_acq() did include a check for the protocol, new __find_acq_core() doesn't. But neither checked for protocol after call to __xfrm_find_acq_byseq(). Joy - 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]: double SAs are created when using AH and ESP together
On Fri, 2007-03-09 at 19:54 -0500, Eric Paris wrote: On Fri, 2007-03-09 at 16:20 -0800, David Miller wrote: From: Joy Latten [EMAIL PROTECTED] Date: Fri, 9 Mar 2007 17:14:54 -0600 I noticed that in xfrm_state_add we look for the larval SA in a few places without checking for protocol match. So when using both AH and ESP, whichever one gets added first, deletes the larval SA. It seems AH always gets added first and ESP is always the larval SA's protocol since the xfrm-tmpl has it first. Thus causing the additional km_query() Adding the check eliminates the double SA creation. I know this may not seem like a complete solution and I will continue to test and be on the lookout, but isn't having the check a good thing? So far I have tested SAs with just ESP, just AH and with both and all seems ok. Please let me know if this patch is ok. My kernel was 2.6.20-rc3-git3. Signed-off-by: Joy Latten [EMAIL PROTECTED] Generally it looks OK, but I'm going to let this one sit for a while before I apply it so that other folks can review it too and spot any unintended consequences. In particular, I find it strance that we didn't check the protocol field all this time and I wonder whether that might be on purpose for some reason. At least the first hunk of this patch used to be checked back in net/ipv4/xfrm4_state.c in __xfrm4_find_acq and looks like it just was accidentally forgotten when there was a transition to using __find_acq_core http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=2770834c9f44afd1bfa13914c7285470775af657 Since Joy found this problem on a 2.6.18 kernel originally which was before this diff and had the proto check I'm guessing it is actually the second hunk which is more relevant to the problem. -Eric Don't mean to be a killjoy on a friday evening, but I just saw something else that I recall seeing a while back but forgot and so want to report now. While initiator is establishing SAs, the responder sends an ACQUIRE. I once got 3 sets of SAs. Now I only got 2 this time with above patch. This occurs when I start netperf to send streams of tcp and udp packets to test. This doesn't always happen. Just once in a while it seems... like a packet escaped and reached remote... (this is from lspp kernel) Initiator's log file: Mar 9 19:43:51 racoon: INFO: ISAKMP-SA established 9.3.192.210[500]-9.3.189.55[500] spi:dcdf529f9e7e1c7b:87400b8505e2f2aa Mar 9 19:43:52 racoon: INFO: initiate new phase 2 negotiation: 9.3.192.210[500]=9.3.189.55[500] Mar 9 19:43:53 racoon: INFO: IPsec-SA established: AH/Transport 9.3.189.55[0]-9.3.192.210[0] spi=193987518(0xb9003be) Mar 9 19:43:53 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.189.55[0]-9.3.192.210[0] spi=137973961(0x83950c9) Mar 9 19:43:53 racoon: INFO: IPsec-SA established: AH/Transport 9.3.192.210[0]-9.3.189.55[0] spi=248476422(0xecf7306) Mar 9 19:43:53 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.192.210[0]-9.3.189.55[0] spi=263395735(0xfb31997) Mar 9 19:43:53 racoon: INFO: respond new phase 2 negotiation: 9.3.192.210[500]=9.3.189.55[500] Mar 9 19:43:55 racoon: INFO: IPsec-SA established: AH/Transport 9.3.189.55[0]-9.3.192.210[0] spi=37664464(0x23eb6d0) Mar 9 19:43:55 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.189.55[0]-9.3.192.210[0] spi=246992333(0xeb8cdcd) Mar 9 19:43:55 racoon: INFO: IPsec-SA established: AH/Transport 9.3.192.210[0]-9.3.189.55[0] spi=203651374(0xc23792e) Mar 9 19:43:55 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.192.210[0]-9.3.189.55[0] spi=48093292(0x2ddd86c) -- Responder's log file: Mar 9 19:43:10 racoon: INFO: respond new phase 2 negotiation: 9.3.189.55[500]=9.3.192.210[500] Mar 9 19:43:11 racoon: INFO: IPsec-SA established: AH/Transport 9.3.192.210[0]-9.3.189.55[0] spi=248476422(0xecf7306) Mar 9 19:43:11 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.192.210[0]-9.3.189.55[0] spi=263395735(0xfb31997) Mar 9 19:43:11 racoon: INFO: IPsec-SA established: AH/Transport 9.3.189.55[0]-9.3.192.210[0] spi=193987518(0xb9003be) Mar 9 19:43:11 racoon: INFO: security context doi: 1 Mar 9 19:43:11 racoon: INFO: security context algorithm: 1 Mar 9 19:43:11 racoon: INFO: security context length: 39 Mar 9 19:43:11 racoon: INFO: security context: root:sysadm_r:sysadm_t:s0-s15:c0.c1023 Mar 9 19:43:11 racoon: INFO: initiate new phase 2 negotiation: 9.3.189.55[500]=9.3.192.210[500] Mar 9 19:43:11 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.189.55[0]-9.3.192.210[0] spi=137973961(0x83950c9) Mar 9 19:43:12 racoon: INFO: IPsec-SA established: AH/Transport 9.3.192.210[0]-9.3.189.55[0] spi=203651374(0xc23792e) Mar 9 19:43:12 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.192.210[0]-9.3.189.55[0] spi=48093292(0x2ddd86c) Mar 9 19:43:12 racoon: INFO: IPsec-SA established: AH
Re: when having to acquire an SA, ipsec drops the packet
On Mon, 2007-03-05 at 22:21 -0500, James Morris wrote: On Mon, 5 Mar 2007, Joy Latten wrote: 5. Around the time the set of SAs for OUT direction are to be inserted into SAD, I see another ACQUIRE happening. I have not yet figured out where this second ACQUIRE comes from and why it happens. As long as the minimal SA or set of valid outgoing SAs exist in SAD, an ACQUIRE should not happen. I saw something similar to this some time ago when testing various failure modes, and discused it with Herbert. IIRC, there's a larval SA which is not torn down properly by Racoon once the full SA is established, and the larval SA keeps resending until it times out. Ok, good to know. I thought a bit more about this last night but am not sure best way to fix it. Perhaps a way to keep larval SA around until all SAs resulting from xfrm_vec[xfrm_nr] are established... oh well, just thinking out loud... :-) Joy - 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: when having to acquire an SA, ipsec drops the packet
From: Joy Latten [EMAIL PROTECTED] Date: Mon, 05 Feb 2007 14:53:39 -0600 I can run some tests with this patch and report any results... Please check out the two most recent patches I posted: 1) Updated core patch with ipv6 side added. 2) Fix for thinko noticed by Venkat. I have been testing this a lot in the lspp kernel. Plan to test also in upstream kernel. I am seeing a second ACQUIRE occur while establishing the SAs. My scenario: My policy states to use both the ESP and AH protocols (may not make much sense but this was for testing purposes). I get double SAs with only difference being SPI. Here is what I see happening... 1. Trigger first ACQUIRE via ping or netperf. 2. xfrm_lookup() calls xfrm_tmpl_resolv() who calls xfrm_state_find(). First time around, we need to establish SA, so a minimal SA get allocated and put in SAD, timer is set for the minimal SA to be ACQUIRED and km_query() gets called. 3. xfrm_tmpl_resolv() returns -EAGAIN causing add_wait_queue(km_waitq, wait) and proceeding code to get called waiting for SA to be established. As long as the minimal SA with XFRM_STATE_ACQUIRE is in SAD, we keep waiting... 4. First set of SAs (one for AH and ESP) for IN direction get inserted in SAD. 5. Around the time the set of SAs for OUT direction are to be inserted into SAD, I see another ACQUIRE happening. I have not yet figured out where this second ACQUIRE comes from and why it happens. As long as the minimal SA or set of valid outgoing SAs exist in SAD, an ACQUIRE should not happen. The minimal SA does not get removed from the SAD until the set of SAs for OUT get added and the xfrm_state_lock released. And the lock pretty much guarantees no one else can step through the SAD until after new SAs are being added... and if someone gets the lock to step though SAD before OUT SAs are added, minimal SA is still there... 6. Since this second ACQUIRE was able to happen, result is identical sets of SAs for the traffic stream. SPIs are only difference. 7. Noticed something while pasting log info below. Perhaps when outgoing AH SA is added, wake_up(km_waitq) gets called, lock released, and minimal SA deleted (xfrm_state_add()), xfrm_tmpl_resolv() is called and it looks first for the outgoing ESP SA. Since it is not there yet and no minimal SA, then km_query() results in an ACQUIRE just before the outgoing ESP SA gets added. It would explain why I only see it when both ESP and AH are specified... that is if I am thinking correctly... Regards, Joy Latten From my log file: Mar 5 19:10:02 racoon: INFO: initiate new phase 2 negotiation: 9.3.192.210[500]=9.3.189.55[500] Mar 5 19:10:03 racoon: INFO: IPsec-SA established: AH/Transport 9.3.189.55[0]-9.3.192.210[0] spi=137942922(0x838d78a) Mar 5 19:10:03 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.189.55[0]-9.3.192.210[0] spi=244321490(0xe900cd2) Mar 5 19:10:03 racoon: INFO: IPsec-SA established: AH/Transport 9.3.192.210[0]-9.3.189.55[0] spi=38721750(0x24ed8d6) Mar 5 19:10:03 racoon: INFO: initiate new phase 2 negotiation: 9.3.192.210[500]=9.3.189.55[500] Mar 5 19:10:03 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.192.210[0]-9.3.189.55[0] spi=265079770(0xfcccbda) Mar 5 19:10:05 racoon: INFO: IPsec-SA established: AH/Transport 9.3.189.55[0]-9.3.192.210[0] spi=108627618(0x67986a2) Mar 5 19:10:05 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.189.55[0]-9.3.192.210[0] spi=182973856(0xae7f5a0) Mar 5 19:10:05 racoon: INFO: IPsec-SA established: AH/Transport 9.3.192.210[0]-9.3.189.55[0] spi=58486297(0x37c6e19) Mar 5 19:10:05 racoon: INFO: IPsec-SA established: ESP/Transport 9.3.192.210[0]-9.3.189.55[0] spi=268295215(0xffddc2f) - 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: when having to acquire an SA, ipsec drops the packet
From: Joy Latten [EMAIL PROTECTED] Date: Mon, 05 Feb 2007 14:53:39 -0600 I can run some tests with this patch and report any results... Please check out the two most recent patches I posted: 1) Updated core patch with ipv6 side added. 2) Fix for thinko noticed by Venkat. Just a quick update. I have patched an lspp kernel and latest kernel.org kernel. I have been using them alternately while doing some minor testing and have not found any problems. Over the next few days, I plan to run a few stress tests for labeled ipsec and regular ipsec and will do so with the patched lspp kernel. I will also try with kernel.org kernel. It may take a few days. Regards, Joy - 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: when having to acquire an SA, ipsec drops the packet
On Thu, 2007-02-01 at 18:44 -0500, James Morris wrote: On Thu, 1 Feb 2007, Joy Latten wrote: IPsec returns EAGAIN when it needs to acquire an SA. There have been a thread or two about this... Has there been any info or progress in how best to fix this? James Morris presented some work/ideas, http://vger.kernel.org/jmorris_ipsec_sa_resolution_netconf2006.pdf The status of this is that I started refactoring some core IPv6 code (to bring it in line with the IPv4 code, e.g. create an ip6_route_connect() to match ip_route_connect(), and manage the packet queuing from there) and ran into some IPv6 bugs, and fixed those, but then ran out of time on the IPsec stuff. AFAIK, no other progress has been made. Generally, the problem is only seen when using the BSD userland, which does not proactively maintain SAs. Openswan does, and general IPsec users running it never see the problem, so it's not clear how high the priority is for fixing this really is in the general case. It's quite a lot of surgery on core networking to fix a problem which doesn't occur with the Linux tools, which seemingly most people use when they're not using proprietary and/or userland IPsec stacks. Non-kernel options include moving to Openswan (which I would hope is getting labeled networking support at some stage in any case), or have the BSD code proactively maintain SAs. Also, applications using TCP, and UDP applications with their own session management, will resend packets while the SA is being negotiated, which I've observed as typically completing before the first resend. I think one of the practical problems with this is that the apps are not expecting an EAGAIN and thus decide to crash. A quick dirty solution, which is what I think the BSD kernels do, is to still drop the packet but just not return an error to the app. The app then just sees a slight delay on the initial connection, as if a DNS lookup took a bit longer than usual. When using labeled xfrms (xfrms that contain a security context), there is potential for a greater amount of SAs to be created than when using regular xfrms. An SA may be created every time a different security context is encountered in a particular traffic stream. This could be many if each networking app has its own security context, making current behavior problematic. Do you have any examples of how many SAs would need to be created on a typical system? No, but I have to admit, just playing around with it gives me a scary idea. We have s0-s15 levels, each with c0-c1023 categories. Then I have noticed, each mapped selinux user is a different context... thus just an ssh session by a sysadm and a regular user (who is mapped to an selinux user) can cause 8 SAs to be generated, whereas with regular ipsec, we only need one... :-( Regards, Joy - 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: when having to acquire an SA, ipsec drops the packet
I can run some tests with this patch and report any results... Regards, Joy On Sun, 2007-02-04 at 20:53 -0800, David Miller wrote: From: James Morris [EMAIL PROTECTED] Date: Thu, 1 Feb 2007 18:44:48 -0500 (EST) A quick dirty solution, which is what I think the BSD kernels do, is to still drop the packet but just not return an error to the app. The app then just sees a slight delay on the initial connection, as if a DNS lookup took a bit longer than usual. I have another idea. Why don't we just flat-out ignore MSG_DONTWAIT for the socket visible cases, and handle connect() similarly? I think this is (just barely) legal, will be simple to implement, and will leave us with semantics that look like: 1) Sockets never see -EAGAIN due to SA resolution. They'll just pause until the route is resolved, even with O_NONBLOCK or MSG_DONTWAIT. 2) Asynchronous contexts such as ICMP replies and firewalling will still see the -EAGAIN and simply drop packets. These sleeps are legal because all of the socket paths involved have to be able to do lock_socket() (at a minimum) anyways. Something like this (untested) on the ipv4 side, for example: diff --git a/include/net/route.h b/include/net/route.h index 486e37a..a8af632 100644 --- a/include/net/route.h +++ b/include/net/route.h @@ -146,7 +146,8 @@ static inline char rt_tos2priority(u8 tos) static inline int ip_route_connect(struct rtable **rp, __be32 dst, __be32 src, u32 tos, int oif, u8 protocol, -__be16 sport, __be16 dport, struct sock *sk) +__be16 sport, __be16 dport, struct sock *sk, +int flags) { struct flowi fl = { .oif = oif, .nl_u = { .ip4_u = { .daddr = dst, @@ -168,7 +169,7 @@ static inline int ip_route_connect(struct rtable **rp, __be32 dst, *rp = NULL; } security_sk_classify_flow(sk, fl); - return ip_route_output_flow(rp, fl, sk, 0); + return ip_route_output_flow(rp, fl, sk, 1); } static inline int ip_route_newports(struct rtable **rp, u8 protocol, diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c index 90c74b4..fa2c982 100644 --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c @@ -72,7 +72,7 @@ int dccp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len) tmp = ip_route_connect(rt, nexthop, inet-saddr, RT_CONN_FLAGS(sk), sk-sk_bound_dev_if, IPPROTO_DCCP, -inet-sport, usin-sin_port, sk); +inet-sport, usin-sin_port, sk, 1); if (tmp 0) return tmp; diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 8640096..5750a2b 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1007,7 +1007,7 @@ static int inet_sk_reselect_saddr(struct sock *sk) RT_CONN_FLAGS(sk), sk-sk_bound_dev_if, sk-sk_protocol, -inet-sport, inet-dport, sk); +inet-sport, inet-dport, sk, 0); if (err) return err; diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c index 7b068a8..0072d79 100644 --- a/net/ipv4/datagram.c +++ b/net/ipv4/datagram.c @@ -49,7 +49,7 @@ int ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len) err = ip_route_connect(rt, usin-sin_addr.s_addr, saddr, RT_CONN_FLAGS(sk), oif, sk-sk_protocol, -inet-sport, usin-sin_port, sk); +inet-sport, usin-sin_port, sk, 1); if (err) return err; if ((rt-rt_flags RTCF_BROADCAST) !sock_flag(sk, SOCK_BROADCAST)) { diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c index a6c63bb..fed6a1e 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -489,7 +489,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg, } security_sk_classify_flow(sk, fl); - err = ip_route_output_flow(rt, fl, sk, !(msg-msg_flagsMSG_DONTWAIT)); + err = ip_route_output_flow(rt, fl, sk, 1); } if (err) goto done; diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index f061ec5..383e4b5 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -191,7 +191,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len) tmp = ip_route_connect(rt, nexthop, inet-saddr, RT_CONN_FLAGS(sk), sk-sk_bound_dev_if, IPPROTO_TCP, -inet-sport, usin-sin_port, sk); +inet-sport, usin-sin_port, sk, 1); if (tmp 0)
when having to acquire an SA, ipsec drops the packet
IPsec returns EAGAIN when it needs to acquire an SA. There have been a thread or two about this... Has there been any info or progress in how best to fix this? James Morris presented some work/ideas, http://vger.kernel.org/jmorris_ipsec_sa_resolution_netconf2006.pdf When using labeled xfrms (xfrms that contain a security context), there is potential for a greater amount of SAs to be created than when using regular xfrms. An SA may be created every time a different security context is encountered in a particular traffic stream. This could be many if each networking app has its own security context, making current behavior problematic. Bugreport 225328 has been opened in the Redhat Bugzilla to address when having to acquire an SA, ipsec drops the packet. Regards, Joy - 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]net/xfrm:fix crash in ipsec audit logging
On Tue, 2006-12-26 at 13:37 -0500, jamal wrote: On Tue, 2006-26-12 at 18:56 +0100, Ingo Molnar wrote: + xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid, + AUDIT_MAC_IPSEC_DELSPD, delete, xp, NULL); + if (!delete) { struct sk_buff *resp_skb; You could move the call into the else from above if (!delete) maybe? Otherwise you have to add back the if (delete) check since that function could be used to either retrieve (which is not subject to an audit) or delete an xp. cheers, jamal My apologies as I am just reading my email. Yes, I think in the else part of the if (!delete). I also added a check to xfrm_audit_log() such that if both xfrm and policy are NULL, we return. There isn't anything to audit since we are only auditing creation and deletion of xfrm and policy. Ingo, could you try this patch and let me know if everything works ok for you. I have built and test in my environment, but not tested as you are using it. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] -- diff -urpN linux-2.6.19.orig/net/xfrm/xfrm_policy.c linux-2.6.19/net/xfrm/xfrm_policy.c --- linux-2.6.19.orig/net/xfrm/xfrm_policy.c2007-01-02 14:24:14.0 -0600 +++ linux-2.6.19/net/xfrm/xfrm_policy.c 2007-01-02 14:28:24.0 -0600 @@ -2003,6 +2003,9 @@ void xfrm_audit_log(uid_t auid, u32 sid, if (audit_enabled == 0) return; + if ((x == NULL) (xp == NULL)) + return; + audit_buf = audit_log_start(current-audit_context, GFP_ATOMIC, type); if (audit_buf == NULL) return; diff -urpN linux-2.6.19.orig/net/xfrm/xfrm_user.c linux-2.6.19/net/xfrm/xfrm_user.c --- linux-2.6.19.orig/net/xfrm/xfrm_user.c 2007-01-02 14:24:14.0 -0600 +++ linux-2.6.19/net/xfrm/xfrm_user.c 2007-01-02 14:28:14.0 -0600 @@ -1268,10 +1268,6 @@ static int xfrm_get_policy(struct sk_buf xp = xfrm_policy_bysel_ctx(type, p-dir, p-sel, tmp.security, delete); security_xfrm_policy_free(tmp); } - if (delete) - xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid, - AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL); - if (xp == NULL) return -ENOENT; @@ -1289,6 +1285,10 @@ static int xfrm_get_policy(struct sk_buf } else { if ((err = security_xfrm_policy_delete(xp)) != 0) goto out; + + xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid, + AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL); + c.data.byid = p-index; c.event = nlh-nlmsg_type; c.seq = nlh-nlmsg_seq; . - 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 1/1] additional change to ipsec audit
Sorry! Sign off included this time. This patch disables auditing in ipsec when CONFIG_AUDITSYSCALL is disabled in the kernel. This patch also includes a bug fix for xfrm_state.c as a result of original ipsec audit patch. regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] --- diff -urpN linux-2.6.18-patch/include/net/xfrm.h linux-2.6.18-patch.2/include/net/xfrm.h --- linux-2.6.18-patch/include/net/xfrm.h 2006-11-27 12:29:11.0 -0600 +++ linux-2.6.18-patch.2/include/net/xfrm.h 2006-11-28 13:26:49.0 -0600 @@ -395,8 +395,13 @@ struct xfrm_audit uid_t loginuid; u32 secid; }; -void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, + +#ifdef CONFIG_AUDITSYSCALL +extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, struct xfrm_policy *xp, struct xfrm_state *x); +#else +#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) { diff -urpN linux-2.6.18-patch/net/xfrm/xfrm_policy.c linux-2.6.18-patch.2/net/xfrm/xfrm_policy.c --- linux-2.6.18-patch/net/xfrm/xfrm_policy.c 2006-11-27 12:29:33.0 -0600 +++ linux-2.6.18-patch.2/net/xfrm/xfrm_policy.c 2006-11-28 14:51:09.0 -0600 @@ -1955,6 +1955,7 @@ int xfrm_bundle_ok(struct xfrm_policy *p EXPORT_SYMBOL(xfrm_bundle_ok); +#ifdef CONFIG_AUDITSYSCALL /* Audit addition and deletion of SAs and ipsec policy */ void xfrm_audit_log(uid_t auid, u32 sid, int type, int result, @@ -2063,6 +2064,7 @@ void xfrm_audit_log(uid_t auid, u32 sid, } EXPORT_SYMBOL(xfrm_audit_log); +#endif /* CONFIG_AUDITSYSCALL */ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo) { diff -urpN linux-2.6.18-patch/net/xfrm/xfrm_state.c linux-2.6.18-patch.2/net/xfrm/xfrm_state.c --- linux-2.6.18-patch/net/xfrm/xfrm_state.c2006-11-27 12:29:33.0 -0600 +++ linux-2.6.18-patch.2/net/xfrm/xfrm_state.c 2006-11-28 12:58:56.0 -0600 @@ -407,7 +407,6 @@ restart: xfrm_state_hold(x); spin_unlock_bh(xfrm_state_lock); - xfrm_state_delete(x); err = xfrm_state_delete(x); xfrm_audit_log(audit_info-loginuid, audit_info-secid, - 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/1] additional ipsec audit patch
On Wed, 2006-11-29 at 19:32 -0500, James Morris wrote: On Wed, 29 Nov 2006, James Morris wrote: On Wed, 29 Nov 2006, Joy Latten wrote: This patch disables auditing in ipsec when CONFIG_AUDITSYSCALL is disabled in the kernel. This patch also includes a bug fix for xfrm_state.c as a result of original ipsec audit patch. Let me know if it looks ok. Also, the last patch contains no Signed-off-by: line, please resend. And, what is the testing status of these patches? I ran a stress test overnight using labeled ipsec on a patched lspp55 kernel using racoon last week. The additional patch to xfrm_state.c was my fault when rebasing to 2.6.19-rc6 to send upstream. I plan to run an ipv4 and ipv6 stress test tonight and tomorrow using labeled ipsec with auditing enabled on the lspp56 kernel, which contains ipsec audit patch, to ensure no regression has occurred. I can also run an ipv4 and ipv6 stress tests with regular ipsec over the weekend for further ensurance. I compiled and did unit test with SELINUX disabled, AUDITSYSCALL disabled, and with both enabled. regards, Joy - 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 1/1] add auditing to ipsec
This patch adds auditing to ipsec. An audit message occurs when an ipsec SA or ipsec policy is created/deleted. Patch was built against linux kernel 2.6.19-rc6. Please let me know if this is acceptable. Regards, Joy Signed-off-by: Joy Latten [EMAIL PROTECTED] --- diff -urpN linux-2.6.18.orig/include/linux/audit.h linux-2.6.18-patch/include/linux/audit.h --- linux-2.6.18.orig/include/linux/audit.h 2006-11-27 11:21:16.0 -0600 +++ linux-2.6.18-patch/include/linux/audit.h2006-11-27 12:28:43.0 -0600 @@ -101,6 +101,10 @@ #define AUDIT_MAC_CIPSOV4_DEL 1408/* NetLabel: del CIPSOv4 DOI entry */ #define AUDIT_MAC_MAP_ADD 1409/* NetLabel: add LSM domain mapping */ #define AUDIT_MAC_MAP_DEL 1410/* NetLabel: del LSM domain mapping */ +#define AUDIT_MAC_IPSEC_ADDSA 1411/* Add a XFRM state */ +#define AUDIT_MAC_IPSEC_DELSA 1412/* Delete a XFRM state */ +#define AUDIT_MAC_IPSEC_ADDSPD 1413/* Add a XFRM policy */ +#define AUDIT_MAC_IPSEC_DELSPD 1414/* Delete a XFRM policy */ #define AUDIT_FIRST_KERN_ANOM_MSG 1700 #define AUDIT_LAST_KERN_ANOM_MSG1799 @@ -377,6 +381,7 @@ extern void auditsc_get_stamp(struct aud struct timespec *t, unsigned int *serial); extern int audit_set_loginuid(struct task_struct *task, uid_t loginuid); extern uid_t audit_get_loginuid(struct audit_context *ctx); +extern void audit_log_task_context(struct audit_buffer *ab); extern int __audit_ipc_obj(struct kern_ipc_perm *ipcp); extern int __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode); extern int audit_bprm(struct linux_binprm *bprm); @@ -449,6 +454,7 @@ extern int audit_n_rules; #define audit_inode_update(i) do { ; } while (0) #define auditsc_get_stamp(c,t,s) do { BUG(); } while (0) #define audit_get_loginuid(c) ({ -1; }) +#define audit_log_task_context(b) do { ; } while (0) #define audit_ipc_obj(i) ({ 0; }) #define audit_ipc_set_perm(q,u,g,m) ({ 0; }) #define audit_bprm(p) ({ 0; }) diff -urpN linux-2.6.18.orig/include/net/xfrm.h linux-2.6.18-patch/include/net/xfrm.h --- linux-2.6.18.orig/include/net/xfrm.h2006-11-27 11:21:43.0 -0600 +++ linux-2.6.18-patch/include/net/xfrm.h 2006-11-27 12:29:11.0 -0600 @@ -389,6 +389,15 @@ extern int xfrm_unregister_km(struct xfr extern unsigned int xfrm_policy_count[XFRM_POLICY_MAX*2]; +/* Audit Information */ +struct xfrm_audit +{ + uid_t loginuid; + u32 secid; +}; +void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, + struct xfrm_policy *xp, struct xfrm_state *x); + static inline void xfrm_pol_hold(struct xfrm_policy *policy) { if (likely(policy != NULL)) @@ -934,7 +943,7 @@ static inline int xfrm_state_sort(struct #endif extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq); extern int xfrm_state_delete(struct xfrm_state *x); -extern void xfrm_state_flush(u8 proto); +extern void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info); extern int xfrm_replay_check(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_advance(struct xfrm_state *x, __be32 seq); extern void xfrm_replay_notify(struct xfrm_state *x, int event); @@ -987,13 +996,13 @@ struct xfrm_policy *xfrm_policy_bysel_ct struct xfrm_selector *sel, struct xfrm_sec_ctx *ctx, int delete); struct xfrm_policy *xfrm_policy_byid(u8, int dir, u32 id, int delete); -void xfrm_policy_flush(u8 type); +void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); u32 xfrm_get_acqseq(void); void xfrm_alloc_spi(struct xfrm_state *x, __be32 minspi, __be32 maxspi); struct xfrm_state * xfrm_find_acq(u8 mode, u32 reqid, u8 proto, xfrm_address_t *daddr, xfrm_address_t *saddr, int create, unsigned short family); -extern void xfrm_policy_flush(u8 type); +extern void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info); extern int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol); extern int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *xdst, struct flowi *fl, int family, int strict); diff -urpN linux-2.6.18.orig/kernel/auditsc.c linux-2.6.18-patch/kernel/auditsc.c --- linux-2.6.18.orig/kernel/auditsc.c 2006-11-27 11:19:36.0 -0600 +++ linux-2.6.18-patch/kernel/auditsc.c 2006-11-27 12:26:39.0 -0600 @@ -730,7 +730,7 @@ static inline void audit_free_context(st printk(KERN_ERR audit: freed %d contexts\n, count); } -static void audit_log_task_context(struct audit_buffer *ab) +void audit_log_task_context(struct audit_buffer *ab) { char *ctx = NULL; ssize_t len = 0; @@ -759,6 +759,8 @@ error_path: return; } +EXPORT_SYMBOL(audit_log_task_context); + static
[PATCH 1/1] additional ipsec audit patch
This patch disables auditing in ipsec when CONFIG_AUDITSYSCALL is disabled in the kernel. This patch also includes a bug fix for xfrm_state.c as a result of original ipsec audit patch. Let me know if it looks ok. My mail gateway has been acting crazy so I apologize for any replicas being sent for ipsec audit patches. regards, Joy diff -urpN linux-2.6.18-patch/include/net/xfrm.h linux-2.6.18-patch.2/include/net/xfrm.h --- linux-2.6.18-patch/include/net/xfrm.h 2006-11-27 12:29:11.0 -0600 +++ linux-2.6.18-patch.2/include/net/xfrm.h 2006-11-28 13:26:49.0 -0600 @@ -395,8 +395,13 @@ struct xfrm_audit uid_t loginuid; u32 secid; }; -void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, + +#ifdef CONFIG_AUDITSYSCALL +extern void xfrm_audit_log(uid_t auid, u32 secid, int type, int result, struct xfrm_policy *xp, struct xfrm_state *x); +#else +#define xfrm_audit_log(a,s,t,r,p,x) do { ; } while (0) +#endif /* CONFIG_AUDITSYSCALL */ static inline void xfrm_pol_hold(struct xfrm_policy *policy) { diff -urpN linux-2.6.18-patch/net/xfrm/xfrm_policy.c linux-2.6.18-patch.2/net/xfrm/xfrm_policy.c --- linux-2.6.18-patch/net/xfrm/xfrm_policy.c 2006-11-27 12:29:33.0 -0600 +++ linux-2.6.18-patch.2/net/xfrm/xfrm_policy.c 2006-11-28 14:51:09.0 -0600 @@ -1955,6 +1955,7 @@ int xfrm_bundle_ok(struct xfrm_policy *p EXPORT_SYMBOL(xfrm_bundle_ok); +#ifdef CONFIG_AUDITSYSCALL /* Audit addition and deletion of SAs and ipsec policy */ void xfrm_audit_log(uid_t auid, u32 sid, int type, int result, @@ -2063,6 +2064,7 @@ void xfrm_audit_log(uid_t auid, u32 sid, } EXPORT_SYMBOL(xfrm_audit_log); +#endif /* CONFIG_AUDITSYSCALL */ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo) { diff -urpN linux-2.6.18-patch/net/xfrm/xfrm_state.c linux-2.6.18-patch.2/net/xfrm/xfrm_state.c --- linux-2.6.18-patch/net/xfrm/xfrm_state.c2006-11-27 12:29:33.0 -0600 +++ linux-2.6.18-patch.2/net/xfrm/xfrm_state.c 2006-11-28 12:58:56.0 -0600 @@ -407,7 +407,6 @@ restart: xfrm_state_hold(x); spin_unlock_bh(xfrm_state_lock); - xfrm_state_delete(x); err = xfrm_state_delete(x); xfrm_audit_log(audit_info-loginuid, audit_info-secid, - 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: [redhat-lspp] ipsec acquire has security context although I a m not using it.
Venkat, This doesn't look right since kzalloc would already have zeroed the structure out. Are you sure you are getting garbage in the acquire from the kernel? If you are, I strongly doubt that this would be the one causing it (unless kzalloc on this arch misbehaved). Or is this a racoon bug? Yes, you are correct! Thanks for pointing this out to me as I missed it! It is racoon that has the bug. Will fix and post correct fix shortly. Please ignore attached fix as it is incorrect. Again, thanks! Regards, Joy When using ipsec while selinux is enabled in my kernel, my racoon daemon fails to establish an SA. I believe the ACQUIRE sent from kernel has a security context although I am not using this feature with ipsec. As a result, racoon fails to establish the SA, because it is looking for a policy with security context. I noticed the security context contains garbage. I am using a pseries, power5, ppc64 box, and it appears that since policy-security structure is not really initialized or zero'd out when not using, it is possible it may contain garbage on my pseries and a call such as if (policy-security) may come back as true such that security context is included in my acquire message although I believe it should not be. Hopefully, the below patch is acceptable. I have compiled and tested it. Regards, Joy Latten diff -urpN linux-2.6.17.orig/net/xfrm/xfrm_policy.c linux-2.6.17.patch/net/xfrm/xfrm_policy.c --- linux-2.6.17.orig/net/xfrm/xfrm_policy.c 2006-09-19 02:11:33.0 -0500 +++ linux-2.6.17.patch/net/xfrm/xfrm_policy.c2006-09-19 04:33:50.0 -0500 @@ -319,6 +319,7 @@ struct xfrm_policy *xfrm_policy_alloc(gf init_timer(policy-timer); policy-timer.data = (unsigned long)policy; policy-timer.function = xfrm_policy_timer; +policy-security = NULL; } return policy; } -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [EMAIL PROTECTED] with the words unsubscribe selinux without quotes as the message. - 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
ipsec acquire has security context although I am not using it.
When using ipsec while selinux is enabled in my kernel, my racoon daemon fails to establish an SA. I believe the ACQUIRE sent from kernel has a security context although I am not using this feature with ipsec. As a result, racoon fails to establish the SA, because it is looking for a policy with security context. I noticed the security context contains garbage. I am using a pseries, power5, ppc64 box, and it appears that since policy-security structure is not really initialized or zero'd out when not using, it is possible it may contain garbage on my pseries and a call such as if (policy-security) may come back as true such that security context is included in my acquire message although I believe it should not be. Hopefully, the below patch is acceptable. I have compiled and tested it. Regards, Joy Latten diff -urpN linux-2.6.17.orig/net/xfrm/xfrm_policy.c linux-2.6.17.patch/net/xfrm/xfrm_policy.c --- linux-2.6.17.orig/net/xfrm/xfrm_policy.c2006-09-19 02:11:33.0 -0500 +++ linux-2.6.17.patch/net/xfrm/xfrm_policy.c 2006-09-19 04:33:50.0 -0500 @@ -319,6 +319,7 @@ struct xfrm_policy *xfrm_policy_alloc(gf init_timer(policy-timer); policy-timer.data = (unsigned long)policy; policy-timer.function = xfrm_policy_timer; + policy-security = NULL; } return policy; } - 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: IPSec kernel oops on ppc64
It works! I applied the patch to linux-2.6.17 + patch-2.6.17-rc1 and tried icmp, tcp and udp as well as sftp with ipsec and they all worked. Thanks Regards, Joy Herbert Xu writes: Interesting. We were previously off by 28 bytes, now we're off by 8 :) You missed a couple of 'beqlr' instructions (branch if equal to LR). I'd be interested to know if it still fails with the patch below. Thanks, Paul. diff --git a/arch/powerpc/lib/memcpy_64.S b/arch/powerpc/lib/memcpy_64.S index fd66acf..7173ba9 100644 --- a/arch/powerpc/lib/memcpy_64.S +++ b/arch/powerpc/lib/memcpy_64.S @@ -11,6 +11,7 @@ #include asm/ppc_asm.h .align 7 _GLOBAL(memcpy) + std r3,48(r1) /* save destination pointer for return value */ mtcrf 0x01,r5 cmpldi cr1,r5,16 neg r6,r3 # LS 3 bits = # bytes to 8-byte dest bdry @@ -38,7 +39,7 @@ _GLOBAL(memcpy) stdur9,16(r3) bdnz1b 3:std r8,8(r3) - beqlr + beq 3f addir3,r3,16 ld r9,8(r4) .Ldo_tail: @@ -53,7 +54,8 @@ _GLOBAL(memcpy) 2:bf cr7*4+3,3f rotldi r9,r9,8 stb r9,0(r3) -3:blr +3:ld r3,48(r1) /* return dest pointer */ + blr .Lsrc_unaligned: srdir6,r5,3 @@ -115,7 +117,7 @@ _GLOBAL(memcpy) 5:srd r12,r9,r11 or r12,r8,r12 std r12,24(r3) - beqlr + beq 4f cmpwi cr1,r5,8 addir3,r3,32 sld r9,r9,r10 @@ -167,4 +169,5 @@ _GLOBAL(memcpy) 3:bf cr7*4+3,4f lbz r0,0(r4) stb r0,0(r3) -4:blr +4:ld r3,48(r1) /* return dest pointer */ + blr - 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: IPSec kernel oops on ppc64
I can try patch-2.6.18-rc1, etc... to see which one it stops working on to narrow it down. If you could do this in the meanwhile, it would help us out a lot. It stops working in patch-2.6.18-rc1. Regards, Joy - 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: IPSec kernel oops on ppc64
Joy Latten [EMAIL PROTECTED] wrote: I installed 2.6.17 + patch-2.6.18-rc4 + 2.6.18-rc4-mm2 onto two pSeries power 5 (ppc64 lpars) machines. I configured IPSec using the configuration listed below. Could you try straight 2.6.17? If that crashes too, then at least we can be sure that it isn't something new. A straight 2.6.17 kernel does not crash and my pings work. A 2.6.17 + patch-2.6.18-rc4 does crash and my pings do not work. The above tests were done on a ppc64. I can try patch-2.6.18-rc1, etc... to see which one it stops working on to narrow it down. Regards, Joy - 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
IPSec kernel oops on ppc64
I installed 2.6.17 + patch-2.6.18-rc4 + 2.6.18-rc4-mm2 onto two pSeries power 5 (ppc64 lpars) machines. I configured IPSec using the configuration listed below. A ping from one machine to the other, hangs. No packets leave the machine issuing the ping. When I tried sftp, I received following oops. Has anyone else had problems with IPSec on pSeries? [EMAIL PROTECTED] jml]# sftp hvracer1 Connecting to hvracer1... kernel BUG in skb_to_sgvec at net/xfrm/xfrm_algo.c:620! cpu 0x0: Vector: 700 (Program Check) at [c000466eb240] pc: c035f2f4: .skb_to_sgvec+0x288/0x2ec lr: d09605e0: .esp_output+0x340/0x494 [esp4] sp: c000466eb4c0 msr: 80029032 current = 0xc00045a69910 paca= 0xc0484400 pid = 2213, comm = ssh kernel BUG in skb_to_sgvec at net/xfrm/xfrm_algo.c:620! enter ? for help 0:mon t [c000466eb590] d09605e0 .esp_output+0x340/0x494 [esp4] [c000466eb680] c0357bd4 .xfrm4_output_finish2+0x2b8/0x3d0 [c000466eb720] c0357ea0 .xfrm4_output+0x74/0x88 [c000466eb7a0] c031b188 .ip_queue_xmit+0x4a8/0x540 [c000466eb8a0] c032e9b8 .tcp_transmit_skb+0x820/0x890 [c000466eb960] c0331b74 .tcp_connect+0x308/0x3b0 [c000466eba00] c03361d0 .tcp_v4_connect+0x52c/0x6c0 [c000466ebb80] c0344664 .inet_stream_connect+0x10c/0x358 [c000466ebc60] c02dba14 .sys_connect+0xd8/0x120 [c000466ebd90] c02fe420 .compat_sys_socketcall+0xdc/0x214 [c000466ebe30] c000871c syscall_exit+0x0/0x40 --- Exception: c00 (System Call) at 07a9f8fc SP (fc63f230) is in userspace Configured IPSec as follows: add x.x.x.55 x.x.x.206 esp 35590 -m transport -E 3des-cbc 06183223c23a21e8b36c566b -A hmac-md5 TAHITEST89ABCDEF; add x.x.x.206 x.x.x.55 esp 12360 -m transport -E 3des-cbc 06183223c23a21e8b36c566b -A hmac-md5 TAHITEST89ABCDEF; spdadd x.x.x.55 x.x.x.206 any -P in ipsec esp/transport//require; spdadd x.x.x.206 x.x.x.55 any -P out ipsec esp/transport//require; Same config on both machines, except for spdadd entry. The in and out are swapped on the other machine. Regards, Joy Latten - 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/1] updated: TCP/UDP getpeersec
On Thu, 2006-02-16 at 01:30 -0500, Catherine Zhang wrote: Joy, Thanks for your comment and sorry for the delay. Did you mean a separate error code for 'null' context? The current code catches the case when the sid is SECSID_NULL, and returns ENOPROTOOPT. The question is whether we want to create a different error code for this case. Any suggestions? Actually, what I meant was should we return an error at all, not create a new error code. Should it be considered an error if a userspace application asks for the peer context and ipsec is not being used? My guess is that most apps will not care if ipsec is being used when they ask for this info. Therefore, I was wondering instead of checking if sid == SECSID_NULL, just let security_to_sid_context() return null and send this to userspace as context. Userspace would then know a null context returned means no context for peer. On 2/10/06, Joy Latten [EMAIL PROTECTED] wrote: Catherine, I am just wondering about something... Should a peer_sid of 0 or SECSID_NULL be an error here if the connection doesn't have a transform? I understand we only get peer's context if a xfrm is involved, but I am thinking most user applications may not kno or care if there is a xfrm. If not treated as an error, it looks like security_to_sid_context() would just return null for context. Would that be acceptable? Perhaps it is just important that we document the behaviour because I am thinking most user apps will not care or know if ipsec is running, so programmers may use this socket option to get peer context and may need to understand why they received an error of ENOPROTOOPT. } + else { err = -ENOPROTOOPT; goto out; } - ssec = sock-sk-sk_security; - - err = security_sid_to_context(ssec-peer_sid, scontext, scontext_len); + err = security_sid_to_context(peer_sid, scontext, scontext_len); + if (err) goto out; @@ -3396,6 +3410,23 @@ out: return err; } + Regards, Joy - 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/1] updated: TCP/UDP getpeersec
Catherine, My mailer may have been acting up, but the from header of your email had [EMAIL PROTECTED] instead of [EMAIL PROTECTED] :-) diff -puN security/selinux/hooks.c~lsm-secpeer security/selinux/hooks.c --- linux-2.6.16-rc1/security/selinux/hooks.c~lsm-secpeer 2006-02-01 00:55:23.0 -0500 +++ linux-2.6.16-rc1-cxzhang/security/selinux/hooks.c 2006-02-03 16:38:06.0 -0500 @@ -3358,24 +3358,38 @@ out: return err; } -static int selinux_socket_getpeersec(struct socket *sock, char __user *optval, - int __user *optlen, unsigned len) +static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *optval, + int __user *optlen, unsigned len) { int err = 0; char *scontext; u32 scontext_len; struct sk_security_struct *ssec; struct inode_security_struct *isec; + u32 peer_sid = 0; isec = SOCK_INODE(sock)-i_security; - if (isec-sclass != SECCLASS_UNIX_STREAM_SOCKET) { + + /* if UNIX_STREAM check peer_sid, if TCP check dst for labelled sa */ + if (isec-sclass == SECCLASS_UNIX_STREAM_SOCKET) { + ssec = sock-sk-sk_security; + peer_sid = ssec-peer_sid; + } + else if (isec-sclass == SECCLASS_TCP_SOCKET) { + peer_sid = selinux_socket_getpeer_stream(sock-sk); + + if (peer_sid == SECSID_NULL) { + err = -ENOPROTOOPT; + goto out; + } I am just wondering about something... Should a peer_sid of 0 or SECSID_NULL be an error here if the connection doesn't have a transform? I understand we only get peer's context if a xfrm is involved, but I am thinking most user applications may not kno or care if there is a xfrm. If not treated as an error, it looks like security_to_sid_context() would just return null for context. Would that be acceptable? Perhaps it is just important that we document the behaviour because I am thinking most user apps will not care or know if ipsec is running, so programmers may use this socket option to get peer context and may need to understand why they received an error of ENOPROTOOPT. } + else { err = -ENOPROTOOPT; goto out; } - ssec = sock-sk-sk_security; - - err = security_sid_to_context(ssec-peer_sid, scontext, scontext_len); + err = security_sid_to_context(peer_sid, scontext, scontext_len); + if (err) goto out; @@ -3396,6 +3410,23 @@ out: return err; } + Regards, Joy - 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