From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Grubb Subject: Re: [PATCH 1/1]:reworked ipsec audit Date: Thu, 16 Nov 2006 11:38:56 -0500 Message-ID: <200611161138.56285.sgrubb@redhat.com> References: <200611102140.kAALeMQM030559@faith.austin.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <200611102140.kAALeMQM030559@faith.austin.ibm.com> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Joy Latten Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com On Friday 10 November 2006 16:40, Joy Latten wrote: > diff -urpN linux-2.6.18.ppc64.orig/include/net/xfrm.h > linux-2.6.18.ppc64.patch/include/net/xfrm.h --- > linux-2.6.18.ppc64.orig/include/net/xfrm.h 2006-11-02 09:17:55.000000000 > -0600 +++ linux-2.6.18.ppc64.patch/include/net/xfrm.h 2006-11-05 > 20:16:05.000000000 -0600 @@ -371,9 +371,17 @@ struct xfrm_mgr > extern int xfrm_register_km(struct xfrm_mgr *km); > extern int xfrm_unregister_km(struct xfrm_mgr *km); > > - > extern struct xfrm_policy *xfrm_policy_list[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)) > @@ -904,7 +912,7 @@ extern int xfrm_state_update(struct xfrm > extern struct xfrm_state *xfrm_state_lookup(xfrm_address_t *daddr, u32 > spi, u8 proto, unsigned short family); 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); This seems to indicate passing the structure by value...did you mean to do it that way or by pointer? > @@ -952,13 +960,12 @@ int xfrm_policy_insert(int dir, struct x > struct xfrm_policy *xfrm_policy_bysel_ctx(int dir, struct xfrm_selector > *sel, struct xfrm_sec_ctx *ctx, int delete); > struct xfrm_policy *xfrm_policy_byid(int dir, u32 id, int delete); > -void xfrm_policy_flush(void); > u32 xfrm_get_acqseq(void); > void xfrm_alloc_spi(struct xfrm_state *x, u32 minspi, u32 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(void); > +extern void xfrm_policy_flush(struct xfrm_audit audit_info); Another pass by value... > diff -urpN linux-2.6.18.ppc64.orig/net/key/af_key.c > linux-2.6.18.ppc64.patch/net/key/af_key.c --- > linux-2.6.18.ppc64.orig/net/key/af_key.c 2006-11-02 09:16:11.000000000 > -0600 +++ linux-2.6.18.ppc64.patch/net/key/af_key.c 2006-11-08 > @@ -1637,12 +1645,14 @@ static int pfkey_flush(struct sock *sk, > { > unsigned proto; > struct km_event c; > + struct xfrm_audit audit_info; > > proto = pfkey_satype2proto(hdr->sadb_msg_satype); > if (proto == 0) > return -EINVAL; > > - xfrm_state_flush(proto); > + audit_info.loginuid = audit_get_loginuid(current->audit_context); > + xfrm_state_flush(proto, audit_info); So, audit_info.secid is unset at this point.... > @@ -2404,8 +2420,10 @@ static int key_notify_policy_flush(struc > static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct > sadb_msg *hdr, void **ext_hdrs) { > struct km_event c; > + struct xfrm_audit audit_info; > > - xfrm_policy_flush(); > + audit_info.loginuid = audit_get_loginuid(current->audit_context); > + xfrm_policy_flush(audit_info); audit_info.secid is unset > diff -urpN linux-2.6.18.ppc64.orig/net/xfrm/xfrm_policy.c > linux-2.6.18.ppc64.patch/net/xfrm/xfrm_policy.c --- > linux-2.6.18.ppc64.orig/net/xfrm/xfrm_policy.c 2006-11-02 > 09:16:10.000000000 -0600 +++ > linux-2.6.18.ppc64.patch/net/xfrm/xfrm_policy.c 2006-11-08 > @@ -1366,6 +1369,106 @@ int xfrm_bundle_ok(struct xfrm_policy *p > > EXPORT_SYMBOL(xfrm_bundle_ok); > > +/* Audit addition and deletion of SAs and ipsec policy */ > + > +void xfrm_audit_log(uid_t auid, u32 sid, int type, int result, > + struct xfrm_policy *xp, struct xfrm_state *x) > +{ > + > + char *secctx; > + u32 secctx_len; > + struct xfrm_sec_ctx *sctx = NULL; > + struct in6_addr saddr6, daddr6; > + struct in_addr saddr, daddr; One of these two will be wasted because it will be either IPv4 or 6 but never both. I'd consider declaring these within the switch/case block that actually uses it or maybe use the non-specific struct sockaddr_storage. >+ struct audit_buffer *audit_buf; >+ int family; >+ >+ >+ audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC, type); >+ if (audit_buf == NULL) >+ return; >+ Also, before calling audit_log_start...you need to see if audit is enabled. You should declare an "extern int audit_enabled;" within this function and do a "if (audit_enabled == 0) return;". -Steve