Re: git: 6ae8d57652fa - main - mac_veriexec: add mac_priv_grant check for NODEV
Date: Sun, 16 Apr 2023 23:40:07 UTC
On 17 Apr 2023, at 00:14, Stephen J. Kiernan <stevek@FreeBSD.org> wrote: > > The branch main has been updated by stevek: > > URL: https://cgit.FreeBSD.org/src/commit/?id=6ae8d57652faf3bb8532ed627676c65eecd94a31 > > 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 > > mac_veriexec: add mac_priv_grant check for NODEV > > Allow other MAC modules to override some veriexec checks. > > 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 > > 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. > > This allows for MAC modules to selectively allow some applications to > run without verification. > > Needless to say, this is extremely dangerous and should only be used > sparingly and carefully. > > Obtained from: Juniper Networks, Inc. > > Reviewers: sjg > Subscribers: imp, dab > > Differential Revision: https://reviews.freebsd.org/D39537 Hi Steve, I see you’ve made a bunch of commits over the past few days that suffer from not following the proper commit message formatting outlined in the Committer’s 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(-) > > 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> > > #include "mac_veriexec.h" > @@ -430,6 +431,18 @@ mac_veriexec_priv_check(struct ucred *cred, int priv) > return (0); > } > > +/** > + * @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) == 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> > #include <sys/mutex.h> > +#include <sys/priv.h> > #include <sys/proc.h> > #include <sys/sbuf.h> > #include <sys/syslog.h> > #include <sys/vnode.h> > > +#include <security/mac/mac_framework.h> > + > #include "mac_veriexec.h" > #include "mac_veriexec_internal.h" > > @@ -292,7 +295,8 @@ mac_veriexec_fingerprint_check_image(struct image_params *imgp, > > 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) != 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 = 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 == EAUTH && > + mac_priv_grant(td->td_ucred, PRIV_VERIEXEC_NOVERIFY) == 0) { > + error = 0; > + } > + break; > + default: > + break; > + } > + > return error; > } > > 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. */ > > +/* > + * 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 > > /* > * Validate that a named privilege is known by the privilege system. Invalid