From nobody Mon Dec 16 14:45:58 2024 X-Original-To: dev-commits-src-all@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 4YBjRq6PdKz5hWPP; Mon, 16 Dec 2024 14:45:59 +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 "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YBjRq0YCpz4dl6; Mon, 16 Dec 2024 14:45:59 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1734360359; 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=nZpbAKxHD2T8txy+JeSn6wK1G/B5VJJZZrzCIVYvhqc=; b=dtu+PbFw5XpyLXvwfpwmXmSPc5NtSCckA3/rTVrmgxVD4jIdlGJd+F+1wEnWysHTKdxTlA kLGW0tABAYq2UQp0Czv/XhU8FvRmsD2LMBAsAY+pbvKutawM+PlKM6vbLlqoyTs8Cu9Mov X6ogYPQSQgoNVMN0EujduaEom0vPL9KgskF7pvF5649kou5yioaTfstPm/2D7wdQcFsG3y fjV5EMflsb98aQJ5TgFuzSf+obsRw3+Rv2xox0sq4pnyoTkBp7qOlhItX+O4V/Or9HbTbL LE9QDbQA+HAXGOkVx7CZM+dw1KnD3K28d5MbCC1fJrtFZq5jES40OUuWfM2a/Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1734360359; 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=nZpbAKxHD2T8txy+JeSn6wK1G/B5VJJZZrzCIVYvhqc=; b=aoCn76F7kK8XptY8E2VJoW0tPHAcJRRKoDec+RXgN7PVZkFawAwBLCoJ3l54FjeOWWTqxm 2jr06YBlrvYg8tNiR1OMfhePKd/ira3hKaFkKfXbkNnM4mA9wBolhR2l77j6faOUzypn6e HNhRQLPV83STRmFS8w1NW2FSZ0ZnLTBjtWyDovMR6OrQDMF3HcNMT6GtBVHS6IGqDladGJ kxtQKw3B0ReKkoGHdJWoWXAOR4es3UGPcF4Va6613zc2citz+94t4v0KdaSfG1vv3ZKYYW xshETJ2iODHNz8kYizoGIGf1pLCk9lgD4FdOpquT+QpqQKyacH7jqdxDDp2+SA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1734360359; a=rsa-sha256; cv=none; b=Lv//vFMeKFJPBK3GQ7tnN2XTY0W6I6S0dkk7YLK1Sje1EsDlYTgYbeHZahM3s48m5oAXXg J2d6ngXUHtk7A/eZy8kTdti2BsQ6b6iocI9908BKDAT49CAxswkGWuokZYWGSeRR8Y/hm8 gUnBjKVHz8voKvVJtKhjIVCen5rQ1hyjUFtFKLGRbFVWfYWXNf88rr9/pzdmFEKAF1a5+A tBO6oZdTHTC1P/yJOkxyOvc7Jv0/Mb71FyBwoC8Iy0VQb7dg1EcsfaAXA6BIJVv6oDSOLd taRV5JWvImeOiASXGrrrHUABBgMoOlNwbtOL2jjL0Inq/iF82LDhUTBkR7y43g== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4YBjRq0996zx0P; Mon, 16 Dec 2024 14:45:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 4BGEjwuh053458; Mon, 16 Dec 2024 14:45:58 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4BGEjwn0053455; Mon, 16 Dec 2024 14:45:58 GMT (envelope-from git) Date: Mon, 16 Dec 2024 14:45:58 GMT Message-Id: <202412161445.4BGEjwn0053455@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: 11eb32958f2c - main - MAC/do: jail_check()/jail_set(): Revamp List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org 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: 11eb32958f2c6e70892201982c1c92a0140d6864 Auto-Submitted: auto-generated The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=11eb32958f2c6e70892201982c1c92a0140d6864 commit 11eb32958f2c6e70892201982c1c92a0140d6864 Author: Olivier Certner AuthorDate: 2024-07-03 15:44:24 +0000 Commit: Olivier Certner CommitDate: 2024-12-16 14:42:37 +0000 MAC/do: jail_check()/jail_set(): Revamp Handle JAIL_SYS_DISABLE the same as JAIL_SYS_NEW with an empty rules specification, coherently with jail_get(). Also accept JAIL_SYS_DISABLE in "mac.do" without "mac.do.rules" being specified. The default value for "mac.do", if not passed explicitly, is either JAIL_SYS_NEW if "mac.do.rules" is present and non-empty, or JAIL_SYS_DISABLE if present and empty or not present. Perform all cheap sanity checks in jail_check(), and have these materialized as well in jail_set() under INVARIANTS. Cheap checks are type and coherency checks between the values of "mac.do" and "mac.do.rules". They don't include parsing the "mac.do.rules" string but just checking its length (when applicable). In a nutshell, JAIL_SYS_DISABLE and JAIL_SYS_INHERIT are allowed iff "mac.do.rules" isn't specified or is with an empty string, and JAIL_SYS_NEW is allowed iff "mac.do.rules" is specified (the latter may be empty, in which case this is equivalent to JAIL_SYS_DISABLE). Normally, vfs_getopts() is the function to use to read string options. Because we need the length of the "mac.do.rules" string to check it, in order to avoid double search within jail options in jail_check(), we use vfs_getopt() instead, but perform some additional checks afterwards (the same as those performed by vfs_getopts()). Reviewed by: bapt Approved by: markj (mentor) Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D47610 --- sys/security/mac_do/mac_do.c | 128 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 111 insertions(+), 17 deletions(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index 2482221e43a3..ed1d0bcfa43c 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -377,31 +377,94 @@ done: return (error); } +/* + * -1 is used as a sentinel in mac_do_jail_check() and mac_do_jail_set() below. + */ +_Static_assert(-1 != JAIL_SYS_DISABLE && -1 != JAIL_SYS_NEW && + -1 != JAIL_SYS_INHERIT, + "mac_do(4) uses -1 as a sentinel for uninitialized 'jsys'."); + +/* + * We perform only cheap checks here, i.e., we do not really parse the rules + * specification string, if any. + */ static int mac_do_jail_check(void *obj, void *data) { struct vfsoptlist *opts = data; char *rules_string; - int error, jsys, len; + int error, jsys, size; error = vfs_copyopt(opts, "mac.do", &jsys, sizeof(jsys)); - if (error != ENOENT) { + if (error == ENOENT) + jsys = -1; + else { if (error != 0) return (error); - if (jsys != JAIL_SYS_NEW && jsys != JAIL_SYS_INHERIT) + if (jsys != JAIL_SYS_DISABLE && jsys != JAIL_SYS_NEW && + jsys != JAIL_SYS_INHERIT) return (EINVAL); } - error = vfs_getopt(opts, "mac.do.rules", (void **)&rules_string, &len); - if (error != ENOENT) { + + /* + * We use vfs_getopt() here instead of vfs_getopts() to get the length. + * We perform the additional checks done by the latter here, even if + * jail_set() calls vfs_getopts() itself later (they becoming + * inconsistent wouldn't cause any security problem). + */ + error = vfs_getopt(opts, "mac.do.rules", (void**)&rules_string, &size); + if (error == ENOENT) { + /* + * Default (in absence of "mac.do.rules") is to disable (and, in + * particular, not inherit). + */ + if (jsys == -1) + jsys = JAIL_SYS_DISABLE; + + if (jsys == JAIL_SYS_NEW) { + vfs_opterror(opts, "'mac.do.rules' must be specified " + "given 'mac.do''s value"); + return (EINVAL); + } + + /* Absence of "mac.do.rules" at this point is OK. */ + error = 0; + } else { if (error != 0) return (error); - if (len > MAC_RULE_STRING_LEN) { - vfs_opterror(opts, "mdo.rules too long"); + + /* Not a proper string. */ + if (size == 0 || rules_string[size - 1] != '\0') { + vfs_opterror(opts, "'mac.do.rules' not a proper string"); + return (EINVAL); + } + + if (size > MAC_RULE_STRING_LEN) { + vfs_opterror(opts, "'mdo.rules' too long"); return (ENAMETOOLONG); } + + if (jsys == -1) + /* Default (if "mac.do.rules" is present). */ + jsys = rules_string[0] == '\0' ? JAIL_SYS_DISABLE : + JAIL_SYS_NEW; + + /* + * Be liberal and accept JAIL_SYS_DISABLE and JAIL_SYS_INHERIT + * with an explicit empty rules specification. + */ + switch (jsys) { + case JAIL_SYS_DISABLE: + case JAIL_SYS_INHERIT: + if (rules_string[0] != '\0') { + vfs_opterror(opts, "'mac.do.rules' specified " + "but should not given 'mac.do''s value"); + return (EINVAL); + } + break; + } } - if (error == ENOENT) - error = 0; + return (error); } @@ -411,24 +474,55 @@ mac_do_jail_set(void *obj, void *data) struct prison *pr = obj; struct vfsoptlist *opts = data; char *rules_string; - int error, jsys, len; + int error, jsys; + + /* + * The invariants checks used below correspond to what has already been + * checked in jail_check() above. + */ error = vfs_copyopt(opts, "mac.do", &jsys, sizeof(jsys)); - if (error == ENOENT) - jsys = -1; - error = vfs_getopt(opts, "mac.do.rules", (void **)&rules_string, &len); - if (error == ENOENT) - rules_string = ""; - else - jsys = JAIL_SYS_NEW; + MPASS(error == 0 || error == ENOENT); + if (error != 0) + jsys = -1; /* Mark unfilled. */ + + rules_string = vfs_getopts(opts, "mac.do.rules", &error); + MPASS(error == 0 || error == ENOENT); + if (error == 0) { + MPASS(strlen(rules_string) < MAC_RULE_STRING_LEN); + if (jsys == -1) + /* Default (if "mac.do.rules" is present). */ + jsys = rules_string[0] == '\0' ? JAIL_SYS_DISABLE : + JAIL_SYS_NEW; + else + MPASS(jsys == JAIL_SYS_NEW || + ((jsys == JAIL_SYS_DISABLE || + jsys == JAIL_SYS_INHERIT) && + rules_string[0] == '\0')); + } else { + MPASS(jsys != JAIL_SYS_NEW); + if (jsys == -1) + /* + * Default (in absence of "mac.do.rules") is to disable + * (and, in particular, not inherit). + */ + jsys = JAIL_SYS_DISABLE; + /* If disabled, we'll store an empty rule specification. */ + if (jsys == JAIL_SYS_DISABLE) + rules_string = ""; + } + switch (jsys) { case JAIL_SYS_INHERIT: remove_rules(pr); error = 0; break; + case JAIL_SYS_DISABLE: case JAIL_SYS_NEW: error = parse_and_set_rules(pr, rules_string); break; + default: + __assert_unreachable(); } return (error); }