* src/groupadd.c: Only call gr_unlock() and sgr_unlock() in the

	group or gshadow files were previously locked.
	* src/groupadd.c: Make sure failures are reported to syslog/audit
	after the change is mentioned.
	* src/groupmod.c: Add logging to syslog & audit on lock/unlock
	failures.
	* src/groupmod.c: Make sure issues are reported to syslog or audit
	after the change is mentioned.
	* src/groupdel.c: Only call gr_unlock() and sgr_unlock() in the
	group or gshadow files were previously locked.
	* src/groupdel.c: Simplify the handling of PAM errors.

diff --git a/ChangeLog b/ChangeLog
index 7c8a0f3a96e737f661275e2d79af7d8118c0b06f..1e7cb1537201ac7e95dd871bb6cff584ca3f9ea9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,11 +1,23 @@
-2008-08-01  Nicolas François  <nicolas.francois@centraliens.net>
+2008-08-02  Nicolas François  <nicolas.francois@centraliens.net>
 
        * src/groupadd.c: Harmonize error & syslog messages.
-       * src/groupadd.c: Add logging to syslog in some error cases.
+       * src/groupadd.c: Add logging to syslog & audit on lock/unlock
+       failures.
+       * src/groupadd.c: Only call gr_unlock() and sgr_unlock() in the
+       group or gshadow files were previously locked.
+       * src/groupadd.c: Make sure failures are reported to syslog/audit
+       after the change is mentioned.
        * src/groupmod.c: Harmonize error & syslog messages.
+       * src/groupmod.c: Add logging to syslog & audit on lock/unlock
+       failures.
+       * src/groupmod.c: Make sure issues are reported to syslog or audit
+       after the change is mentioned.
        * src/groupdel.c: Harmonize error & syslog messages.
        * src/groupdel.c: Add logging to syslog & audit on lock/unlock
        failures.
+       * src/groupdel.c: Only call gr_unlock() and sgr_unlock() in the
+       group or gshadow files were previously locked.
+       * src/groupdel.c: Simplify the handling of PAM errors.
 
 2008-08-01  Nicolas François  <nicolas.francois@centraliens.net>
 

diff --git a/src/groupadd.c b/src/groupadd.c
index e606397cb0c051553b01b22db5df4187e4b66299..a9962ebfe86418469027e75755aab6ec1151deb1 100644 (file)
--- a/src/groupadd.c
+++ b/src/groupadd.c
@@ -53,7 +53,6 @@
 #include "prototypes.h"
 #ifdef SHADOWGRP
 #include "sgroupio.h"
-static bool is_shadow_grp;
 #endif
 
 /*
@@ -82,6 +81,13 @@ static bool fflg = false;    /* if group already exists, do nothing and exit(0) */
 static bool rflg = false;      /* create a system account */
 static bool pflg = false;      /* new encrypted password */
 
+#ifdef SHADOWGRP
+static bool is_shadow_grp;
+static bool gshadow_locked = false;
+#endif
+static bool group_locked = false;
+
+
 #ifdef USE_PAM
 static pam_handle_t *pamh = NULL;
 #endif
@@ -250,16 +256,36 @@ static void close_files (void)
                SYSLOG ((LOG_WARN, "cannot rewrite the group file"));
                fail_exit (E_GRP_UPDATE);
        }
-       gr_unlock ();
-#ifdef SHADOWGRP
-       if (is_shadow_grp && (sgr_close () == 0)) {
-               fprintf (stderr,
-                        _("%s: cannot rewrite the shadow group file\n"), Prog);
-               SYSLOG ((LOG_WARN, "cannot rewrite the shadow group file"));
-               fail_exit (E_GRP_UPDATE);
+       if (gr_unlock () == 0) {
+               fprintf (stderr, _("%s: cannot unlock the group file\n"), Prog);
+               SYSLOG ((LOG_WARN, "cannot unlock the group file"));
+#ifdef WITH_AUDIT
+               audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                             "unlocking group file",
+                             group_name, AUDIT_NO_ID, 0);
+#endif
+               /* continue */
        }
+       group_locked = false;
+#ifdef SHADOWGRP
        if (is_shadow_grp) {
-               sgr_unlock ();
+               if (sgr_close () == 0) {
+                       fprintf (stderr,
+                                _("%s: cannot rewrite the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot rewrite the shadow group file"));
+                       fail_exit (E_GRP_UPDATE);
+               }
+               if (sgr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking gshadow file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
+               gshadow_locked = false;
        }
 #endif                         /* SHADOWGRP */
 }
@@ -279,8 +305,9 @@ static void open_files (void)
                              "locking group file",
                              group_name, AUDIT_NO_ID, 0);
 #endif
