Re: [PATCH v3 20/23] samples/landlock: Do not log denials from the sandboxer by default

[Francis Laniel <flaniel@linux.microsoft.com>]

Le vendredi 22 novembre 2024, 15:33:50 CET Mickaël Salaün a écrit :
> Do not pollute audit logs because of unknown sandboxed programs.
> Indeed, the sandboxer's security policy might not be fitted to the set
> of sandboxed processes that could be spawned (e.g. from a shell).
> 
> The LANDLOCK_RESTRICT_SELF_LOGLESS flag should be used for all similar
> sandboxer tools by default.  Only natively-sandboxed programs should not
> use this flag.
> 
> For test purpose, parse the LL_FORCE_LOG environment variable to still
> log denials.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241122143353.59367-21-mic@digikod.net
> ---
> 
> Changes since v2:
> - New patch.
> ---
>  samples/landlock/sandboxer.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index 57565dfd74a2..49fd4fdb1dcf 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -58,6 +58,7 @@ static inline int landlock_restrict_self(const int
> ruleset_fd, #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
>  #define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
>  #define ENV_SCOPED_NAME "LL_SCOPED"
> +#define ENV_FORCE_LOG_NAME "LL_FORCE_LOG"
>  #define ENV_DELIMITER ":"
> 
>  static int str2num(const char *numstr, __u64 *num_dst)
> @@ -288,7 +289,7 @@ static bool check_ruleset_scope(const char *const
> env_var,
> 
>  /* clang-format on */
> 
> -#define LANDLOCK_ABI_LAST 6
> +#define LANDLOCK_ABI_LAST 7
> 
>  #define XSTR(s) #s
>  #define STR(s) XSTR(s)
> @@ -315,6 +316,9 @@ static const char help[] =
>  	"  - \"a\" to restrict opening abstract unix sockets\n"
>  	"  - \"s\" to restrict sending signals\n"
>  	"\n"
> +	"A sandboxer should not log denied access requests to avoid spamming 
logs,
> " +	"but to test audit we can set " ENV_FORCE_LOG_NAME "=1\n"
> +	"\n"
>  	"Example:\n"
>  	ENV_FS_RO_NAME "=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
>  	ENV_FS_RW_NAME "=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
> @@ -333,7 +337,7 @@ int main(const int argc, char *const argv[], char *const
> *const envp) const char *cmd_path;
>  	char *const *cmd_argv;
>  	int ruleset_fd, abi;
> -	char *env_port_name;
> +	char *env_port_name, *env_force_log;
>  	__u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
>  	      access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
> 
> @@ -344,6 +348,8 @@ int main(const int argc, char *const argv[], char *const
> *const envp) .scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
>  			  LANDLOCK_SCOPE_SIGNAL,
>  	};
> +	/* Do not pollute audit logs because of unknown sandboxed programs. */
> +	int restrict_flags = LANDLOCK_RESTRICT_SELF_LOGLESS;
> 
>  	if (argc < 2) {
>  		fprintf(stderr, help, argv[0]);
> @@ -415,6 +421,12 @@ int main(const int argc, char *const argv[], char
> *const *const envp) /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
>  		ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
>  					 LANDLOCK_SCOPE_SIGNAL);
> +		__attribute__((fallthrough));
> +	case 6:
> +		/* Removes LANDLOCK_RESTRICT_SELF_LOGLESS for ABI < 7 */
> +		restrict_flags &= ~LANDLOCK_RESTRICT_SELF_LOGLESS;
> +
> +		/* Must be printed for any ABI < LANDLOCK_ABI_LAST. */
>  		fprintf(stderr,
>  			"Hint: You should update the running kernel "
>  			"to leverage Landlock features "
> @@ -449,6 +461,23 @@ int main(const int argc, char *const argv[], char
> *const *const envp) if (check_ruleset_scope(ENV_SCOPED_NAME,
> &ruleset_attr))
>  		return 1;
> 
> +	/* Enables optional logs. */
> +	env_force_log = getenv(ENV_FORCE_LOG_NAME);
> +	if (env_force_log) {
> +		if (strcmp(env_force_log, "1") != 0) {
> +			fprintf(stderr,
> +				"Unknown value for " ENV_FORCE_LOG_NAME "\n");

"Unknown value for " ENV_FORCE_LOG_NAME ", got %s, expected 1\n", 
env_force_log)
This really helps users to figure out what they made wrong.
> +			return 1;
> +		}
> +		if (!(restrict_flags & LANDLOCK_RESTRICT_SELF_LOGLESS)) {
> +			fprintf(stderr,
> +				"Audit logs not supported by current kernel\n");
> +			return 1;
> +		}
> +		restrict_flags &= ~LANDLOCK_RESTRICT_SELF_LOGLESS;
> +		unsetenv(ENV_FORCE_LOG_NAME);
> +	}
> +
>  	ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 
0);
>  	if (ruleset_fd < 0) {
> @@ -476,7 +505,7 @@ int main(const int argc, char *const argv[], char *const
> *const envp) perror("Failed to restrict privileges");
>  		goto err_close_ruleset;
>  	}
> -	if (landlock_restrict_self(ruleset_fd, 0)) {
> +	if (landlock_restrict_self(ruleset_fd, restrict_flags)) {
>  		perror("Failed to enforce ruleset");
>  		goto err_close_ruleset;
>  	}