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

Rulelist features requested for OZ #48

Open
shw700 opened this issue Aug 16, 2016 · 19 comments
Open

Rulelist features requested for OZ #48

shw700 opened this issue Aug 16, 2016 · 19 comments

Comments

@shw700
Copy link

shw700 commented Aug 16, 2016

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
Copy link
Member

olabini commented Aug 16, 2016

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
Copy link
Author

shw700 commented Aug 17, 2016

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
Copy link

dma commented Aug 17, 2016

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
Copy link
Member

olabini commented Aug 17, 2016

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
Copy link
Member

olabini commented Aug 17, 2016

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
Copy link
Author

shw700 commented Aug 17, 2016

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

@shw700
Copy link
Author

shw700 commented Aug 17, 2016

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
Copy link
Member

olabini commented Aug 17, 2016

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

@shw700
Copy link
Author

shw700 commented Aug 17, 2016

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
Copy link
Member

olabini commented Aug 17, 2016

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
Copy link
Author

shw700 commented Aug 17, 2016

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

@olabini
Copy link
Member

olabini commented Aug 17, 2016

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

@shw700
Copy link
Author

shw700 commented Aug 17, 2016

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

@olabini
Copy link
Member

olabini commented Aug 17, 2016

Yep.

@shw700
Copy link
Author

shw700 commented Aug 17, 2016

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

@olabini
Copy link
Member

olabini commented Aug 17, 2016

NP. =)

@dma
Copy link

dma commented Aug 17, 2016

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 added a commit that referenced this issue Aug 18, 2016
…uch a way that you can combine sources etc
@olabini
Copy link
Member

olabini commented Aug 18, 2016

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
Copy link
Member

olabini commented Aug 18, 2016

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.

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

No branches or pull requests

3 participants