Profile Checks

[rusty-snake]

First three commits fix things so CI can pass:

• 23c4234: Add alteratives and ld.so.cache to all private-etc lines
• d43904e: Sort disaple-programs.inc
• ac0f95a: Sort src/firecfg/firecfg.config

The last commit (225909d) adds Profile Checks. For now I added four check but once we have this infrastructure for profile checks it is easier to add more in follow-up PRs.

• Run sort.py (and remove the old sort.py workflow)
• private-etc-always-required.sh checks that some files are always allowed with private-etc (ATOW that are alternatives ld.so.cache ld.so.preload). It can not check redirect profiles, therefore some files are double allowed if both profile (redirect profile and redirected profile) contain a private-etc line. Anyway nothing bad will happen.
• sort-disable-programs.sh checks that disable-programs.inc is sorted. I also dropped to separate ~/.cache as this complicates sorting without advantages.
• sort-firecfg.config.sh checks that firecfg.config is sorted.

Possible follow-up PRs:

• Check presents of header and .locals includes
• Check usage of whitelist ${HOME}/... without wc
• Check private-etc containing every necessary network files (for networking progrms)
• Check missing usage of allow includes
• Check machine-id but no nosound
• ...

cc @jose1711

[kmk3]

etc/profile-checks/private-etc-always-required.sh
etc/profile-checks/sort-disable-programs.sh
etc/profile-checks/sort-firecfg.config.sh
etc/profile-checks/sort.py 

GitHub takes a while to load the PR review page (as this changes >200 files)
and the above files (which are the most relevant to this PR IMO) are kind of
lost in the middle of it, given the sort order of:

• profile-a-l
• profile-checks
• profile-m-z

I think it would make sense to put all ci-related files (including for linting
and error-checking) in their own base directory, such as a new ci/ dir.
Example:

ci/etc/private-etc-always-required.sh
ci/etc/sort-disable-programs.sh
ci/etc/sort-firecfg.config.sh
ci/etc/sort.py 

Misc: I put ci/etc/ in the example because firecfg.config is not a profile and
because we could also later add tests for things in etc/apparmor/ and whatever
else is in etc/.

As for the related pre-existing directories:

test/ would maybe also make sense for that, though it is composed solely of
tests related to firejail itself (AFAIK), and they are right at the root, so a
dir just for profile tests wouldn't be very apparent in there.

contrib/ is currently where sort.py is, though I think that it would make more
sense to put new such files in a separate directory instead, as they only deal
with files that are specific to firejail (and no other tool). I think contrib/
makes sense for things like shell completion, syntax highlighting and
distro-specific files (and possibly third-party scripts/helpers).

P.S. I'm writing some more review comments; this is just the main one.

[rusty-snake]

and the above files (which are the most relevant to this PR IMO) are kind of
lost in the middle of it, given the sort order of:

There's a Commits tab 😉

ci/

👍

I think I will go with just ci/ (no etc) or ci-tests/.

[rusty-snake]

Force push changes: Squash in @kmk3's suggestions and move files to ci-tests/

edit: and fix sort.py symlink target

edit2: @kmk3 if you think this is ready, fell free to merge.

[kmk3]

Before I forget, I'll say that these checks are very helpful for reviewing and
that I'll be glad to finally have infrastructure around them.

I had tried doing that many months ago, but I got a little carried away with
the details and adjacent changes and ended up not finishing it (and the setup
from this PR seems better anyway).

In any case, this could be merged as is by me. The following can be discussed
after this PR if you want, but I'm adding it here to possibly avoid moving the
base dir later (so that more git history is visible without --follow, etc).

@rusty-snake commented on Oct 27:

and the above files (which are the most relevant to this PR IMO) are kind of
lost in the middle of it, given the sort order of:

There's a Commits tab wink

Nice, I didn't know reviews could be done from there directly.

ci/

+1

I think I will go with just ci/ (no etc) or ci-tests/.

I think something like plain ci/ would be nice to later keep other
linting-related files and scripts (e.g.: githooks and custom linters for the
source code) nearby. Example:

ci/githooks/pre-commit
ci/githooks/pre-push
ci/c-standard-fmodes.sh
ci/c-wrapper-limits-h.sh
ci/private-etc-always-required.sh
ci/sort-disable-programs.sh
ci/sort-firecfg.config.sh

Some scripts could be later integrated in githooks, to be able to automatically
check for issues before e.g.: opening a PR.

Shorter names would also make it easier to use from the cli (though I'd like to
integrate them in make later either way, so that one could just run
make ci-check or something and only check changed files).

[rusty-snake]

I think something like plain ci/ would be nice to later keep other
linting-related files and scripts (e.g.: githooks and custom linters for the
source code) nearby

Makes sense, though I would put the test in a sub dir

• ci/tests/
• ci/checks/
• ci/profile-tests/
• ci/test/profiles/

[kmk3]

@rusty-snake commented on Oct 29:

I think something like plain ci/ would be nice to later keep other
linting-related files and scripts (e.g.: githooks and custom linters for the
source code) nearby

Makes sense, though I would put the test in a sub dir

Sounds good.

• ci/tests/
• ci/checks/
• ci/profile-tests/
• ci/test/profiles/

The last one looks neat, but to be pedantic, by profile testing, what mostly
comes to mind is checking whether a profile works by e.g.: running
firejail --profile=, while e.g.: sort-disable-programs.sh is about linting,
so I'd just s/test/check/:

• ci/check/profiles

Then we could have something like:

ci/check/c/only-standard-fmodes.sh
ci/check/c/wrapper-limits-h.sh
ci/check/etc/sort-firecfg.config.sh
ci/check/etc/sort-ids.config.sh
ci/check/profiles/private-etc-always-required.sh
ci/check/profiles/sort-disable-programs.sh
ci/githooks/pre-commit
ci/githooks/pre-push

And maybe later something like make ci-check-profiles.

Or:

ci/check/c/only-standard-fmodes.sh
ci/check/c/wrapper-limits-h.sh
ci/check/etc/private-etc-always-required.sh
ci/check/etc/sort-disable-programs.sh
ci/check/etc/sort-firecfg.config.sh
ci/check/etc/sort-ids.config.sh
ci/githooks/pre-commit
ci/githooks/pre-push

With e.g.: make ci-check-etc or make ci-check-profiles

Or:

ci/check/c/only-standard-fmodes.sh
ci/check/c/wrapper-limits-h.sh
ci/check/profiles/private-etc-always-required.sh
ci/check/profiles/sort-disable-programs.sh
ci/check/profiles/sort-firecfg.config.sh
ci/check/profiles/sort-ids.config.sh
ci/githooks/pre-commit
ci/githooks/pre-push

With e.g.: make ci-check-profiles

Alternatively, I think it would also make sense to put all in e.g.: ci/check/
for now and add the other subdirs later:

ci/check/private-etc-always-required.sh
ci/check/sort-disable-programs.sh
ci/check/sort-firecfg.config.sh
ci/check/sort-ids.config.sh