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.

Sign up for GitHub

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

Jump to bottom

[merged] random fixes #117

Closed
wants to merge 3 commits into from
Closed

[merged] random fixes #117

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76b165a
bwrap: setuid to the sandbox uid
giuseppe Nov 10, 2016
5f569bd
bwrap: fix typos
giuseppe Nov 10, 2016
51b762c
bubblewrap: do not leave zombie process
giuseppe Oct 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions bubblewrap.c
View file
Open in desktop
Expand Up @@ -444,7 +444,7 @@ has_caps (void)
* unprivileged user namespaces to be used. This case is
* "is_privileged = FALSE".
*
* If bwarp is setuid, then we do things in phases.
* If bwrap is setuid, then we do things in phases.
* The first part is run as euid 0, but with with fsuid as the real user.
* The second part, inside the child, is run as the real user but with
* capabilities.
Expand Down Expand Up @@ -492,7 +492,7 @@ acquire_privs (void)
else if (real_uid != 0 && has_caps ())
{
/* We have some capabilities in the non-setuid case, which should not happen.
Probablye caused by the binary being setcap instead of setuid which we
Probably caused by the binary being setcap instead of setuid which we
don't support anymore */
die ("Unexpected capabilities but not setuid, old file caps config?");
}
Expand All @@ -511,7 +511,7 @@ switch_to_user_with_privs (void)
if (prctl (PR_SET_KEEPCAPS, 1, 0, 0, 0) < 0)
die_with_error ("prctl(PR_SET_KEEPCAPS) failed");

if (setuid (real_uid) < 0)
if (setuid (opt_sandbox_uid) < 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, we're not currently validating that opt_sandbox_uid being set requires --unshare-user AFAICS. Which I think means this could be used to gain uid 0 in the host userns, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

if --uid was not specified (which is allowed only with --unshare-user), then opt_sandbox_uid is set to real_uid in main.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We do:

  if (!opt_unshare_user && opt_sandbox_uid != real_uid)
    die ("Specifying --uid requires --unshare-user");

Which looks ok to me.

die_with_error ("unable to drop root uid");

/* Regain effective required capabilities from permitted */
Expand All @@ -525,7 +525,7 @@ drop_privs (void)
return;

/* Drop root uid */
if (setuid (real_uid) < 0)
if (setuid (opt_sandbox_uid) < 0)
die_with_error ("unable to drop root uid");

drop_all_caps ();
Expand Down Expand Up @@ -1907,6 +1907,7 @@ main (int argc,
}
else
{
int status;
uint32_t buffer[2048]; /* 8k, but is int32 to guarantee nice alignment */
uint32_t op, flags;
const char *arg1, *arg2;
Expand All @@ -1925,6 +1926,7 @@ main (int argc,
}
while (op != PRIV_SEP_OP_DONE);

waitpid (child, &status, 0);
/* Continue post setup */
}
}
Expand Down