[PATCH] Drop fine-grained privileges on Illumos/Solaris

Alex Wilson alex at cooperi.net
Sat Nov 14 07:20:03 AEDT 2015


On 11/12/15 6:24 PM, Darren Tucker wrote:
> 
> As long as someone is willing to do the work and help with tests
> (which it sounds like you are), the support doesn't compromise other
> platforms or make maintenance significantly harder then I have no
> objections to it going in.

Sounds good to me. We're already running with this patch in
(pre-)production, and I'm definitely happy to help out with any
additional testing needed.

> 
> The code itself looks quite reasonable.  Placing it inline in the main
> source files is problematic since it makes maintenance of those files
> harder, but it it should fit nicely in openbsd-compat/port-solaris.c.
> ...
> The other place these look like the'd be useful is in the pre-auth
> privsep sandbox...
> 

Ok, please find attached a revised version. I've moved all of the
pre-auth privsep bit into a new sandbox-solaris.c, and for the ssh-agent
and sftp-server I've created the platform_drop_agent_privs() and
platform_drop_sftp_server_privs() hooks which, if USE_SOLARIS_PRIVS is
enabled then call out to the code that's now in
openbsd-compat/port-solaris.c

Does this look a bit better? The biggest annoyance I had is that now
ssh-agent and sftp-server have to link against platform.o, and the
easiest way to organise that seemed to be to add it to libssh.a. So now
all the cmdline tools also link against libcontract and libproject,
instead of just the daemon.

Using a platform_* function seems like a nicer interface than just
calling a port-solaris function inside an #ifdef in each of them, though
-- you can just add some code now in platform.c that uses pledge()
instead, for example. So maybe it's fine to have a bit of extra link bloat.
-------------- next part --------------
From 51dbb5957b7d3c2d7c7ae2226ce112bd596e8984 Mon Sep 17 00:00:00 2001
From: Alex Wilson <alex.wilson at joyent.com>
Date: Tue, 4 Aug 2015 15:50:09 -0700
Subject: [PATCH] Support for fine-grained Illumos/Solaris privileges

---
 Makefile.in                   |   6 ++-
 configure.ac                  |  26 ++++++++++-
 openbsd-compat/port-solaris.c |  62 +++++++++++++++++++++++++
 openbsd-compat/port-solaris.h |   3 ++
 platform.c                    |  24 ++++++++++
 platform.h                    |   2 +
 sandbox-solaris.c             | 103 ++++++++++++++++++++++++++++++++++++++++++
 sftp-server.c                 |   3 ++
 ssh-agent.c                   |   3 ++
 uidswap.c                     |  18 +++++---
 10 files changed, 240 insertions(+), 10 deletions(-)
 create mode 100644 sandbox-solaris.c

diff --git a/Makefile.in b/Makefile.in
index 15cf69e..bc8c522 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -91,7 +91,8 @@ LIBSSH_OBJS=${LIBOPENSSH_OBJS} \
 	sc25519.o ge25519.o fe25519.o ed25519.o verify.o hash.o blocks.o \
 	kex.o kexdh.o kexgex.o kexecdh.o kexc25519.o \
 	kexdhc.o kexgexc.o kexecdhc.o kexc25519c.o \
-	kexdhs.o kexgexs.o kexecdhs.o kexc25519s.o
+	kexdhs.o kexgexs.o kexecdhs.o kexc25519s.o \
+	platform.o
 
 SSHOBJS= ssh.o readconf.o clientloop.o sshtty.o \
 	sshconnect.o sshconnect1.o sshconnect2.o mux.o \
@@ -110,7 +111,8 @@ SSHDOBJS=sshd.o auth-rhosts.o auth-passwd.o auth-rsa.o auth-rh-rsa.o \
 	sftp-server.o sftp-common.o \
 	roaming_common.o roaming_serv.o \
 	sandbox-null.o sandbox-rlimit.o sandbox-systrace.o sandbox-darwin.o \
-	sandbox-seccomp-filter.o sandbox-capsicum.o sandbox-pledge.o
+	sandbox-seccomp-filter.o sandbox-capsicum.o sandbox-pledge.o \
+	sandbox-solaris.o
 
 MANPAGES	= moduli.5.out scp.1.out ssh-add.1.out ssh-agent.1.out ssh-keygen.1.out ssh-keyscan.1.out ssh.1.out sshd.8.out sftp-server.8.out sftp.1.out ssh-keysign.8.out ssh-pkcs11-helper.8.out sshd_config.5.out ssh_config.5.out
 MANPAGES_IN	= moduli.5 scp.1 ssh-add.1 ssh-agent.1 ssh-keygen.1 ssh-keyscan.1 ssh.1 sshd.8 sftp-server.8 sftp.1 ssh-keysign.8 ssh-pkcs11-helper.8 sshd_config.5 ssh_config.5
diff --git a/configure.ac b/configure.ac
index 1527a13..10b6da8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -469,6 +469,7 @@ AC_CHECK_HEADERS([sys/un.h], [], [], [
 SIA_MSG="no"
 SPC_MSG="no"
 SP_MSG="no"
+SPP_MSG="no"
 
 # Check for some target-specific stuff
 case "$host" in
@@ -575,6 +576,8 @@ case "$host" in
 	LIBS="$LIBS /usr/lib/textreadmode.o"
 	AC_DEFINE([HAVE_CYGWIN], [1], [Define if you are on Cygwin])
 	AC_DEFINE([USE_PIPES], [1], [Use PIPES instead of a socketpair()])
+	AC_DEFINE([NO_UID_RESTORATION_TEST], [1],
+		[Define to disable UID restoration test])
 	AC_DEFINE([DISABLE_SHADOW], [1],
 		[Define if you want to disable shadow passwords])
 	AC_DEFINE([NO_X11_UNIX_SOCKETS], [1],
@@ -896,7 +899,7 @@ mips-sony-bsd|mips-sony-newsos4)
 		AC_CHECK_LIB([contract], [ct_tmpl_activate],
 			[ AC_DEFINE([USE_SOLARIS_PROCESS_CONTRACTS], [1],
 				[Define if you have Solaris process contracts])
-			  SSHDLIBS="$SSHDLIBS -lcontract"
+			  LIBS="$LIBS -lcontract"
 			  SPC_MSG="yes" ], )
 		],
 	)
@@ -906,10 +909,22 @@ mips-sony-bsd|mips-sony-newsos4)
 		AC_CHECK_LIB([project], [setproject],
 			[ AC_DEFINE([USE_SOLARIS_PROJECTS], [1],
 				[Define if you have Solaris projects])
-			SSHDLIBS="$SSHDLIBS -lproject"
+			LIBS="$LIBS -lproject"
 			SP_MSG="yes" ], )
 		],
 	)
+	AC_ARG_WITH([solaris-privs],
+		[  --with-solaris-privs    Enable Solaris/Illumos privileges (experimental)],
+		[
+		AC_CHECK_FUNC([setppriv],
+			[ AC_CHECK_HEADERS([priv.h])
+			  AC_DEFINE([NO_UID_RESTORATION_TEST], [1],
+				[Define to disable UID restoration test])
+			  AC_DEFINE([USE_SOLARIS_PRIVS], [1],
+				[Define if you have Solaris privileges])
+			SPP_MSG="yes" ], )
+		],
+	)
 	TEST_SHELL=$SHELL	# let configure find us a capable shell
 	;;
 *-*-sunos4*)
@@ -3155,6 +3170,12 @@ elif test "x$sandbox_arg" = "xrlimit" || \
 		AC_MSG_ERROR([rlimit sandbox requires select to work with rlimit])
 	SANDBOX_STYLE="rlimit"
 	AC_DEFINE([SANDBOX_RLIMIT], [1], [Sandbox using setrlimit(2)])
+elif test "x$sandbox_arg" = "xsolaris" || \
+   ( test -z "$sandbox_arg" && test "x$ac_cv_func_setppriv" = "xyes" ) ; then
+	test "x$ac_cv_func_setppriv" != "xyes" && \
+		AC_MSG_ERROR([solaris sandbox requires setppriv(2) support])
+	SANDBOX_STYLE="solaris"
+	AC_DEFINE([SANDBOX_SOLARIS], [1], [Sandbox using Solaris/Illumos privileges])
 elif test -z "$sandbox_arg" || test "x$sandbox_arg" = "xno" || \
      test "x$sandbox_arg" = "xnone" || test "x$sandbox_arg" = "xnull" ; then
 	SANDBOX_STYLE="none"
