netblue30 / firejail

Linux namespaces and seccomp-bpf sandbox
https://firejail.wordpress.com
GNU General Public License v2.0
5.81k stars 567 forks source link

Paths in app profiles with `^` and `!` wildcards in them are not expanded #3862

Open Snugface opened 3 years ago

Snugface commented 3 years ago

Bug and expected behavior

Reproduce Steps to reproduce the behavior:

  1. Make a local FF profile:
    cp /etc/firejail/firefox.profile ~/.config/firejail/
    cp /etc/firejail/firefox-common.profile ~/.config/firejail/
  2. Create test directory structure & files:
    mkdir -p ~/parent_dir/child_dir{1..10}
    touch ~/parent_dir/file{1..10}
  3. Add additional lines to ~/.config/firejail/firefox-common.profile after the line include whitelist-var-common.inc:
    whitelist ${HOME}/parent_dir
    blacklist ${HOME}/parent_dir/[^.]*/
    blacklist ${HOME}/parent_dir/.[^.]*/
  4. Run firejail firefox

Environment

Additional context

Checklist

debug output ``` Autoselecting /bin/bash as shell Building quoted command line: 'firefox' Command name #firefox# Found firefox.profile profile in /home/eviscerator/.config/firejail directory Reading profile /home/eviscerator/.config/firejail/firefox.profile Found whitelist-usr-share-common.inc profile in /etc/firejail directory Reading profile /etc/firejail/whitelist-usr-share-common.inc Found firefox-common.profile profile in /home/eviscerator/.config/firejail directory Reading profile /home/eviscerator/.config/firejail/firefox-common.profile Found disable-common.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-common.inc Found disable-devel.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-devel.inc Found disable-exec.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-exec.inc Found disable-interpreters.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-interpreters.inc Found disable-programs.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-programs.inc Found whitelist-common.inc profile in /etc/firejail directory Reading profile /etc/firejail/whitelist-common.inc Found whitelist-var-common.inc profile in /etc/firejail directory Reading profile /etc/firejail/whitelist-var-common.inc Error: "${HOME}/parent_dir/[^.]*/" is an invalid filename: rejected character: "^" ```
rusty-snake commented 3 years ago

man 7 glob: You can do [!.]* however Error: "${HOME}/parent_dir/.[!.]*/" is an invalid filename: rejected character: "!".

rusty-snake commented 3 years ago

For me this raises two questions:

  1. Can we allow ! (after [) in paths?
  2. Do we want to reimplement path handling using e.g. pcre?
kmk3 commented 3 years ago

@Snugface commented on Jan 1:

Additional context

  • I wanted to blacklist every child directory of ~/parent_dir, and whitelist the parent_dir itself and all files in it. Glob pattern blacklist ${HOME}/parent_dir/*/ was used. This also blacklisted ${HOME}/parent_dir/.. which is the $HOME itself. Which broke firefox and it failed to execute.

  • Attempted a workaround with [^.]*/ and .[^.]*/ wildcards instead of */. The two glob patterns list all regular and . directories except /.. if used directly from a bash shell(such as with echo [^.]*/) However, upon starting firefox I get: Error: "${HOME}/parent_dir/[^.]*/" is an invalid filename: rejected character: "^"

  • I don't really care whether ^ and ! work - it's just important for me to be able to use wildcards to blacklist/whitelist all child directories, but not all files, in a specific parent directory. So if you know a specific recipe for this - please, share it. I'd be glad to tip you some $10-20 if there will be a workaround.

I think it would be simpler to just fix globbing to ignore /..$ and /.$.

@rusty-snake commented 10 hours ago:

For me this raises two questions:

  1. Can we allow ! (after [) in paths?

  2. Do we want to reimplement path handling using e.g. pcre?

PCRE is quite large/complex; POSIX ERE already supports [^.]:

If you really want to use an external implementation, I'd suggest using a smaller one. Examples:

This one seems to be more common (it's used by jq and PHP), but it's on the larger side:

And there's a fork that's used by Ruby, though on Arch it only exists in the AUR:

rusty-snake commented 3 years ago

PCRE is quite large/complex

You're right, however for me:

$ ldd /usr/bin/firejail
    linux-vdso.so.1 (0x00007ffdb2f40000)
    libselinux.so.1 => /lib64/libselinux.so.1 (0x00007c1c43488000)
    libc.so.6 => /lib64/libc.so.6 (0x00007c1c432b9000)
    libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x00007c1c43222000)  <---
    libdl.so.2 => /lib64/libdl.so.2 (0x00007c1c4321b000)
    /lib64/ld-linux-x86-64.so.2 (0x00007c1c4362d000)
kmk3 commented 3 years ago

PCRE is quite large/complex

You're right, however for me:

$ ldd /usr/bin/firejail
  linux-vdso.so.1 (0x00007ffdb2f40000)
  libselinux.so.1 => /lib64/libselinux.so.1 (0x00007c1c43488000)
  libc.so.6 => /lib64/libc.so.6 (0x00007c1c432b9000)
  libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x00007c1c43222000)  <---
  libdl.so.2 => /lib64/libdl.so.2 (0x00007c1c4321b000)
  /lib64/ld-linux-x86-64.so.2 (0x00007c1c4362d000)
$ ldd /usr/bin/firejail
        linux-vdso.so.1 (0x00007ffd92b8c000)
        libapparmor.so.1 => /usr/lib/libapparmor.so.1 (0x00007fee1f8b0000)
        libc.so.6 => /usr/lib/libc.so.6 (0x00007fee1f6e0000)
        libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007fee1f6b8000)
        /lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007fee1fa70000)
rusty-snake commented 3 years ago

./configure without --enable-selinux:

$ ldd /usr/bin/firejail
    linux-vdso.so.1 (0x00007fffdd140000)
    libc.so.6 => /lib64/libc.so.6 (0x000077878c5b9000)
    /lib64/ld-linux-x86-64.so.2 (0x000077878c900000)

./configure with --enable-selinux:

$ ldd /usr/bin/firejail
    linux-vdso.so.1 (0x00007fffd43e9000)
    libselinux.so.1 => /lib64/libselinux.so.1 (0x000071786a3f7000)
    libc.so.6 => /lib64/libc.so.6 (0x000071786a228000)
    libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x000071786a191000)
    libdl.so.2 => /lib64/libdl.so.2 (0x000071786a18a000)
    /lib64/ld-linux-x86-64.so.2 (0x000071786a59c000)
kmk3 commented 3 years ago

./configure without --enable-selinux:

$ ldd /usr/bin/firejail
  linux-vdso.so.1 (0x00007fffdd140000)
  libc.so.6 => /lib64/libc.so.6 (0x000077878c5b9000)
  /lib64/ld-linux-x86-64.so.2 (0x000077878c900000)

./configure with --enable-selinux:

$ ldd /usr/bin/firejail
  linux-vdso.so.1 (0x00007fffd43e9000)
  libselinux.so.1 => /lib64/libselinux.so.1 (0x000071786a3f7000)
  libc.so.6 => /lib64/libc.so.6 (0x000071786a228000)
  libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x000071786a191000)
  libdl.so.2 => /lib64/libdl.so.2 (0x000071786a18a000)
  /lib64/ld-linux-x86-64.so.2 (0x000071786a59c000)

I see, it's a dependency of SELinux, but I don't really get what exactly is your point.

rusty-snake commented 3 years ago

Just to clear what causes the different ldd outputs.I didn't know this before.

kmk3 commented 3 years ago

Just to clear what causes the different ldd outputs.I didn't know this before.

Ah alright; I thought that you were arguing in favor of using PCRE directly.