twtiger / gosecco

Go seccomp parser and compiler
GNU Lesser General Public License v3.0
53 stars 7 forks source link

Rulelist features requested for OZ #48

Open shw700 opened 8 years ago

shw700 commented 8 years ago

OK, at the moment there are two features we'd like to be supported, and they are interconnected. Hopefully, neither issue should present much technical complexity:

  1. Allow for duplicate rules
  2. Allow for including other rule files. For example, we might want to have some sort of global whitelist or blacklist for system calls, and include these in the configurations for all individual applications. This will make it easy to configure rules for common components/libraries, and in the event that a new version of a critical system library invokes an unexpected syscall, it should automatically "fix" all existing applications that are linked to it.

Finally, I'll tag Dave on this one, but there might be some value in allowing us to load a custom list of constant definitions rather than relying on the prepackaged list...

olabini commented 8 years ago

Hi,

Duplicate rules in what way? How should disambiguation happen? Or should it only be allowed when the expansion is the same?

Including other rule files is already possible through the Golang API. The Prepare function takes a SeccompSettings struct which has an ExtraDefinitions field - you can put as many extra rule files as you want there.

You can use the same method to load custom constant definitions. Variable/macro definitions will override the predefined constants.

shw700 commented 8 years ago

Why exactly would disambiguation be necessary? If the rules are absolutely identical, all subsequent invocations would be discarded.

For example, take "uname:1" Say we include this particular rule in our-rules.seccomp. But our-rules.seccomp includes a rule file global-rules.seccomp, and global-rules.seccomp also contains this exact same entry. The first one should be parsed, and any following entries should be ignored.

For your suggested way of including rules, it would mean that we would have to pre-parse the rule files before passing them to gosecco. This seems to me to be a bit less elegant than allowing gosecco to pick up a line like ".include /some/file" and take care of the process inline.

Unrelated question: from skimming the source it seems that seccomp filters are installed in the order that the corresponding rules are written. Is this the correct behavior? Or should the list be reversed before passing it to the kernel?

dma commented 8 years ago

Hi Ola, I'll offer some missing context: what we are considering doing is taking a base collection of system calls and having that as an include file for whitelists. This base list would include calls like read, write, open, and other basics. We could try this if gosecco allows including policy rules in ExtraDefinitions (documentation says we can't -- only variables and macros, but haven't tried TBH) and not have the parser die on finding another rule for a previously seen syscall.

olabini commented 8 years ago

OK, I see. So if the rules are the same, obviously no disambiguation is needed - I thought you were asking for conflicting rules to be added.

When it comes to the order of how the filters are installed, it shouldn't matter what order they are installed, since each one cover its own syscall.

I don't remember why you can't add rules in ExtraDefinitions - I'll take a look at that.

olabini commented 8 years ago

When it comes to having "include" as a statement, the reason we didn't do that was to avoid complicating the grammer, but also to avoid having the parser be reentrant. We can definitely revisit that decision.

shw700 commented 8 years ago

The reason I'm asking about rule order is not for correctness, but for performance purposes...

shw700 commented 8 years ago

If for some reason you decide not to add an include statement, we will need the ability to pass rules as a string in order to implement this functionality on our end.

olabini commented 8 years ago

Well, rule order will be the same as the order of the rules in the policy file - is that not OK?

shw700 commented 8 years ago

Right now we order our human-readable rules with the most likely encountered conditions first - which I think is the logical programmatic way to do so. This is the opposite of what will end up happening when the filters are installed, though...

olabini commented 8 years ago

No, as far as I can tell, the rules will be compiled and installed in the order they are encountered. A very simple test of compiling: open: 1 futex: 1

Generates this code: ld_abs 4 jeq_k 00 04 C000003E ld_abs 0 jeq_k 01 00 2 jeq_k 00 01 CA ret_k 7FFF0000 ret_k 0

shw700 commented 8 years ago

Yes, I am not referring to compilation... rather to installation. Filters are evaluated by the kernel in reverse order of installation.

olabini commented 8 years ago

I'm a bit confused about this - gosecco doesn't explicitly support installation of more than one policy.

shw700 commented 8 years ago

Ahh, so seccomp/SECCOMP_SET_MODE_FILTER is only called once with all the BPF?

olabini commented 8 years ago

Yep.

shw700 commented 8 years ago

My mistake - now that I think about it I'm not sure why I would have thought otherwise.

olabini commented 8 years ago

NP. =)

dma commented 8 years ago

Linux supports more-than-one policy per process. They are stored in a linked list structure, and I suspect what you noticed was that the last-linked filter gets executed first.

olabini commented 8 years ago

OK, I've just pushed a new function called PrepareSource - instead of paths and strings, it uses an interface called Source that can be used to parse files, strings or combinations of other sources. This allows you to combine rules from more than one file.

I've also pushed a fix to ignore duplicate definitions.

olabini commented 8 years ago

Let me know if these changes are enough, or if you strongly feel that including other files with directives in the policy file is necessary - this change would be a bit more intrusive.