summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSumit Bose <sbose@redhat.com>2009-09-28 15:50:22 +0200
committerStephen Gallagher <sgallagh@redhat.com>2009-10-05 10:32:09 -0400
commitb8dede30141cf87fb62aca918d04e411fac82946 (patch)
tree8a3eccfc11d96162d11e3fac165751fbcdc6f0b9
parente3a794633b02411c3d3adc4443e98541f045f41a (diff)
downloadsssd-b8dede30141cf87fb62aca918d04e411fac82946.tar.gz
sssd-b8dede30141cf87fb62aca918d04e411fac82946.tar.bz2
sssd-b8dede30141cf87fb62aca918d04e411fac82946.zip
add utility call check_and_open_readonly
Use this new utility call to ensure that the config file is safe to read from.
-rw-r--r--server/Makefile.am13
-rw-r--r--server/confdb/confdb_setup.c31
-rw-r--r--server/monitor/monitor.c2
-rw-r--r--server/tests/check_and_open-tests.c184
-rw-r--r--server/util/check_and_open.c89
-rw-r--r--server/util/util.h4
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__ */