Date: Mon, 29 Sep 2025 17:38:12 GMT From: Olivier Certner <olce@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 9f269a0a771a - main - MAC/do: Check executable path from the current jail's root Message-ID: <202509291738.58THcCqC061015@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=9f269a0a771aff4f0a735211907a52c52fc0661b commit 9f269a0a771aff4f0a735211907a52c52fc0661b Author: Olivier Certner <olce@FreeBSD.org> AuthorDate: 2025-09-27 09:56:33 +0000 Commit: Olivier Certner <olce@FreeBSD.org> CommitDate: 2025-09-29 17:37:12 +0000 MAC/do: Check executable path from the current jail's root Contrary to my initial belief, vn_fullpath() does return a vnode's path from the current chroot, and not from the global root (which would have been a bug also, but without security consequences). This enables a "confused deputy"-like scenario where a chroot(2) can change which executable can be authorized by MAC/do, which is even more problematic for unprivileged chroot(2). This was found by re-examining the code following two close events: 1. Shawn Webb sent a mail to freebsd-hackers@ on 08/05 saying that in HardenedBSD they had added a check on P2_NO_NEW_PRIVS (in mac_do_priv_grant()), which I responded to on 08/20 saying that P2_NO_NEW_PRIVS was not necessary for mac_do(4), with a correct reasoning but based on the wrong above-mentioned assumption about vn_fullpath(). 2. I reviewed some code by Kushagra Srivastava (GSoC 2025 student working on mac_do(4)/mdo(1)) adding the ability to specify which executables can spawn processes that mac_do(4) may decide to authorize (others are simply ignored), which currently is hardcoded to '/usr/bin/mdo'. MFC after: 3 days Event: EuroBSDCon 2025 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D52758 --- sys/security/mac_do/mac_do.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index 6f3e63d06198..2bcff7bba973 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -1992,6 +1992,10 @@ check_proc(void) /* * Only grant privileges if requested by the right executable. * + * As MAC/do configuration is per-jail, in order to avoid confused + * deputy situations in chroots (privileged or unprivileged), make sure + * to check the path from the current jail's root. + * * XXXOC: We may want to base this check on a tunable path and/or * a specific MAC label. Going even further, e.g., envisioning to * completely replace the path check with the latter, we would need to @@ -2003,7 +2007,7 @@ check_proc(void) * setting a MAC label per file (perhaps via additions to mtree(1)). So * this probably isn't going to happen overnight, if ever. */ - if (vn_fullpath(curproc->p_textvp, &path, &to_free) != 0) + if (vn_fullpath_jail(curproc->p_textvp, &path, &to_free) != 0) return (EPERM); error = strcmp(path, "/usr/bin/mdo") == 0 ? 0 : EPERM; free(to_free, M_TEMP);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202509291738.58THcCqC061015>