Date: Mon, 17 Apr 2023 00:40:07 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: "Stephen J. Kiernan" <stevek@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 6ae8d57652fa - main - mac_veriexec: add mac_priv_grant check for NODEV Message-ID: <C919D121-5342-4A3A-8CEB-07B9346D8024@freebsd.org> In-Reply-To: <202304162314.33GNEwXd039914@gitrepo.freebsd.org> References: <202304162314.33GNEwXd039914@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 17 Apr 2023, at 00:14, Stephen J. Kiernan <stevek@FreeBSD.org> wrote: >=20 > The branch main has been updated by stevek: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D6ae8d57652faf3bb8532ed627676c65e= ecd94a31 >=20 > commit 6ae8d57652faf3bb8532ed627676c65eecd94a31 > Author: Simon J. Gerraty <sjg@juniper.net> > AuthorDate: 2019-07-29 22:38:16 +0000 > Commit: Stephen J. Kiernan <stevek@FreeBSD.org> > CommitDate: 2023-04-16 23:14:40 +0000 >=20 > mac_veriexec: add mac_priv_grant check for NODEV >=20 > Allow other MAC modules to override some veriexec checks. >=20 > We need two new privileges: > PRIV_VERIEXEC_DIRECT process wants to override 'indirect' flag > on interpreter > PRIV_VERIEXEC_NOVERIFY typically associated with = PRIV_VERIEXEC_DIRECT > allow override of O_VERIFY >=20 > We also need to check for PRIV_VERIEXEC_NOVERIFY override > for FINGERPRINT_NODEV and FINGERPRINT_NOENTRY. > This will only happen if parent had PRIV_VERIEXEC_DIRECT override. >=20 > This allows for MAC modules to selectively allow some applications = to > run without verification. >=20 > Needless to say, this is extremely dangerous and should only be = used > sparingly and carefully. >=20 > Obtained from: Juniper Networks, Inc. >=20 > Reviewers: sjg > Subscribers: imp, dab >=20 > Differential Revision: https://reviews.freebsd.org/D39537 Hi Steve, I see you=E2=80=99ve made a bunch of commits over the past few days that = suffer from not following the proper commit message formatting outlined in the Committer=E2=80=99s Guide and templated in tools/tools/git/hooks/prepare-commit-msg; can you please take care and do so in future? Jess > --- > sys/security/mac_veriexec/mac_veriexec.c | 16 ++++++++++++++++ > sys/security/mac_veriexec/veriexec_fingerprint.c | 23 = ++++++++++++++++++++++- > sys/sys/priv.h | 8 +++++++- > 3 files changed, 45 insertions(+), 2 deletions(-) >=20 > diff --git a/sys/security/mac_veriexec/mac_veriexec.c = b/sys/security/mac_veriexec/mac_veriexec.c > index e377f61ad21c..b20df7d694ef 100644 > --- a/sys/security/mac_veriexec/mac_veriexec.c > +++ b/sys/security/mac_veriexec/mac_veriexec.c > @@ -51,6 +51,7 @@ > #include <sys/sysctl.h> > #include <sys/vnode.h> > #include <fs/nullfs/null.h> > +#include <security/mac/mac_framework.h> > #include <security/mac/mac_policy.h> >=20 > #include "mac_veriexec.h" > @@ -430,6 +431,18 @@ mac_veriexec_priv_check(struct ucred *cred, int = priv) > return (0); > } >=20 > +/** > + * @internal > + * @brief Check if the requested sysctl should be allowed > + * > + * @param cred credentials to use > + * @param oidp sysctl OID > + * @param arg1 first sysctl argument > + * @param arg2 second sysctl argument > + * @param req sysctl request information > + * > + * @return 0 if the sysctl should be allowed, otherwise an error = code. > + */ > static int > mac_veriexec_sysctl_check(struct ucred *cred, struct sysctl_oid *oidp, > void *arg1, int arg2, struct sysctl_req *req) > @@ -533,6 +546,9 @@ mac_veriexec_check_vp(struct ucred *cred, struct = vnode *vp, accmode_t accmode) > return (error); > break; > default: > + /* Allow for overriding verification requirement = */ > + if (mac_priv_grant(cred, PRIV_VERIEXEC_NOVERIFY) = =3D=3D 0) > + return (0); > /* > * Caller wants open to fail unless there is a = valid > * fingerprint registered. > diff --git a/sys/security/mac_veriexec/veriexec_fingerprint.c = b/sys/security/mac_veriexec/veriexec_fingerprint.c > index 29b5c19eed1e..500842cbd5ab 100644 > --- a/sys/security/mac_veriexec/veriexec_fingerprint.c > +++ b/sys/security/mac_veriexec/veriexec_fingerprint.c > @@ -42,11 +42,14 @@ > #include <sys/malloc.h> > #include <sys/mount.h>=20 > #include <sys/mutex.h> > +#include <sys/priv.h> > #include <sys/proc.h> > #include <sys/sbuf.h> > #include <sys/syslog.h> > #include <sys/vnode.h> >=20 > +#include <security/mac/mac_framework.h> > + > #include "mac_veriexec.h" > #include "mac_veriexec_internal.h" >=20 > @@ -292,7 +295,8 @@ mac_veriexec_fingerprint_check_image(struct = image_params *imgp, >=20 > case FINGERPRINT_INDIRECT: /* fingerprint ok but need to check > for direct execution */ > - if (!imgp->interpreted) { > + if (!imgp->interpreted && > + mac_priv_grant(td->td_ucred, PRIV_VERIEXEC_DIRECT) = !=3D 0) { > identify_error(imgp, td, "attempted direct = execution"); > if (prison0.pr_securelevel > 1 || > = mac_veriexec_in_state(VERIEXEC_STATE_ENFORCE)) > @@ -326,6 +330,23 @@ mac_veriexec_fingerprint_check_image(struct = image_params *imgp, > identify_error(imgp, td, "invalid status field for = vnode"); > error =3D EPERM; > } > + switch (status) { > + case FINGERPRINT_NODEV: > + case FINGERPRINT_NOENTRY: > + /* > + * Check if this process has override allowed. > + * This will only be true if PRIV_VERIEXEC_DIRECT > + * already succeeded. > + */ > + if (error =3D=3D EAUTH && > + mac_priv_grant(td->td_ucred, PRIV_VERIEXEC_NOVERIFY) = =3D=3D 0) { > + error =3D 0; > + } > + break; > + default: > + break; > + } > + > return error;=20 > } >=20 > diff --git a/sys/sys/priv.h b/sys/sys/priv.h > index cb4dcecea4aa..6574d8c42599 100644 > --- a/sys/sys/priv.h > +++ b/sys/sys/priv.h > @@ -520,10 +520,16 @@ > */ > #define PRIV_KDB_SET_BACKEND 690 /* Allow setting KDB = backend. */ >=20 > +/* > + * veriexec override privileges - very rare! > + */ > +#define PRIV_VERIEXEC_DIRECT 700 /* Can override = 'indirect' */ > +#define PRIV_VERIEXEC_NOVERIFY 701 /* Can override O_VERIFY = */ > + > /* > * Track end of privilege list. > */ > -#define _PRIV_HIGHEST 691 > +#define _PRIV_HIGHEST 702 >=20 > /* > * Validate that a named privilege is known by the privilege system. = Invalid
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?C919D121-5342-4A3A-8CEB-07B9346D8024>