add utility call check_and_open_readonly

Use this new utility call to ensure that the config file is safe
to read from.

6 files changed, 315 insertions, 8 deletions

diff --git a/server/Makefile.am b/server/Makefile.am
index f43cf188..626633e9 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -65,7 +65,8 @@ if HAVE_CHECK
         sysdb-tests \
         strtonum-tests \
         resolv-tests \
-        krb5-utils-tests
+        krb5-utils-tests \
+        check_and_open-tests
 endif
 
 check_PROGRAMS = \
@@ -176,6 +177,7 @@ SSSD_UTIL_OBJ = \
     util/usertools.c \
     util/backup_file.c \
     util/strtonum.c \
+    util/check_and_open.c \
     $(SSSD_DEBUG_OBJ)
 
 SSSD_RESPONDER_OBJ = \
@@ -392,6 +394,15 @@ krb5_utils_tests_CFLAGS = \
 krb5_utils_tests_LDADD = \
     $(CHECK_LIBS) \
     $(TALLOC_LIBS)
+
+check_and_open_tests_SOURCES = \
+    $(SSSD_DEBUG_OBJ) \
+    tests/check_and_open-tests.c \
+    util/check_and_open.c
+check_and_open_tests_CFLAGS = \
+    $(CHECK_CFLAGS)
+check_and_open_tests_LDADD = \
+    $(CHECK_LIBS)
 endif
 
 stress_tests_SOURCES = \
diff --git a/server/confdb/confdb_setup.c b/server/confdb/confdb_setup.c
index 9110a5e9..3c10c06c 100644
--- a/server/confdb/confdb_setup.c
+++ b/server/confdb/confdb_setup.c
@@ -270,6 +270,7 @@ error:
 int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
 {
     int ret, i;
+    int fd = -1;
     struct collection_item *sssd_config = NULL;
     struct collection_item *error_list = NULL;
     struct collection_item *item = NULL;
@@ -284,16 +285,29 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
     tmp_ctx = talloc_new(cdb);
     if (tmp_ctx == NULL) return ENOMEM;
 
-    /* ok, first of all stat conf file */
-    ret = stat(config_file, &cstat);
+    ret = check_and_open_readonly(config_file, &fd, 0, 0, (S_IRUSR|S_IWUSR));
+    if (ret != EOK) {
+        DEBUG(1, ("Permission check on config file failed.\n"));
+        talloc_zfree(tmp_ctx);
+        return EIO;
+    }
+
+    /* Determine if the conf file has changed since we last updated
+     * the confdb
+     */
+    ret = fstat(fd, &cstat);
     if (ret != 0) {
         DEBUG(0, ("Unable to stat config file [%s]! (%d [%s])\n",
                   config_file, errno, strerror(errno)));
+        close(fd);
+        talloc_zfree(tmp_ctx);
         return errno;
     }
     ret = snprintf(timestr, 21, "%llu", (long long unsigned)cstat.st_mtime);
     if (ret <= 0 || ret >= 21) {
         DEBUG(0, ("Failed to convert time_t to string ??\n"));
+        close(fd);
+        talloc_zfree(tmp_ctx);
         return errno ? errno: EFAULT;
     }
 
@@ -304,6 +318,8 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
         /* now check if we lastUpdate and last file modification change differ*/
         if (strcmp(lasttimestr, timestr) == 0) {
             /* not changed, get out, nothing more to do */
+            close(fd);
+            talloc_zfree(tmp_ctx);
             return EOK;
         }
     }
@@ -312,7 +328,8 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
     ret = ldb_transaction_start(cdb->ldb);
     if (ret != LDB_SUCCESS) {
         DEBUG(0, ("Failed to start a transaction for updating the configuration\n"));
-        talloc_free(tmp_ctx);
+        talloc_zfree(tmp_ctx);
+        close(fd);
         return sysdb_error_to_errno(ret);
     }
 
@@ -320,12 +337,14 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
     ret = confdb_purge(cdb);
     if (ret != EOK) {
         DEBUG(0, ("Could not purge existing configuration\n"));
+        close(fd);
         goto done;
     }
 
     /* Read the configuration into a collection */
-    ret = config_from_file("sssd", config_file, &sssd_config,
-                           INI_STOP_ON_ANY, &error_list);
+    ret = config_from_fd("sssd", fd, config_file, &sssd_config,
+                         INI_STOP_ON_ANY, &error_list);
+    close(fd);
     if (ret != EOK) {
         DEBUG(0, ("Parse error reading configuration file [%s]\n",
                   config_file));
@@ -399,6 +418,6 @@ done:
     ret == EOK ?
             ldb_transaction_commit(cdb->ldb) :
             ldb_transaction_cancel(cdb->ldb);
-    talloc_free(tmp_ctx);
+    talloc_zfree(tmp_ctx);
     return ret;
 }
diff --git a/server/monitor/monitor.c b/server/monitor/monitor.c
index 9972397e..ef91b6b6 100644
--- a/server/monitor/monitor.c
+++ b/server/monitor/monitor.c
@@ -2379,7 +2379,7 @@ int main(int argc, const char *argv[])
         SSSD_MAIN_OPTS
         {"daemon", 'D', POPT_ARG_NONE, &opt_daemon, 0, \
          "Become a daemon (default)", NULL }, \
-        {"interactive",	'i', POPT_ARG_NONE, &opt_interactive, 0, \
+        {"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, \
          "Run interactive (not a daemon)", NULL}, \
         {"config", 'c', POPT_ARG_STRING, &opt_config_file, 0, \
          "Specify a non-default config file", NULL}, \
diff --git a/server/tests/check_and_open-tests.c b/server/tests/check_and_open-tests.c
new file mode 100644
index 00000000..2045085e
--- /dev/null
+++ b/server/tests/check_and_open-tests.c
@@ -0,0 +1,184 @@
+/*
+    SSSD
+
+    Utilities tests check_and_open
+
+    Authors:
+        Sumit Bose <sbose@redhat.com>
+
+    Copyright (C) 2009 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program 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 General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <stdlib.h>
+#include <check.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include "util/util.h"
+
+char filename[] = "check_and_open-tests-XXXXXX";
+uid_t uid;
+gid_t gid;
+mode_t mode;
+int fd;
+
+void setup_check_and_open(void)
+{
+    int ret;
+
+    ret = mkstemp(filename);
+    fail_unless(ret != -1, "mkstemp failed [%d][%s]", errno, strerror(errno));
+    close(ret);
+
+    uid = getuid();
+    gid = getgid();
+    mode = (S_IRUSR | S_IWUSR);
+    fd = -1;
+}
+
+void teardown_check_and_open(void)
+{
+    int ret;
+
+    if (fd != -1) {
+        ret = close(fd);
+        fail_unless(ret == 0, "close failed [%d][%s]", errno, strerror(errno));
+    }
+
+    fail_unless(filename != NULL, "unknown filename");
+    ret = unlink(filename);
+    fail_unless(ret == 0, "unlink failed [%d][%s]", errno, strerror(errno));
+}
+
+START_TEST(test_wrong_filename)
+{
+    int ret;
+
+    ret = check_and_open_readonly("/bla/bla/bla", &fd, uid, gid, mode);
+    fail_unless(ret == ENOENT,
+                "check_and_open_readonly succeeded on non-existing file");
+    fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1");
+}
+END_TEST
+
+START_TEST(test_not_regular_file)
+{
+    int ret;
+
+    ret = check_and_open_readonly("/dev/null", &fd, uid, gid, mode);
+    fail_unless(ret == EINVAL,
+                "check_and_open_readonly succeeded on non-regular file");
+    fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1");
+}
+END_TEST
+
+START_TEST(test_wrong_uid)
+{
+    int ret;
+
+    ret = check_and_open_readonly(filename, &fd, uid+1, gid, mode);
+    fail_unless(ret == EINVAL,
+                "check_and_open_readonly succeeded with wrong uid");
+    fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1");
+}
+END_TEST
+
+START_TEST(test_wrong_gid)
+{
+    int ret;
+
+    ret = check_and_open_readonly(filename, &fd, uid, gid+1, mode);
+    fail_unless(ret == EINVAL,
+                "check_and_open_readonly succeeded with wrong gid");
+    fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1");
+}
+END_TEST
+
+START_TEST(test_wrong_permission)
+{
+    int ret;
+
+    ret = check_and_open_readonly(filename, &fd, uid, gid, (mode|S_IWOTH));
+    fail_unless(ret == EINVAL,
+                "check_and_open_readonly succeeded with wrong mode");
+    fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1");
+}
+END_TEST
+
+START_TEST(test_ok)
+{
+    int ret;
+
+    ret = check_and_open_readonly(filename, &fd, uid, gid, mode);
+    fail_unless(ret == EOK,
+                "check_and_open_readonly failed");
+    fail_unless(fd >= 0,
+                "check_and_open_readonly returned illegal file descriptor");
+}
+END_TEST
+
+START_TEST(test_write)
+{
+    int ret;
+    ssize_t size;
+    errno_t my_errno;
+
+    ret = check_and_open_readonly(filename, &fd, uid, gid, mode);
+    fail_unless(ret == EOK,
+                "check_and_open_readonly failed");
+    fail_unless(fd >= 0,
+                "check_and_open_readonly returned illegal file descriptor");
+
+    size = write(fd, "abc", 3);
+    my_errno = errno;
+    fail_unless(size == -1, "check_and_open_readonly file is not readonly");
+    fail_unless(my_errno == EBADF,
+                "write failed for other reason than readonly");
+}
+END_TEST
+
+Suite *check_and_open_suite (void)
+{
+    Suite *s = suite_create ("check_and_open");
+
+    TCase *tc_check_and_open_readonly = tcase_create ("check_and_open_readonly");
+    tcase_add_checked_fixture (tc_check_and_open_readonly,
+                               setup_check_and_open,
+                               teardown_check_and_open);
+    tcase_add_test (tc_check_and_open_readonly, test_wrong_filename);
+    tcase_add_test (tc_check_and_open_readonly, test_not_regular_file);
+    tcase_add_test (tc_check_and_open_readonly, test_wrong_uid);
+    tcase_add_test (tc_check_and_open_readonly, test_wrong_gid);
+    tcase_add_test (tc_check_and_open_readonly, test_wrong_permission);
+    tcase_add_test (tc_check_and_open_readonly, test_ok);
+    tcase_add_test (tc_check_and_open_readonly, test_write);
+    suite_add_tcase (s, tc_check_and_open_readonly);
+
+    return s;
+}
+
+int main(void)
+{
+  int number_failed;
+  Suite *s = check_and_open_suite ();
+  SRunner *sr = srunner_create (s);
+  srunner_run_all (sr, CK_NORMAL);
+  number_failed = srunner_ntests_failed (sr);
+  srunner_free (sr);
+  return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
diff --git a/server/util/check_and_open.c b/server/util/check_and_open.c
new file mode 100644
index 00000000..5d5b5799
--- /dev/null
+++ b/server/util/check_and_open.c
@@ -0,0 +1,89 @@
+/*
+    SSSD
+
+    Check file permissions and open file
+
+    Authors:
+        Sumit Bose <sbose@redhat.com>
+
+    Copyright (C) 2009 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program 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 General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "util/util.h"
+
+errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid,
+                               const gid_t gid, const mode_t mode)
+{
+    int ret;
+    struct stat stat_buf;
+    struct stat fd_stat_buf;
+
+    *fd = -1;
+
+    ret = lstat(filename, &stat_buf);
+    if (ret == -1) {
+        DEBUG(1, ("lstat for [%s] failed: [%d][%s].\n", filename, errno,
+                                                        strerror(errno)));
+        return errno;
+    }
+
+    if (!S_ISREG(stat_buf.st_mode)) {
+        DEBUG(1, ("File [%s] is not a regular file.\n", filename));
+        return EINVAL;
+    }
+
+    if ((stat_buf.st_mode & ~S_IFMT) != mode) {
+        DEBUG(1, ("File [%s] has the wrong mode [%.7o], expected [%.7o].\n",
+                  filename, (stat_buf.st_mode & ~S_IFMT), mode));
+        return EINVAL;
+    }
+
+    if (stat_buf.st_uid != uid || stat_buf.st_gid != gid) {
+        DEBUG(1, ("File [%s] must be owned by uid [%d] and gid [%d].\n",
+                  filename, uid, gid));
+        return EINVAL;
+    }
+
+    *fd = open(filename, O_RDONLY);
+    if (*fd == -1) {
+        DEBUG(1, ("open [%s] failed: [%d][%s].\n", filename, errno,
+                                                        strerror(errno)));
+        return errno;
+    }
+
+    ret = fstat(*fd, &fd_stat_buf);
+    if (ret == -1) {
+        DEBUG(1, ("fstat for [%s] failed: [%d][%s].\n", filename, errno,
+                                                        strerror(errno)));
+        return errno;
+    }
+
+    if (stat_buf.st_dev != fd_stat_buf.st_dev ||
+        stat_buf.st_ino != fd_stat_buf.st_ino) {
+        DEBUG(1, ("File [%s] was modified between lstat and open.\n", filename));
+        close(*fd);
+        *fd = -1;
+        return EIO;
+    }
+
+    return EOK;
+}
+
diff --git a/server/util/util.h b/server/util/util.h
index 0212df06..b7deb854 100644
--- a/server/util/util.h
+++ b/server/util/util.h
@@ -193,4 +193,8 @@ int sss_parse_name(TALLOC_CTX *memctx,
 /* from backup-file.c */
 int backup_file(const char *src, int dbglvl);
 
+/* from check_and_open.c */
+errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid,
+                               const gid_t gid, const mode_t mode);
+
 #endif /* __SSSD_UTIL_H__ */