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

An exec directive #5406

Open
WhyNotHugo opened this issue Oct 7, 2022 · 23 comments
Open

An exec directive #5406

WhyNotHugo opened this issue Oct 7, 2022 · 23 comments
Labels
enhancement New feature request sandbox-ipc Opening links and talking to programs outside of the sandbox (see #6462)

Comments

@WhyNotHugo
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I'd like to allow firefox to launch imv, but imv should run in its own sandbox (with its own profile).

Describe the solution you'd like

An exec directive in profiles. So by specifying exec imv in firefox.local firejail will allow launching $PATH/imv, but imv will run with imv.profile and in its own sandbox.

Because more imv.profile might 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
Copy link
Collaborator

exec will be confused with noexec by a lot of users, it has be a different name.

@rusty-snake
Copy link
Collaborator

See also #3785 and #4557.

Such an socket can be used for more, see #5364.

@rusty-snake rusty-snake added the enhancement New feature request label Oct 7, 2022
@WhyNotHugo
Copy link
Contributor Author

I can write the helpers that sit on either side of the socket. Implementing xdg-open is 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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

I guess null bytes could work, yes.

I'm mostly implementing a proof of concept to validate the ideas so far.

@topimiettinen
Copy link
Collaborator

Also #3315 is doing something similar.

@kmk3
Copy link
Collaborator

kmk3 commented Oct 11, 2022

@WhyNotHugo commented on Oct 10:

I can write the helpers that sit on either side of the socket. Implementing
xdg-open is 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).

So there would be a client running inside the sandbox and a daemon/server
running outside of it?

Request

@WhyNotHugo commented on Oct 10:

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.

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):

firestarter/1 RUN
a=1
b=2

myprogram
--arg1=hello foo
--arg2=hello bar

(Alternatively, use firestarter=1 RUN in the first line for consistency; see
the Response section)

It uses a similar format to an HTTP 1.1 request:

  • Request line (protocol version and command/verb)
  • [Environment variables]
  • (blank line)
  • [Content]
  • (blank line)

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 eval on it). Protocol-specific parameters could be
specified using variables prefixed with FIRESTARTER_. Maybe embedded
newlines 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 environment
being specified first:

int
execve(const char *path, char *const argv[], char *const envp[]);

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 to
start new sandboxes and it has a PID of 1000, then the following directory
would be created by firejail before program1 is executed:

  • /run/firestarter/1000

And when trying to start a new sandbox, the request could be sent by writing
to:

  • /run/firestarter/1000/run

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 execve or posix_spawn
with the arguments.

Response

The response could be similar to an HTTP 1.1 response:

  • Status line (protocol version, code and message)
  • [Environment variables]
  • (blank line)
  • [Content]
  • (blank line)

Example (not a real response; this is just for showing the format):

firestarter/1 0 Success
foo=1
bar=2


Alternatively, put the error code and message in their own lines to make it
easier to run eval on it (which should be helpful for debugging):

firestarter=1 # 0 Success
errcode=0
errmsg="Success"
foo=1
bar=2


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):

firestarter=1 # 0 Success
errorcode=0
errormsg="Success"
parentpid=1000
childpid=1005
stdin=/run/firestarter/1000/1005/stdin
stdout=/run/firestarter/1000/1005/stdout
stderr=/run/firestarter/1000/1005/stderr
exit=/run/firestarter/1000/1005/exit
run=/run/firestarter/1000/1005/run


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:

  • /run/firestarter/1000/1005/exit

No idea about propagating signals though.

@rusty-snake
Copy link
Collaborator

Request

In order to make future use-cases for this socket easier I would split the protocol into two layers:

Layer 1:

+ ----------------+---------------+----------------+
|  Message Length | Message Type  | Message Data   |
|                 |               |                |
|  uint32_t (ne)  | uint32_t (ne) | char [MSG_MAX] |
+-----------------+---------------+----------------+
  • Message Length
    • Length of Message Data
  • Message Type
    • An enum discriminant.
    • Interpretation of Message Data depends on this value
  • Message Data
    • The actual data to send, interpret and act on deepening on Message Type.
    • It has a variable length with an maximum (maybe 4096 - 64).

Layer 2:

Depending on Message Type, Message Data is interpreted, parsed, ...

For a EXTERN_EXECVE message type we can go with @kmk3 s HTTP 1.1 like text based format. However

  • We need to escape newlines then (which can be used in arguments or environment variables). Otherwise we can also do nul-sepearted "lines" like the most programs which can read arguments from an file-descriptor.
  • The Header can be even more HTTP like. Using set-env FOO=bar allows future addition.

Response

If we do responses we use SOCK_STREAM, right?

Filesystem

How do you know the PID? Do we create bind the socket and create the directory after forking?

IPC

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.

Implementation

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
Copy link
Contributor Author

@kmk3

So there would be a client running inside the sandbox and a daemon/server
running outside of it?

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 basically EXEC imv $@ (the name of the command comes from $0 so 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/control for now, but it can be anywhere).

The location outside the sandbox is not important, it only needs to be unique.

The arguments could then be separated by newlines.

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 RUN command). 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
Copy link
Collaborator

Minimal, ugly and unfinish PoC for my Layer 1 protocol using SOCK_STREAM and Rust.

@WhyNotHugo
Copy link
Contributor Author

@WhyNotHugo
Copy link
Contributor Author

I've seen this. You want std::mem::transmute, to convert from one type to another without re-allocating.

@WhyNotHugo
Copy link
Contributor Author

But for that, both data types must have the same length, which they should.

@WhyNotHugo
Copy link
Contributor Author

You should read the first byte (e.g.: size) and then read that many bytes into the L2 type/payload.

@WhyNotHugo
Copy link
Contributor Author

@rusty-snake
Copy link
Collaborator

rusty-snake commented Oct 11, 2022

You should read the first byte (e.g.: size) and then read that many bytes into the L2 type/payload.

  1. It's a "Minimal, ugly and unfinish PoC"
  2. Using the first byte means maximum of 255 bytes in the message.

I've seen this. You want std::mem::transmute, to convert from one type to another without re-allocating.

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.


rusty-snake/extern-execve-poc@617a149

@WhyNotHugo
Copy link
Contributor Author

Using the first byte means maximum of 255 bytes in the message.

Sorry, I meant the first u32.

@WhyNotHugo
Copy link
Contributor Author

read_to_end has 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
Copy link
Collaborator

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".

You want std::mem::transmute, to convert from one type to another

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_copy
  • union
  • raw pointer casts

You 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
Copy link
Contributor Author

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
Copy link
Collaborator

And I Actually wanted to use the union in the from_bytes and the as_bytes method. But it does not work for as_bytes (it would if the union would use the borrowed types).

@kmk3 kmk3 added the sandbox-ipc Opening links and talking to programs outside of the sandbox (see #6462) label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request sandbox-ipc Opening links and talking to programs outside of the sandbox (see #6462)
Projects
None yet
Development

No branches or pull requests

4 participants