containers / udica

This repository contains a tool for generating SELinux security profiles for containers
GNU General Public License v3.0
488 stars 47 forks source link

Add support for custom CIL rules #145

Open FabienGhd opened 4 months ago

FabienGhd commented 4 months ago

Hi,

This PR addresses a suggestion from issue #130 by adding custom CIL templates:

It might also be worth adding the ability to include custom templates when generating a policy file to cover any use cases.

This PR includes:

This feature enables users to define their own additional SELinux rules through custom CIL templates, enhancing Udica's flexibility and extensibility. Any feedback would be greatly appreciated.

Best, Fabien

bachradsusi commented 4 months ago

Could you provide some real world example?

Do I read read it correctly that you need similar option to -a FILEAVCS, --append-rules FILEAVCS but with CIL rules instead of AVC denial messages?

validate_cil_template() is probably not correct:

root@fedora:~# cat block-cil-example.cil 
(block block_1
(type process)
(block block_2
(type process_var_t)
(
  allow
    process
    process_var_t
    (
      dir
      (
        getattr search open
      )
    )
  )
)
)
root@fedora:~# semodule -i block-cil-example.cil 
root@fedora:~# sesearch -A | grep block_1
allow block_1.process block_1.block_2.process_var_t:dir { getattr open search };
FabienGhd commented 4 months ago

Do I read read it correctly that you need similar option to -a FILEAVCS, --append-rules FILEAVCS but with CIL rules instead of AVC denial messages?

Exactly. Reflecting directly on issue #130, we can consider a real-world scenario where someone might need an allow rule that isn't added by default by Udica. For instance, the connectto permission was highlighted in that issue.

In such cases, when Udica doesn't add a specific rule for their configuration by default, users could use their own CIL templates. This would help automate the process, eliminating the need to manually edit the generated Udica CIL file each time.

This is the typical scenario I had in mind.

# User realizes that permissions to connect to UNIX sockets are missing

# Create a custom template file for this
$ echo "    (allow process container_runtime_t ( unix_stream_socket ( connectto )))" > custom_template.cil

# Take 'tests/test_basic.podman.json' and add custom template
$ sudo udica -j test_basic.podman.json --custom-template custom_template.cil test-container

# Verify that the custom policy is added
$ cat test-container.cil | grep "(allow process container_runtime_t ( unix_stream_socket ( connectto )))"

blocks can be nested cil rules could be split to multiline

Very good to know. I'll do my best to fix this.

Thank you for your attention! Fabien

vmojzis commented 4 months ago

Agreed, not sure if we need validation at all given that the policy file will just fail to install in case of invalid rule. Also we may want to rename the command line argument to something like --custom-rules since the code expects a list of policy rules (as opposed to policy blocks that udica uses as templates -- https://github.com/containers/container-selinux/blob/main/udica-templates/base_container.cil). Admittedly it may still be confusing for users since we have --append-rules, which expects AVCs, but renaming that would cause other issues.

It would be quite easy to craft AVCs that can be passed to udica via --append-rules and would perform the same function, but I'm not opposed to adding this.

FabienGhd commented 4 months ago

not sure if we need validation at all given that the policy file will just fail to install in case of invalid rule.

Validation isn't absolutely necessary, but I see it as somewhat helpful for user guidance. The validation function provides specific error messages, assisting users in quickly identifying and correcting any major mistakes in their custom CIL files. Although the validation isn't exhaustive at all for now, as it only ensures balanced parentheses, I think it could still offer some useful checks for the user.