aperezdc / snabb

Snabb Switch: Fast open source packet processing
Apache License 2.0
4 stars 1 forks source link

Add l7fw app and `snabb wall filter` #14

Closed takikawa closed 7 years ago

takikawa commented 7 years ago

This PR adds a l7fw app and a snabb wall filter command.

@aperezdc, @dpino : would be interested in any feedback you have, particularly at a high-level to see if the implemented features look sufficient. Here are some design points that maybe could use feedback:

The firewall currently accepts as its rules a table like the following:

{ DNS = "accept",
  SSL = "drop",
  HTTP = [[match { flow_count > 2 => reject; otherwise => accept }]] }

where the keys are protocol names and the values are firewall actions (the third one is a pfmatch expression). I plan to document this in high-level user docs. This PR includes API docs for the included apps, but no high-level docs yet.

The snabb wall filter command that's implemented lets you run an l7fw and l7spy app together to filter packets from any source that snabb wall spy accepts. Right now it just supports sending accepted packets and reject responses to pcap files.

dpino commented 7 years ago

Supporting pfmatch actions that can reference the number of packets in a given protocol flow required changing the pflua library in a somewhat invasive way. Does changing pflua seem okay, or should I just remove this feature?

The feature is worth to me. It's true that it's a bit invasive. If we can minimize this effect, it would be ideal. It gives pflua more possibilities to be extended in the future.

So far snabb wall filter is mostly for testing and I was assuming someone who wanted a firewall would arrange the l7spy and l7fw apps themselves. Should snabb wall filter also have an option to output directly to a NIC instead?

I think not. To me is fine as it's now. Someone interesting in a firewall app can still use L7Fw, which contains the interesting logic. snabb wall filter is mostly UI and it works as fine an example of how to use L7Fw. There are other programs in snabb codebase that are simply there as examples, for instance the example_spray or example_replay programs. I see "snabb wall filter" as a similar case.

Are there any other features needed? (one that I plan to add before merging is logging actions to a plaintext file, potentially with different levels of verbosity)

Sounds interesting. If you feel like implementing this feature, I'd put it under a debug or verbosity flag (printing to stdout or a file may decrease speed significantly). At this moment, I cannot see more features.

takikawa commented 7 years ago

Thanks for the comments. I think I've addressed them in the new commits I pushed.

I also implemented a simple report method for l7fw for testing and added support for (optional, disabled by default) logging to the system log. I made the logging options similar to those for the ufw firewall (just "off" and "on" for now, but it could support levels like "low", "high", etc. like ufw later).

One issue with merging this though is that there are some commits that should probably go upstream separately, like 3fd40e8 (merged in upstream pflua but not in Snabb), 7417375 (in this PR), and 0e692d5 (included with other pflua changes not yet merged upstream) that are needed to run the new app/program. Any suggestions for what to do there?

(For example, I could just merge them into the main snabbwall repo, and then later just deal with a potential merge conflict when the identical change is merged from the upstream snabb release)

dpino commented 7 years ago

@takikawa I generally I do what you did. If I found fixes that could go to upstream, I send a PR with the individual commit and I keep the commit in my branch too.

So now this will go to Adrián's snabbwall branch. The conflicts will happen once snabb:master is merged into Adrián's snabbwall. I dunno if latest versions of git would detect the commits are duplicated and discard them, but in case not solving them manually sounds like a good approach to me. I don't know anything better :)