On Mon, 2019-06-03 at 21:51 -0400, Mimi Zohar wrote:
> On Mon, 2019-06-03 at 13:13 -0700, Matthew Garrett wrote:
> > Admins may wish to log different measurements using different IMA
> > templates. Add support for overriding the default template on a per-rule
> > basis.
> >
> > Signed-off-by: Matthew Garrett <mjg59@xxxxxxxxxx>
> > ---
> >
> > Updated based on review feedback, verified that I can generate an event
> > log that contains multiple different templates.
> >
> > Documentation/ABI/testing/ima_policy | 6 ++++--
> > security/integrity/ima/ima.h | 13 +++++++++----
> > security/integrity/ima/ima_api.c | 24 ++++++++++++++++-------
> > security/integrity/ima/ima_appraise.c | 2 +-
> > security/integrity/ima/ima_init.c | 2 +-
> > security/integrity/ima/ima_main.c | 9 +++++----
> > security/integrity/ima/ima_policy.c | 28 +++++++++++++++++++++++++--
> > security/integrity/ima/ima_template.c | 10 ++++++++--
> > 8 files changed, 71 insertions(+), 23 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> > index 74c6702de74e..4ded0668a22d 100644
> > --- a/Documentation/ABI/testing/ima_policy
> > +++ b/Documentation/ABI/testing/ima_policy
> > @@ -24,8 +24,7 @@ Description:
> > [euid=] [fowner=] [fsname=]]
> > lsm: [[subj_user=] [subj_role=] [subj_type=]
> > [obj_user=] [obj_role=] [obj_type=]]
> > - option: [[appraise_type=]] [permit_directio]
> > -
> > + option: [[appraise_type=]] [template=] [permit_directio]
> > base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> > [FIRMWARE_CHECK]
> > [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> > @@ -38,6 +37,9 @@ Description:
> > fowner:= decimal value
> > lsm: are LSM specific
> > option: appraise_type:= [imasig]
> > + template:= name or format of a defined IMA template
> > + type (eg,ima-ng or d-ng|n-ng). Only valid when action
> > + is "measure".
>
> This patch only supports specifying the template name, not the
> template format description. Please remove "d-ng|n-ng".
>
> > pcr:= decimal value
> >
> > default policy:
>
> <snip>
>
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index 0f6fe53cef09..cbae2a3a9c5b 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -80,6 +80,7 @@ struct ima_rule_entry {
> > int type; /* audit type */
> > } lsm[MAX_LSM_RULES];
> > char *fsname;
> > + struct ima_template_desc *template;
> > };
> >
> > /*
> > @@ -397,6 +398,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
> > * @func: IMA hook identifier
> > * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
> > * @pcr: set the pcr to extend
> > + * @template_desc: the template that should be used for this rule
> > *
> > * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
> > * conditions.
> > @@ -406,7 +408,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
> > * than writes so ima_match_policy() is classical RCU candidate.
> > */
> > int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> > - enum ima_hooks func, int mask, int flags, int *pcr)
> > + enum ima_hooks func, int mask, int flags, int *pcr,
> > + struct ima_template_desc **template_desc)
> > {
> > struct ima_rule_entry *entry;
> > int action = 0, actmask = flags | (flags << 1);
> > @@ -438,6 +441,11 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> > if ((pcr) && (entry->flags & IMA_PCR))
> > *pcr = entry->pcr;
> >
> > + if (template_desc && entry->template)
> > + *template_desc = entry->template;
> > + else
> > + *template_desc = ima_template_desc_current();
> > +
>
> This code is finding the template format, but is subsequently being
> replaced with the current description. One way of fixing this, is by
> initializing the template_desc before walking the list.
Perhaps not, but the "else" clause also needs to test template_desc.
The other option is for all ima_match_policy callers to provide
template_desc.
Mimi
>
> > if (!actmask)
> > break;
> > }
> > @@ -676,7 +684,7 @@ enum {
> > Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
> > Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
> > Opt_appraise_type, Opt_permit_directio,
> > - Opt_pcr, Opt_err
> > + Opt_pcr, Opt_template, Opt_err
> > };
> >
