copy_file_range: fix capabilities premissions

[oshogbo]

Summary

Align copy_file_range(2) permissions with pread(2)/pwrite(2) for offset usage

Ensure that copy_file_range(2), like pread(2)/pwrite(2), requires the CAP_SEEK capability due to its offset argument.
This alignment prevents unauthorized file offset manipulations by processes.

Reported by: theraven@

[emaste]

It looks like it should be permitted without CAP_SEEK if inoffp/outoffp is NULL?
I think you can commit the bin/cat change now.

[oshogbo]

Yea, I guess you are right when copy_file_range has offset NULLed it behaves more like read/write then pread/pwrite.

[theraven]

Ensure that copy_file_range(2), like pread(2)/pwrite(2), requires the CAP_SEEK capability due to its offset argument.
This alignment prevents unauthorized file offset manipulations by processes.

It should require CAP_SEEK only when given a non-NULL offset. Both of these cases appear to be using it with NULL offsets. I think it would be a POLA violation for it to unconditionally require CAP_SEEK and it would also make it far less usable in Capsicum applications.

[kib]

sys/kern/vfs_syscalls.c

4905 ↗

(On Diff #127742)

incap = inoffp != NULL ? &cap_read_rights : &cap_pread_rights;

or even inline the expression into fget_XXX() call arg, avoiding single-use local

[theraven]

This looks like it fixes the capsicum bits. I believe we're still missing correct auditing events. In kern_{read,write}*, these are handled in dofile{read,write} but, since copy_file_range bypasses these calls, they need to be added explicitly.

[emaste]

This looks like it fixes the capsicum bits. I believe we're still missing correct auditing events.

I think it's OK to follow up with a second commit for those.

[emaste]

The new test can also be sent to https://github.com/google/capsicum-test

[kib]

sys/kern/vfs_syscalls.c

4903 ↗

(On Diff #127769)

4915 ↗

(On Diff #127769)

[theraven]

sys/kern/vfs_syscalls.c

4903 ↗

(On Diff #127769)

Do you really think removing these brackets increases readability? It's obvious to me what the first version does. The second requires me to skip around the line to and mentally parse the code to find the end of the argument.

[kib]

sys/kern/vfs_syscalls.c

4903 ↗

(On Diff #127769)

Yes.

Also it fixes half of the style bug.

[emaste]

Capsicum-related syscall changes from 2014 on, for reference:
af93fea71038 timerfd: Move implementation from linux compat to sys
4a69fc16a583 Add membarrier(2)
61194e9852e6 Add kqueue1() syscall
52a1d90c8bfe Allow posix_fadvise in capability mode
77b2c2f81451 Add sched_getcpu()
0dc332bff200 Add fspacectl(2), vn_deallocate(9) and VOP_DEALLOCATE(9)
022ca2fc7fe0 Add aio_writev and aio_readv
7a202823aa54 Expose eventfd in the native API/ABI using a new __specialfd syscall
bdfe61e05e52 Permit cpuset_(get|set)domain() in capability mode.
472ced39efb5 Implement a close_range(2) syscall
e953765f1579 Allow getloginclass in capability mode
9cdfb2d69a52 Allow fdatasync in capability mode
146fc63fce9b Add a way to manage thread signal mask using shared word, instead of syscall.
20f7057685cd Add a shm_open2 syscall to support upcoming memfd_create
d05b53e0baee Add sysctlbyname system call
f30f7b987015 Enable copy_file_range(2) in capability mode.
a1304030b815 Introduce funlinkat syscall
82560231d322 capsicum: allow ppoll(2) in capability mode
0362ec1e8e80 getrandom(2) should not be restricted in capability mode.
5532aa9bb414 allow posix_fallocate in capability mode
f299c47b52b7 Allow cpuset_{get,set}affinity in capabilities mode
rG:2205e0d1bd28 Add futimens and utimensat system calls.
rG:c297f0e4972 Allow sigwait(2) in capabilities mode.

[theraven]

Capsicum-related syscall changes from 2014 on, for reference:

A quick look at the ones of these that I'd expect to require audit events and capsicum does appear to contain them.

[markj]

The code changes look ok to me.

contrib/capsicum-test/copy_file_range.cc

156 ↗

(On Diff #127769)

It would also be good to check that CAP_READ and CAP_WRITE are honored, i.e., check

EXPECT_NOTCAPABLE(copy_file_range(fd_out, NULL, fd_in, NULL, 8, 0));

I think all of your tests are verifying checks for CAP_SEEK.

contrib/capsicum-test/makefile

2 ↗

(On Diff #127769)

I believe you also need to update tests/sys/capsicum/Makefile, otherwise the new file won't be built. This makefile is upstream's (which is effectively dead now...?) and doesn't get used by our build system, for better or worse.