selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0
@ 2022-10-13 13:23 Petr Lautrbach
  2022-10-13 13:23 ` [PATCH 2/2] sandbox: Use temporary directory for XDG_RUNTIME_DIR Petr Lautrbach
  2022-11-04 18:38 ` [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 James Carter
  0 siblings, 2 replies; 4+ messages in thread
From: Petr Lautrbach @ 2022-10-13 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

If the user is root, tmpdir is already wiped out.

Fixes:
    # sandbox -T /root/tmp -- id
    uid=0(root) gid=0(root) groups=0(root) context=unconfined_u:unconfined_r:sandbox_t:s0:c696,c756
    Failed to remove directory /tmp/.sandbox-root-KIlB59: No such file or directory

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 sandbox/seunshare.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c
index 8917a0f9bd42..dd1d7ddbdc89 100644
--- a/sandbox/seunshare.c
+++ b/sandbox/seunshare.c
@@ -414,7 +414,7 @@ static int cleanup_tmpdir(const char *tmpdir, const char *src,
 		rc++;
 	}
 
-	if (rmdir(tmpdir) == -1)
+	if (pwd->pw_uid != 0 && rmdir(tmpdir) == -1)
 		fprintf(stderr, _("Failed to remove directory %s: %s\n"), tmpdir, strerror(errno));
 	if ((uid_t)setfsuid(pwd->pw_uid) != 0) {
 		fprintf(stderr, _("unable to switch back to user after clearing tmp dir\n"));
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] sandbox: Use temporary directory for XDG_RUNTIME_DIR
  2022-10-13 13:23 [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 Petr Lautrbach
@ 2022-10-13 13:23 ` Petr Lautrbach
  2022-11-04 18:38 ` [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 James Carter
  1 sibling, 0 replies; 4+ messages in thread
From: Petr Lautrbach @ 2022-10-13 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

XDG_RUNTIME_DIR (/run/user/$UID) is used for user-specific data files
such as sockets, named pipes and so on. Therefore, it should not be
available to sandboxed processes.

Usage:
    # ls -a $XDG_RUNTIME_DIR
    .  ..  bus  pipewire-0  systemd
    # sandbox -R /root/sandbox/user -- sh -c "ls -a $XDG_RUNTIME_DIR"
    .  ..

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 sandbox/sandbox     | 15 ++++++++++++++-
 sandbox/sandbox.8   |  7 +++++--
 sandbox/seunshare.8 |  3 +++
 sandbox/seunshare.c | 45 +++++++++++++++++++++++++++++++++++----------
 4 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/sandbox/sandbox b/sandbox/sandbox
index ffac70232875..770807345858 100644
--- a/sandbox/sandbox
+++ b/sandbox/sandbox
@@ -209,6 +209,7 @@ class Sandbox:
         self.__level = None
         self.__homedir = None
         self.__tmpdir = None
+        self.__runuserdir = None
 
     def __validate_mount(self):
         if self.__options.level:
@@ -357,6 +358,11 @@ sandbox [-h] [-l level ] [-[X|M] [-H homedir] [-T tempdir]] [-I includefile ] [-
                           action="callback", callback=self.__validdir,
                           help=_("alternate /tmp directory to use for mounting"))
 
+        parser.add_option("-R", "--runuserdir", dest="runuserdir",
+                          type="string",
+                          action="callback", callback=self.__validdir,
+                          help=_("alternate XDG_RUNTIME_DIR - /run/user/$UID - directory to use for mounting"))
+
         parser.add_option("-w", "--windowsize", dest="windowsize",
                           type="string", default=DEFAULT_WINDOWSIZE,
                           help="size of the sandbox window")
@@ -401,10 +407,12 @@ sandbox [-h] [-l level ] [-[X|M] [-H homedir] [-T tempdir]] [-I includefile ] [-
             self.__options.X_ind = True
             self.__homedir = self.__options.homedir
             self.__tmpdir = self.__options.tmpdir
+            self.__runuserdir = self.__options.runuserdir
         else:
             if self.__options.level:
                 self.__homedir = self.__options.homedir
                 self.__tmpdir = self.__options.tmpdir
+                self.__runuserdir = self.__options.runuserdir
 
             if len(cmds) == 0:
                 self.usage(_("Command required"))
@@ -442,9 +450,14 @@ sandbox [-h] [-l level ] [-[X|M] [-H homedir] [-T tempdir]] [-I includefile ] [-
             self.__tmpdir = self.__options.tmpdir
         else:
             self.__tmpdir = mkdtemp(dir="/tmp", prefix=".sandbox_tmp_")
+        if self.__options.runuserdir:
+            self.__runuserdir = self.__options.runuserdir
+        else:
+            self.__runuserdir = mkdtemp(dir="/tmp", prefix=".sandbox_runuser_")
         self.__copyfiles()
         selinux.chcon(self.__homedir, self.__filecon, recursive=True)
         selinux.chcon(self.__tmpdir, self.__filecon, recursive=True)
+        selinux.chcon(self.__runuserdir, self.__filecon, recursive=True)
         selinux.setfscreatecon(None)
 
     def __execute(self):
@@ -453,7 +466,7 @@ sandbox [-h] [-l level ] [-[X|M] [-H homedir] [-T tempdir]] [-I includefile ] [-
             if self.__options.usecaps:
                 cmds.append('-C')
             if self.__mount:
-                cmds += ["-t", self.__tmpdir, "-h", self.__homedir]
+                cmds += ["-t", self.__tmpdir, "-h", self.__homedir, "-r", self.__runuserdir]
 
                 if self.__options.X_ind:
                     if self.__options.dpi:
diff --git a/sandbox/sandbox.8 b/sandbox/sandbox.8
index d83fee76f335..1ee0ecea96d1 100644
--- a/sandbox/sandbox.8
+++ b/sandbox/sandbox.8
@@ -3,11 +3,11 @@
 sandbox \- Run cmd under an SELinux sandbox
 .SH SYNOPSIS
 .B sandbox
-[\-C] [\-s] [ \-d DPI ] [\-l level ] [[\-M | \-X]  \-H homedir \-T tempdir ] [\-I includefile ] [ \-W windowmanager ] [ \-w windowsize ] [[\-i file ]...] [ \-t type ] cmd
+[\-C] [\-s] [ \-d DPI ] [\-l level ] [[\-M | \-X]  \-H homedir \-T tempdir ] [ \-R runuserdir ] [\-I includefile ] [ \-W windowmanager ] [ \-w windowsize ] [[\-i file ]...] [ \-t type ] cmd
 
 .br
 .B sandbox
-[\-C] [\-s] [ \-d DPI ] [\-l level ] [[\-M | \-X]  \-H homedir \-T tempdir ] [\-I includefile ] [ \-W windowmanager ] [ \-w windowsize ] [[\-i file ]...] [ \-t type ] \-S
+[\-C] [\-s] [ \-d DPI ] [\-l level ] [[\-M | \-X]  \-H homedir \-T tempdir ] [ \-R runuserdir ] [\-I includefile ] [ \-W windowmanager ] [ \-w windowsize ] [[\-i file ]...] [ \-t type ] \-S
 .br
 .SH DESCRIPTION
 .PP
@@ -67,6 +67,9 @@ sandbox_net_client_t	\-	All network ports
 \fB\-T\fR \fB\-\-tmpdir\fR
 Use alternate temporary directory to mount on /tmp.  Defaults to tmpfs. Requires \-X or \-M.
 .TP
+\fB\-R\fR \fB\-\-runuserdir\fR
+Use alternate temporary directory to mount on XDG_RUNTIME_DIR (/run/user/$UID).
+.TP
 \fB\-S\fR \fB\-\-session\fR
 Run a full desktop session, Requires level, and home and tmpdir.
 .TP
diff --git a/sandbox/seunshare.8 b/sandbox/seunshare.8
index 0da352613485..09cf7feae45d 100644
--- a/sandbox/seunshare.8
+++ b/sandbox/seunshare.8
@@ -18,6 +18,9 @@ Alternate homedir to be used by the application.  Homedir must be owned by the u
 \fB\-t\ tmpdir
 Use alternate temporary directory to mount on /tmp.  tmpdir must be owned by the user.
 .TP
+\fB\-r\ runuserdir
+Use alternate temporary directory to mount on XDG_RUNTIME_DIR (/run/user/$UID). runuserdir must be owned by the user.
+.TP
 \fB\-C --capabilities\fR
 Allow apps executed within the namespace to use capabilities.  Default is no capabilities.
 .TP
diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c
index dd1d7ddbdc89..1d38ea92b9ae 100644
--- a/sandbox/seunshare.c
+++ b/sandbox/seunshare.c
@@ -52,7 +52,7 @@
 
 #define BUF_SIZE 1024
 #define DEFAULT_PATH "/usr/bin:/bin"
-#define USAGE_STRING _("USAGE: seunshare [ -v ] [ -C ] [ -k ] [ -t tmpdir ] [ -h homedir ] [ -Z CONTEXT ] -- executable [args] ")
+#define USAGE_STRING _("USAGE: seunshare [ -v ] [ -C ] [ -k ] [ -t tmpdir ] [ -h homedir ] [ -r runuserdir ] [ -Z CONTEXT ] -- executable [args] ")
 
 static int verbose = 0;
 static int child = 0;
@@ -623,15 +623,20 @@ int main(int argc, char **argv) {
 	char *homedir_s = NULL;	/* homedir spec'd by user in argv[] */
 	char *tmpdir_s = NULL;	/* tmpdir spec'd by user in argv[] */
 	char *tmpdir_r = NULL;	/* tmpdir created by seunshare */
+	char *runuserdir_s = NULL;	/* /var/run/user/UID spec'd by user in argv[] */
+	char *runuserdir_r = NULL;	/* /var/run/user/UID created by seunshare */
 
 	struct stat st_curhomedir;
 	struct stat st_homedir;
 	struct stat st_tmpdir_s;
 	struct stat st_tmpdir_r;
+	struct stat st_runuserdir_s;
+	struct stat st_runuserdir_r;
 
 	const struct option long_options[] = {
 		{"homedir", 1, 0, 'h'},
 		{"tmpdir", 1, 0, 't'},
+		{"runuserdir", 1, 0, 'r'},
 		{"kill", 1, 0, 'k'},
 		{"verbose", 1, 0, 'v'},
 		{"context", 1, 0, 'Z'},
@@ -665,7 +670,7 @@ int main(int argc, char **argv) {
 	}
 
 	while (1) {
-		clflag = getopt_long(argc, argv, "Ccvh:t:Z:", long_options, NULL);
+		clflag = getopt_long(argc, argv, "Ccvh:r:t:Z:", long_options, NULL);
 		if (clflag == -1)
 			break;
 
@@ -679,6 +684,9 @@ int main(int argc, char **argv) {
 		case 'h':
 			homedir_s = optarg;
 			break;
+		case 'r':
+			runuserdir_s = optarg;
+			break;
 		case 'v':
 			verbose++;
 			break;
@@ -729,6 +737,10 @@ int main(int argc, char **argv) {
 	if (tmpdir_s && (
 		verify_directory(tmpdir_s, NULL, &st_tmpdir_s) < 0 ||
 		check_owner_uid(uid, tmpdir_s, &st_tmpdir_s))) return -1;
+	if (runuserdir_s && (
+		verify_directory(runuserdir_s, NULL, &st_runuserdir_s) < 0 ||
+		check_owner_uid(uid, runuserdir_s, &st_runuserdir_s))) return -1;
+
 	if ((uid_t)setfsuid(0) != uid) return -1;
 
 	/* create runtime tmpdir */
@@ -737,6 +749,12 @@ int main(int argc, char **argv) {
 		fprintf(stderr, _("Failed to create runtime temporary directory\n"));
 		return -1;
 	}
+	/* create runtime runuserdir */
+	if (runuserdir_s && (runuserdir_r = create_tmpdir(runuserdir_s, &st_runuserdir_s,
+						  &st_runuserdir_r, pwd, execcon)) == NULL) {
+		fprintf(stderr, _("Failed to create runtime $XDG_RUNTIME_DIR directory\n"));
+		return -1;
+	}
 
 	/* spawn child process */
 	child = fork();
@@ -775,7 +793,21 @@ int main(int argc, char **argv) {
 		if (check_owner_uid(uid, resolved_path, &st_curhomedir) < 0)
 			goto childerr;
 
-		/* mount homedir and tmpdir, in this order */
+		if ((RUNTIME_DIR = getenv("XDG_RUNTIME_DIR")) != NULL) {
+			if ((RUNTIME_DIR = strdup(RUNTIME_DIR)) == NULL) {
+				perror(_("Out of memory"));
+				goto childerr;
+			}
+		} else {
+			if (asprintf(&RUNTIME_DIR, "/run/user/%d", uid) == -1) {
+				perror(_("Out of memory\n"));
+				goto childerr;
+			}
+		}
+
+		/* mount homedir, runuserdir and tmpdir, in this order */
+		if (runuserdir_s &&	seunshare_mount(runuserdir_s, RUNTIME_DIR,
+			&st_runuserdir_s) != 0) goto childerr;
 		if (homedir_s && seunshare_mount(homedir_s, resolved_path,
 			&st_homedir) != 0) goto childerr;
 		if (tmpdir_s &&	seunshare_mount(tmpdir_r, "/tmp",
@@ -799,13 +831,6 @@ int main(int argc, char **argv) {
 			}
 		}
 
-		if ((RUNTIME_DIR = getenv("XDG_RUNTIME_DIR")) != NULL) {
-			if ((RUNTIME_DIR = strdup(RUNTIME_DIR)) == NULL) {
-				perror(_("Out of memory"));
-				goto childerr;
-			}
-		}
-
 		if ((rc = clearenv()) != 0) {
 			perror(_("Failed to clear environment"));
 			goto childerr;
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0
  2022-10-13 13:23 [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 Petr Lautrbach
  2022-10-13 13:23 ` [PATCH 2/2] sandbox: Use temporary directory for XDG_RUNTIME_DIR Petr Lautrbach
@ 2022-11-04 18:38 ` James Carter
  2022-11-09 13:46   ` James Carter
  1 sibling, 1 reply; 4+ messages in thread
From: James Carter @ 2022-11-04 18:38 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Thu, Oct 13, 2022 at 9:24 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> If the user is root, tmpdir is already wiped out.
>
> Fixes:
>     # sandbox -T /root/tmp -- id
>     uid=0(root) gid=0(root) groups=0(root) context=unconfined_u:unconfined_r:sandbox_t:s0:c696,c756
>     Failed to remove directory /tmp/.sandbox-root-KIlB59: No such file or directory
>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>

For these two patches:
Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  sandbox/seunshare.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c
> index 8917a0f9bd42..dd1d7ddbdc89 100644
> --- a/sandbox/seunshare.c
> +++ b/sandbox/seunshare.c
> @@ -414,7 +414,7 @@ static int cleanup_tmpdir(const char *tmpdir, const char *src,
>                 rc++;
>         }
>
> -       if (rmdir(tmpdir) == -1)
> +       if (pwd->pw_uid != 0 && rmdir(tmpdir) == -1)
>                 fprintf(stderr, _("Failed to remove directory %s: %s\n"), tmpdir, strerror(errno));
>         if ((uid_t)setfsuid(pwd->pw_uid) != 0) {
>                 fprintf(stderr, _("unable to switch back to user after clearing tmp dir\n"));
> --
> 2.37.3
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0
  2022-11-04 18:38 ` [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 James Carter
@ 2022-11-09 13:46   ` James Carter
  0 siblings, 0 replies; 4+ messages in thread
From: James Carter @ 2022-11-09 13:46 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Fri, Nov 4, 2022 at 2:38 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Oct 13, 2022 at 9:24 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > If the user is root, tmpdir is already wiped out.
> >
> > Fixes:
> >     # sandbox -T /root/tmp -- id
> >     uid=0(root) gid=0(root) groups=0(root) context=unconfined_u:unconfined_r:sandbox_t:s0:c696,c756
> >     Failed to remove directory /tmp/.sandbox-root-KIlB59: No such file or directory
> >
> > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>
> For these two patches:
> Acked-by: James Carter <jwcart2@gmail.com>
>

These two patches have been merged.
Thanks,
Jim

> > ---
> >  sandbox/seunshare.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c
> > index 8917a0f9bd42..dd1d7ddbdc89 100644
> > --- a/sandbox/seunshare.c
> > +++ b/sandbox/seunshare.c
> > @@ -414,7 +414,7 @@ static int cleanup_tmpdir(const char *tmpdir, const char *src,
> >                 rc++;
> >         }
> >
> > -       if (rmdir(tmpdir) == -1)
> > +       if (pwd->pw_uid != 0 && rmdir(tmpdir) == -1)
> >                 fprintf(stderr, _("Failed to remove directory %s: %s\n"), tmpdir, strerror(errno));
> >         if ((uid_t)setfsuid(pwd->pw_uid) != 0) {
> >                 fprintf(stderr, _("unable to switch back to user after clearing tmp dir\n"));
> > --
> > 2.37.3
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-09 13:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13 13:23 [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 Petr Lautrbach
2022-10-13 13:23 ` [PATCH 2/2] sandbox: Use temporary directory for XDG_RUNTIME_DIR Petr Lautrbach
2022-11-04 18:38 ` [PATCH 1/2] sandbox: Do not try to remove tmpdir twice if uid == 0 James Carter
2022-11-09 13:46   ` James Carter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).