On 09/28/2009 01:52 PM, Stephen Gallagher wrote: > On 09/28/2009 12:24 PM, Stephen Gallagher wrote: >> On 09/28/2009 11:49 AM, Sumit Bose wrote: >>> Hi, > >>> with the patch the config file is only read if it is >>> - a regular file >>> - owner and group are 0 (root) >>> - file permissions are 600 > >>> This patch depends on the config_from_fd patch currently under review. > >>> bye, >>> Sumit > > >>> ------------------------------------------------------------------------ > >>> _______________________________________________ >>> sssd-devel mailing list >>> sssd-devel@lists.fedorahosted.org >>> https://fedorahosted.org/mailman/listinfo/sssd-devel > >> Nack. > >> As discussed on IRC, the lstat is redundant. All of the necessary >> file-type checks can be performed with the fstat, with no risk of race >> condition. > > > Per conversation on IRC, this patch is approved. I didn't realize at > first that we want to exclude symlinks as well from the SSSD. > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel >
Updated Sumit's patch to use the new interface. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel -- Stephen Gallagher RHCE 804006346421761 Looking to carve out IT costs? www.redhat.com/carveoutcosts/
From 7042e0bf5d0096164a3e58ebec5452aa8003403e Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Mon, 28 Sep 2009 15:50:22 +0200 Subject: [PATCH 2/2] add utility call check_and_open_readonly --- server/Makefile.am | 13 +++- server/confdb/confdb_setup.c | 12 ++- server/monitor/monitor.c | 2 +- server/tests/check_and_open-tests.c | 184 +++++++++++++++++++++++++++++++++++ server/util/check_and_open.c | 89 +++++++++++++++++ server/util/util.h | 4 + 6 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 server/tests/check_and_open-tests.c create mode 100644 server/util/check_and_open.c diff --git a/server/Makefile.am b/server/Makefile.am index f43cf18..626633e 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 9110a5e..a732ae7 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; @@ -323,9 +324,16 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb) goto done; } + 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")); + 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)); diff --git a/server/monitor/monitor.c b/server/monitor/monitor.c index 9972397..ef91b6b 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 0000000..2045085 --- /dev/null +++ b/server/tests/check_and_open-tests.c @@ -0,0 +1,184 @@ +/* + SSSD + + Utilities tests check_and_open + + Authors: + Sumit Bose <sb...@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 0000000..5d5b579 --- /dev/null +++ b/server/util/check_and_open.c @@ -0,0 +1,89 @@ +/* + SSSD + + Check file permissions and open file + + Authors: + Sumit Bose <sb...@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 0212df0..b7deb85 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__ */ -- 1.6.2.5
signature.asc
Description: OpenPGP digital signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel