[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: DSA 557-1 and CAN-2004-0564



On Mon, Oct 04, 2004 at 12:14:56PM -0400, Christian Hudon wrote:
> Max Vozeler wrote:
> 
> >The pppd in Debian appears to change privileges back to those of the
> >invoking user before calling the program specified in the pty option,
> >preventing normal users from controlling PPPOE connections like other
> >normal PPP connections.
> 
> If this is really the case, then maybe the best solution would be to
> modify that Debian patch to pppd to add a "dont-drop-root-privs"
> option to the "pty" option? Then people using pppd together with pppoe
> could use that option, pppoe would be run as root, and people in group
> dip would be able to launch PPP connections.

How about something like the attached?

It introduces a new option "pty-keep-privs" which tells pppd to keep
euid=0 when calling the pty script. The option is privileged so that
only root can change it. Like other privileged options it can also be
specified in the /etc/ppp/peers/ configs.

  pty "/usr/sbin/pppoe ..."
  pty-keep-privs

It would make it possible for /usr/sbin/pppoe to get rid of setuid root
and still work for unprivileged users. Marco, how does this look to you?
Would you consider including such an option in ppp?

Cheers,
Max

-- 
308E81E7B97963BCA0E6ED889D5BD511B7CDA2DC
--- build-tree/ppp/pppd/main.c	2004-10-05 14:01:33.000000000 +0200
+++ ppp/pppd/main.c	2004-10-05 14:02:10.000000000 +0200
@@ -1512,10 +1512,11 @@
  * stderr gets connected to the log fd or to the _PATH_CONNERRS file.
  */
 int
-device_script(program, in, out, dont_wait)
+device_script(program, in, out, dont_wait, keep_privs)
     char *program;
     int in, out;
     int dont_wait;
+    int keep_privs;
 {
     int pid;
     int status = -1;
@@ -1591,12 +1592,15 @@
 	close(errfd);
     }
 
-    setuid(uid);
-    if (getuid() != uid) {
-	error("setuid failed");
-	exit(1);
+    if (keep_privs != 1) {
+        setuid(uid);
+        if (getuid() != uid) {
+            error("setuid failed");
+            exit(1);
+        }
+        setgid(getgid());
     }
-    setgid(getgid());
+
     execl("/bin/sh", "sh", "-c", program, (char *)0);
     error("could not exec /bin/sh: %m");
     exit(99);
--- build-tree/ppp/pppd/pppd.8	2004-10-05 14:01:34.000000000 +0200
+++ ppp/pppd/pppd.8	2004-10-05 14:02:31.000000000 +0200
@@ -904,6 +904,11 @@
 \fIrecord\fR option is used in conjuction with the \fIpty\fR option,
 the child process will have pipes on its standard input and output.)
 .TP
+.B pty-keep-privs
+Specifies that pppd should keep root privileges when calling the
+script given in the \fIpty\fR option. This option is privileged and
+may only be specified by root or in a system-wide configuration file.
+.TP
 .B receive-all
 With this option, pppd will accept all control characters from the
 peer, including those marked in the receive asyncmap.  Without this
--- build-tree/ppp/pppd/pppd.h	2004-10-05 14:01:34.000000000 +0200
+++ ppp/pppd/pppd.h	2004-10-05 14:02:10.000000000 +0200
@@ -469,7 +469,8 @@
 				/* Cancel call to func(arg) */
 void record_child __P((int, char *, void (*) (void *), void *));
 pid_t safe_fork __P((void));	/* Fork & close stuff in child */
-int  device_script __P((char *cmd, int in, int out, int dont_wait));
+int  device_script __P((char *cmd, int in, int out, int dont_wait,
+                        int keep_privs));
 				/* Run `cmd' with given stdin and stdout */
 pid_t run_program __P((char *prog, char **args, int must_exist,
 		       void (*done)(void *), void *arg));
--- build-tree/ppp/pppd/tty.c	2004-01-17 06:50:12.000000000 +0100
+++ ppp/pppd/tty.c	2004-10-05 14:02:10.000000000 +0200
@@ -148,6 +148,7 @@
 char	*disconnect_script = NULL; /* Script to disestablish physical link */
 char	*welcomer = NULL;	/* Script to run after phys link estab. */
 char	*ptycommand = NULL;	/* Command to run on other side of pty */
+bool	pty_privileged = 0;	/* Keep euid=0 when calling pty program */
 bool	notty = 0;		/* Stdin/out is not a tty */
 char	*record_file = NULL;	/* File to record chars sent/received */
 int	max_data_rate;		/* max bytes/sec through charshunt */
@@ -196,6 +197,10 @@
       "Script to run on pseudo-tty master side",
       OPT_PRIO | OPT_PRIVFIX | OPT_DEVNAM },
 
+    { "pty-keep-privs", o_bool, &pty_privileged,
+      "Keep root privileges when calling the pty program",
+      OPT_PRIO | OPT_PRIV | 1 },
+
     { "notty", o_bool, &notty,
       "Input/output is not a tty", OPT_DEVNAM | 1 },
 
@@ -627,7 +632,7 @@
 			(void) fcntl(ipipe[0], F_SETFD, FD_CLOEXEC);
 			(void) fcntl(opipe[1], F_SETFD, FD_CLOEXEC);
 
-			ok = device_script(ptycommand, opipe[0], ipipe[1], 1) == 0
+			ok = device_script(ptycommand, opipe[0], ipipe[1], 1, pty_privileged) == 0
 				&& start_charshunt(ipipe[0], opipe[1]);
 			close(ipipe[0]);
 			close(ipipe[1]);
@@ -636,7 +641,7 @@
 			if (!ok)
 				return -1;
 		} else {
-			if (device_script(ptycommand, pty_master, pty_master, 1) < 0)
+			if (device_script(ptycommand, pty_master, pty_master, 1, pty_privileged) < 0)
 				return -1;
 			ttyfd = pty_slave;
 			close(pty_master);
@@ -668,7 +673,7 @@
 		}
 
 		if (initializer && initializer[0]) {
-			if (device_script(initializer, ttyfd, ttyfd, 0) < 0) {
+			if (device_script(initializer, ttyfd, ttyfd, 0, 0) < 0) {
 				error("Initializer script failed");
 				status = EXIT_INIT_FAILED;
 				return -1;
@@ -681,7 +686,7 @@
 		}
 
 		if (connector && connector[0]) {
-			if (device_script(connector, ttyfd, ttyfd, 0) < 0) {
+			if (device_script(connector, ttyfd, ttyfd, 0, 0) < 0) {
 				error("Connect script failed");
 				status = EXIT_CONNECT_FAILED;
 				return -1;
@@ -723,7 +728,7 @@
 
 	/* run welcome script, if any */
 	if (welcomer && welcomer[0]) {
-		if (device_script(welcomer, ttyfd, ttyfd, 0) < 0)
+		if (device_script(welcomer, ttyfd, ttyfd, 0, 0) < 0)
 			warn("Welcome script failed");
 	}
 
@@ -745,7 +750,7 @@
 		return;
 	if (real_ttyfd >= 0)
 		set_up_tty(real_ttyfd, 1);
-	if (device_script(disconnect_script, ttyfd, ttyfd, 0) < 0) {
+	if (device_script(disconnect_script, ttyfd, ttyfd, 0, 0) < 0) {
 		warn("disconnect script failed");
 	} else {
 		info("Serial link disconnected.");

Reply to: