Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Landlock support to Firejail #5315

Merged
merged 5 commits into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config.mk.in
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ HAVE_PRIVATE_HOME=@HAVE_PRIVATE_HOME@
HAVE_IDS=@HAVE_IDS@
HAVE_GCOV=@HAVE_GCOV@
HAVE_SELINUX=@HAVE_SELINUX@
HAVE_LANDLOCK=@HAVE_LANDLOCK@
HAVE_SUID=@HAVE_SUID@
HAVE_DBUSPROXY=@HAVE_DBUSPROXY@
HAVE_USERTMPFS=@HAVE_USERTMPFS@
Expand All @@ -49,7 +50,7 @@ HAVE_LTS=@HAVE_LTS@
HAVE_FORCE_NONEWPRIVS=@HAVE_FORCE_NONEWPRIVS@
HAVE_ONLY_SYSCFG_PROFILES=@HAVE_ONLY_SYSCFG_PROFILES@

MANFLAGS = $(HAVE_LTS) $(HAVE_OUTPUT) $(HAVE_X11) $(HAVE_PRIVATE_HOME) $(HAVE_APPARMOR) $(HAVE_IDS) $(HAVE_OVERLAYFS) $(HAVE_USERTMPFS) $(HAVE_DBUSPROXY) $(HAVE_FIRETUNNEL) $(HAVE_GLOBALCFG) $(HAVE_CHROOT) $(HAVE_NETWORK) $(HAVE_USERNS) $(HAVE_FILE_TRANSFER) $(HAVE_SELINUX) $(HAVE_SUID) $(HAVE_FORCE_NONEWPRIVS) $(HAVE_ONLY_SYSCFG_PROFILES)
MANFLAGS = $(HAVE_LTS) $(HAVE_OUTPUT) $(HAVE_X11) $(HAVE_PRIVATE_HOME) $(HAVE_APPARMOR) $(HAVE_IDS) $(HAVE_OVERLAYFS) $(HAVE_USERTMPFS) $(HAVE_DBUSPROXY) $(HAVE_FIRETUNNEL) $(HAVE_GLOBALCFG) $(HAVE_CHROOT) $(HAVE_NETWORK) $(HAVE_USERNS) $(HAVE_FILE_TRANSFER) $(HAVE_SELINUX) $(HAVE_SUID) $(HAVE_LANDLOCK) $(HAVE_FORCE_NONEWPRIVS) $(HAVE_ONLY_SYSCFG_PROFILES)

CC=@CC@
CFLAGS=@CFLAGS@
Expand Down
16 changes: 16 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ HAVE_DBUSPROXY
EXTRA_LDFLAGS
EXTRA_CFLAGS
HAVE_SELINUX
HAVE_LANDLOCK
AA_LIBS
AA_CFLAGS
PKG_CONFIG_LIBDIR
Expand Down Expand Up @@ -713,6 +714,7 @@ enable_sanitizer
enable_ids
enable_apparmor
enable_selinux
enable_landlock
enable_dbusproxy
enable_output
enable_usertmpfs
Expand Down Expand Up @@ -1374,6 +1376,7 @@ Optional Features:
--enable-ids enable ids
--enable-apparmor enable apparmor
--enable-selinux SELinux labeling support
--enable-landlock Landlock self-restriction support
--disable-dbusproxy disable dbus proxy
--disable-output disable --output logging
--disable-usertmpfs disable tmpfs as regular user
Expand Down Expand Up @@ -3342,7 +3345,19 @@ if test "x$enable_selinux" = "xyes"; then :

fi

HAVE_LANDLOCK=""

# Check whether --enable-landlock was given.
if test "${enable_landlock+set}" = set; then :
enableval=$enable_landlock;
fi

if test "x$enable_landlock" = "xyes"; then :

HAVE_LANDLOCK="-DHAVE_LANDLOCK"
EXTRA_LDFLAGS="$EXTRA_LDFLAGS"

fi



Expand Down Expand Up @@ -5277,6 +5292,7 @@ Features:
overlayfs support: $HAVE_OVERLAYFS
private home support: $HAVE_PRIVATE_HOME
SELinux labeling support: $HAVE_SELINUX
Landlock self-restriction support: $HAVE_LANDLOCK
user namespace: $HAVE_USERNS
X11 sandboxing support: $HAVE_X11

