From: Paul Moore <paul@paul-moore.com>
To: Fan Wu <wufan@linux.microsoft.com>
Cc: corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org,
serge@hallyn.com, tytso@mit.edu, ebiggers@kernel.org,
axboe@kernel.dk, agk@redhat.com, snitzer@kernel.org,
eparis@redhat.com, linux-doc@vger.kernel.org,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fscrypt@vger.kernel.org, linux-block@vger.kernel.org,
dm-devel@lists.linux.dev, audit@vger.kernel.org,
linux-kernel@vger.kernel.org,
Deven Bowers <deven.desai@linux.microsoft.com>
Subject: Re: [PATCH RFC v12 15/20] ipe: add support for dm-verity as a trust provider
Date: Tue, 6 Feb 2024 16:53:23 -0500 [thread overview]
Message-ID: <CAHC9VhS3Yb9QE3spJjFn2Mef-6m5Jxk6Yr80O1VkLp-yudp62w@mail.gmail.com> (raw)
In-Reply-To: <05cb5f03-9236-47b7-8dd4-1741c289efdc@linux.microsoft.com>
On Mon, Feb 5, 2024 at 6:11 PM Fan Wu <wufan@linux.microsoft.com> wrote:
> On 2/3/2024 2:25 PM, Paul Moore wrote:
> > On Jan 30, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
> >>
> >> Allows author of IPE policy to indicate trust for a singular dm-verity
> >> volume, identified by roothash, through "dmverity_roothash" and all
> >> signed dm-verity volumes, through "dmverity_signature".
> >>
> >> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> >> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> >> ---
> >> v2:
> >> + No Changes
> >>
> >> v3:
> >> + No changes
> >>
> >> v4:
> >> + No changes
> >>
> >> v5:
> >> + No changes
> >>
> >> v6:
> >> + Fix an improper cleanup that can result in
> >> a leak
> >>
> >> v7:
> >> + Squash patch 08/12, 10/12 to [11/16]
> >>
> >> v8:
> >> + Undo squash of 08/12, 10/12 - separating drivers/md/ from security/
> >> & block/
> >> + Use common-audit function for dmverity_signature.
> >> + Change implementation for storing the dm-verity digest to use the
> >> newly introduced dm_verity_digest structure introduced in patch
> >> 14/20.
> >>
> >> v9:
> >> + Adapt to the new parser
> >>
> >> v10:
> >> + Select the Kconfig when all dependencies are enabled
> >>
> >> v11:
> >> + No changes
> >>
> >> v12:
> >> + Refactor to use struct digest_info* instead of void*
> >> + Correct audit format
> >> ---
> >> security/ipe/Kconfig | 18 ++++++
> >> security/ipe/Makefile | 1 +
> >> security/ipe/audit.c | 37 ++++++++++-
> >> security/ipe/digest.c | 120 +++++++++++++++++++++++++++++++++++
> >> security/ipe/digest.h | 26 ++++++++
> >> security/ipe/eval.c | 90 +++++++++++++++++++++++++-
> >> security/ipe/eval.h | 10 +++
> >> security/ipe/hooks.c | 67 +++++++++++++++++++
> >> security/ipe/hooks.h | 8 +++
> >> security/ipe/ipe.c | 15 +++++
> >> security/ipe/ipe.h | 4 ++
> >> security/ipe/policy.h | 3 +
> >> security/ipe/policy_parser.c | 26 +++++++-
> >> 13 files changed, 421 insertions(+), 4 deletions(-)
> >> create mode 100644 security/ipe/digest.c
> >> create mode 100644 security/ipe/digest.h
> >>
> >> diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
> >> index ac4d558e69d5..7afb1ce0cb99 100644
> >> --- a/security/ipe/Kconfig
> >> +++ b/security/ipe/Kconfig
> >> @@ -8,6 +8,7 @@ menuconfig SECURITY_IPE
> >> depends on SECURITY && SECURITYFS && AUDIT && AUDITSYSCALL
> >> select PKCS7_MESSAGE_PARSER
> >> select SYSTEM_DATA_VERIFICATION
> >> + select IPE_PROP_DM_VERITY if DM_VERITY && DM_VERITY_VERIFY_ROOTHASH_SIG
> >> help
> >> This option enables the Integrity Policy Enforcement LSM
> >> allowing users to define a policy to enforce a trust-based access
> >> @@ -15,3 +16,20 @@ menuconfig SECURITY_IPE
> >> admins to reconfigure trust requirements on the fly.
> >>
> >> If unsure, answer N.
> >> +
> >> +if SECURITY_IPE
> >> +menu "IPE Trust Providers"
> >> +
> >> +config IPE_PROP_DM_VERITY
> >> + bool "Enable support for dm-verity volumes"
> >> + depends on DM_VERITY && DM_VERITY_VERIFY_ROOTHASH_SIG
> >> + help
> >> + This option enables the properties 'dmverity_signature' and
> >> + 'dmverity_roothash' in IPE policy. These properties evaluates
> >> + to TRUE when a file is evaluated against a dm-verity volume
> >> + that was mounted with a signed root-hash or the volume's
> >> + root hash matches the supplied value in the policy.
> >> +
> >> +endmenu
> >> +
> >> +endif
> >> diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> >> index 2279eaa3cea3..66de53687d11 100644
> >> --- a/security/ipe/Makefile
> >> +++ b/security/ipe/Makefile
> >> @@ -6,6 +6,7 @@
> >> #
> >>
> >> obj-$(CONFIG_SECURITY_IPE) += \
> >> + digest.o \
> >> eval.o \
> >> hooks.o \
> >> fs.o \
> >> diff --git a/security/ipe/audit.c b/security/ipe/audit.c
> >> index ed390d32c641..a4ad8e888df0 100644
> >> --- a/security/ipe/audit.c
> >> +++ b/security/ipe/audit.c
> >> @@ -13,6 +13,7 @@
> >> #include "hooks.h"
> >> #include "policy.h"
> >> #include "audit.h"
> >> +#include "digest.h"
> >>
> >> #define ACTSTR(x) ((x) == IPE_ACTION_ALLOW ? "ALLOW" : "DENY")
> >>
> >> @@ -54,8 +55,30 @@ static const char *const audit_prop_names[__IPE_PROP_MAX] = {
> >> "boot_verified=FALSE",
> >> "boot_verified=TRUE",
> >> #endif /* CONFIG_BLK_DEV_INITRD */
> >> +#ifdef CONFIG_IPE_PROP_DM_VERITY
> >> + "dmverity_roothash=",
> >> + "dmverity_signature=FALSE",
> >> + "dmverity_signature=TRUE",
> >> +#endif /* CONFIG_IPE_PROP_DM_VERITY */
> >> };
> >>
> >> +#ifdef CONFIG_IPE_PROP_DM_VERITY
> >> +/**
> >> + * audit_dmv_roothash - audit a roothash of a dmverity volume.
> >> + * @ab: Supplies a pointer to the audit_buffer to append to.
> >> + * @rh: Supplies a pointer to the digest structure.
> >> + */
> >> +static void audit_dmv_roothash(struct audit_buffer *ab, const void *rh)
> >> +{
> >> + audit_log_format(ab, "%s", audit_prop_names[IPE_PROP_DMV_ROOTHASH]);
> >> + ipe_digest_audit(ab, rh);
> >> +}
> >> +#else
> >> +static void audit_dmv_roothash(struct audit_buffer *ab, const void *rh)
> >> +{
> >> +}
> >> +#endif /* CONFIG_IPE_PROP_DM_VERITY */
> >
> > I talked about this back in my review of the v11 patchset and I'm
> > guessing you may have missed it ... the problem with the above code is
> > that the fields in an audit record should remain constant, even if
> > there is no data for that particular field. In cases where there is no
> > data to record for a given field, a "?" should be used as the field's
> > value, for example:
> >
> > dmverify_roothash=?
> >
> > My guess is that you would want to do something like this:
> >
> > #else /* !CONFIG_IPE_PROP_DM_VERITY */
> > static void audit_dmv_roothash(...)
> > {
> > audit_log_format(ab, "%s=?", audit_prop_names[...]);
> > }
> > #endif /* CONFIG_IPE_PROP_DM_VERITY */
> >
> > --
> > paul-moore.com
>
> These code are used for auditing a policy rule, which the parser will
> guarantee the property will always have a valid value. The comments
> might be misleading which sounds like it's auditing a file's state. I
> will correct them.
>
> Also as we previously discussed, the policy grammar shouldn't depend on
> any kernel switch so these preprocessor statement will be removed.
>
> However, as an audit record should remain constant, I guess we should do
> some special treatment to anonymous files? Like audit record for them
> should include "path=? dev=? ino=?"
Yes, if the record type includes those fields just once, the record
type should *always* include those fields.
--
paul-moore.com
next prev parent reply other threads:[~2024-02-06 21:53 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 22:36 [RFC PATCH v12 00/20] Integrity Policy Enforcement LSM (IPE) Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 01/20] security: add ipe lsm Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 02/20] ipe: add policy parser Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 03/20] ipe: add evaluation loop Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 04/20] ipe: add LSM hooks on execution and kernel read Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 05/20] initramfs|security: Add security hook to initramfs unpack Fan Wu
2024-02-03 22:25 ` [PATCH RFC v12 5/20] " Paul Moore
2024-02-05 21:18 ` Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 06/20] ipe: introduce 'boot_verified' as a trust provider Fan Wu
2024-02-03 22:25 ` [PATCH RFC v12 6/20] " Paul Moore
2024-02-05 22:39 ` Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 07/20] security: add new securityfs delete function Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 08/20] ipe: add userspace interface Fan Wu
2024-02-03 22:25 ` [PATCH RFC v12 8/20] " Paul Moore
2024-02-05 23:01 ` Fan Wu
2024-02-05 23:10 ` Paul Moore
2024-02-05 23:21 ` Fan Wu
2024-01-30 22:36 ` [RFC PATCH v12 09/20] uapi|audit|ipe: add ipe auditing support Fan Wu
2024-02-03 22:25 ` [PATCH RFC v12 9/20] " Paul Moore
2024-01-30 22:36 ` [RFC PATCH v12 10/20] ipe: add permissive toggle Fan Wu
2024-02-03 22:25 ` [PATCH RFC " Paul Moore
2024-01-30 22:36 ` [RFC PATCH v12 11/20] block|security: add LSM blob to block_device Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 12/20] dm verity: set DM_TARGET_SINGLETON feature flag Fan Wu
2024-02-02 18:51 ` Mike Snitzer
2024-02-03 3:56 ` Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 13/20] dm: add finalize hook to target_type Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 14/20] dm verity: consume root hash digest and signature data via LSM hook Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 15/20] ipe: add support for dm-verity as a trust provider Fan Wu
2024-02-03 22:25 ` [PATCH RFC " Paul Moore
2024-02-05 23:11 ` Fan Wu
2024-02-06 21:53 ` Paul Moore [this message]
2024-01-30 22:37 ` [RFC PATCH v12 16/20] fsverity: consume builtin signature via LSM hook Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 17/20] ipe: enable support for fs-verity as a trust provider Fan Wu
2024-02-03 22:25 ` [PATCH RFC " Paul Moore
2024-01-30 22:37 ` [RFC PATCH v12 18/20] scripts: add boot policy generation program Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 19/20] ipe: kunit test for parser Fan Wu
2024-01-30 22:37 ` [RFC PATCH v12 20/20] documentation: add ipe documentation Fan Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHC9VhS3Yb9QE3spJjFn2Mef-6m5Jxk6Yr80O1VkLp-yudp62w@mail.gmail.com \
--to=paul@paul-moore.com \
--cc=agk@redhat.com \
--cc=audit@vger.kernel.org \
--cc=axboe@kernel.dk \
--cc=corbet@lwn.net \
--cc=deven.desai@linux.microsoft.com \
--cc=dm-devel@lists.linux.dev \
--cc=ebiggers@kernel.org \
--cc=eparis@redhat.com \
--cc=jmorris@namei.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=serge@hallyn.com \
--cc=snitzer@kernel.org \
--cc=tytso@mit.edu \
--cc=wufan@linux.microsoft.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).