mirror of
https://github.com/netblue30/firejail.git
synced 2026-05-15 14:16:14 -06:00
[GH-ISSUE #5406] An exec directive #2983
Labels
No labels
LTS merge
LTS merge
bug
bug
converted-to-discussion
doc-todo
documentation
duplicate
enhancement
file-transfer
firecfg
firejail-in-firejail
firetools
graphics
help wanted
information_old
installation
invalid
modif
moved
needinfo
networking
notabug
notourbug
old-version
overlayfs
packaging
profile-request
pull-request
question
question_old
removal
runtime-permissions
sandbox-ipc
security
stale
wiki
wiki
wontfix
wordpress
workaround
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/firejail#2983
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @WhyNotHugo on GitHub (Oct 7, 2022).
Original GitHub issue: https://github.com/netblue30/firejail/issues/5406
Is your feature request related to a problem? Please describe.
I'd like to allow
firefoxto launchimv, butimvshould run in its own sandbox (with its own profile).Describe the solution you'd like
An
execdirective in profiles. So by specifyingexec imvinfirefox.localfirejail will allow launching$PATH/imv, butimvwill run withimv.profileand in its own sandbox.Because more
imv.profilemight have a wider set of permissions, this needs to be done outside of firefox's sandbox.For this, I'd propose having a pair of sockets (one inside the sandbox, one another), where a sandbox-external process waits for messages where another process inside the sandbox writes
imv $FILE. The program inside file would be located in/usr/bin/imv(shadowing the real one, if necessary), and would pass args without shelling out.Describe alternatives you've considered
D-Bus, but I think it'd be more work for little benefit.
Additional context
There's 485 issues matching results, and I haven't found any mention on an idea like this before. Opening this as a point of discussion, I'd love to hear other ideas.
@rusty-snake commented on GitHub (Oct 7, 2022):
execwill be confused withnoexecby a lot of users, it has be a different name.@rusty-snake commented on GitHub (Oct 7, 2022):
See also #3785 and #4557.
Such an socket can be used for more, see #5364.
@WhyNotHugo commented on GitHub (Oct 7, 2022):
I can write the helpers that sit on either side of the socket. Implementing
xdg-openis also in my mind, though that would be a separate directive (I've some security concerns for it too).Regarding messages sent over the socket, each message is serialized using UTF-8 encoded and is preceded with an unsigned 32-bit value containing the message length in native byte order. The messages are the command, followed by arguments (all as strings).
@WhyNotHugo commented on GitHub (Oct 10, 2022):
That format for sending the message didn't work, because it's impossible to distinguish arguments that have a space vs multiple args. I could use quotes, but then real quotes have to be escape, and at that point, I'm just re-inventing a serialisation mechanism.
Trying msgpack instead. It's standard, efficient, and has all these issues solved.
@rusty-snake commented on GitHub (Oct 10, 2022):
Die you used spaces to separate arguments? The usual way are null bytes.
Also note that implementing stuff before discussing and agreeing on them might be waste of time.
@WhyNotHugo commented on GitHub (Oct 10, 2022):
I guess null bytes could work, yes.
I'm mostly implementing a proof of concept to validate the ideas so far.
@topimiettinen commented on GitHub (Oct 10, 2022):
Also #3315 is doing something similar.
@kmk3 commented on GitHub (Oct 11, 2022):
@WhyNotHugo commented on Oct 10:
So there would be a client running inside the sandbox and a daemon/server
running outside of it?
Request
@WhyNotHugo commented on Oct 10:
Considering the use case, I don't think that performance would be a big
concern, so maybe the messages could be sent in plain text (UTF-8), which might
be easier to implement/inspect/debug. The arguments could then be separated by
newlines.
That is, maybe the request could be something like this (using
firestarter/firerunner as a placeholder name for the functionality):
(Alternatively, use
firestarter=1 RUNin the first line for consistency; seethe Response section)
It uses a similar format to an HTTP 1.1 request:
That is, the first blank line signifies the end of the environment variables
and the second one signifies the end of the message. Use environment variables
as the "headers" to make it easier to use on a CLI (at least for parsing the
response, or using
evalon it). Protocol-specific parameters could bespecified using variables prefixed with
FIRESTARTER_. Maybe embeddednewlines could be allowed to be sent escaped with a backslash (though such
lines should be very unlikely to occur).
Misc: It is kind of analogous to a call to
execve, but with the environmentbeing specified first:
Filesystem
To simplify the access control and talking to multiple sandboxes at once, the
communication could happen through sockets/pipes in a "PID directory".
For example, when running
firejail program1, if program1 has permission tostart new sandboxes and it has a PID of 1000, then the following directory
would be created by firejail before program1 is executed:
And when trying to start a new sandbox, the request could be sent by writing
to:
Note: The base path (/run/firestarter/1000) could be owned by root:root and
have 700 permissions outside the sandbox (to prevent unnecessary access from
unrelated processes) and be bind-mounted 755 (with a 755 umask) inside of it.
Then the server could read it and run something like
execveorposix_spawnwith the arguments.
Response
The response could be similar to an HTTP 1.1 response:
Example (not a real response; this is just for showing the format):
Alternatively, put the error code and message in their own lines to make it
easier to run
evalon it (which should be helpful for debugging):The error codes/messages could be based on either errno.h or on HTTP response
codes (I didn't give this part too much thought).
IPC
One consideration would be how to return stdout/stderr. Maybe create one pipe
for each and print their paths in the response? Example (assuming that the PID
of the new sandbox is 1005):
IIRC Arcan does something kind of similar by using input/output
named pipes as communication streams for IPC.
Note that this should leave most the burden (such as of connecting the streams)
to the client, which should be the less privileged part. Note also that this
makes the filesystem reflect the "process tree" of the sandboxes.
Then when the program exits, the server could write the exit status code (such
as
0) to a pipe, such as:No idea about propagating signals though.
@rusty-snake commented on GitHub (Oct 11, 2022):
In order to make future use-cases for this socket easier I would split the protocol into two layers:
Layer 1:
enumdiscriminant.Layer 2:
Depending on Message Type, Message Data is interpreted, parsed, ...
For a
EXTERN_EXECVEmessage type we can go with @kmk3 s HTTP 1.1 like text based format. Howeverset-env FOO=barallows future addition.If we do responses we use
SOCK_STREAM, right?How do you know the PID? Do we
createbind the socket and create the directory after forking?How much IPC do we want to support and where is the point where it becomes to complex? Not only signals, also open file-descriptors, PIDs, UIDs, filesystem layout, Network, ...
IMHO we should exclude IPC from the first implementation as it is to complex. We should focus on a working way to spawn independent programs (i.e. browser, email-client, image viewer, video-player, ...) and only keep the format open for future IPC.
Do we want all this message passing and parsing in C? Or Rust? #4386
Since this is a very isolated feature in the code base, it would be a good feature to bring in rust without touching FFI topics.
@WhyNotHugo commented on GitHub (Oct 11, 2022):
@kmk3
There would be a daemon/server outside the sandbox. Inside the sandbox the client doesn't need to run continuously. For my current experiment, I'm installing my client in
/usr/bin/imv, and it merely sends a message to the daemon outside saying basicallyEXEC imv $@(the name of the command comes from$0so the same binary can be linked to any other command that would be allowed.The daemon merely listens to a socket that is placed inside the sandbox is a well-known location (
/var/run/firejail/controlfor now, but it can be anywhere).The location outside the sandbox is not important, it only needs to be unique.
Keep in mind that arguments can contain newlines.
Regarding stdout/stdin: I'd rather leave it out-of-scope for now. The intended usecase is for Firefox to be able to execute an image viewer, or being able to click on links on any firejail'ed application and this gets passed onto Firefox in its own sandbox. Generally, these are "run and forget" style of execution, and don't care about pipes or exit code.
If/when we want to connect stdout/stderr/stdin, the easiest thing would be to send three file descriptors over the socket (as a response to the
RUNcommand). stdout/stderr/stdin are already file descriptors, and sending them over a socket is pretty simple.I haven't kept signals in mind either, but an additional FD to which one can simply write an unsigned 32-bit value (the signal ID itself) might work. Using an FD prevents any PID reuse attacks or similar race conditions too.
@rusty-snake Agree on pretty much everything. I'd prefer Rust mostly because I'm rather fluent in it and my C is terrible (as you may have seen).
@rusty-snake commented on GitHub (Oct 11, 2022):
Minimal, ugly and unfinish PoC for my Layer 1 protocol using
SOCK_STREAMand Rust.@WhyNotHugo commented on GitHub (Oct 11, 2022):
f58b20431d/src/lib.rs (L23-L26)Why do you need this union?
@rusty-snake commented on GitHub (Oct 11, 2022):
f58b20431d/src/bin/server.rs (L13-L19)@WhyNotHugo commented on GitHub (Oct 11, 2022):
I've seen this. You want std::mem::transmute, to convert from one type to another without re-allocating.
@WhyNotHugo commented on GitHub (Oct 11, 2022):
But for that, both data types must have the same length, which they should.
@WhyNotHugo commented on GitHub (Oct 11, 2022):
You should read the first byte (e.g.:
size) and then read that many bytes into the L2 type/payload.@WhyNotHugo commented on GitHub (Oct 11, 2022):
Sample reading from a socket like that:
0c680046f9/src/main.rs (L26-L37)@rusty-snake commented on GitHub (Oct 11, 2022):
Semantically they are the same.
If you speak about heap allocation, there is no.
If you speak about stack allocation, I don't think we need to worry about that.
617a149c0b@WhyNotHugo commented on GitHub (Oct 11, 2022):
Sorry, I meant the first u32.
@WhyNotHugo commented on GitHub (Oct 11, 2022):
read_to_endhas potential issues; if multiple messages are sent at once, they might be read into one buffer, and you'll lose the second message.There are other corner cases like this, hence the suggestion to read first that u32 first.
@rusty-snake commented on GitHub (Oct 11, 2022):
I know them. It's a "Minimal, ugly and unfinish PoC". There are othere things I wanted to focus/check/test. It's a "Minimal, ugly and unfinish PoC". The hole message passing is a PoC. MessageData is 4096 bytes in size, if the arguments of a program are longer than that, buff, if you want to transfer one byte it add 4095 zeros. And that's all on a connection oriented socket, if it would be a Datagram socket, ok, but here. It's a "Minimal, ugly and unfinish PoC".
And how should I do it? From which type should I transmute to which?
In rust you can do (from "safer" but still unsafe, to unsafer and more powerful):
transmute/transmute_copyunionYou can not
transmute::<&[u8], &Message>. So which types should I transmute to safe allocation? If you would say I should use raw pointer casts, ok, but with transmute?@WhyNotHugo commented on GitHub (Oct 11, 2022):
Actually, raw pointer cast is what I was looking for and couldn't remember 😅
That would be much simpler than the union since it's just a single (unsafe) call.
@rusty-snake commented on GitHub (Oct 11, 2022):
And I Actually wanted to use the union in the
from_bytesand theas_bytesmethod. But it does not work foras_bytes(it would if the union would use the borrowed types).