[Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)

[chantra]

@DemiMarie thanks for the feedback.

> That still looks fairly tight. You need to make sure that all unnecessary file descriptors have been closed, since otherwise a compromised process could use them without interference from seccomp. I suggest pointing FDs 0, 1, and 2 at `/dev/null`, and closing all of the others except for the ones that the decompressor is using.

Agreed, cleaning up fds, in general, not just for this specific process, is still on my plate.

> seccomp also supports parameter filtering, and I suggest using it for `futex` and other syscalls.

yes, I saw that but did not go down that rabbit-hole just yet. I merely wanted to PoC what leveraging `seccomp`, when available, would look like.

> Also, as a defense-in-depth measure, it is best to drop privileges by chrooting into `/var/empty` or a deleted directory, and by switching to an unprivileged user.

yeah, @malmond77  and I were talking about plainly refusing to run this as root, forcing a caller to at least ensure that they are not calling rpm2extents as root. As much as which user. systemd dynamic users would be a good fit here vs a widely used nobody or such. As much as chrooting, it appears that rpm already has some userns/chrooting integration which makes this much easier than if we had to re-implement it.



> I recommend `SECCOMP_RET_KILL_PROCESS` instead of `SECCOMP_RET_KILL`, since it kills all threads.

Good point, I just used the widely available [`seccomp-bpf.h`](https://outflux.net/teach-seccomp/step-2/seccomp-bpf.h) from https://outflux.net/teach-seccomp/ as a PoC.

> 
> For sandboxing to be useful, the output of the sandboxee must be assumed untrusted. Hash computations must be done in the parent, for example.

This is being done in a separate process. The sandboxee here is only performing the transcoding, reading the results from digest/validation from a pipe and tacking this to the end of the transcoded file. I am not sure how we could consider the output untrusted given that it is the responsibility of the transcoder to transcode. e.g, whatever the transcoder has produced will be believed to be a legitimate transcoded version of the original content. By sandboxing we mostly want to ensure that vulnerability in the decompression library is not being exploited to gain more privileges.
To your point though, it should be the parent that collects the result of digest/signature and put this into the file (e.g spit it out to stdout).


> Also note that programs that call RPM may be multi-threaded, and using any non-async-signal-safe operation in the child process after `fork()` is undefined behavior for a multi-threaded parent. Ideally, that means you should `execve()` a new process that would then sandbox itself. In practice, I believe that glibc makes at least `malloc()` work. Using seccomp in a dedicated thread will appear to work, but it won’t actually provide any benefit since the address space is still shared.

`rpm2extents` is used as part of a pipeline, taking a streamed rpm in stdin and writing its transcoded version to stdout. The one invoking rpm2extents in its pipeline should be exec-ing it. rpm2extents is single-threaded and spawn worker processes to perform individual actions.


-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-1036526394
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/1470/c1036526394 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20220211/e7822f5a/attachment.html>