-               exit (E_GRP_UPDATE);
+               fail_exit (E_GRP_UPDATE);
        }
+       group_locked = true;
        if (gr_open (O_RDWR) == 0) {
                fprintf (stderr, _("%s: cannot open the group file\n"), Prog);
                SYSLOG ((LOG_WARN, "cannot open the group file"));
@@ -292,17 +319,30 @@ static void open_files (void)
                fail_exit (E_GRP_UPDATE);
        }
 #ifdef SHADOWGRP
-       if (is_shadow_grp && (sgr_lock () == 0)) {
-               fprintf (stderr,
-                        _("%s: cannot lock the shadow group file\n"), Prog);
-               SYSLOG ((LOG_WARN, "cannot lock the shadow group file"));
-               fail_exit (E_GRP_UPDATE);
-       }
-       if (is_shadow_grp && (sgr_open (O_RDWR) == 0)) {
-               fprintf (stderr,
-                        _("%s: cannot open the shadow group file\n"), Prog);
-               SYSLOG ((LOG_WARN, "cannot open the shadow group file"));
-               fail_exit (E_GRP_UPDATE);
+       if (is_shadow_grp) {
+               if (sgr_lock () == 0) {
+                       fprintf (stderr,
+                                _("%s: cannot lock the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot lock the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "locking gshadow file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       fail_exit (E_GRP_UPDATE);
+               }
+               gshadow_locked = true;
+               if (sgr_open (O_RDWR) == 0) {
+                       fprintf (stderr,
+                                _("%s: cannot open the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot open the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "opening gshadow file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       fail_exit (E_GRP_UPDATE);
+               }
        }
 #endif                         /* SHADOWGRP */
 }
@@ -312,10 +352,30 @@ static void open_files (void)
  */
 static void fail_exit (int code)
 {
-       (void) gr_unlock ();
+       if (group_locked) {
+               if (gr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking group file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
+       }
 #ifdef SHADOWGRP
-       if (is_shadow_grp) {
-               sgr_unlock ();
+       if (gshadow_locked) {
+               if (sgr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking gshadow file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
        }
 #endif
 
@@ -470,7 +530,7 @@ static void check_flags (void)
                        /* OK, no need to do anything */
                        fail_exit (E_SUCCESS);
                }
-               fprintf (stderr, _("%s: group %s exists\n"), Prog, group_name);
+               fprintf (stderr, _("%s: group '%s' already exists\n"), Prog, group_name);
                fail_exit (E_NAME_IN_USE);
        }
 
@@ -487,7 +547,7 @@ static void check_flags (void)
                        /* Turn off -g, we can use any GID */
                        gflg = false;
                } else {
-                       fprintf (stderr, _("%s: GID %u is not unique\n"),
+                       fprintf (stderr, _("%s: GID '%u' already exists\n"),
                                 Prog, (unsigned int) group_id);
                        fail_exit (E_GID_IN_USE);
                }

diff --git a/src/groupdel.c b/src/groupdel.c
index aea6e3805af6883b1a0e5ceabf0d5a992acc1321..d84806aec568540024ca23594f2c23b842d698f0 100644 (file)
--- a/src/groupdel.c
+++ b/src/groupdel.c
@@ -94,17 +94,20 @@ static void usage (void)
  */
 static void fail_exit (int code)
 {
-       if (gr_unlock () == 0) {
-               fprintf (stderr, _("%s: cannot unlock the group file\n"), Prog);
-               SYSLOG ((LOG_WARN, "cannot unlock the group file"));
+       if (group_locked) {
+               if (gr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the group file"));
 #ifdef WITH_AUDIT
-               audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
-                             "unlocking group file",
-                             group_name, AUDIT_NO_ID, 0);
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking group file",
+                                     group_name, AUDIT_NO_ID, 0);
 #endif
+                       /* continue */
+               }
        }
 #ifdef SHADOWGRP
-       if (is_shadow_grp) {
+       if (gshadow_locked) {
                if (sgr_unlock () == 0) {
                        fprintf (stderr, _("%s: cannot unlock the shadow group file\n"), Prog);
                        SYSLOG ((LOG_WARN, "cannot unlock the shadow group file"));
@@ -113,6 +116,7 @@ static void fail_exit (int code)
                                      "unlocking gshadow file",
                                      group_name, AUDIT_NO_ID, 0);
 #endif
+                       /* continue */
                }
        }
 #endif
@@ -170,6 +174,7 @@ static void close_files (void)
 
        if (gr_close () == 0) {
                fprintf (stderr, _("%s: cannot rewrite the group file\n"), Prog);
+               SYSLOG ((LOG_WARN, "cannot rewrite the group file"));
                fail_exit (E_GRP_UPDATE);
        }
        if (gr_unlock () == 0) {
@@ -180,12 +185,15 @@ static void close_files (void)
                              "unlocking group file",
                              group_name, AUDIT_NO_ID, 0);
 #endif
+               /* continue */
        }
+       group_locked = false;
 #ifdef SHADOWGRP
        if (is_shadow_grp) {
                if (sgr_close () == 0)) {
                        fprintf (stderr,
                                 _("%s: cannot rewrite the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot rewrite the shadow group file"));
                        fail_exit (E_GRP_UPDATE);
                }
                if (sgr_unlock () == 0) {
@@ -196,7 +204,9 @@ static void close_files (void)
                                      "unlocking gshadow file",
                                      group_name, AUDIT_NO_ID, 0);
 #endif
+                       /* continue */
                }
+               gshadow_locked = false;
        }
 #endif                         /* SHADOWGRP */
 }
@@ -331,19 +341,14 @@ int main (int argc, char **argv)
 
        if (PAM_SUCCESS == retval) {
                retval = pam_authenticate (pamh, 0);
-               if (PAM_SUCCESS != retval) {
-                       (void) pam_end (pamh, retval);
-               }
        }
 
        if (PAM_SUCCESS == retval) {
                retval = pam_acct_mgmt (pamh, 0);
-               if (PAM_SUCCESS != retval) {
-                       (void) pam_end (pamh, retval);
-               }
        }
 
        if (PAM_SUCCESS != retval) {
+               (void) pam_end (pamh, retval);
                fprintf (stderr, _("%s: PAM authentication failed\n"), Prog);
                exit (1);
        }
@@ -416,9 +421,7 @@ int main (int argc, char **argv)
        nscd_flush_cache ("group");
 
 #ifdef USE_PAM
-       if (PAM_SUCCESS == retval) {
-               (void) pam_end (pamh, PAM_SUCCESS);
-       }
+       (void) pam_end (pamh, PAM_SUCCESS);
 #endif                         /* USE_PAM */
 
        return E_SUCCESS;

diff --git a/src/groupmod.c b/src/groupmod.c
index 33f1d1dabfbb785fe34c1fd0d8e4b5e92e1c784f..bfdb468b9d2349bb786f56da3a95aba04a20095d 100644 (file)
--- a/src/groupmod.c
+++ b/src/groupmod.c
@@ -124,15 +124,42 @@ static void usage (void)
 static void fail_exit (int status)
 {
        if (group_locked) {
-               gr_unlock ();
+               if (gr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking group file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
        }
 #ifdef SHADOWGRP
        if (gshadow_locked) {
-               sgr_unlock ();
+               if (sgr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking gshadow file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
        }
 #endif                         /* SHADOWGRP */
        if (passwd_locked) {
-               pw_unlock();
+               if (pw_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the passwd file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the passwd file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking passwd file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
        }
        exit (status);
 }
@@ -145,14 +172,17 @@ static void fail_exit (int status)
  */
 static void new_grent (struct group *grent)
 {
-       if (nflg)
+       if (nflg) {
                grent->gr_name = xstrdup (group_newname);
+       }
 
-       if (gflg)
+       if (gflg) {
                grent->gr_gid = group_newid;
+       }
 
-       if (pflg)
+       if (pflg) {
                grent->gr_passwd = group_passwd;
+       }
 }
 
 #ifdef SHADOWGRP
@@ -164,11 +194,13 @@ static void new_grent (struct group *grent)
  */
 static void new_sgent (struct sgrp *sgent)
 {
-       if (nflg)
+       if (nflg) {
                sgent->sg_name = xstrdup (group_newname);
+       }
 
-       if (pflg)
+       if (pflg) {
                sgent->sg_passwd = group_passwd;
+       }
 }
 #endif                         /* SHADOWGRP */
 
@@ -468,28 +500,73 @@ static void close_files (void)
 {
        if (gr_close () == 0) {
                fprintf (stderr, _("%s: cannot rewrite group file\n"), Prog);
+               SYSLOG ((LOG_WARN, "cannot rewrite the group file"));
+#ifdef WITH_AUDIT
+               audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                             "rewrite group file",
+                             group_name, AUDIT_NO_ID, 0);
+#endif
                fail_exit (E_GRP_UPDATE);
        }
-       gr_unlock ();
+       if (gr_unlock () == 0) {
+               fprintf (stderr, _("%s: cannot unlock the group file\n"), Prog);
+               SYSLOG ((LOG_WARN, "cannot unlock the group file"));
+#ifdef WITH_AUDIT
+               audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                             "unlocking group file",
+                             group_name, AUDIT_NO_ID, 0);
+#endif
+               /* continue */
+       }
        group_locked = false;
 #ifdef SHADOWGRP
-       if (is_shadow_grp && (sgr_close () == 0)) {
-               fprintf (stderr,
-                        _("%s: cannot rewrite shadow group file\n"), Prog);
-               fail_exit (E_GRP_UPDATE);
-       }
        if (is_shadow_grp) {
-               sgr_unlock ();
+               if (sgr_close () == 0)) {
+                       fprintf (stderr,
+                                _("%s: cannot rewrite the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot rewrite the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "rewrite gshadow file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       fail_exit (E_GRP_UPDATE);
+               }
+               if (sgr_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the shadow group file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the shadow group file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking gshadow file",
+                                     group, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
                gshadow_locked = false;
        }
 #endif                         /* SHADOWGRP */
        if (gflg) {
                if (pw_close () == 0) {
                        fprintf (stderr,
-                                _("%s: cannot rewrite passwd file\n"), Prog);
+                                _("%s: cannot rewrite the passwd file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot rewrite the passwd file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "rewrite passwd file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
                        fail_exit (E_GRP_UPDATE);
                }
-               pw_unlock();
+               if (pw_unlock () == 0) {
+                       fprintf (stderr, _("%s: cannot unlock the passwd file\n"), Prog);
+                       SYSLOG ((LOG_WARN, "cannot unlock the passwd file"));
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+                                     "unlocking passwd file",
+                                     group_name, AUDIT_NO_ID, 0);
+#endif
+                       /* continue */
+               }
                passwd_locked = false;
        }
 }
@@ -503,11 +580,13 @@ static void open_files (void)
 {
        if (gr_lock () == 0) {
                fprintf (stderr, _("%s: cannot lock the group file\n"), Prog);
+               SYSLOG ((LOG_WARN, "cannot lock the group file"));
                fail_exit (E_GRP_UPDATE);
        }
        group_locked = true;
        if (gr_open (O_RDWR) == 0) {
                fprintf (stderr, _("%s: cannot open the group file\n"), Prog);
+               SYSLOG ((LOG_WARN, "cannot open the group file"));
                fail_exit (E_GRP_UPDATE);
        }
 #ifdef SHADOWGRP
@@ -516,6 +595,7 @@ static void open_files (void)
                        fprintf (stderr,
                                 _("%s: cannot lock the shadow group file\n"),
                                 Prog);
+                       SYSLOG ((LOG_WARN, "cannot lock the shadow group file"));
                        fail_exit (E_GRP_UPDATE);
                }
                gshadow_locked = true;
@@ -523,6 +603,7 @@ static void open_files (void)
                        fprintf (stderr,
                                 _("%s: cannot open the shadow group file\n"),
                                 Prog);
+                       SYSLOG ((LOG_WARN, "cannot open the shadow group file"));
                        fail_exit (E_GRP_UPDATE);
                }
        }
@@ -532,6 +613,7 @@ static void open_files (void)
                        fprintf (stderr,
                                 _("%s: cannot lock the passwd file\n"),
                                 Prog);
+                       SYSLOG ((LOG_WARN, "cannot lock the passwd file"));
                        fail_exit (E_GRP_UPDATE);
                }
                passwd_locked = true;
@@ -539,6 +621,7 @@ static void open_files (void)
                        fprintf (stderr,
                                 _("%s: cannot open the passwd file\n"),
                                 Prog);
+                       SYSLOG ((LOG_WARN, "cannot open the passwd file"));
                        fail_exit (E_GRP_UPDATE);
                }
        }
@@ -632,19 +715,14 @@ int main (int argc, char **argv)
 
        if (PAM_SUCCESS == retval) {
                retval = pam_authenticate (pamh, 0);
-               if (PAM_SUCCESS != retval) {
-                       (void) pam_end (pamh, retval);
-               }
        }
 
        if (PAM_SUCCESS == retval) {
                retval = pam_acct_mgmt (pamh, 0);
-               if (PAM_SUCCESS != retval) {
-                       (void) pam_end (pamh, retval);
-               }
        }
 
        if (PAM_SUCCESS != retval) {
+               (void) pam_end (pamh, retval);
                fprintf (stderr, _("%s: PAM authentication failed\n"), Prog);
                fail_exit (1);
        }
@@ -729,9 +807,7 @@ int main (int argc, char **argv)
        nscd_flush_cache ("group");
 
 #ifdef USE_PAM
-       if (PAM_SUCCESS == retval) {
-               (void) pam_end (pamh, PAM_SUCCESS);
-       }
+       (void) pam_end (pamh, PAM_SUCCESS);
 #endif                         /* USE_PAM */
        exit (E_SUCCESS);
        /* NOT REACHED */