Hi,
The (preliminary) patch below implements privilege separation for the
xenodm (XDM replacement) greeter window.
Instead of running the greeter as root, xenodm will fork a new process
that will revoke its priveges by switch to the _x11 user and
communicate with the parent xdm process with a pipe. The Xsetup script
is also run as _x11. It's also doing a much stricter pledge() before
entering the greeter main loop.
There are still a few rough edges to polish.
Comments, ok on the principle?
Index: greeter/Makefile.am
===================================================================
RCS file: /cvs/xenocara/app/xenodm/greeter/Makefile.am,v
retrieving revision 1.1
diff -u -r1.1 Makefile.am
--- greeter/Makefile.am 23 Oct 2016 08:30:37 -0000 1.1
+++ greeter/Makefile.am 6 Nov 2016 17:37:52 -0000
@@ -6,6 +6,7 @@
Login.c \
Login.h \
LoginP.h \
+ atomicio.c \
greet.c \
verify.c
Index: greeter/Makefile.in
===================================================================
RCS file: /cvs/xenocara/app/xenodm/greeter/Makefile.in,v
retrieving revision 1.1
diff -u -r1.1 Makefile.in
--- greeter/Makefile.in 23 Oct 2016 08:32:59 -0000 1.1
+++ greeter/Makefile.in 6 Nov 2016 17:37:52 -0000
@@ -67,7 +67,7 @@
LTLIBRARIES = $(noinst_LTLIBRARIES)
am__DEPENDENCIES_1 =
libXdmGreet_la_DEPENDENCIES = $(am__DEPENDENCIES_1)
-am_libXdmGreet_la_OBJECTS = Login.lo greet.lo verify.lo
+am_libXdmGreet_la_OBJECTS = Login.lo atomicio.lo greet.lo verify.lo
libXdmGreet_la_OBJECTS = $(am_libXdmGreet_la_OBJECTS)
AM_V_lt = $(am__v_lt_@AM_V@)
am__v_lt_ = $(am__v_lt_@AM_DEFAULT_V@)
@@ -312,6 +312,7 @@
Login.c \
Login.h \
LoginP.h \
+ atomicio.c \
greet.c \
verify.c
@@ -377,6 +378,7 @@
-rm -f *.tab.c
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/Login.Plo@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/atomicio.Plo@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/greet.Plo@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/verify.Plo@am__quote@
Index: greeter/atomicio.c
===================================================================
RCS file: greeter/atomicio.c
diff -N greeter/atomicio.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ greeter/atomicio.c 6 Nov 2016 17:37:52 -0000
@@ -0,0 +1,83 @@
+/* $OpenBSD: atomicio.c,v 1.28 2016/07/27 23:18:12 djm Exp $ */
+/*
+ * Copyright (c) 2006 Damien Miller. All rights reserved.
+ * Copyright (c) 2005 Anil Madhavapeddy. All rights reserved.
+ * Copyright (c) 1995,1999 Theo de Raadt. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/uio.h>
+
+#include <errno.h>
+#include <poll.h>
+#include <string.h>
+#include <unistd.h>
+#include <limits.h>
+
+#include "atomicio.h"
+
+/*
+ * ensure all of data on socket comes through. f==read || f==vwrite
+ */
+size_t
+atomicio6(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n,
+ int (*cb)(void *, size_t), void *cb_arg)
+{
+ char *s = _s;
+ size_t pos = 0;
+ ssize_t res;
+ struct pollfd pfd;
+
+ pfd.fd = fd;
+ pfd.events = f == read ? POLLIN : POLLOUT;
+ while (n > pos) {
+ res = (f) (fd, s + pos, n - pos);
+ switch (res) {
+ case -1:
+ if (errno == EINTR)
+ continue;
+ if (errno == EAGAIN) {
+ (void)poll(&pfd, 1, -1);
+ continue;
+ }
+ return 0;
+ case 0:
+ errno = EPIPE;
+ return pos;
+ default:
+ pos += (size_t)res;
+ if (cb != NULL && cb(cb_arg, (size_t)res) == -1) {
+ errno = EINTR;
+ return pos;
+ }
+ }
+ }
+ return pos;
+}
+
+size_t
+atomicio(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n)
+{
+ return atomicio6(f, fd, _s, n, NULL, NULL);
+}
+
Index: greeter/atomicio.h
===================================================================
RCS file: greeter/atomicio.h
diff -N greeter/atomicio.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ greeter/atomicio.h 6 Nov 2016 17:37:52 -0000
@@ -0,0 +1,42 @@
+/* $OpenBSD: atomicio.h,v 1.11 2010/09/22 22:58:51 djm Exp $ */
+
+/*
+ * Copyright (c) 2006 Damien Miller. All rights reserved.
+ * Copyright (c) 1995,1999 Theo de Raadt. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _ATOMICIO_H
+#define _ATOMICIO_H
+
+/*
+ * Ensure all of data on socket comes through. f==read || f==vwrite
+ */
+size_t
+atomicio6(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n,
+ int (*cb)(void *, size_t), void *);
+size_t atomicio(ssize_t (*)(int, void *, size_t), int, void *, size_t);
+
+#define vwrite (ssize_t (*)(int, void *, size_t))write
+
+#endif /* _ATOMICIO_H */
Index: greeter/greet.c
===================================================================
RCS file: /cvs/xenocara/app/xenodm/greeter/greet.c,v
retrieving revision 1.4
diff -u -r1.4 greet.c
--- greeter/greet.c 6 Nov 2016 13:30:15 -0000 1.4
+++ greeter/greet.c 6 Nov 2016 17:37:52 -0000
@@ -61,6 +61,11 @@
# include "config.h"
#endif
+#include <sys/socket.h>
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
#include <X11/Intrinsic.h>
#include <X11/StringDefs.h>
#include <X11/Shell.h>
@@ -72,6 +77,7 @@
#include "dm_error.h"
#include "greet.h"
#include "LoginP.h"
+#include "atomicio.h"
#include <syslog.h>
#ifndef LOG_AUTHPRIV
@@ -81,7 +87,16 @@
# define LOG_PID 0
#endif
-#include <string.h>
+struct verify_request {
+ char name[1024];
+ char password[1024];
+ Boolean allow_null_passwd;
+ Boolean allow_root_login;
+};
+
+struct verify_response {
+ Boolean ok;
+};
static int done, code;
static char name[NAME_LEN], password[PASSWORD_LEN];
@@ -137,7 +152,7 @@
int i;
static int argc;
Screen *scrn;
- static char *argv[] = { "xlogin", NULL };
+ static char *argv[] = { "xlogin", NULL };
Display *dpy;
XineramaScreenInfo *screens;
int s_num;
@@ -293,77 +308,11 @@
}
_X_EXPORT
-greet_user_rtn GreetUser(
- struct display *d,
- struct verify_info *verify,
- struct greet_info *greet)
-{
- int i;
- Arg arglist[2];
- Display *dpy;
-
- dpy = InitGreet (d);
- /*
- * Run the setup script - note this usually will not work when
- * the server is grabbed, so we don't even bother trying.
- */
- if (!d->grabServer)
- SetupDisplay (d);
- if (!dpy) {
- LogError ("Cannot reopen display %s for greet window\n", d->name);
- exit (RESERVER_DISPLAY);
- }
-
- XtSetArg (arglist[0], XtNallowNullPasswd,
- (char *) &(greet->allow_null_passwd));
- XtSetArg (arglist[1], XtNallowRootLogin,
- (char *) &(greet->allow_root_login));
- XtGetValues (login, arglist, 2);
-
- for (;;) {
- /*
- * Greet user, requesting name/password
- */
- code = Greet (d, greet);
- if (code != 0)
- {
- CloseGreet (d);
- SessionExit (d, code, FALSE);
- }
- /*
- * Verify user
- */
- if (Verify (d, greet, verify))
- break;
- else
- {
- FailedLogin (d, greet->name);
- explicit_bzero (greet->name, strlen(greet->name));
- explicit_bzero (greet->password, strlen(greet->password));
- }
- }
- DeleteXloginResources (d, dpy);
- CloseGreet (d);
- Debug ("Greet loop finished %d\n", getpid());
- /*
- * Run system-wide initialization file
- */
- if (source (verify->systemEnviron, d->startup) != 0)
- {
- Debug ("Startup program %s exited with non-zero status\n",
- d->startup);
- SessionExit (d, OBEYSESS_DISPLAY, FALSE);
- }
- return Greet_Success;
-}
-
-_X_EXPORT
greet_user_rtn AutoLogin(
struct display *d,
struct verify_info *verify,
struct greet_info *greet)
{
- int i;
if (!autoLoginEnv(d, verify, greet)) {
LogError("Autologin %s failed\n", d->autoLogin);
@@ -381,3 +330,149 @@
}
return Greet_Success;
}
+
+/* send verification request to parent and wait for result */
+
+static Boolean
+privVerify(int fd, struct display *d,
+ struct greet_info *greet, struct verify_info *verify)
+{
+ struct verify_request rqst;
+ struct verify_response resp;
+
+ Debug("privVerify enter\n");
+ strlcpy(rqst.name, greet->name, sizeof(rqst.name));
+ strlcpy(rqst.password, greet->password, sizeof(rqst.password));
+ rqst.allow_root_login = greet->allow_root_login;
+ rqst.allow_null_passwd = greet->allow_null_passwd;
+ if (atomicio(vwrite, fd, &rqst, sizeof(rqst)) != sizeof(rqst))
+ LogPanic("send");
+ explicit_bzero(&rqst, sizeof(rqst));
+ if (atomicio(read, fd, &resp, sizeof(resp)) != sizeof(resp))
+ LogPanic("recv");
+ Debug("privVerify done\n");
+ return resp.ok;
+}
+
+/* Unprivileged greeter - exits once authentification succeeded */
+static __dead void
+GreetUserUnpriv(int fd, struct display *d)
+{
+ Arg arglist[2];
+ Display *dpy;
+ struct greet_info greet;
+ struct verify_info verify;
+
+ Debug("XX GreetUserUnpriv\n");
+ dpy = InitGreet(d);
+ if (!d->grabServer)
+ SetupDisplay(d);
+
+ if (dpy == NULL) {
+ LogError("Cannot open display %s for greet window\n",
+ d->name);
+ exit(RESERVER_DISPLAY); /* xxx */
+ }
+
+ if (pledge("stdio", NULL) != 0)
+ LogPanic("pledge failed\n");
+ XtSetArg (arglist[0], XtNallowNullPasswd,
+ (char *) &(greet.allow_null_passwd));
+ XtSetArg (arglist[1], XtNallowRootLogin,
+ (char *) &(greet.allow_root_login));
+ XtGetValues (login, arglist, 2);
+
+ for (;;) {
+ /*
+ * Greet user, requesting name/password
+ */
+ code = Greet (d, &greet);
+ if (code != 0) {
+ CloseGreet (d);
+ SessionExit (d, code, FALSE);
+ }
+ /*
+ * Verify user
+ */
+ if (privVerify (fd, d, &greet, &verify))
+ break;
+ else {
+ FailedLogin (d, greet.name);
+ bzero (greet.name, strlen(greet.name));
+ bzero (greet.password, strlen(greet.password));
+ }
+ }
+ DeleteXloginResources (d, dpy);
+ CloseGreet (d);
+ Debug ("Greet loop finished %d\n", getpid());
+ exit(0);
+}
+
+_X_EXPORT greet_user_rtn
+GreetUser(struct display *d,
+ struct verify_info *verify, struct greet_info *greet)
+{
+ pid_t pid;
+ struct passwd *pw;
+ int socks[2];
+ struct verify_request rqst;
+ struct verify_response resp;
+
+ pw = getpwnam("_x11");
+ if (!pw)
+ LogPanic("no _x11 user\n");
+ if (socketpair(AF_LOCAL, SOCK_STREAM, PF_UNSPEC, socks) == -1)
+ LogPanic("socketpair");
+
+ pid = fork();
+ if (pid < 0)
+ LogPanic("fork failed\n");
+ if (pid == 0) {
+ /* son - revoque privs */
+ if (setgroups(1, &pw->pw_gid) == -1)
+ return Greet_Failure;
+ if (setegid(pw->pw_gid) == -1)
+ return Greet_Failure;
+ if (seteuid(pw->pw_uid) == -1)
+ return Greet_Failure;
+ if (setuid(pw->pw_uid) == -1)
+ return Greet_Failure;
+ close(socks[1]);
+ GreetUserUnpriv(socks[0], d);
+ }
+ /* parent: wait for verify requests */
+ close(socks[0]);
+ while (1) {
+ size_t n;
+
+ n = atomicio(read, socks[1], &rqst, sizeof(rqst));
+ if (n != sizeof(rqst))
+ LogPanic("read socketn");
+ greet->name = &rqst.name[0];
+ greet->password = rqst.password;
+ greet->allow_root_login = rqst.allow_root_login;
+ greet->allow_null_passwd = rqst.allow_null_passwd;
+ resp.ok = Verify(d, greet, verify);
+ explicit_bzero(&rqst.password, sizeof(rqst.password));
+ if (atomicio(vwrite, socks[1], &resp, sizeof(resp)) !=
sizeof(resp))
+ LogPanic("send socket");
+ /* Greeter process will shutdown and we can continue */
+ if (resp.ok) {
+ close(socks[1]);
+ break;
+ }
+ }
+
+ /*
+ * Run system-wide initialization file
+ */
+ if (source (verify->systemEnviron, d->startup) != 0) {
+ Debug ("Startup program %s exited with non-zero status\n",
+ d->startup);
+ SessionExit (d, OBEYSESS_DISPLAY, FALSE);
+ }
+ return Greet_Success;
+}
+
+
+
--
Matthieu Herrb
