* [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack
@ 2010-11-16 21:52 Eric Paris
2010-11-16 21:52 ` [PATCH 2/3] network: tcp_connect should return certain errors " Eric Paris
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Eric Paris @ 2010-11-16 21:52 UTC (permalink / raw)
To: netdev, linux-kernel, selinux, netfilter-devel
Cc: eparis, equinox, eric.dumazet, davem, hzhong, jmorris, kaber,
kuznet, paul.moore, pekkas, sds, yoshfuji
SELinux would like to pass certain fatal errors back up the stack. This patch
implements the generic netfilter support for this functionality.
Based-on-patch-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
include/linux/netfilter.h | 2 ++
net/netfilter/core.c | 6 ++++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 03317c8..1893837 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -33,6 +33,8 @@
#define NF_QUEUE_NR(x) ((((x) << NF_VERDICT_BITS) & NF_VERDICT_QMASK) | NF_QUEUE)
+#define NF_DROP_ERR(x) (((-x) << NF_VERDICT_BITS) | NF_DROP)
+
/* only for userspace compatibility */
#ifndef __KERNEL__
/* Generic cache responses from hook functions.
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 85dabb8..32fcbe2 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -173,9 +173,11 @@ next_hook:
outdev, &elem, okfn, hook_thresh);
if (verdict == NF_ACCEPT || verdict == NF_STOP) {
ret = 1;
- } else if (verdict == NF_DROP) {
+ } else if ((verdict & NF_VERDICT_MASK) == NF_DROP) {
kfree_skb(skb);
- ret = -EPERM;
+ ret = -(verdict >> NF_VERDICT_BITS);
+ if (ret == 0)
+ ret = -EPERM;
} else if ((verdict & NF_VERDICT_MASK) == NF_QUEUE) {
if (!nf_queue(skb, elem, pf, hook, indev, outdev, okfn,
verdict >> NF_VERDICT_BITS))
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] network: tcp_connect should return certain errors up the stack
2010-11-16 21:52 [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack Eric Paris
@ 2010-11-16 21:52 ` Eric Paris
2010-11-17 18:56 ` David Miller
2010-11-16 21:52 ` [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error Eric Paris
2010-11-17 18:55 ` [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack David Miller
2 siblings, 1 reply; 10+ messages in thread
From: Eric Paris @ 2010-11-16 21:52 UTC (permalink / raw)
To: netdev, linux-kernel, selinux, netfilter-devel
Cc: eparis, equinox, eric.dumazet, davem, hzhong, jmorris, kaber,
kuznet, paul.moore, pekkas, sds, yoshfuji
The current tcp_connect code completely ignores errors from sending an skb.
This makes sense in many situations (like -ENOBUFFS) but I want to be able to
immediately fail connections if they are denied by the SELinux netfilter hook.
Netfilter does not normally return ECONNREFUSED when it drops a packet so we
respect that error code as a final and fatal error that can not be recovered.
Based-on-patch-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
net/ipv4/tcp_output.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index e961522..15dcd7b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2592,6 +2592,7 @@ int tcp_connect(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *buff;
+ int err;
tcp_connect_init(sk);
@@ -2614,7 +2615,9 @@ int tcp_connect(struct sock *sk)
sk->sk_wmem_queued += buff->truesize;
sk_mem_charge(sk, buff->truesize);
tp->packets_out += tcp_skb_pcount(buff);
- tcp_transmit_skb(sk, buff, 1, sk->sk_allocation);
+ err = tcp_transmit_skb(sk, buff, 1, sk->sk_allocation);
+ if (err == -ECONNREFUSED)
+ return err;
/* We change tp->snd_nxt after the tcp_transmit_skb() call
* in order to make this packet get counted in tcpOutSegs.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
2010-11-16 21:52 [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack Eric Paris
2010-11-16 21:52 ` [PATCH 2/3] network: tcp_connect should return certain errors " Eric Paris
@ 2010-11-16 21:52 ` Eric Paris
2010-11-17 11:43 ` Patrick McHardy
` (2 more replies)
2010-11-17 18:55 ` [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack David Miller
2 siblings, 3 replies; 10+ messages in thread
From: Eric Paris @ 2010-11-16 21:52 UTC (permalink / raw)
To: netdev, linux-kernel, selinux, netfilter-devel
Cc: eparis, equinox, eric.dumazet, davem, hzhong, jmorris, kaber,
kuznet, paul.moore, pekkas, sds, yoshfuji
The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
want to signal that a drop in this hook is a permanant fatal error and is not
transient. If we do this the error will be passed back up the stack in some
places and applications will get a faster interaction that something went
wrong.
Signed-off-by: Eric Paris <eparis@redhat.com>
---
security/selinux/hooks.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8ba5001..b1104f9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4594,11 +4594,11 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
secmark_perm = PACKET__SEND;
break;
default:
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
}
if (secmark_perm == PACKET__FORWARD_OUT) {
if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
} else
peer_sid = SECINITSID_KERNEL;
} else {
@@ -4611,28 +4611,28 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
ad.u.net.netif = ifindex;
ad.u.net.family = family;
if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (secmark_active)
if (avc_has_perm(peer_sid, skb->secmark,
SECCLASS_PACKET, secmark_perm, &ad))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (peerlbl_active) {
u32 if_sid;
u32 node_sid;
if (sel_netif_sid(ifindex, &if_sid))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (avc_has_perm(peer_sid, if_sid,
SECCLASS_NETIF, NETIF__EGRESS, &ad))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (sel_netnode_sid(addrp, family, &node_sid))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (avc_has_perm(peer_sid, node_sid,
SECCLASS_NODE, NODE__SENDTO, &ad))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
}
return NF_ACCEPT;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
2010-11-16 21:52 ` [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error Eric Paris
@ 2010-11-17 11:43 ` Patrick McHardy
2010-11-17 14:38 ` Eric Paris
2010-11-17 18:56 ` David Miller
2010-11-19 15:11 ` Paul Moore
2 siblings, 1 reply; 10+ messages in thread
From: Patrick McHardy @ 2010-11-17 11:43 UTC (permalink / raw)
To: Eric Paris
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, davem, hzhong, jmorris, kuznet, paul.moore, pekkas,
sds, yoshfuji
On 16.11.2010 22:52, Eric Paris wrote:
> The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
> want to signal that a drop in this hook is a permanant fatal error and is not
> transient. If we do this the error will be passed back up the stack in some
> places and applications will get a faster interaction that something went
> wrong.
Looks good to me. I'd suggest to have these patches go through Dave's
tree since I want to make use of the netfilter error propagation
mechanism to return proper errno codes for netfilter re-routing
failures.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
2010-11-17 11:43 ` Patrick McHardy
@ 2010-11-17 14:38 ` Eric Paris
2010-11-17 18:55 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Eric Paris @ 2010-11-17 14:38 UTC (permalink / raw)
To: Patrick McHardy
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, davem, hzhong, jmorris, kuznet, paul.moore, pekkas,
sds, yoshfuji
On Wed, 2010-11-17 at 12:43 +0100, Patrick McHardy wrote:
> On 16.11.2010 22:52, Eric Paris wrote:
> > The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
> > want to signal that a drop in this hook is a permanant fatal error and is not
> > transient. If we do this the error will be passed back up the stack in some
> > places and applications will get a faster interaction that something went
> > wrong.
>
> Looks good to me. I'd suggest to have these patches go through Dave's
> tree since I want to make use of the netfilter error propagation
> mechanism to return proper errno codes for netfilter re-routing
> failures.
I'd be happy if Dave pulled patches 1 and 2. I can resend patch #3 once
I can cajole another of the SELinux maintainers to look at it (I believe
he most likely one is on vacation this week)
-Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
2010-11-17 14:38 ` Eric Paris
@ 2010-11-17 18:55 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-11-17 18:55 UTC (permalink / raw)
To: eparis
Cc: kaber, netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, hzhong, jmorris, kuznet, paul.moore, pekkas, sds,
yoshfuji
From: Eric Paris <eparis@redhat.com>
Date: Wed, 17 Nov 2010 09:38:59 -0500
> On Wed, 2010-11-17 at 12:43 +0100, Patrick McHardy wrote:
>> On 16.11.2010 22:52, Eric Paris wrote:
>> > The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
>> > want to signal that a drop in this hook is a permanant fatal error and is not
>> > transient. If we do this the error will be passed back up the stack in some
>> > places and applications will get a faster interaction that something went
>> > wrong.
>>
>> Looks good to me. I'd suggest to have these patches go through Dave's
>> tree since I want to make use of the netfilter error propagation
>> mechanism to return proper errno codes for netfilter re-routing
>> failures.
>
>
> I'd be happy if Dave pulled patches 1 and 2. I can resend patch #3 once
> I can cajole another of the SELinux maintainers to look at it (I believe
> he most likely one is on vacation this week)
I think it's best to pull this all into net-next-2.6 now, so that's what
I'm doing right now.
If there are problems we can apply changes on top.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack
2010-11-16 21:52 [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack Eric Paris
2010-11-16 21:52 ` [PATCH 2/3] network: tcp_connect should return certain errors " Eric Paris
2010-11-16 21:52 ` [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error Eric Paris
@ 2010-11-17 18:55 ` David Miller
2 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-11-17 18:55 UTC (permalink / raw)
To: eparis
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, hzhong, jmorris, kaber, kuznet, paul.moore, pekkas,
sds, yoshfuji
From: Eric Paris <eparis@redhat.com>
Date: Tue, 16 Nov 2010 16:52:38 -0500
> SELinux would like to pass certain fatal errors back up the stack. This patch
> implements the generic netfilter support for this functionality.
>
> Based-on-patch-by: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] network: tcp_connect should return certain errors up the stack
2010-11-16 21:52 ` [PATCH 2/3] network: tcp_connect should return certain errors " Eric Paris
@ 2010-11-17 18:56 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-11-17 18:56 UTC (permalink / raw)
To: eparis
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, hzhong, jmorris, kaber, kuznet, paul.moore, pekkas,
sds, yoshfuji
From: Eric Paris <eparis@redhat.com>
Date: Tue, 16 Nov 2010 16:52:49 -0500
> The current tcp_connect code completely ignores errors from sending an skb.
> This makes sense in many situations (like -ENOBUFFS) but I want to be able to
> immediately fail connections if they are denied by the SELinux netfilter hook.
> Netfilter does not normally return ECONNREFUSED when it drops a packet so we
> respect that error code as a final and fatal error that can not be recovered.
>
> Based-on-patch-by: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
2010-11-16 21:52 ` [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error Eric Paris
2010-11-17 11:43 ` Patrick McHardy
@ 2010-11-17 18:56 ` David Miller
2010-11-19 15:11 ` Paul Moore
2 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-11-17 18:56 UTC (permalink / raw)
To: eparis
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, hzhong, jmorris, kaber, kuznet, paul.moore, pekkas,
sds, yoshfuji
From: Eric Paris <eparis@redhat.com>
Date: Tue, 16 Nov 2010 16:52:57 -0500
> The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
> want to signal that a drop in this hook is a permanant fatal error and is not
> transient. If we do this the error will be passed back up the stack in some
> places and applications will get a faster interaction that something went
> wrong.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
2010-11-16 21:52 ` [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error Eric Paris
2010-11-17 11:43 ` Patrick McHardy
2010-11-17 18:56 ` David Miller
@ 2010-11-19 15:11 ` Paul Moore
2 siblings, 0 replies; 10+ messages in thread
From: Paul Moore @ 2010-11-19 15:11 UTC (permalink / raw)
To: Eric Paris
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, davem, hzhong, jmorris, kaber, kuznet, pekkas, sds,
yoshfuji
On Tue, 2010-11-16 at 16:52 -0500, Eric Paris wrote:
> The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
> want to signal that a drop in this hook is a permanant fatal error and is not
> transient. If we do this the error will be passed back up the stack in some
> places and applications will get a faster interaction that something went
> wrong.
Sorry for the delay, but I wasn't able to respond until today ...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 8ba5001..b1104f9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4594,11 +4594,11 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> secmark_perm = PACKET__SEND;
> break;
> default:
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
> }
> if (secmark_perm == PACKET__FORWARD_OUT) {
> if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
The error condition here isn't due to a policy decision, but some
runtime error that happened when trying to determine the peer label of
an individual packet. I think leaving this as just NF_DROP is the right
thing to do as an error here can be temporary.
> } else
> peer_sid = SECINITSID_KERNEL;
> } else {
> @@ -4611,28 +4611,28 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> ad.u.net.netif = ifindex;
> ad.u.net.family = family;
> if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Same thing, this is a transient error and not due to a policy decision.
We should keep this as NF_DROP.
> if (secmark_active)
> if (avc_has_perm(peer_sid, skb->secmark,
> SECCLASS_PACKET, secmark_perm, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Yep, this is a good place as the error is the result of a policy
decision, i.e. an avc_has_perm() call.
> if (peerlbl_active) {
> u32 if_sid;
> u32 node_sid;
>
> if (sel_netif_sid(ifindex, &if_sid))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Another transient error case, should be NF_DROP.
> if (avc_has_perm(peer_sid, if_sid,
> SECCLASS_NETIF, NETIF__EGRESS, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Good.
> if (sel_netnode_sid(addrp, family, &node_sid))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Bad.
> if (avc_has_perm(peer_sid, node_sid,
> SECCLASS_NODE, NODE__SENDTO, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Good. I think you get the idea now.
Also, while I think we can ignore the forwarding and output hooks, we do
need to change the NF_DROPs in selinux_ip_postroute_compat() ... I'd
suggest the following (the last change catches more than just policy
decisions but considering the "compat" nature I think a little wiggle
room is okay here):
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65fa8bf..de1b79e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4520,11 +4520,11 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
if (selinux_secmark_enabled())
if (avc_has_perm(sksec->sid, skb->secmark,
SECCLASS_PACKET, PACKET__SEND, &ad))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (selinux_policycap_netpeer)
if (selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
return NF_ACCEPT;
}
--
paul moore
linux @ hp
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-11-19 15:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16 21:52 [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack Eric Paris
2010-11-16 21:52 ` [PATCH 2/3] network: tcp_connect should return certain errors " Eric Paris
2010-11-17 18:56 ` David Miller
2010-11-16 21:52 ` [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error Eric Paris
2010-11-17 11:43 ` Patrick McHardy
2010-11-17 14:38 ` Eric Paris
2010-11-17 18:55 ` David Miller
2010-11-17 18:56 ` David Miller
2010-11-19 15:11 ` Paul Moore
2010-11-17 18:55 ` [PATCH 1/3] netfilter: allow hooks to pass error code back up the stack David Miller
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).