[Bug 1637] Change the context when starting internal-sftp

[bugzilla-daemon at bugzilla.mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=1637



--- Comment #2 from Darren Tucker <dtucker at zip.com.au> 2009-08-28 17:38:39 EST ---
(From update of attachment 1681)
>diff -up openssh-5.2p1/session.c.sesftp openssh-5.2p1/session.c
>--- openssh-5.2p1/session.c.sesftp	2009-01-28 06:29:49.000000000 +0100
>+++ openssh-5.2p1/session.c	2009-08-08 13:13:54.670122454 +0200
>@@ -58,6 +58,7 @@
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>+#include <selinux/selinux.h>
> 
> #include "openbsd-compat/sys-queue.h"
> #include "xmalloc.h"
>@@ -1791,8 +1792,8 @@ do_child(Session *s, const char *command
> 
> 	if (s->is_subsystem == SUBSYSTEM_INT_SFTP) {
> 		extern int optind, optreset;
>-		int i;
>-		char *p, *args;
>+		int i, l;

please don't use "l" as a variable name, my eyeballs easily misparse as
a 1.

>+		char *p, *args, *c1, *c2, *cx;
> 
> 		setproctitle("%s at internal-sftp-server", s->pw->pw_name);
> 		args = xstrdup(command ? command : "sftp-server");
>@@ -1802,6 +1803,27 @@ do_child(Session *s, const char *command
> 		argv[i] = NULL;
> 		optind = optreset = 1;
> 		__progname = argv[0];
>+		if (getcon (&c1) < 0) {

getcon is a linux (in fact, selinux) specific function, so having this
here this will break on every other platform.  Please put this in its
own function in port-linux.c with the rest of the selinux code and wrap
the call in #ifdef WITH_SELINUX

Also, the man page for getcon says the returned context must be freed. 
It also says that it takes a security_context_t not a char * (typedefs
in the headers notwithstanding).

>+			logit("do_child: getcon failed witch %s", strerror (errno));
>+		} else {
>+			c2 = xmalloc (strlen (c1) + 8);

8 is a magic number.  I assume it's sizeof("sftpd_t"), in which case
you should make sftpd_t a #define and use the sizeof.

c2 is never freed.

>+			if (!(cx = index (c1, ':')))
>+				goto badcontext;
>+			if (!(cx = index (cx + 1, ':'))) {
>+badcontext:
>+				logit ("do_child: unparseable context %s", c1);
>+			} else {
>+				l = cx - c1 + 1;
>+				memcpy (c2, c1, l);
>+				strcpy (c2 + l, "sftpd_t");

unbounded str* functions are poor form even if this particular one is
safe.  Please use strl{cat,cpy}.

>+				if ((cx = index (cx + 1, ':')))
>+					strcat (c2, cx);

ditto.

>+				if (setcon (c2) < 0) 
>+					logit("do_child: setcon failed witch %s", strerror (errno));

s/witch/with/

>+			
>+			}
>+		}		
>+			
> 		exit(sftp_server_main(i, argv, s->pw));
> 	}
>

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.