[RFC] Die to NOTRACK/TRACE, long live MARK!

[RFC] Die to NOTRACK/TRACE, long live MARK!

[Jozsef Kadlecsik]

Hi,

Willy Tarreau suggested that the TCP window tracking code could be used to
protect connections against the "TCP vulnerabilities" brought up recently
(see for example http://www.uniras.gov.uk/vuls/2004/236929). After some
discussion and code-fragments the result is that it could be done fairly
easily: the SYN/RST probes could be counted and when a threshold is
reached, we'd deliberately drop in-window SYN/RST segments to protect the
connection. However, it'd then mean a violation of the TCP protocol
specification and therefore I'd definitely not like to enable it by
default or to enable it for all traffic. So there should be a way to tell
the system which connections (packets) to protect.

We could introduce a new silly target for that purpose like NOTRACK or
TRACE. However it's pure marking and we do have a interface to mark
packets: the MARK target. So if we could tell the system which mark value
has got the given special meaning, we wouldn't need a new target and we
could even eliminate the NOTRACK/TRACE targets.

Such natural way could be to use the proc interface. Say the files
"mark_notrack", "mark_trace", "mark_syn_rst_protection" in
/proc/sys/net/ipv4/netfilter/ could store the mark values which'd enable
the given functionality for the marked packets. Even the symbolic names
could be supported, say

	... -j MARK --set-mark syn_rst_protection

It seems more natural and what is much more important, extendable anytime
without blowing up the system.

Is there any flawn in the plan? I think NOTRACK/TRACE is not rooted
yet deeply so it won't be painful to get rid of them :-).

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Pablo Neira]

Hi Jozsef,

Jozsef Kadlecsik wrote:

>Willy Tarreau suggested that the TCP window tracking code could be used to
>protect connections against the "TCP vulnerabilities" brought up recently
>(see for example http://www.uniras.gov.uk/vuls/2004/236929). After some
>discussion and code-fragments the result is that it could be done fairly
>easily: the SYN/RST probes could be counted and when a threshold is
>reached, we'd deliberately drop in-window SYN/RST segments to protect the
>connection. However, it'd then mean a violation of the TCP protocol
>specification and therefore I'd definitely not like to enable it by
>default or to enable it for all traffic. So there should be a way to tell
>the system which connections (packets) to protect.
>  
>

I agree, these options shouldn't be enable by default, as I told you in 
my last email related to window-tracking stuff that we could set the 
default action (NF_*) applied to packet via sysctl, so a sysadmin could 
choose to perform an agressive window /secuence tracking, this way if we 
see out-of-window packets, we could log and drop them.

>We could introduce a new silly target for that purpose like NOTRACK or
>TRACE. However it's pure marking and we do have a interface to mark
>packets: the MARK target. So if we could tell the system which mark value
>has got the given special meaning, we wouldn't need a new target and we
>could even eliminate the NOTRACK/TRACE targets.
>  
>

yes, I think that's ok, we could implement a set of "security" matches 
which perform some action depending on the mark. For example, 
mark_rst_protection could use internal tcp conntrack information to see 
if an rst was retransmited too many times and block rst's during a 
timeout to reduce effects of an rst attack or whatsoever. So we should 
reserve a range of marks.

regards,
Pablo

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Philip Craig]

Jozsef Kadlecsik wrote:
> We could introduce a new silly target for that purpose like NOTRACK or
> TRACE. However it's pure marking and we do have a interface to mark
> packets: the MARK target. So if we could tell the system which mark value
> has got the given special meaning, we wouldn't need a new target and we
> could even eliminate the NOTRACK/TRACE targets.

I assume this will mean allowing the MARK target to be used in the raw
table, otherwise you can't mark the packets before conntrack.

Will this mean that only one of these features can be used at a time,
since a packet cannot have multiple marks?  And use of any of these
features would preclude current uses of MARK, such as policy routing?
You could add support for masking bits of the mark, but you would have
to add this to the policy routing also.

What happens when you modify the /proc setting?  This will change the
state of packets currently being processed, which could cause problems.

-- 
Philip Craig - SnapGear, A CyberGuard Company - http://www.SnapGear.com

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Patrick McHardy]

Hi,

Philip Craig wrote:
> Jozsef Kadlecsik wrote:
> 
>> We could introduce a new silly target for that purpose like NOTRACK or
>> TRACE. However it's pure marking and we do have a interface to mark
>> packets: the MARK target. So if we could tell the system which mark value
>> has got the given special meaning, we wouldn't need a new target and we
>> could even eliminate the NOTRACK/TRACE targets.
> 
> 
> I assume this will mean allowing the MARK target to be used in the raw
> table, otherwise you can't mark the packets before conntrack.
> 
> Will this mean that only one of these features can be used at a time,
> since a packet cannot have multiple marks?  And use of any of these
> features would preclude current uses of MARK, such as policy routing?
> You could add support for masking bits of the mark, but you would have
> to add this to the policy routing also.

