Add uname parsing fallback path to KernelVersion() #823
Conversation
It turns out if filecaps are assigned to a binary, it affects the
processes "dumpability" (see `PR_SET_DUMPABLE` section in `man prctl`).
When a process isn't globally dumpable, `/proc/self/auxv` is owned by
`root:root`.
Put together, what this means is that if an executable uses this library
and is assigned filecaps but run as a normal user, this library errors
out when reading from `/proc/self/auxv`.
You can confirm this existing behavior by running this on master:
$ cd internal
$ go test -c
$ sudo setcap cap_net_admin,cap_sys_admin+p internal.test
$ ./internal.test
--- FAIL: TestCurrentKernelVersion (0.00s)
version_test.go:53: opening auxv: open /proc/self/auxv: permission denied
FAIL
This commit makes `KernelVersion()` more reliable by adding a
best-effort uname parsing codepath.
More details about the filecaps interaction is available here:
https://dxuuu.xyz/filecaps.html .
|
Ah, that is unfortunate. We used to have uname parsing, but removed it in favour of auxv in #500. The old code first tried to read I'm a bit unhappy that we'd have to maintain three different code paths for the same piece of information. Does it make sense to keep auxv parsing if we need uname anyways? Alternatively, could we do auxv + uname but skip If we decide to resurrect the old code we have to remove the dependency on Thanks for the nice write up as well :) |
I think it's probably worthwhile. It looks like auxv works well in almost all circumstances. And if it doesn't work, rather than give up, could do a best effort uname parse. That being said, I'm doubtful any distro would mess up the first
👍 |
|
@danobi does the problem persist, if you grant/elevate yourself to use the permitted capabilities? |
|
@brycekahle yes: https://pastes.dxuuu.xyz/lgvdhu |
|
@danobi Interesting. Is this the only thing failing when using filecaps? |
|
@brycekahle yeah, at least for how my company is using bpf. |
|
I think I'm missing the bigger picture here. It's a cool war story, but is there a reason the process binary can't be made dumpable? The way I see it, it would be just another permission required, and the best course of action might be making the error clearer ( Completely understand from a principle of least privilege, but I'm also interested in seeing some concrete arguments (apart from the surprising end result of auxv being root-owned) for the practical concerns of granting this privilege. Of course, giving an attacker less info is better, but consider the fact that this process already has CAP_BPF and CAP_NET_ADMIN in most cases, so this defense is of marginal added value at best.
On the face of it, yes. But as I'll outline below, it also risks silently feeding users wrong version numbers if done incorrectly, as we're planning to use this value for As a reminder, there's also the patch version overflow problem that was introduced in the 4.19 series. The value in auxv was left clamped to 255 (or was it 254?) and is always guaranteed to match If we decide to go forward with this, can we just revive my old code instead? It's battle-tested and contained a good set of test strings as well. We can drop regex, of course.
Let me introduce you to our good friend Debian: In bullseye, it looks like there's now a new file, So yet another location to scan, although it seems like bullseye at least has the full version in |
|
As an aside, Explicitly setting |
I think you only need to call
See https://www.kernel.org/doc/html/latest/admin-guide/LSM/Yama.html#ptrace-scope and https://man7.org/linux/man-pages/man2/ptrace.2.html The other option would be to grant something like CAP_DAC_OVERRIDE, possibly CAP_DAC_READ_SEARCH too, since now the ownership of /proc/self/auxv is ignored. The problem is that the process can now read /etc/passwd and so on. It's a pretty big hammer.
👍 |
|
@ti-mo thanks for the detailed explanation. Like @lmb suggests, I too feel a bit uneasy about requiring more capabilities than necessary to parse kernel version. But since capabilities are probably somewhat of a rare use case and it sounds annoying to maintain a bunch of mostly unused code, it might be fine to just ask for user to provide If we go that route, I think we should probably check for |
|
@danobi would deploying with Another option could be to call into https://man7.org/linux/man-pages/man3/getauxval.3.html via CGo iff that is enabled. Again, a partial solution, but maybe better than requiring the additional cap. |
|
@lmb Yeah, that ought to be fine for my use case. |
…ilities As reported [0] setting capabilities on an executable file will prevent reading /proc/self/auxv since the file is owned by root. The work arounds to this have various trade offs, so the best we can do is tell the user why we failed and hope they check out our discussion or create an issue. 0: cilium#823
As reported [0] setting capabilities on an executable file will prevent reading /proc/self/auxv since the file is owned by root. The work arounds to this have various trade offs, so the best we can do is tell the user why we failed and hope they check out our discussion or create an issue. 0: cilium#823
As reported by Daniel Xu, setting capabilities on an executable file will prevent reading /proc/self/auxv since the file is owned by root. The work arounds to this have various trade offs, so the best we can do is tell the user why we failed and hope they check out our discussion or create an issue. See cilium#823
|
I made a PR with a hopefully better error message, let's see where that gets us. |
As reported by Daniel Xu, setting capabilities on an executable file will prevent reading /proc/self/auxv since the file is owned by root. The work arounds to this have various trade offs, so the best we can do is tell the user why we failed and hope they check out our discussion or create an issue. See #823
It turns out if filecaps are assigned to a binary, it affects the processes "dumpability" (see
PR_SET_DUMPABLEsection inman prctl). When a process isn't globally dumpable,/proc/self/auxvis owned byroot:root.Put together, what this means is that if an executable uses this library and is assigned filecaps but run as a normal user, this library errors out when reading from
/proc/self/auxv.You can confirm this existing behavior by running this on master:
This commit makes
KernelVersion()more reliable by adding a best-effort uname parsing codepath.More details about the filecaps interaction is available here: https://dxuuu.xyz/filecaps.html .