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