* Re: [PATCH]: Add security check before flushing SAD/SPD
@ 2007-03-26 19:39 Joy Latten
2007-03-26 20:05 ` James Morris
2007-03-26 20:34 ` Eric Paris
0 siblings, 2 replies; 26+ messages in thread
From: Joy Latten @ 2007-03-26 19:39 UTC (permalink / raw)
To: netdev; +Cc: davem, eparis, jmorris, selinux
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<latten@austin.ibm.com>
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.h 2007-03-23 11:01:48.000000000 -0500
+++ linux-2.6.20.patch/include/net/xfrm.h 2007-03-25 21:36:05.000000000 -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.000000000 -0500
+++ linux-2.6.20.patch/net/key/af_key.c 2007-03-25 21:34:35.000000000 -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.c 2007-03-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-25 21:32:51.000000000 -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,
+ &xfrm_policy_inexact[dir], bydst) {
+ if (pol->type != type)
+ continue;
+ if ((err = security_xfrm_policy_delete(pol)) != 0) {
+ xfrm_audit_log(audit_info->loginuid,
+ audit_info->secid,
+ AUDIT_MAC_IPSEC_DELSPD,
+ err ? 0 : 1, pol, NULL);
+ return err;
+ }
+ }
+ for (i = xfrm_policy_bydst[dir].hmask; i >= 0; i--) {
+ hlist_for_each_entry(pol, entry,
+ xfrm_policy_bydst[dir].table + i,
+ bydst) {
+ if (pol->type != type)
+ continue;
+ if ((err = security_xfrm_policy_delete(pol)) != 0) {
+ xfrm_audit_log(audit_info->loginuid,
+ audit_info->secid,
+ AUDIT_MAC_IPSEC_DELSPD,
+ err ? 0 : 1, pol, NULL);
+ return err;
+ }
+ }
+ }
+ }
+ return err;
+}
+#else
+static inline int
+xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
+{
+ int dir, err = 0;
write_lock_bh(&xfrm_policy_lock);
+
+ err = xfrm_policy_flush_secctx_check(type, audit_info);
+ if (err)
+ goto out;
+
for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
struct xfrm_policy *pol;
struct hlist_node *entry;
@@ -870,7 +924,9 @@ void xfrm_policy_flush(u8 type, struct x
xfrm_policy_count[dir] -= killed;
}
atomic_inc(&flow_cache_genid);
+out:
write_unlock_bh(&xfrm_policy_lock);
+ return err;
}
EXPORT_SYMBOL(xfrm_policy_flush);
@@ -2485,4 +2541,3 @@ restore_state:
}
EXPORT_SYMBOL(xfrm_migrate);
#endif
-
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-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_state.c 2007-03-25 21:33:02.000000000 -0500
@@ -388,12 +388,48 @@ int xfrm_state_delete(struct xfrm_state
}
EXPORT_SYMBOL(xfrm_state_delete);
-void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+#ifdef CONFIG_SECURITY_NETWORK_XFRM
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
{
- int i;
- int err = 0;
+ int i, err = 0;
+
+ for (i = 0; i <= xfrm_state_hmask; i++) {
+ struct hlist_node *entry;
+ struct xfrm_state *x;
+
+ hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
+ if (xfrm_id_proto_match(x->id.proto, proto) &&
+ (err = security_xfrm_state_delete(x)) != 0) {
+ xfrm_audit_log(audit_info->loginuid,
+ audit_info->secid,
+ AUDIT_MAC_IPSEC_DELSA,
+ err ? 0 : 1, NULL, x);
+
+ return err;
+ }
+ }
+ }
+
+ return err;
+}
+#else
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+{
+ int i, err = 0;
spin_lock_bh(&xfrm_state_lock);
+ err = xfrm_state_flush_secctx_check(proto, audit_info);
+ if (err)
+ goto out;
+
for (i = 0; i <= xfrm_state_hmask; i++) {
struct hlist_node *entry;
struct xfrm_state *x;
@@ -416,8 +452,12 @@ restart:
}
}
}
+ err = 0;
+
+out:
spin_unlock_bh(&xfrm_state_lock);
wake_up(&km_waitq);
+ return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_user.c linux-2.6.20.patch/net/xfrm/xfrm_user.c
--- linux-2.6.20.orig/net/xfrm/xfrm_user.c 2007-03-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_user.c 2007-03-25 21:33:52.000000000 -0500
@@ -1312,10 +1312,13 @@ static int xfrm_flush_sa(struct sk_buff
struct km_event c;
struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
struct xfrm_audit audit_info;
+ int err;
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_state_flush(p->proto, &audit_info);
+ err = xfrm_state_flush(p->proto, &audit_info);
+ if (err)
+ return err;
c.data.proto = p->proto;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
@@ -1476,7 +1479,9 @@ static int xfrm_flush_policy(struct sk_b
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_policy_flush(type, &audit_info);
+ err = xfrm_policy_flush(type, &audit_info);
+ if (err)
+ return err;
c.data.type = type;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-26 19:39 [PATCH]: Add security check before flushing SAD/SPD Joy Latten
@ 2007-03-26 20:05 ` James Morris
2007-03-26 20:14 ` James Morris
2007-03-26 20:34 ` Eric Paris
1 sibling, 1 reply; 26+ messages in thread
From: James Morris @ 2007-03-26 20:05 UTC (permalink / raw)
To: Joy Latten; +Cc: netdev, davem, eparis, selinux
On Mon, 26 Mar 2007, Joy Latten wrote:
> Signed-off-by: Joy Latten<latten@austin.ibm.com>
This looks ok to me, although I have a couple of minor issues (which
should probably not stop it being merged):
> + if ((err = security_xfrm_policy_delete(pol)) != 0) {
The value of 'err' is implicitly inverted several times in this function
(and similarly in the state flush one). Something like
ret = (fn() != 0);
might be better.
> + }
> + for (i = xfrm_policy_bydst[dir].hmask; i >= 0; i--) {
Tab damage?
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-26 20:05 ` James Morris
@ 2007-03-26 20:14 ` James Morris
0 siblings, 0 replies; 26+ messages in thread
From: James Morris @ 2007-03-26 20:14 UTC (permalink / raw)
To: Joy Latten; +Cc: netdev, davem, eparis, selinux
On Mon, 26 Mar 2007, James Morris wrote:
> On Mon, 26 Mar 2007, Joy Latten wrote:
>
> > Signed-off-by: Joy Latten<latten@austin.ibm.com>
>
> This looks ok to me, although I have a couple of minor issues (which
> should probably not stop it being merged):
>
> > + if ((err = security_xfrm_policy_delete(pol)) != 0) {
>
> The value of 'err' is implicitly inverted several times in this function
> (and similarly in the state flush one). Something like
>
> ret = (fn() != 0);
Oops, ignore the above.
The correct idiom is:
err = fn();
if (err) {
/* handle error */
}
Please use that, to reduce confusion :-)
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-26 19:39 [PATCH]: Add security check before flushing SAD/SPD Joy Latten
2007-03-26 20:05 ` James Morris
@ 2007-03-26 20:34 ` Eric Paris
1 sibling, 0 replies; 26+ messages in thread
From: Eric Paris @ 2007-03-26 20:34 UTC (permalink / raw)
To: Joy Latten; +Cc: netdev, davem, jmorris, selinux
On Mon, 2007-03-26 at 13:39 -0600, Joy Latten wrote:
> + if ((err = security_xfrm_policy_delete(pol)) != 0) {
> + xfrm_audit_log(audit_info->loginuid,
> + audit_info->secid,
> + AUDIT_MAC_IPSEC_DELSPD,
> + err ? 0 : 1, pol, NULL);
> + return err;
In all of the denial log statements you keep the "err ? 0 : 1" which are
common among audit, but in this patch we always know that err is 1. Is
it worth simplifying this down to just a 0 in the all of the
xfrm_audit_log calls?
-Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
@ 2007-03-26 23:21 Joy Latten
2007-03-27 3:08 ` James Morris
2007-06-04 19:05 ` Eric Paris
0 siblings, 2 replies; 26+ messages in thread
From: Joy Latten @ 2007-03-26 23:21 UTC (permalink / raw)
To: netdev; +Cc: davem, eparis, jmorris
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<latten@austin.ibm.com>
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.h 2007-03-23 11:01:48.000000000 -0500
+++ linux-2.6.20.patch/include/net/xfrm.h 2007-03-25 21:36:05.000000000 -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.000000000 -0500
+++ linux-2.6.20.patch/net/key/af_key.c 2007-03-25 21:34:35.000000000 -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.c 2007-03-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-26 17:19:26.000000000 -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,
+ xfrm_policy_bydst[dir].table + i,
+ 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;
+ }
+ }
+ }
+ }
+ return err;
+}
+#else
+static inline int
+xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
+{
+ int dir, err = 0;
write_lock_bh(&xfrm_policy_lock);
+
+ err = xfrm_policy_flush_secctx_check(type, audit_info);
+ if (err)
+ goto out;
+
for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
struct xfrm_policy *pol;
struct hlist_node *entry;
@@ -870,7 +926,9 @@ void xfrm_policy_flush(u8 type, struct x
xfrm_policy_count[dir] -= killed;
}
atomic_inc(&flow_cache_genid);
+out:
write_unlock_bh(&xfrm_policy_lock);
+ return err;
}
EXPORT_SYMBOL(xfrm_policy_flush);
@@ -2485,4 +2543,3 @@ restore_state:
}
EXPORT_SYMBOL(xfrm_migrate);
#endif
-
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-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_state.c 2007-03-26 17:19:56.000000000 -0500
@@ -388,12 +388,48 @@ int xfrm_state_delete(struct xfrm_state
}
EXPORT_SYMBOL(xfrm_state_delete);
-void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+#ifdef CONFIG_SECURITY_NETWORK_XFRM
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
{
- int i;
- int err = 0;
+ int i, err = 0;
+
+ for (i = 0; i <= xfrm_state_hmask; i++) {
+ struct hlist_node *entry;
+ struct xfrm_state *x;
+
+ hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
+ if (xfrm_id_proto_match(x->id.proto, proto) &&
+ (err = security_xfrm_state_delete(x)) != 0) {
+ xfrm_audit_log(audit_info->loginuid,
+ audit_info->secid,
+ AUDIT_MAC_IPSEC_DELSA,
+ 0, NULL, x);
+
+ return err;
+ }
+ }
+ }
+
+ return err;
+}
+#else
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+{
+ int i, err = 0;
spin_lock_bh(&xfrm_state_lock);
+ err = xfrm_state_flush_secctx_check(proto, audit_info);
+ if (err)
+ goto out;
+
for (i = 0; i <= xfrm_state_hmask; i++) {
struct hlist_node *entry;
struct xfrm_state *x;
@@ -416,8 +452,12 @@ restart:
}
}
}
+ err = 0;
+
+out:
spin_unlock_bh(&xfrm_state_lock);
wake_up(&km_waitq);
+ return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_user.c linux-2.6.20.patch/net/xfrm/xfrm_user.c
--- linux-2.6.20.orig/net/xfrm/xfrm_user.c 2007-03-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_user.c 2007-03-25 21:33:52.000000000 -0500
@@ -1312,10 +1312,13 @@ static int xfrm_flush_sa(struct sk_buff
struct km_event c;
struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
struct xfrm_audit audit_info;
+ int err;
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_state_flush(p->proto, &audit_info);
+ err = xfrm_state_flush(p->proto, &audit_info);
+ if (err)
+ return err;
c.data.proto = p->proto;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
@@ -1476,7 +1479,9 @@ static int xfrm_flush_policy(struct sk_b
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_policy_flush(type, &audit_info);
+ err = xfrm_policy_flush(type, &audit_info);
+ if (err)
+ return err;
c.data.type = type;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-26 23:21 Joy Latten
@ 2007-03-27 3:08 ` James Morris
2007-06-04 19:05 ` Eric Paris
1 sibling, 0 replies; 26+ messages in thread
From: James Morris @ 2007-03-27 3:08 UTC (permalink / raw)
To: Joy Latten; +Cc: netdev, davem, eparis
On Mon, 26 Mar 2007, Joy Latten wrote:
>
> Sending again since one of the email addresses was incorrect.
>
> ----------------
>
> Ok, I have made improvements based on James' and Eric's comments.
>
Acked-by: James Morris <jmorris@namei.org>
> + if (xfrm_id_proto_match(x->id.proto, proto) &&
> + (err = security_xfrm_state_delete(x)) != 0) {
Still not a showstopper, but standard idiom would be nice at some point.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-26 23:21 Joy Latten
2007-03-27 3:08 ` James Morris
@ 2007-06-04 19:05 ` Eric Paris
2007-06-04 19:44 ` James Morris
1 sibling, 1 reply; 26+ messages in thread
From: Eric Paris @ 2007-06-04 19:05 UTC (permalink / raw)
To: netdev; +Cc: davem, jmorris, selinux, Joy Latten
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?
-Eric
Currently we check for permission before deleting entries from SAD and
SPD, (see security_xfrm_policy_delete() security_xfrm_state_delete())
However we are not checking for authorization when flushing the SPD and
the SAD completely. It was perhaps missed in the original security hooks
patch.
This patch adds a security check when flushing entries from the SAD and
SPD. It runs the entire database and checks each entry for a denial.
If the process attempting the flush is unable to remove all of the
entries a denial is logged the the flush function returns an error
without removing anything.
This is particularly useful when a process may need to create or delete
its own xfrm entries used for things like labeled networking but that
same process should not be able to delete other entries or flush the
entire database.
WAS Signed-off-by: Signed-off-by: Joy Latten<latten@austin.ibm.com> NOT NOW
WAS Acked-by: James Morris <jmorris@namei.org> NOT NOW
WAS Acked-by: Eric Paris <eparis@parisplace.org> NOT NOW
---
include/net/xfrm.h | 6 ++--
net/key/af_key.c | 10 ++++++-
net/xfrm/xfrm_policy.c | 63 +++++++++++++++++++++++++++++++++++++++++++++--
net/xfrm/xfrm_state.c | 46 ++++++++++++++++++++++++++++++++--
net/xfrm/xfrm_user.c | 9 +++++-
5 files changed, 121 insertions(+), 13 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 90185e8..311f25a 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -964,7 +964,7 @@ struct xfrmk_spdinfo {
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 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);
@@ -1020,13 +1020,13 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
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 --git a/net/key/af_key.c b/net/key/af_key.c
index d302dda..0f8304b 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1682,6 +1682,7 @@ static int pfkey_flush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hd
unsigned proto;
struct km_event c;
struct xfrm_audit audit_info;
+ int err;
proto = pfkey_satype2proto(hdr->sadb_msg_satype);
if (proto == 0)
@@ -1689,7 +1690,9 @@ static int pfkey_flush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hd
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;
@@ -2683,10 +2686,13 @@ static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct sadb_msg
{
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 --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 64a3751..157bfbd 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -834,11 +834,67 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
}
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,
+ xfrm_policy_bydst[dir].table + i,
+ 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;
+ }
+ }
+ }
+ }
+ return err;
+}
+#else
+static inline int
+xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
+{
+ int dir, err = 0;
write_lock_bh(&xfrm_policy_lock);
+
+ err = xfrm_policy_flush_secctx_check(type, audit_info);
+ if (err)
+ goto out;
+
for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
struct xfrm_policy *pol;
struct hlist_node *entry;
@@ -891,7 +947,9 @@ void xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
xfrm_policy_count[dir] -= killed;
}
atomic_inc(&flow_cache_genid);
+out:
write_unlock_bh(&xfrm_policy_lock);
+ return err;
}
EXPORT_SYMBOL(xfrm_policy_flush);
@@ -2583,4 +2641,3 @@ restore_state:
}
EXPORT_SYMBOL(xfrm_migrate);
#endif
-
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 372f06e..2cc9d87 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -391,12 +391,48 @@ int xfrm_state_delete(struct xfrm_state *x)
}
EXPORT_SYMBOL(xfrm_state_delete);
-void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+#ifdef CONFIG_SECURITY_NETWORK_XFRM
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
{
- int i;
- int err = 0;
+ int i, err = 0;
+
+ for (i = 0; i <= xfrm_state_hmask; i++) {
+ struct hlist_node *entry;
+ struct xfrm_state *x;
+
+ hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
+ if (xfrm_id_proto_match(x->id.proto, proto) &&
+ (err = security_xfrm_state_delete(x)) != 0) {
+ xfrm_audit_log(audit_info->loginuid,
+ audit_info->secid,
+ AUDIT_MAC_IPSEC_DELSA,
+ 0, NULL, x);
+
+ return err;
+ }
+ }
+ }
+
+ return err;
+}
+#else
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+{
+ int i, err = 0;
spin_lock_bh(&xfrm_state_lock);
+ err = xfrm_state_flush_secctx_check(proto, audit_info);
+ if (err)
+ goto out;
+
for (i = 0; i <= xfrm_state_hmask; i++) {
struct hlist_node *entry;
struct xfrm_state *x;
@@ -419,8 +455,12 @@ restart:
}
}
}
+ err = 0;
+
+out:
spin_unlock_bh(&xfrm_state_lock);
wake_up(&km_waitq);
+ return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b14c7e5..c06883b 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1418,10 +1418,13 @@ static int xfrm_flush_sa(struct sk_buff *skb, struct nlmsghdr *nlh,
struct km_event c;
struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
struct xfrm_audit audit_info;
+ int err;
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_state_flush(p->proto, &audit_info);
+ err = xfrm_state_flush(p->proto, &audit_info);
+ if (err)
+ return err;
c.data.proto = p->proto;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
@@ -1582,7 +1585,9 @@ static int xfrm_flush_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_policy_flush(type, &audit_info);
+ err = xfrm_policy_flush(type, &audit_info);
+ if (err)
+ return err;
c.data.type = type;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-06-04 19:05 ` Eric Paris
@ 2007-06-04 19:44 ` James Morris
2007-06-04 23:13 ` James Morris
2007-06-05 17:56 ` Joy Latten
0 siblings, 2 replies; 26+ messages in thread
From: James Morris @ 2007-06-04 19:44 UTC (permalink / raw)
To: Eric Paris; +Cc: netdev, davem, selinux, Joy Latten
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 <jmorris@namei.org>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-06-04 19:44 ` James Morris
@ 2007-06-04 23:13 ` James Morris
2007-06-04 23:55 ` David Miller
2007-06-05 17:56 ` Joy Latten
1 sibling, 1 reply; 26+ messages in thread
From: James Morris @ 2007-06-04 23:13 UTC (permalink / raw)
To: Eric Paris, David S. Miller; +Cc: netdev, selinux, Joy Latten
I've applied this patch to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-davem
Dave, feel free to pull from that branch.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-06-04 23:13 ` James Morris
@ 2007-06-04 23:55 ` David Miller
0 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2007-06-04 23:55 UTC (permalink / raw)
To: jmorris; +Cc: eparis, netdev, selinux, latten
From: James Morris <jmorris@namei.org>
Date: Mon, 4 Jun 2007 19:13:10 -0400 (EDT)
> I've applied this patch to
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-davem
>
>
> Dave, feel free to pull from that branch.
Thanks James, I'll pull that in for my next round of
networking fixes.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-06-04 19:44 ` James Morris
2007-06-04 23:13 ` James Morris
@ 2007-06-05 17:56 ` Joy Latten
1 sibling, 0 replies; 26+ messages in thread
From: Joy Latten @ 2007-06-05 17:56 UTC (permalink / raw)
To: James Morris; +Cc: Eric Paris, netdev, davem, selinux
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 <jmorris@namei.org>
>
>
I have also tested with 2.6.22-rc3-git7 and all appears to be working as
expected.
Acked-by: Joy Latten <latten@austin.ibm.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
@ 2007-03-26 22:58 Joy Latten
0 siblings, 0 replies; 26+ messages in thread
From: Joy Latten @ 2007-03-26 22:58 UTC (permalink / raw)
To: netdev; +Cc: davemloft, eparis, jmorris, selinux
I have made improvements based on James' and Eric's comments.
Regards,
Joy
Signed-off-by: Joy Latten<latten@austin.ibm.com>
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.h 2007-03-23 11:01:48.000000000 -0500
+++ linux-2.6.20.patch/include/net/xfrm.h 2007-03-25 21:36:05.000000000 -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.000000000 -0500
+++ linux-2.6.20.patch/net/key/af_key.c 2007-03-25 21:34:35.000000000 -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.c 2007-03-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_policy.c 2007-03-26 17:19:26.000000000 -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,
+ xfrm_policy_bydst[dir].table + i,
+ 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;
+ }
+ }
+ }
+ }
+ return err;
+}
+#else
+static inline int
+xfrm_policy_flush_secctx_check(u8 type, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
+{
+ int dir, err = 0;
write_lock_bh(&xfrm_policy_lock);
+
+ err = xfrm_policy_flush_secctx_check(type, audit_info);
+ if (err)
+ goto out;
+
for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
struct xfrm_policy *pol;
struct hlist_node *entry;
@@ -870,7 +926,9 @@ void xfrm_policy_flush(u8 type, struct x
xfrm_policy_count[dir] -= killed;
}
atomic_inc(&flow_cache_genid);
+out:
write_unlock_bh(&xfrm_policy_lock);
+ return err;
}
EXPORT_SYMBOL(xfrm_policy_flush);
@@ -2485,4 +2543,3 @@ restore_state:
}
EXPORT_SYMBOL(xfrm_migrate);
#endif
-
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-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_state.c 2007-03-26 17:19:56.000000000 -0500
@@ -388,12 +388,48 @@ int xfrm_state_delete(struct xfrm_state
}
EXPORT_SYMBOL(xfrm_state_delete);
-void xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+#ifdef CONFIG_SECURITY_NETWORK_XFRM
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
{
- int i;
- int err = 0;
+ int i, err = 0;
+
+ for (i = 0; i <= xfrm_state_hmask; i++) {
+ struct hlist_node *entry;
+ struct xfrm_state *x;
+
+ hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
+ if (xfrm_id_proto_match(x->id.proto, proto) &&
+ (err = security_xfrm_state_delete(x)) != 0) {
+ xfrm_audit_log(audit_info->loginuid,
+ audit_info->secid,
+ AUDIT_MAC_IPSEC_DELSA,
+ 0, NULL, x);
+
+ return err;
+ }
+ }
+ }
+
+ return err;
+}
+#else
+static inline int
+xfrm_state_flush_secctx_check(u8 proto, struct xfrm_audit *audit_info)
+{
+ return 0;
+}
+#endif
+
+int xfrm_state_flush(u8 proto, struct xfrm_audit *audit_info)
+{
+ int i, err = 0;
spin_lock_bh(&xfrm_state_lock);
+ err = xfrm_state_flush_secctx_check(proto, audit_info);
+ if (err)
+ goto out;
+
for (i = 0; i <= xfrm_state_hmask; i++) {
struct hlist_node *entry;
struct xfrm_state *x;
@@ -416,8 +452,12 @@ restart:
}
}
}
+ err = 0;
+
+out:
spin_unlock_bh(&xfrm_state_lock);
wake_up(&km_waitq);
+ return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
diff -urpN linux-2.6.20.orig/net/xfrm/xfrm_user.c linux-2.6.20.patch/net/xfrm/xfrm_user.c
--- linux-2.6.20.orig/net/xfrm/xfrm_user.c 2007-03-23 11:02:46.000000000 -0500
+++ linux-2.6.20.patch/net/xfrm/xfrm_user.c 2007-03-25 21:33:52.000000000 -0500
@@ -1312,10 +1312,13 @@ static int xfrm_flush_sa(struct sk_buff
struct km_event c;
struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
struct xfrm_audit audit_info;
+ int err;
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_state_flush(p->proto, &audit_info);
+ err = xfrm_state_flush(p->proto, &audit_info);
+ if (err)
+ return err;
c.data.proto = p->proto;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
@@ -1476,7 +1479,9 @@ static int xfrm_flush_policy(struct sk_b
audit_info.loginuid = NETLINK_CB(skb).loginuid;
audit_info.secid = NETLINK_CB(skb).sid;
- xfrm_policy_flush(type, &audit_info);
+ err = xfrm_policy_flush(type, &audit_info);
+ if (err)
+ return err;
c.data.type = type;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH]: Add security check before flushing SAD/SPD
@ 2007-03-22 18:35 Joy Latten
2007-03-22 19:01 ` David Miller
0 siblings, 1 reply; 26+ messages in thread
From: Joy Latten @ 2007-03-22 18:35 UTC (permalink / raw)
To: netdev; +Cc: davem, jmorris, vyekkirala
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<latten@austin.ibm.com>
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.c 2007-03-21 14:25:51.000000000 -0500
+++ linux-2.6.20/net/xfrm/xfrm_policy.c 2007-03-21 14:30:59.000000000 -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.000000000 -0500
+++ linux-2.6.20/net/xfrm/xfrm_state.c 2007-03-21 14:27:48.000000000 -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);
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-22 18:35 Joy Latten
@ 2007-03-22 19:01 ` David Miller
2007-03-22 21:23 ` Joy Latten
0 siblings, 1 reply; 26+ messages in thread
From: David Miller @ 2007-03-22 19:01 UTC (permalink / raw)
To: latten; +Cc: netdev, jmorris, vyekkirala
From: Joy Latten <latten@austin.ibm.com>
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<latten@austin.ibm.com>
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.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-22 19:01 ` David Miller
@ 2007-03-22 21:23 ` Joy Latten
2007-03-22 23:49 ` James Morris
0 siblings, 1 reply; 26+ messages in thread
From: Joy Latten @ 2007-03-22 21:23 UTC (permalink / raw)
To: David Miller; +Cc: selinux, netdev, jmorris, vyekkirala
On Thu, 2007-03-22 at 12:01 -0700, David Miller wrote:
> From: Joy Latten <latten@austin.ibm.com>
> 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<latten@austin.ibm.com>
>
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-22 21:23 ` Joy Latten
@ 2007-03-22 23:49 ` James Morris
2007-03-22 23:50 ` Joy Latten
2007-03-23 5:39 ` Eric Paris
0 siblings, 2 replies; 26+ messages in thread
From: James Morris @ 2007-03-22 23:49 UTC (permalink / raw)
To: Joy Latten; +Cc: David Miller, selinux, netdev, vyekkirala
On Thu, 22 Mar 2007, Joy Latten wrote:
> > 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.
Indeed. Removing an entry is modifying MAC policy, which requires
appropriate authorization.
The security label is encapsulated with the object, which is why it's
passed to the security layer.
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.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-22 23:49 ` James Morris
@ 2007-03-22 23:50 ` Joy Latten
2007-03-23 0:56 ` James Morris
2007-03-23 5:39 ` Eric Paris
1 sibling, 1 reply; 26+ messages in thread
From: Joy Latten @ 2007-03-22 23:50 UTC (permalink / raw)
To: James Morris; +Cc: David Miller, selinux, netdev, vyekkirala
On Thu, 2007-03-22 at 19:49 -0400, James Morris wrote:
> On Thu, 22 Mar 2007, Joy Latten wrote:
>
> > > 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.
>
> Indeed. Removing an entry is modifying MAC policy, which requires
> appropriate authorization.
>
> The security label is encapsulated with the object, which is why it's
> passed to the security layer.
>
> 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.
Joy
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-22 23:50 ` Joy Latten
@ 2007-03-23 0:56 ` James Morris
0 siblings, 0 replies; 26+ messages in thread
From: James Morris @ 2007-03-23 0:56 UTC (permalink / raw)
To: Joy Latten; +Cc: David Miller, selinux, netdev, vyekkirala
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.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-22 23:49 ` James Morris
2007-03-22 23:50 ` Joy Latten
@ 2007-03-23 5:39 ` Eric Paris
2007-03-23 6:22 ` David Miller
` (2 more replies)
1 sibling, 3 replies; 26+ messages in thread
From: Eric Paris @ 2007-03-23 5:39 UTC (permalink / raw)
To: James Morris; +Cc: Joy Latten, David Miller, selinux, netdev, vyekkirala
On Thu, 2007-03-22 at 19:49 -0400, James Morris wrote:
> On Thu, 22 Mar 2007, Joy Latten wrote:
>
> > > 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.
>
> Indeed. Removing an entry is modifying MAC policy, which requires
> appropriate authorization.
>
> The security label is encapsulated with the object, which is why it's
> passed to the security layer.
>
> 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.
Maybe I'm way out on a limb here but if I am a regular user and I say
rm /tmp/* and I only have permissions to delete some of the files I
expect just those couple to be delete, not the whole operation denied.
It seems reasonable to me that the check for every policy (which is
between current->security->sid and xp->security->ctx_sid) makes sense.
There doesn't appear to me right offhand to be anything intrinsic in the
code which says that a flush request must flush everything or nothing.
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?
-Eric
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 5:39 ` Eric Paris
@ 2007-03-23 6:22 ` David Miller
2007-03-23 16:33 ` Joy Latten
2007-03-23 18:46 ` James Morris
2 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2007-03-23 6:22 UTC (permalink / raw)
To: eparis; +Cc: jmorris, latten, selinux, netdev, vyekkirala
From: Eric Paris <eparis@parisplace.org>
Date: Fri, 23 Mar 2007 01:39:47 -0400
> It seems reasonable to me that the check for every policy (which is
> between current->security->sid and xp->security->ctx_sid) makes sense.
> There doesn't appear to me right offhand to be anything intrinsic in the
> code which says that a flush request must flush everything or nothing.
The "intrinsic" part comes from the heritage of routing daemons and
what they expect from flush operations, these assumption live in the
ipsec daemons as well.
A daemon, routing or ipsec, is flushing the tables in order to create
an entirely "clean slate", and they very much expect the policy and
state tables to be %100 empty after the operation.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 5:39 ` Eric Paris
2007-03-23 6:22 ` David Miller
@ 2007-03-23 16:33 ` Joy Latten
2007-03-23 16:59 ` Eric Paris
2007-03-23 18:46 ` James Morris
2 siblings, 1 reply; 26+ messages in thread
From: Joy Latten @ 2007-03-23 16:33 UTC (permalink / raw)
To: Eric Paris; +Cc: James Morris, David Miller, selinux, netdev, vyekkirala
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 16:33 ` Joy Latten
@ 2007-03-23 16:59 ` Eric Paris
2007-03-23 18:50 ` Joy Latten
0 siblings, 1 reply; 26+ messages in thread
From: Eric Paris @ 2007-03-23 16:59 UTC (permalink / raw)
To: Joy Latten; +Cc: James Morris, David Miller, selinux, netdev, vyekkirala
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?
-Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 16:59 ` Eric Paris
@ 2007-03-23 18:50 ` Joy Latten
0 siblings, 0 replies; 26+ messages in thread
From: Joy Latten @ 2007-03-23 18:50 UTC (permalink / raw)
To: Eric Paris; +Cc: James Morris, David Miller, selinux, netdev, vyekkirala
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 5:39 ` Eric Paris
2007-03-23 6:22 ` David Miller
2007-03-23 16:33 ` Joy Latten
@ 2007-03-23 18:46 ` James Morris
2007-03-23 18:47 ` David Miller
2 siblings, 1 reply; 26+ messages in thread
From: James Morris @ 2007-03-23 18:46 UTC (permalink / raw)
To: Eric Paris; +Cc: Joy Latten, David Miller, selinux, netdev, vyekkirala
On Fri, 23 Mar 2007, Eric Paris wrote:
> Maybe I'm way out on a limb here but if I am a regular user and I say
> rm /tmp/* and I only have permissions to delete some of the files I
> expect just those couple to be delete, not the whole operation denied.
I don't think this analogy holds up, as rm is a per-file deletion
operation, and it is the shell which expands the wildcard for you.
A 'flush' has a semantic implication that all entries will be removed, and
it should be atomic and either succeed or fail at that granularity.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 18:46 ` James Morris
@ 2007-03-23 18:47 ` David Miller
2007-03-23 18:50 ` Eric Paris
0 siblings, 1 reply; 26+ messages in thread
From: David Miller @ 2007-03-23 18:47 UTC (permalink / raw)
To: jmorris; +Cc: eparis, latten, selinux, netdev, vyekkirala
From: James Morris <jmorris@namei.org>
Date: Fri, 23 Mar 2007 14:46:48 -0400 (EDT)
> A 'flush' has a semantic implication that all entries will be removed, and
> it should be atomic and either succeed or fail at that granularity.
Correct.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH]: Add security check before flushing SAD/SPD
2007-03-23 18:47 ` David Miller
@ 2007-03-23 18:50 ` Eric Paris
0 siblings, 0 replies; 26+ messages in thread
From: Eric Paris @ 2007-03-23 18:50 UTC (permalink / raw)
To: David Miller; +Cc: jmorris, latten, selinux, netdev, vyekkirala
On Fri, 2007-03-23 at 11:47 -0700, David Miller wrote:
> From: James Morris <jmorris@namei.org>
> Date: Fri, 23 Mar 2007 14:46:48 -0400 (EDT)
>
> > A 'flush' has a semantic implication that all entries will be removed, and
> > it should be atomic and either succeed or fail at that granularity.
>
> Correct.
Fair enough, does it matter that we have no way to report failure back
to users who can no longer assume success?
-Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2007-06-05 18:14 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-26 19:39 [PATCH]: Add security check before flushing SAD/SPD Joy Latten
2007-03-26 20:05 ` James Morris
2007-03-26 20:14 ` James Morris
2007-03-26 20:34 ` Eric Paris
-- strict thread matches above, loose matches on Subject: below --
2007-03-26 23:21 Joy Latten
2007-03-27 3:08 ` James Morris
2007-06-04 19:05 ` Eric Paris
2007-06-04 19:44 ` James Morris
2007-06-04 23:13 ` James Morris
2007-06-04 23:55 ` David Miller
2007-06-05 17:56 ` Joy Latten
2007-03-26 22:58 Joy Latten
2007-03-22 18:35 Joy Latten
2007-03-22 19:01 ` David Miller
2007-03-22 21:23 ` Joy Latten
2007-03-22 23:49 ` James Morris
2007-03-22 23:50 ` Joy Latten
2007-03-23 0:56 ` James Morris
2007-03-23 5:39 ` Eric Paris
2007-03-23 6:22 ` David Miller
2007-03-23 16:33 ` Joy Latten
2007-03-23 16:59 ` Eric Paris
2007-03-23 18:50 ` Joy Latten
2007-03-23 18:46 ` James Morris
2007-03-23 18:47 ` David Miller
2007-03-23 18:50 ` Eric Paris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).