@@ -4944,6 +4965,7 @@ echo "              MD5 password support: $MD5_MSG"
 echo "                   libedit support: $LIBEDIT_MSG"
 echo "  Solaris process contract support: $SPC_MSG"
 echo "           Solaris project support: $SP_MSG"
+echo " Solaris/Illumos privilege support: $SPP_MSG"
 echo "       IP address in \$DISPLAY hack: $DISPLAY_HACK_MSG"
 echo "           Translate v4 in v6 hack: $IPV4_IN6_HACK_MSG"
 echo "                  BSD Auth support: $BSD_AUTH_MSG"
diff --git a/openbsd-compat/port-solaris.c b/openbsd-compat/port-solaris.c
index 25382f1..50e6fdc 100644
--- a/openbsd-compat/port-solaris.c
+++ b/openbsd-compat/port-solaris.c
@@ -227,3 +227,65 @@ solaris_set_default_project(struct passwd *pw)
 	}
 }
 #endif /* USE_SOLARIS_PROJECTS */
+
+#ifdef USE_SOLARIS_PRIVS
+# ifdef HAVE_PRIV_H
+#  include <priv.h>
+# endif
+
+void
+solaris_drop_fork_privs(void)
+{
+	priv_set_t *pset = NULL;
+
+	if ((pset = priv_allocset()) == NULL)
+		fatal("priv_allocset: %s", strerror(errno));
+
+	/* Start with "basic" and drop everything we don't need. */
+	priv_basicset(pset);
+
+	priv_delset(pset, PRIV_PROC_EXEC);
+	priv_delset(pset, PRIV_PROC_FORK);
+	priv_delset(pset, PRIV_FILE_LINK_ANY);
+	priv_delset(pset, PRIV_PROC_INFO);
+	priv_delset(pset, PRIV_PROC_SESSION);
+
+	if (setppriv(PRIV_SET, PRIV_PERMITTED, pset))
+		fatal("setppriv: %s", strerror(errno));
+	if (setppriv(PRIV_SET, PRIV_LIMIT, pset))
+		fatal("setppriv: %s", strerror(errno));
+	if (setppriv(PRIV_SET, PRIV_INHERITABLE, pset))
+		fatal("setppriv: %s", strerror(errno));
+
+	priv_freeset(pset);
+}
+
+void
+solaris_drop_fork_net_privs(void)
+{
+	priv_set_t *pset = NULL;
+
+	if ((pset = priv_allocset()) == NULL)
+		fatal("priv_allocset: %s", strerror(errno));
+
+	/* Start with "basic" and drop everything we don't need. */
+	priv_basicset(pset);
+
+	priv_delset(pset, PRIV_FILE_LINK_ANY);
+	priv_delset(pset, PRIV_PROC_INFO);
+	priv_delset(pset, PRIV_PROC_SESSION);
+	priv_delset(pset, PRIV_PROC_FORK);
+	priv_delset(pset, PRIV_NET_ACCESS);
+	priv_delset(pset, PRIV_PROC_EXEC);
+
+	if (setppriv(PRIV_SET, PRIV_PERMITTED, pset))
+		fatal("setppriv: %s", strerror(errno));
+	if (setppriv(PRIV_SET, PRIV_LIMIT, pset))
+		fatal("setppriv: %s", strerror(errno));
+	if (setppriv(PRIV_SET, PRIV_INHERITABLE, pset))
+		fatal("setppriv: %s", strerror(errno));
+
+	priv_freeset(pset);
+}
+
+#endif
diff --git a/openbsd-compat/port-solaris.h b/openbsd-compat/port-solaris.h
index cd442e7..686bce2 100644
--- a/openbsd-compat/port-solaris.h
+++ b/openbsd-compat/port-solaris.h
@@ -26,5 +26,8 @@ void solaris_contract_pre_fork(void);
 void solaris_contract_post_fork_child(void);
 void solaris_contract_post_fork_parent(pid_t pid);
 void solaris_set_default_project(struct passwd *);
