From aaa54151aea7df74dd12fa6a5641f01464e4d2c2 Mon Sep 17 00:00:00 2001 From: Zygo Blaxell Date: Sun, 30 Aug 2026 01:51:50 -0400 Subject: [PATCH] config: remove untrusted local-config support "config: disable untrusted configuration files" made a relative config.local-config-filename a hard configuration error, so a local config file loaded from a non-absolute path can no longer exist. What was left behind is the machinery that used to police such a file: two merged views, a get_trusted() that reads one of them, and the comments and documentation describing which sections an untrusted file could not reach. The residue is not inert. m_local_trusted is initialised false and never assigned, so m_trusted never gains the local config as a fallback, and every get_trusted() key -- path.home, path.reports, state.persistent, state.create, and all of [report.*] -- silently ignores the operator's per-filesystem config file. A setting the operator can see in --show-config is not the setting bees uses. Remove the distinction: - bees-config.h/.cc: m_trusted, m_untrusted and m_local_trusted collapse into one m_innie chaining argv -> local -> global -> builtin, and get_trusted() goes away. - bees-context.cc, bees.cc: the eleven get_trusted() call sites become get(), so the local config reaches those keys. - bees-usage.cc: --write-trusted-config and --write-untrusted-config described the two views. The implementation was removed, so passing one is an unrecognised-option error. - bees-config-v1.cc: local-config-filename's comment, the [path] and [report.*] section headers, and tempfile.dir each described the restrictions on an untrusted file. - docs/config-file.md, docs/config-format.md: the relative/absolute trust split, and the security notes built around it, replaced by why the path has to be absolute. Every key a local file can set is a control surface, not just the ones naming a path, so restricting sections was never the real defence -- requiring an absolute path is. Assisted-by: Claude-Code:claude-opus-5 Signed-off-by: Zygo Blaxell --- docs/config-file.md | 45 ++++++++++++++++-------------------- docs/config-format.md | 22 ++++++++---------- src/bees-config-v1.cc | 14 ++---------- src/bees-config.cc | 23 ++++++++----------- src/bees-config.h | 47 ++++---------------------------------- src/bees-context.cc | 6 ++--- src/bees-usage.cc | 15 ------------ src/bees.cc | 16 ++++++------- test/test-innie-comment.cc | 2 +- 9 files changed, 57 insertions(+), 133 deletions(-) diff --git a/docs/config-file.md b/docs/config-file.md index 2a31effa..31390103 100644 --- a/docs/config-file.md +++ b/docs/config-file.md @@ -43,15 +43,8 @@ overrides global, command-line overrides all). * **`local-config-filename`** Path to an optional filesystem-specific config file. - * Relative paths are considered _untrusted_, and have the following restrictions: - * They must be resolved within the filesystem root. - * The local config is ignored on kernels that do not support `openat2`. - * They _must_ resolve to a file within the target filesystem (paths outside the target filesystem, such as `../foo`, will be rejected). - * They may not specify any options in the `[paths]` section. - * Absolute paths are considered _trusted_, and have fewer restrictions: - * Symlinks are allowed along the path, but the last component of the filename must not be a symlink. - * The kernel need not support `openat2`. - * They may specify options in the `[paths]` section. + * Must be absolute (start with `/`). + * Symlinks are allowed along the path, but the last component of the filename must not be a symlink. * If the filename is empty, the local config is ignored. * If the file does not exist, the local config is ignored. * If the file exists but is unreadable, bees treats this as an error and will not start. @@ -63,24 +56,26 @@ overrides global, command-line overrides all). * `${UUID}` – filesystem UUID * **Note:** `${LABEL}` may be empty. -### Security notes +### Why local config must be absolute -A local configuration file may be located on the target filesystem. This may -have security implications if the target filesystem is untrusted (e.g. -removable media), because the local config can still affect settings outside -the `[paths]` section. +A local configuration file is a control surface: every key it sets affects how +bees runs. Even keys that don't name a path can become DoS vectors when an +attacker controls the file — for example, `thread.thread-max = 1000000` or +pathological `loop.transid-lag` values. -If the `local-config-filename` is an absolute path starting with `/`, the local configuration is _trusted_, and may set options in the `[path]` and `[report.*]` sections. -If the `local-config-filename` is a relative path not starting with `/`, the -local configuration is _untrusted_, and may _not_ set options in the -`[path]` or `[report.*]` sections. It may still set other options that are allowed in local -configuration files. +Allowing the file to live on the target filesystem (a relative path resolved +against the mount root) would let anyone who can write to that filesystem +reconfigure the daemon. This matters for removable media, mounted +attacker-supplied images, and any filesystem with untrusted writers. -If you know what you're doing--or want to throw caution to the wind--use `${ROOT}` to set an absolute path through the mount point of the target filesystem. e.g. +bees therefore requires the local config filename to be absolute. Operators +who genuinely want a config file on the target filesystem can still do so +explicitly with `${ROOT}`: - local-config-filename = ${ROOT}/relative/path/on/target/filesystem.conf + local-config-filename = ${ROOT}/path/on/target/filesystem.conf -Note that this will enable the use of symlinks in the path as well. +The substitution expands to an absolute path before the check, so this opts +back in with eyes open. ### Notes @@ -119,9 +114,9 @@ removed when bees closes them. * Any other value requires `openat2(2)` (Linux 5.6+). bees will exit at startup if `openat2` is unavailable rather than fall back to `openat(2)`, which cannot enforce `RESOLVE_BENEATH` or `RESOLVE_NO_SYMLINKS`. - * Because `O_TMPFILE` inodes are anonymous, `dir` does not need to be listed - in the trusted `[path]` section — an attacker cannot redirect bees's - persistent writes by setting this option. + * Because `O_TMPFILE` inodes are anonymous, the choice of `dir` does not + let an attacker redirect bees's persistent writes — the inode is never + visible in any directory. * **`compression`** Controls how bees sets flags and attributes on its temporary files. diff --git a/docs/config-format.md b/docs/config-format.md index eef189a7..b710cdb7 100644 --- a/docs/config-format.md +++ b/docs/config-format.md @@ -77,19 +77,15 @@ the configuration will be rejected due to the duplicate key. ### Local configuration restrictions -Local (filesystem-specific) configuration files have additional -restrictions if the local configuration filename does not begin with `/`. -Such configuration files are called "untrusted". - -These restrictions protect against abuse if the filesystem is untrusted -(e.g. removable media): - -* Writable path options (`[path.*]`) are ignored in an untrusted local configuration file. -* Report output options (`[report.*]`) are ignored in an untrusted local configuration file. -* Configuration file paths (`[config.*-config-filename]`) are ignored in all local configuration files. -* Symlinks are not permitted in the path to an untrusted local configuration file. -* An untrusted local configuration file must exist within the filesystem (i.e. escape from the filesystem with "../" is not allowed). -* An untrusted local configuration file is only enabled on kernels that implement the `openat2` system call. On older kernels, an untrusted local configuration file is ignored. +Local (filesystem-specific) configuration files must use an absolute path. +A path on the target filesystem itself is a control-surface hazard — any +config knob, not just paths, can be turned into a DoS vector by an attacker +controlling that filesystem. See [Why local config must be +absolute](config-file.md#why-local-config-must-be-absolute) for details. + +Configuration file paths (`[config.*-config-filename]`) are also ignored in +all local configuration files, since by the time the local file is being read +those choices have already been made. ### Comments and whitespace diff --git a/src/bees-config-v1.cc b/src/bees-config-v1.cc index b308b27e..298724e4 100644 --- a/src/bees-config-v1.cc +++ b/src/bees-config-v1.cc @@ -29,10 +29,8 @@ static const char bees_config_v1[] = R"--v1-config--( # Optional system-wide config. global-config-filename = /etc/bees/bees.conf - # optional per-filesystem config. Can use variables (BASENAME, - # UUID, LABEL, ROOT). Relative paths are treated as untrusted: - # they cannot override [path] or [report.*], but they may - # still affect other settings (for example [tempfile]). + # Optional per-filesystem config. Must be an absolute path. + # Variables: ${BASENAME}, ${UUID}, ${LABEL}, ${ROOT}. local-config-filename = /etc/bees/uuid.d/${UUID}.conf # The [thread] section controls worker count and loadavg throttling. @@ -111,8 +109,6 @@ static const char bees_config_v1[] = R"--v1-config--( level = 5 # The [path] section defines paths that bees can write to. -# This section is ignored in untrusted local configuration files -# (see `config.local-config-filename` for details). [path] @@ -135,10 +131,6 @@ static const char bees_config_v1[] = R"--v1-config--( # The [report.*] sections define status output files written periodically # by bees. Each named section produces one output file. -# -# All keys in [report.*] are read from the trusted config only (absolute-path -# local config, global config, or command line). Untrusted sources (relative- -# path local config) are ignored to prevent arbitrary file writes. [report.*] @@ -258,8 +250,6 @@ static const char bees_config_v1[] = R"--v1-config--( # are followed. The default "." opens the filesystem root directly and # works on any kernel. Any other value requires openat2(2) (Linux # 5.6+); bees will exit at startup if openat2 is unavailable. - # - # This key may appear in untrusted config files. dir = . # Compression mode: diff --git a/src/bees-config.cc b/src/bees-config.cc index 33a4cea7..fa94d84f 100644 --- a/src/bees-config.cc +++ b/src/bees-config.cc @@ -345,21 +345,16 @@ BeesConfig::set_argv(const Innie &startup_ini) // Update defaults from "config.version" from local config reset_default(boot_innie.get(cv_key)); - // Final defaults. The chain is attached to each merged view itself: - // set_default() takes only the other Innie's own rep, and so do Innie's - // copy and assignment, so a chain built on a separate object would not - // survive being handed over. + // Final defaults. The chain is attached to m_innie itself: set_default() + // takes only the other Innie's own rep, and so do Innie's copy and + // assignment, so a chain built on a separate object would not survive + // being handed over. m_builtin = s_defaults->at(default_version); - m_trusted = m_argv; - set_version_defaults(m_trusted, default_version); - m_trusted.set_fallback(m_global); - if (m_local_trusted) m_trusted.set_fallback(m_local); - - m_untrusted = m_argv; - set_version_defaults(m_untrusted, default_version); - m_untrusted.set_fallback(m_global); - m_untrusted.set_fallback(m_local); + m_innie = m_argv; + set_version_defaults(m_innie, default_version); + m_innie.set_fallback(m_global); + m_innie.set_fallback(m_local); // Log every schema in the chain, not just the selected one: which // version supplied a default is otherwise invisible. @@ -384,7 +379,7 @@ BeesConfig::set_argv(const Innie &startup_ini) set BeesConfig::sections(const string &prefix) const { - return m_untrusted.sections(prefix); + return m_innie.sections(prefix); } bool diff --git a/src/bees-config.h b/src/bees-config.h index 621cb4c7..d50a0643 100644 --- a/src/bees-config.h +++ b/src/bees-config.h @@ -29,10 +29,6 @@ using namespace std; * 3. Global config file (@c m_global). * 4. Built-in defaults (@c m_builtin) — lowest priority. * - * The trusted/untrusted distinction controls whether values from non-absolute - * paths (e.g. relative or environment-derived paths) are allowed to influence - * security-sensitive settings such as the hash table path. - * * Configuration values may contain @c ${VAR} substitutions resolved via * @c m_subst_map (populated from the filesystem root path and basename). */ @@ -118,12 +114,9 @@ class BeesConfig { Innie m_global; ///< Values from the global config file. Innie m_builtin; ///< Compiled-in defaults. - /// Merged views used for lookup. - /// Untrusted means the origin is not an absolute path. - Innie m_trusted; ///< Lookup chain that accepts only trusted sources. - Innie m_untrusted; ///< Lookup chain that accepts all sources. - /// True if the local config file was loaded from an absolute path. - bool m_local_trusted = false; + /// Merged view used for lookup. Built once at the end of set_argv() + /// by chaining m_argv -> m_local -> m_global -> m_builtin. + Innie m_innie; /// FD to the btrfs root filesystem (used for filesystem-specific config). Fd m_root_fd; @@ -192,7 +185,7 @@ public: set sections(const string &prefix) const; /** - * Look up @p section_and_key (e.g. @c "scanner/threads") in the untrusted + * Look up @p section_and_key (e.g. @c "scanner/threads") in the merged * config chain, apply @p parse_fn to the raw string, and return the result. * Logs and re-throws any exception thrown by @p parse_fn. */ @@ -204,7 +197,7 @@ public: try { { BEESTRACE("Parsing configuration key " << section_and_key); - val = m_untrusted.get(section_and_key); + val = m_innie.get(section_and_key); } BEESTRACE("Parsing configuration: " << section_and_key << " = '" << val << "'"); return invoke(std::forward(parse_fn), val); @@ -214,36 +207,6 @@ public: } } - /** - * Like get(), but looks up @p section_and_key in the trusted config chain only. - * Use this for keys that specify paths to files that bees will persistently - * read or modify (e.g. @c paths.hash for the hash table file). All such keys - * belong in the @c [paths] section. Note that @c tempfile.dir does NOT use - * get_trusted(): when set to @c "." it opens root_fd() directly; when set to - * any other value it requires openat2(2) and bees exits at startup if that - * syscall is unavailable. Untrusted sources (config files loaded via a - * relative or environment-derived path) are not allowed to influence these keys. - * The primary threat is removable media: an attacker could place a local config - * file on a filesystem that is later mounted and scanned by bees, redirecting - * bees's writes to arbitrary locations on the host. - */ - template - auto - get_trusted(const string& section_and_key, F&& parse_fn) const -> invoke_result_t - { - string val; - try { - { - BEESTRACE("Parsing trusted configuration key " << section_and_key); - val = m_trusted.get(section_and_key); - } - BEESTRACE("Parsing trusted configuration: " << section_and_key << " = '" << val << "'"); - return invoke(std::forward(parse_fn), val); - } catch (const exception& e) { - BEESLOGERR("CONFIG: ERROR while parsing trusted configuration option '" << section_and_key << "' = '" << val << "': " << e.what()); - throw; - } - } }; /// @name Configuration value parsing helpers diff --git a/src/bees-context.cc b/src/bees-context.cc index 1b403c76..84ea79c9 100644 --- a/src/bees-context.cc +++ b/src/bees-context.cc @@ -120,10 +120,10 @@ BeesContext::home_fd() BEESTRACE("Opening home directory"); - const auto home_dir = m_config->get_trusted("path.home", [](const string &s) { return s; } ); + const auto home_dir = m_config->get("path.home", [](const string &s) { return s; } ); BEESTRACE("Opening home directory '" << home_dir << "'"); - const bool persistent = m_config->get_trusted("state.persistent", bees_parse_bool); - const bool create = m_config->get_trusted("state.create", bees_parse_bool); + const bool persistent = m_config->get("state.persistent", bees_parse_bool); + const bool create = m_config->get("state.create", bees_parse_bool); const bool absolute = (home_dir.at(0) == '/'); diff --git a/src/bees-usage.cc b/src/bees-usage.cc index 2b0c115a..6a7f4a96 100644 --- a/src/bees-usage.cc +++ b/src/bees-usage.cc @@ -17,21 +17,6 @@ Options: --config FILE Read configuration options from FILE -o, --option Set one configuration option in `key=value` format -Configuration debug options: - --write-trusted-config FILE - Dump the merged trusted configuration view to FILE - and exit without running dedupe. FILE may be '-' - for stdout. Trusted view: only config files loaded - from absolute paths can override [paths] keys. - See `docs/options.md` for details. - - --write-untrusted-config FILE - Dump the merged untrusted configuration view to FILE - and exit without running dedupe. FILE may be '-' - for stdout. Untrusted view: all sources (including - relative-path local config files) are included. - See `docs/options.md` for details. - Load management options: -c, --thread-count Exact worker thread count (overrides factor) [thread.thread-max] diff --git a/src/bees.cc b/src/bees.cc index 9fc7e745..249061bd 100644 --- a/src/bees.cc +++ b/src/bees.cc @@ -389,7 +389,7 @@ bees_main(int argc, char *argv[]) // Build the BeesReporter from [path] and [report.*] config { const auto reports_dir = bc->get_config().subst( - bc->get_config().get_trusted("path.reports", [](const string &s) { return s; })); + bc->get_config().get("path.reports", [](const string &s) { return s; })); BEESLOGINFO("paths.reports = '" << reports_dir << "' [path.reports]"); // Parse all report sections into a vector first @@ -403,13 +403,13 @@ bees_main(int argc, char *argv[]) const auto prefix = string("report.") + sec_name + "."; BeesReportConfig cfg; cfg.name = sec_name; - cfg.active = bc->get_config().get_trusted(prefix + "active", bees_parse_bool); + cfg.active = bc->get_config().get(prefix + "active", bees_parse_bool); BEESLOGINFO("report." << sec_name << ".active = " << (cfg.active ? "yes" : "no") << " [report." << sec_name << ".active]"); - cfg.interval_s = bc->get_config().get_trusted(prefix + "interval", parse_report_interval); + cfg.interval_s = bc->get_config().get(prefix + "interval", parse_report_interval); cfg.filename = bc->get_config().subst( - bc->get_config().get_trusted(prefix + "filename", [](const string &s) { return s; })); - cfg.mode = bc->get_config().get_trusted(prefix + "mode", [](const string &s) { + bc->get_config().get(prefix + "filename", [](const string &s) { return s; })); + cfg.mode = bc->get_config().get(prefix + "mode", [](const string &s) { return mode_t(stoul(s, nullptr, 8)); }); cfg.log_level = bc->get_config().get(prefix + "level", [](const string &s) { @@ -431,7 +431,7 @@ bees_main(int argc, char *argv[]) } // user -> uid (name or numeric) - const auto user_str = bc->get_config().get_trusted(prefix + "user", [](const string &s) { return s; }); + const auto user_str = bc->get_config().get(prefix + "user", [](const string &s) { return s; }); if (!user_str.empty()) { // A leading ':' forces a name lookup of the rest of the // string, so a user literally named e.g. "1000" can be named @@ -453,7 +453,7 @@ bees_main(int argc, char *argv[]) } // group -> gid (name or numeric) - const auto group_str = bc->get_config().get_trusted(prefix + "group", [](const string &s) { return s; }); + const auto group_str = bc->get_config().get(prefix + "group", [](const string &s) { return s; }); if (!group_str.empty()) { // A leading ':' forces a name lookup of the rest of the // string, so a group literally named e.g. "1000" can be named @@ -475,7 +475,7 @@ bees_main(int argc, char *argv[]) } // content: space- or comma-separated token list - const auto content_str = bc->get_config().get_trusted(prefix + "content", [](const string &s) { return s; }); + const auto content_str = bc->get_config().get(prefix + "content", [](const string &s) { return s; }); if (cfg.active) { BEESLOGINFO("report." << sec_name << ".content = " << content_str << " [report." << sec_name << ".content]"); diff --git a/test/test-innie-comment.cc b/test/test-innie-comment.cc index c133ec7c..633d4418 100644 --- a/test/test-innie-comment.cc +++ b/test/test-innie-comment.cc @@ -186,7 +186,7 @@ test_section_comment_does_not_mask_key_comment_in_chain() Innie local; local.set_origin(""); // Leading file-comment becomes the [rewrite] section-header comment. - local.read(R"(# bees test config for --write-trusted-config test + local.read(R"(# bees test config for merged-config dump test [rewrite] # Operator override. -- 2.53.0