Add new casper execution service

[yzhong_freebsdfoundation.org]

Summary

Added a casper execution service. Provides an interface for user to:

• define a set of allowed executables.
• execute command if allowed, returns a file descriptor back to user.

The test file attached has an example of how to use cap_exec.

Test Plan

Run the tests in the tests directory.

[tig_freebsdfoundation.org]

Updated license.

[yzhong_freebsdfoundation.org]

In the process of Capsicumizing sort(1), I found that I couldn't properly close the file descriptors that came from this service. It used to return the file descriptor of a FILE* opened via popen(3). This meant that the user can't use pclose to close the file descriptor they receive; pclose expects the same FILE * that popen returned, which the user can't recover. Pclose is responsible for waiting until the opened process ends, and if it isn't called, the user will get concurrency issues (like writing to a file, "closing" it, and then trying to read from it before the write actually finishes).

Now, cap_exec comes with matching open and close functions. cap_exec_open() (previously just cap_exec) now uses pdfork(2) and pipe(2) instead of popen to run the command - the process descriptor from pdfork and the file descriptor from pipe are stored together in an internal list structure. cap_exec_close() needs the process descriptor so that it can use kqueue(2) to wait for the process to finish executing. (We can't use wait() here because this solution is implemented in the sandboxed part of the program.)

I also changed the service to return FILE*s instead of file descriptors. I suspect that a common use case for this service is to replace popen, and this change makes doing that straightforward.

[oshogbo]

I wonder we can't just use fileargs and fexecve?

Do you plan to add some test cases?

lib/libcasper/services/cap_exec/cap_exec.c

60

Multiple calls to this function will break the service.
For example in case when we have multiple exec channels.

111

Do we want to do fdopen on -1, this won't override the errno set by us?

[yzhong_freebsdfoundation.org]

Thinking about it more, the change to FILE* makes it somewhat unintuitive (or impossible?) to execute commands that you neither read from nor write to. Initially, I made the change because I was doing:
1 - get a file descriptor from cap_exec_open
2 - call fdopen() on it to get a FILE* to work with
3 - later, close the file descriptor with cap_exec_close
But at this point I'm left with a 'dangling' FILE*. I can't call fclose on it as it gives me an unsurprising "bad file descriptor" error. So at this point I felt that it was more sensible for cap_exec to work with FILE*s, since it matches popen in any case. But now, I see that returning a FILE* doesn't make sense in all cases.

I would appreciate any input on this issue.

[yzhong_freebsdfoundation.org]

In D24327#598833, @oshogbo wrote:

I wonder we can't just use fileargs and fexecve?

Could you elaborate on how these would be used?

[oshogbo]

Please don't take this an a criticisms I just would like to know the advantages of this approach.

In pseudo code something like:

const char *filenames[] = { '/bin/cat' };

fa = fileargs_init(filenames, 1, O_EXEC, 0, ...)
fd = fileargs_open(fa, '/bin/cat');
fork();
fexecve(fd, ...);

[oshogbo]

Ou but I guess you wan't your new process not being in sandbox, right?

[yzhong_freebsdfoundation.org]

Would such an approach work if I don't have the full path of the program to be executed?

And also, do you mean that fileargs and fexecve can replace the internals of cap_exec, or that cap_exec can be discarded entirely in favour of them? I was recommended to look at cap_exec when I was working on sort, and I made fixes to it due to that - if a workable solution already exists I would not mind using that instead. One advantage of cap_exec as it is is that it can be a drop-in replacement for popen, whereas using fileargs looks like it takes more rewriting work, so its usefulness really depends on how often popen is used, I suppose.

[tig_freebsdfoundation.org]

Note that in Casper service user interface and Casper backend are separate processes. Casper does all the work and the user interface simply passes and receives parameters.
One issue I noticed is that the code is doing the actual work (closing fds) in the user interface commands cap_exec_open() and cap_exec_close(), this should be done from exec_command(), perhaps via functions cap_exec_command_open() and cap_exec_command_close(). cap_fileargs.c has a good example of how to do this.
Do you plan to add some test cases?

[tig_freebsdfoundation.org]

lib/libcasper/services/cap_exec/cap_exec.c

158–159

Likely you will need to change this since we now have two user interface functions cap_exec_open() and cap_exec_close() instead of just cap_exec()

[yzhong_freebsdfoundation.org]

Doing the work on the user interface side was a conscious decision on my part. What are the disadvantages of doing it this way? And yes, I do plan to write tests.

[tig_freebsdfoundation.org]

lib/libcasper/services/cap_exec/cap_exec.c

60

I don't think this would be an issue. cap_exec_init has the same functionality as fileargs_init. It is only called once in the beginning of user program to initialize the set of allowed executables.

[oshogbo]

I was mistaken we need service like this, we just need to work a little bit more on it.

lib/libcasper/services/cap_exec/cap_exec.c

60

You can call fileargs_init multiple time, there is no global state on the user side.
Each time you will get a new fileargs service.
You can pass the fileargs channel to different process and this will still work.

lib/libcasper/services/cap_exec/cap_exec.h

37

The WITHOUT_CASPER interface is missing.

[tig_freebsdfoundation.org]

In D24327#598914, @yzhong_freebsdfoundation.org wrote:

Doing the work on the user interface side was a conscious decision on my part. What are the disadvantages of doing it this way? And yes, I do plan to write tests.

From my understanding, doing work in the user interface functions is the same as doing work in user program, as they are the same process. It won't be allowed if program is in cap mode.
exec_command is called by Casper which is a different process, so doing work there is desirable.
It's the pattern I've seen from other Casper services.
I could be wrong though, would appreciate if someone could confirm.

[oshogbo]

From my understanding, doing work in the user interface functions is the same as doing work in user program, as they are the same process. It won't be allowed if program is in cap mode.

Thats right, cap_exec_init, cap_exec_open, cap_exec_close are done potentialy in the sandboxed process.

exec_command is called by Casper which is a different process, so doing work there is desirable.
It's the pattern I've seen from other Casper services.
I could be wrong though, would appreciate if someone could confirm.

That's right. Casper is outside the sandbox so we can do fancy things under exec_command.

[yzhong_freebsdfoundation.org]

In D24327#598924, @tig_freebsdfoundation.org wrote:

From my understanding, doing work in the user interface functions is the same as doing work in user program, as they are the same process. It won't be allowed if program is in cap mode.

I understand what you mean. That's why I used pdfork - having the process descriptor lets me wait for the program even in Capability mode. All the current functionality works, so I'd prefer to keep things as it is unless we want the service to do more.

[tig_freebsdfoundation.org]

lib/libcasper/services/cap_exec/cap_exec.c

60

I see that now - didn't consider the case where a casper channel can be passed to a different process.
I believe moving the "state keeping responsibility" to Casper would solve this problem.

[yzhong_freebsdfoundation.org]

lib/libcasper/services/cap_exec/cap_exec.c

60

That sounds right. Then, it would make sense move the other work into the service as well.

[tig_freebsdfoundation.org]

In D24327#598927, @yzhong_freebsdfoundation.org wrote:

In D24327#598924, @tig_freebsdfoundation.org wrote:

From my understanding, doing work in the user interface functions is the same as doing work in user program, as they are the same process. It won't be allowed if program is in cap mode.

I understand what you mean. That's why I used pdfork - having the process descriptor lets me wait for the program even in Capability mode. All the current functionality works, so I'd prefer to keep things as it is unless we want the service to do more.

It's quite interesting that even though a sandbox process is not allowed to open a file, it is allowed to close one.
I guess the program work as is since the privileged actions are done in exec_command - even though I strongly recommend refactoring the functions to the Casper service. There are several advantages to this:

1. Easier to make changes in the future.
2. Better design/style. All other Casper services follow this pattern.

[yzhong_freebsdfoundation.org]

Yes, that makes sense.

[yzhong_freebsdfoundation.org]

lib/libcasper/services/cap_exec/cap_exec.c

60

You can call fileargs_init multiple time, there is no global state on the user side.
Each time you will get a new fileargs service.
You can pass the fileargs channel to different process and this will still work.

Could you tell me a bit about how fileargs handles state? Is it stored on the user side, or is it in the service side? To me, it appears to be stored on the user side, in the fileargs_t structures.

[markj]

lib/libcasper/services/cap_exec/cap_exec.c

167

It is not a good idea to define dynamically sized buffers on the stack. In this case especially, the command string is being sent from an untrusted source and we have no size checking, and the compiler has no idea how much stack space the OS has allocated for us here.

You can either malloc() a copy or try using nvlist_take_string(). The latter is destructive and removes the "command" value from nvlin, but I believe that's ok based on the fact that nvlin is not declared with const.

[yzhong_freebsdfoundation.org]

Added some simple tests, and made changes according to feedback. The biggest change is the addition of cap_exec_t structures, which behave very similarly to fileargs_t structures in cap_fileargs. Now there is no global state on the user's side, and you can now open multiple cap_exec services without issue.