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

Advertise SELinux support #122

Merged
merged 11 commits into from
Feb 26, 2023
Merged

Advertise SELinux support #122

merged 11 commits into from
Feb 26, 2023

Conversation

DemiMarie
Copy link
Contributor

Also some cleanups. See individual commit messages for details.

Marked as draft because this has received no testing (yet).

Qubes OS no longer supports either.
No point in including them, and they might be trusted when they should
not be.
qvm-template will use this to set the selinux feature, which tells
qubesd to enable SELinux in this qube.
template_rpm/09_cleanup.sh Outdated Show resolved Hide resolved
template_rpm/09_cleanup.sh Outdated Show resolved Hide resolved
@marmarek
Copy link
Member

marmarek commented Feb 4, 2023

Besides specific comments above, this needs also:

  1. Installing selinux policy
  2. relabeling fs at the very end
  3. creating flag file for qubes scripts that the fs is already relabeled.

All of the above should be conditional on some template option/flavor, you can for example use containsFlavor "selinux" as the condition. And then build the template as "fc37+selinux" for example (or put that into "options" list if using builderv2).
Currently linux-template-builder is duplicated into builderv2. You can use whichever you like, I'll port that part (if any changes actually will be needed) to the other one when merging.

Otherwise a partially-relabeled system could result, which is bad.
@DemiMarie
Copy link
Contributor Author

All of the above should be conditional on some template option/flavor, you can for example use containsFlavor "selinux" as the condition. And then build the template as "fc37+selinux" for example (or put that into "options" list if using builderv2).

What is the advantage of making this conditional? The SELinux package is not very large and disabling it after the fact is easy.

@marmarek
Copy link
Member

marmarek commented Feb 5, 2023

(I'm not sure how to make a comment to a whole commit in a PR, so I'm posting it here)

Please drop the google-chrome removing commit in its current shape. First of all, the claim in the commit message is false - packages seems to be signed with DSA key, not RSA. But then, if you read the comment you remove, you can see where you can get a signing key. The file under that URL contains actually two keys: DSA 1024 used to sign packages, and RSA 4096 used to sign repo metadata. So, a much better fix is to update the repo to actually use the latter. But please do that in a separate PR.

@marmarek
Copy link
Member

marmarek commented Feb 5, 2023

What is the advantage of making this conditional? The SELinux package is not very large and disabling it after the fact is easy.

First of all, adding it unconditionally will break all template builds for R4.1, as selinux policy is only in R4.2. But also, it will pull in extra dependencies on minimal template - which already is quite big as a "minimal" one. And no, don't just add another check for qubes version - the correct place for this selection in builder config. This will avoid the need for any further changes here, for example if somebody would want centos template with selinux enabled (possibly after adjusting few other packages first).

@DemiMarie DemiMarie requested a review from marmarek February 5, 2023 16:53
@marmarek
Copy link
Member

+ unshare --mount -- chroot -- /builder/mnt /bin/sh -euc 'mount --bind -- / /mnt
        umask 0755
        mkdir -p -m 0700 -- /dev /var /run
        mkdir -p -m 1777 -- /tmp /var/tmp /dev/shm
        find /tmp /var/tmp /run /dev/shm -mindepth 1 -delete 
        : > /.qubes-relabeled
        rm -f /.autorelabel
        setfiles -r /mnt -- "/etc/selinux/$1/contexts/files/file_contexts"' sh targeted
usage:  setfiles [-diIDlmnpqvCEFWT] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file pathname...
usage:  setfiles [-diIDlmnpqvCEFWT] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file -f filename
usage:  setfiles -s [-diIDlmnpqvFWT] spec_file

This avoids needing a relabel after system installation.
Flash is long since end of life.
This will cause qubes-core-agent to pull in qubes-core-agent-selinux via
RPM rich dependencies.
This avoids pulling in unnecessary dependencies, fixes the R4.1 build,
and allows using SELinux on CentOS Stream later.
Do not use the list file.
@marmarek
Copy link
Member

Ok, now relabeling works. There are two remaining issues:

  1. When building with builderv2, the current directory is different, so template.conf modification doesn't work. I'll add variable with explicit directory for that and generally take care of this point.
  2. Selinux remains in permissive mode. That's still better than not enabled, as one can actually enable it manually and have it working immediately (labels seems to be properly preserved), but it's more useful if it would be enforcing out of the box :)

Suggested-by: Marek Marczykowski-Górecki <[email protected]>
Otherwise SELinux provides no security advantages for those who do not
turn it on manually.
@DemiMarie DemiMarie marked this pull request as ready for review February 11, 2023 19:42
@DemiMarie
Copy link
Contributor Author

PipelineRetryFailed

@marmarek marmarek merged commit ac022b1 into QubesOS:main Feb 26, 2023
@DemiMarie DemiMarie deleted the selinux-fix branch February 26, 2023 16:47
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.

2 participants