Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stateful packet_filter #344

Merged
merged 2 commits into from
Jan 21, 2015
Merged

Conversation

javierguerragiraldez
Copy link
Contributor

This is a much better API for stateful filters. In short: there are named "connection tables". If a filter rule includes a state_track = "name" entry, any packet that passes that specific rule is tracked in that table. A state_check = "name" entry adds to the rule the condition that this packet belongs to an existing connection. Either of these lines can also appear 'naked', outside of any rule, which adds a state checking and/or tracking before any filter test.

The code seems clean and readable to me, but still need some real tests; the current selftest() just exercises the different options to show the generated code and track tables.

@lukego
Copy link
Member

lukego commented Jan 18, 2015

This is a very pleasingly short implementation :-)

Tests covering the interesting cases (including filling up tables) would be great.

I will think about whether the Neutron front-end would have any more requirements on the API. I don't think of anything immediately.

@lukego
Copy link
Member

lukego commented Jan 21, 2015

@eugeneia Do you have an idea for how we should integrate this into the Neutron config? Goal is to be able to selectively enable stateful filtering e.g. on a per-port basis.

@javierguerragiraldez
Copy link
Contributor Author

Added real tests and squashed commits. Should be ready for merge if the snabbot agrees.

lukego added a commit that referenced this pull request Jan 21, 2015
@lukego lukego merged commit e5235a6 into snabbco:master Jan 21, 2015
@eugeneia
Copy link
Member

@lukego Is adding column to the "security rules" table an option?

@lukego
Copy link
Member

lukego commented Jan 21, 2015

@eugeneia That is probably hard compared with extending one of the JSON fields. (Risk is that we have to update the OpenStack database schema, get mixed into their ORM, and slog for years to sync with upstream.)

@eugeneia
Copy link
Member

@lukego One way I see is to add a securty_rules slot to vif_details that maps security rule ids to a map of key value pairs (to be inserted into the rule by neutron2snabb). It would basically be a way to inject into neutron security rules. This would be very very wrong from an API design standpoint... heck its wrong from any standpoint. Let's do it?

(I assume OpenStack's secrules table spec implicitly describes a stateless PF mechanism?)

@javierguerragiraldez javierguerragiraldez deleted the stateful branch January 25, 2015 07:01
@lukego
Copy link
Member

lukego commented Jan 27, 2015

@eugeneia what test coverage do you think we need for this in the NFV context?

I'm thinking something like:

  1. State table initially empty - connections can be made with e.g. telnet/ssh/iperf.
  2. Fill up the connection table e.g. with nmap.
  3. New connections cannot be made anymore.
  4. Existing connections keep working.

and to get some performance visibility with iperf both with table quite full and table quite empty.

@eugeneia
Copy link
Member

@lukego I think semantics are best tested in the PacketFilter selftest, which has good coverage imho. Regarding performance, I feel like I don't really understand how the stateful PacketFilter works. When is a table "full"?

@lukego
Copy link
Member

lukego commented Feb 10, 2015

Sorry, my explanation is not a good match for the code, and maybe I am really suggesting requirements for the implementation that I should have written down earlier :-). My bad!

Questions:

  • How many connections can the table track in parallel?
  • How are connections beyond that limit treated?
  • How does performance scale as more connections are added, up to and above the limit?

A reasonable answer might be: The table tracks up to 1000 connections. Once the table is full, no new connections are allowed until an existing entry times out. Performance is similar for all table sizes: you always get >= 9 Gbps in iperf. A denial of service attack spamming random ports will temporarily block new connections but will not consume especially much CPU or memory.

A problematic answer would be: The table size has no fixed limit. Connections are added without bound. Performance of the Snabb Switch process will degrade as the Lua garbage collector is invoked and swap memory is needed. A denial of service attack will cause the Linux "Out Of Memory Killer" to be invoked.

For current purposes it is more important to be stable and predictable than to be very fast or scalable. The main use case for stateful filtering at the moment is on legacy management ports that may be carelessly exposing themselves to attack in some unpredictable way. (Like when chur was hacked via the Supermicro management port that was reachable via the internet and woefully insecure.)

@javierguerragiraldez
Copy link
Contributor Author

On Tue, Feb 10, 2015 at 10:55 AM, Luke Gorrie notifications@github.com wrote:

A problematic answer would be: The table size has no fixed limit. Connections are added without bound. Performance of the Snabb Switch process will degrade as the Lua garbage collector is invoked and swap memory is needed. A denial of service attack will cause the Linux "Out Of Memory Killer" to be invoked.

