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

build: building with --disable-sandbox-check breaks tests #6619

Open
kmk3 opened this issue Jan 16, 2025 · 3 comments
Open

build: building with --disable-sandbox-check breaks tests #6619

kmk3 opened this issue Jan 16, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@kmk3
Copy link
Collaborator

kmk3 commented Jan 16, 2025

Description

Building with --disable-sandbox-check breaks the following tests:

  • test/appimage/appimage-v2.exp
  • test/environment/firejail-in-firejail.exp
  • test/environment/umask.exp

Steps to Reproduce

./configure --disable-sandbox-check

From test_main:

test_main build errors

make -C test appimage
make[1]: Entering directory '/home/runner/work/firejail/firejail/test'
cd appimage && ./appimage.sh 2>&1 | tee appimage.log
TESTING: AppImage v2 (test/appimage/appimage-v2.exp)
spawn /bin/bash
firejail --name=test --appimage hello-x86_64.AppImage
runner@fv-az700-649:~/work/firejail/firejail/test/appimage$ 
<rejail --name=test --appimage hello-x86_64.AppImage
Reading profile /etc/firejail/default.profile
Reading profile /etc/firejail/disable-common.inc
Reading profile /etc/firejail/disable-programs.inc
Reading profile /etc/firejail/landlock-common.inc

** Note: you can use --noprofile to disable default.profile **

firejail version 0.9.73

Parent pid 5002, child pid 5005

** Warning: dropping all Linux capabilities and setting NO_NEW_PRIVS prctl **

Mounting appimage type 2
Base filesystem installed in 65.65 ms
�]0;firejail hello-x86_64.AppImage �Child process initialized in 112.47 ms
Hello, World!
firejail: ../../src/firejail/util.c:1039: create_empty_dir_as_root: Assertion `s.st_uid == 0' failed.
Aborted (core dumped)
Hello, again!
TESTING ERROR 1

From test_environment:

test_environment build errors

TESTING: firejail in firejail - single sandbox (test/environment/firejail-in-firejail.exp)
spawn /bin/bash
firejail
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ firejail
Reading profile /etc/firejail/default.profile
Reading profile /etc/firejail/disable-common.inc
Reading profile /etc/firejail/disable-programs.inc
Reading profile /etc/firejail/landlock-common.inc

** Note: you can use --noprofile to disable default.profile **

firejail version 0.9.73

Parent pid 4728, child pid 4729
Base filesystem installed in 64.86 ms
�]0;firejail /bin/bash�Child process initialized in 103.44 ms
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ firejail
firejail: ../../src/firejail/util.c:1039: create_empty_dir_as_root: Assertion `s.st_uid == 0' failed.
Aborted (core dumped)
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ TESTING ERROR 2
TESTING: retain umask (test/environment/umask.exp)
spawn /bin/bash
firejail --noprofile
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ 
<ail/firejail/test/environment$ firejail --noprofile
firejail version 0.9.73

Parent pid 5204, child pid 5205
Base filesystem installed in 0.06 ms
�]0;firejail /bin/bash�Child process initialized in 6.42 ms
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ umask
0123
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ firejail
firejail: ../../src/firejail/util.c:1039: create_empty_dir_as_root: Assertion `(s.st_mode & 07777) == (mode)' failed.
Aborted (core dumped)
runner@fv-az1269-744:~/work/firejail/firejail/test/environment$ TESTING ERROR 2

Environment

  • Name/version/arch of the Linux kernel (uname -srm): Linux 6.5.0-1025-azure #26~22.04.1-Ubuntu SMP Thu Jul 11 22:33:04 UTC 2024 x86_64
  • Name/version of the Linux distribution (e.g. "Ubuntu 20.04" or "Arch Linux"):
    Ubuntu 22.04.5 LTS
  • Name/version of the C compiler (e.g. "gcc 14.1.1-1"): gcc 12.3.0-1ubuntu1~22.04
  • Name/version of the libc (e.g. "glibc 2.40-1"): libc-bin 2.35-0ubuntu3.8
  • Name/version of the Linux API headers (e.g. "linux-api-headers 6.10-1" on
    Arch Linux): ?
  • Version of the source code being built (git rev-parse HEAD):
    a53de49

Relates to:

@kmk3 kmk3 added the bug Something isn't working label Jan 16, 2025
@kmk3 kmk3 changed the title build: building with --disable-sandbox causes tests to fail build: building with --disable-sandbox breaks tests Jan 16, 2025
kmk3 added a commit that referenced this issue Jan 16, 2025
This reverts commit 5c6fa6a.

The commit in question causes `HAVE_SANDBOX_CHECK` to always be unset
(instead of only when `--disable-sandbox-check` is used), as its value
was being passed to the compiler through `MANFLAGS`.  Move the macro
back into `MANFLAGS` for simplicity.

Also, using `--disable-sandbox-check` breaks the tests and thus also
breaks CI (see #6619).

Relates to #6592.
@kmk3 kmk3 changed the title build: building with --disable-sandbox breaks tests build: building with --disable-sandbox-check breaks tests Jan 16, 2025
kmk3 added a commit that referenced this issue Jan 16, 2025
Clarify that it is only intended for development (and thus that it may
potentially cause issues).

Relates to #6592 #6619.
@powerjungle
Copy link
Contributor

Oh sorry, I totally forgot about the tests, my bad! 😓 I assumed that when the pull request tests pass, it's fine, but apparently not. I should've looked into it beforehand. I'll take a look now if I can contribute anything since my PR broke them.

@kmk3
Copy link
Collaborator Author

kmk3 commented Jan 20, 2025

Oh sorry, I totally forgot about the tests, my bad! 😓 I assumed that when
the pull request tests pass, it's fine, but apparently not.

--disable-sandbox-check modifies the source code, so a CI job would have to
explicitly build firejail with such a configuration in order to check whether
it passes the tests.

It would be nice to have a CI job for it, though there are many PRs open
already and I have a WIP branch that touches on related CI code, so please
avoid opening a PR for this for now.

I should've looked into it beforehand. I'll take a look now if I can
contribute anything since my PR broke them.

Besides the tests themselves, it's unclear to me if there are potential
security implications when using --disable-sandbox-check.

Could you look into it and see if/how it relates to the failed tests?

@powerjungle
Copy link
Contributor

Besides the tests themselves, it's unclear to me if there are potential
security implications when using --disable-sandbox-check.

I will look into it a bit deeper, but like I said in the PR introducing the argument, the reason I wanted it, was to develop and test firejail inside of a sandbox. I wouldn't want anyone to use it in production like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants