Re: [PATCH nf-next] netfilter: nf_conncount: fix tracking of connections from localhost

[Florian Westphal <fw@strlen.de>]

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> Since commit be102eb6a0e7 ("netfilter: nf_conncount: rework API to use
> sk_buff directly"), we skip the adding and trigger a GC when the ct is
> confirmed. For connections originated from local to local it doesn't
> work because the connection is confirmed from a early stage, therefore
> tracking is always skipped.

Alternative:

@@ -415,7 +415,7 @@ insert_tree(struct net *net,
                        if (ret && ret != -EEXIST)
                                count = 0; /* hotdrop */
                        else
-                               count = rbconn->list.count;
+                               count = rbconn->list.count ? rbconn->list.count : 1;

?

connlimit for localhost connections only works correctly in output or
postrouting, even before any of your changes.

As you say; for local connections, confirmation happens in postrouting,
i.e., before prerouting rules are evaluated.

Hence, even before any of your changes, the conntrack limit is never
effective because the connections are confirmed before.
In the reported example, its no problem to create 1000k connections,
500 will go through, rest will eventually time out.  But they are
created.

AFAICS the problem is erroneous trigger of "hotdrop" mode.  First
connection attempt allocates new node, with count == 1.

Subsequent attempt encounter -EEXIST check instead of add, then
return list length with is 0, not 1  so packet gets dropped.

Without your patches, connections won't complete once reaching
the limit, but the conntrack entries can be allocated and are confirmed
regardless of "-m connlimit".

Thus I'm not sold on this use case, it doesn't limit connections,
it only limits established ones.

If its legit case, then we should have a test case for this in
iptables.git .

That said, the patch looks correct to me.