[PR #2935] [MERGED] Fix profile builder #4590

New issue

Closed

opened 2026-05-05 10:22:57 -06:00 by gitea-mirror · 0 comments

gitea-mirror commented

2026-05-05 10:22:57 -06:00

Owner

📋 Pull Request Information

Original PR: https://github.com/netblue30/firejail/pull/2935
Author: @crass
Created: 8/30/2019
Status: ✅ Merged
Merged: 9/15/2019
Merged by: @netblue30

Base: master ← Head: fix-profile-builder

📝 Commits (9)

dbff552 Profile builder helper should use correct firejail binary path.
9af2c14 Better debug handling.
6620aac Fix issue where strace output file path has leading space making it an invalid path.
1b02467 Allow libtrace preload library to use for trace output a logfile specified by the environment variable FIREJAIL_TRACEFILE or as the RUN_TRACE_FILE if it exists ortherwise use the console as before.
f6584ea Allow firejail --trace option to take an optional parameter which is the trace log file path. The trace log file will be created if it does not exist and then bind mounted to RUN_TRACE_FILE so that the sandboxed program can access it.
96505fd Update man page to note that --trace can now take an optional parameter.
02580c8 When running builder trace output should go to separate file because (1) trace output is logged to console, which is a pain to capture, and (2) it should not be mingled with program output anyway, which it was when sending to stdout.
742d2a2 Make sure that we are unprivileged before creating the trace log file.
99da774 Merge branch 'master' into fix-profile-builder

📊 Changes

8 files changed (+69 additions, -25 deletions)

View changed files

📝 src/common.mk.in (+5 -1)
📝 src/fbuilder/build_profile.c (+11 -10)
📝 src/firejail/firejail.h (+1 -0)
📝 src/firejail/fs_trace.c (+21 -0)
📝 src/firejail/main.c (+5 -0)
📝 src/include/rundefs.h (+1 -0)
📝 src/libtrace/libtrace.c (+18 -7)
📝 src/man/firejail.txt (+7 -7)

📄 Description

@netblue30
Per @smitsohu's suggestion, I've fixed the --build option to work with fix for --trace. This actually was a little more involved because the builder parses the trace output to build an rough profile. Since --trace outputs to console now and not stdout, the output would be harder to capture. So I've allowed --trace to accept an optional parameter which is a path to the trace log file. Now the builder logs to a logfile and parses that log file. This is actually better than before, where program, firejail debug, and trace output were all combined together.

One part I'm slightly concerned with is that --output has code that has a bit of validation on the given path and then pipes ftee to capture the output. Is the usage of ftee just to make sure all the output is capture from the beginning? It the purpose of the path validation just for better error messaging? Why are hardlinks not allowed? The git log provides no clues because this code predates the import into this git repo. I've some some tests to check that symbolic links to files the user doesn't have permission to access are not allowed to be accessed and that the given path is created and opened for writing as an unprivileged user (when run as an unprivileged user). I'm wondering if I'm missing a security issue?

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/netblue30/firejail/pull/2935 **Author:** [@crass](https://github.com/crass) **Created:** 8/30/2019 **Status:** ✅ Merged **Merged:** 9/15/2019 **Merged by:** [@netblue30](https://github.com/netblue30) **Base:** `master` ← **Head:** `fix-profile-builder` --- ### 📝 Commits (9) - [`dbff552`](https://github.com/netblue30/firejail/commit/dbff5520e049bd5a18bc5e26e3a41be6bf637a4a) Profile builder helper should use correct firejail binary path. - [`9af2c14`](https://github.com/netblue30/firejail/commit/9af2c14723da9a2e0559d36d37b3d480f77eeb6e) Better debug handling. - [`6620aac`](https://github.com/netblue30/firejail/commit/6620aac849bb6d95eece058bfca53d025721c862) Fix issue where strace output file path has leading space making it an invalid path. - [`1b02467`](https://github.com/netblue30/firejail/commit/1b02467adfef544c17d1e1606c0167777100c328) Allow libtrace preload library to use for trace output a logfile specified by the environment variable FIREJAIL_TRACEFILE or as the RUN_TRACE_FILE if it exists ortherwise use the console as before. - [`f6584ea`](https://github.com/netblue30/firejail/commit/f6584eaf3b5bfa166fed56b900dbda6629c4fbd3) Allow firejail --trace option to take an optional parameter which is the trace log file path. The trace log file will be created if it does not exist and then bind mounted to RUN_TRACE_FILE so that the sandboxed program can access it. - [`96505fd`](https://github.com/netblue30/firejail/commit/96505fd6765a124016cc7e64ea8191f38efb09a5) Update man page to note that --trace can now take an optional parameter. - [`02580c8`](https://github.com/netblue30/firejail/commit/02580c890f28766ffef0b9aa8d4f17fd7b8ab905) When running builder trace output should go to separate file because (1) trace output is logged to console, which is a pain to capture, and (2) it should not be mingled with program output anyway, which it was when sending to stdout. - [`742d2a2`](https://github.com/netblue30/firejail/commit/742d2a26ca5281b9d1b161011d92164a4f3dc90e) Make sure that we are unprivileged before creating the trace log file. - [`99da774`](https://github.com/netblue30/firejail/commit/99da7745bfd2a7c3a8c982e15b7d9b38e4df9b4b) Merge branch 'master' into fix-profile-builder ### 📊 Changes **8 files changed** (+69 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `src/common.mk.in` (+5 -1) 📝 `src/fbuilder/build_profile.c` (+11 -10) 📝 `src/firejail/firejail.h` (+1 -0) 📝 `src/firejail/fs_trace.c` (+21 -0) 📝 `src/firejail/main.c` (+5 -0) 📝 `src/include/rundefs.h` (+1 -0) 📝 `src/libtrace/libtrace.c` (+18 -7) 📝 `src/man/firejail.txt` (+7 -7) </details> ### 📄 Description @netblue30 Per @smitsohu's suggestion, I've fixed the `--build` option to work with fix for `--trace`. This actually was a little more involved because the builder parses the trace output to build an rough profile. Since `--trace` outputs to console now and not `stdout`, the output would be harder to capture. So I've allowed `--trace` to accept an optional parameter which is a path to the trace log file. Now the builder logs to a logfile and parses that log file. This is actually better than before, where program, firejail debug, and trace output were all combined together. One part I'm slightly concerned with is that `--output` has code that has a bit of validation on the given path and then pipes `ftee` to capture the output. Is the usage of `ftee` just to make sure all the output is capture from the beginning? It the purpose of the path validation just for better error messaging? Why are hardlinks not allowed? The `git log` provides no clues because this code predates the import into this git repo. I've some some tests to check that symbolic links to files the user doesn't have permission to access are not allowed to be accessed and that the given path is created and opened for writing as an unprivileged user (when run as an unprivileged user). I'm wondering if I'm missing a security issue? --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

gitea-mirror

2026-05-05 10:22:57 -06:00

closed this issue
added the
pull-request
label

gitea-mirror referenced this issue

2026-05-05 10:34:05 -06:00

[PR #4590] [MERGED] Read mount id also on legacy kernels #5199

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/firejail#4590

No description provided.

Rows
Columns