l4_packet returning NF_DROP

l4_packet returning NF_DROP

[Christoph Paasch]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

Hi,

I have a question regarding the call to l4_packet, in nf_conntrack_in(...) 
from nf_conntrack_core.c

When a module like TCP returns -NF_DROP in tcp_packet(...), the packet won't 
get dropped, because NF_DROP = 0, and in nf_conntrack_in the return of the 
call to l4_packet is checked:
if (ret < 0) {
	...
}

So, there is no way to drop packets after l4_packet.

Why does this is implemented that way?

There are several points in tcp_packet where the function returns -NF_DROP and 
the comments in this function say that the packet will get blocked.

For example (from tcp_packet):

	if (index == TCP_SYNACK_SET
		    && ct->proto.tcp.last_index == TCP_SYN_SET
		    && ct->proto.tcp.last_dir != dir
		    && ntohl(th->ack_seq) == ct->proto.tcp.last_end) {
			/* b) This SYN/ACK acknowledges a SYN that we earlier
			 * ignored as invalid. This means that the client and
			 * the server are both in sync, while the firewall is
			 * not. We kill this session and block the SYN/ACK so
			 * that the client cannot but retransmit its SYN and
			 * thus initiate a clean new session.
			 */
			write_unlock_bh(&tcp_lock);
			if (LOG_INVALID(net, IPPROTO_TCP))
				nf_log_packet(pf, 0, skb, NULL, NULL, NULL,
					  "nf_ct_tcp: killing out of sync session ");
			nf_ct_kill(ct);
			return -NF_DROP;
		}

I hope, that I was clear.
Could someone please explain this to me?
And how can I block packets after the call to l4_packet?

Thanks

--
Christoph Paasch

www.rollerbulls.be
--

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: l4_packet returning NF_DROP

[Christoph Paasch]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

Well, I have seen, that in one of the previous patches from pablo, he adds 
if (ret == -NF_DROP)

inside the if (ret < 0)

but as NF_DROP is 0, it won't ever enter in if(ret == -NF_DROP)

I will make a patch...

On Thu March 12 2009 wrote Christoph Paasch:
> Hi,
>
> I have a question regarding the call to l4_packet, in nf_conntrack_in(...)
> from nf_conntrack_core.c
>
> When a module like TCP returns -NF_DROP in tcp_packet(...), the packet
> won't get dropped, because NF_DROP = 0, and in nf_conntrack_in the return
> of the call to l4_packet is checked:
> if (ret < 0) {
> 	...
> }
>
> So, there is no way to drop packets after l4_packet.
>
> Why does this is implemented that way?
>
> There are several points in tcp_packet where the function returns -NF_DROP
> and the comments in this function say that the packet will get blocked.
>
> For example (from tcp_packet):
>
> 	if (index == TCP_SYNACK_SET
> 		    && ct->proto.tcp.last_index == TCP_SYN_SET
> 		    && ct->proto.tcp.last_dir != dir
> 		    && ntohl(th->ack_seq) == ct->proto.tcp.last_end) {
> 			/* b) This SYN/ACK acknowledges a SYN that we earlier
> 			 * ignored as invalid. This means that the client and
> 			 * the server are both in sync, while the firewall is
> 			 * not. We kill this session and block the SYN/ACK so
> 			 * that the client cannot but retransmit its SYN and
> 			 * thus initiate a clean new session.
> 			 */
> 			write_unlock_bh(&tcp_lock);
> 			if (LOG_INVALID(net, IPPROTO_TCP))
> 				nf_log_packet(pf, 0, skb, NULL, NULL, NULL,
> 					  "nf_ct_tcp: killing out of sync session ");
> 			nf_ct_kill(ct);
> 			return -NF_DROP;
> 		}
>
> I hope, that I was clear.
> Could someone please explain this to me?
> And how can I block packets after the call to l4_packet?
>
> Thanks
>
> --
> Christoph Paasch
>
> www.rollerbulls.be
> --

--
Christoph Paasch

www.rollerbulls.be
--

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Christoph Paasch]

As NF_DROP = 0, no packets would ever have been dropped.
---
 net/netfilter/nf_conntrack_core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ebc2756..452db52 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -727,7 +727,7 @@ nf_conntrack_in(struct net *net, u_int8_t pf, unsigned int hooknum,
 	NF_CT_ASSERT(skb->nfct);
 
 	ret = l4proto->packet(ct, skb, dataoff, ctinfo, pf, hooknum);
-	if (ret < 0) {
+	if (ret <= 0) {
 		/* Invalid: inverse of the return code tells
 		 * the netfilter core what to do */
 		pr_debug("nf_conntrack_in: Can't track with proto module\n");
-- 
1.5.6.3

Re: [PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Jan Engelhardt]

On Thursday 2009-03-12 16:13, Christoph Paasch wrote:

>As NF_DROP = 0, no packets would ever have been dropped.

Mh would not it be safer to actually give NF_DROP a real value so that 
-NF_DROP also makes sense?
(Might need checking places where NF_DROP is used.)

Re: [PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Christoph Paasch]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]


On Thu March 12 2009 wrote Jan Engelhardt:
> On Thursday 2009-03-12 16:13, Christoph Paasch wrote:
> >As NF_DROP = 0, no packets would ever have been dropped.
>
> Mh would not it be safer to actually give NF_DROP a real value so that
> -NF_DROP also makes sense?
Yes, why not...

> (Might need checking places where NF_DROP is used.)
NF_DROP is used only in tcp_packet (as far as I can see, no other l4 proto 
uses it in *_packet)


--
Christoph Paasch

www.rollerbulls.be
--

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Jan Engelhardt]

On Thursday 2009-03-12 16:49, Christoph Paasch wrote:
>On Thu March 12 2009 wrote Jan Engelhardt:
>> On Thursday 2009-03-12 16:13, Christoph Paasch wrote:
>> >As NF_DROP = 0, no packets would ever have been dropped.
>>
>> Mh would not it be safer to actually give NF_DROP a real value so that
>> -NF_DROP also makes sense?
>Yes, why not...
>
>> (Might need checking places where NF_DROP is used.)
>NF_DROP is used only in tcp_packet (as far as I can see, no other l4 proto 
>uses it in *_packet)

NF_DROP is also used in net/netfilter/x_tables.c - that's what I meant.

Re: [PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Pablo Neira Ayuso]

Jan Engelhardt wrote:
> On Thursday 2009-03-12 16:13, Christoph Paasch wrote:
> 
>> As NF_DROP = 0, no packets would ever have been dropped.
> 
> Mh would not it be safer to actually give NF_DROP a real value so that 
> -NF_DROP also makes sense?
> (Might need checking places where NF_DROP is used.)

We cannot change the current value of NF_DROP. This is exposed to
userspace in libnetfilter_queue. Changing the value would break backward
compatibility of existing applications.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Pablo Neira Ayuso]

Christoph Paasch wrote:
> As NF_DROP = 0, no packets would ever have been dropped.

Good catch. I'll also change -NF_DROP by NF_DROP in
nf_conntrack_proto_tcp.c. No need to resend the patch. Thanks.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH] netfilter: Allow dropping packet after call to l4proto->packet

[Christoph Paasch]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

On Thu March 12 2009 wrote Pablo Neira Ayuso:
> Christoph Paasch wrote:
> > As NF_DROP = 0, no packets would ever have been dropped.
>
> Good catch. I'll also change -NF_DROP by NF_DROP in
> nf_conntrack_proto_tcp.c. No need to resend the patch. Thanks.

OK, then change also -NF_DROP to NF_DROP in nf_conntrack_in(...), at
if (ret == -NF_DROP)

--
Christoph Paasch

www.rollerbulls.be
--

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]