diff mbox series

samples/landlock: Don't error out if a file path cannot be opened

Message ID 20240307143849.1517218-1-mic@digikod.net (mailing list archive)
State Handled Elsewhere
Headers show
Series samples/landlock: Don't error out if a file path cannot be opened | expand

Commit Message

Mickaël Salaün March 7, 2024, 2:38 p.m. UTC
Instead of creating a hard error and aborting the sandbox creation,
accept file path not usable in the LL_FS_RO and LL_FS_RW environment
variables but only print a warning.  This makes it easier to test, for
instance with LL_FS_RO="${PATH}:/usr/lib:/lib"

Print that we are going to execute the command in the sandbox before
doing so.

Rename "launch" to "execute".

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 samples/landlock/sandboxer.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Günther Noack March 7, 2024, 3:15 p.m. UTC | #1
On Thu, Mar 07, 2024 at 03:38:49PM +0100, Mickaël Salaün wrote:
> Instead of creating a hard error and aborting the sandbox creation,
> accept file path not usable in the LL_FS_RO and LL_FS_RW environment
> variables but only print a warning.  This makes it easier to test, for
> instance with LL_FS_RO="${PATH}:/usr/lib:/lib"
> 
> Print that we are going to execute the command in the sandbox before
> doing so.
> 
> Rename "launch" to "execute".
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  samples/landlock/sandboxer.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index d7323e5526be..22e8c35103ce 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
> - * Simple Landlock sandbox manager able to launch a process restricted by a
> + * Simple Landlock sandbox manager able to execute a process restricted by a
>   * user-defined filesystem access control policy.

Slightly out of scope, but I think it should be "...restricted by user-defined
file system and network access control policies."

