docs: mention risk of SUID binaries and also firejail-users(5) #5290
Conversation
kmk3
added
the
documentation
|
We could later expand on the tradeoffs as discussed on #4601; this is more just |
| @@ -68,6 +68,17 @@ Each profile defines a set of permissions for a specific application or group | |||
| of applications. The software includes security profiles for a number of more common | |||
| Linux programs, such as Mozilla Firefox, Chromium, VLC, Transmission etc. | |||
| .PP | |||
| Firejail is currently implemented as an SUID binary, which means that if a | |||
There was a problem hiding this comment.
Not a native speaker, but shouldn't it be 'a' SUID binary... ?
There was a problem hiding this comment.
Not a native speaker, but shouldn't it be 'a' SUID binary... ?
I'm fairly sure it depends on how you pronounce the word in question (at least
when spoken out loud; not sure about when writing manuals).
I personally read SUID spelled out as "S U I D" (that is, "ess you I d", in
which case "ess" starts with a vowel sound, and so "an" would be used), though
I can imagine someone saying it as "a soo'd binary" (that is, where "soo"
starts with a consonant sound and so "a" would be used). Not sure which one
would be the most common.
There was a problem hiding this comment.
I happenend to be on IRC and asked around. Everyone agreed your version is the better, correct one :-) Thanks for the explanation, learned something today!
src/man/firejail.txt
Outdated
| Firejail is currently implemented as an SUID binary, which means that if a | ||
| malicious or compromised user account manages to exploit a bug in Firejail, | ||
| that could ultimately lead to a privilege escalation to root. | ||
| To mitigate this, by default only the root user is allowed to run Firejail. |
There was a problem hiding this comment.
That is only true if the file /etc/firejail/firejail.users exists.
Most installations are not assumed to be used in this use-case, feels like an empty statement |
src/man/firejail.txt
Outdated
| that could ultimately lead to a privilege escalation to root. | ||
| To mitigate this, by default only the root user is allowed to run Firejail. | ||
| To allow more users, see firejail-users(5). | ||
| For more details on the security/usability tradeoffs of Firejail, see the |
There was a problem hiding this comment.
It is maybe more a personal preference, but I think that ideally the man pages should be self contained, and references to the internet should remain an exception.
Your description of the issue is good, there is probably no need to refer to another place.
And btw, thank you for tackling this.
There was a problem hiding this comment.
It is maybe more a personal preference, but I think that ideally the man
pages should be self contained, and references to the internet should remain
an exception.Your description of the issue is good, there is probably no need to refer to
another place.
Misc: Do you mean avoiding "link-only" descriptions, or avoiding links in
general? If the latter, we could talk about it in a new discussion.
I agree that they should be self-contained as much as possible (and that
eventually parts of #4601 should be added), however I think that giving the
user a more detailed explanation is important in this case because being SUID
is arguably one of the key security aspects of firejail and so users might be
weary of using it after reading about the privilege escalation possibility.
Also, it is where I would point users to anyway if they have questions about it
(or if someone entirely dismisses firejail because of it).
Additionally, #4601 is kind of a meta-discussion (as it lists other related
discussions) and it contains a lot of the rationale and other (potentially
useful) information, which seems too long to fit in a man page.
I thought about later adding a "security considerations" section to the man
page to explain things such as when firejail should be used vs something else
(as noted on #4601). I made a brief attempt at that, but failed to make it
clear and concise, hence this briefer PR.
And also maybe create a "configuration" or "post-install" section to guide the
user (similar to --guide, but in text form).
And btw, thank you for tackling this.
No problem!
On the introduction of firejail(1), mention the main risk of SUID binaries and that by default, only trusted users should be allowed to run firejail (and how to accomplish that). Note: The added comment line is completely discarded (so there is no extraneous blank line); see groff_man(7) for details. Suggested by @emerajid on netblue30#5288. Relates to netblue30#4601.
kmk3
force-pushed
the
docs-suid-firejail-users
branch
from
e076cc0
to
ba0ac27
Compare
@SkewedZeppelin commented on Aug 3:
Sorry; I failed on reading firejail-users(5) myself it seems. I really thought that users had to be configured on the file (that's what I Updated and force-pushed. |
Since the man pages in src/man use a ".txt" file extension (rather than
".1" or ".5"), their filetype is detected by (neo)vim as "text".
So at the bottom of every man page, add a vim modeline in a comment and
set the filetype to "groff", to enable syntax highlighting.
Note: All of the generated ".man", ".1" and ".5" files are currently
being detected as "nroff".
Note2: Set the filetype to "groff" rather than "nroff" because at least
.UR and .UE are groff extensions. These macros look the same with
either filetype, but there may be more extensions being used and the
nroff.vim syntax file (which is included by groff.vim) does things
differently based on which filetype is used.
Based on the following example from (neo)vim's filetype.txt:
or add this modeline to the file:
/* vim: set filetype=idl : */
See `:help groff.vim` and `:help filetype.txt` in (neo)vim.
See also groff_man(7) for the man page macros (including extensions).
Environment: neovim 0.7.2-3 on Artix Linux.
Misc: I noticed this on netblue30#5290.
|
merged! |
On the introduction of firejail(1), mention the main risk of SUID
binaries and that by default, only trusted users should be allowed to
run firejail (and how to accomplish that).
Note: The added comment line is completely discarded (so there is no
extraneous blank line); see groff_man(7) for details.
Suggested by @emerajid on #5288.
Relates to #4601.