Skip to content

Commit

Permalink
Merge branch 'unprivileged'
Browse files Browse the repository at this point in the history
This fixes test crashes when the following conditions are met: the
unprivileged_user is defined in the configuration; the test case requests
to be run as an unprivileged user; and the current user is not root.
  • Loading branch information
jmmv committed Nov 21, 2015
2 parents 7c8d304 + 6caa3ae commit ca7548d
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
4 changes: 2 additions & 2 deletions utils/process/isolation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ process::isolate_child(const optional< passwd::user >& unprivileged_user,
prepare_environment(work_directory);
(void)::umask(0022);

if (unprivileged_user) {
if (unprivileged_user && passwd::current_user().is_root()) {
const passwd::user& user = unprivileged_user.get();

if (user.gid != ::getgid()) {
Expand Down Expand Up @@ -187,7 +187,7 @@ void
process::isolate_path(const optional< passwd::user >& unprivileged_user,
const fs::path& file)
{
if (!unprivileged_user)
if (!unprivileged_user || !passwd::current_user().is_root())
return;
const passwd::user& user = unprivileged_user.get();

Expand Down
50 changes: 50 additions & 0 deletions utils/process/isolation_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,25 @@ ATF_TEST_CASE_BODY(isolate_child__clean_environment)
}


ATF_TEST_CASE(isolate_child__other_user_when_unprivileged);
ATF_TEST_CASE_HEAD(isolate_child__other_user_when_unprivileged)
{
set_md_var("require.user", "unprivileged");
}
ATF_TEST_CASE_BODY(isolate_child__other_user_when_unprivileged)
{
const passwd::user user = passwd::current_user();

passwd::user other_user = user;
other_user.uid += 1;
other_user.gid += 1;
process::isolate_child(utils::make_optional(other_user), fs::path("."));

ATF_REQUIRE_EQ(user.uid, ::getuid());
ATF_REQUIRE_EQ(user.gid, ::getgid());
}


ATF_TEST_CASE(isolate_child__drop_privileges);
ATF_TEST_CASE_HEAD(isolate_child__drop_privileges)
{
Expand All @@ -340,6 +359,13 @@ ATF_TEST_CASE_HEAD(isolate_child__drop_privileges_fail_uid)
}
ATF_TEST_CASE_BODY(isolate_child__drop_privileges_fail_uid)
{
// Fake the current user as root so that we bypass the protections in
// isolate_child that prevent us from attempting a user switch when we are
// not root. We do this so we can trigger the setuid failure.
passwd::user root = passwd::user("root", 0, 0);
ATF_REQUIRE(root.is_root());
passwd::set_current_user_for_testing(root);

passwd::user unprivileged_user = passwd::current_user();
unprivileged_user.uid += 1;

Expand All @@ -359,6 +385,13 @@ ATF_TEST_CASE_HEAD(isolate_child__drop_privileges_fail_gid)
}
ATF_TEST_CASE_BODY(isolate_child__drop_privileges_fail_gid)
{
// Fake the current user as root so that we bypass the protections in
// isolate_child that prevent us from attempting a user switch when we are
// not root. We do this so we can trigger the setgid failure.
passwd::user root = passwd::user("root", 0, 0);
ATF_REQUIRE(root.is_root());
passwd::set_current_user_for_testing(root);

passwd::user unprivileged_user = passwd::current_user();
unprivileged_user.gid += 1;

Expand Down Expand Up @@ -497,6 +530,21 @@ ATF_TEST_CASE_BODY(isolate_path__same_user)
}


ATF_TEST_CASE(isolate_path__other_user_when_unprivileged);
ATF_TEST_CASE_HEAD(isolate_path__other_user_when_unprivileged)
{
set_md_var("require.user", "unprivileged");
}
ATF_TEST_CASE_BODY(isolate_path__other_user_when_unprivileged)
{
passwd::user user = passwd::current_user();
user.uid += 1;
user.gid += 1;

do_isolate_path_test(utils::make_optional(user), none, none);
}


ATF_TEST_CASE(isolate_path__drop_privileges);
ATF_TEST_CASE_HEAD(isolate_path__drop_privileges)
{
Expand Down Expand Up @@ -550,6 +598,7 @@ ATF_TEST_CASE_BODY(isolate_path__drop_privileges_only_gid)
ATF_INIT_TEST_CASES(tcs)
{
ATF_ADD_TEST_CASE(tcs, isolate_child__clean_environment);
ATF_ADD_TEST_CASE(tcs, isolate_child__other_user_when_unprivileged);
ATF_ADD_TEST_CASE(tcs, isolate_child__drop_privileges);
ATF_ADD_TEST_CASE(tcs, isolate_child__drop_privileges_fail_uid);
ATF_ADD_TEST_CASE(tcs, isolate_child__drop_privileges_fail_gid);
Expand All @@ -563,6 +612,7 @@ ATF_INIT_TEST_CASES(tcs)

ATF_ADD_TEST_CASE(tcs, isolate_path__no_user);
ATF_ADD_TEST_CASE(tcs, isolate_path__same_user);
ATF_ADD_TEST_CASE(tcs, isolate_path__other_user_when_unprivileged);
ATF_ADD_TEST_CASE(tcs, isolate_path__drop_privileges);
ATF_ADD_TEST_CASE(tcs, isolate_path__drop_privileges_only_uid);
ATF_ADD_TEST_CASE(tcs, isolate_path__drop_privileges_only_gid);
Expand Down

0 comments on commit ca7548d

Please sign in to comment.