Fixed FreeBSD support. We now find both /bin/login and /usr/bin/login. We

provide our own conversation function, if PAM misc is not available. We
rely on login_tty to set up the terminal for us. And we avoid a few other
API uses that turned out to be Linux specific extensions.


git-svn-id: https://shellinabox.googlecode.com/svn/trunk@63 0da03de8-d603-11dd-86c2-0f8696b7b6f9

config.h

@@ -5,11 +5,17 @@
 #define HAVE_DLFCN_H 1
 
 /* Define to 1 if you have the `dlopen' function. */
-/* #undef HAVE_DLOPEN */
+#define HAVE_DLOPEN 1
 
 /* Define to 1 if you have the <inttypes.h> header file. */
 #define HAVE_INTTYPES_H 1
 
+/* Define to 1 if you have the <libutil.h> header file. */
+/* #undef HAVE_LIBUTIL_H */
+
+/* Define to 1 if you have the `login_tty' function. */
+#define HAVE_LOGIN_TTY 1
+
 /* Define to 1 if you have the <memory.h> header file. */
 #define HAVE_MEMORY_H 1
 
@@ -31,6 +37,9 @@
 /* Define to 1 if you have the <security/pam_appl.h> header file. */
 #define HAVE_SECURITY_PAM_APPL_H 1
 
+/* Define to 1 if you have the <security/pam_client.h> header file. */
+#define HAVE_SECURITY_PAM_CLIENT_H 1
+
 /* Define to 1 if you have the <security/pam_misc.h> header file. */
 #define HAVE_SECURITY_PAM_MISC_H 1
 
@@ -61,6 +70,9 @@
 /* Define to 1 if you have the <utmpx.h> header file. */
 #define HAVE_UTMPX_H 1
 
+/* Define to 1 if you have the <utmp.h> header file. */
+#define HAVE_UTMP_H 1
+
 /* Name of package */
 #define PACKAGE "shellinabox"
 

config.h.in

@@ -9,6 +9,12 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if you have the <libutil.h> header file. */
+#undef HAVE_LIBUTIL_H
+
+/* Define to 1 if you have the `login_tty' function. */
+#undef HAVE_LOGIN_TTY
+
 /* Define to 1 if you have the <memory.h> header file. */
 #undef HAVE_MEMORY_H
 
@@ -30,6 +36,9 @@
 /* Define to 1 if you have the <security/pam_appl.h> header file. */
 #undef HAVE_SECURITY_PAM_APPL_H
 
+/* Define to 1 if you have the <security/pam_client.h> header file. */
+#undef HAVE_SECURITY_PAM_CLIENT_H
+
 /* Define to 1 if you have the <security/pam_misc.h> header file. */
 #undef HAVE_SECURITY_PAM_MISC_H
 
@@ -60,6 +69,9 @@
 /* Define to 1 if you have the <utmpx.h> header file. */
 #undef HAVE_UTMPX_H
 
+/* Define to 1 if you have the <utmp.h> header file. */
+#undef HAVE_UTMP_H
+
 /* Name of package */
 #undef PACKAGE
 

configure

@@ -19998,9 +19998,12 @@ fi
 
 
 
-for ac_header in openssl/bio.h openssl/err.h openssl/ssl.h pthread.h         \
+
-                  security/pam_appl.h security/pam_misc.h sys/prctrl.h        \
+
-                  utmpx.h
+
+for ac_header in libutil.h openssl/bio.h openssl/err.h openssl/ssl.h         \
+                  pthread.h security/pam_appl.h security/pam_client.h         \
+                  security/pam_misc.h sys/prctrl.h utmp.h utmpx.h
 do
 as_ac_Header=`echo "ac_cv_header_$ac_header" | $as_tr_sh`
 if { as_var=$as_ac_Header; eval "test \"\${$as_var+set}\" = set"; }; then
