Improving Nova privilege escalation model, part 3
In the previous two posts of this series, we explored the deficiencies of the current model and the features of an alternative implementation. In this last post, we’ll discuss the advantages of a Python implementation and open discussion on how to secure it properly.
Python implementation
It’s quite easy to implement the features that were mentioned in the previous post in Python. The main advantage of doing so is that the code can happily live inside Nova code, in particular the filters definition files can be implemented as Python modules that are loaded if present. That solves the issue of shipping definitions within Nova and also the separation of allowed commands based on locally-deployed nodes. The code is simple and easy to review. The trick is to make sure that no malicious code can be injected in the elevated rights process. This is why I’d like to present a model and open it for comments in the community.
Proposed security model
The idea would be to have Nova code optionally use “sudo nova-rootwrap” instead of “sudo” as the root_helper. A generic sudoers file would allow the nova user to run /usr/bin/nova-rootwrap as root, while stripping environment variables like PYTHONPATH. To load its filters definitions, nova-rootwrap would try to import a set of predefined modules (like nova.rootwrap.compute), but if those aren’t present, it should ignore them. Can this model be abused ?
The obvious issue is to make sure sys.path (the set of directories from which Python imports its modules) is secure, so that nobody can insert their own modules in the process. I’ve given some thoughts to various checks, but actually there is no way around trusting the default sys.path you’re given when you start python as root from a cleaned env. If that’s compromised, you’re toasted the moment you “import sys” anyway. So using sudo to only allow /usr/bin/nova-rootwrap and cleaning the environment should be enough. Or am I missing something ?
Insecure mode ?
One thing we could do is check that sys.path all belongs to root and refuse to run in the case it’s not. That would tell the user that his setup is insecure (potentially allowing him to bypass that by running “sudo nova-rootwrap –insecure” as the root_helper). But that’s a convenience to detect insecure setups, not a security addition (the fact that it doesn’t complain doesn’t mean you’re safe, it could mean you’re already compromised).
Test mode ?
For tests, it’s convenient to allow to run code from branches. To allow this (unsafe) mode, you would tweak sudoers to allow it to run $BRANCH/bin/nova-rootwrap as root, and prepend “..” to sys.path in order to allow modules to be loaded from $BRANCH (maybe requiring –insecure mode for good measure). It sounds harmless, since if you run from /usr/bin/nova-rootwrap you can assume that /usr is safe… Or should that idea be abandoned altogether ?
Audit
Nothing beats peer review when it comes to secure design. I call all Python module-loading experts and security white-hats out there: would this work ? Are those safe assumptions ? How much do you like insecure and test modes ? Would you suggest something else ? If you’re one of those that can’t think in words but require code, you can get a glimpse of work in progress here. It will all be optional (and not used by default), so it can be added to Nova without much damage, but I’d rather do it right from the beginning
Please comment !
You’re right to worry about how this might be compromised, because if it is then the nova user can run any command as root
Relying on sudo’s env cleaning should be fine, I’m not sure what more we can do there
I’m also not sure “insecure mode” would help much. Sounds like you’re suggesting it to allow people discover their “accidentally insecure” setups? How about other env variables like $HOME and $PATH. IMHO if anyone messes with sudo’s env cleaning config, on their head be it
Test mode makes sense, I think
Even after filtering, though, we’ve a pretty scary set of commands that the nova user can run – e.g. ‘kill -9 pid’ with no checking of what’s being killed
Thanks ! Note that the whole point of advanced filters is to enable stronger filtering. For example, with the proposed setup we can actually check what is being killed before allowing that command to run. We can also limit its rights to the owner of the process being killed.
Few things spring to mind:
1) doesn’t sys.path often include dirname(argv[0]) and ‘.’ ? If so, need to consider who can write to cwd (is that /var/lib/nova, writable by nova?)
2) regexp becomes an attack vector (easy to write unreadable/complex regexp). For example, something like “../..” in the middle of a path fundamentally alters the path, is it properly filtered? Or “…./malicious-symlink/…” (any part of the path nova or world writable?)
I believe this is one reason why Toshiharu-san pointed at the existing Mandatory Access Control mechanisms.
Had you considered a privileged service with a limited and specific API that receives requests and operates on them? The reduces the attack surface from “all the ways a privileged binary like iptables or dd” could get abused to the API the service exports.
the building blocks are there.
Seems with run_as_root (or for your dnsmasq example, run as nobody
1) Yes sys.path includes dirname(argv[0]), but not “.” so as long as you use sudoers to only allow /usr/bin/nova-rootwrap, you just need to assume /usr/bin is root-owned, which is a rather safe assumption.
2) The regexp is provided in the code (filter files) so it’s not attacker-controllable. If you mean that regexps are easy to get wrong, they are just one of multiple options. For example I think we’ll also have a a glob-filter to facilitate directory filtering.
Good point on the privileged service, that’s definitely another way to solve the problem. It sounds to me you increase the attack surface (more code running as root), but the benefits may outweigh the costs.
What the fuck are you doing using sudo??? BAD IDEA, switch to PolicyKit.
PolicyKit is traditionally not installed on servers, which are the target environment. That sounds like an abusive dependency, if we can get to the same result with less-desktop-oriented tools.
The existing “env_reset” sudo option appears to have already solved your problem (by stripping most environment variables, including PYTHONPATH, before executing the requested command). It’s on by default.