> From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx] > Sent: Thursday, September 17, 2020 4:25 PM > Hi Roberto, > > On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote: > > With the patch to accept EVM portable signatures when the > > appraise_type=imasig requirement is specified in the policy, appraisal can > > be successfully done even if the file does not have an IMA signature. > > > > However, remote attestation would not see that a different signature > type > > was used, as only IMA signatures can be included in the measurement list. > > This patch solves the issue by introducing the new template field 'evmsig' > > to show EVM portable signatures and by including its value in the existing > > field 'sig' if the IMA signature is not found. > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > Suggested-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > > Thank you! Just a minor comment below. > > <snip> > > > diff --git a/security/integrity/ima/ima_template_lib.c > b/security/integrity/ima/ima_template_lib.c > > index c022ee9e2a4e..2c596c2a89cc 100644 > > --- a/security/integrity/ima/ima_template_lib.c > > +++ b/security/integrity/ima/ima_template_lib.c > > > > @@ -438,7 +439,7 @@ int ima_eventsig_init(struct ima_event_data > *event_data, > > struct evm_ima_xattr_data *xattr_value = event_data->xattr_value; > > > > if ((!xattr_value) || (xattr_value->type != > EVM_IMA_XATTR_DIGSIG)) > > - return 0; > > + return ima_eventevmsig_init(event_data, field_data); > > > > return ima_write_template_field_data(xattr_value, event_data- > >xattr_len, > > DATA_FMT_HEX, field_data); > > @@ -484,3 +485,39 @@ int ima_eventmodsig_init(struct ima_event_data > *event_data, > > return ima_write_template_field_data(data, data_len, > DATA_FMT_HEX, > > field_data); > > } > > + > > +/* > > + * ima_eventevmsig_init - include the EVM portable signature as part of > the > > + * template data > > + */ > > +int ima_eventevmsig_init(struct ima_event_data *event_data, > > + struct ima_field_data *field_data) > > +{ > > + struct evm_ima_xattr_data *xattr_data = NULL; > > + int rc = 0; > > + > > + if (!event_data->file) > > + return 0; > > + > > + if (!(file_inode(event_data->file)->i_opflags & IOP_XATTR)) > > + return 0; > > + > > + rc = vfs_getxattr_alloc(file_dentry(event_data->file), > XATTR_NAME_EVM, > > + (char **)&xattr_data, 0, GFP_NOFS); > > + if (rc <= 0) { > > + if (!rc || rc == -ENODATA) > > + return 0; > > + > > + return rc; > > We're including the EVM signature on a best effort basis to help with > attestation. Do we really care why it failed? Are we going to act on > it? Hi Mimi other template field functions have a similar behavior. They return an error if an operation necessary to retrieve the data cannot be performed. Should I always return 0? Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > Mimi > > > + } > > + > > + if (xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG) { > > + kfree(xattr_data); > > + return 0; > > + } > > + > > + rc = ima_write_template_field_data((char *)xattr_data, rc, > DATA_FMT_HEX, > > + field_data); > > + kfree(xattr_data); > > + return rc; > > +} >
