On Tue, Aug 01, 2006 at 10:38:35AM -0400, Chad Mynhier wrote:
>
> I'm including a non-Sun-sshd-derived patch for this problem below.
>
> To restate the problem, sshd doesn't put child processes into separate
> process contracts, so every process for which sshd was ever an
> ancestor (including processes that have otherwise been daemonized)
> are in the same process contract. This means that if sshd was started
> via the service management facility (SMF), any normal or abnormal
> termination of sshd via SMF will cause all of the child sshd processes
> (and any process that was ever started from within an ssh session,
> regardless of whether that ssh session still exists) to be terminated.
>
> The output below shows the wrong behavior, i.e., that the child
> processes are in process contract 562 with the parent sshd process:


In general this looks pretty good.

There were a few things:

* in general we prefer not to add platform-specific code to the files
shared with OpenBSD, and sshd.c is one such file. Because of this,
I moved the process contract code to a platform-specific file in the
compat library.

* process IDs should be pid_t's not ints.

* snprintf's return can be a negative value if it fails entirely and doesn't
include the null terminator.

* testing for the presence of the facilities used is preferable to using
the OS version.

* because it's only used for sshd, I added "SSHDLIBS" specifically for
sshd libs.

* the code could be simplified by removing the goto's, eg instead of:

if ((tmpl_fd = open64(...) == -1) {
error(...);
goto cleanup;
}
if (ct_pr_tmpl_set_fatal(...) != 0) {
error(...);
goto cleanup;
} [...]
return;
cleanup:
if (tmpl_fd > 0)
close(tmpl_fd);

you could have:

if ((tmpl_fd = open64(...) == -1)
error(...);
else if (ct_pr_tmpl_set_fatal(...) != 0)
error(...);
else
return; /* success */

/* failed */
if (tmpl_fd > 0)
close(tmpl_fd);

(the patch below does not have this, though.)

* I also made the hooks in sshd general ones that can be reused (this
is something I mentioned on this list a while back and has been planned
for a while). It's not specific to your patch, but it seemed like a
good time to start using it.

* Various minor style changes to conform with the preferred style.

Since you wrote it, the new file's copyright belongs to you and you get to
pick the license. We prefer either ISC-style (this is what is in the patch
below) or 2-term BSD. Please let me know if either is acceptable to you.

Updated patch follows.


Index: CREDITS
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/CREDITS,v
retrieving revision 1.80
diff -u -p -r1.80 CREDITS
--- CREDITS 26 Aug 2005 20:15:20 -0000 1.80
+++ CREDITS 2 Aug 2006 13:29:14 -0000
@@ -25,6 +25,7 @@ Chris, the Young One - P
Christos Zoulas - Autoconf fixes
Chun-Chung Chen - RPM fixes
Corinna Vinschen - Cygwin support
+Chad Mynhier - Solaris Process Contract support
Dan Brosemer - Autoconf support, build fixes
Darren Hall - AIX patches
Darren Tucker - AIX BFF package scripts
Index: Makefile.in
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/Makefile.in,v
retrieving revision 1.279
diff -u -p -r1.279 Makefile.in
--- Makefile.in 24 Jul 2006 05:30:19 -0000 1.279
+++ Makefile.in 2 Aug 2006 12:09:33 -0000
@@ -44,6 +44,7 @@ CFLAGS=@CFLAGS@
CPPFLAGS=-I. -I$(srcdir) @CPPFLAGS@ $(PATHS) @DEFS@
LIBS=@LIBS@
LIBSELINUX=@LIBSELINUX@
+SSHDLIBS=@SSHDLIBS@
LIBEDIT=@LIBEDIT@
LIBPAM=@LIBPAM@
LIBWRAP=@LIBWRAP@
@@ -87,7 +88,7 @@ SSHDOBJS=sshd.o auth-rhosts.o auth-passw
auth-krb5.o \
auth2-gss.o gss-serv.o gss-serv-krb5.o \
loginrec.o auth-pam.o auth-shadow.o auth-sia.o md5crypt.o \
- audit.o audit-bsm.o
+ audit.o audit-bsm.o platform.o

MANPAGES = 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-rand-helper.8.out ssh-keysign.8.out sshd_config.5.out ssh_config.5.out
MANPAGES_IN = scp.1 ssh-add.1 ssh-agent.1 ssh-keygen.1 ssh-keyscan.1 ssh.1 sshd.8 sftp-server.8 sftp.1 ssh-rand-helper.8 ssh-keysign.8 sshd_config.5 ssh_config.5
@@ -137,7 +138,7 @@ ssh$(EXEEXT): $(LIBCOMPAT) libssh.a $(SS
$(LD) -o $@ $(SSHOBJS) $(LDFLAGS) -lssh -lopenbsd-compat $(LIBS)

sshd$(EXEEXT): libssh.a $(LIBCOMPAT) $(SSHDOBJS)
- $(LD) -o $@ $(SSHDOBJS) $(LDFLAGS) -lssh -lopenbsd-compat $(LIBWRAP) $(LIBPAM) $(LIBSELINUX) $(LIBS)
+ $(LD) -o $@ $(SSHDOBJS) $(LDFLAGS) -lssh -lopenbsd-compat $(LIBWRAP) $(LIBPAM) $(LIBSELINUX) $(SSHDLIBS) $(LIBS)

scp$(EXEEXT): $(LIBCOMPAT) libssh.a scp.o progressmeter.o
$(LD) -o $@ scp.o progressmeter.o bufaux.o $(LDFLAGS) -lssh -lopenbsd-compat $(LIBS)
Index: configure.ac
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/configure.ac,v
retrieving revision 1.347
diff -u -p -r1.347 configure.ac
--- configure.ac 12 Jul 2006 09:02:57 -0000 1.347
+++ configure.ac 2 Aug 2006 12:51:06 -0000
@@ -430,6 +430,15 @@ mips-sony-bsd|mips-sony-newsos4)
else
AC_MSG_RESULT(no)
fi
+ AC_MSG_CHECKING(for process contracts)
+ AC_CHECK_LIB(contract, ct_tmpl_activate,
+ [ AC_MSG_RESULT(yes)
+ AC_DEFINE(USE_SOLARIS_PROCESS_CONTRACTS, 1,
+ [Define if you have Solaris process contracts])
+ SSHDLIBS="$SSHDLIBS -lcontract"
+ AC_SUBST(SSHDLIBS) ],
+ [ AC_MSG_RESULT(no) ]
+ )
;;
*-*-sunos4*)
CPPFLAGS="$CPPFLAGS -DSUNOS4"
Index: includes.h
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/includes.h,v
retrieving revision 1.118
diff -u -p -r1.118 includes.h
--- includes.h 24 Jul 2006 04:13:34 -0000 1.118
+++ includes.h 2 Aug 2006 12:34:13 -0000
@@ -172,6 +172,7 @@

#include "defines.h"

+#include "platform.h"
#include "openbsd-compat/openbsd-compat.h"
#include "openbsd-compat/bsd-nextstep.h"

Index: platform.c
================================================== =================
RCS file: platform.c
diff -N platform.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ platform.c 2 Aug 2006 13:30:14 -0000
@@ -0,0 +1,43 @@
+/* $Id$ */
+
+/*
+ * Copyright (c) 2006 Darren Tucker. All rights reserved.
+ *
+ * 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 "platform.h"
+
+void
+platform_pre_fork(void)
+{
+#ifdef USE_SOLARIS_PROCESS_CONTRACTS
+ solaris_contract_pre_fork();
+#endif
+}
+
+void
+platform_post_fork_parent(pid_t child_pid)
+{
+#ifdef USE_SOLARIS_PROCESS_CONTRACTS
+ solaris_contract_post_fork(child_pid);
+#endif
+}
+
+void
+platform_post_fork_child(void)
+{
+#ifdef USE_SOLARIS_PROCESS_CONTRACTS
+ solaris_contract_post_fork(0);
+#endif
+}
Index: platform.h
================================================== =================
RCS file: platform.h
diff -N platform.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ platform.h 2 Aug 2006 13:30:23 -0000
@@ -0,0 +1,23 @@
+/* $Id$ */
+
+/*
+ * Copyright (c) 2006 Darren Tucker. All rights reserved.
+ *
+ * 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
+
+void platform_pre_fork(void);
+void platform_post_fork_parent(pid_t child_pid);
+void platform_post_fork_child(void);
Index: sshd.c
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/sshd.c,v
retrieving revision 1.345
diff -u -p -r1.345 sshd.c
--- sshd.c 24 Jul 2006 04:51:01 -0000 1.345
+++ sshd.c 2 Aug 2006 13:30:35 -0000
@@ -1527,6 +1527,8 @@ main(int ac, char **av)
* the child process the connection. The
* parent continues listening.
*/
+ platform_pre_fork();
+
if ((pid = fork()) == 0) {
/*
* Child. Close the listening and
@@ -1536,6 +1538,8 @@ main(int ac, char **av)
* We break out of the loop to handle
* the connection.
*/
+ platform_post_fork_child();
+
startup_pipe = startup_p[1];
close_startup_pipes();
close_listen_socks();
@@ -1552,6 +1556,7 @@ main(int ac, char **av)
}

/* Parent. Stay in the loop. */
+ platform_post_fork_parent(pid);
if (pid < 0)
error("fork: %.100s", strerror(errno));
else
Index: openbsd-compat/Makefile.in
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/Makefile.in,v
retrieving revision 1.39
diff -u -p -r1.39 Makefile.in
--- openbsd-compat/Makefile.in 22 Apr 2006 11:26:08 -0000 1.39
+++ openbsd-compat/Makefile.in 2 Aug 2006 11:31:18 -0000
@@ -20,7 +20,7 @@ OPENBSD=base64.o basename.o bindresvport

COMPAT=bsd-arc4random.o bsd-asprintf.o bsd-closefrom.o bsd-cray.o bsd-cygwin_util.o bsd-getpeereid.o bsd-misc.o bsd-nextstep.o bsd-openpty.o bsd-snprintf.o bsd-waitpid.o fake-rfc2553.o openssl-compat.o xmmap.o xcrypt.o

-PORTS=port-irix.o port-linux.o port-aix.o port-uw.o port-tun.o
+PORTS=port-aix.o port-irix.o port-linux.o port-solaris.o port-tun.o port-uw.o

.c.o:
$(CC) $(CFLAGS) $(CPPFLAGS) -c $<
Index: openbsd-compat/openbsd-compat.h
================================================== =================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/openbsd-compat.h,v
retrieving revision 1.40
diff -u -p -r1.40 openbsd-compat.h
--- openbsd-compat/openbsd-compat.h 12 Jul 2006 13:10:34 -0000 1.40
+++ openbsd-compat/openbsd-compat.h 2 Aug 2006 12:54:09 -0000
@@ -190,10 +190,12 @@ char *shadow_pw(struct passwd *pw);
/* Routines for a single OS platform */
#include "bsd-cray.h"
#include "bsd-cygwin_util.h"
+
+#include "port-aix.h"
#include "port-irix.h"
#include "port-linux.h"
-#include "port-aix.h"
-#include "port-uw.h"
+#include "port-solaris.h"
#include "port-tun.h"
+#include "port-uw.h"

#endif /* _OPENBSD_COMPAT_H */
Index: openbsd-compat/port-solaris.c
================================================== =================
RCS file: openbsd-compat/port-solaris.c
diff -N openbsd-compat/port-solaris.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ openbsd-compat/port-solaris.c 2 Aug 2006 13:16:16 -0000
@@ -0,0 +1,177 @@
+/* $Id$ */
+
+/*
+ * Copyright (c) 2006 Chad Mynhier.
+ *
+ * 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 "config.h"
+
+#ifdef USE_SOLARIS_PROCESS_CONTRACTS
+
+#include
+#include
+#include
+
+#include
+#ifdef HAVE_FCNTL_H
+# include
+#endif
+#include
+#include
+
+#include
+#include
+#include
+
+#include "log.h"
+
+static int tmpl_fd = -1;
+
+void
+solaris_contract_pre_fork(void)
+{
+ if ((tmpl_fd = open64(CTFS_ROOT "/process/template", O_RDWR)) == -1) {
+ error("Error opening " CTFS_ROOT "/process/template: %.100s",
+ strerror(errno));
+ goto cleanup_pre_fork_contract_processing;
+ }
+ /*
+ * We have to set certain attributes before activating the template.
+ */
+ if (ct_pr_tmpl_set_fatal(tmpl_fd, CT_PR_EV_HWERR|CT_PR_EV_SIGNAL) != 0) {
+ error("Error setting process contract template fatal events: "
+ "%.100s", strerror(errno));
+ goto cleanup_pre_fork_contract_processing;
+ }
+ if (ct_tmpl_set_critical(tmpl_fd, CT_PR_EV_HWERR) != 0) {
+ error("Error setting process contract template critical "
+ "events: %.100s", strerror(errno));
+ goto cleanup_pre_fork_contract_processing;
+ }
+ /*
+ * Now make this the active template for this process.
+ */
+ if (ct_tmpl_activate(tmpl_fd) != 0) {
+ error("Error activating process contract template: %.100s",
+ strerror(errno));
+ goto cleanup_pre_fork_contract_processing;
+ }
+ return;
+
+cleanup_pre_fork_contract_processing:
+ /*
+ * Certain failure modes could lead to file descriptor leakage
+ * if we don't clean up after ourselves.
+ */
+ if (tmpl_fd > 0)
+ close(tmpl_fd);
+ tmpl_fd = -1;
+}
+
+void
+solaris_contract_post_fork(pid_t pid)
+{
+ char ctl_path[MAXPATHLEN];
+ ctid_t ctid;
+ ct_stathdl_t stathdl;
+ int ctl_fd = -1;
+ int pathlen;
+ int stat_fd = -1;
+
+ /*
+ * First clear the active template.
+ */
+ if (ct_tmpl_clear(tmpl_fd) != 0) {
+ error("Error clearing active process contract template: %.100s",
+ strerror(errno));
+ goto cleanup_post_fork_contract_processing;
+ }
+ close(tmpl_fd);
+ tmpl_fd = -1;
+
+ /*
+ * If the fork didn't succeed (pid < 0), or if we're the child
+ * (pid == 0), we have nothing more to do.
+ */
+ if (pid <= 0)
+ return;
+
+ /*
+ * Now abandon the contract we've created. This involves the
+ * following steps:
+ * - Get the contract id (ct_status_read(), ct_status_get_id())
+ * - Get an fd for the ctl file for this contract
+ * (/proc/all/contracts//ctl)
+ * - Abandon the contract (ct_ctl_abandon(fd))
+ */
+ if ((stat_fd = open64(CTFS_ROOT "/process/latest", O_RDONLY)) == -1) {
+ error("Error opening 'latest' process contract: %.100s",
+ strerror(errno));
+ goto cleanup_post_fork_contract_processing;
+ }
+ if (ct_status_read(stat_fd, CTD_COMMON, &stathdl) != 0) {
+ error("Error reading process contract status: %.100s",
+ strerror(errno));
+ goto cleanup_post_fork_contract_processing;
+ }
+ if ((ctid = ct_status_get_id(stathdl)) < 0) {
+ error("Error getting process contract id: %.100s",
+ strerror(errno));
+ goto cleanup_post_fork_contract_processing;
+ }
+ ct_status_free(stathdl);
+ close(stat_fd);
+
+ pathlen = snprintf(ctl_path, MAXPATHLEN, CTFS_ROOT "/process/%ld/ctl",
+ ctid);
+ if (pathlen < 0 || pathlen >= MAXPATHLEN) {
+ /*
+ * Note: Even though this case is unlikely, this could
+ * theoretically lead to contract id leakage, as it would
+ * prevent us from abandoning the contract. If we were to
+ * run long enough with enough leakage, we would bump
+ * up against process resource limits.
+ */
+ error("Won't attempt to open process contract ctl file: %.100s",
+ strerror(ENAMETOOLONG));
+ goto cleanup_post_fork_contract_processing;
+ }
+ if ((ctl_fd = open64(ctl_path, O_WRONLY)) < 0) {
+ error("Error opening process contract ctl file: %.100s",
+ strerror(errno));
+ goto cleanup_post_fork_contract_processing;
+ }
+ if (ct_ctl_abandon(ctl_fd) < 0) {
+ error("Error abandoning process contract: %.100s",
+ strerror(errno));
+ goto cleanup_post_fork_contract_processing;
+ }
+ close(ctl_fd);
+ return;
+
+cleanup_post_fork_contract_processing:
+ /*
+ * Certain failure modes could lead to file descriptor leakage
+ * if we don't clean up after ourselves.
+ */
+ if (tmpl_fd > 0)
+ close(tmpl_fd);
+ if (stat_fd > 0)
+ close(stat_fd);
+ if (ctl_fd > 0)
+ close(ctl_fd);
+ return;
+}
+#endif
Index: openbsd-compat/port-solaris.h
================================================== =================
RCS file: openbsd-compat/port-solaris.h
diff -N openbsd-compat/port-solaris.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ openbsd-compat/port-solaris.h 2 Aug 2006 12:56:14 -0000
@@ -0,0 +1,26 @@
+/* $Id$ */
+
+/*
+ * Copyright (c) 2006 Chad Mynhier.
+ *
+ * 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.
+ */
+
+#ifndef _PORT_SOLARIS_H
+
+#include
+
+void solaris_contract_pre_fork(void);
+void solaris_contract_post_fork(pid_t pid);
+
+#endif

--
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
http://lists.mindrot.org/mailman/lis...enssh-unix-dev