From nobody Fri May 29 16:01:48 2026 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4gRp591kFNz6fl7y for ; Fri, 29 May 2026 16:01:49 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R13" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4gRp585zq4z3Hsd for ; Fri, 29 May 2026 16:01:48 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1780070508; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PqviM8v3AcrhTZ1hLo02YCe/YV7AeKwOTakhPitDD1Y=; b=juy9Ebl+m48K2evFt3BzbrLFILDzKDZW6Tv/pnafCmH9cp78y64Yle//cVVcNpCS4L68or ee4zQKbNsvRkWxr2C2fB7QFj6h+dh4dxR32Hwt8gbgiK3WTE2yw47lqCdDIp6g/EVfIQad r7fb+fXYvSPqh8X/GavGzm5kkipeN6PkFTU+vvjj44vFRSb6hXg/xTRgcdtNe4tdxEsP0z EkkXxek0Rvs7G4eSN2M16pVTOcFQVWWvkGd5DDu9cql8Cy/c7G1N59m2i0a8dVw6w1jXGY dZVwCg3BsVzEatVqdZUY7ZBsZMjuGh53xCk3XB7bTlERzFExDTBJgWYqVHSOQw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1780070508; a=rsa-sha256; cv=none; b=UUN1uBNk2AY7IYHxsUmI/XE8gwmHVxZjQCsv+Ttkbie18AYjKG331MUJhwGsjgmrE8ifeN j2eU2W4ppGrFbdw6Cz5CnVHPg1Bbe+1qHQgETlfv1oJvqS0jQiDDjCo1AefxVEbtvZQZBd SHBdmFo0aJA//7D7TgW7Wo3N0F9w9ZH7p229B2/6aH/K/6H5yyF+vSjz5MuZd6Sh55XN38 5foRWa9W8u0jAPQ5pcj5tmYPF6LK3AIIGrZhgpZjc/Kj/TsyoeIdyB7xR/gJP6kEytJ4QC FzReDFnJY5GRSbQZfm9mWsYIYHygPnCGAM85UuVXI9oiscXhKGRToKnK+5g5yA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1780070508; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PqviM8v3AcrhTZ1hLo02YCe/YV7AeKwOTakhPitDD1Y=; b=BYJFpaY7Al6cFUSYvULZCiqPpJeM2U5F21w7iwjMYCF4YfBrVxqADQ38zozO196dDOx5PJ Md8On8qeLUn381asrWhyK+ovMnOqFqFdTl5qKFktMgli/jP8Y/NMxqHSMSqwYYXhX1p5r8 x5WjRsxSEA7cpFLZV/onrPXmfEuclKKUWlOSfbLkqQm7q4ExemHUfdyrhidVLG6+SwCNiq tFxMgPhUrdNkx65jESbmskHW0I3U6tm3eMT61IbrbQOXUbP4LETD1K005JdBi25eaLl5mb gmX2TIUD8LFHtj0UrKNz2HxdpAOl9ICV0CpOHds2YhoWyNPYir1oew9eOodFyA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4gRp585SCBzgrn for ; Fri, 29 May 2026 16:01:48 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 3652c by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 29 May 2026 16:01:48 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: 7929f364ef51 - main - MAC/do: Fix the recent logic to set jail parameters, make it more tolerant List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: olce X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 7929f364ef5135f712e061d291d1c2c0fb48abde Auto-Submitted: auto-generated Date: Fri, 29 May 2026 16:01:48 +0000 Message-Id: <6a19b86c.3652c.80b6732@gitrepo.freebsd.org> The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=7929f364ef5135f712e061d291d1c2c0fb48abde commit 7929f364ef5135f712e061d291d1c2c0fb48abde Author: Olivier Certner AuthorDate: 2026-04-28 12:40:19 +0000 Commit: Olivier Certner CommitDate: 2026-05-29 15:27:30 +0000 MAC/do: Fix the recent logic to set jail parameters, make it more tolerant The logic introduced in the initial commit for the "executable paths" feature did not match the specification we discussed in that specifying an empty value (for rules or executable paths) on "mac.do" being "new" would be treated as an absence of value and trigger a copy from the currently applicable configuration, instead of being an override that deactivates mac_do(4) in the jail. Fix that by distinguishing both cases. More generally, a non-explicitly specified parameter is set to the same value it has in the currently applicable configuration (that of the closest ancestor jail that has one; 'prison0' (the host) always has one), with an exception in the disable case. On disable (explicit: "mac.do" to "disable", implicit: no parameters passed, or at least one is empty), now accept parameters with a non-empty value as long as at least one of them is empty (which alone is enough to disable mac_do(4)). If no parameters are passed, both are copied from the currently applicable configuration; if none of them is empty, then the rules are emptied to effectively disable mac_do(4) (see the inline comment as to why this was chosen). On explicit enable ("mac.do" to "enable"), allow not specifying any of the rules and executable paths, in which case both are copied from the currently applicable configuration (consistently with what is done when only one is missing). Note that, as mentioned above, not specifying any of them by default still resolves to disabling mac_do(4) (i.e., on no explicit "mac.do" parameter). On (explicit) inheritance, allow specifying non-empty parameters, provided they match the values we are going to inherit. This enables re-applying jail parameters' reported values verbatim to the current jail (idempotence) or, e.g., to some sibling jail. (While here, make some existing code easier to read by leveraging is_null_or_empty().) Reviewed by: bapt Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38 --- sys/security/mac_do/mac_do.c | 201 ++++++++++++++++++++++++++++++------------- 1 file changed, 140 insertions(+), 61 deletions(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index 493dcaf66f89..7890af9bcfec 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -326,6 +326,18 @@ unexpected_flags: #define check_type_and_type_flags(...) #endif /* INVARIANTS */ +static bool +has_rules(const struct rules *const rules) +{ + return (rules->string[0] != '\0'); +} + +static bool +has_exec_paths(const struct exec_paths *const exec_paths) +{ + return (exec_paths->exec_paths_str[0] != '\0'); +} + /* * Returns EALREADY if both flags have some overlap, or EINVAL if flags are * incompatible, else 0 with flags successfully merged into 'dest'. @@ -1598,7 +1610,7 @@ mac_do_jail_check(void *obj, void *data) struct vfsoptlist *opts = data; char *rules_string, *exec_paths_string; int error, jsys, rules_size = 0, exec_paths_size = 0; - bool has_rules, has_exec_paths; + bool absent_or_empty_rules, absent_or_empty_exec_paths; error = vfs_copyopt(opts, "mac.do", &jsys, sizeof(jsys)); if (error == ENOENT) @@ -1661,22 +1673,17 @@ mac_do_jail_check(void *obj, void *data) } } - /* - * Be liberal, considering that an empty rule or execution paths - * specification is equivalent to no specification. This affects the - * JAIL_SYS_DISABLE and JAIL_SYS_INHERIT sanity checks below. - */ - has_rules = rules_string != NULL && rules_string[0] != '\0'; - has_exec_paths = exec_paths_string != NULL && - exec_paths_string[0] != '\0'; + absent_or_empty_rules = is_null_or_empty(rules_string); + absent_or_empty_exec_paths = is_null_or_empty(exec_paths_string); /* If not specified, infer 'jsys' from passed options. */ if (jsys == -1) { /* * Default in absence of "mac.do.rules" and "mac.do.exec_paths" - * is to disable (and, in particular, not inherit). + * is to disable. We never implicitly inherit, as that changes + * reasoning about configurations. */ - if (has_rules || has_exec_paths) + if (!absent_or_empty_rules || !absent_or_empty_exec_paths) jsys = JAIL_SYS_NEW; else jsys = JAIL_SYS_DISABLE; @@ -1685,31 +1692,38 @@ mac_do_jail_check(void *obj, void *data) /* Final checks based on resolved 'jsys'. */ switch (jsys) { case JAIL_SYS_DISABLE: - case JAIL_SYS_INHERIT: - if (has_rules) { - vfs_opterror(opts, - "'mac.do.rules' specified but should not be when " - "'mac.do' is 'disabled' or 'inherited'"); - return (EINVAL); - } - if (has_exec_paths) { + /* + * Tolerate specified but empty rules or execution paths + * (instead of not being specified). Also, tolerate that one of + * them is not empty (but not both). Indeed, as soon as one is + * empty, mac_do(4) is effectively disabled. This allows the + * administrator to still specify a value for one of them, which + * is then used for new sub-jails that do not inherit and for + * which no value for the parameter is explicitly specified + * (because then the value passed here is copied). + */ + if (!absent_or_empty_rules && !absent_or_empty_exec_paths) { vfs_opterror(opts, - "'mac.do.exec_paths' specified but should not be " - "when 'mac.do' is 'disabled' or 'inherited'"); + "One of 'mac.do.rules' and 'mac_do.exec_paths' " + "should not be specified or should be empty when " + "'mac.do' is 'disabled'"); return (EINVAL); } break; - case JAIL_SYS_NEW: - if (!has_rules && !has_exec_paths) { - vfs_opterror(opts, "'mac.do' set to 'new' but neither " - "rules nor executable paths specified"); - return (EINVAL); - } + case JAIL_SYS_INHERIT: + /* + * Canonically, no parameters should be specified in this case. + * However, we tolerate empty ones, and also non-empty ones + * provided they match the inherited values, so that we can + * report the *resolved* value of current parameters via + * mac_do_jail_get() and have them re-applicable to this jail in + * a similar situation. Testing that inherited values are the + * same as passed ones is more expensive than a single test and + * requires some atomicity, which is why we do not perform that + * here but only in mac_do_jail_set(). + */ break; - - default: - __assert_unreachable(); } return (0); @@ -1718,13 +1732,13 @@ mac_do_jail_check(void *obj, void *data) static int mac_do_jail_set(void *obj, void *data) { - struct prison *pr = obj; - struct vfsoptlist *opts = data; + struct prison *const pr = obj; + struct vfsoptlist *const opts = data; char *rules_string, *exec_paths_string; struct parse_error *parse_error = NULL; struct conf *model_conf; int error, jsys; - bool has_rules, has_exec_paths; + bool absent_or_empty_rules, absent_or_empty_exec_paths; /* * The invariants checks used below correspond to what has already been @@ -1741,59 +1755,124 @@ mac_do_jail_set(void *obj, void *data) exec_paths_string = vfs_getopts(opts, "mac.do.exec_paths", &error); MPASS(error == 0 || error == ENOENT); - has_rules = rules_string != NULL && rules_string[0] != '\0'; - has_exec_paths = exec_paths_string != NULL && - exec_paths_string[0] != '\0'; + absent_or_empty_rules = is_null_or_empty(rules_string); + absent_or_empty_exec_paths = is_null_or_empty(exec_paths_string); if (jsys == -1) { - if (has_rules || has_exec_paths) + if (!absent_or_empty_rules || !absent_or_empty_exec_paths) jsys = JAIL_SYS_NEW; else jsys = JAIL_SYS_DISABLE; } if (jsys == JAIL_SYS_INHERIT) { - MPASS(!has_rules && !has_exec_paths); - remove_conf(pr); - return (0); + error = 0; + + if (!absent_or_empty_rules || !absent_or_empty_exec_paths) { + /* + * Some values specified. Check that they match the + * ones we are going to inherit. + */ + model_conf = find_conf(pr->pr_parent, NULL); + if (strcmp(model_conf->rules.string, rules_string) + != 0) { + error = EINVAL; + vfs_opterror(opts, + "'mac.do' is 'inherited' but 'mac.do.rules'" + " was specified with a different value " + "than the one to be inherited (\"%s\")", + model_conf->rules.string); + } + if (strcmp(model_conf->exec_paths.exec_paths_str, + exec_paths_string) != 0) { + error = EINVAL; + vfs_opterror(opts, + "'mac.do' is 'inherited' but " + "'mac.do.exec_paths' was specified with a " + "different value than the one to be " + "inherited (\"%s\")", + model_conf->exec_paths.exec_paths_str); + } + drop_conf(model_conf); + } + + if (error == 0) + /* + * There's no TOCTOU problem here as the removal of the + * current jail's configuration commutes with changing + * the inherited configuration we checked against. + */ + remove_conf(pr); + + return (error); } model_conf = NULL; switch (jsys) { case JAIL_SYS_DISABLE: - rules_string = ""; - has_rules = true; - /* FALLTHROUGH */ - - case JAIL_SYS_NEW: /* - * If 'pr' has a configuration, we want to use it as the model - * (i.e., only change what has been explicitly specified). - * Else, we want as default values those that are inherited. + * mac_do(4) is disabled iff one of the parameter's string is + * empty. The parse_and_set_conf() call below treats passing + * NULL for a parameter as a flag to copy its value from the + * relevant ancestor jail's configuration, so we have to watch + * for the final result having an empty parameter if no + * parameter has been explicitly passed as empty. Thanks to + * mac_do_jail_check(), we know that at least one parameter is + * absent or empty (see the comment for the corresponding case + * there). */ - model_conf = !has_rules || !has_exec_paths ? - find_conf(pr, NULL) : NULL; - error = parse_and_set_conf(pr, - has_rules ? rules_string : NULL, - has_exec_paths ? exec_paths_string : NULL, - model_conf, - &parse_error); - - if (error != 0) { - vfs_opterror(opts, - "MAC/do: Parse error at index %zu: %s\n", - parse_error->pos, parse_error->msg); - free_parse_error(parse_error); + MPASS(absent_or_empty_rules || absent_or_empty_exec_paths); + if (!absent_or_empty_rules) + exec_paths_string = ""; + else if (!absent_or_empty_exec_paths) + rules_string = ""; + else { + /* + * Both are either empty or absent. If at least one is + * absent, we retrieve the applicable configuration as + * it will serve as a template (provides default + * values). + */ + if (rules_string == NULL || exec_paths_string == NULL) + model_conf = find_conf(pr, NULL); + /* If both are absent, we have to examine if, in the + * currently applicable configuration, one of the + * parameters, which we are going to copy, is + * effectively empty. If both of those are non-empty, + * we keep the executable paths and empty the rules, + * since we expect that this is more convenient to + * administrators that may want to enable mac_do(4) + * later by just setting new rules. + */ + if (rules_string == NULL && exec_paths_string == NULL && + has_rules(&model_conf->rules) && + has_exec_paths(&model_conf->exec_paths)) + rules_string = ""; } break; + case JAIL_SYS_NEW: + /* See the comment before the call to find_conf() above. */ + if (rules_string == NULL || exec_paths_string == NULL) + model_conf = find_conf(pr, NULL); + break; + default: __assert_unreachable(); } + error = parse_and_set_conf(pr, rules_string, exec_paths_string, + model_conf, &parse_error); if (model_conf != NULL) drop_conf(model_conf); + if (error != 0) { + vfs_opterror(opts, + "MAC/do: Parse error at index %zu: %s\n", + parse_error->pos, parse_error->msg); + free_parse_error(parse_error); + } + return (error); }