since the connection tracking tables are just Lua tables, the answer
is roughly on these terms. since the track specs are very tiny
strings it would take a really heavy and specific DoD attack to hit
swap memory, but it's a real possibility.

since Lua tables don't report the number of items stored, the easiest
solution would be to keep a counter each time a new connection is
added. it might also complicate a little the expiration process,
since it might have to check how many connections are actually being
disposed.

the "right" solution would be to replace the tables with something
else. like our own hash table or bloom filter (probably based on
SIPhash, which is crypto-safe and on the same order of throughput as
murmur). with size limits and expiration built-in.

actually, a fixed-size hash table isn't so hard to do...

Javier

@lukego
Copy link
Member

lukego commented Feb 10, 2015

Short term it seems like the simplest solution would be to stick with Lua tables but have a counter to keep them small? And have a test case for simple DoS?

@lukego
Copy link
Member

lukego commented Feb 10, 2015

Agree that sooner or later we will want an FFI table that handles large datasets gracefully. I've used Judy arrays in the past but they are not very Snabbish (big and complex). I don't think we need this acutely yet though.

@plajjan
Copy link
Contributor

plajjan commented Feb 10, 2015

What ballpark are we talking about for the size of these tables? Thousands? Millions?

I suppose that one usually doesn't track the state on incoming connections but rather on outgoing ones and therefore the incoming DDoS will hit a (hopefully) rather small table, correct?

If on the other hand one tracks incoming connections I think this could easily blow up. DDoS attacks are easily in the millions of new states per second...

@lukego
Copy link
Member

lukego commented Feb 10, 2015

Current requirement seems modest to me: between 100 - 10,000 entries would be adequate for protecting a management port. Overload behavior (if spammed with legitimate connection requests) would be to deny new connections without slowing down the process (i.e. affecting other ports or previously accepted connections).

Future requirements will be tougher. If we were talking about NAT/firewall for all traffic on an ISP we might be talking more about more like 1M sessions per gigabit of traffic. I'd prefer to cross that bridge when we come to it though rather than trying to anticipate future needs without a realistic test case.

@lukego
Copy link
Member

lukego commented Feb 10, 2015

(Tracking only outgoing might be reasonable in this case. I do think we need predicable overload behavior in that case too though e.g. in case somebody innocently runs nmap.)

Generally for NFV I reckon that stateless filtering is more practical. The main value of stateful filtering now is to support deploying e.g. management ports that you have zero confidence in security-wise e.g. could have a telnet server running in the ephemeral port range or something nuts like that.

Make sense?

@javierguerragiraldez
Copy link
Contributor Author

On Tue, Feb 10, 2015 at 11:37 AM, Kristian Larsson
notifications@github.com wrote:

What ballpark are we talking about for the size of these tables? Thousands? Millions?

just throwing numbers around, i guess tens of thousands shouldn't be a
problem. Millions of tracked connections might or might not;
depending on hard-to-guess hash collisions. LuaJIT's string hash is
quick and predictable performance-wise, but it's not the best in
avoiding collisions on similar strings.

I suppose that one usually doesn't track the state on incoming connections but rather on outgoing ones and therefore the incoming DDoS will hit a (hopefully) rather small table, correct?

right. unlike iptables, it's explicit which connections are tracked,
so you could just track outgoing connections, or even start tracking
only established connections.

for example: let pass all incoming SYN packets, track outgoing SYN+ACK
packets, and block incoming packets without SYN that don't belong to a
tracked connection.

that way, a SYNflood wouldn't fill the connection tables, and if any
syn-cookie code in the server chooses to drop a connection it won't be
needlesly tracked.

If on the other hand one tracks incoming connections I think this could easily blow up. DDoS attacks are easily in the millions of new states per second...

right. with high performance networks it's easy to overwhelm any
limited resource. the delicate part here is to provide a
non-catastrofic degraded case.

Javier

@lukego
Copy link
Member

lukego commented Feb 10, 2015

I would like to get a test case up and running that shows us where we stand e.g. run background traffic with randomized addresses and see what happens.

I would not be surprised if the lookup operation itself will be a bottleneck due to allocating interned strings for hashtable keys. (I'd wager a beer that LuaJIT will not sink those allocations.) Then we might need to switch to an FFI table immediately. But speculating about performance is a dangerous habit...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants