File pacemaker-libcrmcommon-check-filedirectory-writ... of Package pacemaker.26927

Overview Repositories Revisions Requests Users Attributes Meta

File pacemaker-libcrmcommon-check-filedirectory-writability-better.patch of Package pacemaker.26927

commit e2e8a1ea79602237409cab9b828c824e34196edc
Author: Ken Gaillot <kgaillot@redhat.com>
Date:   Fri Apr 27 15:35:05 2018 -0500

Low: libcrmcommon: check file/directory writability better
    
    This replaces crm_is_writable() with a new similar function
    pcmk__daemon_can_write() (starting a new convention of prefixing internal
    library API functions with "pcmk__").
    
    The old function was called with need_both=FALSE so that both user and group
    write permissions were not required. However, it was implemented such that the
    check would pass even if neither could write.
    
    The new function fixes that and simplifies the interface by restricting it to
    the only case we're interested in (either CRM_DAEMON_USER or CRM_DAEMON_GROUP
    owns and can write the target). It also gives more detailed log messages when
    something doesn't match.

Index: pacemaker-1.1.18+20180430.b12c320f5/cib/io.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/cib/io.c
+++ pacemaker-1.1.18+20180430.b12c320f5/cib/io.c
@@ -194,7 +194,7 @@ readCibXmlFile(const char *dir, const ch
     xmlNode *root = NULL;
     xmlNode *status = NULL;
 
-    if (!crm_is_writable(dir, file, CRM_DAEMON_USER, NULL, FALSE)) {
+    if (pcmk__daemon_can_write(dir, file) == FALSE) {
         cib_status = -EACCES;
         return NULL;
     }
Index: pacemaker-1.1.18+20180430.b12c320f5/cib/main.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/cib/main.c
+++ pacemaker-1.1.18+20180430.b12c320f5/cib/main.c
@@ -190,9 +190,10 @@ main(int argc, char **argv)
         crm_notice("Using custom config location: %s", cib_root);
     }
 
-    if (crm_is_writable(cib_root, NULL, CRM_DAEMON_USER, CRM_DAEMON_GROUP, FALSE) == FALSE) {
-        crm_err("Bad permissions on %s. Terminating", cib_root);
-        fprintf(stderr, "ERROR: Bad permissions on %s. See logs for details\n", cib_root);
+    if (pcmk__daemon_can_write(cib_root, NULL) == FALSE) {
+        crm_err("Terminating due to bad permissions on %s", cib_root);
+        fprintf(stderr, "ERROR: Bad permissions on %s (see logs for details)\n",
+                cib_root);
         fflush(stderr);
         return CRM_EX_FATAL;
     }
Index: pacemaker-1.1.18+20180430.b12c320f5/crmd/main.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/crmd/main.c
+++ pacemaker-1.1.18+20180430.b12c320f5/crmd/main.c
@@ -104,16 +104,17 @@ main(int argc, char **argv)
         crm_help('?', CRM_EX_USAGE);
     }
 
-    if (crm_is_writable(PE_STATE_DIR, NULL, CRM_DAEMON_USER, CRM_DAEMON_GROUP, FALSE) == FALSE) {
-        crm_err("Bad permissions on " PE_STATE_DIR ". Terminating");
-        fprintf(stderr, "ERROR: Bad permissions on " PE_STATE_DIR ". See logs for details\n");
+    if (pcmk__daemon_can_write(PE_STATE_DIR, NULL) == FALSE) {
+        crm_err("Terminating due to bad permissions on " PE_STATE_DIR);
+        fprintf(stderr,
+                "ERROR: Bad permissions on " PE_STATE_DIR " (see logs for details)\n");
         fflush(stderr);
         return CRM_EX_FATAL;
 
-    } else if (crm_is_writable(CRM_CONFIG_DIR, NULL, CRM_DAEMON_USER, CRM_DAEMON_GROUP, FALSE) ==
-               FALSE) {
-        crm_err("Bad permissions on " CRM_CONFIG_DIR ". Terminating");
-        fprintf(stderr, "ERROR: Bad permissions on " CRM_CONFIG_DIR ". See logs for details\n");
+    } else if (pcmk__daemon_can_write(CRM_CONFIG_DIR, NULL) == FALSE) {
+        crm_err("Terminating due to bad permissions on " CRM_CONFIG_DIR);
+        fprintf(stderr,
+                "ERROR: Bad permissions on " CRM_CONFIG_DIR " (see logs for details)\n");
         fflush(stderr);
         return CRM_EX_FATAL;
     }
Index: pacemaker-1.1.18+20180430.b12c320f5/pengine/main.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/pengine/main.c
+++ pacemaker-1.1.18+20180430.b12c320f5/pengine/main.c
@@ -150,9 +150,10 @@ main(int argc, char **argv)
     }
 
     crm_log_init(NULL, LOG_INFO, TRUE, FALSE, argc, argv, FALSE);
-    if (crm_is_writable(PE_STATE_DIR, NULL, CRM_DAEMON_USER, CRM_DAEMON_GROUP, FALSE) == FALSE) {
-        crm_err("Bad permissions on " PE_STATE_DIR ". Terminating");
-        fprintf(stderr, "ERROR: Bad permissions on " PE_STATE_DIR ". See logs for details\n");
+    if (pcmk__daemon_can_write(PE_STATE_DIR, NULL) == FALSE) {
+        crm_err("Terminating due to bad permissions on " PE_STATE_DIR);
+        fprintf(stderr,
+                "ERROR: Bad permissions on " PE_STATE_DIR " (see logs for details)\n");
         fflush(stderr);
         return CRM_EX_FATAL;
     }
Index: pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/internal.h
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/include/crm/common/internal.h
+++ pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/internal.h
@@ -23,9 +23,7 @@ int get_last_sequence(const char *direct
 void write_last_sequence(const char *directory, const char *series, int sequence, int max);
 int crm_chown_last_sequence(const char *directory, const char *series, uid_t uid, gid_t gid);
 
-gboolean crm_is_writable(const char *dir, const char *file, const char *user, const char *group,
-                         gboolean need_both);
-
+bool pcmk__daemon_can_write(const char *dir, const char *file);
 void crm_sync_directory(const char *name);
 
 char *crm_read_contents(const char *filename);
Index: pacemaker-1.1.18+20180430.b12c320f5/lib/common/io.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/lib/common/io.c
+++ pacemaker-1.1.18+20180430.b12c320f5/lib/common/io.c
@@ -1,19 +1,8 @@
 /*
- * Copyright (C) 2004 Andrew Beekhof <andrew@beekhof.net>
+ * Copyright 2004-2018 Andrew Beekhof <andrew@beekhof.net>
  *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library 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
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ * This source code is licensed under the GNU Lesser General Public License
+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
  */
 
 #include <crm_internal.h>
@@ -240,88 +229,137 @@ crm_chown_last_sequence(const char *dire
     return rc;
 }
 
+static bool
+pcmk__daemon_user_can_write(const char *target_name, struct stat *target_stat)
+{
+    struct passwd *sys_user = NULL;
+
+    errno = 0;
+    sys_user = getpwnam(CRM_DAEMON_USER);
+    if (sys_user == NULL) {
+        crm_notice("Could not find user %s: %s",
+                   CRM_DAEMON_USER, pcmk_strerror(errno));
+        return FALSE;
+    }
+    if (target_stat->st_uid != sys_user->pw_uid) {
+        crm_notice("%s is not owned by user %s " CRM_XS " uid %d != %d",
+                   target_name, CRM_DAEMON_USER, sys_user->pw_uid,
+                   target_stat->st_uid);
+        return FALSE;
+    }
+    if ((target_stat->st_mode & (S_IRUSR | S_IWUSR)) == 0) {
+        crm_notice("%s is not readable and writable by user %s "
+                   CRM_XS " st_mode=0%lo",
+                   target_name, CRM_DAEMON_USER,
+                   (unsigned long) target_stat->st_mode);
+        return FALSE;
+    }
+    return TRUE;
+}
+
+static bool
+pcmk__daemon_group_can_write(const char *target_name, struct stat *target_stat)
+{
+    struct group *sys_grp = NULL;
+
+    errno = 0;
+    sys_grp = getgrnam(CRM_DAEMON_GROUP);
+    if (sys_grp == NULL) {
+        crm_notice("Could not find group %s: %s",
+                   CRM_DAEMON_GROUP, pcmk_strerror(errno));
+        return FALSE;
+    }
+
+    if (target_stat->st_gid != sys_grp->gr_gid) {
+        crm_notice("%s is not owned by group %s " CRM_XS " uid %d != %d",
+                   target_name, CRM_DAEMON_GROUP,
+                   sys_grp->gr_gid, target_stat->st_gid);
+        return FALSE;
+    }
+
+    if ((target_stat->st_mode & (S_IRGRP | S_IWGRP)) == 0) {
+        crm_notice("%s is not readable and writable by group %s "
+                   CRM_XS " st_mode=0%lo",
+                   target_name, CRM_DAEMON_GROUP,
+                   (unsigned long) target_stat->st_mode);
+        return FALSE;
+    }
+    return TRUE;
+}
+
 /*!
  * \internal
- * \brief Return whether a directory or file is writable by a user/group
+ * \brief Check whether a directory or file is writable by the cluster daemon
+ *
+ * Return TRUE if either the cluster daemon user or cluster daemon group has
+ * write permission on a specified file or directory.
  *
- * \param[in] dir Directory to check or that contains file
- * \param[in] file File name to check (or NULL to check directory)
- * \param[in] user Name of user that should have write permission
- * \param[in] group Name of group that should have write permission
- * \param[in] need_both Whether both user and group must be able to write
+ * \param[in] dir      Directory to check (this argument must be specified, and
+ *                     the directory must exist)
+ * \param[in] file     File to check (only the directory will be checked if this
+ *                     argument is not specified or the file does not exist)
  *
- * \return TRUE if permissions match, FALSE if they don't or on error
+ * \return TRUE if target is writable by cluster daemon, FALSE otherwise
  */
-gboolean
-crm_is_writable(const char *dir, const char *file,
-                const char *user, const char *group, gboolean need_both)
+bool
+pcmk__daemon_can_write(const char *dir, const char *file)
 {
-    int s_res = -1;
+    int s_res = 0;
     struct stat buf;
     char *full_file = NULL;
     const char *target = NULL;
 
-    gboolean pass = TRUE;
-    gboolean readwritable = FALSE;
-
+    // Caller must supply directory
     CRM_ASSERT(dir != NULL);
+
+    // If file is given, check whether it exists as a regular file
     if (file != NULL) {
         full_file = crm_concat(dir, file, '/');
         target = full_file;
+
         s_res = stat(full_file, &buf);
-        if (s_res == 0 && S_ISREG(buf.st_mode) == FALSE) {
-            crm_err("%s must be a regular file", target);
-            pass = FALSE;
-            goto out;
+        if (s_res < 0) {
+            crm_notice("%s not found: %s", target, pcmk_strerror(errno));
+            free(full_file);
+            full_file = NULL;
+            target = NULL;
+
+        } else if (S_ISREG(buf.st_mode) == FALSE) {
+            crm_err("%s must be a regular file " CRM_XS " st_mode=0%lo",
+                    target, (unsigned long) buf.st_mode);
+            free(full_file);
+            return FALSE;
         }
     }
 
-    if (s_res != 0) {
+    // If file is not given, ensure dir exists as directory
+    if (target == NULL) {
         target = dir;
         s_res = stat(dir, &buf);
-        if (s_res != 0) {
-            crm_err("%s must exist and be a directory", dir);
-            pass = FALSE;
-            goto out;
+        if (s_res < 0) {
+            crm_err("%s not found: %s", dir, pcmk_strerror(errno));
+            return FALSE;
 
         } else if (S_ISDIR(buf.st_mode) == FALSE) {
-            crm_err("%s must be a directory", dir);
-            pass = FALSE;
-        }
-    }
-
-    if (user) {
-        struct passwd *sys_user = NULL;
-
-        sys_user = getpwnam(user);
-        readwritable = (sys_user != NULL
-                        && buf.st_uid == sys_user->pw_uid && (buf.st_mode & (S_IRUSR | S_IWUSR)));
-        if (readwritable == FALSE) {
-            crm_err("%s must be owned and r/w by user %s", target, user);
-            if (need_both) {
-                pass = FALSE;
-            }
+            crm_err("%s must be a directory " CRM_XS " st_mode=0%lo",
+                    dir, (unsigned long) buf.st_mode);
+            return FALSE;
         }
     }
 
-    if (group) {
-        struct group *sys_grp = getgrnam(group);
+    if (!pcmk__daemon_user_can_write(target, &buf)
+        && !pcmk__daemon_group_can_write(target, &buf)) {
 
-        readwritable = (sys_grp != NULL
-                        && buf.st_gid == sys_grp->gr_gid && (buf.st_mode & (S_IRGRP | S_IWGRP)));
-        if (readwritable == FALSE) {
-            if (need_both || user == NULL) {
-                pass = FALSE;
-                crm_err("%s must be owned and r/w by group %s", target, group);
-            } else {
-                crm_warn("%s should be owned and r/w by group %s", target, group);
-            }
-        }
+        crm_err("%s must be owned and writable by either user %s or group %s "
+                CRM_XS " st_mode=0%ol",
+                target, CRM_DAEMON_USER, CRM_DAEMON_GROUP,
+                (unsigned long) buf.st_mode);
+        free(full_file);
+        return FALSE;
     }
 
-  out:
     free(full_file);
-    return pass;
+    return TRUE;
 }
 
 /*!

Places

File pacemaker-libcrmcommon-check-filedirectory-writability-better.patch of Package pacemaker.26927

Places