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.

diff --git a/CHANGELOG b/CHANGELOG
index b3cc17dbe6f5a94a70ff5a705170987820045004..8f30e0205bc7324c3ba206c97186761d8a14e5bb 100644 (file)
--- 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 8c07a8b77eb64dac6c7fb13961fa4a4879c9630b..85841923c717e8a31510fdb80b5f15e0fe76923a 100644 (file)
--- 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,12 +55,16 @@ 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.
@@ -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 336bb31ef1333b055eedc69d0ede37bd11ca6d84..b75689e8f3faeb2affbc6921e233db449398034d 100644 (file)
--- 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 c460abc972e67864534e2f06f92786bd685d03d8..d127791ba70d40fb526d4fa4ad783950d57df77c 100644 (file)
--- 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 --- */