PR: 21463
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I'm not sure this is all that useful, to be honest. What's the usage scenario? Why would one globally disable suid for Linux binaries?
One thing that might be useful is implementing PR_SET_NO_NEW_PRIVS prctl(2). This disables suid (like the patch above) for the calling process (and I'm guessing also for its descendants).
Motivation is provided by the notes in the referenced PR. Briefly, convincing argument for me was that we do not provide environment that would be expected by the linux setuid binaries, so assumptions made during coding of them could be broken by us unknowingly. More, I should add that it is possible that user break them due to our /compat/linux+normal freebsd fs fallback setup.
I do not know what is prctl(2). WRT making the settings more granular, this patch is required pre-requisite for any further progress, because you do need hook into execve(2) to avoid doing setid. This is the main part of the change (and also I did it with a function instead of just bool exactly to ease future improvements, see the patch update history).
I'm not convinced about the environment, tbh. Can you give some specific example of what we don't provide, that could affect security of suid binaries?
Now, regarding prctl(2): think of it as Linux equivalent of procctl(2); in Linux, one can use PR_SET_NO_NEW_PRIVS to set a per-process flag, inherited by children, to do what the sysctl does, except per-process. It would also be useful for native FreeBSD processes, so we'd probably implement it natively first, not just in Linuxulator. The flag is used by various real-world software, so we'll need to implement it anyway - and to me it renders the sysentvec-related parts of this patch redundant.
Also: I wonder if we could start allowing chroot(2) for non-root users, as long as the calling process has the NO_NEW_PRIVS flag set.
Fully support this change and motivation, propose to add a short description to the linux(4) manual about sysctl
Note that I'm not in any way opposed to this change.
Also, there's now a review which shows what I've meant with NO_NEW_PRIVS flag: https://reviews.freebsd.org/D30130. It reuses a part of this patch, but is otherwise orthogonal.
I've tested this patch for both 64, and 32-bit Linux binaries, for both suid and sgid bit, and it works just fine.
One nit, though: I'd strongly prefer the sysctl to be enabled by default, ie to permit suid/sgid by default. Otherwise it will break some pretty common use cases.
All kinds of cases, tbh - from sudo apt update, to Chromium, which (for some reason) depends on a setuid helper.
Do we have an agreement there?
I think that switching the defaults, if ever, should be done by separate change indeed.
if it makes sence to allow setid by default, I have no objection, although it seems strange to me