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

feature: firecfg: add firecfg.d & add ignore command #5876

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented Jul 1, 2023

Main changes:

  • firecfg: parse config files in /etc/firejail/firecfg.d

/etc/firejail/firecfg.d/*.conf files are parsed before
/etc/firejail/firecfg.config, so the former can ignore/override any item in the
latter.

  • Add ignore command (!PROGRAM)

It prevents firecfg from creating a symlink for the given program.

Also, document the paths used and the config file syntax.

Suggested by @WhyNotHugo:

Closes #2097.

@kmk3 kmk3 marked this pull request as draft July 1, 2023 19:13
@kmk3
Copy link
Collaborator Author

kmk3 commented Jul 1, 2023

A few considerations:

Instead of !PROGRAM, I thought about using ignore PROGRAM for better
readability and to be consistent with firejail, but ignore works slightly
differently in firejail:

  • static int is_in_ignore_list(char *ptr) {
    // check ignore list
    int i;
    for (i = 0; i < MAX_PROFILE_IGNORE; i++) {
    if (cfg.profile_ignore[i] == NULL)
    break;
    int len = strlen(cfg.profile_ignore[i]);
    if (strncmp(ptr, cfg.profile_ignore[i], len) == 0) {
    // full word match
    if (*(ptr + len) == '\0' || *(ptr + len) == ' ')
    return 1; // ignore line
    }
    }
    return 0;
    }

It only compares up to the first space, so it considers ignore foo and
ignore foo bar to be equivalent, for example. That makes sense for ignoring
commands with arguments (like ignore name ignores name foo), but not so
much for paths.

To avoid confusion (if a program name has a space in it for whatever reason),
it seems like a good idea to use a different name/syntax.

Other than that, I'd say it's fairly straightforward.

@kmk3 kmk3 added enhancement New feature request firecfg Anything related to firecfg and not firejail itself labels Jul 1, 2023
Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to make it explicit in release notes that this introduces a dependency on zenity to the firejail package.

@kmk3
Copy link
Collaborator Author

kmk3 commented Jul 3, 2023

We might need to make it explicit in release notes that this introduces a
dependency on zenity to the firejail package.

The zenity-related code is already there and is only executed if --guide is
used:

  • firejail/src/firecfg/main.c

    Lines 449 to 464 in 33c75b8

    if (arg_guide) {
    char *cmd;
    if (arg_debug) {
    if (asprintf(&cmd, "sudo %s/firejail/firejail-welcome.sh /usr/lib/firejail/fzenity %s %s", LIBDIR, SYSCONFDIR, user) == -1)
    errExit("asprintf");
    }
    else {
    if (asprintf(&cmd, "sudo %s/firejail/firejail-welcome.sh /usr/bin/zenity %s %s", LIBDIR, SYSCONFDIR, user) == -1)
    errExit("asprintf");
    }
    int status = system(cmd);
    if (status == -1) {
    fprintf(stderr, "Error: cannot run firejail-welcome.sh\n");
    exit(1);
    }
    free(cmd);

It was added in the following commits:

  • 62e33cf ("more on firecfg --guide", 2022-04-21)
  • 1cdfa6f ("more on firecfg --guide: fzenity", 2022-04-25)

This PR only moves such paths to constants; see commit 1f07de8 ("firecfg:
turn constant strings into constants", 2023-06-30).

@glitsj16
Copy link
Collaborator

glitsj16 commented Jul 3, 2023

@kmk3 Thank you for clearing up this zenity situation in such a detailed manner.

@kmk3 kmk3 marked this pull request as ready for review July 5, 2023 18:59
@netblue30
Copy link
Owner

Cool job! If you don't have anything coming, check it in!

Changes:

* fix inconsistent indentation/braces
* add missing free
Instead of using asprintf + free.

Also, use LIBDIR instead of hardcoded "/usr/lib" for fzenity.
Add ignore command (`!PROGRAM`), as suggested by @WhyNotHugo[1].

It prevents firecfg from creating a symlink for the given program.

Also, document the paths used and the config file syntax.

Note that `/etc/firejail/firecfg.d/*.conf` files are parsed before
/etc/firejail/firecfg.config, so the former can ignore/override any item
in the latter.

Closes netblue30#2097.

[1] netblue30#2097 (comment)
@kmk3 kmk3 force-pushed the firecfg-add-confdir-ignore branch from 2822b51 to ef6cfb8 Compare August 4, 2023 20:26
@netblue30 netblue30 marked this pull request as ready for review December 4, 2023 14:10
@netblue30 netblue30 merged commit 2033e98 into netblue30:master Dec 4, 2023
9 checks passed
@netblue30
Copy link
Owner

All merged, thanks!

@kmk3 kmk3 deleted the firecfg-add-confdir-ignore branch January 4, 2024 00:50
kmk3 added a commit that referenced this pull request Jan 4, 2024
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 11, 2024
Currently it is only used when parsing the configuration files:

* /etc/firecfg.d/*.conf
* /etc/firecfg.config

Use it when searching for profile filenames as well:

* ~/.config/firejail/*.profile

Relates to netblue30#5876.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 11, 2024
Changes:

* Export `in_ignorelist` function
* Allow only building the ignorelist without setting the symlinks
* Rename the functions to reflect the above
* Add a function that parses all config files (`parse_config_all`)

Also, make sure that `parse_config_all` only parses config files once,
even if called multiple times.

Relates to netblue30#5876.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 11, 2024
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 11, 2024
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 11, 2024
Changes:

* Export `in_ignorelist` function
* Allow only building the ignorelist without setting the symlinks
* Rename the functions to reflect the above
* Add a function that parses all config files (`parse_config_all`)

Also, make sure that `parse_config_all` only parses config files once,
even if called multiple times.

Relates to netblue30#5876.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 11, 2024
@kmk3 kmk3 mentioned this pull request May 1, 2024
kmk3 added a commit that referenced this pull request Sep 10, 2024
Move the "FILES" section to right before the "LICENSE" section in
firecfg.1.in, to match what is done in the other man pages.

This amends commit ef6cfb8 ("firecfg: add ignore command and docs",
2023-06-29) / PR #5876.

Relates to #6451.
kmk3 added a commit that referenced this pull request Sep 10, 2024
Move the "FILES" section to right before the "LICENSE" section in
firecfg.1.in, to match what is done in the other man pages.

This amends commit ef6cfb8 ("firecfg: add ignore command and docs",
2023-06-29) / PR #5876.

Relates to #6451.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request firecfg Anything related to firecfg and not firejail itself
Projects
Status: Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

firecfg: allow for ignoring specific apps
3 participants