Instead of modifying the nf_mark value (which I don't think we should
do), we could just let the mark target perform the same operations that
NOTRACK/TRACE perform, namely attaching a dummy-conntrack (NOTRACK)
and setting a bit in nfcache (TRACE). For the tcp-window-tracking
spoofed-rst protection we could set a bit in the conntrack structure.
I suppose this feature need to be enabled on a per-connection base
anyway. Unfortunately adding NOTRACK functionality to MARK in this
way would create a dependency on ip_conntrack. Maybe we should add
it to CONNMARK ?

Regards
Patrick

PS: Joszef, I changed your address to @netfilter.org, your mailserver
doesn't like me.

PPS: I'll be away 'till Sunday so don't expect fast responses

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Jozsef Kadlecsik]

Hi,

On Fri, 21 May 2004, Philip Craig wrote:

> Jozsef Kadlecsik wrote:
> > We could introduce a new silly target for that purpose like NOTRACK or
> > TRACE. However it's pure marking and we do have a interface to mark
> > packets: the MARK target. So if we could tell the system which mark value
> > has got the given special meaning, we wouldn't need a new target and we
> > could even eliminate the NOTRACK/TRACE targets.
>
> I assume this will mean allowing the MARK target to be used in the raw
> table, otherwise you can't mark the packets before conntrack.

Oops.

> Will this mean that only one of these features can be used at a time,
> since a packet cannot have multiple marks?  And use of any of these
> features would preclude current uses of MARK, such as policy routing?
> You could add support for masking bits of the mark, but you would have
> to add this to the policy routing also.

Boom! Bye-bye idea, that won't go.

> What happens when you modify the /proc setting?  This will change the
> state of packets currently being processed, which could cause problems.

The raw table is the very first, so no other parts of netfilter could see
the packets.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Jozsef Kadlecsik]

On Fri, 21 May 2004, Patrick McHardy wrote:

> Instead of modifying the nf_mark value (which I don't think we should
> do), we could just let the mark target perform the same operations that
> NOTRACK/TRACE perform, namely attaching a dummy-conntrack (NOTRACK)
> and setting a bit in nfcache (TRACE). For the tcp-window-tracking
> spoofed-rst protection we could set a bit in the conntrack structure.
> I suppose this feature need to be enabled on a per-connection base
> anyway. Unfortunately adding NOTRACK functionality to MARK in this
> way would create a dependency on ip_conntrack. Maybe we should add
> it to CONNMARK ?

Hm. Actually, as we already use a bit in nfcache for TRACE, we could
reserve bits generally:

#define NFC_HOOKS_MASK			0x0000FFFF
#define NFC_FLAGS_MASK			0xFFFF0000

/* Special netfilter flags */

#define NFC_PKT_FLAGS_MASK		0x00FF0000
#define NFC_CT_FLAGS_MASK		0xFF000000

#define NFC_FLAG_TRACE			0x00010000
#define NFC_FLAG_NOTRACK		0x00020000
#define NFC_FLAG_CT_TCP_SYNRST		0x01000000

The lower half could be manipulated by MARK while the upper half by
CONNMARK. But the latter would require a new element in ip_conntrack
itself and I wouldn' really like to make it bigger.
Do we really need an unsigned long to store the status? :-)

> PS: Joszef, I changed your address to @netfilter.org, your mailserver
> doesn't like me.

I whitelisted you, sorry for the inconvenience.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> Hm. Actually, as we already use a bit in nfcache for TRACE, we could
> reserve bits generally:
> 
> #define NFC_HOOKS_MASK			0x0000FFFF
> #define NFC_FLAGS_MASK			0xFFFF0000
> 
> /* Special netfilter flags */
> 
> #define NFC_PKT_FLAGS_MASK		0x00FF0000
> #define NFC_CT_FLAGS_MASK		0xFF000000
> 
> #define NFC_FLAG_TRACE			0x00010000
> #define NFC_FLAG_NOTRACK		0x00020000
> #define NFC_FLAG_CT_TCP_SYNRST		0x01000000
> 
> The lower half could be manipulated by MARK while the upper half by
> CONNMARK. But the latter would require a new element in ip_conntrack
> itself and I wouldn' really like to make it bigger.

I like the idea for MARK, but why should we manipulate nfcache from
CONNMARK, and not let the users of the bits get them directly from
struct ip_conntrack (f.i. CT_TCP_SYNRST).

> Do we really need an unsigned long to store the status? :-)

I always thought we would, because set_bit and test_bit take unsigned
long *, but I'm not sure anymore if that really means we can't use
something smaller.

But good news, I'm currently busy shrinking struct ip_conntrack (I bet
myself I could shrink it 30% within 24 hours). I'm already at 28% ;)

