From: Mickaël Salaün Subject: Re: GoT Landlock fixes To: gameoftrees@openbsd.org, Omar Polo , Thomas Adam , landlock@lists.linux.dev, "Alejandro Colomar (man-pages)" Date: Mon, 14 Feb 2022 12:23:16 +0100 On 11/02/2022 21:11, Stefan Sperling wrote: > On Fri, Feb 11, 2022 at 06:15:43PM +0100, Mickaël Salaün wrote: >> The documentation about handled_access_fs says: "Bitmask of actions (cf. >> Filesystem flags) that is handled by this ruleset and should then be >> forbidden if no rule explicitly allow them. This is needed for backward >> compatibility reasons." >> >> I guess it is too succinct. What do you think about that: (applied to https://docs.kernel.org/userspace-api/landlock.html) >> >> "[...] if no rule explicitly allow them: it is a deny-by-default list that >> should contain as much Landlock access rights as possible. Indeed, all >> Landlock filesystem access rights that are not part of handled_access_fs are >> allowed. This is needed for [...]" >> >> And before that, in the example, after "The ruleset then needs to handle >> both of these kind of actions", I can add "This is required for backward and >> forward compatibility (i.e. the kernel and user space may not know each >> other's supported restrictions), hence the need to be explicit about the >> denied-by-default access rights." > > Yes, this would make the docs a lot clearer. > > I suppose developers using landlock are supposed to keep their applications > up-to-date and expand their bitmask definition whenever new flags are > added to the landlock API? > > If so, it might be worth pointing this out in the documentation, to ensure > readers understand that writing this list in their code is not a one-time > action, and their attention will be required when new flags are added. > Something like: > "Future versions of landlock may define additional filesystem flags, > which applications must add to handled_access_fs if a deny-by-default > policy is desired." Right, I'll improve the documentation in this area. > > Which makes me wonder if applications which prefer to always fail closed > could simply set all the bits. Would this break anything? > > struct landlock_ruleset_attr ruleset_attr = { > .handled_access_fs = 0xffffffffffffffff; > }; > > Otherwise, how about providing a macro which programs like Got could > use to initialize this mask to all currently known filesystem flags? > > #define LANDLOCK_ACCESS_FS_POLICY_DENY_ALL \ > (LANDLOCK_ACCESS_FS_EXECUTE | \ > LANDLOCK_ACCESS_FS_WRITE_FILE | \ > ... etc.) > > > struct landlock_ruleset_attr ruleset_attr = { > .handled_access_fs = LANDLOCK_ACCESS_FS_POLICY_DENY_ALL; > }; > > With this, only one copy of a default-deny mask would need to be maintained. > Applications could stay in sync and keep failing closed by recompiling them > against new landlock header files. Would this work? The issue with these approaches is that we don't know the future and new Landlock accesses could break an application. I don't want this to be silent: the same (application) code should produce the same restrictions (which would not be the true if an included variable change over time). I agree that there is a use case to block all filesystem-related actions (even if the frontier with other subsystems might be blurry, e.g. UNIX socket), but I think it would be too bug-prone for applications to rely on that. As a kernel developer, I cannot assure that applications using this API would not suddenly break if built with a new set of (kernel-provided) headers. In this case, a conservative approach seems better for users but also for (the sanity of) most developers. ;) One of the goal of the landlock@lists.linux.dev [1] mailing list is for developers to keep an eye on Landlock evolution. [1] https://subspace.kernel.org/lists.linux.dev.html