| Summary: | [Future EAPI] Functions for sandbox control | ||
|---|---|---|---|
| Product: | Gentoo Hosted Projects | Reporter: | Ulrich Müller <ulm> |
| Component: | PMS/EAPI | Assignee: | PMS/EAPI <pms> |
| Status: | CONFIRMED --- | ||
| Severity: | enhancement | CC: | arthur, esigra, kentnl, mgorny |
| Priority: | High | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| See Also: |
https://bugs.gentoo.org/show_bug.cgi?id=161045 https://bugs.gentoo.org/show_bug.cgi?id=623854 https://bugs.gentoo.org/show_bug.cgi?id=729464 https://bugs.gentoo.org/show_bug.cgi?id=631722 |
||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 174380 | ||
|
Description
Ulrich Müller
2017-09-08 20:02:07 UTC
What would sandboxoff do? Does it merely disable sandboxing (i.e. addwrite /), or remove sandbox from LD_PRELOAD completely? What are the use cases for using it? Does it imply we're making ebuilds rely on implementation details of sandbox? In particular, is this intended to be used to workaround bugs in Gentoo sandbox? I'm concerned that this function will be only used to workaround issues with Gentoo sandbox, while by specification it will also force disabling another implementations of sandbox that could certainly work better. (In reply to Michał Górny from comment #1) > What would sandboxoff do? Does it merely disable sandboxing (i.e. addwrite > /), or remove sandbox from LD_PRELOAD completely? It needs to remove the sandbox completely. > What are the use cases for using it? Does it imply we're making ebuilds rely > on implementation details of sandbox? In particular, is this intended to be > used to workaround bugs in Gentoo sandbox? One use case is app-editors/emacs which dumps its executable, and we don't want it to dump any sandbox code with it. The ebuild assigns SANDBOX_ON=0 in src_compile, which relies on a sandbox implementation detail. IIRC similar issues exist with some lisp implementations. Unless I'm mistaken, this is purely related to LD_PRELOAD implementation, correct? The problem I see with this, is that right now I can think of at least three ways of implementing filesystem sandbox: 1. LD_PRELOAD -- the way Gentoo sandbox historically worked, 2. ptrace() -- sydbox from Exherbo does that, 3. filesystem level, e.g. FUSE -- it's on my TODO. At the same time, all the issues I can think of are related to the specific implementation. In particular, use cases for disabling sandbox are limited to: a. working around bugs in sandbox (like the one with path lengths) -- which are better solved by actually fixing the sandbox, and b. working around problems caused by the design of sandbox -- i.e. the lot of problems related to LD_PRELOAD. Now, as I see it 'sandboxoff' as you propose it will inevitably disable any kind of sandbox even though the developers would only use it because one specific implementation of sandbox doesn't work. Given that all the issues mentioned so far are specifically related to how Gentoo sandbox is implemented, I would rather have people employ Gentoo sandbox specific overrides than codify a global kill-switch in the PMS. At least unless someone actually comes up with a good use case that does not have a better solution than disabling all kinds of sandboxing entirely. See bug 161045, this is intended as a replacement for RESTRICT=sandbox: "The sandbox tool must not be used when building the package." (In reply to Michał Górny from comment #3) > Now, as I see it 'sandboxoff' as you propose it will inevitably disable any > kind of sandbox That's the idea. It breaks my build, so I disable it. > even though the developers would only use it because one specific > implementation of sandbox doesn't work. The ebuild has no control over what sandbox implementation is used by the package manager, so I don't see how we could be more specific here than disabling it unconditionally. But it's not *sandbox by idea* that breaks the ebuild. It's very specific thing, e.g. LD_PRELOAD-ing stuff. If LD_PRELOAD is the problem, then you should disable LD_PRELOAD, not sandbox in general because one sandbox is implemented using LD_PRELOAD. Let's say I'm using sydbox instead of sandbox. Why would you disable sydbox if it doesn't use LD_PRELOAD and works just fine? So you say that ebuilds should unset LD_PRELOAD rather than doing SANDBOX_ON=0 or (as proposed) sandboxoff? Yes, something like that. Except that Gentoo sandbox ignores unsetting LD_PRELOAD, so you need to use a specialized tool like app-portage/unsandbox. Which also has the advantage of not breaking any other sandbox. (In reply to Michał Górny from comment #7) > Yes, something like that. Except that Gentoo sandbox ignores unsetting > LD_PRELOAD, so you need to use a specialized tool like > app-portage/unsandbox. Which also has the advantage of not breaking any > other sandbox. IMHO adding additional complexity on top of the sandbox isn't a viable alternative, especially when it would require additional dependencies (only to replace a single SANDBOX_ON=0 line). Can we please follow the KISS principle here? Having the PM disable the sandbox is straight forward. Except that you're talking about solving the wrong problem. Gentoo sandbox is the problem, so deal with Gentoo sandbox, instead of killing the concept of filesystem access sandboxing completely. If you want to discuss the KISS principle, then I suggest you start by fixing emacs not to do this horribly convoluted stuff. KISS isn't about 'this breaks my convoluted idea, so let's kill half of the system to make it work'. I don't mind having 'sandboxoff' if we have a valid use case for it. However, AFAICS the only uses of 'sandboxoff' so far would be abuses which will only create more work for me to report and prevent. (In reply to Michał Górny from comment #9) > If you want to discuss the KISS principle, then I suggest you start by > fixing emacs not to do this horribly convoluted stuff. KISS isn't about > 'this breaks my convoluted idea, so let's kill half of the system to make it > work'. I am not Emacs upstream, and besides the problem occurs with other packages too. > However, AFAICS the only uses of 'sandboxoff' so far would be abuses which > will only create more work for me to report and prevent. So what "problem" was ever caused by app-editors/emacs disabling the sandbox? This is working since years. Anyway, the consensus in bug 161045 was that there would be a replacement for RESTRICT="sandbox" in the next EAPI. So if this gets rejected, I suggest that we revert PMS commit 4055e2091ab22d34b45790462513bbaf3d70a1bf and have Portage follow the spec. I'm sorry but are you using the argument "we're doing it wrong for years, so we must make it official" now? What problem would using app-portage/unsandbox to solve the exact problem cause? How is using an existing, tested, working solution more complex than disabling a large subsystem of Portage to workaround the problem? RESTRICT=sandbox might have been a reasonable option at the time. However, given that it didn't work, I've spent the effort to research the problem and found a better solution. So why now push for something that's obsolete when you can solve the same problem easier and more correctly? (In reply to Michał Górny from comment #11) Maybe let's go back one step. Portage provides a sandboxed environment for packages to be built. In some rare cases like GNU Emacs, the build will fail in that environment, while it succeeds outside of it (and we have little leverage for changing the upstream build system). Now the ebuild can work around the problem, either by interacting with the sandbox directly (via SANDBOX_ON, which is semi-documented) or by using some other solution like app-portage/unsandbox (which so far I haven't tested, so I can only assume that it would work for my packages). IMHO neither solution is the right approach here, because both rely on the specific sandbox implementation. However, PMS does not specify what kind of sandbox implementation is to be used by the PM, and therefore ebuild should not try to interact with the sandbox directly (by SANDBOX_ON=0, or by executing unsandbox). Any interaction should be via the package manager. We have precedents for that, because PMS already specifies commands like addwrite and addpredict. For other use cases, like saving and restoring the sandbox state, such commands are missing, therefore you find eclasses messing with SANDBOX_WRITE (cvs.eclass) which relies on sandbox internals, or using addwrite in a subshell (git-r3.eclass) which relies on internals of the Portage implementation not specified by PMS. Finally, a command to disable the sandbox environment (therefore unbreaking the build of above mentioned packages) is missing. I don't mind how that will be achieved, by Portage disabling the sandbox entirely, or by your unsandbox. But it should be via interaction with the PM, because only the PM is aware of the specific sandbox implementation that is used. (In reply to Ulrich Müller from comment #12) > (In reply to Michał Górny from comment #11) > > Maybe let's go back one step. Portage provides a sandboxed environment for > packages to be built. In some rare cases like GNU Emacs, the build will fail > in that environment, while it succeeds outside of it (and we have little > leverage for changing the upstream build system). > > Now the ebuild can work around the problem, either by interacting with the > sandbox directly (via SANDBOX_ON, which is semi-documented) or by using some > other solution like app-portage/unsandbox (which so far I haven't tested, so > I can only assume that it would work for my packages). > > IMHO neither solution is the right approach here, because both rely on the > specific sandbox implementation. However, PMS does not specify what kind of > sandbox implementation is to be used by the PM, and therefore ebuild should > not try to interact with the sandbox directly (by SANDBOX_ON=0, or by > executing unsandbox). You're getting the right points but the wrong conclusion. The issue you are hitting is *specific* to this sandbox implementation, so the solution (or rather the workaround) should also be specific to this sandbox implementation. Otherwise you're ruling out non-broken sandboxes (think sydbox) because you have a problem with one of them. > Any interaction should be via the package manager. We > have precedents for that, because PMS already specifies commands like > addwrite and addpredict. Those commands apply ideas generic to filesystem sandboxing, and apply the same to different sandboxes. > For other use cases, like saving and restoring the sandbox state, such > commands are missing, therefore you find eclasses messing with SANDBOX_WRITE > (cvs.eclass) which relies on sandbox internals, or using addwrite in a > subshell (git-r3.eclass) which relies on internals of the Portage > implementation not specified by PMS. I have serious doubts about being able to implement push/pop logic for different kinds of sandboxes. Once again, you're taking implementation details of one sandbox and trying to expand them to a generic idea without even considering how poorly they map to the generic implementation. Of course, you can implement emulation of push/pop just to keep things working. But I'd prefer if we went for something that could work reliably across all possible sandbox implementations without penalizing implementations other than the one you used. That said, the more universal idea would be to have functions to remove previously added paths. This can be achieved in other sandboxes without having to keep additional state variables. > Finally, a command to disable the sandbox environment (therefore unbreaking > the build of above mentioned packages) is missing. I don't mind how that > will be achieved, by Portage disabling the sandbox entirely, or by your > unsandbox. But it should be via interaction with the PM, because only the PM > is aware of the specific sandbox implementation that is used. Besides what I've written above, this won't actually work. Because: 1. SANDBOX_ON doesn't disable LD_PRELOAD, so it doesn't solve the problem. 2. unsandbox requires running a subcommand, so you can't implement it as in-place bash function. I should also point out that Gentoo sandbox has no maintainer, and fixing even trivial bugs is taking months so far. (In reply to Michał Górny from comment #13) > The issue you are hitting is *specific* to this sandbox implementation, so > the solution (or rather the workaround) should also be specific to this > sandbox implementation. That may well be so, but PMS doesn't specify what sandbox implementation is to be used by the package manager. For example, how would the ebuild know when it is necessary to require app-portage/unsandbox as dependency? (Maybe that program should be included with sys-apps/sandbox instead, so it will be available exactly when it is needed?) > That said, the more universal idea would be to have functions to remove > previously added paths. This can be achieved in other sandboxes without > having to keep additional state variables. Let's go this way then for that part of the proposal, rather than trying to save/restore the full state. > 1. SANDBOX_ON doesn't disable LD_PRELOAD, so it doesn't solve the problem. SANDBOX_ON=0 is sufficient to unbreak the GNU Emacs build, see bug 131505. Without it, it will result in a larger binary (the most extreme case I've seen had doubled size) and/or in random segmentation faults at runtime. (In reply to Ulrich Müller from comment #14) > (In reply to Michał Górny from comment #13) > > The issue you are hitting is *specific* to this sandbox implementation, so > > the solution (or rather the workaround) should also be specific to this > > sandbox implementation. > > That may well be so, but PMS doesn't specify what sandbox implementation is > to be used by the package manager. For example, how would the ebuild know > when it is necessary to require app-portage/unsandbox as dependency? (Maybe > that program should be included with sys-apps/sandbox instead, so it will be > available exactly when it is needed?) I can look into doing that. > > 1. SANDBOX_ON doesn't disable LD_PRELOAD, so it doesn't solve the problem. > > SANDBOX_ON=0 is sufficient to unbreak the GNU Emacs build, see bug 131505. > Without it, it will result in a larger binary (the most extreme case I've > seen had doubled size) and/or in random segmentation faults at runtime. Curious. But then, it's not enough to unbreak ASAN. I think in principle, the idea of "just skip the sandbox when it doesn't suit us", when the user states FEATURES="sandbox", is not necessarily something that should be encouraged. It *may* be worth while stipulating that sandbox reversal features ( if we provide them ), require some sort of control verb in order to be used, so that users are made aware in advance that sandbox will be disabled. And then they can employ a site-wide policy via make.conf if they're OK with packages electing to remove their own sandboxes in controlled scopes, or granularly via package.env , using FEATURES= Esp seeing the first line of documentation on FEATURES= in make.conf(5) is: > FEATURES = "sandbox" > .... The sandbox feature is very important and should not be disabled by default Maybe if we're thinking down the road to the future, we may want to have a list of specific sandboxing behaviours a package needs to avoid. For instance, even though its standard practice to forbid internet access, some test suites for network software can be broken by FEATURES="network-sandbox" simply due to needing to call inet_aton("localhost", ), ... which can fail depending on /etc/nsswitch.conf and /etc/hosts configuration ( bug #631306 ) Please see bug 623854 comment 2. Keep in mind that people may expect something when they set FEATURES="sandbox" or FEATURES="usersandbox". A possibility to allow ebuild maintainer to disable sandbox would clash with user choice and may have undesired impact. |