+void solaris_drop_fork_privs(void);
+void solaris_drop_fork_net_privs(void);
+void solaris_drop_all_privs(void);
 
 #endif
diff --git a/platform.c b/platform.c
index ee313da..703d47e 100644
--- a/platform.c
+++ b/platform.c
@@ -213,3 +213,27 @@ platform_sys_dir_uid(uid_t uid)
 #endif
 	return 0;
 }
+
+/*
+ * Drop any fine-grained privileges that are not needed for post-startup
+ * operation of ssh-agent
+ */
+void
+platform_drop_agent_privs(void)
+{
+#ifdef USE_SOLARIS_PRIVS
+	solaris_drop_fork_privs();
+#endif
+}
+
+/*
+ * Drop any fine-grained privileges that are not needed for post-startup
+ * operation of sftp-server
+ */
+void
+platform_drop_sftp_server_privs(void)
+{
+#ifdef USE_SOLARIS_PRIVS
+	solaris_drop_fork_net_privs();
+#endif
+}
diff --git a/platform.h b/platform.h
index 1c7a45d..eac9b9d 100644
--- a/platform.h
+++ b/platform.h
@@ -31,3 +31,5 @@ void platform_setusercontext_post_groups(struct passwd *);
 char *platform_get_krb5_client(const char *);
 char *platform_krb5_get_principal_name(const char *);
 int platform_sys_dir_uid(uid_t);
+void platform_drop_agent_privs(void);
+void platform_drop_sftp_server_privs(void);
diff --git a/sandbox-solaris.c b/sandbox-solaris.c
new file mode 100644
index 0000000..3369531
--- /dev/null
+++ b/sandbox-solaris.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2015 Joyent, Inc
+ * Author: Alex Wilson <alex.wilson at joyent.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "includes.h"
+
+#ifdef SANDBOX_SOLARIS
+#ifndef USE_SOLARIS_PRIVS
+# error "--with-solaris-privs must be used with the Solaris sandbox"
+#endif
+
+#include <sys/types.h>
+
+#include <errno.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#ifdef HAVE_PRIV_H
+# include <priv.h>
+#endif
+
+#include "log.h"
+#include "ssh-sandbox.h"
+#include "xmalloc.h"
+
+struct ssh_sandbox {
+	priv_set_t *pset;
+};
+
+struct ssh_sandbox *
+ssh_sandbox_init(struct monitor *monitor)
+{
+	struct ssh_sandbox *box = NULL;
+
+	box = xcalloc(1, sizeof(*box));
+	box->pset = priv_allocset();
+
+	if (box->pset == NULL) {
+		free(box);
+		return NULL;
+	}
+
+	/* Start with "basic" and drop everything we don't need. */
+	priv_basicset(box->pset);
+
+	/* Drop everything except the ability to use already-opened files */
+	priv_delset(box->pset, PRIV_FILE_LINK_ANY);
+	priv_delset(box->pset, PRIV_PROC_INFO);
+	priv_delset(box->pset, PRIV_PROC_SESSION);
+	priv_delset(box->pset, PRIV_PROC_FORK);
+	priv_delset(box->pset, PRIV_NET_ACCESS);
+	priv_delset(box->pset, PRIV_PROC_EXEC);
+
+	/* These may not be available on older Solaris-es */
+# if defined(PRIV_FILE_READ) && defined(PRIV_FILE_WRITE)
+	priv_delset(box->pset, PRIV_FILE_READ);
+	priv_delset(box->pset, PRIV_FILE_WRITE);
+# endif
+
+	return box;
+}
+
+void
+ssh_sandbox_child(struct ssh_sandbox *box)
+{
+	if (setppriv(PRIV_SET, PRIV_PERMITTED, box->pset))
+		fatal("setppriv: %s", strerror(errno));
+	if (setppriv(PRIV_SET, PRIV_LIMIT, box->pset))
+		fatal("setppriv: %s", strerror(errno));
+	if (setppriv(PRIV_SET, PRIV_INHERITABLE, box->pset))
+		fatal("setppriv: %s", strerror(errno));
+}
+
+void
+ssh_sandbox_parent_finish(struct ssh_sandbox *box)
+{
+	priv_freeset(box->pset);
+	box->pset = NULL;
+	free(box);
+}
+
+void
+ssh_sandbox_parent_preauth(struct ssh_sandbox *box, pid_t child_pid)
+{
+	/* Nothing to do here */
+}
+
+#endif /* SANDBOX_SOLARIS */
diff --git a/sftp-server.c b/sftp-server.c
index eac11d7..2a11d3f 100644
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -1598,6 +1598,9 @@ sftp_server_main(int argc, char **argv, struct passwd *user_pw)
 		fatal("unable to make the process undumpable");
 #endif /* defined(HAVE_PRCTL) && defined(PR_SET_DUMPABLE) */
 
