From 22b954a566c0ac8886a1131c79da16589ec8a51e Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Tue, 28 Jan 2025 15:15:00 +0100 Subject: [PATCH 1/3] Added body syntax for controlling file system attributes Added syntax for `body fsattrs` with a boolean constraint `immutable`. It currently does nothing, but this will change in following commits. Ticket: ENT-10961, CFE-1840 Signed-off-by: Lars Erik Wik --- libpromises/attributes.c | 19 +++++++++++++++++++ libpromises/attributes.h | 1 + libpromises/cf3.defs.h | 10 ++++++++++ libpromises/mod_files.c | 10 ++++++++++ 4 files changed, 40 insertions(+) diff --git a/libpromises/attributes.c b/libpromises/attributes.c index b78ffdd998..647bcc42ec 100644 --- a/libpromises/attributes.c +++ b/libpromises/attributes.c @@ -55,6 +55,7 @@ Attributes GetFilesAttributes(const EvalContext *ctx, const Promise *pp) attr.haveselect = PromiseGetConstraintAsBoolean(ctx, "file_select", pp); attr.haverename = PromiseGetConstraintAsBoolean(ctx, "rename", pp); attr.havedelete = PromiseGetConstraintAsBoolean(ctx, "delete", pp); + attr.havefsattrs = PromiseBundleOrBodyConstraintExists(ctx, "fsattrs", pp); attr.content = PromiseGetConstraintAsRval(pp, "content", RVAL_TYPE_SCALAR); attr.haveperms = PromiseGetConstraintAsBoolean(ctx, "perms", pp); attr.havechange = PromiseGetConstraintAsBoolean(ctx, "changes", pp); @@ -89,6 +90,7 @@ Attributes GetFilesAttributes(const EvalContext *ctx, const Promise *pp) attr.perms = GetPermissionConstraints(ctx, pp); attr.select = GetSelectConstraints(ctx, pp); attr.delete = GetDeleteConstraints(ctx, pp); + attr.fsattrs = GetFSAttrsConstraints(ctx, pp); attr.rename = GetRenameConstraints(ctx, pp); attr.change = GetChangeMgtConstraints(ctx, pp); attr.copy = GetCopyConstraints(ctx, pp); @@ -830,6 +832,23 @@ FileDelete GetDeleteConstraints(const EvalContext *ctx, const Promise *pp) return f; } +/*******************************************************************/ + +FileFSAttrs GetFSAttrsConstraints(const EvalContext *ctx, const Promise *pp) +{ + assert(ctx != NULL); + assert(pp != NULL); + + FileFSAttrs f = + { + .immutable = PromiseGetConstraintAsBoolean(ctx, "immutable", pp), + .haveimmutable = PromiseBundleOrBodyConstraintExists(ctx, "immutable", pp), + }; + + return f; +} + + /*******************************************************************/ FileRename GetRenameConstraints(const EvalContext *ctx, const Promise *pp) diff --git a/libpromises/attributes.h b/libpromises/attributes.h index f7089c1688..02237f08b1 100644 --- a/libpromises/attributes.h +++ b/libpromises/attributes.h @@ -68,6 +68,7 @@ ENTERPRISE_FUNC_0ARG_DECLARE(HashMethod, GetBestFileChangeHashMethod); FileChange GetChangeMgtConstraints(const EvalContext *ctx, const Promise *pp); FileCopy GetCopyConstraints(const EvalContext *ctx, const Promise *pp); FileDelete GetDeleteConstraints(const EvalContext *ctx, const Promise *pp); +FileFSAttrs GetFSAttrsConstraints(const EvalContext *ctx, const Promise *pp); FileLink GetLinkConstraints(const EvalContext *ctx, const Promise *pp); FileRename GetRenameConstraints(const EvalContext *ctx, const Promise *pp); FileSelect GetSelectConstraints(const EvalContext *ctx, const Promise *pp); diff --git a/libpromises/cf3.defs.h b/libpromises/cf3.defs.h index 6294c92868..47e9bffe53 100644 --- a/libpromises/cf3.defs.h +++ b/libpromises/cf3.defs.h @@ -1051,6 +1051,14 @@ typedef struct /*************************************************************************/ +typedef struct +{ + int immutable; + int haveimmutable; +} FileFSAttrs; + +/*************************************************************************/ + typedef struct { char *newname; @@ -1520,6 +1528,7 @@ typedef struct FilePerms perms; FileCopy copy; FileDelete delete; + FileFSAttrs fsattrs; char *content; FileRename rename; FileChange change; @@ -1571,6 +1580,7 @@ typedef struct int haveselect; int haverename; int havedelete; + int havefsattrs; int haveperms; int havechange; int havecopy; diff --git a/libpromises/mod_files.c b/libpromises/mod_files.c index 7f2abf904a..5ee17053ee 100644 --- a/libpromises/mod_files.c +++ b/libpromises/mod_files.c @@ -227,6 +227,15 @@ static const ConstraintSyntax delete_constraints[] = static const BodySyntax delete_body = BodySyntaxNew("delete", delete_constraints, NULL, SYNTAX_STATUS_NORMAL); +static const ConstraintSyntax fsattrs_constraints[] = +{ + CONSTRAINT_SYNTAX_GLOBAL, + ConstraintSyntaxNewBool("immutable", "true to set / false to clear the immutable flag", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewNull() +}; + +static const BodySyntax fsattrs_body = BodySyntaxNew("fsattrs", fsattrs_constraints, NULL, SYNTAX_STATUS_NORMAL); + static const ConstraintSyntax rename_constraints[] = { CONSTRAINT_SYNTAX_GLOBAL, @@ -339,6 +348,7 @@ static const ConstraintSyntax CF_FILES_BODIES[] = ConstraintSyntaxNewBody("copy_from", ©_from_body, "Criteria for copying file from a source", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewBool("create", "true/false whether to create non-existing file. Default value: false", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewBody("delete", &delete_body, "Criteria for deleting files", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewBody("fsattrs", &fsattrs_body, "Control file system attributes", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewString("content", CF_ANYSTRING, "Complete content the promised file should contain", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewBody("depth_search", &depth_search_body, "Criteria for file depth searches", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewBody("edit_defaults", &edit_defaults_body, "Default promise details for file edits", SYNTAX_STATUS_NORMAL), From 93b5bb77ba8639691db084b76e68aeb71428a1f4 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 6 Feb 2025 16:08:33 +0100 Subject: [PATCH 2/3] Files promise can now modify immutable bit in file system attributes Ticket: ENT-10961, CFE-1840 Changelog: Title Signed-off-by: Lars Erik Wik --- cf-agent/verify_files.c | 148 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/cf-agent/verify_files.c b/cf-agent/verify_files.c index bc65c046a0..827568ca93 100644 --- a/cf-agent/verify_files.c +++ b/cf-agent/verify_files.c @@ -59,6 +59,7 @@ #include #include #include /* PrepareChangesChroot(), RecordFileChangedInChroot() */ +#include static PromiseResult FindFilePromiserObjects(EvalContext *ctx, const Promise *pp); static PromiseResult VerifyFilePromise(EvalContext *ctx, char *path, const Promise *pp); @@ -303,6 +304,7 @@ static PromiseResult VerifyFilePromise(EvalContext *ctx, char *path, const Promi CfLock thislock; int exists; bool link = false; + bool is_immutable = false; Attributes attr = GetFilesAttributes(ctx, pp); @@ -351,6 +353,82 @@ static PromiseResult VerifyFilePromise(EvalContext *ctx, char *path, const Promi changes_path = chrooted_path; } + FSAttrsResult res = FSAttrsGetImmutableFlag(path, &is_immutable); + switch (res) + { + case FS_ATTRS_SUCCESS: + /* Nothing more to do */ + break; + case FS_ATTRS_FAILURE: + Log(LOG_LEVEL_VERBOSE, "Failed to get immutable bit for file '%s': %s", changes_path, GetErrorStr()); + break; + case FS_ATTRS_NOT_SUPPORTED: + Log(LOG_LEVEL_VERBOSE, "Failed to get immutable bit for file '%s': Operation not supported", changes_path); + break; + case FS_ATTRS_DOES_NOT_EXIST: + /* The file does not exist. Nothing more to do */ + break; + } + + if (a.havefsattrs && a.fsattrs.haveimmutable) + { + if (a.fsattrs.immutable) /* file should be immutable */ + { + if (is_immutable) /* file is already immutable */ + { + /* Temporarily remove the immutable flag */ + res = FSAttrsUpdateImmutableFlag(changes_path, false); + switch (res) + { + case FS_ATTRS_SUCCESS: + Log(LOG_LEVEL_VERBOSE, "Temporarily cleared immutable bit for file '%s'", changes_path); + break; + case FS_ATTRS_FAILURE: + /* Things still may be fine as long as the agent does not try to mutate the file */ + Log(LOG_LEVEL_VERBOSE, "Failed to temporarily clear immutable bit for file '%s': %s", changes_path, GetErrorStr()); + break; + case FS_ATTRS_NOT_SUPPORTED: + /* Things still may be fine as long as the agent does not try to mutate the file */ + Log(LOG_LEVEL_VERBOSE, "Failed to temporarily clear immutable bit for file '%s': Operation not supported", changes_path); + break; + case FS_ATTRS_DOES_NOT_EXIST: + /* The file does not exist. Nothing more to do */ + break; + } + } /* else then file is not immutable, but we will fix that later */ + } + else /* file should not be immutable */ + { + if (is_immutable) /* file is immutable */ + { + /* Remove immutable flag for good */ + res = FSAttrsUpdateImmutableFlag(changes_path, false); + switch (res) + { + case FS_ATTRS_SUCCESS: + RecordChange(ctx, pp, &a, "Cleared immutable bit for file '%s'", changes_path); + result = PromiseResultUpdate(result, PROMISE_RESULT_CHANGE); + break; + case FS_ATTRS_FAILURE: + RecordFailure(ctx, pp, &a, "Failed to clear immutable bit for file '%s'", changes_path); + result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL); + break; + case FS_ATTRS_NOT_SUPPORTED: + /* We will not treat this as a promise failure because this will happen on many platforms. Instead we will log a verbose message. */ + Log(LOG_LEVEL_VERBOSE, "Failed to temporarily clear immutable bit for file '%s': Operation not supported", changes_path); + break; + case FS_ATTRS_DOES_NOT_EXIST: + /* Nothing to do */ + break; + } + } + else /* file is not immutable */ + { + RecordNoChange(ctx, pp, &a, "Immutable bit is not set for file '%s' as promised", changes_path); + } + } + } + if (lstat(changes_path, &oslb) == -1) /* Careful if the object is a link */ { if ((a.create) || (a.touch)) @@ -609,6 +687,76 @@ static PromiseResult VerifyFilePromise(EvalContext *ctx, char *path, const Promi } exit: + ; /* A label can only be part of a statement and a declaration is not a + * statement. This is a quirk of standard C grammar. Hence, we put an + * empty statement here. */ + bool was_immutable = is_immutable; + is_immutable = false; + res = FSAttrsGetImmutableFlag(path, &is_immutable); + switch (res) + { + case FS_ATTRS_SUCCESS: + /* Nothing to do */ + break; + case FS_ATTRS_FAILURE: + Log(LOG_LEVEL_VERBOSE, "Failed to get immutable bit for file '%s': %s", changes_path, GetErrorStr()); + break; + case FS_ATTRS_NOT_SUPPORTED: + Log(LOG_LEVEL_VERBOSE, "Failed to get immutable bit for file '%s': Operation not supported", changes_path); + break; + case FS_ATTRS_DOES_NOT_EXIST: + /* Nothing to do */ + break; + } + + if (a.havefsattrs && a.fsattrs.haveimmutable && a.fsattrs.immutable) { + if (was_immutable && !is_immutable) + { + /* Reset immutable flag that we cleared before */ + res = FSAttrsUpdateImmutableFlag(changes_path, true); + switch (res) + { + case FS_ATTRS_SUCCESS: + Log(LOG_LEVEL_VERBOSE, "Reset immutable bit for file '%s' after temporarily clearing it", changes_path); + break; + case FS_ATTRS_FAILURE: + /* Things still may be fine as long as the agent does not try to mutate the file */ + Log(LOG_LEVEL_VERBOSE, "Failed to reset immutable bit for file '%s' after temporarily clearing it: %s", changes_path, GetErrorStr()); + break; + case FS_ATTRS_NOT_SUPPORTED: + /* Things still may be fine as long as the agent does not try to mutate the file */ + Log(LOG_LEVEL_VERBOSE, "Failed to reset immutable bit for file '%s' after temporarily clearing it: Operation not supported", changes_path); + break; + case FS_ATTRS_DOES_NOT_EXIST: + /* The file must have been removed by the agent. Nothing more to do */ + break; + } + } + else + { + /* The file was never immutable, but should be. We'll fix that now */ + res = FSAttrsUpdateImmutableFlag(changes_path, true); + switch (res) + { + case FS_ATTRS_SUCCESS: + RecordChange(ctx, pp, &a, "Set immutable bit for file '%s'", changes_path); + result = PromiseResultUpdate(result, PROMISE_RESULT_CHANGE); + break; + case FS_ATTRS_FAILURE: + RecordFailure(ctx, pp, &a, "Failed to set immutable bit for file '%s'", changes_path); + result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL); + break; + case FS_ATTRS_NOT_SUPPORTED: + /* We will not treat this as a promise failure because this will happen on many platforms. Instead we will log a verbose message. */ + Log(LOG_LEVEL_VERBOSE, "Failed set immutable bit for file '%s': Operation not supported", changes_path); + break; + case FS_ATTRS_DOES_NOT_EXIST: + /* The file must have been removed by the agent. Nothing more to do */ + break; + } + } + } + free(chrooted_path); if (AttrHasNoAction(&a)) { From 5da8b1161c39076b3eba077d5b898a33ea4ab038 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Fri, 14 Feb 2025 16:36:23 +0100 Subject: [PATCH 3/3] Added acceptance tests for immutable bit Ticket: ENT-10961, CFE-1840 Signed-off-by: Lars Erik Wik --- .../10_files/14_immutable/00_immutable.cf | 75 ++++++++++++++++++ .../10_files/14_immutable/00_immutable.cf.sub | 12 +++ .../10_files/14_immutable/01_immutable.cf | 78 ++++++++++++++++++ .../10_files/14_immutable/01_immutable.cf.sub | 12 +++ .../10_files/14_immutable/02_immutable.cf | 79 +++++++++++++++++++ .../10_files/14_immutable/02_immutable.cf.sub | 12 +++ 6 files changed, 268 insertions(+) create mode 100644 tests/acceptance/10_files/14_immutable/00_immutable.cf create mode 100644 tests/acceptance/10_files/14_immutable/00_immutable.cf.sub create mode 100644 tests/acceptance/10_files/14_immutable/01_immutable.cf create mode 100644 tests/acceptance/10_files/14_immutable/01_immutable.cf.sub create mode 100644 tests/acceptance/10_files/14_immutable/02_immutable.cf create mode 100644 tests/acceptance/10_files/14_immutable/02_immutable.cf.sub diff --git a/tests/acceptance/10_files/14_immutable/00_immutable.cf b/tests/acceptance/10_files/14_immutable/00_immutable.cf new file mode 100644 index 0000000000..fc47c36de1 --- /dev/null +++ b/tests/acceptance/10_files/14_immutable/00_immutable.cf @@ -0,0 +1,75 @@ +############################################################################## +# +# Test that immutable file CANNOT be edited by agent without specifying +# fsattrs immutable constraint. +# +############################################################################## + +body common control +{ + inputs => { "../../default.cf.sub" }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +body fsattrs set_immutable +{ + immutable => "true"; +} + +body fsattrs set_mutable +{ + immutable => "false"; +} + +bundle agent init +{ + files: + "/tmp/00_immutable.txt" + content => "I'm immutable", + fsattrs => set_immutable; +} + +bundle agent test +{ + meta: + "description" -> { "CFE-1840", "ENT-10961" } + string => "Test that immutable file cannot be edited by agent without specifying fsattrs immutable constraint"; + + "test_soft_fail" + string => "hpux|aix|windows", + meta => { "CFE-1840", "ENT-10961" }; + + commands: + "$(sys.cf_agent) -Kf $(this.promise_filename).sub"; +} + +bundle agent check +{ + vars: + "expected" + string => "I'm immutable"; + "actual" + string => readfile("/tmp/00_immutable.txt"); + + classes: + "ok" + expression => strcmp("$(actual)", "$(expected)"); + + methods: + "any" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; + + reports: + "Expected: '$(expected)', actual: '$(actual)'"; +} + +bundle agent destroy +{ + files: + # Make sure immutable bit is not set + "/tmp/00_immutable.txt" + fsattrs => set_mutable, + delete => tidy; +} diff --git a/tests/acceptance/10_files/14_immutable/00_immutable.cf.sub b/tests/acceptance/10_files/14_immutable/00_immutable.cf.sub new file mode 100644 index 0000000000..1d44e0a398 --- /dev/null +++ b/tests/acceptance/10_files/14_immutable/00_immutable.cf.sub @@ -0,0 +1,12 @@ +body fsattrs set_immutable +{ + # Nothing here +} + +bundle agent main +{ + files: + "/tmp/00_immutable.txt" + content => "I'm mutable", + fsattrs => set_immutable; +} diff --git a/tests/acceptance/10_files/14_immutable/01_immutable.cf b/tests/acceptance/10_files/14_immutable/01_immutable.cf new file mode 100644 index 0000000000..8607c67c73 --- /dev/null +++ b/tests/acceptance/10_files/14_immutable/01_immutable.cf @@ -0,0 +1,78 @@ +############################################################################## +# +# Test that immutable file CAN be edited by agent with fsattrs immutable +# constraint set to true. +# +############################################################################## + +body common control +{ + inputs => { "../../default.cf.sub" }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +body fsattrs set_immutable +{ + immutable => "true"; +} + +body fsattrs set_mutable +{ + immutable => "false"; +} + +bundle agent init +{ + files: + "/tmp/01_immutable.txt" + content => "I'm immutable", + fsattrs => set_immutable; +} + +bundle agent test +{ + meta: + "description" -> { "CFE-1840", "ENT-10961" } + string => "Test that immutable file CAN be edited by agent with fsattrs immutable constraint set to true"; + + # "test_soft_fail" will not work here, because init will not be able to set + # the immutable bit, hence the agent will be able to mutate it + "test_skip_unsupported" + string => "hpux|aix|windows", + meta => { "CFE-1840", "ENT-10961" }; + + commands: + "$(sys.cf_agent) -Kf $(this.promise_filename).sub"; +} + +bundle agent check +{ + vars: + "expected" + string => "I'm mutable"; + "actual" + string => readfile("/tmp/01_immutable.txt"); + + classes: + "ok" + expression => strcmp("$(actual)", "$(expected)"); + + methods: + "any" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; + + reports: + any:: + "Expected: '$(expected)', actual: '$(actual)'"; +} + +bundle agent destroy +{ + files: + # Make sure immutable bit is not set + "/tmp/01_immutable.txt" + fsattrs => set_mutable, + delete => tidy; +} diff --git a/tests/acceptance/10_files/14_immutable/01_immutable.cf.sub b/tests/acceptance/10_files/14_immutable/01_immutable.cf.sub new file mode 100644 index 0000000000..11a444da0e --- /dev/null +++ b/tests/acceptance/10_files/14_immutable/01_immutable.cf.sub @@ -0,0 +1,12 @@ +body fsattrs set_immutable +{ + immutable => "true"; +} + +bundle agent main +{ + files: + "/tmp/01_immutable.txt" + content => "I'm mutable", + fsattrs => set_immutable; +} diff --git a/tests/acceptance/10_files/14_immutable/02_immutable.cf b/tests/acceptance/10_files/14_immutable/02_immutable.cf new file mode 100644 index 0000000000..34d3fab003 --- /dev/null +++ b/tests/acceptance/10_files/14_immutable/02_immutable.cf @@ -0,0 +1,79 @@ +############################################################################## +# +# Test that immutable bit can be cleared by agent with fsattrs immutable +# constraint set to false. +# +############################################################################## + +body common control +{ + inputs => { "../../default.cf.sub" }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +body fsattrs set_immutable +{ + immutable => "true"; +} + +body fsattrs set_mutable +{ + immutable => "false"; +} + +bundle agent init +{ + files: + "/tmp/02_immutable.txt" + content => "I'm immutable", + fsattrs => set_immutable; +} + +bundle agent test +{ + meta: + "description" -> { "CFE-1840", "ENT-10961" } + string => "Test that immutable bit can be cleared by agent with fsattrs immutable constraint set to false"; + + # "test_soft_fail" will not work here, because init will not be able to set + # the immutable bit, hence the agent will be able to mutate it, even without + # actually clearing it. + "test_skip_unsupported" + string => "hpux|aix|windows", + meta => { "CFE-1840", "ENT-10961" }; + + commands: + "$(sys.cf_agent) -Kf $(this.promise_filename).sub"; +} + +bundle agent check +{ + vars: + "expected" + string => "I'm mutable"; + "actual" + string => readfile("/tmp/02_immutable.txt"); + + classes: + "ok" + expression => strcmp("$(actual)", "$(expected)"); + + methods: + "any" + usebundle => dcs_passif("ok", "$(this.promise_filename)"), + inherit => "true"; + + reports: + any:: + "Expected: '$(expected)', actual: '$(actual)'"; +} + +bundle agent destroy +{ + files: + # Make sure immutable bit is not set + "/tmp/02_immutable.txt" + fsattrs => set_mutable, + delete => tidy; +} diff --git a/tests/acceptance/10_files/14_immutable/02_immutable.cf.sub b/tests/acceptance/10_files/14_immutable/02_immutable.cf.sub new file mode 100644 index 0000000000..18e3a0b1dc --- /dev/null +++ b/tests/acceptance/10_files/14_immutable/02_immutable.cf.sub @@ -0,0 +1,12 @@ +body fsattrs set_immutable +{ + immutable => "false"; +} + +bundle agent main +{ + files: + "/tmp/02_immutable.txt" + content => "I'm mutable", + fsattrs => set_immutable; +}