Re: [Linux-ima-user] [RFC][PATCH][1/1] ima: extending the format of the measurement list

[Roberto S. <rob...@po...>]

On Thursday 15 April 2010 01:20:23 Mimi Zohar wrote:
> On Wed, 2010-04-14 at 11:52 +0200, Roberto Sassu wrote:
> > The format of the measurement list has been modified to carry additional 
> > information.
> > 
> > Signed-off-by: Roberto Sassu <rob...@po...>
> > Acked-by: Gianluca Ramunno <ra...@po...>
> 
> As this patchset contains only 1 patch, how about putting the
> cover-letter description, or an abbreviated description, here in the
> patch instead.
> 
Ok, i'll post another mail after applying all comments.

> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index 47fb65d..7fb66ef 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -13,6 +13,29 @@
> >   * File: ima.h
> >   *	internal Integrity Measurement Architecture (IMA) definitions
> >   */
> > +/* ====================================================================
> > + * Copyright (C) 2009-2010 Politecnico di Torino, Italy
> > + * TORSEC group -- http://security.polito.it
> > + * Author: Roberto Sassu <rob...@po...>
> > + *
> > + * The original IMA source code from IBM, has been modified by Roberto Sassu 
> > + * to display extra information in the measurement list displayed through the
> > + * securityfs filesystem.
> > + *
> > + * DISCLAIMER of WARRANTY
> > + *
> > + * The following software "IMA" with measurement list format enhanced
> > + * is experimental and is provided "as is", and no guarantee or warranty
> > + * is given by Politecnico di Torino (TORSEC group), that the software is
> > + * fit for any particular purpose.  The user thereof uses the software at its
> > + * sole risk and liability. Politecnico di Torino shall have no obligation to
> > + * maintain or support this software. Politecnico di Torino MAKES NO
> > + * EXPRESS OR IMPLIED WARRANTY OF ANY KIND REGARDING THIS SOFTWARE.
> > + * Politecnico di Torino SHALL NOT BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES, WHETHER BASED ON
> > + * CONTRACT, TORT OR ANY OTHER LEGAL THEORY, IN CONNECTION WITH OR
> > + * ARISING OUT OF THE FURNISHING, PERFORMANCE OR USE OF THIS SOFTWARE.
> > + */
> 
> The Linux kernel is the normal GPL v2 license. Instead, how about adding
> a line like:
> Copyright (C) 2009-2010 Politecnico di Torino, Italy, 
> 	Roberto Sassu <rob...@po...>
> 

Does this text is correct?

/*
 * Copyright (C) 2009-2010 Politecnico di Torino, Italy
 * TORSEC group -- http://security.polito.it
 * Author: Roberto Sassu <rob...@po...>
 */

> >  #ifndef __LINUX_IMA_H
> >  #define __LINUX_IMA_H
> > @@ -30,6 +53,7 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> >  /* digest size for IMA, fits SHA1 or MD5 */
> >  #define IMA_DIGEST_SIZE		20
> >  #define IMA_EVENT_NAME_LEN_MAX	255
> > +#define IMA_LABEL_LEN_MAX	255
> 
> The LSMs must have already defined something like this.
> 
I searched the variable in the kernel sources, but i didn't found it. I think there's no fixed size since the function 
"security_secid_to_secctx" returns itself the length of a newly allocated string.

> >  #define IMA_HASH_BITS 9
> >  #define IMA_MEASURE_HTABLE_SIZE (1 << IMA_HASH_BITS)
> > @@ -43,6 +67,8 @@ extern char *ima_hash;
> >  struct ima_template_data {
> >  	u8 digest[IMA_DIGEST_SIZE];	/* sha1/md5 measurement hash */
> >  	char file_name[IMA_EVENT_NAME_LEN_MAX + 1];	/* name + \0 */
> > +	char subj_label[IMA_LABEL_LEN_MAX + 1];	/* subj_label + \0 */
> > +	char obj_label[IMA_LABEL_LEN_MAX + 1];	/* obj_label + \0 */
> >  };
> 
> For backwards compatibility, this probably needs to be a new template,
> with a runtime option - orig vs. new template.
> 
I can implement a new "setup" function to add the new kernel parameter "ima_report_format="
At this point there are two possibilities:

- specifying a mask as value: this gives to platform owner the flexibility to specify the custom format it recognizes;
						 by default, the format will be the actual one;
- specifying a format version string: this permits to use an identifier to name a format with well defined fields;

The first one seems fine to me.

> >  struct ima_template_entry {
> > diff --git a/security/integrity/ima/ima_api.c 
> > b/security/integrity/ima/ima_api.c
> > index 2a5e0bc..e612a49 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -12,10 +12,34 @@
> >   *	Implements must_measure, collect_measurement, store_measurement,
> >   *	and store_template.
> >   */
> > +/* ====================================================================
> > + * Copyright (C) 2009-2010 Politecnico di Torino, Italy
> > + * TORSEC group -- http://security.polito.it
> > + * Author: Roberto Sassu <rob...@po...>
> > + *
> > + * The original IMA source code from IBM, has been modified by Roberto Sassu 
> > + * to display extra information in the measurement list displayed through the
> > + * securityfs filesystem.
> > + *
> > + * DISCLAIMER of WARRANTY
> > + *
> > + * The following software "IMA" with measurement list format enhanced
> > + * is experimental and is provided "as is", and no guarantee or warranty
> > + * is given by Politecnico di Torino (TORSEC group), that the software is
> > + * fit for any particular purpose.  The user thereof uses the software at its
> > + * sole risk and liability. Politecnico di Torino shall have no obligation to
> > + * maintain or support this software. Politecnico di Torino MAKES NO
> > + * EXPRESS OR IMPLIED WARRANTY OF ANY KIND REGARDING THIS SOFTWARE.
> > + * Politecnico di Torino SHALL NOT BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES, WHETHER BASED ON
> > + * CONTRACT, TORT OR ANY OTHER LEGAL THEORY, IN CONNECTION WITH OR
> > + * ARISING OUT OF THE FURNISHING, PERFORMANCE OR USE OF THIS SOFTWARE.
> > + */
> 
> ditto here
> 
> >  #include <linux/module.h>
> >  
> >  #include "ima.h"
> >  static const char *IMA_TEMPLATE_NAME = "ima";
> > +static const char *IMA_LABEL_FALLBACK = "n/a";
> 
> >  /*
> >   * ima_store_template - store ima template measurements
> > @@ -171,7 +195,13 @@ void ima_store_measurement(struct ima_iint_cache *iint, 
> > struct file *file,
> 
> When posting patches, please use a mailer that doesn't wrap lines.
> 
> >  	struct inode *inode = file->f_dentry->d_inode;
> >  	struct ima_template_entry *entry;
> >  	int violation = 0;
> > -
> > +	char *subj_label = NULL;
> > +	char *obj_label = NULL;
> > +	int obj_label_len;
> > +	int subj_label_len;
> > +	struct task_struct *tsk = current;
> > +	u32 osid, sid;
> > +	
> >  	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> >  	if (!entry) {
> >  		integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
> > @@ -181,7 +211,25 @@ void ima_store_measurement(struct ima_iint_cache *iint, 
> > struct file *file,
> >  	memset(&entry->template, 0, sizeof(entry->template));
> >  	memcpy(entry->template.digest, iint->digest, IMA_DIGEST_SIZE);
> >  	strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX);
> > +	
> > +	if(file->f_dentry->d_inode) {
> > +		security_inode_getsecid(file->f_dentry->d_inode, &osid);
> > +		result = security_secid_to_secctx(osid, &obj_label, &obj_label_len);
> > +	} else
> > +		result = -1;
> > +
> > +	if(result)
> > +		strncpy(entry->template.obj_label, IMA_LABEL_FALLBACK, 
> > strlen(IMA_LABEL_FALLBACK));
> 
> Why bother? In the display code, check to to see if it is NULL.
> 
I think the label must be defined here, because the function "ima_store_template", called after these lines, 
calculates at this time the digest of the template.
 
> > +	else
> > +		strncpy(entry->template.obj_label, obj_label, obj_label_len);
> >  
> > +	security_task_getsecid(tsk, &sid);
> > +	result = security_secid_to_secctx(sid, &subj_label, &subj_label_len);
> > +	if(result)
> > +		strncpy(entry->template.subj_label, IMA_LABEL_FALLBACK, 
> > strlen(IMA_LABEL_FALLBACK));
> > +	else
> > +		strncpy(entry->template.subj_label, subj_label, subj_label_len);
> > + 
> >  	result = ima_store_template(entry, violation, inode);
> >  	if (!result)
> >  		iint->flags |= IMA_MEASURED;
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 0c72c9c..465b8b3 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -15,6 +15,30 @@
> >   *	implemenents security file system for reporting
> >   *	current measurement list and IMA statistics
> >   */
> > +/* ====================================================================
> > + * Copyright (C) 2009-2010 Politecnico di Torino, Italy
> > + * TORSEC group -- http://security.polito.it
> > + * Author: Roberto Sassu <rob...@po...>
> > + *
> > + * The original IMA source code from IBM, has been modified by Roberto Sassu 
> > + * to display extra information in the measurement list displayed through the
> > + * securityfs filesystem.
> > + *
> > + * DISCLAIMER of WARRANTY
> > + *
> > + * The following software "IMA" with measurement list format enhanced
> > + * is experimental and is provided "as is", and no guarantee or warranty
> > + * is given by Politecnico di Torino (TORSEC group), that the software is
> > + * fit for any particular purpose.  The user thereof uses the software at its
> > + * sole risk and liability. Politecnico di Torino shall have no obligation to
> > + * maintain or support this software. Politecnico di Torino MAKES NO
> > + * EXPRESS OR IMPLIED WARRANTY OF ANY KIND REGARDING THIS SOFTWARE.
> > + * Politecnico di Torino SHALL NOT BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES, WHETHER BASED ON
> > + * CONTRACT, TORT OR ANY OTHER LEGAL THEORY, IN CONNECTION WITH OR
> > + * ARISING OUT OF THE FURNISHING, PERFORMANCE OR USE OF THIS SOFTWARE.
> > + */
> > +
> 
> ditto here
> 
> >  #include <linux/fcntl.h>
> >  #include <linux/module.h>
> >  #include <linux/seq_file.h>
> > @@ -177,11 +201,16 @@ void ima_template_show(struct seq_file *m, void *e, enum 
> > ima_show_type show)
> >  {
> >  	struct ima_template_data *entry = e;
> >  	int namelen;
> > -
> > +	int obj_label_len;
> > +	int subj_label_len;
> > +	
> >  	switch (show) {
> >  	case IMA_SHOW_ASCII:
> >  		ima_print_digest(m, entry->digest);
> > -		seq_printf(m, " %s\n", entry->file_name);
> > +		seq_printf(m, " %s", entry->file_name);
> > +		seq_printf(m, " %s", entry->subj_label);
> > +		seq_printf(m, " %s\n", entry->obj_label);
> > +		
> >  		break;
> >  	case IMA_SHOW_BINARY:
> >  		ima_putc(m, entry->digest, IMA_DIGEST_SIZE);
> > @@ -189,6 +218,13 @@ void ima_template_show(struct seq_file *m, void *e, enum 
> > ima_show_type show)
> >  		namelen = strlen(entry->file_name);
> >  		ima_putc(m, &namelen, sizeof namelen);
> >  		ima_putc(m, entry->file_name, namelen);
> > +		subj_label_len = strlen(entry->subj_label);
> > +		ima_putc(m, &subj_label_len, sizeof subj_label_len);
> > +		ima_putc(m, entry->subj_label, subj_label_len);
> > +		obj_label_len = strlen(entry->obj_label);
> > +		ima_putc(m, &obj_label_len, sizeof obj_label_len);
> > +		ima_putc(m, entry->obj_label, obj_label_len);
> > +		
> >  	default:
> >  		break;
> >  	}
> > 
> 
> 
> 
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Linux-ima-user mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-user
>