>   *
>   * Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
> @@ -121,9 +121,11 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
>  		if (path_beneath.parent_fd < 0) {
>  			fprintf(stderr, "Failed to open \"%s\": %s\n",
>  				path_list[i], strerror(errno));
> -			goto out_free_name;
> +			continue;
>  		}
>  		if (fstat(path_beneath.parent_fd, &statbuf)) {
> +			fprintf(stderr, "Failed to stat \"%s\": %s\n",
> +				path_list[i], strerror(errno));
>  			close(path_beneath.parent_fd);
>  			goto out_free_name;
>  		}
> @@ -229,7 +231,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
>  			ENV_TCP_CONNECT_NAME, argv[0]);
>  		fprintf(stderr,
> -			"Launch a command in a restricted environment.\n\n");
> +			"Execute a command in a restricted environment.\n\n");
>  		fprintf(stderr,
>  			"Environment variables containing paths and ports "
>  			"each separated by a colon:\n");
> @@ -250,7 +252,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  			ENV_TCP_CONNECT_NAME);
>  		fprintf(stderr,
>  			"\nexample:\n"
> -			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
> +			"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
>  			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
>  			"%s=\"9418\" "
>  			"%s=\"80:443\" "
> @@ -390,6 +392,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  
>  	cmd_path = argv[1];
>  	cmd_argv = argv + 1;
> +	fprintf(stderr, "Executing the sandboxed command...\n");
>  	execvpe(cmd_path, cmd_argv, envp);
>  	fprintf(stderr, "Failed to execute \"%s\": %s\n", cmd_path,
>  		strerror(errno));
> -- 
> 2.44.0
> 

Reviewed-by: Günther Noack <gnoack@google.com>
Mickaël Salaün March 7, 2024, 3:21 p.m. UTC | #2
On Thu, Mar 07, 2024 at 04:15:31PM +0100, Günther Noack wrote:
> On Thu, Mar 07, 2024 at 03:38:49PM +0100, Mickaël Salaün wrote:
> > Instead of creating a hard error and aborting the sandbox creation,
> > accept file path not usable in the LL_FS_RO and LL_FS_RW environment
> > variables but only print a warning.  This makes it easier to test, for
> > instance with LL_FS_RO="${PATH}:/usr/lib:/lib"
> > 
> > Print that we are going to execute the command in the sandbox before
> > doing so.
> > 
> > Rename "launch" to "execute".
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  samples/landlock/sandboxer.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> > index d7323e5526be..22e8c35103ce 100644
> > --- a/samples/landlock/sandboxer.c
> > +++ b/samples/landlock/sandboxer.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: BSD-3-Clause
> >  /*
> > - * Simple Landlock sandbox manager able to launch a process restricted by a
> > + * Simple Landlock sandbox manager able to execute a process restricted by a
> >   * user-defined filesystem access control policy.
> 
> Slightly out of scope, but I think it should be "...restricted by user-defined
> file system and network access control policies."

Good catch. I integrated your suggestion. Thanks.

> 
> >   *
> >   * Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
> > @@ -121,9 +121,11 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
> >  		if (path_beneath.parent_fd < 0) {
> >  			fprintf(stderr, "Failed to open \"%s\": %s\n",
> >  				path_list[i], strerror(errno));
> > -			goto out_free_name;
> > +			continue;
> >  		}
> >  		if (fstat(path_beneath.parent_fd, &statbuf)) {
> > +			fprintf(stderr, "Failed to stat \"%s\": %s\n",
> > +				path_list[i], strerror(errno));
> >  			close(path_beneath.parent_fd);
> >  			goto out_free_name;
> >  		}
> > @@ -229,7 +231,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
> >  			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
> >  			ENV_TCP_CONNECT_NAME, argv[0]);
> >  		fprintf(stderr,
> > -			"Launch a command in a restricted environment.\n\n");
> > +			"Execute a command in a restricted environment.\n\n");
> >  		fprintf(stderr,
> >  			"Environment variables containing paths and ports "
> >  			"each separated by a colon:\n");
> > @@ -250,7 +252,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
> >  			ENV_TCP_CONNECT_NAME);
> >  		fprintf(stderr,
> >  			"\nexample:\n"
> > -			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
> > +			"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
> >  			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
> >  			"%s=\"9418\" "
> >  			"%s=\"80:443\" "
> > @@ -390,6 +392,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
> >  
> >  	cmd_path = argv[1];
> >  	cmd_argv = argv + 1;
> > +	fprintf(stderr, "Executing the sandboxed command...\n");
> >  	execvpe(cmd_path, cmd_argv, envp);
> >  	fprintf(stderr, "Failed to execute \"%s\": %s\n", cmd_path,
> >  		strerror(errno));
> > -- 
> > 2.44.0
> > 
> 
> Reviewed-by: Günther Noack <gnoack@google.com>
>
diff mbox series

Patch

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index d7323e5526be..22e8c35103ce 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -1,6 +1,6 @@ 
 // SPDX-License-Identifier: BSD-3-Clause
 /*
- * Simple Landlock sandbox manager able to launch a process restricted by a
+ * Simple Landlock sandbox manager able to execute a process restricted by a
  * user-defined filesystem access control policy.
  *
  * Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
@@ -121,9 +121,11 @@  static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
 		if (path_beneath.parent_fd < 0) {
 			fprintf(stderr, "Failed to open \"%s\": %s\n",
 				path_list[i], strerror(errno));
-			goto out_free_name;
+			continue;
 		}
 		if (fstat(path_beneath.parent_fd, &statbuf)) {
+			fprintf(stderr, "Failed to stat \"%s\": %s\n",
+				path_list[i], strerror(errno));
 			close(path_beneath.parent_fd);
 			goto out_free_name;
 		}
@@ -229,7 +231,7 @@  int main(const int argc, char *const argv[], char *const *const envp)
 			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
 			ENV_TCP_CONNECT_NAME, argv[0]);
 		fprintf(stderr,
-			"Launch a command in a restricted environment.\n\n");
+			"Execute a command in a restricted environment.\n\n");
 		fprintf(stderr,
 			"Environment variables containing paths and ports "
 			"each separated by a colon:\n");
@@ -250,7 +252,7 @@  int main(const int argc, char *const argv[], char *const *const envp)
 			ENV_TCP_CONNECT_NAME);
 		fprintf(stderr,
 			"\nexample:\n"
-			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
+			"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
 			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
 			"%s=\"9418\" "
 			"%s=\"80:443\" "
@@ -390,6 +392,7 @@  int main(const int argc, char *const argv[], char *const *const envp)
 
 	cmd_path = argv[1];
 	cmd_argv = argv + 1;
+	fprintf(stderr, "Executing the sandboxed command...\n");
 	execvpe(cmd_path, cmd_argv, envp);
 	fprintf(stderr, "Failed to execute \"%s\": %s\n", cmd_path,
 		strerror(errno));