Expand Down
10 changes: 9 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,18 @@ AS_IF([test "x$enable_selinux" = "xyes"], [
EXTRA_LDFLAGS="$EXTRA_LDFLAGS -lselinux"
])

HAVE_LANDLOCK=""
AC_SUBST([HAVE_LANDLOCK])
AC_ARG_ENABLE([landlock])
[AS_HELP_STRING([--enable-landlock], [Landlock self-restriction support])]
AS_IF([test "x$enable_landlock" = "xyes"], [
HAVE_LANDLOCK="-DHAVE_LANDLOCK"
EXTRA_LDFLAGS="$EXTRA_LDFLAGS"
])

AC_SUBST([EXTRA_CFLAGS])
AC_SUBST([EXTRA_LDFLAGS])


HAVE_DBUSPROXY=""
AC_SUBST([HAVE_DBUSPROXY])
AC_ARG_ENABLE([dbusproxy],
Expand Down
2 changes: 1 addition & 1 deletion contrib/vim/syntax/firejail.vim
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ syn match fjVar /\v\$\{(CFG|DESKTOP|DOCUMENTS|DOWNLOADS|HOME|MUSIC|PATH|PICTURES

" Commands grabbed from: src/firejail/profile.c
" Generate list with: { rg -o 'strn?cmp\(ptr, "([^"]+) "' -r '$1' src/firejail/profile.c; echo private-lib; } | grep -vEx '(include|ignore|caps\.drop|caps\.keep|protocol|restrict-namespaces|seccomp|seccomp\.drop|seccomp\.keep|env|rmenv|net|ip)' | sort -u | tr $'\n' '|' # private-lib is special-cased in the code and doesn't match the regex; grep-ed patterns are handled later with 'syn match nextgroup=' directives (except for include which is special-cased as a fjCommandNoCond keyword)
syn match fjCommand /\v(apparmor|bind|blacklist|blacklist-nolog|cpu|defaultgw|dns|hostname|hosts-file|ip6|iprange|join-or-start|mac|mkdir|mkfile|mtu|name|netfilter|netfilter6|netmask|nice|noblacklist|noexec|nowhitelist|overlay-named|private|private-bin|private-cwd|private-etc|private-home|private-lib|private-opt|private-srv|read-only|read-write|rlimit-as|rlimit-cpu|rlimit-fsize|rlimit-nofile|rlimit-nproc|rlimit-sigpending|timeout|tmpfs|veth-name|whitelist|xephyr-screen) / skipwhite contained
syn match fjCommand /\v(apparmor|bind|blacklist|blacklist-nolog|cpu|defaultgw|dns|hostname|hosts-file|ip6|iprange|join-or-start|landlock|landlock.proc|landlock.read|landlock.write|landlock.special|landlock.execute|mac|mkdir|mkfile|mtu|name|netfilter|netfilter6|netmask|nice|noblacklist|noexec|nowhitelist|overlay-named|private|private-bin|private-cwd|private-etc|private-home|private-lib|private-opt|private-srv|read-only|read-write|rlimit-as|rlimit-cpu|rlimit-fsize|rlimit-nofile|rlimit-nproc|rlimit-sigpending|timeout|tmpfs|veth-name|whitelist|xephyr-screen) / skipwhite contained
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be sorted by name.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

landlock takes no value, right?

" Generate list with: rg -o 'strn?cmp\(ptr, "([^ "]*[^ ])"' -r '$1' src/firejail/profile.c | grep -vEx '(include|rlimit|quiet)' | sed -e 's/\./\\./' | sort -u | tr $'\n' '|' # include/rlimit are false positives, quiet is special-cased below
syn match fjCommand /\v(allow-debuggers|allusers|apparmor|caps|deterministic-exit-code|deterministic-shutdown|disable-mnt|ipc-namespace|keep-config-pulse|keep-dev-shm|keep-fd|keep-var-tmp|machine-id|memory-deny-write-execute|netfilter|no3d|noautopulse|nodbus|nodvd|nogroups|noinput|nonewprivs|noprinters|noroot|nosound|notv|nou2f|novideo|overlay|overlay-tmpfs|private|private-cache|private-cwd|private-dev|private-lib|private-tmp|seccomp|seccomp\.32|seccomp\.block-secondary|tracelog|writable-etc|writable-run-user|writable-var|writable-var-log|x11)$/ contained
syn match fjCommand /ignore / nextgroup=fjCommand,fjCommandNoCond skipwhite contained
Expand Down
16 changes: 16 additions & 0 deletions src/bash_completion/firejail.bash_completion.in
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,22 @@ _firejail()
_filedir -d
return 0
;;
--landlock.read)
_filedir
return 0
;;
--landlock.write)
_filedir
return 0
;;
--landlock.special)
_filedir
return 0
;;
--landlock.execute)
_filedir
return 0
;;
--tmpfs)
_filedir
return 0
Expand Down
26 changes: 26 additions & 0 deletions src/firejail/firejail.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,35 @@
#include "../include/common.h"
#include "../include/euid_common.h"
#include "../include/rundefs.h"
#ifdef HAVE_LANDLOCK
#include <linux/landlock.h>
#endif
#include <linux/limits.h> // Note: Plain limits.h may break ARG_MAX (see #4583)
#include <stdarg.h>
#include <sys/stat.h>

// debug restricted shell
//#define DEBUG_RESTRICTED_SHELL

#ifdef HAVE_LANDLOCK

extern int landlock_create_ruleset(struct landlock_ruleset_attr *rsattr,size_t size,__u32 flags);

extern int landlock_add_rule(int fd,enum landlock_rule_type t,void *attr,__u32 flags);

extern int landlock_restrict_self(int fd,__u32 flags);

extern int create_full_ruleset();

extern int add_read_access_rule_by_path(int rset_fd,char *allowed_path);

extern int add_write_access_rule_by_path(int rset_fd,char *allowed_path);

extern int add_create_special_rule_by_path(int rset_fd,char *allowed_path);

extern int add_execute_rule_by_path(int rset_fd,char *allowed_path);

#endif
Comment on lines +35 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the #ifdef (see the new gcov comment) and extraneous blank lines (see
the rest of the file).


// profiles
#define DEFAULT_USER_PROFILE "default"
Expand Down Expand Up @@ -286,6 +307,11 @@ extern int arg_seccomp32; // enable default seccomp filter for 32 bit arch
extern int arg_seccomp_postexec; // need postexec ld.preload library?
extern int arg_seccomp_block_secondary; // block any secondary architectures

#ifdef HAVE_LANDLOCK
extern int arg_landlock; // Landlock ruleset file descriptor
extern int arg_landlock_proc; // Landlock rule for accessing /proc (0 for no access, 1 for read-only and 2 for read-write)
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the #ifdef and move them below this line (see the new gcov comment):

extern char *apparmor_profile;	// apparmor profile

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was very confusing to see an arg_ variable refer to a data structure,
rather than a flag or something that is actually specified on the command line.

I thought that it meant the path argument that is eventually passed to a
landlock call (such as the PATH in landlock.foo PATH) and only noticed that
that is not the case because argv[i] + N is also passed to the landlock calls
in main.c:

+		else if (strncmp(argv[i], "--landlock.read=", 16) == 0) {
+			if (arg_landlock == -1)
+				arg_landlock = create_full_ruleset();
+			if (add_read_access_rule_by_path(arg_landlock, argv[i] + 16)) {
+				fprintf(stderr, "An error has occured while adding a rule to the Landlock ruleset.\n");
+			}
+		}

Suggestion: Avoid being too clever and just use separate variables. Use
arg_landlock to check if --landlock is used (for example, in sandbox.c) and
add a new landlock_ruleset_fd variable to store the file descriptor (note
that this variable name matches the kernel API):

 extern int arg_apparmor;	// apparmor
 extern char *apparmor_profile;	// apparmor profile
+extern int arg_landlock;	// Landlock
+extern int arg_landlock_proc;	// Landlock rule for accessing /proc (0 for no access, 1 for read-only and 2 for read-write)
+extern int landlock_ruleset_fd;	// Landlock ruleset file descriptor

Example usage on main.c:

 		else if (strcmp(argv[i], "--landlock") == 0) {
+			arg_landlock = 1;
+
-			if (arg_landlock == -1)
-				arg_landlock = create_full_ruleset();
+			if (landlock_ruleset_fd < 0) {
+				landlock_ruleset_fd = create_full_ruleset();
+				if (landlock_ruleset_fd < 0)
+					errExit("create_full_ruleset");
+			}

The same applies to the other --landlock arguments (other than maybe setting
arg_landlock = 1).


extern int arg_caps_default_filter; // enable default capabilities filter
extern int arg_caps_drop; // drop list
extern int arg_caps_drop_all; // drop all capabilities
Expand Down
79 changes: 79 additions & 0 deletions src/firejail/landlock.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#define _GNU_SOURCE
#include <stdio.h>
#include <stddef.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/prctl.h>
#include <linux/prctl.h>
#include <linux/landlock.h>
Copy link
Collaborator

@kmk3 kmk3 Aug 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is breaking the build on systems that do not support Landlock (such as on
Ubuntu 20.04, which uses an older kernel), even when Landlock is not enabled.

Partial error log of the codeql analyze job from
https://github.com/netblue30/firejail/runs/7853807421:

gcc -g -O2 -ggdb  -O2 -DVERSION='"0.9.71"'  -DPREFIX='"/usr"' -DSYSCONFDIR='"/etc/firejail"' -DLIBDIR='"/usr/lib"' -DBINDIR='"/usr/bin"'   -DVARDIR='"/var/lib/firejail"'  -DHAVE_OUTPUT -DHAVE_X11 -DHAVE_PRIVATE_HOME    -DHAVE_USERTMPFS -DHAVE_DBUSPROXY  -DHAVE_GLOBALCFG -DHAVE_CHROOT -DHAVE_NETWORK -DHAVE_USERNS -DHAVE_FILE_TRANSFER  -DHAVE_SUID    -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -Wformat -Wformat-security -fstack-clash-protection -fstack-protector-strong  -c landlock.c -o landlock.o
  landlock.c:11:10: fatal error: linux/landlock.h: No such file or directory
     11 | #include <linux/landlock.h>
        |          ^~~~~~~~~~~~~~~~~~
  compilation terminated.
  make[1]: *** [Makefile:8: landlock.o] Error 1
  make[1]: Leaving directory '/home/runner/work/firejail/firejail/src/firejail'
  make: *** [Makefile:31: src/firejail/firejail] Error 2

See src/include/gcov_wrapper.h and try to create a landlock_wrapper.h in a
similar fashion. That is:

  1. In landlock_wrapper.h, declare dummy functions if HAVE_LANDLOCK is not
    defined.
  2. Outside of landlock_wrapper.h, only include "landlock_wrapper.h" instead
    of <linux/landlock.h>.
  3. Outside of landlock_wrapper.h, avoid using #ifdef HAVE_LANDLOCK where it
    can be helped (just call the landlock functions as if LandLock is always
    enabled).


int landlock_create_ruleset(struct landlock_ruleset_attr *rsattr,size_t size,__u32 flags) {
return syscall(__NR_landlock_create_ruleset,rsattr,size,flags);
}

int landlock_add_rule(int fd,enum landlock_rule_type t,void *attr,__u32 flags) {
return syscall(__NR_landlock_add_rule,fd,t,attr,flags);
}

int landlock_restrict_self(int fd,__u32 flags) {
prctl(PR_SET_NO_NEW_PRIVS,1,0,0,0);
int result = syscall(__NR_landlock_restrict_self,fd,flags);
if (result!=0) return result;
else {
close(fd);
return 0;
}
Comment on lines +24 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (result!=0) return result;
else {
close(fd);
return 0;
}
if (result != 0)
return result;
else {
close(fd);
return 0;
}

if () foo is done nowhere in the code AFAIK.

The same applies to other if () foo changes in this PR.

}

int create_full_ruleset() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All functions in this file should probably have a landlock_ prefix to make it
clear on the call site that they are about Landlock (rather than being about
something else, like AppArmor).

struct landlock_ruleset_attr attr;
attr.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR | LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_REMOVE_FILE | LANDLOCK_ACCESS_FS_REMOVE_DIR | LANDLOCK_ACCESS_FS_MAKE_CHAR | LANDLOCK_ACCESS_FS_MAKE_DIR | LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_MAKE_SOCK | LANDLOCK_ACCESS_FS_MAKE_FIFO | LANDLOCK_ACCESS_FS_MAKE_BLOCK | LANDLOCK_ACCESS_FS_MAKE_SYM | LANDLOCK_ACCESS_FS_EXECUTE;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
attr.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR | LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_REMOVE_FILE | LANDLOCK_ACCESS_FS_REMOVE_DIR | LANDLOCK_ACCESS_FS_MAKE_CHAR | LANDLOCK_ACCESS_FS_MAKE_DIR | LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_MAKE_SOCK | LANDLOCK_ACCESS_FS_MAKE_FIFO | LANDLOCK_ACCESS_FS_MAKE_BLOCK | LANDLOCK_ACCESS_FS_MAKE_SYM | LANDLOCK_ACCESS_FS_EXECUTE;
attr.handled_access_fs = \
LANDLOCK_ACCESS_FS_READ_FILE |
LANDLOCK_ACCESS_FS_READ_DIR |
LANDLOCK_ACCESS_FS_WRITE_FILE |
LANDLOCK_ACCESS_FS_REMOVE_FILE |
LANDLOCK_ACCESS_FS_REMOVE_DIR |
LANDLOCK_ACCESS_FS_MAKE_CHAR |
LANDLOCK_ACCESS_FS_MAKE_DIR |
LANDLOCK_ACCESS_FS_MAKE_REG |
LANDLOCK_ACCESS_FS_MAKE_SOCK |
LANDLOCK_ACCESS_FS_MAKE_FIFO |
LANDLOCK_ACCESS_FS_MAKE_BLOCK |
LANDLOCK_ACCESS_FS_MAKE_SYM |
LANDLOCK_ACCESS_FS_EXECUTE;

To make it readable.

The same applies to the other target.allowed_access changes as well.

return landlock_create_ruleset(&attr,sizeof(attr),0);
}

int add_read_access_rule_by_path(int rset_fd,char *allowed_path) {
int result;
int allowed_fd = open(allowed_path,O_PATH | O_CLOEXEC);
struct landlock_path_beneath_attr target;
target.parent_fd = allowed_fd;
target.allowed_access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
result = landlock_add_rule(rset_fd,LANDLOCK_RULE_PATH_BENEATH,&target,0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many whitespace issues. See this again:

close(allowed_fd);
return result;
}

int add_write_access_rule_by_path(int rset_fd,char *allowed_path) {
int result;
int allowed_fd = open(allowed_path,O_PATH | O_CLOEXEC);
struct landlock_path_beneath_attr target;
target.parent_fd = allowed_fd;
target.allowed_access = LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_REMOVE_FILE | LANDLOCK_ACCESS_FS_REMOVE_DIR | LANDLOCK_ACCESS_FS_MAKE_CHAR | LANDLOCK_ACCESS_FS_MAKE_DIR | LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_MAKE_SYM;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add LANDLOCK_ACCESS_FS_MAKE_CHAR here instead of special? (I want to understand)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a mistake. I plan to move it to special

result = landlock_add_rule(rset_fd,LANDLOCK_RULE_PATH_BENEATH,&target,0);
close(allowed_fd);
return result;
}

int add_create_special_rule_by_path(int rset_fd,char *allowed_path) {
int result;
int allowed_fd = open(allowed_path,O_PATH | O_CLOEXEC);
struct landlock_path_beneath_attr target;
target.parent_fd = allowed_fd;
target.allowed_access = LANDLOCK_ACCESS_FS_MAKE_SOCK | LANDLOCK_ACCESS_FS_MAKE_FIFO | LANDLOCK_ACCESS_FS_MAKE_BLOCK;
result = landlock_add_rule(rset_fd,LANDLOCK_RULE_PATH_BENEATH,&target,0);
close(allowed_fd);
return result;
}

int add_execute_rule_by_path(int rset_fd,char *allowed_path) {
int result;
int allowed_fd = open(allowed_path,O_PATH | O_CLOEXEC);
struct landlock_path_beneath_attr target;
target.parent_fd = allowed_fd;
target.allowed_access = LANDLOCK_ACCESS_FS_EXECUTE;
result = landlock_add_rule(rset_fd,LANDLOCK_RULE_PATH_BENEATH,&target,0);
close(allowed_fd);
return result;
}
81 changes: 81 additions & 0 deletions src/firejail/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ int arg_seccomp_postexec = 0; // need postexec ld.preload library?
int arg_seccomp_block_secondary = 0; // block any secondary architectures
int arg_seccomp_error_action = 0;

#ifdef HAVE_LANDLOCK
int arg_landlock = -1; // Landlock ruleset file descriptor (-1 if it doesn't exist)
int arg_landlock_proc = 0; // Landlock rule for accessing /proc (0 for no access, 1 for read-only and 2 for read-write)
#endif

int arg_caps_default_filter = 0; // enable default capabilities filter
int arg_caps_drop = 0; // drop list
int arg_caps_drop_all = 0; // drop all capabilities
Expand Down Expand Up @@ -1401,6 +1406,82 @@ int main(int argc, char **argv, char **envp) {
else
exit_err_feature("seccomp");
}
#ifdef HAVE_LANDLOCK
else if (strcmp(argv[i], "--landlock") == 0) {
if (arg_landlock == -1) arg_landlock = create_full_ruleset();
const char *home_dir = env_get("HOME");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use cfg.homedir instead.

The same applies to the other env_get("HOME") calls.

int home_fd = open(home_dir,O_PATH | O_CLOEXEC);
struct landlock_path_beneath_attr target;
target.parent_fd = home_fd;
target.allowed_access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR | LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_REMOVE_FILE | LANDLOCK_ACCESS_FS_REMOVE_DIR | LANDLOCK_ACCESS_FS_MAKE_CHAR | LANDLOCK_ACCESS_FS_MAKE_DIR | LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_MAKE_SYM;
if (landlock_add_rule(arg_landlock,LANDLOCK_RULE_PATH_BENEATH,&target,0)) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
close(home_fd);
if (add_read_access_rule_by_path(arg_landlock, "/bin/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
Comment on lines +1417 to +1423
Copy link
Collaborator

@kmk3 kmk3 Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (landlock_add_rule(arg_landlock,LANDLOCK_RULE_PATH_BENEATH,&target,0)) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
close(home_fd);
if (add_read_access_rule_by_path(arg_landlock, "/bin/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (landlock_add_rule(arg_landlock, LANDLOCK_RULE_PATH_BENEATH, &target, 0)) {
fprintf(stderr, "Error: cannot add a landlock rule to %s\n: %s",
cfg.homedir, strerror(errno));
}
if (add_read_access_rule_by_path(arg_landlock, "/bin/")) {
fprintf(stderr, "Error: cannot add a landlock rule to /bin: \n",
strerror(errno));
}

Make the error messages match the rest of the code and make them more
informative.

The same applies to the other fprintf calls.

if (add_execute_rule_by_path(arg_landlock, "/bin/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_read_access_rule_by_path(arg_landlock, "/dev/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_read_access_rule_by_path(arg_landlock, "/etc/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_read_access_rule_by_path(arg_landlock, "/lib/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_execute_rule_by_path(arg_landlock, "/lib/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_read_access_rule_by_path(arg_landlock, "/opt/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_execute_rule_by_path(arg_landlock, "/opt/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_read_access_rule_by_path(arg_landlock, "/usr/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_execute_rule_by_path(arg_landlock, "/usr/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
if (add_read_access_rule_by_path(arg_landlock, "/var/")) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
}
else if (strncmp(argv[i], "--landlock.proc=", 16) == 0) {
if (strncmp(argv[i]+16, "no", 2) == 0) arg_landlock_proc = 0;
else if (strncmp(argv[i]+16, "ro", 2) == 0) arg_landlock_proc = 1;
else if (strncmp(argv[i]+16, "rw", 2) == 0) arg_landlock_proc = 2;
}
else if (strncmp(argv[i], "--landlock.read=", 16) == 0) {
if (arg_landlock == -1) arg_landlock = create_full_ruleset();
if (add_read_access_rule_by_path(arg_landlock, argv[i]+16)) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
}
else if (strncmp(argv[i], "--landlock.write=", 17) == 0) {
if (arg_landlock == -1) arg_landlock = create_full_ruleset();
if (add_write_access_rule_by_path(arg_landlock, argv[i]+17)) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
}
else if (strncmp(argv[i], "--landlock.special=", 17) == 0) {
if (arg_landlock == -1) arg_landlock = create_full_ruleset();
if (add_create_special_rule_by_path(arg_landlock, argv[i]+17)) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
}
else if (strncmp(argv[i], "--landlock.execute=", 19) == 0) {
if (arg_landlock == -1) arg_landlock = create_full_ruleset();
if (add_execute_rule_by_path(arg_landlock, argv[i]+19)) {
fprintf(stderr,"An error has occured while adding a rule to the Landlock ruleset.\n");
}
}
#endif
else if (strcmp(argv[i], "--memory-deny-write-execute") == 0) {
if (checkcfg(CFG_SECCOMP))
arg_memory_deny_write_execute = 1;
Expand Down
Loading