On 04/08/2022 18:10, Günther Noack wrote:

On Fri, Jul 29, 2022 at 01:58:17PM +0200, Mickaël Salaün wrote:

[...]

quoted

quoted

quoted

While we may question whether a dedicated access right should be added for
the Landlock use case, two arguments are in favor of this approach:
- For compatibility reasons, the kernel must follow the semantic of a
specific Landlock ABI, otherwise it could break user space. We could still
backport this patch and merge it with the ABI 1 and treat it as a bug, but
the initial version of Landlock was meant to be an MVP, hence this lack of
access right.
- There is a specific access right for Capsicum (CAP_FTRUNCATE) that could
makes more sense in the future.

Following the Capsicum semantic, I think it would be a good idea to also
check for the O_TRUNC open flag:
https://www.freebsd.org/cgi/man.cgi?query=rights

open() with O_TRUNC was indeed a case I had not thought about - thanks
for pointing it out.

I started adding some tests for it, and found to my surprise that
open() *is* already checking security_path_truncate() when it is
truncating files. So there is a chance that we can get away without a
special check for O_TRUNC in the security_file_open hook.

The exact semantics might be slightly different to Capsicum though -
in particular, the creat() call (= open with O_TRUNC|O_CREAT|O_WRONLY)
will require the Landlock truncate right when it's overwriting an
existing regular file, but it will not require the Landlock truncate
right when it's creating a new file.

Is the creat() check really different from what is done by Capsicum?

TBH, I'm not sure, it might also do the same thing. I don't have a
FreeBSD machine at hand and am not familiar with Capsicum in detail.
Let me know if you think we should go to the effort of ensuring the
compatibility down to that level.

I'll take a look at the code, but it makes sense to implement it like 
you did.