Re: [RFC][PATCH 2/8] Integrity Service API and dummy provider

[Seth Arnold <seth.arnold@suse.de>]

On Thu, Aug 17, 2006 at 12:53:12PM -0700, Kylene Jo Hall wrote:
> --- linux-2.6.18-rc3/security/integrity_dummy.c	1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.18-rc3-working/security/integrity_dummy.c	2006-08-04 15:30:41.000000000 -0500
> @@ -0,0 +1,77 @@
> +/*
> + * integrity_dummy.c
> + *
> + * Instantiate integrity subsystem
> + *
> + * Copyright (C) 2005,2006 IBM Corporation
> + * Author: Mimi Zohar <zohar@us.ibm.com>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/config.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/integrity.h>
> +
> +/*
> + *  Return the extended attribute
> + */
> +static int dummy_verify_metadata(struct dentry *dentry, char *xattr_name,
> +				 char **xattr_value, int *xattr_value_len,
> +				 int *status)
> +{
> +	char *value;
> +	int size;
> +	int error;
> +
> +	if (!xattr_value || !xattr_value_len || !status)
> +		return -EINVAL;
> +
> +	if (!dentry || !dentry->d_inode || !dentry->d_inode->i_op
> +	    || !dentry->d_inode->i_op->getxattr) {
> +		return -EOPNOTSUPP;
> +	}
> +
> +	size = dentry->d_inode->i_op->getxattr(dentry, xattr_name, NULL, 0);
> +	if (size < 0) {
> +		if (size == -ENODATA) {
> +			*status = INTEGRITY_NOLABEL;
> +			return 0;
> +		}
> +		return size;
> +	}
> +
> +	value = kzalloc(size + 1, GFP_KERNEL);
> +	if (!value) 
> +		return -ENOMEM;
> +
> +	error = dentry->d_inode->i_op->getxattr(dentry, xattr_name,
> +						value, size);
> +	*xattr_value_len = size;
> +	*xattr_value = value;
> +	*status = INTEGRITY_PASS;
> +	return error;
> +}

If the second call to ->getxattr returns an error, is it a good idea to
overwrite the values in xattr_value_len and *xattr_value? Does the
integrity really "pass" if there is an error?

(Or is the point of the 'dummy' verification that .. well .. no
verification is done?)

> +static int dummy_verify_data(struct dentry *dentry, int *status)
> +{
> +	if (status)
> +		*status = INTEGRITY_PASS;
> +	return 0;
> +}
> +
> +static void dummy_measure(struct dentry *dentry,
> +			  const unsigned char *filename, int mask)
> +{
> +	return;
> +}
> +
> +struct integrity_operations dummy_integrity_ops = {
> +	.verify_metadata = dummy_verify_metadata,
> +	.verify_data = dummy_verify_data,
> +	.measure = dummy_measure
> +};
> --- linux-2.6.18-rc3/include/linux/integrity.h	1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.18-rc3-working/include/linux/integrity.h	2006-08-04 15:30:41.000000000 -0500
> @@ -0,0 +1,90 @@
> +/*
> + * integrity.h
> + *
> + * Copyright (C) 2005,2006 IBM Corporation
> + * Author: Mimi Zohar <zohar@us.ibm.com>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation, version 2 of the License.
> + */
> +
> +#ifndef _LINUX_INTEGRITY_H
> +#define _LINUX_INTEGRITY_H
> +
> +#include <linux/fs.h>
> +
> +/*
> + * struct integrity_operations - main integrity structure
> + *
> + * @verify_data:
> + *	Verify the integrity of a dentry.
> + *	@dentry contains the dentry structure to be verified.
> + *	@status contains INTEGRITY_PASS, INTEGRITY_FAIL, or
> + * 		INTEGRITY_NOLABEL
> + *	Return 0 on success or errno values
> + *
> + * @verify_metadata:
> + *	Verify the integrity of a dentry's metadata; return the value
> + * 	of the requested xattr_name and the verification result of the
> + *	dentry's metadata.
> + *	@dentry contains the dentry structure of the metadata to be verified.
> + *	@xattr_name, if not null, contains the name of the xattr
> + *		 being requested.
> + *	@xattr_value, if not null, is a pointer for the xattr value.
> + *	@xattr_val_len will be set to the length of the xattr value.
> + *	@status contains INTEGRITY_PASS, INTEGRITY_FAIL, or
> + * 		INTEGRITY_NOLABEL
> + *	Return 0 on success or errno values
> + *
> + * @measure:
> + *	Update an aggregate integrity value with the inode's measurement.
> + *	The aggregate integrity value is maintained in secure storage such
> + *	as in a TPM PCR.
> + *	@dentry contains the dentry structure of the inode to be measured.
> + *	@filename either contains the full pathname/short file name.
> + *	@mask contains the filename permission status(i.e. read, write, append).
> + *
> + */

I wouldn't normally expect a function named 'measure' to update a
datastructure, especially not one potentially stored in hardware. Is
this just my unfamiliarity with TPM nomenclature?
What is the proper use of the filename?

> +#define PASS_STR "INTEGRITY_PASS"
> +#define FAIL_STR "INTEGRITY_FAIL"
> +#define NOLABEL_STR "INTEGRITY_NOLABEL"
> +
> +struct integrity_operations {
> +	int (*verify_metadata) (struct dentry *dentry, char *xattr_name,
> +			char **xattr_value, int *xattr_val_len, int *status);
> +	int (*verify_data) (struct dentry *dentry, int *status);
> +	void (*measure) (struct dentry *dentry,
> +			const unsigned char *filename, int mask);
> +};
> +extern int register_integrity(struct integrity_operations *ops);
> +extern int unregister_integrity(struct integrity_operations *ops);
> +
> +/* global variables */
> +extern struct integrity_operations *integrity_ops;
> +enum integrity_verify_status {
> +	INTEGRITY_PASS = 0, INTEGRITY_FAIL = -1, INTEGRITY_NOLABEL = -2
> +};
> +
> +/* inline stuff */
> +static inline int integrity_verify_metadata(struct dentry *dentry,
> +			char *xattr_name, char **xattr_value,
> +			int *xattr_val_len, int *status)
> +{
> +	return integrity_ops->verify_metadata(dentry, xattr_name,
> +			xattr_value, xattr_val_len, status);
> +}
> +
> +static inline int integrity_verify_data(struct dentry *dentry, 
> +					int *status)
> +{
> +	return integrity_ops->verify_data(dentry, status);
> +}
> +
> +static inline void integrity_measure(struct dentry *dentry,
> +			const unsigned char *filename, int mask)
> +{
> +	return integrity_ops->measure(dentry, filename, mask);
> +}
> +#endif
> --- linux-2.6.18-rc3/security/Makefile	2006-07-30 01:15:36.000000000 -0500
> +++ linux-2.6.18-rc3-working/security/Makefile	2006-08-01 12:21:24.000000000 -0500
> @@ -12,6 +13,7 @@ endif
>  
>  # Object file lists
>  obj-$(CONFIG_SECURITY)			+= security.o dummy.o inode.o
> +obj-$(CONFIG_SECURITY)			+= integrity.o integrity_dummy.o

Not CONFIG_SECURITY_INTEGRITY or similar?

>  # Must precede capability.o in order to stack properly.
>  obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
>  obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o

Thanks

Attachment: pgp2Lo44Ejihq.pgp
Description: PGP signature