diff --git a/cf-agent/nfs.c b/cf-agent/nfs.c index 4ba7d095c7..0a7889211b 100644 --- a/cf-agent/nfs.c +++ b/cf-agent/nfs.c @@ -50,9 +50,18 @@ static Item *FSTABLIST = NULL; /* GLOBAL_X */ static void GetHostAndSource(const char *buf, char *host, char *source); -static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *options); +static char **ParseOptionsList(const char *opts); +static void FreeOptionsList(char **arr); +static bool InverseOptions(const char *a, const char *b); +static bool OptionPresent(const char *opt, char **actual); +static bool InversePresent(const char *opt, char **actual); +static char *RemountOptionString(const char *opts); + +static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *fstype, char *options); static bool MatchFSInFstab(char *match); static void DeleteThisItem(Item **liststart, Item *entry); +static char *GetFstabEntryOptions(char *mountpt); +static void ReplaceFstabEntry(Item **liststart, char *mountpt, char *new_entry); static const char *const VMOUNTCOMM[] = { @@ -174,6 +183,234 @@ static void GetHostAndSource(const char *buf, char *host, char *source) source[source_index] = '\0'; } +/* Parse comma-separated options string into an array of individual option strings. + * Returns a newly allocated array of char* pointers, terminated by NULL. + * Caller must free the array itself (not the individual strings). */ +static char **ParseOptionsList(const char *opts) +{ + if (opts == NULL || opts[0] == '\0') + { + char **arr = xcalloc(1, sizeof(char *)); + arr[0] = NULL; + return arr; + } + + /* First pass: count options */ + int count = 0; + for (const char *p = opts; *p; p++) + { + if (*p == ',') + count++; + } + count++; /* number of tokens = number of commas + 1 */ + + char **arr = xcalloc(count + 1, sizeof(char *)); + char *copy = xstrdup(opts); + int idx = 0; + char *token = strtok(copy, ","); + while (token != NULL && idx < count) + { + arr[idx++] = xstrdup(token); + token = strtok(NULL, ","); + } + free(copy); + return arr; +} + +static void FreeOptionsList(char **arr) +{ + if (arr == NULL) + return; + for (int i = 0; arr[i] != NULL; i++) + free(arr[i]); + free(arr); +} + +static bool InverseOptions(const char *a, const char *b) +/* True if 'a' and 'b' are mutually exclusive mount options that cannot both + * hold: ro/rw, hard/soft, sync/async, noatime/relatime, or a "no"-prefixed + * option and its bare form (e.g. noexec/exec). */ +{ + /* A "no"-prefixed option vs its bare form, in either direction. */ + if ((strncmp(a, "no", 2) == 0) && (strcmp(a + 2, b) == 0)) + { + return true; + } + if ((strncmp(b, "no", 2) == 0) && (strcmp(b + 2, a) == 0)) + { + return true; + } + + /* Inverse pairs that do not share the "no" prefix. */ + static const char *const pairs[][2] = { + { "noatime", "relatime" }, + { "hard", "soft" }, + { "sync", "async" }, + { "ro", "rw" }, + }; + for (size_t i = 0; i < sizeof(pairs) / sizeof(pairs[0]); i++) + { + if (((strcmp(a, pairs[i][0]) == 0) && (strcmp(b, pairs[i][1]) == 0)) || + ((strcmp(a, pairs[i][1]) == 0) && (strcmp(b, pairs[i][0]) == 0))) + { + return true; + } + } + return false; +} + +static bool OptionPresent(const char *opt, char **actual) +/* True if 'opt' is present in the live options, directly or via a + * tcp/udp<->proto= alias. */ +{ + for (int a = 0; actual[a] != NULL; a++) + { + if (strcmp(opt, actual[a]) == 0) + { + return true; + } + if ((strcmp(opt, "tcp") == 0 && strcmp(actual[a], "proto=tcp") == 0) || + (strcmp(opt, "udp") == 0 && strcmp(actual[a], "proto=udp") == 0) || + (strcmp(opt, "proto=tcp") == 0 && strcmp(actual[a], "tcp") == 0) || + (strcmp(opt, "proto=udp") == 0 && strcmp(actual[a], "udp") == 0)) + { + return true; + } + } + return false; +} + +static bool InversePresent(const char *opt, char **actual) +/* True if an option contradicting 'opt' is present in the live mount. */ +{ + for (int a = 0; actual[a] != NULL; a++) + { + if (InverseOptions(opt, actual[a])) + { + return true; + } + } + return false; +} + +static char *RemountOptionString(const char *opts) +/* Options for a `mount -o remount,...` command, expanding "defaults" to its + * positives rw,suid,dev,exec,async: util-linux ignores the flags implied by a + * bare "defaults" on a remount, so the explicit form is needed to restore a + * drifted mount in place. Newly allocated (caller frees), NULL if opts empty. */ +{ + if ((opts == NULL) || (opts[0] == '\0')) + { + return NULL; + } + + char **arr = ParseOptionsList(opts); + char buf[CF_BUFSIZE]; + size_t n = 0; + buf[0] = '\0'; + for (int i = 0; arr[i] != NULL; i++) + { + const char *tok = (strcmp(arr[i], "defaults") == 0) + ? "rw,suid,dev,exec,async" : arr[i]; + int w = snprintf(buf + n, sizeof(buf) - n, "%s%s", (i > 0) ? "," : "", tok); + if ((w < 0) || ((size_t) w >= sizeof(buf) - n)) + { + break; /* option list longer than CF_BUFSIZE; stop appending */ + } + n += (size_t) w; + } + FreeOptionsList(arr); + return xstrdup(buf); +} + +bool OptionsSubsetMatches(const char *promised_opts, const char *actual_opts) +/* Whether the live mount (actual_opts, from /proc/mounts) satisfies the promise. + * Only named options are enforced; unnamed ones are left alone. The promise is + * resolved "last wins" like `mount -o` (defaults,ro is read-only, ro,rw is rw), + * with "defaults" expanded to rw,suid,dev,exec,async (auto/nouser aren't runtime + * state). Each surviving option must then hold: its inverse absent, and it + * present or a default-on flag (suid/dev/exec/async only show when negated). */ +{ + if (promised_opts == NULL || promised_opts[0] == '\0') + { + return true; + } + + char **promised = ParseOptionsList(promised_opts); + char **actual = ParseOptionsList(actual_opts); + + /* Expand "defaults" in place, preserving order for the last-wins pass. */ + Seq *eff = SeqNew(16, free); + for (int p = 0; promised[p] != NULL; p++) + { + if (strcmp(promised[p], "defaults") == 0) + { + static const char *const comps[] = { "rw", "suid", "dev", "exec", "async" }; + for (size_t c = 0; c < sizeof(comps) / sizeof(comps[0]); c++) + { + SeqAppend(eff, xstrdup(comps[c])); + } + } + else + { + SeqAppend(eff, xstrdup(promised[p])); + } + } + + /* On-by-default flags the kernel does not echo (only their negatives show). */ + static const char *const default_on[] = { "suid", "dev", "exec", "async" }; + + bool mismatch = false; + for (size_t i = 0; (i < SeqLength(eff)) && !mismatch; i++) + { + const char *x = SeqAt(eff, i); + + /* Last wins: skip this option if a later one overrides it (its inverse) + * or repeats it. */ + bool overridden = false; + for (size_t j = i + 1; j < SeqLength(eff); j++) + { + const char *y = SeqAt(eff, j); + if (InverseOptions(x, y)) + { + Log(LOG_LEVEL_VERBOSE, "Mount option '%s' overridden by later '%s'", x, y); + overridden = true; + break; + } + if (strcmp(x, y) == 0) + { + overridden = true; + break; + } + } + if (overridden) + { + continue; + } + + bool is_default_on = false; + for (size_t d = 0; d < sizeof(default_on) / sizeof(default_on[0]); d++) + { + if (strcmp(x, default_on[d]) == 0) + { + is_default_on = true; + break; + } + } + + if (InversePresent(x, actual) || !(OptionPresent(x, actual) || is_default_on)) + { + mismatch = true; + } + } + + SeqDestroy(eff); + FreeOptionsList(promised); + FreeOptionsList(actual); + + return !mismatch; +} + bool LoadMountInfo(Seq *list) /* This is, in fact, the most portable way to read the mount info! */ /* Depressing, isn't it? */ @@ -346,21 +583,41 @@ bool LoadMountInfo(Seq *list) Log(LOG_LEVEL_DEBUG, "LoadMountInfo: host '%s', source '%s', mounton '%s'", host, source, mounton); + /* Extract the actual mount options from the parenthesized portion of mount output. + * mount -va output format: host:source on /mountpoint type fstype (opts) */ + char mountopts[CF_BUFSIZE]; + mountopts[0] = '\0'; + char *paren = strstr(vbuff, "("); + if (paren != NULL) + { + char *end = strchr(paren, ')'); + if (end != NULL) + { + strlcpy(mountopts, paren + 1, sizeof(mountopts)); + /* Strip trailing whitespace */ + size_t len = strlen(mountopts); + while (len > 0 && (mountopts[len - 1] == ' ' || mountopts[len - 1] == '\t')) + { + mountopts[--len] = '\0'; + } + } + } + if (panfs) { - AugmentMountInfo(list, host, source, mounton, "panfs"); + AugmentMountInfo(list, host, source, mounton, "panfs", mountopts); } else if (nfs) { - AugmentMountInfo(list, host, source, mounton, "nfs"); + AugmentMountInfo(list, host, source, mounton, "nfs", mountopts); } else if (cifs) { - AugmentMountInfo(list, host, source, mounton, "cifs"); + AugmentMountInfo(list, host, source, mounton, "cifs", mountopts); } else { - AugmentMountInfo(list, host, source, mounton, NULL); + AugmentMountInfo(list, host, source, mounton, NULL, mountopts); } } @@ -373,7 +630,7 @@ bool LoadMountInfo(Seq *list) /*******************************************************************/ -static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *options) +static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, char *fstype, char *options) { Mount *entry = xcalloc(1, sizeof(Mount)); @@ -392,9 +649,19 @@ static void AugmentMountInfo(Seq *list, char *host, char *source, char *mounton, entry->mounton = xstrdup(mounton); } - if (options) + /* Store the fstype in options so IsForeignFileSystem can detect + * foreign filesystems via strstr(entry->options, "nfs"/"panfs"/"cifs"). */ + if (fstype) { - entry->options = xstrdup(options); + entry->options = xstrdup(fstype); + } + + /* Store the full kernel-resolved options in raw_opts. + * For unmounted filesystems (options == NULL or empty), raw_opts stays NULL + * and will be checked in FileSystemMountedCorrectly as "not mounted". */ + if (options != NULL && options[0] != '\0') + { + entry->raw_opts = xstrdup(options); } SeqAppend(list, entry); @@ -408,25 +675,11 @@ void DeleteMountInfo(Seq *list) { Mount *entry = SeqAt(list, i); - if (entry->host) - { - free(entry->host); - } - - if (entry->source) - { - free(entry->source); - } - - if (entry->mounton) - { - free(entry->mounton); - } - - if (entry->options) - { - free(entry->options); - } + free(entry->host); + free(entry->source); + free(entry->mounton); + free(entry->options); + free(entry->raw_opts); } SeqClear(list); @@ -534,6 +787,7 @@ int VerifyInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promi if (!MatchFSInFstab(mountpt)) { + /* CFE-90: Entry not in fstab - add it */ AppendItem(&FSTABLIST, fstab, NULL); FSTAB_EDITS++; cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, "Adding file system entry '%s' to '%s'", fstab, @@ -541,6 +795,29 @@ int VerifyInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promi *result = PromiseResultUpdate(*result, PROMISE_RESULT_CHANGE); changes += 1; } + else + { + /* CFE-90: Entry exists - rewrite it if the options differ. The compare + * is exact (order-sensitive) on purpose: for duplicated/conflicting + * options the kernel uses the last one, so option order is significant + * in an fstab line and must not be normalized away. Since + * GetFstabEntryOptions now reads the real options field (it previously + * returned the fstype), this converges - a differently-ordered entry is + * rewritten once to the promised form, then matches - rather than being + * rewritten on every run. */ + char *existing_opts = GetFstabEntryOptions(mountpt); + if (existing_opts != NULL && strcmp(existing_opts, opts) != 0) + { + /* Replace the entire fstab entry with the corrected options */ + ReplaceFstabEntry(&FSTABLIST, mountpt, fstab); + FSTAB_EDITS++; + cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, "Updating file system entry for '%s' in '%s' (options: '%s' -> '%s')", + mountpt, VFSTAB[VSYSTEMHARDCLASS], existing_opts, opts); + *result = PromiseResultUpdate(*result, PROMISE_RESULT_CHANGE); + changes += 1; + } + free(existing_opts); + } free(opts); return changes; @@ -618,7 +895,7 @@ int VerifyNotInFstab(EvalContext *ctx, char *name, const Attributes *a, const Pr if (strstr(line, "busy")) { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be removed from '%s'", + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be removed from '%s'", mountpt, VFSTAB[VSYSTEMHARDCLASS]); *result = PromiseResultUpdate(*result, PROMISE_RESULT_INTERRUPTED); free(line); @@ -679,55 +956,65 @@ PromiseResult VerifyMount(EvalContext *ctx, char *name, const Attributes *a, con } PromiseResult result = PROMISE_RESULT_NOOP; - if (!DONTDO) + + /* CFE-3366: gate the mount on the promise action, not just DONTDO, so a + * dry-run (or action_policy => "warn") reports a warning and does not + * define promise_repaired for a run that changes nothing. */ + if (!MakingInternalChanges(ctx, pp, a, &result, "mount '%s' to keep promise", mountpt)) { - if (StringEqual(a->mount.mount_type, "panfs")) - { - snprintf(comm, CF_BUFSIZE, "%s -t panfs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); - } - else if (StringEqual(a->mount.mount_type, "cifs")) - { - snprintf(comm, CF_BUFSIZE, "%s -t cifs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); - } - else - { - snprintf(comm, CF_BUFSIZE, "%s -o %s %s:%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); - } + free(opts); + return result; + } - if ((pfp = cf_popen(comm, "r", true)) == NULL) - { - Log(LOG_LEVEL_ERR, "Failed to open pipe from '%s'", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS])); - return PROMISE_RESULT_FAIL; - } + if (StringEqual(a->mount.mount_type, "panfs")) + { + snprintf(comm, CF_BUFSIZE, "%s -t panfs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); + } + else if (StringEqual(a->mount.mount_type, "cifs")) + { + snprintf(comm, CF_BUFSIZE, "%s -t cifs -o %s %s%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); + } + else + { + snprintf(comm, CF_BUFSIZE, "%s -o %s %s:%s %s", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), opts, host, rmountpt, mountpt); + } - size_t line_size = CF_BUFSIZE; - char *line = xmalloc(line_size); + if ((pfp = cf_popen(comm, "r", true)) == NULL) + { + Log(LOG_LEVEL_ERR, "Failed to open pipe from '%s'", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS])); + free(opts); + return PROMISE_RESULT_FAIL; + } - ssize_t res = CfReadLine(&line, &line_size, pfp); + size_t line_size = CF_BUFSIZE; + char *line = xmalloc(line_size); - if (res == -1) - { - if (!feof(pfp)) - { - Log(LOG_LEVEL_ERR, "Unable to read output of mount command. (fread: %s)", GetErrorStr()); - cf_pclose(pfp); - free(line); - return PROMISE_RESULT_FAIL; - } - } - else if ((strstr(line, "busy")) || (strstr(line, "Busy"))) + ssize_t res = CfReadLine(&line, &line_size, pfp); + + if (res == -1) + { + if (!feof(pfp)) { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be mounted", mountpt); - result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); + Log(LOG_LEVEL_ERR, "Unable to read output of mount command. (fread: %s)", GetErrorStr()); cf_pclose(pfp); free(line); - return 1; + free(opts); + return PROMISE_RESULT_FAIL; } - - free(line); + } + else if ((strstr(line, "busy")) || (strstr(line, "Busy"))) + { + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be mounted", mountpt); + result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); cf_pclose(pfp); + free(line); + free(opts); + return result; } + free(line); + cf_pclose(pfp); + /* Since opts is either Rlist2String or xstrdup'd, we need to always free it */ free(opts); @@ -748,40 +1035,51 @@ PromiseResult VerifyUnmount(EvalContext *ctx, char *name, const Attributes *a, c mountpt = name; PromiseResult result = PROMISE_RESULT_NOOP; - if (!DONTDO) + + /* CFE-3366: gate the unmount on the promise action, not just DONTDO, so a + * dry-run (or action_policy => "warn") reports a warning and does not + * define promise_repaired for a run that changes nothing. */ + if (!MakingInternalChanges(ctx, pp, a, &result, "unmount '%s' to keep promise", mountpt)) { - snprintf(comm, CF_BUFSIZE, "%s %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS], mountpt); + return result; + } - if ((pfp = cf_popen(comm, "r", true)) == NULL) - { - Log(LOG_LEVEL_ERR, "Failed to open pipe from %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS]); - return result; - } + snprintf(comm, CF_BUFSIZE, "%s %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS], mountpt); - size_t line_size = CF_BUFSIZE; - char *line = xmalloc(line_size); + if ((pfp = cf_popen(comm, "r", true)) == NULL) + { + Log(LOG_LEVEL_ERR, "Failed to open pipe from %s", VUNMOUNTCOMM[VSYSTEMHARDCLASS]); + return result; + } - ssize_t res = CfReadLine(&line, &line_size, pfp); - if (res == -1) - { - cf_pclose(pfp); - free(line); + size_t line_size = CF_BUFSIZE; + char *line = xmalloc(line_size); - if (!feof(pfp)) - { - Log(LOG_LEVEL_ERR, "Unable to read output of unmount command. (fread: %s)", GetErrorStr()); - return result; - } - } - else if (res > 0 && ((strstr(line, "busy")) || (strstr(line, "Busy")))) + ssize_t res = CfReadLine(&line, &line_size, pfp); + if (res == -1) + { + /* CfReadLine() returns -1 both at end-of-output and on a read error. A + * successful unmount is silent, so EOF is the normal case here; only a + * genuine read error should be reported. feof() must be consulted + * before cf_pclose() closes and invalidates the stream. */ + bool read_error = !feof(pfp); + cf_pclose(pfp); + free(line); + + if (read_error) { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be unmounted", mountpt); - result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); - cf_pclose(pfp); - free(line); + Log(LOG_LEVEL_ERR, "Unable to read output of unmount command. (fread: %s)", GetErrorStr()); return result; } } + else if (res > 0 && ((strstr(line, "busy")) || (strstr(line, "Busy")))) + { + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a, "The device under '%s' cannot be unmounted", mountpt); + result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED); + cf_pclose(pfp); + free(line); + return result; + } cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, "Unmounting '%s' to keep promise", mountpt); result = PromiseResultUpdate(result, PROMISE_RESULT_CHANGE); @@ -956,6 +1254,276 @@ static void DeleteThisItem(Item **liststart, Item *entry) } } +/*******************************************************************/ +/* CFE-90: Helper functions for fstab options comparison */ +/*******************************************************************/ + +static char *GetFstabEntryOptions(char *mountpt) +/* Extract the options field from the fstab entry matching mountpt. + * Returns a dynamically allocated string or NULL. */ +{ + for (Item *ip = FSTABLIST; ip != NULL; ip = ip->next) + { + if (ip->name == NULL || ip->name[0] == '#') + { + continue; + } + + /* Parse the fstab line to find the options field */ + char *orig = xstrdup(ip->name); + char *saveptr = NULL; + char *token = strtok_r(orig, " \t", &saveptr); + int field = 0; + bool found = false; + + while (token != NULL && !found) + { + if (field == 1) + { + if (strcmp(token, mountpt) == 0) + { + /* Found matching mountpoint - skip field 2 (fstype), return field 3 (options) */ + char *skip_tok = strtok_r(NULL, " \t", &saveptr); /* field 2: type */ + if (skip_tok != NULL) + { + char *tok = strtok_r(NULL, " \t", &saveptr); /* field 3: options */ + if (tok != NULL) + { + free(orig); + return xstrdup(tok); + } + } + } + } + field++; + token = strtok_r(NULL, " \t", &saveptr); + } + free(orig); + } + + return NULL; +} + +static void ReplaceFstabEntry(Item **liststart, char *mountpt, char *new_entry) +/* Replace the fstab entry for mountpt with new_entry */ +{ + for (Item *ip = *liststart; ip != NULL; ip = ip->next) + { + if (ip->name != NULL && ip->name[0] != '#') + { + char *orig = xstrdup(ip->name); + char *saveptr = NULL; + char *token = strtok_r(orig, " \t", &saveptr); + int field = 0; + bool found = false; + + while (token != NULL && !found) + { + if (field == 1) + { + if (strcmp(token, mountpt) == 0) + { + /* Found matching mountpoint - replace the entire line */ + free(ip->name); + ip->name = xstrdup(new_entry); + found = true; + } + } + field++; + token = strtok_r(NULL, " \t", &saveptr); + } + free(orig); + if (found) + { + break; + } + } + } +} + +/*******************************************************************/ +/* CFE-90: Remount reconciliation */ +/*******************************************************************/ + +static bool LiveMountConverged(const char *name, const Attributes *a) +/* Re-read the mount table from scratch (the cached global list is stale after + * a mount operation) and report whether the filesystem now mounted at 'name' + * satisfies the promise: correct source (when specified) and, when options are + * promised, a superset of the promised options. NFS-specific options + * (transport, NFS version, etc.) cannot be changed by a remount - see nfs(5) + * "THE REMOUNT OPTION": https://man7.org/linux/man-pages/man5/nfs.5.html + * The remount call can still return success without applying them, so we + * verify the resulting state rather than trust the command's exit status. */ +{ + assert(a != NULL); + Seq *tmp = SeqNew(100, free); + bool converged = false; + + if (LoadMountInfo(tmp)) + { + for (size_t i = 0; i < SeqLength(tmp); i++) + { + Mount *mp = SeqAt(tmp, i); + if (mp == NULL || mp->mounton == NULL || strcmp(mp->mounton, name) != 0) + { + continue; + } + + /* Something is mounted here - check source, then options. */ + if ((a->mount.mount_source != NULL) + && ((mp->source == NULL) || (strcmp(mp->source, a->mount.mount_source) != 0))) + { + converged = false; + } + else if ((a->mount.mount_server != NULL) + && ((mp->host == NULL) || (strcmp(mp->host, a->mount.mount_server) != 0))) + { + /* CFE-2350: the server is part of the mount identity, so a + * remounted-in-place mount that kept the old server has not + * converged - a remount in place cannot change the server, so + * reconciling it requires unmount_mount in remount_methods. */ + converged = false; + } + else if (a->mount.mount_options != NULL) + { + char *opts = Rlist2String(a->mount.mount_options, ","); + converged = (mp->raw_opts != NULL) && OptionsSubsetMatches(opts, mp->raw_opts); + free(opts); + } + else + { + converged = true; + } + break; + } + } + + DeleteMountInfo(tmp); + SeqDestroy(tmp); + return converged; +} + +PromiseResult ReconcileMountOptions(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp) +/* CFE-90: Reconcile an already-mounted filesystem that has drifted from the + * promise, trying each a->mount.remount_methods mechanism in order (default: + * just "remount"; the disruptive "unmount_mount" is opt-in) and re-checking + * after each. Honors DONTDO; reports its own cfPS outcome. */ +{ + assert(a != NULL); + PromiseResult result = PROMISE_RESULT_NOOP; + char *opts = Rlist2String(a->mount.mount_options, ","); + int timeout = (a->mount.remount_timeout != CF_NOINT) ? a->mount.remount_timeout : RPCTIMEOUT; + + /* Ordered method list: promise-specified, else the default remount. */ + Seq *methods = SeqNew(4, NULL); /* borrows const char*, does not own them */ + if (a->mount.remount_methods != NULL) + { + for (const Rlist *rp = a->mount.remount_methods; rp != NULL; rp = rp->next) + { + SeqAppend(methods, RlistScalarValue(rp)); + } + } + else + { + /* Default: in-place remount only. unmount_mount tears the filesystem + * down and back up, so it is opt-in (needed for options a remount + * can't change, e.g. NFS-negotiated vers=/rsize= or the server). */ + SeqAppend(methods, "remount"); + } + + /* CFE-3366: gate on the promise action, not just DONTDO, so a dry-run or + * action_policy => "warn" reports a warning without defining + * promise_repaired on a run that changes nothing. */ + if (!MakingInternalChanges(ctx, pp, a, &result, + "reconcile mount '%s' to promised options '%s'", name, + (opts != NULL) ? opts : "")) + { + SeqDestroy(methods); + free(opts); + return result; + } + + bool converged = false; + for (size_t i = 0; (i < SeqLength(methods)) && !converged; i++) + { + const char *method = SeqAt(methods, i); + + if (strcmp(method, "remount") == 0) + { + char comm[CF_BUFSIZE]; + char *ropts = RemountOptionString(opts); + if (ropts != NULL) + { + snprintf(comm, CF_BUFSIZE, "%s -o remount,%s %s", + CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), ropts, name); + } + else + { + snprintf(comm, CF_BUFSIZE, "%s -o remount %s", + CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS]), name); + } + free(ropts); + + Log(LOG_LEVEL_VERBOSE, "Reconciling '%s' via remount: %s", name, comm); + SetTimeOut(timeout); + + FILE *pfp = cf_popen(comm, "r", true); + if (pfp == NULL) + { + Log(LOG_LEVEL_ERR, "Failed to open pipe from '%s'", CommandArg0(VMOUNTCOMM[VSYSTEMHARDCLASS])); + } + else + { + size_t line_size = CF_BUFSIZE; + char *line = xmalloc(line_size); + while (CfReadLine(&line, &line_size, pfp) != -1) + { + /* drain command output */ + } + free(line); + cf_pclose(pfp); + } + } + else if (strcmp(method, "unmount_mount") == 0) + { + /* Reuse the tested helpers - both honor DONTDO and build the correct + * per-fstype command. This also handles a wrong-source mount, which + * an in-place remount cannot fix. */ + Log(LOG_LEVEL_VERBOSE, "Reconciling '%s' via unmount + mount", name); + SetTimeOut(timeout); + result = PromiseResultUpdate(result, VerifyUnmount(ctx, name, a, pp)); + result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp)); + } + else + { + Log(LOG_LEVEL_WARNING, "Unknown remount_method '%s' for '%s' - skipping", method, name); + continue; + } + + /* Verify-after-act: confirm the live mount actually satisfies the promise. */ + if (LiveMountConverged(name, a)) + { + cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_CHANGE, pp, a, + "Reconciled mount '%s' to promised options '%s' via '%s'", name, + (opts != NULL) ? opts : "", method); + result = PromiseResultUpdate(result, PROMISE_RESULT_CHANGE); + converged = true; + } + } + + if (!converged) + { + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a, + "Could not reconcile mount '%s' to promised options '%s'", name, + (opts != NULL) ? opts : ""); + result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL); + } + + SeqDestroy(methods); + free(opts); + return result; +} + void CleanupNFS(void) { Log(LOG_LEVEL_VERBOSE, "Number of changes observed in '%s' is %d", VFSTAB[VSYSTEMHARDCLASS], FSTAB_EDITS); diff --git a/cf-agent/nfs.h b/cf-agent/nfs.h index 03567c7b35..b3c88c21bf 100644 --- a/cf-agent/nfs.h +++ b/cf-agent/nfs.h @@ -28,7 +28,20 @@ #include #include // Seq -bool LoadMountInfo(Seq *list); +extern bool LoadMountInfo(Seq *list); + +/* Option subset matching for CFE-90 mount option verification. + * Checks whether all options in 'promised_opts' are present in + * 'actual_opts' (kernel-resolved options). Kernel-added NFS + * auto-negotiated options are ignored. Returns true if all + * user-specified options are satisfied. */ +extern bool OptionsSubsetMatches(const char *promised_opts, const char *actual_opts); + +/* CFE-90: Reconcile an already-mounted filesystem whose source or options + * diverge from the promise, using the mechanisms in a->mount.remount_methods + * (in order, verifying after each). Only invoked when remount is enabled. */ +PromiseResult ReconcileMountOptions(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp); + void DeleteMountInfo(Seq *list); int VerifyNotInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp, PromiseResult *result); int VerifyInFstab(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp, PromiseResult *result); diff --git a/cf-agent/verify_storage.c b/cf-agent/verify_storage.c index 698585b9d8..bb9eb5c9ba 100644 --- a/cf-agent/verify_storage.c +++ b/cf-agent/verify_storage.c @@ -101,18 +101,11 @@ PromiseResult VerifyStoragePromise(EvalContext *ctx, char *path, const Promise * /* No parameter conflicts here */ - if (a.mount.unmount) - { - if ((a.mount.mount_source)) - { - Log(LOG_LEVEL_VERBOSE, "An unmount promise indicates a mount-source information - probably an error"); - } - if ((a.mount.mount_server)) - { - Log(LOG_LEVEL_VERBOSE, "An unmount promise indicates a mount-server information - probably an error"); - } - } - else if (a.havemount) + /* CFE-2350: mount_source / mount_server on an unmount promise are not an + * error - they select which specific mount to unmount (e.g. one from a + * particular server) rather than every mount of a type. Only a *mount* + * promise needs both. */ + if (!a.mount.unmount && a.havemount) { if ((a.mount.mount_source == NULL) || (a.mount.mount_server == NULL)) { @@ -341,6 +334,7 @@ static PromiseResult VolumeScanArrivals(ARG_UNUSED char *file, ARG_UNUSED const #if !defined(__MINGW32__) static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes *a) { + assert(a != NULL); bool found = false; for (size_t i = 0; i < SeqLength(list); i++) @@ -366,11 +360,42 @@ static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes * mp->host, mp->source, name); return false; } - else + + /* CFE-2350: the server is part of mount identity, but acting on a + * mismatch is disruptive (umount+mount), so only check it when the + * promise opts in via remount or unmount. */ + if ((a->mount.remount || a->mount.unmount) + && (a->mount.mount_server != NULL) + && ((mp->host == NULL) || (strcmp(mp->host, a->mount.mount_server) != 0))) { - Log(LOG_LEVEL_VERBOSE, "File system '%s' seems to be mounted correctly", mp->source); - break; + Log(LOG_LEVEL_INFO, + "Filesystem on '%s' is from server '%s', not the promised server '%s'", + name, mp->host ? mp->host : "(unknown)", a->mount.mount_server); + return false; + } + + /* CFE-90: option drift is only managed when 'remount' is on; without + * it a mount with the correct source is "mounted correctly" whatever + * the options (they still drive the initial mount and fstab). */ + if (a->mount.remount && a->mount.mount_options != NULL) + { + char *opts = Rlist2String(a->mount.mount_options, ","); + if (mp->raw_opts == NULL || mp->raw_opts[0] == '\0' + || !OptionsSubsetMatches(opts, mp->raw_opts)) + { + Log(LOG_LEVEL_INFO, + "Mount options for '%s' do not match promise (actual: '%s', promised: '%s')", + name, + mp->raw_opts ? mp->raw_opts : "(none)", + opts); + free(opts); + return false; + } + free(opts); } + + Log(LOG_LEVEL_VERBOSE, "File system '%s' seems to be mounted correctly", mp->source); + break; } } @@ -378,8 +403,10 @@ static bool FileSystemMountedCorrectly(Seq *list, char *name, const Attributes * { if (!a->mount.unmount) { + /* CFE-1863: do not arm MountAll ('mount -a') here - the caller + * mounts this one filesystem surgically. CF_MOUNTALL is reserved + * for the explicit 'mountfilesystems' agent control. */ Log(LOG_LEVEL_VERBOSE, "File system '%s' seems not to be mounted correctly", name); - CF_MOUNTALL = true; } } @@ -440,7 +467,7 @@ static bool IsForeignFileSystem(struct stat *childstat, char *dir) static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attributes *a, const Promise *pp) { - char *options; + assert(a != NULL); char dir[CF_BUFSIZE]; int changes = 0; @@ -454,13 +481,28 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr return PROMISE_RESULT_INTERRUPTED; } - options = Rlist2String(a->mount.mount_options, ","); - PromiseResult result = PROMISE_RESULT_NOOP; if (!FileSystemMountedCorrectly(GetGlobalMountedFSList(), name, a)) { + /* Whether something is mounted at the promiser at all (vs. nothing + * mounted there). Computed for both mount and unmount: the unmount + * path uses it to tell "a different filesystem is mounted here" from + * "nothing is mounted here". */ + bool already_mounted = false; + for (size_t i = 0; i < SeqLength(GetGlobalMountedFSList()); i++) + { + Mount *mp = SeqAt(GetGlobalMountedFSList(), i); + if (mp != NULL && mp->mounton != NULL && strcmp(name, mp->mounton) == 0) + { + already_mounted = true; + break; + } + } + if (!a->mount.unmount) { + /* Ensure the mount point exists before mounting or remounting. + * dir is "/.", so this creates the mount point directory. */ if (!MakeParentDirectory(dir, a->move_obstructions, NULL)) { // Could not create parent directory, assume this is okay, @@ -470,32 +512,59 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr dir); } - if (a->mount.editfstab) + if (already_mounted) { - changes += VerifyInFstab(ctx, name, a, pp, &result); + /* CFE-90: mounted but not as promised. Correct the live mount + * first (only when remount is enabled), then update fstab. */ + if (a->mount.remount) + { + result = PromiseResultUpdate(result, ReconcileMountOptions(ctx, name, a, pp)); + changes++; + } + else + { + /* Reachable only for a wrong-source mount: option drift with + * remount disabled is reported as mounted correctly. */ + cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a, + "A different filesystem is mounted on '%s' than promised; enable 'remount' to correct", name); + result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL); + } + + /* Persist to fstab regardless of the live outcome (reported + * above); it converges at the next remount/reboot. */ + if (a->mount.editfstab) + { + changes += VerifyInFstab(ctx, name, a, pp, &result); + } } else { - cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a, - "Filesystem '%s' was not mounted as promised, and no edits were promised in '%s'", name, - VFSTAB[VSYSTEMHARDCLASS]); - result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL); - // Mount explicitly + /* CFE-1863: not mounted - mount just THIS filesystem instead of + * arming MountAll ('mount -a'), which would also mount every + * other unmounted fstab entry. Then persist to fstab. */ result = PromiseResultUpdate(result, VerifyMount(ctx, name, a, pp)); + if (a->mount.editfstab) + { + changes += VerifyInFstab(ctx, name, a, pp, &result); + } } } else { - if (a->mount.editfstab) + /* CFE-2350: a *different* filesystem here is not ours to unmount - + * leave it (and its fstab entry) alone; otherwise nothing of ours + * is mounted, so just ensure it is not in fstab. */ + if (already_mounted) + { + cfPS(ctx, LOG_LEVEL_VERBOSE, PROMISE_RESULT_NOOP, pp, a, + "A different filesystem is mounted on '%s' than the unmount promise targets; leaving it untouched", + name); + } + else if (a->mount.editfstab) { changes += VerifyNotInFstab(ctx, name, a, pp, &result); } } - - if (changes > 0) - { - CF_MOUNTALL = true; - } } else { @@ -509,11 +578,21 @@ static PromiseResult VerifyMountPromise(EvalContext *ctx, char *name, const Attr } else { - cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_NOOP, pp, a, "Filesystem '%s' seems to be mounted as promised", name); + /* CFE-1539: mounted correctly, but still maintain fstab (add a + * missing entry, correct drifted options) so the promise persists + * across reboots. Independent of 'remount' - keeping fstab correct + * is documented mount_options behavior; remounting live is not. */ + if (a->mount.editfstab) + { + changes += VerifyInFstab(ctx, name, a, pp, &result); + } + if (changes == 0) + { + cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_NOOP, pp, a, "Filesystem '%s' seems to be mounted as promised", name); + } } } - free(options); return result; } diff --git a/libpromises/attributes.c b/libpromises/attributes.c index ddc3138e8e..50851aaada 100644 --- a/libpromises/attributes.c +++ b/libpromises/attributes.c @@ -1694,6 +1694,9 @@ StorageMount GetMountConstraints(const EvalContext *ctx, const Promise *pp) m.mount_options = PromiseGetConstraintAsList(ctx, "mount_options", pp); m.editfstab = PromiseGetConstraintAsBoolean(ctx, "edit_fstab", pp); m.unmount = PromiseGetConstraintAsBoolean(ctx, "unmount", pp); + m.remount = PromiseGetConstraintAsBoolean(ctx, "remount", pp); + m.remount_methods = PromiseGetConstraintAsList(ctx, "remount_methods", pp); + m.remount_timeout = PromiseGetConstraintAsInt(ctx, "remount_timeout", pp); return m; } diff --git a/libpromises/cf3.defs.h b/libpromises/cf3.defs.h index fd53bf8a68..b3bd138507 100644 --- a/libpromises/cf3.defs.h +++ b/libpromises/cf3.defs.h @@ -913,7 +913,8 @@ typedef struct char *host; char *source; char *mounton; - char *options; + char *options; /* fstype string (e.g. "nfs", "panfs", "cifs") for foreign-FS detection */ + char *raw_opts; /* full kernel-resolved options from /proc/mounts */ int unmount; } Mount; @@ -1292,6 +1293,9 @@ typedef struct Rlist *mount_options; int editfstab; int unmount; + int remount; + Rlist *remount_methods; + int remount_timeout; } StorageMount; typedef struct diff --git a/libpromises/mod_storage.c b/libpromises/mod_storage.c index cd663a0f4a..cb0869c80d 100644 --- a/libpromises/mod_storage.c +++ b/libpromises/mod_storage.c @@ -48,6 +48,9 @@ static const ConstraintSyntax mount_constraints[] = ConstraintSyntaxNewString("mount_server", "", "Hostname or IP or remote file system server", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewStringList("mount_options", "", "List of option strings to add to the file system table (\"fstab\")", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewBool("unmount", "true/false unmount a previously mounted filesystem. Default value: false", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewBool("remount", "true/false correct the options of an already-mounted filesystem when they differ from the promise. Default value: false", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewOptionList("remount_methods", "remount,unmount_mount", "Ordered list of mechanisms to reconcile a mounted filesystem with the promise (tried in order). Default: remount", SYNTAX_STATUS_NORMAL), + ConstraintSyntaxNewInt("remount_timeout", CF_VALRANGE, "Timeout in seconds for each remount_method", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewNull() }; diff --git a/tests/unit/nfs_test.c b/tests/unit/nfs_test.c index 8e5d8ab78a..9b8e8d80bf 100644 --- a/tests/unit/nfs_test.c +++ b/tests/unit/nfs_test.c @@ -33,11 +33,84 @@ static void test_MatchFSInFstab(void) assert_false(MatchFSInFstab("/mnt/fileserver3/vol1")); } +static void test_OptionsSubsetMatches(void) +{ + /* Empty/NULL promise is always satisfied. */ + assert_true(OptionsSubsetMatches(NULL, "rw,noatime")); + assert_true(OptionsSubsetMatches("", "rw,noatime")); + + /* Subset: all promised present, kernel-added options ignored. */ + assert_true(OptionsSubsetMatches("rw,noatime", + "rw,noatime,vers=4.2,rsize=524288,wsize=524288,hard,proto=tcp,addr=10.0.0.1")); + + /* Order-insensitive. */ + assert_true(OptionsSubsetMatches("noatime,rw", "rw,noatime,vers=4.2")); + + /* A promised option that is simply absent -> mismatch. */ + assert_false(OptionsSubsetMatches("rw,noatime,acl", "rw,noatime,vers=4.2")); + + /* Inverse pairs contradict. */ + assert_false(OptionsSubsetMatches("noatime", "rw,relatime,vers=4.2")); + assert_false(OptionsSubsetMatches("ro", "rw,relatime")); + assert_false(OptionsSubsetMatches("rw", "ro,relatime")); + assert_false(OptionsSubsetMatches("hard", "rw,soft")); + assert_false(OptionsSubsetMatches("sync", "rw,async")); + + /* Generic "no" vs "" contradiction. */ + assert_false(OptionsSubsetMatches("nodev", "rw,dev")); + assert_false(OptionsSubsetMatches("atime", "rw,noatime")); + + /* Protocol aliases, both directions. */ + assert_true(OptionsSubsetMatches("tcp", "rw,proto=tcp,vers=4.2")); + assert_true(OptionsSubsetMatches("proto=tcp", "rw,tcp")); + assert_true(OptionsSubsetMatches("udp", "rw,proto=udp")); + + /* "defaults" is never echoed by the kernel; it holds iff none of the + * negatives that would violate it (ro/nosuid/nodev/noexec/sync) are + * present. atime/relatime and kernel-added options are irrelevant to it. */ + assert_true(OptionsSubsetMatches("defaults", "rw,relatime,vers=4.2,hard")); + assert_true(OptionsSubsetMatches("defaults,noatime", "rw,noatime,vers=4.2")); + assert_true(OptionsSubsetMatches("defaults", "rw,noatime,relatime,vers=4.2")); + assert_false(OptionsSubsetMatches("defaults", "ro,relatime,vers=4.2")); + assert_false(OptionsSubsetMatches("defaults", "rw,nosuid,vers=4.2")); + assert_false(OptionsSubsetMatches("defaults", "rw,noexec")); + assert_false(OptionsSubsetMatches("defaults", "rw,sync")); + + /* Last wins: a later option overrides an earlier conflicting one, exactly + * as `mount -o` applies the list (defaults,ro is read-only; ro,rw is rw). */ + assert_true(OptionsSubsetMatches("defaults,ro", "ro,relatime,vers=4.2")); /* ro overrides defaults' rw */ + assert_false(OptionsSubsetMatches("defaults,ro", "rw,relatime,vers=4.2")); /* wants ro, mount is rw */ + assert_true(OptionsSubsetMatches("ro,rw", "rw,relatime")); /* rw wins */ + assert_false(OptionsSubsetMatches("ro,rw", "ro,relatime")); /* wants rw, mount is ro */ + assert_true(OptionsSubsetMatches("rw,ro", "ro,relatime")); /* ro wins */ + assert_true(OptionsSubsetMatches("defaults,nosuid", "rw,nosuid,vers=4.2")); /* nosuid overrides defaults' suid */ +} + +static void test_RemountOptionString(void) +{ + char *s; + + /* "defaults" expands to its checkable positives so an in-place remount + * restores a mount that drifted to ro/nosuid/etc (util-linux does not honor + * the options implied by "defaults" on a remount). */ + s = RemountOptionString("defaults"); + assert_true(strcmp(s, "rw,suid,dev,exec,async") == 0); free(s); + s = RemountOptionString("defaults,noatime"); + assert_true(strcmp(s, "rw,suid,dev,exec,async,noatime") == 0); free(s); + /* Everything else is passed through unchanged and order-preserved. */ + s = RemountOptionString("rw,noatime"); assert_true(strcmp(s, "rw,noatime") == 0); free(s); + s = RemountOptionString("ro"); assert_true(strcmp(s, "ro") == 0); free(s); + assert_true(RemountOptionString("") == NULL); + assert_true(RemountOptionString(NULL) == NULL); +} + int main() { PRINT_TEST_BANNER(); const UnitTest tests[] = { unit_test(test_MatchFSInFstab), + unit_test(test_OptionsSubsetMatches), + unit_test(test_RemountOptionString), }; return run_tests(tests);