Extending Landlocked Processes
Mickaël Salaün posted a patch to improve communication between landlocked processes. Landlock is a security module that creates an isolated "sandbox" where a process is prevented from interacting with the rest of the system, even if that process itself is compromised by a hostile attacker. The ultimate goal is to allow regular user processes to isolate themselves in this way, reducing the likelihood that they could be an entry point for an attack against the system.
Mickaël's patch, which didn't get very far in the review process, aimed specifically at allowing landlocked processes to use system calls to manipulate other processes. To do that, he wanted to force the landlocked process to obey any constraints that also might apply to the target process. For example, the target process may not allow other processes to trace its execution. In that case, the landlocked process should be prevented from doing so.
Andy Lutomirski looked at the patch and offered some technical suggestions, but on further reflection, he felt Mickaël's approach was too complicated. He felt it was possible that the patch itself was simply unnecessary, but that if it did have a value, it simply should prevent any landlocked process from tracing another process' execution. Andy pointed to certain kernel features that would make the whole issue a lot more problematic. He said, "If something like Tycho's notifiers goes in, then it's not obvious that, just because you have the same set of filters, you have the same privilege. Similarly, if a feature that lets a filter query its cgroup goes in (and you proposed this once!), then the logic you implemented here is wrong."
Andy's overall assessment of landlock was, "I take this as further evidence that Landlock makes much more sense as part of seccomp than as a totally separate thing. We've very carefully reviewed these things for seccomp. Please don't make us do it again from scratch."
But Mickaël felt that landlock did have some valid use cases Andy hadn't mentioned—for example, "running a container constrained with some Landlock programs". Without his patch, Mickaël felt it would be impossible for users in that situation to debug their work. As he put it, "This patch adds the minimal protections which are needed to have a meaningful Landlock security policy. Without it, they may be easily bypassable, hence useless."
And as for folding landlock into seccomp, Mickaël replied, "Landlock is more complex than seccomp, because of its different goal. seccomp is less restrictive because it is more simple."
Andy replied to Mickaël's example of running a container constrained with Landlocked programs, saying, "Any sane container trying to use Landlock like this would also create a PID namespace. Problem solved." He added, "It sounds like you want Landlock to be a complete container system all by itself. I disagree with that design goal." And, he said he still felt the patch should simply be dropped.
But apparently, after delving further into the code, Andy felt his criticism was not quite right. He still felt the patch should be dropped, but he had refined his reason why. He said:
I can see an argument for having a flag that one can set when adding a seccomp filter that says "prevent ptrace of any child that doesn't have this exact stack installed", but I think that could be added later and should not be part of an initial submission. For now, Landlock users can block ptrace() entirely or use PID namespaces.
But Mickaël said there were other use cases beyond his container example. He said:
...another is build-in sandboxing (e.g. for web browser) and another one is for sandbox managers (e.g. Firejail, Bubblewrap, Flatpack). In some of these use cases, especially from a developer point of view, you may want/need to debug your applications (without requiring to be root). For nested Landlock access-controls (e.g. container + user session + web browser), it may not be allowed to create a PID namespace, but you still want to have a meaningful access-control.
But, Andy was not convinced. He argued even more strongly that "If there's a real use case for adding this type of automatic ptrace protection, then by all means, let's add it as a general seccomp feature."
Mickaël agreed that these features also would make sense for seccomp, but he still felt his own patch was useful aside from that. And at that point, the discussion came to an end.
This sort of security debate is often really tough to follow or predict. Developers never know when someone from a seemingly distant part of the kernel is going to hone in on their patch, saying either that it creates a security hole somewhere else, or that the feature itself really belongs somewhere else. Many hours of work can be lost, just because developers didn't know that the thing they were working on would be more acceptable in a completely different part of the kernel. Likewise, the person criticizing their patch may have missed a crucial detail, and suddenly after many email messages, it turns out the original coder was doing the right thing after all—and lo and behold, there are plenty of uses for that code. It's impossible to guess which way the debate will turn, which is one reason kernel developers often will push very hard to get their point across, even to the point of seeming intractable on a given issue.
Note: if you're mentioned above and want to post a response above the comment section, send a message with your response text to firstname.lastname@example.org.