Add a new package radius-shell with a setcap radius_shell front end

Ticket: CM-19457
Reviewed By: nobody
Testing Done:  multiple logins, separately and simultaneously

Because we can't determine privilege level separately and up front with
the RADIUS protocol, unlike TACACS+, we wind up with all logins as the
same unprivileged radius uid.  But we can set the auid (accounting or
auditing uid) correctly, and a separate setcap radius_shell can be set as
the login shell, and can fixup the uid before running /bin/bash.

To set the auid correctly, we need to know the privileged radius user
account.  Added mapped_priv_user to the configuration file to handle
that.  mapped_priv_user has to match the account used by libnss-mapuser.
That's a bit ugly, but a common config file would be uglier.

The radius shell is in a new package, since it has binaries. The new
package is radius-shell.  In it's post actions, it changes the radius
users shell to radius_shell if they are present, and back to /bin/bash
on package removal.   It uses capabilities, tries to be very restrictive
in what it changes, and depends on being installed setcap cap_setuid

Make the existing libpam-radius-auth package depend on radius-shell, so
it will pull in the new package on upgrades.

Also fixed another issue with reparsing changed config file, have to
handle case where there were servers defined, but aren't any longer.

14 files changed, 326 insertions, 30 deletions

diff --git a/Makefile b/Makefile
index 21027e0..3da24a4 100644
--- a/Makefile
+++ b/Makefile
@@ -24,14 +24,17 @@ CFLAGS := -Wall -Werror -fPIC ${CFLAGS}
 # Then copy pam_radius_auth.so to /usr/freeware/lib32/security (PAM dir)
 # CFLAGS =
 
-#LDFLAGS += -shared -Wl,--version-script=pamsymbols.ver
-LDFLAGS += -shared
+#LDSHFLAGS += -shared -Wl,--version-script=pamsymbols.ver
+LDSHFLAGS = -shared
+LDLIBS += -laudit
+BINLIBS += -lcap
+LIBLIBS += -lpam
 
 ######################################################################
 #
 #  The default rule to build everything.
 #
-all: pam_radius_auth.so
+all: pam_radius_auth.so radius_shell
 
 ######################################################################
 #
@@ -48,6 +51,9 @@ src/pam_radius_auth.o: src/pam_radius_auth.c src/pam_radius_auth.h
 src/md5.o: src/md5.c src/md5.h
 	@$(MAKE) -C src $(notdir $@)
 
+src/radius_shell.o: src/radius_shell.c
+	@$(MAKE) -C src $(notdir $@)
+
 #
 # This is what should work on Irix:
 #pam_radius_auth.so: pam_radius_auth.o md5.o
@@ -67,7 +73,10 @@ src/md5.o: src/md5.c src/md5.h
 #	gcc -shared pam_radius_auth.o md5.o -lpam -lc -o pam_radius_auth.so
 #
 pam_radius_auth.so: src/pam_radius_auth.o src/support.o src/md5.o
-	$(CC) $(LDFLAGS) $^ -lpam -o pam_radius_auth.so
+	$(CC) $(LDFLAGS) $(LDSHFLAGS) $^ $(LDLIBS) $(LIBLIBS) -o $@
+
+radius_shell: src/radius_shell.o
+	$(CC) $(LDFLAGS) $^ $(LDLIBS) $(BINLIBS) -o $@
 
 ######################################################################
 #
@@ -85,4 +94,4 @@ dist:
 #
 .PHONY: clean
 clean:
-	@rm -f *~ *.so *.o src/*.o src/*~
+	@rm -f *~ *.so *.o src/*.o src/*~ radius_shell
diff --git a/debian/control b/debian/control
index b70c948..b8022db 100644
--- a/debian/control
+++ b/debian/control
@@ -3,13 +3,22 @@ Maintainer: dev-support <dev-support@cumulusnetworks.com>
 Section: libs
 Priority: extra
 Standards-Version: 3.9.6
-Build-Depends: libpam0g-dev | libpam-dev, debhelper (>= 9~), libaudit-dev
+Build-Depends: libpam0g-dev | libpam-dev, debhelper (>= 9~), libaudit-dev, libcap-dev
 
 Package: libpam-radius-auth
 Architecture: any
-Depends: ${shlibs:Depends}, ${misc:Depends}, libaudit1
+Depends: ${shlibs:Depends}, ${misc:Depends}, libaudit1, radius-shell
 Description: PAM RADIUS client authentication module
  This is the PAM to RADIUS authentication module. It allows any PAM-capable
  machine to become a RADIUS client for authentication and accounting
  requests. You will, however, need to supply your own RADIUS server to
  perform the actual authentication
+
+Package: radius-shell
+Architecture: any
+Depends: ${shlibs:Depends}, ${misc:Depends}, libaudit1, libcap2-bin, libcap2, libnss-mapuser
+Description: Shell front-end used for radius users.
+ This provides a uid fixup program.  Due to the limitations of the
+ RADIUS protocol, we can't tell whether a user is privileged until
+ after authentication.  This packages provides a shell front-end that
+ sets the uid to the auid, if set and > 1000, and not already matching.
diff --git a/debian/radius-shell.install b/debian/radius-shell.install
new file mode 100644
index 0000000..7671d36
--- /dev/null
+++ b/debian/radius-shell.install
@@ -0,0 +1 @@
+radius_shell sbin
diff --git a/debian/radius-shell.manpages b/debian/radius-shell.manpages
new file mode 100644
index 0000000..bb1a970
--- /dev/null
+++ b/debian/radius-shell.manpages
@@ -0,0 +1 @@
+radius_shell.8
diff --git a/debian/radius-shell.postinst b/debian/radius-shell.postinst
new file mode 100644
index 0000000..55ebd22
--- /dev/null
+++ b/debian/radius-shell.postinst
@@ -0,0 +1,29 @@
+#! /bin/sh
+
+set -e
+
+# we depend on libnss-mapuser, so that the radius group will have been
+# created before this script runs.
+
+case "$1" in
+    configure)
+        radshell=/sbin/radius_shell
+        chmod 750 $radshell
+        chgrp radius_users $radshell
+        setcap  cap_setuid+ep $radshell
+        # The users will have been created by the libnss-mapuser package
+        # and possibly by an older version, so change the shells here.
+        # This also prevents a loop in package install ordering dependencies
+        for usr in radius_user radius_priv_user; do
+            uent="$(getent -s compat passwd $usr 2>/dev/null)" || true
+            [ -z "$uent" ] && continue
+            case "$uent" in
+                *${radshell}*) ;;
+                *) chsh -s $radshell $usr ;;
+            esac
+        done
+        ;;
+esac
+
+#DEBHELPER#
+
diff --git a/debian/radius-shell.postrm b/debian/radius-shell.postrm
new file mode 100644
index 0000000..89ae97f
--- /dev/null
+++ b/debian/radius-shell.postrm
@@ -0,0 +1,23 @@
+#! /bin/sh
+
+set -e
+
+# we depend on libnss-mapuser, so that the radius group will have been
+# created before this script runs.
+
+case "$1" in
+    remove|purge)
+        # fixup the shell for the users we may have modified on installation,
+        # if still present, and using our shell
+        for usr in radius_user radius_priv_user; do
+            uent="$(getent -s compat passwd $usr 2>/dev/null)" || true
+            [ -z "$uent" ] && continue
+            case "$uent" in
+                *${radshell}*) chsh -s /bin/bash $usr ;;
+            esac
+        done
+        ;;
+esac
+
+#DEBHELPER#
+
diff --git a/debian/rules b/debian/rules
index 52172f8..3039568 100755
--- a/debian/rules
+++ b/debian/rules
@@ -21,8 +21,10 @@ export CFLAGS
 # all the installing is here, not in Makefile.
 # The configuration file with the share secrets needs to be 600
 override_dh_install:
-	dh_install -v --sourcedir=.
+	dh_install -v --sourcedir=. --package=libpam-radius-auth
+	dh_install -v --sourcedir=.  --package=radius-shell
 	chmod 600 debian/*/${PAM_CONF_FILE}
