[PATCH v3 0/4] Add support for using libuser to chsh and chfn

[PATCH v3 0/4] Add support for using libuser to chsh and chfn

[Cody Maloney]

I believe these are finally ready for merging. Freshly rebased.
Patches 3 and 4 could be merged, but I think it's a little 
cleaner to read/review with the changes seperate.

Version 3 of the patches, incorporating all changes suggested to 
this point. Only changes from v2 are fixing the error string and
dropping initgroups in patch 3/4.

Tested:
Building and running both with and without libuser support enabled,
with a valid password, invalid password, and running as root.

Cody Maloney (4):
  chsh-chfn: Add flag for enabling/disabling libuser support.
  chsh-chfn: Move pam auth to its own function, factoring out common
    code
  chsh: Add libuser support
  chfn: Add libuser support

 configure.ac              | 17 ++++++++++
 login-utils/Makemodule.am | 10 ++++++
 login-utils/auth.c        | 47 ++++++++++++++++++++++++++++
 login-utils/auth.h        | 13 ++++++++
 login-utils/chfn.c        | 58 +++++++++++++++-------------------
 login-utils/chsh.c        | 53 ++++++++++++++-----------------
 login-utils/libuser.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 login-utils/libuser.h     | 14 +++++++++
 8 files changed, 229 insertions(+), 63 deletions(-)
 create mode 100644 login-utils/auth.c
 create mode 100644 login-utils/auth.h
 create mode 100644 login-utils/libuser.c
 create mode 100644 login-utils/libuser.h

-- 
1.8.1

[PATCH v3 1/4] chsh-chfn: Add flag for enabling/disabling libuser support.

[Cody Maloney]

Signed-off-by: Cody Maloney <cmaloney@theoreticalchaos.com>
---
 configure.ac              | 17 +++++++++++++++++
 login-utils/Makemodule.am |  5 +++++
 2 files changed, 22 insertions(+)

diff --git a/configure.ac b/configure.ac
index d26a686..cf6e922 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1126,6 +1126,23 @@ AS_HELP_STRING([--disable-chsh-only-listed], [chsh: allow shells not in /etc/she
 [], enable_chsh_only_listed=yes
 )
 
+AC_ARG_WITH([libuser], AS_HELP_STRING([--without-libuser], [compile without libuser (remote chsh)]),
+  [], with_user=auto
+)
+
+if test "x$with_user" = xno; then
+  AM_CONDITIONAL(HAVE_USER, false)
+else
+  PKG_CHECK_MODULES(LIBUSER,[libuser >= 0.58])
+  UL_CHECK_LIB(user, lu_start)
+  case "$with_user:$have_user" in
+  yes:no)
+   AC_MSG_ERROR([user selected but libuser not found])
+   ;;
+  esac
+fi
+
+
 if test "x$enable_chsh_only_listed" = xyes; then
 AC_DEFINE(ONLY_LISTED_SHELLS, 1, [Should chsh allow only shells in /etc/shells?])
 fi
diff --git a/login-utils/Makemodule.am b/login-utils/Makemodule.am
index aef8177..479b87b 100644
--- a/login-utils/Makemodule.am
+++ b/login-utils/Makemodule.am
@@ -68,6 +68,11 @@ chfn_chsh_cflags = $(SUID_CFLAGS) $(AM_CFLAGS)
 chfn_chsh_ldflags = $(SUID_LDFLAGS) $(AM_LDFLAGS)
 chfn_chsh_ldadd = libcommon.la -lpam -lpam_misc
 
+if HAVE_USER
+chfn_chsh_ldflags += $(LIBUSER_LIBS)
+chfn_chsh_cflags += $(LIBUSER_CFLAGS)
+endif
+
 if HAVE_SELINUX
 chfn_chsh_sources += \
 	login-utils/selinux_utils.c \
-- 
1.8.1

[PATCH v3 2/4] chsh-chfn: Move pam auth to its own function, factoring out common code

[Cody Maloney]

This makes it easier to add support for libuser, which needs the same PAM
authentication. Also removes duplicate code between chsh and chfn.

Signed-off-by: Cody Maloney <cmaloney@theoreticalchaos.com>
---
 login-utils/Makemodule.am |  2 ++
 login-utils/auth.c        | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 login-utils/auth.h        | 13 +++++++++++++
 login-utils/chfn.c        | 33 +++------------------------------
 login-utils/chsh.c        | 33 +++------------------------------
 5 files changed, 68 insertions(+), 60 deletions(-)
 create mode 100644 login-utils/auth.c
 create mode 100644 login-utils/auth.h

diff --git a/login-utils/Makemodule.am b/login-utils/Makemodule.am
index 479b87b..ee85329 100644
--- a/login-utils/Makemodule.am
+++ b/login-utils/Makemodule.am
@@ -62,6 +62,8 @@ dist_man_MANS += \
 chfn_chsh_sources = \
 	login-utils/islocal.c \
 	login-utils/islocal.h \
+	login-utils/auth.c \
+	login-utils/auth.h \
 	login-utils/setpwnam.c \
 	login-utils/setpwnam.h
 chfn_chsh_cflags = $(SUID_CFLAGS) $(AM_CFLAGS)
diff --git a/login-utils/auth.c b/login-utils/auth.c
new file mode 100644
index 0000000..373bd22
--- /dev/null
+++ b/login-utils/auth.c
@@ -0,0 +1,47 @@
+/*
+ *   auth.c -- PAM authorization code, common between chsh and chfn
+ *   (c) 2012 by Cody Maloney <cmaloney@theoreticalchaos.com>
+ *
+ *   this program is free software.  you can redistribute it and
+ *   modify it under the terms of the gnu general public license.
+ *   there is no warranty.
+ *
+ */
+
+#include "auth.h"
+
+#include "pamfail.h"
+
+int auth_pam(const char *service_name, uid_t uid, const char *username) {
+#ifdef REQUIRE_PASSWORD
+	if (uid != 0) {
+		pam_handle_t *pamh = NULL;
+		struct pam_conv conv = { misc_conv, NULL };
+		int retcode;
+
+		retcode = pam_start(service_name, username, &conv, &pamh);
+		if (pam_fail_check(pamh, retcode))
+			return FALSE;
+
+		retcode = pam_authenticate(pamh, 0);
+		if (pam_fail_check(pamh, retcode))
+			return FALSE;
+
+		retcode = pam_acct_mgmt(pamh, 0);
+		if (retcode == PAM_NEW_AUTHTOK_REQD)
+			retcode =
+			    pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK);
+		if (pam_fail_check(pamh, retcode))
+			return FALSE;
+
+		retcode = pam_setcred(pamh, 0);
+		if (pam_fail_check(pamh, retcode))
+			return FALSE;
+
+		pam_end(pamh, 0);
+		/* no need to establish a session; this isn't a
+		 * session-oriented activity...  */
+	}
+	return TRUE;
+#endif	/* REQUIRE_PASSWORD */
+}
diff --git a/login-utils/auth.h b/login-utils/auth.h
new file mode 100644
index 0000000..bf7c369
--- /dev/null
+++ b/login-utils/auth.h
@@ -0,0 +1,13 @@
+/*
+ *   auth.h -- PAM authorization code, common between chsh and chfn
+ *   (c) 2012 by Cody Maloney <cmaloney@theoreticalchaos.com>
+ *
+ *   this program is free software.  you can redistribute it and
+ *   modify it under the terms of the gnu general public license.
+ *   there is no warranty.
+ *
+ */
+
+#include <sys/types.h>
+
+extern int auth_pam(const char *service_name, uid_t uid, const char *username);
diff --git a/login-utils/chfn.c b/login-utils/chfn.c
index b240ec7..7c9af84 100644
--- a/login-utils/chfn.c
+++ b/login-utils/chfn.c
@@ -31,12 +31,12 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "auth.h"
 #include "c.h"
 #include "env.h"
 #include "closestream.h"
 #include "islocal.h"
 #include "nls.h"
-#include "pamfail.h"
 #include "setpwnam.h"
 #include "strutils.h"
 #include "xalloc.h"
@@ -157,36 +157,9 @@ int main(int argc, char **argv)
 
 	printf(_("Changing finger information for %s.\n"), oldf.username);
 
-#ifdef REQUIRE_PASSWORD
-	if (uid != 0) {
-		pam_handle_t *pamh = NULL;
-		struct pam_conv conv = { misc_conv, NULL };
-		int retcode;
-
-		retcode = pam_start("chfn", oldf.username, &conv, &pamh);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		retcode = pam_authenticate(pamh, 0);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		retcode = pam_acct_mgmt(pamh, 0);
-		if (retcode == PAM_NEW_AUTHTOK_REQD)
-			retcode =
-			    pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		retcode = pam_setcred(pamh, 0);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		pam_end(pamh, 0);
-		/* no need to establish a session; this isn't a
-		 * session-oriented activity...  */
+	if(!auth_pam("chfn", uid, oldf.username)) {
+		return EXIT_FAILURE;
 	}
-#endif	/* REQUIRE_PASSWORD */
 
 	if (interactive)
 		ask_info(&oldf, &newf);
diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index f83b057..7d3963f 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -32,12 +32,12 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "auth.h"
 #include "c.h"
 #include "env.h"
 #include "closestream.h"
 #include "islocal.h"
 #include "nls.h"
-#include "pamfail.h"
 #include "pathnames.h"
 #include "setpwnam.h"
 #include "xalloc.h"
@@ -147,36 +147,9 @@ int main(int argc, char **argv)
 
 	printf(_("Changing shell for %s.\n"), pw->pw_name);
 
-#ifdef REQUIRE_PASSWORD
-	if (uid != 0) {
-		pam_handle_t *pamh = NULL;
-		struct pam_conv conv = { misc_conv, NULL };
-		int retcode;
-
-		retcode = pam_start("chsh", pw->pw_name, &conv, &pamh);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		retcode = pam_authenticate(pamh, 0);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		retcode = pam_acct_mgmt(pamh, 0);
-		if (retcode == PAM_NEW_AUTHTOK_REQD)
-			retcode =
-			    pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		retcode = pam_setcred(pamh, 0);
-		if (pam_fail_check(pamh, retcode))
-			return EXIT_FAILURE;
-
-		pam_end(pamh, 0);
-		/* no need to establish a session; this isn't a
-		 * session-oriented activity...  */
+	if(!auth_pam("chsh", uid, pw->pw_name)) {
+		return EXIT_FAILURE;
 	}
-#endif	/* REQUIRE_PASSWORD */
 
 	if (!shell) {
 		shell = prompt(_("New shell"), oldshell);
-- 
1.8.1

[PATCH v3 3/4] chsh: Add libuser support

[Cody Maloney]

This is based directly on lchsh which is a part of libuser. libuser.{c,h}
exist because exactly the same code is needed for both chsh and chfn.

Signed-off-by: Cody Maloney <cmaloney@theoreticalchaos.com>
---
 login-utils/Makemodule.am |  3 ++
 login-utils/chsh.c        | 22 ++++++++++++-
 login-utils/libuser.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 login-utils/libuser.h     | 14 +++++++++
 4 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 login-utils/libuser.c
 create mode 100644 login-utils/libuser.h

diff --git a/login-utils/Makemodule.am b/login-utils/Makemodule.am
index ee85329..0c57118 100644
--- a/login-utils/Makemodule.am
+++ b/login-utils/Makemodule.am
@@ -73,6 +73,9 @@ chfn_chsh_ldadd = libcommon.la -lpam -lpam_misc
 if HAVE_USER
 chfn_chsh_ldflags += $(LIBUSER_LIBS)
 chfn_chsh_cflags += $(LIBUSER_CFLAGS)
+chfn_chsh_sources+= \
+	login-utils/libuser.c \
+	login-utils/libuser.h
 endif
 
 if HAVE_SELINUX
diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 7d3963f..66800ca 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -1,6 +1,7 @@
 /*
  *   chsh.c -- change your login shell
  *   (c) 1994 by salvatore valente <svalente@athena.mit.edu>
+ *   (c) 2012 by Cody Maloney <cmaloney@theoreticalchaos.com>
  *
  *   this program is free software.  you can redistribute it and
  *   modify it under the terms of the gnu general public license.
@@ -32,7 +33,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "auth.h"
 #include "c.h"
 #include "env.h"
 #include "closestream.h"
@@ -48,6 +48,14 @@
 # include "selinux_utils.h"
 #endif
 
+
+#ifdef HAVE_LIBUSER
+# include <libuser/user.h>
+# include "libuser.h"
+#else
+# include "auth.h"
+#endif
+
 struct sinfo {
 	char *username;
 	char *shell;
@@ -131,7 +139,12 @@ int main(int argc, char **argv)
 		oldshell = _PATH_BSHELL;	/* default */
 
 	/* reality check */
+#ifdef HAVE_LIBUSER
+	/* If we're setuid and not really root, disallow the password change. */
+	if (geteuid() != getuid() && uid != pw->pw_uid) {
+#else
 	if (uid != 0 && uid != pw->pw_uid) {
+#endif
 		errno = EACCES;
 		err(EXIT_FAILURE,
 		    _("running UID doesn't match UID of user we're "
@@ -147,9 +160,11 @@ int main(int argc, char **argv)
 
 	printf(_("Changing shell for %s.\n"), pw->pw_name);
 
+#ifndef HAVE_LIBUSER
 	if(!auth_pam("chsh", uid, pw->pw_name)) {
 		return EXIT_FAILURE;
 	}
+#endif
 
 	if (!shell) {
 		shell = prompt(_("New shell"), oldshell);
@@ -162,10 +177,15 @@ int main(int argc, char **argv)
 
 	if (strcmp(oldshell, shell) == 0)
 		errx(EXIT_SUCCESS, _("Shell not changed."));
+
+#ifdef HAVE_LIBUSER
+	set_value_libuser("chsh", pw->pw_name, uid, LU_LOGINSHELL, shell);
+#else
 	pw->pw_shell = shell;
 	if (setpwnam(pw) < 0)
 		err(EXIT_FAILURE, _("setpwnam failed\n"
 			"Shell *NOT* changed.  Try again later."));
+#endif
 
 	printf(_("Shell changed.\n"));
 	return EXIT_SUCCESS;
diff --git a/login-utils/libuser.c b/login-utils/libuser.c
new file mode 100644
index 0000000..0501d1a
--- /dev/null
+++ b/login-utils/libuser.c
@@ -0,0 +1,80 @@
+/*
+ *   libuser.c -- Utilize libuser to set a user attribute
+ *   (c) 2012 by Cody Maloney <cmaloney@theoreticalchaos.com>
+ *
+ *   this program is free software.  you can redistribute it and
+ *   modify it under the terms of the gnu general public license.
+ *   there is no warranty.
+ *
+ */
+
+#include "libuser.h"
+
+#include <grp.h>
+#include <libuser/user.h>
+#include <unistd.h>
+
+#include "auth.h"
+#include "c.h"
+#include "nls.h"
+
+static int auth_lu(const char *service_name, struct lu_context *ctx, uid_t uid,
+			const char *username);
+
+static int auth_lu(const char *service_name, struct lu_context *ctx, uid_t uid,
+			const char *username) {
+	if(!lu_uses_elevated_privileges(ctx)) {
+		/* Drop privileges */
+		if (setegid(getgid()) == -1) {
+			errx(EXIT_FAILURE, _("Couldn't drop group privileges"));
+			return FALSE;
+		}
+		if (seteuid(getuid()) == -1) {
+			errx(EXIT_FAILURE, _("Couldn't drop group privileges"));
+			return FALSE;
+		}
+		return TRUE;
+	}
+
+	return auth_pam(service_name, uid, username);
+}
+
+int set_value_libuser(const char *service_name, const char *username, uid_t uid,
+			const char *attr, const char *val) {
+	struct lu_context *ctx;
+	struct lu_error *error = NULL;
+	struct lu_ent *ent;
+
+	ctx = lu_start(username, lu_user, NULL, NULL, lu_prompt_console_quiet,
+			NULL, &error);
+	if (ctx == NULL) {
+		err(EXIT_FAILURE, _("Error initializing %s: %s.\n"), PACKAGE,
+			lu_strerror(error));
+		return FALSE;
+	}
+
+	if(!auth_lu(service_name, ctx, uid, username)) {
+		errno = EACCES;
+		err(EXIT_FAILURE, _("Permisison denied for changing user attribute"));
+		return FALSE;
+	}
+
+	/* Look up the user's record. */
+	ent = lu_ent_new();
+	if (lu_user_lookup_name(ctx, username, ent, &error) == FALSE) {
+		lu_end(ctx);
+		err(EXIT_FAILURE, _("user \"%s\" does not exist."), username);
+		return FALSE;
+	}
+
+	lu_ent_set_string(ent, attr, val);
+	if (!lu_user_modify(ctx, ent, &error)) {
+		lu_ent_free(ent);
+		lu_end(ctx);
+		err(EXIT_FAILURE, _("User attribute not changed: %s\n"), lu_strerror(error));
+		return FALSE;
+	}
+	lu_ent_free(ent);
+	lu_end(ctx);
+	return TRUE;
+}
diff --git a/login-utils/libuser.h b/login-utils/libuser.h
new file mode 100644
index 0000000..7454b99
--- /dev/null
+++ b/login-utils/libuser.h
@@ -0,0 +1,14 @@
+/*
+ *   libuser.h -- Utilize libuser to set a user attribute
+ *   (c) 2012 by Cody Maloney <cmaloney@theoreticalchaos.com>
+ *
+ *   this program is free software.  you can redistribute it and
+ *   modify it under the terms of the gnu general public license.
+ *   there is no warranty.
+ *
+ */
+
+#include <sys/types.h>
+
+extern int set_value_libuser(const char *service_name, const char *username,
+			uid_t uid, const char *attr, const char *val);
-- 
1.8.1

[PATCH v3 4/4] chfn: Add libuser support

[Cody Maloney]

Signed-off-by: Cody Maloney <cmaloney@theoreticalchaos.com>
---
 login-utils/chfn.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/login-utils/chfn.c b/login-utils/chfn.c
index 7c9af84..7ea3f3e 100644
--- a/login-utils/chfn.c
+++ b/login-utils/chfn.c
@@ -1,6 +1,7 @@
 /*
  *   chfn.c -- change your finger information
  *   (c) 1994 by salvatore valente <svalente@athena.mit.edu>
+ *   (c) 2012 by Cody Maloney <cmaloney@theoreticalchaos.com>
  *
  *   this program is free software.  you can redistribute it and
  *   modify it under the terms of the gnu general public license.
@@ -31,7 +32,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "auth.h"
 #include "c.h"
 #include "env.h"
 #include "closestream.h"
@@ -47,6 +47,13 @@
 # include "selinux_utils.h"
 #endif
 
+#ifdef HAVE_LIBUSER
+# include <libuser/user.h>
+# include "libuser.h"
+#else
+# include "auth.h"
+#endif
+
 static char buf[1024];
 
 struct finfo {
@@ -149,17 +156,24 @@ int main(int argc, char **argv)
 	}
 #endif
 
-	/* Reality check */
-	if (uid != 0 && uid != oldf.pw->pw_uid) {
+#ifdef HAVE_LIBUSER
+	/* If we're setuid and not really root, disallow the password change. */
+	if (geteuid() != getuid() && uid != pw->pw_uid) {
+#else
+	if (uid != 0 && uid != pw->pw_uid) {
+#endif
 		errno = EACCES;
-		err(EXIT_FAILURE, NULL);
+		err(EXIT_FAILURE, _("running UID doesn't match UID of user we're "
+		      "altering, change denied")););
 	}
 
 	printf(_("Changing finger information for %s.\n"), oldf.username);
 
+#ifndef HAVE_LIBUSER
 	if(!auth_pam("chfn", uid, oldf.username)) {
 		return EXIT_FAILURE;
 	}
+#endif
 
 	if (interactive)
 		ask_info(&oldf, &newf);
@@ -445,9 +459,14 @@ static int save_new_data(struct finfo *pinfo)
 		gecos[len] = 0;
 	}
 
+#ifdef HAVE_LIBUSER
+	if(set_value_libuser("chfn", pinfo->pw->pw_name, pinfo->pw->pw_uid,
+			LU_GECOS, gecos)) {
+#else /* HAVE_LIBUSER */
 	/* write the new struct passwd to the passwd file. */
 	pinfo->pw->pw_gecos = gecos;
 	if (setpwnam(pinfo->pw) < 0) {
+#endif
 		warn("setpwnam");
 		printf(_
 		       ("Finger information *NOT* changed.  Try again later.\n"));
-- 
1.8.1

Re: [PATCH v3 1/4] chsh-chfn: Add flag for enabling/disabling libuser support.

[Karel Zak]

On Wed, Feb 06, 2013 at 11:22:18PM -0700, Cody Maloney wrote:
>  configure.ac              | 17 +++++++++++++++++
>  login-utils/Makemodule.am |  5 +++++
>  2 files changed, 22 insertions(+)

 Fixed and applied.

> +AC_ARG_WITH([libuser], AS_HELP_STRING([--without-libuser], [compile without libuser (remote chsh)]),
> +  [], with_user=auto
> +)

 you have to use the same "with_" variable name as you have in AC_ARG_WITH([libuser]

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v3 2/4] chsh-chfn: Move pam auth to its own function, factoring out common code

[Karel Zak]

On Wed, Feb 06, 2013 at 11:22:19PM -0700, Cody Maloney wrote:
>  login-utils/Makemodule.am |  2 ++
>  login-utils/auth.c        | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  login-utils/auth.h        | 13 +++++++++++++
>  login-utils/chfn.c        | 33 +++------------------------------
>  login-utils/chsh.c        | 33 +++------------------------------
>  5 files changed, 68 insertions(+), 60 deletions(-)
>  create mode 100644 login-utils/auth.c
>  create mode 100644 login-utils/auth.h

 Fixed and applied.

> +int auth_pam(const char *service_name, uid_t uid, const char *username) {
> +#ifdef REQUIRE_PASSWORD
> +	if (uid != 0) {
> +		pam_handle_t *pamh = NULL;
> +		struct pam_conv conv = { misc_conv, NULL };
> +		int retcode;
> +
> +		retcode = pam_start(service_name, username, &conv, &pamh);
> +		if (pam_fail_check(pamh, retcode))
> +			return FALSE;
> +
> +		retcode = pam_authenticate(pamh, 0);
> +		if (pam_fail_check(pamh, retcode))
> +			return FALSE;
> +
> +		retcode = pam_acct_mgmt(pamh, 0);
> +		if (retcode == PAM_NEW_AUTHTOK_REQD)
> +			retcode =
> +			    pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK);
> +		if (pam_fail_check(pamh, retcode))
> +			return FALSE;
> +
> +		retcode = pam_setcred(pamh, 0);
> +		if (pam_fail_check(pamh, retcode))
> +			return FALSE;
> +
> +		pam_end(pamh, 0);
> +		/* no need to establish a session; this isn't a
> +		 * session-oriented activity...  */
> +	}
> +	return TRUE;
> +#endif	/* REQUIRE_PASSWORD */
> +}

 what happen if REQUIRE_PASSWORD is not defined? 

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v3 3/4] chsh: Add libuser support

[Karel Zak]

On Wed, Feb 06, 2013 at 11:22:20PM -0700, Cody Maloney wrote:
>  login-utils/Makemodule.am |  3 ++
>  login-utils/chsh.c        | 22 ++++++++++++-
>  login-utils/libuser.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++
>  login-utils/libuser.h     | 14 +++++++++
>  4 files changed, 118 insertions(+), 1 deletion(-)
>  create mode 100644 login-utils/libuser.c
>  create mode 100644 login-utils/libuser.h

 Fixed and applied.

> +static int auth_lu(const char *service_name, struct lu_context *ctx, uid_t uid,
> +			const char *username) {
> +	if(!lu_uses_elevated_privileges(ctx)) {
> +		/* Drop privileges */
> +		if (setegid(getgid()) == -1) {
> +			errx(EXIT_FAILURE, _("Couldn't drop group privileges"));
> +			return FALSE;
> +		}

 errx() and err() are no-return functions ;-)

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v3 4/4] chfn: Add libuser support

[Karel Zak]

On Wed, Feb 06, 2013 at 11:22:21PM -0700, Cody Maloney wrote:
> @@ -149,17 +156,24 @@ int main(int argc, char **argv)
>  	}
>  #endif
>  
> -	/* Reality check */
> -	if (uid != 0 && uid != oldf.pw->pw_uid) {
                           ^^^^^^^^^^^^^^^
> +#ifdef HAVE_LIBUSER
> +	/* If we're setuid and not really root, disallow the password change. */
> +	if (geteuid() != getuid() && uid != pw->pw_uid) {
> +#else
> +	if (uid != 0 && uid != pw->pw_uid) {
> +#endif

 Copy & past from chsh, right? 
 
login-utils/chfn.c: In function ‘main’:
login-utils/chfn.c:161:38: error: ‘pw’ undeclared (first use in this function)
login-utils/chfn.c:161:38: note: each undeclared identifier is
reported only once for each function it appears in

>  		errno = EACCES;
> -		err(EXIT_FAILURE, NULL);
> +		err(EXIT_FAILURE, _("running UID doesn't match UID of user we're "
> +		      "altering, change denied")););
                                       ^^^
>  	}

login-utils/chfn.c:167:37: error: expected statement before ‘)’ token


Man, that's suid binary... 

Fixed and applied.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v3 2/4] chsh-chfn: Move pam auth to its own function, factoring out common code

[Cody Maloney]

On Wed, Feb 13, 2013 at 5:53 AM, Karel Zak <kzak@redhat.com> wrote:
>
>  what happen if REQUIRE_PASSWORD is not defined?
>
>     Karel

The file will still be compiled (and linked against) PAM and the code
path operates as it did previously. This does make it simple though if
someone wants to remove the dependency of PAM / compile without it.

Cody

Re: [PATCH v3 4/4] chfn: Add libuser support

[Cody Maloney]

On Wed, Feb 13, 2013 at 6:15 AM, Karel Zak <kzak@redhat.com> wrote:
> On Wed, Feb 06, 2013 at 11:22:21PM -0700, Cody Maloney wrote:
>> @@ -149,17 +156,24 @@ int main(int argc, char **argv)
>>       }
>>  #endif
>>
>> -     /* Reality check */
>> -     if (uid != 0 && uid != oldf.pw->pw_uid) {
>                            ^^^^^^^^^^^^^^^
>> +#ifdef HAVE_LIBUSER
>> +     /* If we're setuid and not really root, disallow the password change. */
>> +     if (geteuid() != getuid() && uid != pw->pw_uid) {
>> +#else
>> +     if (uid != 0 && uid != pw->pw_uid) {
>> +#endif
>
>  Copy & past from chsh, right?
>
> login-utils/chfn.c: In function ‘main’:
> login-utils/chfn.c:161:38: error: ‘pw’ undeclared (first use in this function)
> login-utils/chfn.c:161:38: note: each undeclared identifier is
> reported only once for each function it appears in

Yes it was (And in turn based on lchsh)
>
>>               errno = EACCES;
>> -             err(EXIT_FAILURE, NULL);
>> +             err(EXIT_FAILURE, _("running UID doesn't match UID of user we're "
>> +                   "altering, change denied")););
>                                        ^^^
>>       }
>
> login-utils/chfn.c:167:37: error: expected statement before ‘)’ token
>
>
> Man, that's suid binary...
>
> Fixed and applied.
>
>     Karel

Thanks

Cody