+	/* Drop any fine-grained privileges we don't need */
+	platform_drop_sftp_server_privs();
+
 	if ((cp = getenv("SSH_CONNECTION")) != NULL) {
 		client_addr = xstrdup(cp);
 		if ((cp = strchr(client_addr, ' ')) == NULL) {
diff --git a/ssh-agent.c b/ssh-agent.c
index a335ea3..7971a57 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -1361,6 +1361,9 @@ main(int ac, char **av)
 	/* child */
 	log_init(__progname, SYSLOG_LEVEL_INFO, SYSLOG_FACILITY_AUTH, 0);
 
+	/* Drop any fine-grained privileges we don't need */
+	platform_drop_agent_privs();
+
 	if (setsid() == -1) {
 		error("setsid: %s", strerror(errno));
 		cleanup_exit(1);
diff --git a/uidswap.c b/uidswap.c
index 0702e1d..8bf6b24 100644
--- a/uidswap.c
+++ b/uidswap.c
@@ -134,7 +134,7 @@ temporarily_use_uid(struct passwd *pw)
 void
 permanently_drop_suid(uid_t uid)
 {
-#ifndef HAVE_CYGWIN
+#ifndef NO_UID_RESTORATION_TEST
 	uid_t old_uid = getuid();
 #endif
 
@@ -142,8 +142,14 @@ permanently_drop_suid(uid_t uid)
 	if (setresuid(uid, uid, uid) < 0)
 		fatal("setresuid %u: %.100s", (u_int)uid, strerror(errno));
 
-#ifndef HAVE_CYGWIN
-	/* Try restoration of UID if changed (test clearing of saved uid) */
+#ifndef NO_UID_RESTORATION_TEST
+	/*
+	 * Try restoration of UID if changed (test clearing of saved uid).
+	 *
+	 * Note that we don't do this on Cygwin, or on Solaris-based platforms
+	 * where fine-grained privileges are available (the user might be
+	 * deliberately allowed the right to setuid back to root).
+	 */
 	if (old_uid != uid &&
 	    (setuid(old_uid) != -1 || seteuid(old_uid) != -1))
 		fatal("%s: was able to restore old [e]uid", __func__);
@@ -199,7 +205,7 @@ restore_uid(void)
 void
 permanently_set_uid(struct passwd *pw)
 {
-#ifndef HAVE_CYGWIN
+#ifndef NO_UID_RESTORATION_TEST
 	uid_t old_uid = getuid();
 	gid_t old_gid = getgid();
 #endif
@@ -227,7 +233,7 @@ permanently_set_uid(struct passwd *pw)
 	if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) < 0)
 		fatal("setresuid %u: %.100s", (u_int)pw->pw_uid, strerror(errno));
 
-#ifndef HAVE_CYGWIN
+#ifndef NO_UID_RESTORATION_TEST
 	/* Try restoration of GID if changed (test clearing of saved gid) */
 	if (old_gid != pw->pw_gid && pw->pw_uid != 0 &&
 	    (setgid(old_gid) != -1 || setegid(old_gid) != -1))
@@ -241,7 +247,7 @@ permanently_set_uid(struct passwd *pw)
 		    (u_int)pw->pw_gid);
 	}
 
-#ifndef HAVE_CYGWIN
+#ifndef NO_UID_RESTORATION_TEST
 	/* Try restoration of UID if changed (test clearing of saved uid) */
 	if (old_uid != pw->pw_uid &&
 	    (setuid(old_uid) != -1 || seteuid(old_uid) != -1))
-- 
2.4.9 (Apple Git-60)



More information about the openssh-unix-dev mailing list