Relevant BUGIDs: 476951, 476953

Purpose of commit: bugfix

Commit summary:
---------------
Be more careful when using the deny option - pay attention to the trust
option before you grant access.
Fix from Nalin.

4 files changed, 48 insertions, 26 deletions

diff --git a/CHANGELOG b/CHANGELOG
index b3cc17db..8f30e020 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -55,9 +55,12 @@ bug report - outstanding bugs are listed here:
 0.77: please submit patches for this section with actual code/doc
       patches!
 
+* pam_wheel was too aggressive to grant access (in the case of the
+  'deny' option you want to pay attention to 'trust'). Fix from
+  Nalin (Bugs 476951, 476953 - agmorgan)
 * account management support for: pam_shells, pam_listfile, pam_wheel
   and pam_securetty (+ static module fix for pam_nologin). Patch from
-  redhat through Harrold Welte (Bug 436435 - agmorgan).
+  redhat through Harald Welte (Bug 436435 - agmorgan).
 * pam_wheel feature from Nalin - can use the module to provide wheel
   access to non-root accounts. Also from Nalin, a bugfix related to
   the primary group of the applicant is the 'wheel' group. (Bugs
diff --git a/doc/modules/pam_wheel.sgml b/doc/modules/pam_wheel.sgml
index 8c07a8b7..85841923 100644
--- a/doc/modules/pam_wheel.sgml
+++ b/doc/modules/pam_wheel.sgml
@@ -22,7 +22,7 @@ Cristian Gafton <gafton@redhat.com>
 Author.
 
 <tag><bf>Management groups provided:</bf></tag>
-authentication
+authentication; account
 
 <tag><bf>Cryptographically sensitive:</bf></tag>
 	
@@ -31,7 +31,6 @@ authentication
 <tag><bf>Clean code base:</bf></tag>
 
 <tag><bf>System dependencies:</bf></tag>
-Requires libpwdb.
 
 <tag><bf>Network aware:</bf></tag>
 
@@ -42,7 +41,7 @@ Requires libpwdb.
 <p>
 Only permit root access to members of the wheel (<tt/gid=0/) group.
 
-<sect2>Authentication component
+<sect2>Authentication and Account components
 
 <p>
 <descrip>
@@ -56,13 +55,17 @@ Only permit root access to members of the wheel (<tt/gid=0/) group.
 
 <tag><bf>Description:</bf></tag>
 
-This module is used to enforce the so-called <em/wheel/ group. By
+This module is used to enforce the so-called <em/wheel/ group.  By
 default, it permits root access to the system if the applicant user is
 a member of the <tt/wheel/ group (first, the module checks for the
 existence of a '<tt/wheel/' group. Otherwise the module defines the
 group with group-id <tt/0/ to be the <em/wheel/ group).
 
 <p>
+The module can be used as either an '<tt/auth/' or an '<tt/account/'
+module.
+
+<p>
 The action of the module may be modified from this default by one or
 more of the following flags in the <tt>/etc/pam.conf</tt> file.
 <itemize>
@@ -88,10 +91,13 @@ password. <bf/USE WITH CARE/.
 
 <item>
 <tt/deny/ - 
-This is used to reverse the logic of the module's behavior.
-If the user is trying to get <tt/uid=0/ access and is a member of the wheel
+This is used to reverse the logic of the module's behavior.  If the
+user is trying to get <tt/uid=0/ access and is a member of the wheel
 group, deny access (for the wheel group, this is perhaps nonsense!):
 it is intended for use in conjunction with the <tt/group=/ argument...
+Conversely, if the user is not in the group, return <tt/PAM_IGNORE/
+(unless <tt/trust/ was also specified, in which case we return
+<tt/PAM_SUCCESS/).
 
 <item>
 <tt/group=XXXX/ -
@@ -114,7 +120,7 @@ file:
 #
 su	auth	 sufficient	pam_rootok.so
 su	auth	 required	pam_wheel.so
-su	auth	 required	pam_unix_auth.so
+su	auth	 required	pam_unix.so
 </verb>
 </tscreen>
 
diff --git a/modules/pam_wheel/README b/modules/pam_wheel/README
index 336bb31e..b75689e8 100644
--- a/modules/pam_wheel/README
+++ b/modules/pam_wheel/README
@@ -1,6 +1,6 @@
 
 pam_wheel:
-	only permit root authentication too members of wheel group
+	only permit root authentication to members of wheel group
 
 RECOGNIZED ARGUMENTS:
 	debug		write a message to syslog indicating success or
@@ -21,13 +21,16 @@ RECOGNIZED ARGUMENTS:
 			is trying to get UID 0 access and is a member of the
 			wheel group, deny access (well, kind of nonsense, but
 			for use in conjunction with 'group' argument... :-)
+	                Conversely, if the user is not in the group, return
+                        PAM_IGNORE (unless 'trust' was also specified, in
+                        which case we return PAM_SUCCESS).
 
 	group=xxxx	Instead of checking the GID 0 group, use the xxxx
 			group to perform the authentification.
 
 MODULE SERVICES PROVIDED:
-	auth		_authetication and _setcred (blank)
+	auth		_authentication, _setcred (blank) and _acct_mgmt
 
 AUTHOR:
-	Cristian Gafton <gafton@sorosis.ro>
+	Cristian Gafton <gafton@redhat.com>
 
diff --git a/modules/pam_wheel/pam_wheel.c b/modules/pam_wheel/pam_wheel.c
index c460abc9..d127791b 100644
--- a/modules/pam_wheel/pam_wheel.c
+++ b/modules/pam_wheel/pam_wheel.c
@@ -192,33 +192,43 @@ static int perform_check(pam_handle_t *pamh, int flags, int ctrl,
 
     if (is_on_list(grp->gr_mem, fromsu) || (tpwd->pw_gid == grp->gr_gid)) {
 
-	if (ctrl & PAM_DEBUG_ARG) {
-	    _pam_log(LOG_NOTICE,"Access %s to '%s' for '%s'",
-		     (ctrl & PAM_DENY_ARG)?"denied":"granted",
-		     fromsu,username);
+	if (ctrl & PAM_DENY_ARG) {
+	    retval = PAM_PERM_DENIED;
+
+	} else if (ctrl & PAM_TRUST_ARG) {
+	    retval = PAM_SUCCESS;        /* this can be a sufficient check */
+
+	} else {
+	    retval = PAM_IGNORE;
 	}
 
+    } else {
+
 	if (ctrl & PAM_DENY_ARG) {
-	    return PAM_PERM_DENIED;
-	} else {
+
 	    if (ctrl & PAM_TRUST_ARG) {
-		return PAM_SUCCESS;    /* this can be a sufficient check */
+		retval = PAM_SUCCESS;    /* this can be a sufficient check */
 	    } else {
-		return PAM_IGNORE;
+		retval = PAM_IGNORE;
 	    }
+
+	} else {
+	    retval = PAM_PERM_DENIED;
 	}
     }
 
     if (ctrl & PAM_DEBUG_ARG) {
-	_pam_log(LOG_NOTICE,"Access %s for '%s' to '%s'",
-		 (ctrl & PAM_DENY_ARG)?"granted":"denied",fromsu,username);
+	if (retval == PAM_IGNORE) {
+	    _pam_log(LOG_NOTICE, "Ignoring access request '%s' for '%s'",
+		     fromsu, username);
+	} else {
+	    _pam_log(LOG_NOTICE, "Access %s to '%s' for '%s'",
+		     (retval != PAM_SUCCESS) ? "denied":"granted",
+		     fromsu, username);
+	}
     }
 
-    if (ctrl & PAM_DENY_ARG) {
-	return PAM_SUCCESS;            /* this can be a sufficient check */
-    } else {
-	return PAM_PERM_DENIED;
-    }
+    return retval;
 }
 
 /* --- authentication management functions --- */