utils: fdinfo: do not require "ino" field for pre-5.14 kernels#337
Merged
utils: fdinfo: do not require "ino" field for pre-5.14 kernels#337
Conversation
This is purely an implementation detail of ProcfsHandleBuilder::build and doesn't need to live in the global namespace. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There was an off-by-one when writing the check for valid file descriptor values, resulting in fd 0 not beign usable with FdExt::get_fdinfo_field. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
f31b359 to
0fe1820
Compare
4 tasks
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
0fe1820 to
ab7be81
Compare
While we do need to have fallbacks if a feature is missing, and we need to handle the case where a feature (like openat2) is disabled via seccomp, it is handy to be able to have logic based on kernel version in some other cases. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
RHEL 8 backported the fd-based mount API to their 4.18 kernel, but based on testing in runc it seems there is a pretty serious performance bug in their backport that makes it unusable. To avoid using any such broken backports, we refuse to even try to use the new mount API if the running kernel is older than when the fd-based mount API was merged (Linux 5.2). This mirrors logic we have in pathrs-lite. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The "ino" field was added to /proc/$pid/fdinfo/$n in Linux 5.14
(by kcommit 3845f256a8b52 ("procfs/dmabuf: add inode number to
/proc/*/fdinfo")), which means that we cannot require it exist on any
kernels older than that.
Unfortunately this does weaken the non-openat2 case somewhat (on
pre-5.14 kernels, an attacker can now use a single /proc/self/environ
file to fake any fdinfo) but there doesn't appear to be nice way of
authenticating the fdinfo file we read from:
* The inode number of fdinfo files is randomly generated and thus can't
be used to figure out what fdinfo file it is, nor that it is an
authentic fdinfo file at all.
* The "flags" field would present no real barrier to attackers because
in practice in libpathrs it will always be "02", and there isn't
really
* You could imagine using the "pos" field as a very rudimentary
challenge-response mechanism (lseek() to a random offset in the fd
and then check that the fdinfo contains the same offset in the "pos"
field). The hope is that it would be too difficult for an attacker to
mirror challenge in their fake fdinfo file. Unfortunately, this is
not workable for several reasons:
- We are almost always dealing with O_PATH file descriptors in
libpathrs when looking at fdinfo, which means we cannot use
lseek() in the first place.
- The seek position is global state for the file descriptor so we
would need to take a &mut version of BorrowedFd, but no such API
really exists and &mut OwnedFd is very unergonomic.
- Most importantly, this is _really_ ugly.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
ab7be81 to
96b00a1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The "ino" field was added to /proc/$pid/fdinfo/$n in Linux 5.14
(by torvalds/linux@3845f256a8b52 ("procfs/dmabuf: add inode number to
/proc/*/fdinfo")), which means that we cannot require it exist on any
kernels older than that.
Unfortunately this does weaken the non-openat2 case somewhat (on
pre-5.14 kernels, an attacker can now use a single /proc/self/environ
file to fake any fdinfo) but there doesn't appear to be nice way of
authenticating the fdinfo file we read from:
The inode number of fdinfo files is randomly generated and thus can't
be used to figure out what fdinfo file it is, nor that it is an
authentic fdinfo file at all.
The "flags" field would present no real barrier to attackers because
in practice in libpathrs it will always be "02", and there isn't
really
You could imagine using the "pos" field as a very rudimentary
challenge-response mechanism (lseek() to a random offset in the fd
and then check that the fdinfo contains the same offset in the "pos"
field). The hope is that it would be too difficult for an attacker to
mirror challenge in their fake fdinfo file. Unfortunately, this is
not workable for several reasons:
We are almost always dealing with O_PATH file descriptors in
libpathrs when looking at fdinfo, which means we cannot use
lseek() in the first place.
The seek position is global state for the file descriptor so we
would need to take a &mut version of BorrowedFd, but no such API
really exists and &mut OwnedFd is very unergonomic.
Most importantly, this is really ugly.
Fixes #334
Signed-off-by: Aleksa Sarai cyphar@cyphar.com