Orphaned expectations

In trying to track down the cause of the oops on read of /proc/net/ip_conntrack I stumbled upon a larger issue which may be responsible for the livelocks I have seen on a few heavily used firewalls.

For reference, the relevant bug report is:

https://bugzilla.netfilter.org/cgi-bin/bugzilla/show_bug.cgi?id=131

The oops is actually caused by a mangled expect which has lost its helper. So when print_expect hits this expect and tries to dereference like so:

	if (expect->expectant->helper->timeout)

The oops occurs. Simply making this change prevents the oops:

	if (expect->expectant->helper && expect->expectant->helper->timeout)

But doesn't fix the underlying problem of how the expect lost its helper.

The problem is the TFTP helper can create orphaned expectations. Consider the following testcase:

on a:
iptables -I INPUT -p udp --dport 69 -j DROP

on b:
tftp -c get a:foo

The expect is setup by the tftp helper, but the master conntrack is not confirmed. Instead, the master is immediately sent to destroy_conntrack. Whoops! We've now got an orphaned expectation which will not go away (ever!). This makes for a nice DOS btw, if it was known the TFTP helper was loaded one could trivially fill up the conntrack table with expects.

Further, now that we have this orphaned expectation, we can Oops the kernel without even reading /proc/net/ip_conntrack by:

on a:
iptables -D INPUT 1

on b:
tftp -c get a:foo (assuming we get the same udp port as before)

This time, the kernel will oops in init_conntrack.

So we need to make sure that expectations don't get orphaned in this way, which I believe the below patch will ensure. Of course it will also stop the oops on read of /proc/net/ip_conntrack.

Let me know if anyone sees problems with it.

Phil Oester

diff -ru linux-2.4.26/net/ipv4/netfilter/ip_conntrack_core.c linux-2.4.26-po/net/ipv4/netfilter/ip_conntrack_core.c
@@ -326,6 +322,11 @@
                ip_conntrack_destroyed(ct);
 
        WRITE_LOCK(&ip_conntrack_lock);
+
+       /* Make sure we don't leave any orphaned expectations lying around */
+       if (ct->expecting)
+               remove_expectations(ct,1);
+
        /* Delete us from our own list to prevent corruption later */
        list_del(&ct->sibling_list);
 

Hi Phil,

Thanks for fixing that one, it's been bugging me for a while!

Do you think that any other helpers might be vulnerable to the same problem?

Cheers, Chris.

_  __ __     _
 / __/ / ,__(_)_  | Chris Wilson -- UNIX Firewall Lead Developer |
/ (_  ,\/ _/ /_ \ | NetServers.co.uk http://www.netservers.co.uk |
\__/_/_/_//_/___/ | 21 Signet Court, Cambridge, UK. 01223 576516 |

I don't think any other helper is vulnerable, since it would likely have to be a UDP helper for it to generate the expect in advance of conntrack confirmation.

Phil

Hi,

I think that only helpers for udp based protocols. So, I think that it could be also reproduced with the amanda helper in a weird scenario since this helper only looks for patterns in packets which comes from server to client.

Actually, when an udp packet hits the connection tracking system for first time, in the ip_conntrack_in function:

a) a new conntrack is created.
b) a helper is asigned to this conntrack.
c) and if this helper exists (case of tftp helper), it is also called.

So now, with only an udp packet, we have a conntrack which is not confirmed with an expectation associated, and as Phil pointed out, if this conntrack is destroyed the expectation keeps there forever.

regards,
Pablo

Hi Pablo,

Thanks very much for your explanation. So, to prevent this from happening, what is the best thing to do?

1. Audit all helpers to check that they don't create expectations to unconfirmed conntracks

2. Modify destroy_conntrack to destroy any associated expectations, whether or not the conntrack is confirmed (presumably right now it only does so if the conntrack is confirmed? hence the problem?)

Cheers, Chris.

_  __ __     _
 / __/ / ,__(_)_  | Chris Wilson -- UNIX Firewall Lead Developer |
/ (_  ,\/ _/ /_ \ | NetServers.co.uk http://www.netservers.co.uk |
\__/_/_/_//_/___/ | 21 Signet Court, Cambridge, UK. 01223 576516 |

Hi Chris,

I think that this assumption is wrong for udp based helpers. When connection tracking sees udp packet which has a helper associated for first time, a conntrack (not confirmed yet) and its respective expectation are created, and surely this conntrack will be confirmed later.

yes, I think that Phil's solution is ok, this could also prevent any other weird combinations.

regards,
Pablo

Thanks Phil, I've added this patch to CVS:

@@ -324,8 +324,9 @@

                ip_conntrack_destroyed(ct);

        WRITE_LOCK(&ip_conntrack_lock);
-       /* Delete us from our own list to prevent corruption later */
-       list_del(&ct->sibling_list);
+       /* Make sure don't leave any orphaned expectations lying around */
+       if (ct->expecting)

+ remove_expectations(ct, 1);

        /* Delete our master expectation */
        if (ct->master) {

Regards,
Patrick