containers / udica

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

Policies are not generated in order/not reproduceable #84

Open JAORMX opened 3 years ago

JAORMX commented 3 years ago

Describe the bug

When generating selinux policies in CI, one expects that subsequent calls to Udica will generate the same policy, however, this doesn't seem to be the case. While the policies are equivalent, the order of the items in the policy differs. This makes it really hard to detect if new changes come in the policy as the container evolves, and thus, prevents us from checking this in CI.

For instance:

$ diff /tmp/ci/selinuxd.cil selinuxd/security/selinuxd.cil
5,7d4
<     (allow process sysfs_t ( dir ( add_name create getattr ioctl lock open read remove_name rmdir search setattr write ))) 
<     (allow process sysfs_t ( file ( append create getattr ioctl lock map open read rename setattr unlink write ))) 
<     (allow process sysfs_t ( sock_file ( append getattr open read write ))) 
22a20,22
>     (allow process sysfs_t ( dir ( add_name create getattr ioctl lock open read remove_name rmdir search setattr write ))) 
>     (allow process sysfs_t ( file ( append create getattr ioctl lock map open read rename setattr unlink write ))) 
>     (allow process sysfs_t ( sock_file ( append getattr open read write ))) 

While that diff doesn't differ in content, the issue there is that that section was created a different order in the policy.

To Reproduce Steps to reproduce the behavior:

  1. generate a policy for a container and store the file
  2. run the policy generation again and store the file
  3. diff them

Expected behavior Running Udica for a container should always generate the same policy in the same order (so commands like diff show they're equivalent.

JAORMX commented 3 years ago

@wrabcak anybody from the team that can check this out? This is preventing us from using Udica in CI environments.

vmojzis commented 3 years ago

@JAORMX Thank you for reporting the issue, could you please share a container inspection file (or 2 that have the same content, just ordered differently) that is causing this issue?

JAORMX commented 3 years ago

I don't have one handy right now, but this is how we were generating it https://github.com/JAORMX/selinuxd/blob/main/hack/ci/daemon-and-trace.sh#L48

vmojzis commented 3 years ago

I'm sorry, but after trying several different containers I haven't been able to reproduce the issue.

I added sorting to some of the container inspect data, which should diminish differences between policies generated for the same container. But, without a reliable reproducer I cannot be sure this resolves your issue. https://github.com/containers/udica/pull/97

JAORMX commented 3 years ago

@vmojzis I'm on PTO, but I'll provide a reproducer when I'm back. Or @jhrozek any chance you could look into this?

jhrozek commented 3 years ago

@vmojzis I'm on PTO, but I'll provide a reproducer when I'm back. Or @jhrozek any chance you could look into this?

Hmm, I can't reproduce this either now because udica is throwing Couldn't create policy: 'BPF'. Is that a new problem? Should I create a new ticket for that?

vmojzis commented 3 years ago

@vmojzis I'm on PTO, but I'll provide a reproducer when I'm back. Or @jhrozek any chance you could look into this?

Hmm, I can't reproduce this either now because udica is throwing Couldn't create policy: 'BPF'. Is that a new problem? Should I create a new ticket for that?

Yes, that is a different issue addressed by https://github.com/containers/udica/commit/6e74f83e6afa2bb4fc3277ece64300b0779d86c5 (selinux-policy contains new capabilities unknown to udica).

vmojzis commented 2 years ago

@JAORMX @jhrozek https://github.com/containers/udica/commit/6e74f83e6afa2bb4fc3277ece64300b0779d86c5 should be fixed now. Any update on the reproducer? Did https://github.com/containers/udica/commit/aa2da32f11bca59eea0193b3b13a8efc4082a643 help at all?