diff --git a/cf-agent/cf-agent.c b/cf-agent/cf-agent.c index 035da48af0..bdd7a8eb9a 100644 --- a/cf-agent/cf-agent.c +++ b/cf-agent/cf-agent.c @@ -221,6 +221,7 @@ static const struct option OPTIONS[] = {"skip-bootstrap-service-start", no_argument, 0, 0 }, {"skip-db-check", optional_argument, 0, 0 }, {"simulate", required_argument, 0, 0}, + {"simulate-keep-chroot", required_argument, 0, 0}, {NULL, 0, 0, '\0'} }; @@ -257,6 +258,7 @@ static const char *const HINTS[] = "Do not start CFEngine services as part of the bootstrap process", "Do not run database integrity checks and repairs at startup", "Run in simulate mode, either 'manifest', 'manifest-full' or 'diff'", + "Keep the changes chroot from a --simulate run, creating it at the given path, which must not already exist", NULL }; @@ -795,6 +797,38 @@ static GenericAgentConfig *CheckOpts(int argc, char **argv) DoCleanupAndExit(EXIT_FAILURE); } } + else if (StringEqual(option_name, "simulate-keep-chroot")) + { + if (optarg == NULL) + { + Log(LOG_LEVEL_ERR, + "Missing argument for --simulate-keep-chroot, a directory path required"); + DoCleanupAndExit(EXIT_FAILURE); + } + else if (!IsAbsPath(optarg)) + { + Log(LOG_LEVEL_ERR, + "Invalid argument for --simulate-keep-chroot, an absolute path required, not '%s'", + optarg); + DoCleanupAndExit(EXIT_FAILURE); + } + else if (strlen(optarg) > (PATH_MAX / 2)) + { + /* Every path the run touches is mapped to a path under + * this directory and the result has to fit in PATH_MAX, + * so roughly half of that budget is reserved for the + * original paths (and mapping a path that still does not + * fit aborts the run instead of truncating the path). */ + Log(LOG_LEVEL_ERR, + "Invalid argument for --simulate-keep-chroot, path longer than %d bytes", + PATH_MAX / 2); + DoCleanupAndExit(EXIT_FAILURE); + } + else + { + config->agent_specific.agent.simulate_keep_chroot = xstrdup(optarg); + } + } break; } default: @@ -822,6 +856,14 @@ static GenericAgentConfig *CheckOpts(int argc, char **argv) DoCleanupAndExit(EXIT_FAILURE); } + if ((config->agent_specific.agent.simulate_keep_chroot != NULL) && + !ChrootChanges()) + { + Log(LOG_LEVEL_ERR, + "Option --simulate-keep-chroot can only be used together with --simulate"); + DoCleanupAndExit(EXIT_FAILURE); + } + FreeFixedStringArray(argc_new, argv_new); return config; diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c index 4d25864eaf..d03e6ccc43 100644 --- a/libpromises/eval_context.c +++ b/libpromises/eval_context.c @@ -3872,17 +3872,18 @@ const char *ToChangesChroot(const char *orig_path) assert(orig_path != NULL); assert(IsAbsPath(orig_path)); - assert(strlen(orig_path) <= (PATH_MAX - chroot_len - 1)); + + const char *const given_path = orig_path; size_t offset = 0; #ifdef __MINGW32__ /* On Windows, absolute path starts with the drive letter and colon followed * by '\'. Let's replace the ":\" with just "\" so that each drive has its * own directory tree in the chroot. */ + char drive_letter = '\0'; if ((orig_path[0] > 'A') && ((orig_path[0] < 'Z')) && (orig_path[1] == ':')) { - chrooted_path[chroot_len] = orig_path[0]; - chrooted_path[chroot_len + 1] = FILE_SEPARATOR; + drive_letter = orig_path[0]; orig_path += 2; offset += 2; } @@ -3893,8 +3894,29 @@ const char *ToChangesChroot(const char *orig_path) orig_path++; } - /* Adds/copies the NUL-byte at the end of the string. */ - strncpy(chrooted_path + chroot_len + offset, orig_path, (PATH_MAX - chroot_len - offset - 1)); + /* A path that does not fit must not be truncated -- the copy would + * silently be made at a different path than the one recorded and + * reported, and two long paths could even be mapped to the same copy. + * Checked before anything is written into the buffer. */ + const size_t orig_len = strlen(orig_path); + if ((chroot_len + offset + orig_len) >= PATH_MAX) + { + Log(LOG_LEVEL_ERR, + "The path '%s' is too long to be mapped into the changes chroot, aborting", + given_path); + DoCleanupAndExit(EXIT_FAILURE); + } + +#ifdef __MINGW32__ + if (drive_letter != '\0') + { + chrooted_path[chroot_len] = drive_letter; + chrooted_path[chroot_len + 1] = FILE_SEPARATOR; + } +#endif + + /* Copies the NUL-byte at the end of the string. */ + memcpy(chrooted_path + chroot_len + offset, orig_path, orig_len + 1); return chrooted_path; } diff --git a/libpromises/generic_agent.c b/libpromises/generic_agent.c index 3e282c946e..b684776639 100644 --- a/libpromises/generic_agent.c +++ b/libpromises/generic_agent.c @@ -89,6 +89,10 @@ static char PIDFILE[CF_BUFSIZE] = ""; /* GLOBAL_C */ /* Used for 'ident' argument to openlog() */ static char CF_PROGRAM_NAME[256] = ""; +/* Path of the changes chroot kept after a run because of + * --simulate-keep-chroot, reported at exit by KeepChangesChroot(). */ +static char KEEP_CHANGES_CHROOT[PATH_MAX] = ""; /* GLOBAL_C */ + static void CheckWorkingDirectories(EvalContext *ctx); static void GetAutotagDir(char *dirname, size_t max_size, const char *maybe_dirname); @@ -106,6 +110,7 @@ static bool LoadAugmentsFiles(EvalContext *ctx, const char* filename); static void GetChangesChrootDir(char *buf, size_t buf_size); static void DeleteChangesChroot(); +static void KeepChangesChroot(); static int ParseFacility(const char *name); static inline const char *LogFacilityToString(int facility); @@ -1629,9 +1634,89 @@ void GenericAgentInitialize(EvalContext *ctx, GenericAgentConfig *config) if (ChrootChanges()) { char changes_chroot[PATH_MAX] = {0}; - GetChangesChrootDir(changes_chroot, sizeof(changes_chroot)); + const char *keep_chroot = + config->agent_specific.agent.simulate_keep_chroot; + if (keep_chroot != NULL) + { + strlcpy(changes_chroot, keep_chroot, sizeof(changes_chroot)); + + /* Strip any trailing separators so that the parent directory is + * determined correctly below. */ + DeleteSlash(changes_chroot); + + /* The chroot is created in the requested directory and kept after + * the run instead of being deleted. The directory is required to + * not exist yet so that the copies of potentially sensitive system + * files made in it below cannot mix with stale contents from a + * previous run and are only made in a directory created by this + * process, with the permissions enforced below. */ +#ifndef __MINGW32__ + /* Creating the directory with restrictive permissions is no + * protection if another user can replace the directory itself, so + * refuse a parent directory writable by group or others, unless + * the sticky bit keeps them from renaming or unlinking entries + * they don't own. */ + char *parent = GetParentDirectoryCopy(changes_chroot); + if (parent == NULL) + { + FatalError( + ctx, + "Failed to determine the parent directory of '%s' for keeping the changes chroot", + changes_chroot); + } + struct stat sb; + if (stat(parent, &sb) != 0) + { + FatalError( + ctx, + "Failed to check the parent directory '%s' for keeping the changes chroot (stat: %s)", + parent, + GetErrorStr()); + } + if (((sb.st_mode & (S_IWGRP | S_IWOTH)) != 0) && + ((sb.st_mode & S_ISVTX) == 0)) + { + FatalError( + ctx, + "Refusing to create the directory '%s' for keeping the changes chroot, its parent directory '%s' is writable by other users", + changes_chroot, + parent); + } + free(parent); +#endif /* !__MINGW32__ */ + + if (mkdir(changes_chroot, 0700) != 0) + { + FatalError( + ctx, + "Failed to create the directory '%s' for keeping the changes chroot (mkdir: %s)", + changes_chroot, + GetErrorStr()); + } + + /* mkdir()'s mode argument is masked by umask, make sure the + * directory really is only accessible to its owner. */ + if (chmod(changes_chroot, 0700) != 0) + { + FatalError( + ctx, + "Failed to set the permissions of the directory '%s' for keeping the changes chroot (chmod: %s)", + changes_chroot, + GetErrorStr()); + } + + strlcpy( + KEEP_CHANGES_CHROOT, + changes_chroot, + sizeof(KEEP_CHANGES_CHROOT)); + RegisterCleanupFunction(KeepChangesChroot); + } + else + { + GetChangesChrootDir(changes_chroot, sizeof(changes_chroot)); + RegisterCleanupFunction(DeleteChangesChroot); + } SetChangesChroot(changes_chroot); - RegisterCleanupFunction(DeleteChangesChroot); Log(LOG_LEVEL_WARNING, "All changes in files will be made in the '%s' chroot", changes_chroot); } @@ -1799,6 +1884,11 @@ static void DeleteChangesChroot() } } +static void KeepChangesChroot() +{ + Log(LOG_LEVEL_NOTICE, "Keeping changes chroot '%s'", KEEP_CHANGES_CHROOT); +} + void GenericAgentFinalize(EvalContext *ctx, GenericAgentConfig *config) { /* TODO, FIXME: what else from the above do we need to undo here ? */ @@ -2614,6 +2704,9 @@ GenericAgentConfig *GenericAgentConfigNewDefault(AgentType agent_type, bool tty_ /* By default we start services during bootstrap */ config->agent_specific.agent.skip_bootstrap_service_start = false; + /* By default the changes chroot from a simulate run is deleted at exit */ + config->agent_specific.agent.simulate_keep_chroot = NULL; + /* Log classes */ config->agent_specific.agent.report_class_log = false; @@ -2661,6 +2754,7 @@ void GenericAgentConfigDestroy(GenericAgentConfig *config) free(config->agent_specific.agent.bootstrap_host); free(config->agent_specific.agent.bootstrap_ip); free(config->agent_specific.agent.bootstrap_port); + free(config->agent_specific.agent.simulate_keep_chroot); free(config); } } diff --git a/libpromises/generic_agent.h b/libpromises/generic_agent.h index 58f792ab76..595f55be60 100644 --- a/libpromises/generic_agent.h +++ b/libpromises/generic_agent.h @@ -94,6 +94,7 @@ typedef struct bool skip_bootstrap_service_start; char *show_evaluated_classes; char *show_evaluated_variables; + char *simulate_keep_chroot; // --simulate-keep-chroot // BODY AGENT CONTROL bool report_class_log; diff --git a/tests/acceptance/29_simulate_mode/keep_chroot.cf b/tests/acceptance/29_simulate_mode/keep_chroot.cf new file mode 100644 index 0000000000..14aa513aea --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot.cf @@ -0,0 +1,86 @@ +body common control +{ + inputs => { + "../default.sub.cf", + }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +bundle agent init +{ + files: + "$(G.testdir)$(const.dirsep)keep-chroot-file" + create => "true", + content => "This is the original content of the file."; +} + +bundle agent test +{ + meta: + "test_soft_fail" + string => "(solaris|aix|hpux|windows)", + # ENT-6540 exotics fail to delete chroot + # On Windows the chroot maps drive letters to a different layout so the + # path of the kept copy checked below does not apply there + meta => { "ENT-6540" }; + + "description" + string => "Test that --simulate-keep-chroot keeps the changes chroot and that it is still deleted by default"; + + vars: + "kept_chroot" string => "$(G.testdir)$(const.dirsep)kept.changes"; + + commands: + # add --verbose here and look at the log for debugging sub policy runs + "$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot.cf.sub --simulate=manifest --simulate-keep-chroot=$(kept_chroot) > $(G.testdir)$(const.dirsep)keep.log 2>&1" + contain => in_shell, + comment => "Run sub policy in simulate mode, keeping the changes chroot"; + + "$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot.cf.sub --simulate=manifest > $(G.testdir)$(const.dirsep)default.log 2>&1" + contain => in_shell, + comment => "Run sub policy in simulate mode with the default chroot cleanup"; +} + +bundle agent check +{ + vars: + "kept_copy" + string => "$(test.kept_chroot)$(G.testdir)$(const.dirsep)keep-chroot-file"; + + classes: + "kept_copy_has_changes" + expression => strcmp(readfile("$(kept_copy)", 0), + "This is the would-be content of the file."), + if => fileexists("$(kept_copy)"); + + "real_file_untouched" + expression => strcmp(readfile("$(G.testdir)$(const.dirsep)keep-chroot-file", 0), + "This is the original content of the file."), + if => fileexists("$(G.testdir)$(const.dirsep)keep-chroot-file"); + + "keep_notice_logged" + expression => regline(".*Keeping changes chroot.*", + "$(G.testdir)$(const.dirsep)keep.log"), + if => fileexists("$(G.testdir)$(const.dirsep)keep.log"); + + # The default.log check below makes sure this is not evaluated before the + # run without --simulate-keep-chroot has actually happened + "default_chroot_deleted" + expression => strcmp(length(findfiles("$(sys.statedir)$(const.dirsep)*.changes")), "0"), + if => fileexists("$(G.testdir)$(const.dirsep)default.log"); + + "keep_requires_simulate" + expression => not(returnszero("$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot.cf.sub --simulate-keep-chroot=$(G.testdir)$(const.dirsep)unused.changes", "noshell")), + if => fileexists("$(G.testdir)$(const.dirsep)default.log"); + + "ok" + expression => and("kept_copy_has_changes", "real_file_untouched", + "keep_notice_logged", "default_chroot_deleted", + "keep_requires_simulate"); + + methods: + "Pass/Fail" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; # We want dcs_passif to inherit bundle scoped classes from our check bundle +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot.cf.sub b/tests/acceptance/29_simulate_mode/keep_chroot.cf.sub new file mode 100644 index 0000000000..e55972ec63 --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot.cf.sub @@ -0,0 +1,17 @@ +bundle common simulate_keep_chroot_promises +{ + vars: + "inputs" slist => { "$(this.promise_dirname)/../default.sub.cf" }; +} + +body common control +{ + inputs => { "@(simulate_keep_chroot_promises.inputs)" }; +} + +bundle agent main +{ + files: + "$(G.testdir)$(const.dirsep)keep-chroot-file" + content => "This is the would-be content of the file."; +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot_parent_perms.cf b/tests/acceptance/29_simulate_mode/keep_chroot_parent_perms.cf new file mode 100644 index 0000000000..e279264c60 --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot_parent_perms.cf @@ -0,0 +1,88 @@ +body common control +{ + inputs => { + "../default.sub.cf", + }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +bundle agent init +{ + vars: + "loose_parent" string => "$(G.testdir)$(const.dirsep)loose"; + "sticky_parent" string => "$(G.testdir)$(const.dirsep)sticky"; + + files: + "$(loose_parent)/." + create => "true", + perms => m("777"); + + "$(sticky_parent)/." + create => "true", + perms => m("1777"); +} + +bundle agent test +{ + meta: + "test_skip_unsupported" + string => "windows", + # The parent directory permission check is Unix-only + comment => "the check is not compiled in on Windows where these + permission bits do not apply"; + + "description" + string => "Test that --simulate-keep-chroot refuses a parent directory writable by other users unless it has the sticky bit set"; + + vars: + "refused" string => "$(init.loose_parent)$(const.dirsep)refused.changes"; + "kept" string => "$(init.sticky_parent)$(const.dirsep)kept.changes"; + + commands: + "$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_parent_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(refused) > $(G.testdir)$(const.dirsep)parent-loose.log 2>&1" + contain => in_shell, + comment => "Run sub policy with the kept chroot inside a world-writable directory"; + + # The umask makes sure the 0700 permissions are still enforced when the + # sticky bit on the parent directory is what allows the creation. + "{ umask 0777; $(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_parent_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(kept); } > $(G.testdir)$(const.dirsep)parent-sticky.log 2>&1" + contain => in_shell, + comment => "Run sub policy with the kept chroot inside a world-writable directory with the sticky bit"; +} + +bundle agent check +{ + classes: + "loose_parent_error_logged" + expression => regline(".*Refusing to create the directory.*writable by other users.*", + "$(G.testdir)$(const.dirsep)parent-loose.log"), + if => fileexists("$(G.testdir)$(const.dirsep)parent-loose.log"); + + "loose_parent_nothing_created" + expression => not(isdir("$(test.refused)")), + if => fileexists("$(G.testdir)$(const.dirsep)parent-loose.log"); + + "loose_parent_refused" + expression => not(returnszero("$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_parent_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(test.refused)", "noshell")), + if => fileexists("$(G.testdir)$(const.dirsep)parent-loose.log"); + + "sticky_parent_kept_chroot" + expression => regline(".*Keeping changes chroot.*", + "$(G.testdir)$(const.dirsep)parent-sticky.log"), + if => fileexists("$(G.testdir)$(const.dirsep)parent-sticky.log"); + + "sticky_parent_dir_mode_0700" + expression => strcmp(filestat("$(test.kept)", "permoct"), "700"), + if => isdir("$(test.kept)"); + + "ok" + expression => and("loose_parent_error_logged", "loose_parent_nothing_created", + "loose_parent_refused", "sticky_parent_kept_chroot", + "sticky_parent_dir_mode_0700"); + + methods: + "Pass/Fail" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; # We want dcs_passif to inherit bundle scoped classes from our check bundle +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot_parent_perms.cf.sub b/tests/acceptance/29_simulate_mode/keep_chroot_parent_perms.cf.sub new file mode 100644 index 0000000000..6b5e77aeb2 --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot_parent_perms.cf.sub @@ -0,0 +1,18 @@ +bundle common keep_chroot_parent_perms_promises +{ + vars: + "inputs" slist => { "$(this.promise_dirname)/../default.sub.cf" }; +} + +body common control +{ + inputs => { "@(keep_chroot_parent_perms_promises.inputs)" }; +} + +bundle agent main +{ + files: + "$(G.testdir)$(const.dirsep)keep-chroot-parent-perms-file" + create => "true", + content => "This is the would-be content of the file."; +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot_path_limits.cf b/tests/acceptance/29_simulate_mode/keep_chroot_path_limits.cf new file mode 100644 index 0000000000..11b2f438c4 --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot_path_limits.cf @@ -0,0 +1,110 @@ +body common control +{ + inputs => { + "../default.sub.cf", + }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +bundle agent test +{ + meta: + "test_skip_unsupported" + string => "windows", + comment => "the limits below assume Unix absolute paths"; + + "test_skip_needs_work" + string => "!has_perl"; + + "description" + string => "Test that --simulate-keep-chroot rejects a path longer than the limit it prints, accepts one of exactly that length, and that mapping a too long path into the chroot aborts the run instead of truncating the path"; + + vars: + "toolong_keep" string => "$(G.testdir)$(const.dirsep)path-limits-toolong.changes"; + + commands: + # An argument this long is over the limit on all platforms (the limit is + # PATH_MAX / 2 and the error message prints its value, used by the + # boundary checks below). + "$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_path_limits.cf.sub --simulate=manifest --simulate-keep-chroot=`$(G.perl) -e 'print \"/\", \"a\" x 2999'` > $(G.testdir)$(const.dirsep)pl-reject.log 2>&1" + contain => in_shell, + comment => "Run sub policy with a kept chroot path longer than the limit"; + + # The sub policy promises a path of more than 4000 bytes, which cannot be + # mapped to a path under the chroot within PATH_MAX on any platform. The + # run has to abort -- a truncated mapping would record the copy at a + # different path than reported and could map two long paths to one copy. + "$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_path_limits.cf.sub --simulate=manifest --simulate-keep-chroot=$(toolong_keep) > $(G.testdir)$(const.dirsep)pl-mapped.log 2>&1 || echo SIMULATE_RUN_FAILED >> $(G.testdir)$(const.dirsep)pl-mapped.log" + contain => in_shell, + comment => "Run sub policy that promises a path too long to be mapped into the chroot"; +} + +bundle agent check +{ + vars: + "reject_output" + string => readfile("$(G.testdir)$(const.dirsep)pl-reject.log", 0), + if => fileexists("$(G.testdir)$(const.dirsep)pl-reject.log"); + + classes: + "arg_rejected" + expression => regline(".*Invalid argument for --simulate-keep-chroot, path longer than [0-9]+ bytes.*", + "$(G.testdir)$(const.dirsep)pl-reject.log"), + if => fileexists("$(G.testdir)$(const.dirsep)pl-reject.log"); + + # The limit printed in the message ends up in $(bound[1]) and the runs + # below make sure it is really the enforced one. + "have_bound" + expression => regextract("(?s).*path longer than ([0-9]+) bytes.*", + "$(reject_output)", "bound"); + + # A path of exactly the printed length has to make it past the argument + # check. It fails later (no path component this long can be created), but + # with the error from the creation attempt, not the argument check. + "bound_arg_ran" + expression => not(returnszero("$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_path_limits.cf.sub --simulate=manifest --simulate-keep-chroot=`$(G.perl) -e 'print \"/\", \"a\" x ($(bound[1]) - 1)'` > $(G.testdir)$(const.dirsep)pl-boundary-pass.log 2>&1", "useshell")), + if => "have_bound"; + + "bound_arg_not_rejected" + not => regline(".*Invalid argument for --simulate-keep-chroot.*", + "$(G.testdir)$(const.dirsep)pl-boundary-pass.log"), + if => fileexists("$(G.testdir)$(const.dirsep)pl-boundary-pass.log"); + + "bound_arg_reached_creation" + expression => regline(".*Failed to create the directory.*", + "$(G.testdir)$(const.dirsep)pl-boundary-pass.log"), + if => fileexists("$(G.testdir)$(const.dirsep)pl-boundary-pass.log"); + + # One byte more has to be rejected by the argument check. + "over_bound_arg_rejected" + expression => not(returnszero("$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_path_limits.cf.sub --simulate=manifest --simulate-keep-chroot=`$(G.perl) -e 'print \"/\", \"a\" x $(bound[1])'` > $(G.testdir)$(const.dirsep)pl-boundary-reject.log 2>&1", "useshell")), + if => "have_bound"; + + "over_bound_arg_reject_logged" + expression => regline(".*Invalid argument for --simulate-keep-chroot, path longer than [0-9]+ bytes.*", + "$(G.testdir)$(const.dirsep)pl-boundary-reject.log"), + if => fileexists("$(G.testdir)$(const.dirsep)pl-boundary-reject.log"); + + "mapped_error_logged" + expression => regline(".*too long to be mapped into the changes chroot.*", + "$(G.testdir)$(const.dirsep)pl-mapped.log"), + if => fileexists("$(G.testdir)$(const.dirsep)pl-mapped.log"); + + "mapped_run_failed" + expression => regline("SIMULATE_RUN_FAILED", + "$(G.testdir)$(const.dirsep)pl-mapped.log"), + if => fileexists("$(G.testdir)$(const.dirsep)pl-mapped.log"); + + "ok" + expression => and("arg_rejected", "have_bound", + "bound_arg_ran", "bound_arg_not_rejected", + "bound_arg_reached_creation", + "over_bound_arg_rejected", "over_bound_arg_reject_logged", + "mapped_error_logged", "mapped_run_failed"); + + methods: + "Pass/Fail" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; # We want dcs_passif to inherit bundle scoped classes from our check bundle +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot_path_limits.cf.sub b/tests/acceptance/29_simulate_mode/keep_chroot_path_limits.cf.sub new file mode 100644 index 0000000000..53deaa4f1b --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot_path_limits.cf.sub @@ -0,0 +1,27 @@ +bundle common keep_chroot_path_limits_promises +{ + vars: + "inputs" slist => { "$(this.promise_dirname)/../default.sub.cf" }; +} + +body common control +{ + inputs => { "@(keep_chroot_path_limits_promises.inputs)" }; +} + +bundle agent main +{ + vars: + # A path of more than 4000 bytes, built without external tools so that it + # works in simulate mode: prepended with the chroot directory path it can + # never fit in PATH_MAX (4096 at most on the supported platforms), so + # mapping it has to abort the run. + "a16" string => "aaaaaaaaaaaaaaaa"; + "a240" string => "$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)$(a16)"; + "a256" string => "$(a240)$(a16)"; + "long" string => "$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a256)$(a240)"; + + files: + "/$(long)" + create => "true"; +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot_perms.cf b/tests/acceptance/29_simulate_mode/keep_chroot_perms.cf new file mode 100644 index 0000000000..b9b214da60 --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot_perms.cf @@ -0,0 +1,95 @@ +body common control +{ + inputs => { + "../default.sub.cf", + }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +bundle agent init +{ + vars: + "existing" string => "$(G.testdir)$(const.dirsep)existing.changes"; + + files: + "$(existing)/." + create => "true"; +} + +bundle agent test +{ + meta: + "test_skip_unsupported" + string => "windows", + comment => "umask and the 0700 permissions of the kept chroot are Unix + permission semantics"; + + "description" + string => "Test that the changes chroot kept by --simulate-keep-chroot is created with 0700 permissions regardless of umask, that a trailing separator in the path is handled, and that an existing directory is refused"; + + vars: + "kept_umask" string => "$(G.testdir)$(const.dirsep)kept-umask.changes"; + "kept_slash" string => "$(G.testdir)$(const.dirsep)kept-slash.changes"; + + commands: + # The braces make sure the log file is created before umask is changed, + # otherwise it would not be readable by the check bundle below. mkdir(2) + # applies umask to its mode argument, so with an umask this restrictive a + # kept chroot that is not explicitly given its permissions would end up + # with mode 0000. + "{ umask 0777; $(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(kept_umask); } > $(G.testdir)$(const.dirsep)perms-umask.log 2>&1" + contain => in_shell, + comment => "Run sub policy keeping the changes chroot under a fully restrictive umask"; + + "{ umask 0777; $(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(kept_slash)$(const.dirsep); } > $(G.testdir)$(const.dirsep)perms-slash.log 2>&1" + contain => in_shell, + comment => "Run sub policy with a trailing separator in the kept chroot path"; + + "$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(init.existing) > $(G.testdir)$(const.dirsep)perms-existing.log 2>&1" + contain => in_shell, + comment => "Run sub policy with an already existing directory as the kept chroot path"; +} + +bundle agent check +{ + classes: + "umask_run_kept_chroot" + expression => regline(".*Keeping changes chroot.*", + "$(G.testdir)$(const.dirsep)perms-umask.log"), + if => fileexists("$(G.testdir)$(const.dirsep)perms-umask.log"); + + "umask_dir_mode_0700" + expression => strcmp(filestat("$(test.kept_umask)", "permoct"), "700"), + if => isdir("$(test.kept_umask)"); + + "slash_run_kept_chroot" + expression => regline(".*Keeping changes chroot.*", + "$(G.testdir)$(const.dirsep)perms-slash.log"), + if => fileexists("$(G.testdir)$(const.dirsep)perms-slash.log"); + + # The trailing separator is stripped, the directory is created at the + # normalized path (with the same enforced permissions). + "slash_dir_mode_0700" + expression => strcmp(filestat("$(test.kept_slash)", "permoct"), "700"), + if => isdir("$(test.kept_slash)"); + + "existing_dir_error_logged" + expression => regline(".*Failed to create the directory.*for keeping the changes chroot.*", + "$(G.testdir)$(const.dirsep)perms-existing.log"), + if => fileexists("$(G.testdir)$(const.dirsep)perms-existing.log"); + + "existing_dir_refused" + expression => not(returnszero("$(sys.cf_agent) -Kf $(this.promise_dirname)$(const.dirsep)keep_chroot_perms.cf.sub --simulate=manifest --simulate-keep-chroot=$(init.existing)", "noshell")), + if => fileexists("$(G.testdir)$(const.dirsep)perms-existing.log"); + + "ok" + expression => and("umask_run_kept_chroot", "umask_dir_mode_0700", + "slash_run_kept_chroot", "slash_dir_mode_0700", + "existing_dir_error_logged", "existing_dir_refused"); + + methods: + "Pass/Fail" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; # We want dcs_passif to inherit bundle scoped classes from our check bundle +} diff --git a/tests/acceptance/29_simulate_mode/keep_chroot_perms.cf.sub b/tests/acceptance/29_simulate_mode/keep_chroot_perms.cf.sub new file mode 100644 index 0000000000..11ad8fc74b --- /dev/null +++ b/tests/acceptance/29_simulate_mode/keep_chroot_perms.cf.sub @@ -0,0 +1,18 @@ +bundle common keep_chroot_perms_promises +{ + vars: + "inputs" slist => { "$(this.promise_dirname)/../default.sub.cf" }; +} + +body common control +{ + inputs => { "@(keep_chroot_perms_promises.inputs)" }; +} + +bundle agent main +{ + files: + "$(G.testdir)$(const.dirsep)keep-chroot-perms-file" + create => "true", + content => "This is the would-be content of the file."; +} diff --git a/tests/unit/eval_context_test.c b/tests/unit/eval_context_test.c index 77234beda8..a82d1aa8a3 100644 --- a/tests/unit/eval_context_test.c +++ b/tests/unit/eval_context_test.c @@ -122,6 +122,46 @@ void test_changes_chroot(void) /* Inverse should work as expected */ const char *normal = ToNormalRoot(chrooted); assert_string_equal(normal, "/etc/sysctl.d/00-default.conf"); + + /* The longest path that still fits: with the chroot above the mapped path + * is PATH_MAX - 1 bytes long. */ + const size_t chroot_len = strlen("/changes/go/here/"); + char long_path[PATH_MAX]; + size_t orig_len = PATH_MAX - chroot_len; + memset(long_path, 'a', orig_len); + long_path[0] = '/'; + long_path[orig_len] = '\0'; + chrooted = ToChangesChroot(long_path); + assert_int_equal(strlen(chrooted), PATH_MAX - 1); + assert_true(strncmp(chrooted, "/changes/go/here/", chroot_len) == 0); + + /* One byte more must not be truncated -- the run has to abort cleanly + * with EXIT_FAILURE (in a child process, since ToChangesChroot() does not + * return in that case). */ + orig_len = PATH_MAX - chroot_len + 1; + memset(long_path, 'a', orig_len); + long_path[0] = '/'; + long_path[orig_len] = '\0'; + pid_t child = fork(); + assert_true(child >= 0); + if (child == 0) + { + /* Silence the error message logged before the abort. */ + int devnull = open("/dev/null", O_WRONLY); + if (devnull != -1) + { + dup2(devnull, STDOUT_FILENO); + dup2(devnull, STDERR_FILENO); + } + ToChangesChroot(long_path); + + /* Not reached -- returning means the path was truncated. */ + _exit(0); + } + int status; + assert_int_equal(waitpid(child, &status, 0), child); + assert_true(WIFEXITED(status)); + assert_int_equal(WEXITSTATUS(status), EXIT_FAILURE); #endif }