-8bytes replacing struct ip_nat_hash by struct list_head because of two
saved conntrack pointers
-20bytes replacing struct nf_ct_info array by new value in struct
sk_buff indicating relation between skb and conntrack .. maybe we could
also put this in nfcache.
-56bytes dynamically allocating helper data, but crashes with NAT atm

I see 4 more bytes by using u_int16_t for initialized and num_manips in
struct ip_nat_info, and maybe I can also squeeze masq_index in there.

> 
> 
>>PS: Joszef, I changed your address to @netfilter.org, your mailserver
>>doesn't like me.
> 
> 
> I whitelisted you, sorry for the inconvenience.

Thanks.

Regards
Patrick

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Martin Josefsson]

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

On Sat, 2004-05-22 at 16:57, Patrick McHardy wrote:

> But good news, I'm currently busy shrinking struct ip_conntrack (I bet
> myself I could shrink it 30% within 24 hours). I'm already at 28% ;)
> 
> -8bytes replacing struct ip_nat_hash by struct list_head because of two
> saved conntrack pointers
> -20bytes replacing struct nf_ct_info array by new value in struct
> sk_buff indicating relation between skb and conntrack .. maybe we could
> also put this in nfcache.
> -56bytes dynamically allocating helper data, but crashes with NAT atm
> 
> I see 4 more bytes by using u_int16_t for initialized and num_manips in
> struct ip_nat_info, and maybe I can also squeeze masq_index in there.

Nice, I hope you succeed. We also need to rearrange struct ip_conntrack
so often updated fields aren't sharing the same cacheline as mostly read
fields. That and adding more test_bit() before we set_bit() (or
something similar) will bring down the cache-trashing we have right now,
especially on SMP.

-- 
/Martin

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

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Patrick McHardy]

Martin Josefsson wrote:

> Nice, I hope you succeed. We also need to rearrange struct ip_conntrack
> so often updated fields aren't sharing the same cacheline as mostly read
> fields. That and adding more test_bit() before we set_bit() (or
> something similar) will bring down the cache-trashing we have right now,
> especially on SMP.

I will make a test_bit patch on top of my memsave-patches (which are
on top of Joszef's conntrack patches). When we rearrange struct
ip_conntrack we should also try to get rid of the holes in the
structure, which are responsible for approximately 20bytes of wasted
memory.

Regards
Patrick

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Pablo Neira]

Hi Patrick,

Patrick McHardy wrote:

>> Do we really need an unsigned long to store the status? :-)
>
>
> I always thought we would, because set_bit and test_bit take unsigned
> long *, but I'm not sure anymore if that really means we can't use
> something smaller.


if we use atomic operations *_bit for status field in ip_conntrack, we 
only use 24 bits of that unsigned long, this issue lets us keep 
compatibility with some special platforms which only support atomic 
operations of that size.

I read that in  Robert Love's linux kernel development book I think. If 
missing anything, please let me know.

regards,
Pablo

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Patrick McHardy]

Pablo Neira wrote:

> if we use atomic operations *_bit for status field in ip_conntrack, we 
> only use 24 bits of that unsigned long, this issue lets us keep 
> compatibility with some special platforms which only support atomic 
> operations of that size.
>
> I read that in  Robert Love's linux kernel development book I think. 
> If missing anything, please let me know.


This applies to atomic_t and atomic_inc, atomic_dec_and_test, etc., but 
not to set_bit/test_bit.
It can operate on bitmaps larger than unsigned long, but I don't know if 
it will cause problems
with smaller types.

Regards
Patrick

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Henrik Nordstrom]

On Tue, 25 May 2004, Pablo Neira wrote:

> if we use atomic operations *_bit for status field in ip_conntrack, we 
> only use 24 bits of that unsigned long, this issue lets us keep 
> compatibility with some special platforms which only support atomic 
> operations of that size.

Correct. This is also documented int the Kernel API IIRC.

It can be noted however that out of those 24 bits conntrac only uses a 
few, leaving plenty of bits for expansion usage without having to increase 
the size of ip_conntrack.

Regards
Henrik

Re: [RFC] Die to NOTRACK/TRACE, long live MARK!

[Pablo Neira]

Hi,

Henrik Nordstrom wrote:

>On Tue, 25 May 2004, Pablo Neira wrote:
>  
>
>>if we use atomic operations *_bit for status field in ip_conntrack, we 
>>only use 24 bits of that unsigned long, this issue lets us keep 
>>compatibility with some special platforms which only support atomic 
>>operations of that size.
>>    
>>
>
>Correct. This is also documented int the Kernel API IIRC.
>  
>
yes, but I made a mistake, I was talking about atomic operations but 
actually Patrick asked about test_bit/set_bit operations.

regards,
Pablo