* RE: [PATCH 1/1] selinux: secid reconciliation fixes V02
@ 2006-10-09 16:01 Venkat Yekkirala
2006-10-09 16:12 ` Paul Moore
0 siblings, 1 reply; 6+ messages in thread
From: Venkat Yekkirala @ 2006-10-09 16:01 UTC (permalink / raw)
To: Paul Moore, Venkat Yekkirala
Cc: selinux, jmorris, sds, eparis, jbrindle, redhat-lspp
> Venkat Yekkirala wrote:
> > --- net-2.6.sid6/include/linux/security.h 2006-10-05
> 12:03:39.000000000 -0500
> > +++ net-2.6/include/linux/security.h 2006-10-08
> 14:10:49.000000000 -0500
> > @@ -67,6 +67,7 @@ struct xfrm_selector;
> > struct xfrm_policy;
> > struct xfrm_state;
> > struct xfrm_user_sec_ctx;
> > +struct net_device;
> >
> > extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
> > extern int cap_netlink_recv(struct sk_buff *skb, int cap);
> > @@ -828,8 +829,8 @@ struct request_sock;
> > * Sets the new child socket's sid to the openreq sid.
> > * @inet_conn_established:
> > * Sets the connection's peersid to the secmark on skb.
> > - * @req_classify_flow:
> > - * Sets the flow's sid to the openreq sid.
> > + * @igmp_classify_skb:
> > + * Classifies an skb representing an igmp packet.
>
> I wonder if it might be cleaner to have a generic
> classify_skb() function? That
> seems to be more inline with what James commented on earlier
> and I'm almost
> certain the netdev crowd would be much more open to a generic
> hook. It
> shouldn't be too expensive to check if the packet is an IGMP
> packet inside the hook.
It wouldn't necessarily be clean in that you would have to put
all the logic to determine what kind of packet it is inside the
hook, if it's even possible to do it under all circumstances.
>
> > * @skb_flow_in:
> > * Checks to see if security policy would allow skb into the system
> > * while also reconciling the xfrm secid, cipso, etc, if any, and
> > @@ -1385,9 +1386,10 @@ struct security_operations {
> > struct request_sock *req);
> > void (*inet_csk_clone)(struct sock *newsk, const struct
> request_sock *req);
> > void (*inet_conn_established)(struct sock *sk, struct
> sk_buff *skb);
> > - void (*req_classify_flow)(const struct request_sock
> *req, struct flowi *fl);
> > + void (*igmp_classify_skb)(struct sk_buff *skb);
> > int (*skb_flow_in)(struct sk_buff *skb, unsigned short family);
> > - int (*skb_flow_out)(struct sk_buff *skb, u32 nf_secid);
> > + int (*skb_flow_out)(struct sk_buff *skb, u32 nf_secid,
> > + const struct net_device *out, unsigned
> short family);
> > #endif /* CONFIG_SECURITY_NETWORK */
> >
> > #ifdef CONFIG_SECURITY_NETWORK_XFRM
> > @@ -2953,14 +2955,20 @@ static inline void security_sk_clone(con
> > return security_ops->sk_clone_security(sk, newsk);
> > }
> >
> > +/*static inline void security_sk_classify_ipcm(struct sock *sk,
> > + struct ipcm_cookie *ipc)
> > +{
> > + security_ops->sk_getsecid(sk, &ipc->secid);
> > +}*/
> > +
>
> If this really isn't needed shouldn't we just remove the code
> altogether instead
> of commenting it out?
Yes. If you could take it out, I would appreciate it. (let me know if
you want a new patch with this taken out).
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] selinux: secid reconciliation fixes V02
2006-10-09 16:01 [PATCH 1/1] selinux: secid reconciliation fixes V02 Venkat Yekkirala
@ 2006-10-09 16:12 ` Paul Moore
0 siblings, 0 replies; 6+ messages in thread
From: Paul Moore @ 2006-10-09 16:12 UTC (permalink / raw)
To: Venkat Yekkirala; +Cc: selinux, jmorris, sds, eparis, jbrindle, redhat-lspp
Venkat Yekkirala wrote:
>>Venkat Yekkirala wrote:
>>
>>>--- net-2.6.sid6/include/linux/security.h 2006-10-05
>>
>>12:03:39.000000000 -0500
>>
>>>+++ net-2.6/include/linux/security.h 2006-10-08
>>
>>14:10:49.000000000 -0500
>>
>>>@@ -67,6 +67,7 @@ struct xfrm_selector;
>>> struct xfrm_policy;
>>> struct xfrm_state;
>>> struct xfrm_user_sec_ctx;
>>>+struct net_device;
>>>
>>> extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
>>> extern int cap_netlink_recv(struct sk_buff *skb, int cap);
>>>@@ -828,8 +829,8 @@ struct request_sock;
>>> * Sets the new child socket's sid to the openreq sid.
>>> * @inet_conn_established:
>>> * Sets the connection's peersid to the secmark on skb.
>>>- * @req_classify_flow:
>>>- * Sets the flow's sid to the openreq sid.
>>>+ * @igmp_classify_skb:
>>>+ * Classifies an skb representing an igmp packet.
>>
>>I wonder if it might be cleaner to have a generic
>>classify_skb() function? That
>>seems to be more inline with what James commented on earlier
>>and I'm almost
>>certain the netdev crowd would be much more open to a generic
>>hook. It
>>shouldn't be too expensive to check if the packet is an IGMP
>>packet inside the hook.
>
>
> It wouldn't necessarily be clean in that you would have to put
> all the logic to determine what kind of packet it is inside the
> hook, if it's even possible to do it under all circumstances.
Yes, but the *interface* would be cleaner which was the point I was trying to
make. There have been a lot of new LSM networking hooks introduced lately and I
thought it might be worthwhile to keep the interfaces as flexibile as possibile.
I guess we can always change it later if needed.
>>>+/*static inline void security_sk_classify_ipcm(struct sock *sk,
>>>+ struct ipcm_cookie *ipc)
>>>+{
>>>+ security_ops->sk_getsecid(sk, &ipc->secid);
>>>+}*/
>>>+
>>
>>If this really isn't needed shouldn't we just remove the code
>>altogether instead
>>of commenting it out?
>
> Yes. If you could take it out, I would appreciate it. (let me know if
> you want a new patch with this taken out).
It's probably quickest if I just drop it.
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/1] selinux: secid reconciliation fixes V02
@ 2006-10-09 16:51 Venkat Yekkirala
0 siblings, 0 replies; 6+ messages in thread
From: Venkat Yekkirala @ 2006-10-09 16:51 UTC (permalink / raw)
To: Paul Moore, Venkat Yekkirala
Cc: selinux, jmorris, sds, eparis, jbrindle, redhat-lspp
OK. Thanks.
PS: Somehow I didn't run into these on my system.
> -----Original Message-----
> From: Paul Moore [mailto:paul.moore@hp.com]
> Sent: Monday, October 09, 2006 11:49 AM
> To: Venkat Yekkirala
> Cc: selinux@tycho.nsa.gov; jmorris@namei.org; sds@tycho.nsa.gov;
> eparis@redhat.com; jbrindle@tresys.com; redhat-lspp@redhat.com
> Subject: Re: [PATCH 1/1] selinux: secid reconciliation fixes V02
>
>
> FYI: some changes need to be made to avoid compilation
> warnings (see below, and
> selinux_ip_postroute_last() to see what I mean), I'm taking
> the liberty of
> changing the patch myself.
>
> Venkat Yekkirala wrote:
> > -static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid)
> > +static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid,
> > + const struct net_device *out, unsigned
> short family)
> > {
> > int err;
> > + char *addrp;
> > + int len;
> > + struct avc_audit_data ad;
>
> Add the following:
>
> struct net_device *dev = (struct net_device *)out;
>
>
> > if (selinux_compat_net)
> > return 1;
> > @@ -3738,9 +3749,17 @@ static int selinux_skb_flow_out(struct s
> > }
> > }
> >
> > + AVC_AUDIT_DATA_INIT(&ad, NET);
> > + ad.u.net.netif = out->name;
>
> Replace the above line with:
>
> ad.u.net.netif = dev->name;
>
> > + ad.u.net.family = family;
> > + err = selinux_parse_skb(skb, &ad, &addrp, &len, 0);
> > + if (err)
> > + goto out;
> > +
> > err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
> > - PACKET__FLOW_OUT, NULL);
> > + PACKET__FLOW_OUT, &ad);
> >
> > +out:
> > return err ? 0 : 1;
> > }
>
> --
> paul moore
> linux security @ hp
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/1] selinux: secid reconciliation fixes V02
@ 2006-10-09 15:26 Venkat Yekkirala
2006-10-09 15:51 ` Paul Moore
2006-10-09 16:48 ` Paul Moore
0 siblings, 2 replies; 6+ messages in thread
From: Venkat Yekkirala @ 2006-10-09 15:26 UTC (permalink / raw)
To: selinux; +Cc: jmorris, sds, paul.moore, eparis, jbrindle, redhat-lspp
This fixes the secid reconciliation code in the following ways:
1. Null-out secmark on an outgoing packet after we are done with all
the checks. This has been necessitated by the fact that some packets
sent to a multicast address could arrive back on a non-loopback
interface but with the secmark intact. This would result in the
current flow_out control logic to use it as a security point context
when no explicit security points have been defined for the inbound
packet.
2. Label udp/raw packets with the label of the socket.
3. Label igmp traffic with the igmp_packet initial context.
4. Limit flow-controlling of loopback traffic to the socket.recv permission
check. This means that packet.flow_in/flow_out checks are no longer
applicable to loopback traffic. This is because of current implementation
constraints.
DOCUMENTATION OF SECID RECONCILIATION AND FLOW CONTROL FOR POLICY WRITERS:
ON INBOUND:
1. PACKETS ENTERING SYSTEM FROM A NON-LOOPBACK DEVICE:
Can a packet "carrying" external domain label x_t "flow_in" thru the
security point with the peer domain label p_d_t?
NOTE:
a. x_t defaults to unlabeled_t, if no external label.
b. p_d_t defaults to network_t in the absence of any applicable
[conn]secmark rules for the packet. If there are multiple
secmark rules applicable to a packet, the context on the LAST
rule will apply.
NO: Drop packet.
YES: If no external label, let packet "carry" p_d_t.
2. INPUT ONLY: Can a socket "recv" a packet from domain p_d_t?
NO: Drop packet.
YES: If setting up a tcp connection, set peer context to p_d_t.
ON OUTBOUND:
1. Let packet "carry" the originating socket domain label.
2. IPSEC Handling:
LABELED IPSEC: If packet "polmatch"es to an otherwise applicable and
labeled SPD entry, choose a Security Association (SA) with the SAME context
as the domain label being carried by packet.
NOTE: If no such SA present, call into IKE with context on packet.
NON-LABELED (PLAIN/TRADITIONAL) IPSEC: If there's an applicable SPD entry
that does NOT have an explicit context associated with it, an applicable SA
that does NOT have an explicit context associated with it is chosen.
NOTE: If no such SA present, call into IKE, but with NO context.
3. PACKETS DESTINED FOR NON-LOOPBACK DEVICE:
a. IPTABLES Processing:
As EACH applicable iptables [CONN]SECMARK rule with domain p_d_t is
encountered, do the following:
Can a packet carrying domain label a_t "flow_out" of the security point
with the domain label p_d_t?
NO: Drop packet.
YES: Replace the domain label a_t on the packet with the security point
label p_d_t.
b. Before a packet is let out of the system:
Can a packet with domain label p_d_t "flow_out" into the network domain
network_t?
NO: Drop packet.
YES: Let packet out.
NOTE: Ideally this check should be applicable only to packets that
didn't go thru [conn]secmark checks for outbound, but there's
currently no way to know this due to implementation constrains.
Hence a blanket check for ALL packets leaving the system.
FORWARDED TRAFFIC:
Forwarded Traffic will undergo the following:
1. Step 1 under ON INBOUND.
2. Steps 2 and 3 under ON OUTBOUND.
Signed-off-by: Venkat Yekkirala <vyekkirala@TrustedCS.com>
---
include/linux/security.h | 30 ++++++++++-----
include/net/ip.h | 13 ++++++
include/net/request_sock.h | 11 +++++
net/ipv4/igmp.c | 4 ++
net/ipv4/raw.c | 2 +
net/ipv4/udp.c | 2 +
net/netfilter/xt_CONNSECMARK.c | 21 ++++++++--
net/netfilter/xt_SECMARK.c | 16 ++++++--
security/dummy.c | 8 ++--
security/selinux/hooks.c | 60 +++++++++++++++++++++++++------
10 files changed, 134 insertions(+), 33 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 77e265d..7d36aaa 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -588,6 +588,8 @@ int udp_sendmsg(struct kiocb *iocb, stru
if (!ipc.opt)
ipc.opt = inet->opt;
+ security_sk_classify_ipcm(sk, &ipc);
+
saddr = ipc.addr;
ipc.addr = faddr = daddr;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 0e935b4..fa4d5fb 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -433,6 +433,8 @@ static int raw_sendmsg(struct kiocb *ioc
ipc.opt = NULL;
ipc.oif = sk->sk_bound_dev_if;
+ security_sk_classify_ipcm(sk, &ipc);
+
if (msg->msg_controllen) {
err = ip_cmsg_send(msg, &ipc);
if (err)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 58be822..b630634 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -293,6 +293,8 @@ static struct sk_buff *igmpv3_newpack(st
if (skb == NULL)
return NULL;
+ security_igmp_classify_skb(skb);
+
{
struct flowi fl = { .oif = dev->ifindex,
.nl_u = { .ip4_u = {
@@ -658,6 +660,8 @@ static int igmp_send_report(struct in_de
return -1;
}
+ security_igmp_classify_skb(skb);
+
skb->dst = &rt->u.dst;
skb_reserve(skb, LL_RESERVED_SPACE(dev));
--- net-2.6.sid6/include/linux/security.h 2006-10-05 12:03:39.000000000 -0500
+++ net-2.6/include/linux/security.h 2006-10-08 14:10:49.000000000 -0500
@@ -67,6 +67,7 @@ struct xfrm_selector;
struct xfrm_policy;
struct xfrm_state;
struct xfrm_user_sec_ctx;
+struct net_device;
extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
extern int cap_netlink_recv(struct sk_buff *skb, int cap);
@@ -828,8 +829,8 @@ struct request_sock;
* Sets the new child socket's sid to the openreq sid.
* @inet_conn_established:
* Sets the connection's peersid to the secmark on skb.
- * @req_classify_flow:
- * Sets the flow's sid to the openreq sid.
+ * @igmp_classify_skb:
+ * Classifies an skb representing an igmp packet.
* @skb_flow_in:
* Checks to see if security policy would allow skb into the system
* while also reconciling the xfrm secid, cipso, etc, if any, and
@@ -1385,9 +1386,10 @@ struct security_operations {
struct request_sock *req);
void (*inet_csk_clone)(struct sock *newsk, const struct request_sock *req);
void (*inet_conn_established)(struct sock *sk, struct sk_buff *skb);
- void (*req_classify_flow)(const struct request_sock *req, struct flowi *fl);
+ void (*igmp_classify_skb)(struct sk_buff *skb);
int (*skb_flow_in)(struct sk_buff *skb, unsigned short family);
- int (*skb_flow_out)(struct sk_buff *skb, u32 nf_secid);
+ int (*skb_flow_out)(struct sk_buff *skb, u32 nf_secid,
+ const struct net_device *out, unsigned short family);
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_NETWORK_XFRM
@@ -2953,14 +2955,20 @@ static inline void security_sk_clone(con
return security_ops->sk_clone_security(sk, newsk);
}
+/*static inline void security_sk_classify_ipcm(struct sock *sk,
+ struct ipcm_cookie *ipc)
+{
+ security_ops->sk_getsecid(sk, &ipc->secid);
+}*/
+
static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
{
security_ops->sk_getsecid(sk, &fl->secid);
}
-static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
+static inline void security_igmp_classify_skb(struct sk_buff *skb)
{
- security_ops->req_classify_flow(req, fl);
+ security_ops->igmp_classify_skb(skb);
}
static inline int security_skb_flow_in(struct sk_buff *skb,
@@ -2970,9 +2978,10 @@ static inline int security_skb_flow_in(s
}
static inline int security_skb_flow_out(struct sk_buff *skb,
- u32 nf_secid)
+ u32 nf_secid, const struct net_device *out,
+ unsigned short family)
{
- return security_ops->skb_flow_out(skb, nf_secid);
+ return security_ops->skb_flow_out(skb, nf_secid, out, family);
}
static inline void security_sock_graft(struct sock* sk, struct socket *parent)
@@ -3128,7 +3137,7 @@ static inline void security_sk_classify_
{
}
-static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
+static inline void security_igmp_classify_skb(struct sk_buff *skb)
{
}
@@ -3139,7 +3148,8 @@ static inline int security_skb_flow_in(s
}
static inline int security_skb_flow_out(struct sk_buff *skb,
- u32 nf_secid)
+ u32 nf_secid, const struct net_device *out,
+ unsigned short family)
{
return -ENOENT;
}
--- net-2.6.sid6/include/net/ip.h 2006-09-29 17:43:08.000000000 -0500
+++ net-2.6/include/net/ip.h 2006-10-08 10:41:14.000000000 -0500
@@ -388,6 +388,14 @@ extern struct ctl_table ipv4_table[];
#ifdef CONFIG_SECURITY_NETWORK
+extern struct security_operations *security_ops;
+
+static inline void security_sk_classify_ipcm(struct sock *sk,
+ struct ipcm_cookie *ipc)
+{
+ security_ops->sk_getsecid(sk, &ipc->secid);
+}
+
static inline void security_skb_classify_ipcm(struct sk_buff *skb,
struct ipcm_cookie *ipc)
{
@@ -402,6 +410,11 @@ static inline void security_ipcm_classif
#else
+static inline void security_sk_classify_ipcm(struct sock *sk,
+ struct ipcm_cookie *ipc)
+{
+}
+
static inline void security_skb_classify_ipcm(struct sk_buff *skb,
struct ipcm_cookie *ipc)
{
--- net-2.6.sid6/include/net/request_sock.h 2006-09-30 14:57:55.000000000 -0500
+++ net-2.6/include/net/request_sock.h 2006-10-08 11:59:51.000000000 -0500
@@ -262,6 +262,12 @@ static inline void reqsk_queue_hash_req(
#ifdef CONFIG_SECURITY_NETWORK
+static inline void security_req_classify_flow(const struct request_sock *req,
+ struct flowi *fl)
+{
+ fl->secid = req->secid;
+}
+
static inline void security_req_classify_skb(struct request_sock *req,
struct sk_buff *skb)
{
@@ -270,6 +276,11 @@ static inline void security_req_classify
#else
+static inline void security_req_classify_flow(const struct request_sock *req,
+ struct flowi *fl)
+{
+}
+
static inline void security_req_classify_skb(struct request_sock *req,
struct sk_buff *skb)
{
--- net-2.6.sid6/net/netfilter/xt_CONNSECMARK.c 2006-09-30 14:24:24.000000000 -0500
+++ net-2.6/net/netfilter/xt_CONNSECMARK.c 2006-10-08 15:50:35.000000000 -0500
@@ -68,7 +68,8 @@ static void secmark_save(struct sk_buff
* On the outbound, filter based on the current secmark.
*/
static unsigned int secmark_restore(struct sk_buff *skb, unsigned int hooknum,
- const struct net_device *in, unsigned short family)
+ const struct net_device *in, const struct net_device *out,
+ unsigned short family)
{
u32 *psecmark;
enum ip_conntrack_info ctinfo;
@@ -81,14 +82,24 @@ static unsigned int secmark_restore(stru
if (outbound(family, hooknum)) {
int err;
- err = security_skb_flow_out(skb, *psecmark);
+ /*
+ * We can't currently flow-control loopback traffic
+ * very well since we want to retain the secmark of
+ * the originating socket and if we do retain it,
+ * it will cause connsecmark save & restore to use this
+ * socket label and mess up the semantics of the "peer
+ * security point".
+ */
+ if (out == &loopback_dev)
+ goto out;
+
+ err = security_skb_flow_out(skb, *psecmark, out, family);
if (!err)
return NF_DROP;
} else
/*
* inbound:
- * loopback traffic should already be labeled
- * and any filtering on outbound should suffice
+ * No filtering of loopback traffic.
*/
if (in == &loopback_dev)
goto out;
@@ -119,7 +130,7 @@ static unsigned int target(struct sk_buf
break;
case CONNSECMARK_RESTORE:
- return secmark_restore(skb, hooknum, in, target->family);
+ return secmark_restore(skb, hooknum, in, out, target->family);
break;
default:
--- net-2.6.sid6/net/netfilter/xt_SECMARK.c 2006-09-30 13:19:44.000000000 -0500
+++ net-2.6/net/netfilter/xt_SECMARK.c 2006-10-08 15:54:06.000000000 -0500
@@ -70,14 +70,24 @@ static unsigned int target(struct sk_buf
if (outbound(target->family, hooknum)) {
int err;
- err = security_skb_flow_out(*pskb, secmark);
+ /*
+ * We can't currently flow-control loopback traffic
+ * very well since we want to retain the secmark of
+ * the originating socket and if we do retain it,
+ * it will cause connsecmark save & restore to use this
+ * socket label and mess up the semantics of the "peer
+ * security point".
+ */
+ if (out == &loopback_dev)
+ goto out;
+
+ err = security_skb_flow_out(*pskb, secmark, out, target->family);
if (!err)
return NF_DROP;
} else
/*
* inbound:
- * loopback traffic should already be labeled
- * and any filtering on outbound should suffice
+ * No filtering of loopback traffic.
*/
if (in == &loopback_dev)
goto out;
--- net-2.6.sid6/security/dummy.c 2006-10-03 12:36:00.000000000 -0500
+++ net-2.6/security/dummy.c 2006-10-08 13:56:25.000000000 -0500
@@ -833,8 +833,7 @@ static inline void dummy_inet_conn_estab
{
}
-static inline void dummy_req_classify_flow(const struct request_sock *req,
- struct flowi *fl)
+static inline void dummy_igmp_classify_skb(struct sk_buff *skb)
{
}
@@ -844,7 +843,8 @@ static inline int dummy_skb_flow_in(stru
return -ENOENT;
}
-static inline int dummy_skb_flow_out(struct sk_buff *skb, u32 nf_secid)
+static inline int dummy_skb_flow_out(struct sk_buff *skb, u32 nf_secid,
+ const struct net_device *out, unsigned short family)
{
return -ENOENT;
}
@@ -1125,7 +1125,7 @@ void security_fixup_ops (struct security
set_to_dummy_if_null(ops, inet_conn_request);
set_to_dummy_if_null(ops, inet_csk_clone);
set_to_dummy_if_null(ops, inet_conn_established);
- set_to_dummy_if_null(ops, req_classify_flow);
+ set_to_dummy_if_null(ops, igmp_classify_skb);
set_to_dummy_if_null(ops, skb_flow_in);
set_to_dummy_if_null(ops, skb_flow_out);
#endif /* CONFIG_SECURITY_NETWORK */
--- net-2.6.sid6/security/selinux/hooks.c 2006-10-03 16:43:21.000000000 -0500
+++ net-2.6/security/selinux/hooks.c 2006-10-09 06:29:56.000000000 -0500
@@ -3677,35 +3677,42 @@ static void selinux_inet_conn_establishe
sksec->peer_sid = skb->secmark;
}
-static void selinux_req_classify_flow(const struct request_sock *req,
- struct flowi *fl)
+static void selinux_igmp_classify_skb(struct sk_buff *skb)
{
- fl->secid = req->secid;
+ skb->secmark = SECINITSID_IGMP_PACKET;
}
static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family)
{
u32 xfrm_sid;
int err;
+ struct avc_audit_data ad;
+ char *addrp;
+ int len;
if (selinux_compat_net)
return 1;
/*
- * loopback traffic already labeled and
- * flow-controlled on outbound. We may
- * need to flow-control on the inbound
- * as well if there's ever a use-case for it.
+ * loopback traffic should already be labeled with
+ * the originating socket label.
*/
if (skb->dev == &loopback_dev)
return 1;
+ AVC_AUDIT_DATA_INIT(&ad, NET);
+ ad.u.net.netif = skb->dev ? skb->dev->name : "[unknown]";
+ ad.u.net.family = family;
+ err = selinux_parse_skb(skb, &ad, &addrp, &len, 1);
+ if (err)
+ goto out;
+
err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
BUG_ON(err);
err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
SECCLASS_PACKET,
- PACKET__FLOW_IN, NULL);
+ PACKET__FLOW_IN, &ad);
if (err)
goto out;
@@ -3718,9 +3725,13 @@ out:
return err ? 0 : 1;
};
-static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid)
+static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid,
+ const struct net_device *out, unsigned short family)
{
int err;
+ char *addrp;
+ int len;
+ struct avc_audit_data ad;
if (selinux_compat_net)
return 1;
@@ -3738,9 +3749,17 @@ static int selinux_skb_flow_out(struct s
}
}
+ AVC_AUDIT_DATA_INIT(&ad, NET);
+ ad.u.net.netif = out->name;
+ ad.u.net.family = family;
+ err = selinux_parse_skb(skb, &ad, &addrp, &len, 0);
+ if (err)
+ goto out;
+
err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
- PACKET__FLOW_OUT, NULL);
+ PACKET__FLOW_OUT, &ad);
+out:
return err ? 0 : 1;
}
@@ -3901,8 +3920,27 @@ static unsigned int selinux_ip_postroute
skb->secmark = sksec->sid;
}
}
+ if (out == &loopback_dev)
+ return NF_ACCEPT;
+
err = avc_has_perm(skb->secmark, SECINITSID_NETMSG,
SECCLASS_PACKET, PACKET__FLOW_OUT, &ad);
+
+ if (skb->secmark)
+ /*
+ * Our multicast packets could get copied back
+ * to us, arriving on a non-loopback device.
+ * Leaving the secmark intact here will cause it
+ * to be used as a security point context in
+ * the flow_in hook above while it's not in fact
+ * a security point context.
+ *
+ * We may be able to retain this marking if
+ * we can reliably determine that it was a local
+ * packet although it arrived on a non-loopback
+ * device, in the flow_in hook above.
+ */
+ skb->secmark = SECSID_NULL;
}
out:
return err ? NF_DROP : NF_ACCEPT;
@@ -4810,7 +4848,7 @@ static struct security_operations selinu
.inet_conn_request = selinux_inet_conn_request,
.inet_csk_clone = selinux_inet_csk_clone,
.inet_conn_established = selinux_inet_conn_established,
- .req_classify_flow = selinux_req_classify_flow,
+ .igmp_classify_skb = selinux_igmp_classify_skb,
.skb_flow_in = selinux_skb_flow_in,
.skb_flow_out = selinux_skb_flow_out,
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] selinux: secid reconciliation fixes V02
2006-10-09 15:26 Venkat Yekkirala
@ 2006-10-09 15:51 ` Paul Moore
2006-10-09 16:48 ` Paul Moore
1 sibling, 0 replies; 6+ messages in thread
From: Paul Moore @ 2006-10-09 15:51 UTC (permalink / raw)
To: Venkat Yekkirala; +Cc: selinux, jmorris, sds, eparis, jbrindle, redhat-lspp
Venkat Yekkirala wrote:
> --- net-2.6.sid6/include/linux/security.h 2006-10-05 12:03:39.000000000 -0500
> +++ net-2.6/include/linux/security.h 2006-10-08 14:10:49.000000000 -0500
> @@ -67,6 +67,7 @@ struct xfrm_selector;
> struct xfrm_policy;
> struct xfrm_state;
> struct xfrm_user_sec_ctx;
> +struct net_device;
>
> extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
> extern int cap_netlink_recv(struct sk_buff *skb, int cap);
> @@ -828,8 +829,8 @@ struct request_sock;
> * Sets the new child socket's sid to the openreq sid.
> * @inet_conn_established:
> * Sets the connection's peersid to the secmark on skb.
> - * @req_classify_flow:
> - * Sets the flow's sid to the openreq sid.
> + * @igmp_classify_skb:
> + * Classifies an skb representing an igmp packet.
I wonder if it might be cleaner to have a generic classify_skb() function? That
seems to be more inline with what James commented on earlier and I'm almost
certain the netdev crowd would be much more open to a generic hook. It
shouldn't be too expensive to check if the packet is an IGMP packet inside the hook.
> * @skb_flow_in:
> * Checks to see if security policy would allow skb into the system
> * while also reconciling the xfrm secid, cipso, etc, if any, and
> @@ -1385,9 +1386,10 @@ struct security_operations {
> struct request_sock *req);
> void (*inet_csk_clone)(struct sock *newsk, const struct request_sock *req);
> void (*inet_conn_established)(struct sock *sk, struct sk_buff *skb);
> - void (*req_classify_flow)(const struct request_sock *req, struct flowi *fl);
> + void (*igmp_classify_skb)(struct sk_buff *skb);
> int (*skb_flow_in)(struct sk_buff *skb, unsigned short family);
> - int (*skb_flow_out)(struct sk_buff *skb, u32 nf_secid);
> + int (*skb_flow_out)(struct sk_buff *skb, u32 nf_secid,
> + const struct net_device *out, unsigned short family);
> #endif /* CONFIG_SECURITY_NETWORK */
>
> #ifdef CONFIG_SECURITY_NETWORK_XFRM
> @@ -2953,14 +2955,20 @@ static inline void security_sk_clone(con
> return security_ops->sk_clone_security(sk, newsk);
> }
>
> +/*static inline void security_sk_classify_ipcm(struct sock *sk,
> + struct ipcm_cookie *ipc)
> +{
> + security_ops->sk_getsecid(sk, &ipc->secid);
> +}*/
> +
If this really isn't needed shouldn't we just remove the code altogether instead
of commenting it out?
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] selinux: secid reconciliation fixes V02
2006-10-09 15:26 Venkat Yekkirala
2006-10-09 15:51 ` Paul Moore
@ 2006-10-09 16:48 ` Paul Moore
1 sibling, 0 replies; 6+ messages in thread
From: Paul Moore @ 2006-10-09 16:48 UTC (permalink / raw)
To: Venkat Yekkirala; +Cc: selinux, jmorris, sds, eparis, jbrindle, redhat-lspp
FYI: some changes need to be made to avoid compilation warnings (see below, and
selinux_ip_postroute_last() to see what I mean), I'm taking the liberty of
changing the patch myself.
Venkat Yekkirala wrote:
> -static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid)
> +static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid,
> + const struct net_device *out, unsigned short family)
> {
> int err;
> + char *addrp;
> + int len;
> + struct avc_audit_data ad;
Add the following:
struct net_device *dev = (struct net_device *)out;
> if (selinux_compat_net)
> return 1;
> @@ -3738,9 +3749,17 @@ static int selinux_skb_flow_out(struct s
> }
> }
>
> + AVC_AUDIT_DATA_INIT(&ad, NET);
> + ad.u.net.netif = out->name;
Replace the above line with:
ad.u.net.netif = dev->name;
> + ad.u.net.family = family;
> + err = selinux_parse_skb(skb, &ad, &addrp, &len, 0);
> + if (err)
> + goto out;
> +
> err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
> - PACKET__FLOW_OUT, NULL);
> + PACKET__FLOW_OUT, &ad);
>
> +out:
> return err ? 0 : 1;
> }
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-10-09 16:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-09 16:01 [PATCH 1/1] selinux: secid reconciliation fixes V02 Venkat Yekkirala
2006-10-09 16:12 ` Paul Moore
-- strict thread matches above, loose matches on Subject: below --
2006-10-09 16:51 Venkat Yekkirala
2006-10-09 15:26 Venkat Yekkirala
2006-10-09 15:51 ` Paul Moore
2006-10-09 16:48 ` Paul Moore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.