Unverified Commit eba39ca4 authored by Mickaël Salaün's avatar Mickaël Salaün
Browse files

landlock: Change landlock_restrict_self(2) check ordering 

According to the Landlock goal to be a security feature available to
unprivileges processes, it makes more sense to first check for
no_new_privs before checking anything else (i.e. syscall arguments).

Merge inval_fd_enforce and unpriv_enforce_without_no_new_privs tests
into the new restrict_self_checks_ordering.  This is similar to the
previous commit checking other syscalls.

Link: https://lore.kernel.org/r/20220506160820.524344-10-mic@digikod.net


Cc: stable@vger.kernel.org
Signed-off-by: default avatarMickaël Salaün <mic@digikod.net>
parent 589172e5

security/landlock/syscalls.c

+4 −4
Original line number Diff line number Diff line
@@ -405,10 +405,6 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, 
	if (!landlock_initialized)

		return -EOPNOTSUPP;



	/* No flag for now. */

	if (flags)

		return -EINVAL;



	/*

	 * Similar checks as for seccomp(2), except that an -EPERM may be

	 * returned.
@@ -417,6 +413,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, 
	    !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))

		return -EPERM;



	/* No flag for now. */

	if (flags)

		return -EINVAL;



	/* Gets and checks the ruleset. */

	ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);

	if (IS_ERR(ruleset))

tools/testing/selftests/landlock/base_test.c

+37 −10
Original line number Diff line number Diff line
@@ -168,22 +168,49 @@ TEST(add_rule_checks_ordering) 
	ASSERT_EQ(0, close(ruleset_fd));

}



TEST(inval_fd_enforce)

/* Tests ordering of syscall argument and permission checks. */

TEST(restrict_self_checks_ordering)

{

	const struct landlock_ruleset_attr ruleset_attr = {

		.handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE,

	};

	struct landlock_path_beneath_attr path_beneath_attr = {

		.allowed_access = LANDLOCK_ACCESS_FS_EXECUTE,

		.parent_fd = -1,

	};

	const int ruleset_fd =

		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);



	ASSERT_LE(0, ruleset_fd);

	path_beneath_attr.parent_fd =

		open("/tmp", O_PATH | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC);

	ASSERT_LE(0, path_beneath_attr.parent_fd);

	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,

				       &path_beneath_attr, 0));

	ASSERT_EQ(0, close(path_beneath_attr.parent_fd));



	/* Checks unprivileged enforcement without no_new_privs. */

	drop_caps(_metadata);

	ASSERT_EQ(-1, landlock_restrict_self(-1, -1));

	ASSERT_EQ(EPERM, errno);

	ASSERT_EQ(-1, landlock_restrict_self(-1, 0));

	ASSERT_EQ(EPERM, errno);

	ASSERT_EQ(-1, landlock_restrict_self(ruleset_fd, 0));

	ASSERT_EQ(EPERM, errno);



	ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));



	/* Checks invalid flags. */

	ASSERT_EQ(-1, landlock_restrict_self(-1, -1));

	ASSERT_EQ(EINVAL, errno);



	/* Checks invalid ruleset FD. */

	ASSERT_EQ(-1, landlock_restrict_self(-1, 0));

	ASSERT_EQ(EBADF, errno);

}



TEST(unpriv_enforce_without_no_new_privs)

{

	int err;



	drop_caps(_metadata);

	err = landlock_restrict_self(-1, 0);

	ASSERT_EQ(EPERM, errno);

	ASSERT_EQ(err, -1);

	/* Checks valid call. */

	ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));

	ASSERT_EQ(0, close(ruleset_fd));

}



TEST(ruleset_fd_io)