Reviewed-by: Günther Noack <gnoack3000@xxxxxxxxx> This patch is a strict improvement over what the sample code was before, so that's fine with me review wise. I still think it would be good to point out more explicitly that the "refer" right needs a different fallback strategy for the best effort mode than the other rights will do in the future, as discussed in [1]. In many "best effort" scenarios that people need for their code, the part that is actually fixed are the access rights that their code declares that it needs. So if they actually need the "refer" right for their programs to work, they cannot use Landlock on kernels that only support Landlock ABI v1, because under ABI v1 they will never be able to hardlink or rename between directories when Landlock is enabled. The way that the sandboxer example is dealing with it, it just gives the user a smaller set of access rights than they requested if the kernel just supports ABI v1. It's somewhat reasonable for the sandboxer tool to do because it doesn't give hard guarantees in its command line interface, but it might not be negotiable in more practical examples. :) [1] https://docs.google.com/document/d/1SkFpl_Xxyl4E6G2uYIlzL0gY2PFo-Nl8ikblLvnpvlU/edit —Günther On Fri, Sep 23, 2022 at 05:42:05PM +0200, Mickaël Salaün wrote: > Extend the help with the latest Landlock ABI version supported by the > sandboxer. > > Inform users about the sandboxer or the kernel not being up-to-date. > > Make the version check code easier to update and harder to misuse. > > Cc: Günther Noack <gnoack3000@xxxxxxxxx> > Cc: Paul Moore <paul@xxxxxxxxxxxxxx> > Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx> > Link: https://lore.kernel.org/r/20220923154207.3311629-2-mic@xxxxxxxxxxx > --- > samples/landlock/sandboxer.c | 37 ++++++++++++++++++++++++++++-------- > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c > index 3e404e51ec64..f29bb3c72230 100644 > --- a/samples/landlock/sandboxer.c > +++ b/samples/landlock/sandboxer.c > @@ -162,11 +162,10 @@ static int populate_ruleset(const char *const env_var, const int ruleset_fd, > LANDLOCK_ACCESS_FS_MAKE_SYM | \ > LANDLOCK_ACCESS_FS_REFER) > > -#define ACCESS_ABI_2 ( \ > - LANDLOCK_ACCESS_FS_REFER) > - > /* clang-format on */ > > +#define LANDLOCK_ABI_LAST 2 > + > int main(const int argc, char *const argv[], char *const *const envp) > { > const char *cmd_path; > @@ -196,8 +195,12 @@ int main(const int argc, char *const argv[], char *const *const envp) > "\nexample:\n" > "%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" " > "%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" " > - "%s bash -i\n", > + "%s bash -i\n\n", > ENV_FS_RO_NAME, ENV_FS_RW_NAME, argv[0]); > + fprintf(stderr, > + "This sandboxer can use Landlock features " > + "up to ABI version %d.\n", > + LANDLOCK_ABI_LAST); > return 1; > } > > @@ -225,12 +228,30 @@ int main(const int argc, char *const argv[], char *const *const envp) > } > return 1; > } > + > /* Best-effort security. */ > - if (abi < 2) { > - ruleset_attr.handled_access_fs &= ~ACCESS_ABI_2; > - access_fs_ro &= ~ACCESS_ABI_2; > - access_fs_rw &= ~ACCESS_ABI_2; > + switch (abi) { > + case 1: > + /* Removes LANDLOCK_ACCESS_FS_REFER for ABI < 2 */ > + ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER; > + > + fprintf(stderr, > + "Hint: You should update the running kernel " > + "to leverage Landlock features " > + "provided by ABI version %d (instead of %d).\n", > + LANDLOCK_ABI_LAST, abi); > + __attribute__((fallthrough)); > + case LANDLOCK_ABI_LAST: > + break; > + default: > + fprintf(stderr, > + "Hint: You should update this sandboxer " > + "to leverage Landlock features " > + "provided by ABI version %d (instead of %d).\n", > + abi, LANDLOCK_ABI_LAST); > } > + access_fs_ro &= ruleset_attr.handled_access_fs; > + access_fs_rw &= ruleset_attr.handled_access_fs; > > ruleset_fd = > landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); > -- > 2.37.2 > --