@@ -20299,6 +20302,174 @@ fi
 echo "${ECHO_T}$ac_cv_lib_dl_dlopen" >&6; }
 if test $ac_cv_lib_dl_dlopen = yes; then
   LIBS="-ldl $LIBS"
+                 cat >>confdefs.h <<\_ACEOF
+#define HAVE_DLOPEN 1
+_ACEOF
+
+fi
+
+fi
+done
+
+
+for ac_func in login_tty
+do
+as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
+{ echo "$as_me:$LINENO: checking for $ac_func" >&5
+echo $ECHO_N "checking for $ac_func... $ECHO_C" >&6; }
+if { as_var=$as_ac_var; eval "test \"\${$as_var+set}\" = set"; }; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+/* Define $ac_func to an innocuous variant, in case <limits.h> declares $ac_func.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $ac_func innocuous_$ac_func
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $ac_func (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $ac_func
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char $ac_func ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined __stub_$ac_func || defined __stub___$ac_func
+choke me
+#endif
+
+int
+main ()
+{
+return $ac_func ();
+  ;
+  return 0;
+}
+_ACEOF
+rm -f conftest.$ac_objext conftest$ac_exeext
+if { (ac_try="$ac_link"
+case "(($ac_try" in
+  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+  *) ac_try_echo=$ac_try;;
+esac
+eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
+  (eval "$ac_link") 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } && {
+	 test -z "$ac_c_werror_flag" ||
+	 test ! -s conftest.err
+       } && test -s conftest$ac_exeext &&
+       $as_test_x conftest$ac_exeext; then
+  eval "$as_ac_var=yes"
+else
+  echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+	eval "$as_ac_var=no"
+fi
+
+rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
+      conftest$ac_exeext conftest.$ac_ext
+fi
+ac_res=`eval echo '${'$as_ac_var'}'`
+	       { echo "$as_me:$LINENO: result: $ac_res" >&5
+echo "${ECHO_T}$ac_res" >&6; }
+if test `eval echo '${'$as_ac_var'}'` = yes; then
+  cat >>confdefs.h <<_ACEOF
+#define `echo "HAVE_$ac_func" | $as_tr_cpp` 1
+_ACEOF
+
+else
+  { echo "$as_me:$LINENO: checking for login_tty in -lutil" >&5
+echo $ECHO_N "checking for login_tty in -lutil... $ECHO_C" >&6; }
+if test "${ac_cv_lib_util_login_tty+set}" = set; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  ac_check_lib_save_LIBS=$LIBS
+LIBS="-lutil  $LIBS"
+cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char login_tty ();
+int
+main ()
+{
+return login_tty ();
+  ;
+  return 0;
+}
+_ACEOF
+rm -f conftest.$ac_objext conftest$ac_exeext
+if { (ac_try="$ac_link"
+case "(($ac_try" in
+  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+  *) ac_try_echo=$ac_try;;
+esac
+eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
+  (eval "$ac_link") 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } && {
+	 test -z "$ac_c_werror_flag" ||
+	 test ! -s conftest.err
+       } && test -s conftest$ac_exeext &&
+       $as_test_x conftest$ac_exeext; then
+  ac_cv_lib_util_login_tty=yes
+else
+  echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+	ac_cv_lib_util_login_tty=no
+fi
+
+rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
+      conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+{ echo "$as_me:$LINENO: result: $ac_cv_lib_util_login_tty" >&5
+echo "${ECHO_T}$ac_cv_lib_util_login_tty" >&6; }
+if test $ac_cv_lib_util_login_tty = yes; then
+  LIBS="-lutil $LIBS"
+                 cat >>confdefs.h <<\_ACEOF
+#define HAVE_LOGIN_TTY 1
+_ACEOF
+
 fi
 
 fi

configure.ac

@@ -8,11 +8,17 @@ AC_PROG_LIBTOOL
 AC_SUBST(LIBTOOL_DEPS)
 AC_C_CONST
 AC_PROG_GCC_TRADITIONAL
-AC_CHECK_HEADERS([openssl/bio.h openssl/err.h openssl/ssl.h pthread.h         \
+AC_CHECK_HEADERS([libutil.h openssl/bio.h openssl/err.h openssl/ssl.h         \
-                  security/pam_appl.h security/pam_misc.h sys/prctrl.h        \
+                  pthread.h security/pam_appl.h security/pam_client.h         \
-                  utmpx.h])
+                  security/pam_misc.h sys/prctrl.h utmp.h utmpx.h])
 AC_CHECK_FUNCS(dlopen, [],
-               [AC_CHECK_LIB(dl, dlopen, LIBS="-ldl $LIBS")])
+               [AC_CHECK_LIB(dl, dlopen,
+                [LIBS="-ldl $LIBS"
+                 AC_DEFINE(HAVE_DLOPEN)])])
+AC_CHECK_FUNCS(login_tty, [],
+               [AC_CHECK_LIB(util, login_tty,
+                [LIBS="-lutil $LIBS"
+                 AC_DEFINE(HAVE_LOGIN_TTY)])])
 AC_TRY_LINK([#ifndef _XOPEN_SOURCE
              #define _XOPEN_SOURCE
              #endif

libhttp/ssl.c

@@ -256,7 +256,8 @@ int serverSupportsSSL(void) {
   // this is straight-forward. For threaded applications, we need to call
   // pthread_once(), instead. We perform run-time checks for whether we are
   // single- or multi-threaded, so that the same code can be used.
-#if defined(HAVE_PTHREAD_H)
+  // This currently only works on Linux.
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
   if (!!&pthread_once) {
     static pthread_once_t once = PTHREAD_ONCE_INIT;
     pthread_once(&once, loadSSL);

shellinabox/launcher.c

@@ -65,13 +65,24 @@
 #include <termios.h>
 #include <unistd.h>
 
+#ifdef HAVE_LIBUTIL_H
+#include <libutil.h>
+#endif
+
+#ifdef HAVE_UTMP_H
+#include <utmp.h>
+#endif
+
 #ifdef HAVE_UTMPX_H
 #include <utmpx.h>
 #endif
 
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
 #include <security/pam_appl.h>
+
+#if defined(HAVE_SECURITY_PAM_MISC_H)
 #include <security/pam_misc.h>
+#endif
 #else
 struct pam_message;
 struct pam_response;
@@ -92,9 +103,12 @@ extern int pthread_once(pthread_once_t *, void (*)(void))__attribute__((weak));
 
 // If PAM support is available, take advantage of it. Otherwise, silently fall
 // back on legacy operations for session management.
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
 static int (*x_pam_acct_mgmt)(pam_handle_t *, int);
 static int (*x_pam_authenticate)(pam_handle_t *, int);
+#if defined(HAVE_SECURITY_PAM_CLIENT_H)
+static int (**x_pam_binary_handler_fn)(void *, pamc_bp_t *);
+#endif
 static int (*x_pam_close_session)(pam_handle_t *, int);
 static int (*x_pam_end)(pam_handle_t *, int);
 static int (*x_pam_get_item)(const pam_handle_t *, int, const void **);
@@ -114,12 +128,133 @@ static int   launcher = -1;
 static uid_t restricted;
 
 
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
+
+// If the PAM misc library cannot be found, we have to provide our own basic
+// conversation function. As we know that this code is only ever called from
+// ShellInABox, it can be kept significantly simpler than the more generic
+// code that the PAM library implements.
+
+static int read_string(int echo, const char *prompt, char **retstr) {
+  *retstr                = NULL;
+  struct termios term_before, term_tmp;
+  if (tcgetattr(0, &term_before) != 0) {
+    return -1;
+  }
+  memcpy(&term_tmp, &term_before, sizeof(term_tmp));
+  if (!echo) {
+    term_tmp.c_lflag    &= ~ECHO;
+  }
+  int nc;
+  for (;;) {
+    tcsetattr(0, TCSAFLUSH, &term_tmp);
+    fprintf(stderr, "%s", prompt);
+    char *line;
+    const int lineLength = 512;
+    check(line           = calloc(1, lineLength));
+    nc                   = read(0, line, lineLength - 1);
+    tcsetattr(0, TCSADRAIN, &term_before);
+    if (!echo) {
+      fprintf(stderr, "\n");
+    }
+    if (nc > 0) {
+      if (line[nc-1] == '\n') {
+        nc--;
+      } else if (echo) {
+        fprintf(stderr, "\n");
+      }
+      line[nc]           = '\000';
+      check(*retstr      = line);
+      break;
+    } else {
+      memset(line, 0, lineLength);
+      free(line);
+      if (echo) {
+        fprintf(stderr, "\n");
+      }
+      break;
+    }
+  }
+  tcsetattr(0, TCSADRAIN, &term_before);
+  return nc;
+}
+
+static pamc_bp_t *p(pamc_bp_t *p) {
+  // GCC is too smart for its own good, and triggers a warning in
+  // PAM_BP_RENEW, unless we pass the first argument through a function.
+  return p;
+}
+
+static int my_misc_conv(int num_msg, const struct pam_message **msgm,
+                        struct pam_response **response, void *appdata_ptr) {
+  if (num_msg <= 0) {
+    return PAM_CONV_ERR;
+  }
+  struct pam_response *reply;
+  check(reply = (struct pam_response *)calloc(num_msg,
+                                              sizeof(struct pam_response)));
+  for (int count = 0; count < num_msg; count++) {
+    char *string                 = NULL;
+    switch(msgm[count]->msg_style) {
+      case PAM_PROMPT_ECHO_OFF:
+        if (read_string(0, msgm[count]->msg, &string) < 0) {
+          goto failed_conversation;
+        }
+        break;
+      case PAM_PROMPT_ECHO_ON:
+        if (read_string(1, msgm[count]->msg, &string) < 0) {
+          goto failed_conversation;
+        }
+        break;
+      case PAM_ERROR_MSG:
+        if (fprintf(stderr, "%s\n", msgm[count]->msg) < 0) {
+          goto failed_conversation;
+        }
+        break;
+      case PAM_TEXT_INFO:
+        if (fprintf(stdout, "%s\n", msgm[count]->msg) < 0) {
+          goto failed_conversation;
+        }
+        break;
+#if defined(HAVE_SECURITY_PAM_CLIENT_H)
+      case PAM_BINARY_PROMPT: {
+        pamc_bp_t binary_prompt = NULL;
+        if (!msgm[count]->msg || !*x_pam_binary_handler_fn) {
+          goto failed_conversation;
+        }
+        PAM_BP_RENEW(p(&binary_prompt), PAM_BP_RCONTROL(msgm[count]->msg),
+                     PAM_BP_LENGTH(msgm[count]->msg));
+        PAM_BP_FILL(binary_prompt, 0, PAM_BP_LENGTH(msgm[count]->msg),
+                    PAM_BP_RDATA(msgm[count]->msg));
+        if ((*x_pam_binary_handler_fn)(appdata_ptr, &binary_prompt) !=
+            PAM_SUCCESS || !binary_prompt) {
+          goto failed_conversation;
+        }
+        string                  = (char *)binary_prompt;
+        break; }
+#endif
+      default:
+        goto failed_conversation;
+    }
+    if (string) {
+      reply[count].resp_retcode = 0;
+      reply[count].resp         = string;
+    }
+  }
+failed_conversation:
+  *response                     = reply;
+  return PAM_SUCCESS;
+}
+
 static void *loadSymbol(const char *lib, const char *fn) {
   void *dl = RTLD_DEFAULT;
   void *rc = dlsym(dl, fn);
   if (!rc) {
+#ifdef RTLD_NOLOAD
     dl     = dlopen(lib, RTLD_LAZY|RTLD_GLOBAL|RTLD_NOLOAD);
+#else
+    dl     = NULL;
+#endif
     if (dl == NULL) {
       dl   = dlopen(lib, RTLD_LAZY|RTLD_GLOBAL);
     }
@@ -141,18 +276,32 @@ static void loadPAM(void) {
     const char *lib;
     const char *fn;
   } symbols[] = {
-    { { &x_pam_acct_mgmt },     "libpam.so",      "pam_acct_mgmt"     },
+    { { &x_pam_acct_mgmt },         "libpam.so",      "pam_acct_mgmt"     },
-    { { &x_pam_authenticate },  "libpam.so",      "pam_authenticate"  },
+    { { &x_pam_authenticate },      "libpam.so",      "pam_authenticate"  },
-    { { &x_pam_close_session }, "libpam.so",      "pam_close_session" },
+#if defined(HAVE_SECURITY_PAM_CLIENT_H)
-    { { &x_pam_end },           "libpam.so",      "pam_end"           },
+    { { &x_pam_binary_handler_fn }, "libpam_misc.so", "pam_binary_handler_fn"},
-    { { &x_pam_get_item },      "libpam.so",      "pam_get_item"      },
+#endif
-    { { &x_pam_open_session },  "libpam.so",      "pam_open_session"  },
+    { { &x_pam_close_session },     "libpam.so",      "pam_close_session" },
-    { { &x_pam_set_item },      "libpam.so",      "pam_set_item"      },
+    { { &x_pam_end },               "libpam.so",      "pam_end"           },
-    { { &x_pam_start },         "libpam.so",      "pam_start"         },
+    { { &x_pam_get_item },          "libpam.so",      "pam_get_item"      },
-    { { &x_misc_conv },         "libpam_misc.so", "misc_conv"         }
+    { { &x_pam_open_session },      "libpam.so",      "pam_open_session"  },
+    { { &x_pam_set_item },          "libpam.so",      "pam_set_item"      },
+    { { &x_pam_start },             "libpam.so",      "pam_start"         },
+    { { &x_misc_conv },             "libpam_misc.so", "misc_conv"         }
   };
   for (int i = 0; i < sizeof(symbols)/sizeof(symbols[0]); i++) {
     if (!(*symbols[i].var = loadSymbol(symbols[i].lib, symbols[i].fn))) {
+#if defined(HAVE_SECURITY_PAM_CLIENT_H)
+      if (!strcmp(symbols[i].fn, "pam_binary_handler_fn")) {
+        // Binary conversation support is optional
+        continue;
+      } else
+#endif
+      if (!strcmp(symbols[i].fn, "misc_conv")) {
+        // PAM misc is optional
+        *symbols[i].var = (void *)my_misc_conv;
+        continue;
+      }
       debug("Failed to load PAM support. Could not find \"%s\"",
             symbols[i].fn);
       for (int j = 0; j < sizeof(symbols)/sizeof(symbols[0]); j++) {
@@ -166,13 +315,14 @@ static void loadPAM(void) {
 #endif
 
 int supportsPAM(void) {
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
 
   // We want to call loadPAM() exactly once. For single-threaded applications,
   // this is straight-forward. For threaded applications, we need to call
   // pthread_once(), instead. We perform run-time checks for whether we are
   // single- or multi-threaded, so that the same code can be used.
-#if defined(HAVE_PTHREAD_H)
+  // This currently only works on Linux.
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
   if (!!&pthread_once) {
     static pthread_once_t once = PTHREAD_ONCE_INIT;
     pthread_once(&once, loadPAM);
@@ -430,6 +580,9 @@ static int forkPty(int *pty, int useLogin, struct Utmp **utmp,
 
     closeAllFds((int []){ slave }, 1);
 
+#ifdef HAVE_LOGIN_TTY
+    login_tty(slave);
+#else
     // Become the session/process-group leader
     setsid();
     setpgid(0, 0);
@@ -441,6 +594,7 @@ static int forkPty(int *pty, int useLogin, struct Utmp **utmp,
     if (slave > 2) {
       NOINTR(close(slave));
     }
+#endif
     *pty                  = 0;
 
     // Force the pty to be our control terminal
@@ -509,7 +663,7 @@ static pam_handle_t *internalLogin(struct Service *service, struct Utmp *utmp,
   // Use PAM to negotiate user authentication and authorization
   const struct passwd *pw;
   pam_handle_t *pam            = NULL;
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
   struct pam_conv conv         = { .conv = x_misc_conv };
   if (service->authUser) {
     check(supportsPAM());
@@ -590,13 +744,13 @@ static pam_handle_t *internalLogin(struct Service *service, struct Utmp *utmp,
   if (restricted &&
       (service->uid != restricted || service->gid != pw->pw_gid)) {
     puts("\nAccess denied!");
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
     x_pam_end(pam, PAM_SUCCESS);
 #endif
     _exit(1);
   }
 
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
   if (pam) {
 #ifdef HAVE_UTMPX_H
     check(x_pam_set_item(pam, PAM_TTY, (const void **)utmp->utmpx.ut_line) ==
@@ -608,22 +762,27 @@ static pam_handle_t *internalLogin(struct Service *service, struct Utmp *utmp,
 #endif
 
   // Retrieve supplementary group ids.
-  int ngroups                  = 0;
+  int ngroups;
+#if defined(__linux__)
+  // On Linux, we can query the number of supplementary groups. On all other
+  // platforms, we play it safe and just assume a fixed upper bound.
+  ngroups                      = 0;
   getgrouplist(service->user, pw->pw_gid, NULL, &ngroups);
+#else
+  ngroups                      = 128;
+#endif
   check(ngroups >= 0);
   if (ngroups > 0) {
     // Set supplementary group ids
     gid_t *groups;
     check(groups               = malloc((ngroups + 1) * sizeof(gid_t)));
-    groups[ngroups]            = service->gid;
+    check(getgrouplist(service->user, pw->pw_gid, groups, &ngroups) >= 0);
-    check(getgrouplist(service->user, pw->pw_gid, groups, &ngroups) ==
-          ngroups);
 
     // Make sure that any group that was requested on the command line is
     // included, if it is not one of the normal groups for this user.
     for (int i = 0; ; i++) {
       if (i == ngroups) {
-        ngroups++;
+        groups[ngroups++]      = service->gid;
         break;
       } else if (groups[i] == service->gid) {
         break;
@@ -917,7 +1076,7 @@ static void childProcess(struct Service *service, int width, int height,
   // In that case, we do not bother about session management.
   if (!service->useLogin) {
     pam_handle_t *pam           = internalLogin(service, utmp, &environment);
-#if defined(HAVE_SECURITY_PAM_APPL_H) && defined(HAVE_SECURITY_PAM_MISC_H)
+#if defined(HAVE_SECURITY_PAM_APPL_H)
     if (pam && !geteuid()) {
       check(x_pam_open_session(pam, PAM_SILENT) == PAM_SUCCESS);
       pid_t pid                 = fork();
@@ -929,7 +1088,7 @@ static void childProcess(struct Service *service, int width, int height,
       default:;
         // Finish all pending PAM operations.
         int status, rc;
-        check(waitpid(pid, &status, 0) == pid);
+        check(NOINTR(waitpid(pid, &status, 0)) == pid);
         check((rc               = x_pam_close_session(pam, PAM_SILENT)) ==
               PAM_SUCCESS);
         check(x_pam_end(pam, rc) == PAM_SUCCESS);
@@ -971,6 +1130,7 @@ static void childProcess(struct Service *service, int width, int height,
   // Finally, launch the child process.
   if (service->useLogin) {
     execle("/bin/login", "login", "-p", "-h", peerName, NULL, environment);
+    execle("/usr/bin/login", "login", "-p", "-h", peerName, NULL, environment);
   } else {
     execService(width, height, service, peerName, environment);
   }