+	chmod 750 debian/*/sbin/radius_shell
 
 override_dh_fixperms:
 	dh_fixperms --exclude ${PAM_CONF_FILE}
diff --git a/pam_radius_auth.5 b/pam_radius_auth.5
index 05a9f0b..b4b67f8 100644
--- a/pam_radius_auth.5
+++ b/pam_radius_auth.5
@@ -66,6 +66,12 @@ to be considered a
 privileged login (ability to configure via nclu 'net' commands, and able to sudo.
 The default is 15.  The range is 0-15.  Only matters when the VSA attribute is
 returned.
+.TP
+.I mapped_priv_user accountname
+This sets the name of the privileged mapping account in /etc/passwd.
+It must match the privileged account name in
+.IR nss_mapuser (5)
+for correct operation.
 .SH "SEE ALSO"
 .BR pam_radius_auth (8),
 .BR nss_mapuser (5)
diff --git a/pam_radius_auth.conf b/pam_radius_auth.conf
index 199d383..51b6595 100644
--- a/pam_radius_auth.conf
+++ b/pam_radius_auth.conf
@@ -55,3 +55,7 @@
 #
 #  Uncomment to enable debugging, can be used instead of altering pam files
 # debug
+#
+# Account for privileged radius user mapping.  If you change it here,  you need
+# to change /etc/nss_mapuser.conf as well
+mapped_priv_user radius_priv_user
diff --git a/radius_shell.8 b/radius_shell.8
new file mode 100644
index 0000000..94b1930
--- /dev/null
+++ b/radius_shell.8
@@ -0,0 +1,47 @@
+.TH radius_shell 8
+.\" Copyright 2018 Cumulus Networks, Inc.  All rights reserved.
+.SH NAME
+radius_shell - front end shell for radius users
+.SH SYNOPSIS
+.B /sbin/radius_shell
+is  RADIUS client front end shell that will ensure that the uid is set
+to the auid (the accounting uid).
+.SH DESCRIPTION
+This shell front-end needed because at login, it's
+not possible to determine if a user is privileged up front, because
+the RADIUS protocol combines authentication and authorization in a single
+transaction.
+.P
+That means that all RADIUS users login as the same base mapped user and therefore
+the same UID, although the auid will be set differently.
+.P
+The
+.B radius_shell
+is installed with setcap permissions that allow it to set the uid.
+It is set as the login shell for the radius users via the
+.I libnss-mapuser
+package.
+.P
+For security, the uid of the process is only changed if the auid is set,
+and is 1000 or larger (this is the normal minimum uid for non-privileged
+users via
+.I adduser
+and
+.IR useradd .
+The value is hardcoded in the source, it is not read from the
+.I adduser.conf
+configuration file.
+.P
+Whether the uid is changed or not, a login shell is exec'ed.
+At this time, the login shell is only
+.BR /bin/bash ,
+although the other shells listed in
+.I /etc/shells
+may be allowed in the future.
+.SH "SEE ALSO"
+.BR setcap (8),
+.BR pam_radius_auth (8),
+.BR nss_mapuser (5)
+.SH FILES
+.SH AUTHOR
+Dave Olson <olson@cumulusnetworks.com>
diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c
index c4274fa..7f29f37 100644
--- a/src/pam_radius_auth.c
+++ b/src/pam_radius_auth.c
@@ -203,10 +203,11 @@ static int get_ipaddr(char *host, struct sockaddr *addr, char *port)
 }
 
 /*
- * take server->hostname, and convert it to server->ip
+ * Lookup server->hostname, to get server->ip
  * Done once when server list parsed.  The last part, the
  * if port isn't set in config, it needs to be set to either
  * radius or raddacct
+ * returns 0 on success, otherwise non-zero
  */
 static int host2server(pam_handle_t * pamh, radius_server_t * server)
 {
@@ -216,7 +217,7 @@ static int host2server(pam_handle_t * pamh, radius_server_t * server)
 
 	/* hostname might be [ipv6::address] */
 	strncpy(hostbuffer, server->hostname, sizeof(hostbuffer) - 1);
-	hostbuffer[sizeof(hostbuffer) - 1] = 0;
+	hostbuffer[sizeof(hostbuffer) - 1] = 0; /* ensure null term */
 	hostname = hostbuffer;
 	portstart = hostbuffer;
 	if (hostname[0] == '[') {
@@ -653,7 +654,7 @@ static void cleanup_conf(pam_handle_t * pamh, void *arg, int unused)
 static int parse_conffile(pam_handle_t * pamh, radius_conf_t * cf)
 {
 	static struct stat last_st;
-	int line = 0, timeout;
+	int line = 0, timeout, ret = 0;
 	const char *cfname = cf->conf_file;
 	char *p;
 	radius_server_t *server = NULL, *tmp;
@@ -661,8 +662,10 @@ static int parse_conffile(pam_handle_t * pamh, radius_conf_t * cf)
 	char hostname[BUFFER_SIZE], secret[BUFFER_SIZE], buffer[BUFFER_SIZE];
 	char srcip[BUFFER_SIZE];
 
-	if (!cfname || !*cfname)
-		return -1;
+	if (!cfname || !*cfname) {
+		ret = -1;
+		goto done;
+	}
 
 	if (last_st.st_ino) {
 		struct stat st;
@@ -670,18 +673,22 @@ static int parse_conffile(pam_handle_t * pamh, radius_conf_t * cf)
 		rst = stat(cfname, &st);
 		if (!rst && st.st_ino == last_st.st_ino && st.st_mtime ==
 		    last_st.st_mtime && st.st_ctime == last_st.st_ctime) {
+			/* no changes to savconf, so just return */
 			return 1;
 		}
 	}
 
-	if (cf->server)		/* we already had sockets open and bound, cleanup */
+	if (cf->server)	{ /* we already had sockets open and bound, cleanup */
 		pam_set_data(pamh, "rad_conf_cleanup", NULL, NULL);
+		cf->server = NULL; /* in case reuse and no servers found */
+	}
 
 	/* the first time around, read the configuration file */
 	if ((fserver = fopen(cfname, "r")) == (FILE *) NULL) {
 		_pam_log(pamh, LOG_ERR, "Could not open configuration file %s:"
 			 " %m", cfname);
-		return -1;
+		ret = -1;
+		goto done;
 	}
 
 	while (!feof(fserver) &&
@@ -746,6 +753,20 @@ static int parse_conffile(pam_handle_t * pamh, radius_conf_t * cf)
 				}
 			}
 			continue;
+		} else if (!strcmp(hostname, "mapped_priv_user")) {
+			/* mapped account name of radius privileged user for
+			 * uid/auid fixup */
+			if (scancnt < 2)
+				_pam_log(pamh, LOG_ERR,
+					 "ERROR reading %s, line %d:"
+					 " only %d fields", cf->conf_file, line,
+					 scancnt);
+			else
+				snprintf(cf->privusrmap, sizeof cf->privusrmap, "%s",
+					 secret);
+			snprintf(savconf.privusrmap, sizeof savconf.privusrmap,
+				 "%s", secret);
+			continue;
 		} else if (!strcmp(hostname, "debug")) {
 			/* allow setting debug in config file as well */
 			cf->debug = cfg_debug = 1;
@@ -766,7 +787,8 @@ static int parse_conffile(pam_handle_t * pamh, radius_conf_t * cf)
 			_pam_log(pamh, LOG_ERR,
 				 "Unable to allocate server info for %s: %m",
 				 hostname);
-			return -1;
+			ret = -1;
+			goto done;
 		}
 		tmp->sockfd = -1;	/* mark as uninitialized */
 		if (server) {
@@ -804,17 +826,18 @@ static int parse_conffile(pam_handle_t * pamh, radius_conf_t * cf)
 	if (!cf->server) {	/* no server found, die a horrible death */
 		_pam_log(pamh, LOG_ERR, "No server found in"
 			 " configuration file %s", cf->conf_file);
-		return -1;
+		ret = -1;
 	}
 
 	/*
 	 * save the server in savconf for next call (if any) to _parse_args()
 	 * for the same config file (will be overridden if a different config
-	 * file
+	 * file; need to do that even if NULL, so we don't re-use old bad data
 	 */
+done:
 	savconf.server = cf->server;
 
-	return 0;
+	return ret;
 }
 
 static int setup_sock(pam_handle_t * pamh, radius_server_t * server,
@@ -1178,7 +1201,7 @@ static int talk_radius(radius_conf_t * conf, AUTH_HDR * request,
 						 * If the user says he wants the bug,
 						 * give in.
 						 */
-					} else {	/* authentication request */
+					} else { /* authentication request */
 						if (conf->accounting_bug) {
 							p = "";
 						}
@@ -1318,21 +1341,23 @@ static int rad_converse(pam_handle_t * pamh, int msg_style, char *message,
 /*
  *	We'll create the home directory if needed, and we'll write the flat file
  *	mapping entry.  It's done at this point, because this is the end of the
- *	authentication phase (and authorization, too, since authorization is part of
- *	authentication phase for RADIUS) for ssh, login, etc.
+ *	authentication phase (and authorization, too, since authorization is
+ *	part of *	authentication phase for RADIUS) for ssh, login, etc.
  */
 static void
-setup_userinfo(pam_handle_t * pamh, const char *user, int debug, int privileged)
+setup_userinfo(pam_handle_t * pamh, radius_conf_t *cfg, const char *user,
+	       int debug, int privileged)
 {
 	struct passwd *pw;
 
 	/*
-	 * set SUDO_PROMPT in env so that it prompts as the login user, not the mapped
-	 * user, unless (unlikely) the prompt has already been set.
+	 * set SUDO_PROMPT in env so that it prompts as the login user, not the
+	 * mapped * user, unless (unlikely) the prompt has already been set.
 	 * It won't hurt to do this if the user wasn't mapped.
 	 */
 	if (!pam_getenv(pamh, "SUDO_PROMPT")) {
-		char nprompt[strlen("SUDO_PROMPT=[sudo] password for ") + strlen(user) + 3];	/* + 3 for ": " and the \0 */
+		char nprompt[strlen("SUDO_PROMPT=[sudo] password for ") +
+			strlen(user) + 3];	/* + 3 for ": " and the \0 */
 		snprintf(nprompt, sizeof nprompt,
 			 "SUDO_PROMPT=[sudo] password for %s: ", user);
 		if (pam_putenv(pamh, nprompt) != PAM_SUCCESS)
@@ -1349,6 +1374,27 @@ setup_userinfo(pam_handle_t * pamh, const char *user, int debug, int privileged)
 	}
 
 	/*
+	 * because the RADIUS protocol is single pass, we always have the
+	 * pw_uid of the unprivileged account at this point.  Set things up
+	 * so we use the uid of the privileged radius account.
+	 */
+	if (privileged) {
+		struct passwd *pwp;
+		if (!cfg->privusrmap[0] || !(pwp = getpwnam(cfg->privusrmap))) {
+			_pam_log(pamh, LOG_WARNING, "Failed to find uid for"
+				 " privileged account %s, uid may be wrong"
+				 " for user %s",
+				 cfg->privusrmap[0] ? cfg->privusrmap :
+				 "(unset in config)", user);
+		}
+		else if (pwp && pw->pw_uid != pwp->pw_uid) {
+	syslog(LOG_DEBUG, "OLSON wrmap user=%s, but uid=%u, change to %u",
+	       user, pw->pw_uid, pwp->pw_uid);
+		pw->pw_uid = pwp->pw_uid;
+		}
+	}
+
+	/*
 	 * We don't "fail" on errors here, since they are not fatal for
 	 * the session, although they can result in name or uid lookups not
 	 * working correctly.
@@ -1601,7 +1647,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags, int argc,
 					 "=%d, min for priv=%d", privlvl,
 					 config.min_priv_lvl);
 		}
-		setup_userinfo(pamh, user, debug,
+		setup_userinfo(pamh, &config, user, debug,
 			       privlvl >= config.min_priv_lvl);
 		retval = PAM_SUCCESS;
 	} else {
diff --git a/src/pam_radius_auth.h b/src/pam_radius_auth.h
index 2b1e48d..b1a3173 100644
--- a/src/pam_radius_auth.h
+++ b/src/pam_radius_auth.h
@@ -154,6 +154,7 @@ typedef struct radius_conf_t {
 	int min_priv_lvl;
 	char prompt[MAXPROMPT];
 	char vrfname[64];
+	char privusrmap[64];
 } radius_conf_t;
 
 void __write_mapfile(pam_handle_t * p, const char *usr, uid_t uid, int priv,
diff --git a/src/radius_shell.c b/src/radius_shell.c
new file mode 100644
index 0000000..a94c7f3
--- /dev/null
+++ b/src/radius_shell.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2018 Cumulus Networks, Inc.
+ * All rights reserved.
+ * Author: Dave Olson <olson@cumulusnetworks.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program - see the file COPYING.
+ */
+
+/*
+ * This program exists to set the uid of privileged radius login users.
+ * Due to the limitations of the RADIUS protocol, we can't determine
+ * whether a user is privileged or not until they have authenticated,
+ * and by then, some of the login mechanisms (openssh, e.g.) have already
+ * determined the uid.
+ *
+ * This program looks at the accounting uid, and if set, and not the same
+ * as the uid, and the auid is >= 1000, will try to reset the uid to the auid
+ * as well as the fsuid.
+ *
+ * For this to work, the program must be installed as setcap cap_setuid.
+ * As a minor additional safeguard, the program should be installed as
+ * a member of the radius_users group, and permissions 750.
+ *
+ * Errors are written to stderr so the user logging in will see them,
+ * rather than using syslog.
+ */
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <libaudit.h>
+#include <string.h>
+#include <errno.h>
+#include <stdio.h>
+#include <sys/fsuid.h>
+#include <sys/capability.h>
+
+int main(int cnt, char **args)
+{
+	uid_t uid, auid;
+	cap_value_t capability[] = { CAP_SETUID};
+	cap_t capabilities;
+	char *shell = NULL, *check = NULL, execshell[64];
+
+	uid = getuid();
+	auid = audit_getloginuid();
+
+	if (uid < 1000 || auid < 1000 || auid == (uid_t)-1 || uid == auid) {
+		/*  We try to be careful in what we will change  */
+		goto execit;
+	}
+
+	if (setfsuid(auid) == -1)
+		fprintf(stderr, "Failed to set fsuid to %u: %s\n",
+			auid, strerror(errno));
+	if (setresuid(auid, auid, auid))
+		fprintf(stderr, "Failed to set uid to %u: %s\n",
+			auid, strerror(errno));
+	if (getuid() != auid)
+		fprintf(stderr, "Failed to set uid to %u it's still %u\n",
+			auid, getuid());
+
+execit:
+	/*  be paranoid, and clear our expected CAP_SETUID capability,
+	 *  even though it should be cleared on exec.
+	 */
+	capabilities = cap_get_proc();
+	if (capabilities) {
+		if (!cap_set_flag(capabilities, CAP_EFFECTIVE, 1,
+				   capability, CAP_CLEAR) &&
+		    !cap_set_flag(capabilities, CAP_PERMITTED, 1,
+					   capability, CAP_CLEAR)) {
+		    if (cap_set_proc(capabilities))
+			fprintf(stderr, "Failed to clear cap_setuid: %s\n",
+				strerror(errno));
+		    }
+	}
+
+#ifdef LATER
+	/*
+	 * Eventually handle this program being linked or symlinked
+	 * and that the shell is one of the shells in /etc/shells
+	 */
+	shell = strrchr(args[0], '/');
+	if (!shell)
+		shell = args[0];
+
+	if (*shell == '-') {
+		check = shell + 1;
+	}
+	else
+		check = shell;
+
+	/* should really check this against /etc/shell */
+	snprintf(execshell, sizeof execshell, "/bin/%s", check);
+#else
+	check = "bash";
+	shell = "-bash";
+	snprintf(execshell, sizeof execshell, "/bin/%s", check);
+#endif
+
+	execl(execshell, shell, NULL);
+	fprintf(stderr, "Exec of shell %s failed: %s\n", execshell,
+		strerror(errno));
+	exit(1);
+}
diff --git a/src/support.c b/src/support.c
index ed72f85..78c21f4 100644
--- a/src/support.c
+++ b/src/support.c
@@ -107,12 +107,12 @@ __write_mapfile(pam_handle_t * pamh, const char *user, uid_t uid,
 
 	}
 
-	/*  won't hurt if it already exists, no more overhead than stat() first */
+	/*  won't hurt if it already exists, same overhead as stat() first */
 	mkdir(mapdir, 0755);
 	snprintf(tmpstr, sizeof tmpstr, "%s/%u", mapdir, session);
 	/*
-	 * Only create if it doesn't exist.  It might exist if we are called from
-	 * su or sudo after a login, for example
+	 * Only create if it doesn't exist.  It might exist if we are called
+	 * from su or sudo after a login, for example
 	 */
 	f = fopen(tmpstr, "wx");
 	if (!f) {