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

turn arbitrary SELinux booleans on or off at boot #291

Closed
wants to merge 1 commit into from

Conversation

jamescassell
Copy link

to use it after first boot, do something like
systemctl enable --now setsebool-on@container_manage_cgroup.service

To use it, you'd normally use an fcc like this one:

---
variant: fcos
version: 1.0.0
ignition: {}
systemd:
  units:
  - name: setsebool-on@container_manage_cgroup.service
    enabled: true

That doesn't actually work because of coreos/ignition#586 or a related bug.

to make it work for now, use something like this:

variant: fcos
version: 1.0.0
ignition: {}
systemd:
  units:
  - name: ignition-firstboot-selinux-booleans.service
    enabled: true
    contents: |
      [Unit]
      Description=Tweak SELinux booleans at first boot
      ConditionFirstBoot=true
      After=local-fs.target
      
      [Service]
      ExecStart=/usr/bin/systemctl enable --now setsebool-on@container_manage_cgroup.service
      Type=oneshot
      RemainAfterExit=yes
      
      [Install]
      WantedBy=sysinit.target

@dustymabe
Copy link
Member

tangent: grrr - i wish we had fixed coreos/ignition#586 already

This seems like a good idea to me as a short term solution to our "configuring SELinux in a more cloud native way" problem.

Is there any concern about multiple of these systemd units running at the same time and possibly colliding with one another? An alternative approach would be to have users drop down a config file and have a single service run, read the configuration, and apply the booleans.

@jamescassell
Copy link
Author

You can run arbitrary numbers of these units, using just the single template. For the firstboot workaround, you can space-separate the list of units to be enabled.

@dustymabe
Copy link
Member

dustymabe commented Mar 4, 2020

You can run arbitrary numbers of these units, using just the single template.

Yep. That's how systemd works. What I'm asking is if a user were to define 20 systemd unit instances of this service and many of them fire off at the same time does running setsebool concurrently many times have a possible issue (race condition or otherwise)?

For the firstboot workaround, you can space-separate the list of units to be enabled.

I'm not sure what you mean? Are referring to your suggested workaround for coreos/ignition#586. There's actually a simpler solution I think which is what I used for the kmods-via-containers work (example copied below), which just adds another unit that requires the instantiated unit.

cat <<EOF > ./baseconfig.ign
{
  "ignition": { "version": "2.2.0" },
  "systemd": {
    "units": [{
      "name": "require-kvc-simple-kmod.service",
      "enabled": true,
      "contents": "[Unit]\[email protected]\n[Service]\nType=oneshot\nExecStart=/usr/bin/true\n\n[Install]\nWantedBy=multi-user.target"
    }]
  }
}
EOF

@jamescassell
Copy link
Author

Yeah, your hack is simpler, but doesn't go away after first boot. If the admin checks whether the service will come back up, the obvious answer from systemd would be incorrect.

@cgwalters
Copy link
Member

does running setsebool concurrently many times have a possible issue (race condition or otherwise)?

I think that the underlying selinux tooling has locking, but it's much more efficient to set multiple booleans at once.

@jamescassell
Copy link
Author

does running setsebool concurrently many times have a possible issue (race condition or otherwise)?

I think that the underlying selinux tooling has locking, but it's much more efficient to set multiple booleans at once.

Indeed, this method takes about 0.02 seconds per boolean, or 1 second for 50 booleans... so I wouldn't worry about the boot-time impact.

I have no idea about any locking issues. @rhatdan is an expert.

[root@localhost system]# time for i in {1..100} ; do setsebool container_manage_cgroup off ; setsebool container_manage_cgroup on ; done

real	0m4.141s
user	0m0.354s
sys	0m2.035s

@jamescassell
Copy link
Author

I'd less-prefer to enable services such as these thru a bogus service dependency, as systemd will then report such services as not enabled:

[root@localhost system]# systemctl is-enabled setsebool-on@container_manage_cgroup.service 
disabled

Even though I've Requires'ed it in an enabled service.

My more-troublesome firstboot-only service avoids this. Really, we should just fix the ignition bug.

@dustymabe
Copy link
Member

@jamescassell 👍 to everything you said

@cgwalters
Copy link
Member

Overall I like the idea and it's basically a nicer elaboration on the suggested workaround in openshift/machine-config-operator#852 (comment)

So it will avoid the ostree selinux bug if we make these transient - and that also implies removing ConditionFirstBoot=yes.

@dustymabe
Copy link
Member

OK let me see if I can dig into this a bit. My first reaction is that we should merge this, but now I have some concerns:

  1. we are providing a service that we might not ever be able to take away even if the SELinux story improves
  2. because of systemd instantiated services are not started when enabled via ignition ignition#586 the current suggestion in this PR is actually worse than just having the user implementing it directly
  3. @lucab did mention overdrop-sebool in the original issue - maybe we should give that some consideration.

I'm thinking that since we don't really get a benefit (because it's not less complicated) without fixing coreos/ignition#586 then we shouldn't merge this right now. While we wait maybe our long term SELinux story will gain some clarity and we'll have a better idea on if 1 is worth it.

@lucab
Copy link
Contributor

lucab commented Mar 5, 2020

As we already know we have to live with an hackish workaround for some time, I'd rather suggest we put this templating logic into fcct and we sugar to some high-level key - <on|off> syntax.
That way we don't have to be concerned about mistakenly breaking it on auto-upgrades, and we don't have to deal with deprecation&removal later on.

@jamescassell
Copy link
Author

yeah, I'd be in favor of some syntactic sugar to

selinux:
  booleans:
  - name: container_manage_cgroup
    state: on

I'll be sending another PR to make the workaround closer to a one-liner, but the above would also be nice.

@LorbusChris
Copy link
Contributor

Just to reference a PR where semanage usage is replaced by a call to semodule, which is part of the policycoreutils package that is included in FCOS (as opposed to the policycoreutils-python-utils which is not):
kubevirt/kubevirt#2694

Not an expert here at all, but maybe semodule can serve as an alternative for some more use-cases?

@rhatdan
Copy link

rhatdan commented Mar 7, 2020

setsebool is available for turning on and off booleans and is part of policycoreutils and does not require python.

@dustymabe
Copy link
Member

good news: WIP PR to fix the ignition bug: coreos/ignition#934

@lucab
Copy link
Contributor

lucab commented May 12, 2020

I'm going ahead and closing this stale PR.
@jamescassell as per your comment at #291 (comment), can you please follow up with a ticket/PR directly to fcct to implement this logic there?

@lucab lucab closed this May 12, 2020
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

Successfully merging this pull request may close these issues.

6 participants