All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-v2 0/6] ima: support per-measurement templates
@ 2013-11-19 12:33 Roberto Sassu
  2013-11-19 12:33 ` [PATCH-v2 1/6] ima: connect defined IMA templates through a linked list Roberto Sassu
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 4395 bytes --]

Hi everyone

one functionality that is missing in the new template management mechanism
is the possibility to specify a custom template format per-measurement entry.
Until now, once the template is determined from the kernel configuration
or from the kernel command line parameters 'ima_template' and ima_template_fmt',
this template is used to generate all measurements entries in the list.

However, it is desirable to differentiate information included in measurement
entries depending on the event type from which they are generated. To explain
the usefulness of the proposed modification, I'll make an example.

One feature coming soon is the addition of new template fields that represent
LSM labels applied either to the current process and to the inode being
measured. However, these information are not enough to capture the mapping
between the executable code loaded for a given subject. Indeed, for example
in SELinux, a type transition may occur if the type of the current process
and the type of the inode being executed are matched in a 'type_transition'
policy rule. In this case, the code should be mapped not to the label of the
current process but instead to the label in the credentials (stored in the
'linux_binprm' structure) being installed during the execution of the execve()
system call.

To correctly perform the mapping code - LSM label, it is needed to introduce
a new template field to represent the LSM label in the 'linux_binprm' structure
(e.g. with identifier 'target-subj') and a new IMA policy action
(e.g. measure_log_all) to record a measurement for every event that match
rule criteria, although the accessed inode has already been measured. Then,
assuming that the format of the default template is "d-ng|n-ng|subj|obj"
(digest + hash algo, long event name, subject LSM label, object LSM label),
the policy to capture the mapping should be:

---
measure_log_all func=BPRM_CHECK mask=MAY_EXEC \
    ima_template_fmt=d-ng|n-ng|subj|obj|target-subj
measure_log_all func=FILE_MMAP mask=MAY_EXEC
---

In the first rule, the default template is overridden with a template that
contains the label stored in the 'linux_binprm' structure. Thus, in the
resulting measurements list, all entries that record file execution will
include the additional template field, while those generated from the
mapping into memory of shared libraries will contain only fields listed
in the default template.



=== UPDATE ===

This new version of the patch set includes two important updates in respect
to the previous one. First, newly created templates are added to the created
linked list (whose head is the first element of the 'defined_templates' array)
to avoid duplicates. Second, instead of storing the template name or format
strings in the IMA policy structure (ima_rule_entry), this patch set records
directly the pointer to newly created template descriptors obtained from
the function ima_get_template_desc(). Callers of ima_alloc_init_template()
determine the template to be used to produce a new measurement entry
(the default template or that stored in a matched policy rule) and supply
it as argument to this function. The benefit of the latter change is that
template lookup operations happen only during IMA initialization or the
loading of a custom policy (before, lookup was done each time an event matches
a rule with a custom template to translate the name or format string into
a template descriptor).

Roberto Sassu


Roberto Sassu (6):
  ima: connect defined IMA templates through a linked list
  ima: added new template helper lookup_template_desc_by_fmt()
  ima: added ima_get_template_desc() for templates dynamic registration
  ima: added ima_template and ima_template_fmt new policy options
  ima: pass template descriptor to ima_alloc_init_template()
  ima: use custom template obtained from a matched policy rule

 Documentation/ABI/testing/ima_policy     |  6 ++-
 Documentation/security/IMA-templates.txt | 19 +++++----
 security/integrity/ima/ima.h             | 14 +++++--
 security/integrity/ima/ima_api.c         | 22 ++++++----
 security/integrity/ima/ima_init.c        |  3 +-
 security/integrity/ima/ima_main.c        |  6 ++-
 security/integrity/ima/ima_policy.c      | 37 ++++++++++++++++-
 security/integrity/ima/ima_template.c    | 70 ++++++++++++++++++++++++++++++--
 8 files changed, 147 insertions(+), 30 deletions(-)

-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH-v2 1/6] ima: connect defined IMA templates through a linked list
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
@ 2013-11-19 12:33 ` Roberto Sassu
  2013-11-19 12:33 ` [PATCH-v2 2/6] ima: added new template helper lookup_template_desc_by_fmt() Roberto Sassu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

This patch connects defined templates through a linked list so that it
will be possible to append new descriptors when the functionality
of specifying a custom template in the policy will be introduced.
Template search by name is still performed by iterating over
'defined_templates' array items.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/ima.h          | 1 +
 security/integrity/ima/ima_template.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 9636e17..8b4a4f3 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -68,6 +68,7 @@ struct ima_template_field {
 
 /* IMA template descriptor definition */
 struct ima_template_desc {
+	struct list_head list;
 	char *name;
 	char *fmt;
 	int num_fields;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 5a95d06..33c911a 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -204,6 +204,7 @@ static int init_defined_templates(void)
 	int result = 0;
 
 	/* Init defined templates. */
+	INIT_LIST_HEAD(&defined_templates[0].list);
 	for (i = 0; i < ARRAY_SIZE(defined_templates); i++) {
 		struct ima_template_desc *template = &defined_templates[i];
 
@@ -219,6 +220,10 @@ static int init_defined_templates(void)
 			       template->name : template->fmt), result);
 			return result;
 		}
+
+		if (i > 0)
+			list_add_tail(&defined_templates[i].list,
+				      &defined_templates[0].list);
 	}
 	return result;
 }
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH-v2 2/6] ima: added new template helper lookup_template_desc_by_fmt()
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
  2013-11-19 12:33 ` [PATCH-v2 1/6] ima: connect defined IMA templates through a linked list Roberto Sassu
@ 2013-11-19 12:33 ` Roberto Sassu
  2013-12-09 13:34   ` Mimi Zohar
  2013-11-19 12:33 ` [PATCH-v2 3/6] ima: added ima_get_template_desc() for templates dynamic registration Roberto Sassu
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

This patch adds a new helper to search a template descriptor by its format.
Also, the old function lookup_template_desc(), which performs the search
by name, has been renamed to lookup_template_desc_by_name().

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/ima_template.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 33c911a..c849723 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -38,7 +38,7 @@ static struct ima_template_field supported_fields[] = {
 };
 
 static struct ima_template_desc *ima_template;
-static struct ima_template_desc *lookup_template_desc(const char *name);
+static struct ima_template_desc *lookup_template_desc_by_name(const char *name);
 static struct ima_template_field *lookup_template_field(const char *field_id);
 
 static int __init ima_template_setup(char *str)
@@ -53,7 +53,7 @@ static int __init ima_template_setup(char *str)
 	 * Verify that a template with the supplied name exists.
 	 * If not, use CONFIG_IMA_DEFAULT_TEMPLATE.
 	 */
-	template_desc = lookup_template_desc(str);
+	template_desc = lookup_template_desc_by_name(str);
 	if (!template_desc) {
 		pr_err("IMA: template %s not found, using %s\n",
 		       str, CONFIG_IMA_DEFAULT_TEMPLATE);
@@ -117,7 +117,7 @@ static int __init ima_template_fmt_setup(char *str)
 }
 __setup("ima_template_fmt=", ima_template_fmt_setup);
 
-static struct ima_template_desc *lookup_template_desc(const char *name)
+static struct ima_template_desc *lookup_template_desc_by_name(const char *name)
 {
 	int i;
 
@@ -129,6 +129,18 @@ static struct ima_template_desc *lookup_template_desc(const char *name)
 	return NULL;
 }
 
+static struct ima_template_desc *lookup_template_desc_by_fmt(const char *fmt)
+{
+	struct ima_template_desc *desc;
+
+	list_for_each_entry(desc, &defined_templates[0].list, list) {
+		if (strcmp(desc->fmt, fmt) == 0)
+			return desc;
+	}
+
+	return NULL;
+}
+
 static struct ima_template_field *lookup_template_field(const char *field_id)
 {
 	int i;
@@ -232,7 +244,7 @@ struct ima_template_desc *ima_template_desc_current(void)
 {
 	if (!ima_template)
 		ima_template =
-		    lookup_template_desc(CONFIG_IMA_DEFAULT_TEMPLATE);
+		    lookup_template_desc_by_name(CONFIG_IMA_DEFAULT_TEMPLATE);
 	return ima_template;
 }
 
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH-v2 3/6] ima: added ima_get_template_desc() for templates dynamic registration
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
  2013-11-19 12:33 ` [PATCH-v2 1/6] ima: connect defined IMA templates through a linked list Roberto Sassu
  2013-11-19 12:33 ` [PATCH-v2 2/6] ima: added new template helper lookup_template_desc_by_fmt() Roberto Sassu
@ 2013-11-19 12:33 ` Roberto Sassu
  2013-12-09 13:35   ` Mimi Zohar
  2013-11-19 12:33 ` [PATCH-v2 4/6] ima: added ima_template and ima_template_fmt new policy options Roberto Sassu
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]

This patch introduces the ima_get_template_desc() function which returns
a template descriptor depending on the template name and format passed
as arguments (at least one argument should be not NULL). If the first
argument is not NULL, the new function searches an existing template
descriptor by name among those defined and returns it to the caller.
Instead, if the second argument is not NULL and the first is NULL,
it does a template lookup by format and, if not found, creates a new one
before returning the pointer to the caller. Newly created templates
are cached to avoid duplicates.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/ima.h          |  2 ++
 security/integrity/ima/ima_template.c | 45 +++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 8b4a4f3..632d92e 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -109,6 +109,8 @@ int ima_init_crypto(void);
 void ima_putc(struct seq_file *m, void *data, int datalen);
 void ima_print_digest(struct seq_file *m, u8 *digest, int size);
 struct ima_template_desc *ima_template_desc_current(void);
+struct ima_template_desc *ima_get_template_desc(char *template_name,
+						char *template_fmt);
 int ima_init_template(void);
 
 int ima_init_template(void);
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index c849723..9bec7d4 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -41,6 +41,8 @@ static struct ima_template_desc *ima_template;
 static struct ima_template_desc *lookup_template_desc_by_name(const char *name);
 static struct ima_template_field *lookup_template_field(const char *field_id);
 
+static DEFINE_MUTEX(ima_templates_mutex);
+
 static int __init ima_template_setup(char *str)
 {
 	struct ima_template_desc *template_desc;
@@ -248,6 +250,49 @@ struct ima_template_desc *ima_template_desc_current(void)
 	return ima_template;
 }
 
+struct ima_template_desc *ima_get_template_desc(char *template_name,
+						char *template_fmt)
+{
+	struct ima_template_desc *desc;
+	int result;
+
+	if (template_name == NULL && template_fmt == NULL)
+		return NULL;
+
+	if (template_name)
+		desc = lookup_template_desc_by_name(template_name);
+	else {
+		mutex_lock(&ima_templates_mutex);
+		desc = lookup_template_desc_by_fmt(template_fmt);
+		if (desc == NULL) {
+			desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+			if (desc == NULL)
+				goto out_unlock;
+		}
+		desc->name = "";
+		desc->fmt = kstrdup(template_fmt, GFP_KERNEL);
+		if (desc->fmt == NULL)
+			goto out_free;
+
+		result = template_desc_init_fields(desc->fmt, &(desc->fields),
+						   &(desc->num_fields));
+		if (result < 0)
+			goto out_free_fmt;
+
+		list_add_tail(&desc->list, &defined_templates[0].list);
+		mutex_unlock(&ima_templates_mutex);
+	}
+
+	return desc;
+out_free_fmt:
+	kfree(desc->fmt);
+out_free:
+	kfree(desc);
+out_unlock:
+	mutex_unlock(&ima_templates_mutex);
+	return NULL;
+}
+
 int ima_init_template(void)
 {
 	int result;
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH-v2 4/6] ima: added ima_template and ima_template_fmt new policy options
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
                   ` (2 preceding siblings ...)
  2013-11-19 12:33 ` [PATCH-v2 3/6] ima: added ima_get_template_desc() for templates dynamic registration Roberto Sassu
@ 2013-11-19 12:33 ` Roberto Sassu
  2013-12-09 13:52   ` Mimi Zohar
  2013-11-19 12:33 ` [PATCH-v2 5/6] ima: pass template descriptor to ima_alloc_init_template() Roberto Sassu
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 6358 bytes --]

This patch adds the support for 'ima_template' and 'ima_template_fmt'
policy options. They allow to define which template should be used
and, thus, which information should be included in measurements entries
generated from events that match other rules' criteria.

With this feature, it is possible to include for each measurement entry
only relevant information. For example, while measurements that report
the execution of the execve() system call may contain the credentials
being installed on the current process (stored in the 'cred' field of the
'linux_binprm' structure), others should not include it (also because
the pointer to the above structure is not available from other IMA hooks).

A sample policy to add to measurement entries the LSM label in the
'linux_binprm' structure only for file execution events should be:

---
measure func=BPRM_CHECK mask=MAY_EXEC \
    ima_template_fmt=d-ng|n-ng|target-subj
measure func=FILE_MMAP mask=MAY_EXEC
---

where 'target-subj' is the identifier of a new field (whose code is not yet
upstreamed) which displays the additional information.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 Documentation/ABI/testing/ima_policy     |  6 +++++-
 Documentation/security/IMA-templates.txt | 19 +++++++++++--------
 security/integrity/ima/ima_policy.c      | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index f1c5cc9..7fbe47d 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -23,7 +23,7 @@ Description:
 				 [fowner]]
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
-			option:	[[appraise_type=]]
+			option:	[[appraise_type=] [ima_template=] [ima_template_fmt=]]
 
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
 			mask:= [MAY_READ] [MAY_WRITE] [MAY_APPEND] [MAY_EXEC]
@@ -33,6 +33,10 @@ Description:
 			fowner:=decimal value
 		lsm:  	are LSM specific
 		option:	appraise_type:= [imasig]
+			ima_template:= an already defined template
+			ima_template_fmt:= a custom template format
+					   (see Documentation/security/IMA-templates.txt
+					   for more details)
 
 		default policy:
 			# PROC_SUPER_MAGIC
diff --git a/Documentation/security/IMA-templates.txt b/Documentation/security/IMA-templates.txt
index 08ea2da..61d9f0d 100644
--- a/Documentation/security/IMA-templates.txt
+++ b/Documentation/security/IMA-templates.txt
@@ -36,13 +36,14 @@ from the set of the supported ones.
 After the initialization step, IMA will call ima_alloc_init_template()
 (new function defined within the patches for the new template management
 mechanism) to generate a new measurement entry by using the template
-descriptor chosen through the kernel configuration or through the newly
-introduced 'ima_template' and 'ima_template_fmt' kernel command line parameters.
-It is during this phase that the advantages of the new architecture are
-clearly shown: the latter function will not contain specific code to handle
-a given template but, instead, it simply calls the init() method of the template
-fields associated to the chosen template descriptor and store the result
-(pointer to allocated data and data length) in the measurement entry structure.
+descriptor chosen through the kernel configuration, the newly introduced
+'ima_template' and 'ima_template_fmt' kernel command line parameters and
+new policy options with the same names. It is during this phase that the
+advantages of the new architecture are clearly shown: the latter function
+will not contain specific code to handle a given template but, instead, it
+simply calls the init() method of the template fields associated to the
+chosen template descriptor and store the result (pointer to allocated data
+and data length) in the measurement entry structure.
 
 The same mechanism is employed to display measurements entries.
 The functions ima[_ascii]_measurements_show() retrieve, for each entry,
@@ -83,4 +84,6 @@ currently the following methods are supported:
  - specify a template descriptor name from the kernel command line through
    the 'ima_template=' parameter;
  - register a new template descriptor with custom format through the kernel
-   command line parameter 'ima_template_fmt='.
+   command line parameter 'ima_template_fmt=';
+ - provide desired template name or custom format for specific events through
+   the new policy options 'ima_template=' and 'ima_template_fmt='.
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a9c3d3c..f4b3fd0 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -50,6 +50,7 @@ struct ima_rule_entry {
 	u8 fsuuid[16];
 	kuid_t uid;
 	kuid_t fowner;
+	struct ima_template_desc *desc;
 	struct {
 		void *rule;	/* LSM file metadata specific */
 		void *args_p;	/* audit value */
@@ -351,7 +352,8 @@ enum {
 	Opt_obj_user, Opt_obj_role, Opt_obj_type,
 	Opt_subj_user, Opt_subj_role, Opt_subj_type,
 	Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
-	Opt_appraise_type, Opt_fsuuid
+	Opt_appraise_type, Opt_fsuuid,
+	Opt_ima_template, Opt_ima_template_fmt
 };
 
 static match_table_t policy_tokens = {
@@ -373,6 +375,8 @@ static match_table_t policy_tokens = {
 	{Opt_uid, "uid=%s"},
 	{Opt_fowner, "fowner=%s"},
 	{Opt_appraise_type, "appraise_type=%s"},
+	{Opt_ima_template, "ima_template=%s"},
+	{Opt_ima_template_fmt, "ima_template_fmt=%s"},
 	{Opt_err, NULL}
 };
 
@@ -621,6 +625,32 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			else
 				result = -EINVAL;
 			break;
+		case Opt_ima_template:
+			ima_log_string(ab, "ima_template", args[0].from);
+
+			if (entry->desc) {
+				result = -EINVAL;
+				break;
+			}
+
+			entry->desc = ima_get_template_desc(args[0].from, NULL);
+			if (entry->desc == NULL)
+				result = -EINVAL;
+
+			break;
+		case Opt_ima_template_fmt:
+			ima_log_string(ab, "ima_template_fmt", args[0].from);
+
+			if (entry->desc) {
+				result = -EINVAL;
+				break;
+			}
+
+			entry->desc = ima_get_template_desc(NULL, args[0].from);
+			if (entry->desc == NULL)
+				result = -EINVAL;
+
+			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
 			result = -EINVAL;
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH-v2 5/6] ima: pass template descriptor to ima_alloc_init_template()
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
                   ` (3 preceding siblings ...)
  2013-11-19 12:33 ` [PATCH-v2 4/6] ima: added ima_template and ima_template_fmt new policy options Roberto Sassu
@ 2013-11-19 12:33 ` Roberto Sassu
  2013-11-19 12:33 ` [PATCH-v2 6/6] ima: use custom template obtained from a matched policy rule Roberto Sassu
  2013-11-21 14:11 ` [Linux-ima-devel] [PATCH-v2 0/6] ima: support per-measurement templates Mimi Zohar
  6 siblings, 0 replies; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 5168 bytes --]

This patch adds the template descriptor as a new argument to the function
ima_alloc_init_template() so that callers can specify the format of the
new measurement entry being generated.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/ima.h      |  6 ++++--
 security/integrity/ima/ima_api.c  | 15 ++++++++++-----
 security/integrity/ima/ima_init.c |  3 ++-
 security/integrity/ima/ima_main.c |  2 +-
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 632d92e..fc2fbf3 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -142,13 +142,15 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
-			   int xattr_len);
+			   int xattr_len, struct ima_template_desc *desc);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
 int ima_alloc_init_template(struct integrity_iint_cache *iint,
 			    struct file *file, const unsigned char *filename,
 			    struct evm_ima_xattr_data *xattr_value,
-			    int xattr_len, struct ima_template_entry **entry);
+			    int xattr_len,
+			    struct ima_template_desc *template_desc,
+			    struct ima_template_entry **entry);
 int ima_store_template(struct ima_template_entry *entry, int violation,
 		       struct inode *inode, const unsigned char *filename);
 const char *ima_d_path(struct path *path, char **pathbuf);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 8037484..444ec53 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -27,11 +27,15 @@
 int ima_alloc_init_template(struct integrity_iint_cache *iint,
 			    struct file *file, const unsigned char *filename,
 			    struct evm_ima_xattr_data *xattr_value,
-			    int xattr_len, struct ima_template_entry **entry)
+			    int xattr_len,
+			    struct ima_template_desc *template_desc,
+			    struct ima_template_entry **entry)
 {
-	struct ima_template_desc *template_desc = ima_template_desc_current();
 	int i, result = 0;
 
+	if (template_desc == NULL)
+		return -EINVAL;
+
 	*entry = kzalloc(sizeof(**entry) + template_desc->num_fields *
 			 sizeof(struct ima_field_data), GFP_NOFS);
 	if (!*entry)
@@ -120,6 +124,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 {
 	struct ima_template_entry *entry;
 	struct inode *inode = file->f_dentry->d_inode;
+	struct ima_template_desc *desc = ima_template_desc_current();
 	int violation = 1;
 	int result;
 
@@ -127,7 +132,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 	atomic_long_inc(&ima_htable.violations);
 
 	result = ima_alloc_init_template(NULL, file, filename,
-					 NULL, 0, &entry);
+					 NULL, 0, desc, &entry);
 	if (result < 0) {
 		result = -ENOMEM;
 		goto err_out;
@@ -245,7 +250,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 void ima_store_measurement(struct integrity_iint_cache *iint,
 			   struct file *file, const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
-			   int xattr_len)
+			   int xattr_len, struct ima_template_desc *desc)
 {
 	const char *op = "add_template_measure";
 	const char *audit_cause = "ENOMEM";
@@ -258,7 +263,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 		return;
 
 	result = ima_alloc_init_template(iint, file, filename,
-					 xattr_value, xattr_len, &entry);
+					 xattr_value, xattr_len, desc, &entry);
 	if (result < 0) {
 		integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
 				    op, audit_cause, result, 0);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 15f34bd..3a4df15 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -44,6 +44,7 @@ static void __init ima_add_boot_aggregate(void)
 {
 	struct ima_template_entry *entry;
 	struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
+	struct ima_template_desc *desc = ima_template_desc_current();
 	const char *op = "add_boot_aggregate";
 	const char *audit_cause = "ENOMEM";
 	int result = -ENOMEM;
@@ -69,7 +70,7 @@ static void __init ima_add_boot_aggregate(void)
 	}
 
 	result = ima_alloc_init_template(iint, NULL, boot_aggregate_name,
-					 NULL, 0, &entry);
+					 NULL, 0, desc, &entry);
 	if (result < 0)
 		return;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 149ee11..e08ce72 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -226,7 +226,7 @@ static int process_measurement(struct file *file, const char *filename,
 
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
-				      xattr_value, xattr_len);
+				      xattr_value, xattr_len, template_desc);
 	if (action & IMA_APPRAISE_SUBMASK)
 		rc = ima_appraise_measurement(_func, iint, file, pathname,
 					      xattr_value, xattr_len);
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH-v2 6/6] ima: use custom template obtained from a matched policy rule
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
                   ` (4 preceding siblings ...)
  2013-11-19 12:33 ` [PATCH-v2 5/6] ima: pass template descriptor to ima_alloc_init_template() Roberto Sassu
@ 2013-11-19 12:33 ` Roberto Sassu
  2013-11-21 14:11 ` [Linux-ima-devel] [PATCH-v2 0/6] ima: support per-measurement templates Mimi Zohar
  6 siblings, 0 replies; 11+ messages in thread
From: Roberto Sassu @ 2013-11-19 12:33 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, linux-ima-devel, zohar, d.kasatkin, james.l.morris,
	Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 4452 bytes --]

This patch modifies existing IMA functions to retrieve the template
descriptor from a matched policy rule and provide it to
ima_alloc_init_template().

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/ima.h        | 5 +++--
 security/integrity/ima/ima_api.c    | 7 ++++---
 security/integrity/ima/ima_main.c   | 4 +++-
 security/integrity/ima/ima_policy.c | 5 ++++-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fc2fbf3..9df015c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -133,7 +133,8 @@ static inline unsigned long ima_hash_key(u8 *digest)
 }
 
 /* LIM API function definitions */
-int ima_get_action(struct inode *inode, int mask, int function);
+int ima_get_action(struct inode *inode, int mask, int function,
+		   struct ima_template_desc **desc);
 int ima_must_measure(struct inode *inode, int mask, int function);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
 			    struct file *file,
@@ -165,7 +166,7 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
 enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, POST_SETATTR };
 
 int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags);
+		     int flags, struct ima_template_desc **desc);
 void ima_init_policy(void);
 void ima_update_policy(void);
 ssize_t ima_parse_add_rule(char *);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 444ec53..ebfd6cf 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -161,19 +161,20 @@ err_out:
  * Returns IMA_MEASURE, IMA_APPRAISE mask.
  *
  */
-int ima_get_action(struct inode *inode, int mask, int function)
+int ima_get_action(struct inode *inode, int mask, int function,
+		   struct ima_template_desc **desc)
 {
 	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE;
 
 	if (!ima_appraise)
 		flags &= ~IMA_APPRAISE;
 
-	return ima_match_policy(inode, function, mask, flags);
+	return ima_match_policy(inode, function, mask, flags, desc);
 }
 
 int ima_must_measure(struct inode *inode, int mask, int function)
 {
-	return ima_match_policy(inode, function, mask, IMA_MEASURE);
+	return ima_match_policy(inode, function, mask, IMA_MEASURE, NULL);
 }
 
 /*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index e08ce72..44a1af1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -167,6 +167,7 @@ static int process_measurement(struct file *file, const char *filename,
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint;
 	struct ima_template_desc *template_desc = ima_template_desc_current();
+	struct ima_template_desc *tmp_desc;
 	char *pathbuf = NULL;
 	const char *pathname = NULL;
 	int rc = -ENOMEM, action, must_appraise, _func;
@@ -180,7 +181,7 @@ static int process_measurement(struct file *file, const char *filename,
 	 * bitmask based on the appraise/audit/measurement policy.
 	 * Included is the appraise submask.
 	 */
-	action = ima_get_action(inode, mask, function);
+	action = ima_get_action(inode, mask, function, &tmp_desc);
 	if (!action)
 		return 0;
 
@@ -210,6 +211,7 @@ static int process_measurement(struct file *file, const char *filename,
 		goto out_digsig;
 	}
 
+	template_desc = (tmp_desc != NULL) ? tmp_desc : template_desc;
 	if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) {
 		if (action & IMA_APPRAISE_SUBMASK)
 			xattr_ptr = &xattr_value;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f4b3fd0..edc38ac 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -260,7 +260,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
  * change.)
  */
 int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags)
+		     int flags, struct ima_template_desc **desc)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -273,6 +273,9 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
 		if (!ima_match_rules(entry, inode, func, mask))
 			continue;
 
+		if (desc)
+			*desc = entry->desc;
+
 		action |= entry->flags & IMA_ACTION_FLAGS;
 
 		action |= entry->action & IMA_DO_MASK;
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Linux-ima-devel] [PATCH-v2 0/6] ima: support per-measurement templates
  2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
                   ` (5 preceding siblings ...)
  2013-11-19 12:33 ` [PATCH-v2 6/6] ima: use custom template obtained from a matched policy rule Roberto Sassu
@ 2013-11-21 14:11 ` Mimi Zohar
  6 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2013-11-21 14:11 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-security-module, zohar, linux-kernel, james.l.morris,
	linux-ima-devel

On Tue, 2013-11-19 at 13:33 +0100, Roberto Sassu wrote:
> Hi everyone
> 
> one functionality that is missing in the new template management mechanism
> is the possibility to specify a custom template format per-measurement entry.
> Until now, once the template is determined from the kernel configuration
> or from the kernel command line parameters 'ima_template' and ima_template_fmt',
> this template is used to generate all measurements entries in the list.
> 
> However, it is desirable to differentiate information included in measurement
> entries depending on the event type from which they are generated. To explain
> the usefulness of the proposed modification, I'll make an example.

Adding support for multiple templates in the measurement list at the
same time is a good idea.  Extending the IMA policy to define a per rule
template definition is also a good idea.  However, the example to
illustrate these ideas is unnecessarily complex.  I think the complexity
is due to using fields, that haven't yet been defined, that are
themselves complex.  Please try and find a simpler example.

(Still reviewing the code.)

thanks,

Mimi

> One feature coming soon is the addition of new template fields that represent
> LSM labels applied either to the current process and to the inode being
> measured. However, these information are not enough to capture the mapping
> between the executable code loaded for a given subject. Indeed, for example
> in SELinux, a type transition may occur if the type of the current process
> and the type of the inode being executed are matched in a 'type_transition'
> policy rule. In this case, the code should be mapped not to the label of the
> current process but instead to the label in the credentials (stored in the
> 'linux_binprm' structure) being installed during the execution of the execve()
> system call.
> 
> To correctly perform the mapping code - LSM label, it is needed to introduce
> a new template field to represent the LSM label in the 'linux_binprm' structure
> (e.g. with identifier 'target-subj') and a new IMA policy action
> (e.g. measure_log_all) to record a measurement for every event that match
> rule criteria, although the accessed inode has already been measured. Then,
> assuming that the format of the default template is "d-ng|n-ng|subj|obj"
> (digest + hash algo, long event name, subject LSM label, object LSM label),
> the policy to capture the mapping should be:
> 
> ---
> measure_log_all func=BPRM_CHECK mask=MAY_EXEC \
>     ima_template_fmt=d-ng|n-ng|subj|obj|target-subj
> measure_log_all func=FILE_MMAP mask=MAY_EXEC
> ---
> 
> In the first rule, the default template is overridden with a template that
> contains the label stored in the 'linux_binprm' structure. Thus, in the
> resulting measurements list, all entries that record file execution will
> include the additional template field, while those generated from the
> mapping into memory of shared libraries will contain only fields listed
> in the default template.
> 
> 
> 
> === UPDATE ===
> 
> This new version of the patch set includes two important updates in respect
> to the previous one. First, newly created templates are added to the created
> linked list (whose head is the first element of the 'defined_templates' array)
> to avoid duplicates. Second, instead of storing the template name or format
> strings in the IMA policy structure (ima_rule_entry), this patch set records
> directly the pointer to newly created template descriptors obtained from
> the function ima_get_template_desc(). Callers of ima_alloc_init_template()
> determine the template to be used to produce a new measurement entry
> (the default template or that stored in a matched policy rule) and supply
> it as argument to this function. The benefit of the latter change is that
> template lookup operations happen only during IMA initialization or the
> loading of a custom policy (before, lookup was done each time an event matches
> a rule with a custom template to translate the name or format string into
> a template descriptor).
> 
> Roberto Sassu
> 
> 
> Roberto Sassu (6):
>   ima: connect defined IMA templates through a linked list
>   ima: added new template helper lookup_template_desc_by_fmt()
>   ima: added ima_get_template_desc() for templates dynamic registration
>   ima: added ima_template and ima_template_fmt new policy options
>   ima: pass template descriptor to ima_alloc_init_template()
>   ima: use custom template obtained from a matched policy rule
> 
>  Documentation/ABI/testing/ima_policy     |  6 ++-
>  Documentation/security/IMA-templates.txt | 19 +++++----
>  security/integrity/ima/ima.h             | 14 +++++--
>  security/integrity/ima/ima_api.c         | 22 ++++++----
>  security/integrity/ima/ima_init.c        |  3 +-
>  security/integrity/ima/ima_main.c        |  6 ++-
>  security/integrity/ima/ima_policy.c      | 37 ++++++++++++++++-
>  security/integrity/ima/ima_template.c    | 70 ++++++++++++++++++++++++++++++--
>  8 files changed, 147 insertions(+), 30 deletions(-)
> 
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing 
> conversations that shape the rapidly evolving mobile landscape. Sign up now. 
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-v2 2/6] ima: added new template helper lookup_template_desc_by_fmt()
  2013-11-19 12:33 ` [PATCH-v2 2/6] ima: added new template helper lookup_template_desc_by_fmt() Roberto Sassu
@ 2013-12-09 13:34   ` Mimi Zohar
  0 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2013-12-09 13:34 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-security-module, linux-kernel, linux-ima-devel, zohar,
	d.kasatkin, james.l.morris

On Tue, 2013-11-19 at 13:33 +0100, Roberto Sassu wrote: 
> This patch adds a new helper to search a template descriptor by its format.

The initial template and larger hash digest patch set was quite large.
In order not to break backward compatibility and move forward, functions
were added before they were actually used.  For future reference, we
normally define new functions and use them in the same patch.

thanks,

Mimi

> Also, the old function lookup_template_desc(), which performs the search
> by name, has been renamed to lookup_template_desc_by_name().
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> ---
>  security/integrity/ima/ima_template.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 33c911a..c849723 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -38,7 +38,7 @@ static struct ima_template_field supported_fields[] = {
>  };
> 
>  static struct ima_template_desc *ima_template;
> -static struct ima_template_desc *lookup_template_desc(const char *name);
> +static struct ima_template_desc *lookup_template_desc_by_name(const char *name);
>  static struct ima_template_field *lookup_template_field(const char *field_id);
> 
>  static int __init ima_template_setup(char *str)
> @@ -53,7 +53,7 @@ static int __init ima_template_setup(char *str)
>  	 * Verify that a template with the supplied name exists.
>  	 * If not, use CONFIG_IMA_DEFAULT_TEMPLATE.
>  	 */
> -	template_desc = lookup_template_desc(str);
> +	template_desc = lookup_template_desc_by_name(str);
>  	if (!template_desc) {
>  		pr_err("IMA: template %s not found, using %s\n",
>  		       str, CONFIG_IMA_DEFAULT_TEMPLATE);
> @@ -117,7 +117,7 @@ static int __init ima_template_fmt_setup(char *str)
>  }
>  __setup("ima_template_fmt=", ima_template_fmt_setup);
> 
> -static struct ima_template_desc *lookup_template_desc(const char *name)
> +static struct ima_template_desc *lookup_template_desc_by_name(const char *name)
>  {
>  	int i;
> 
> @@ -129,6 +129,18 @@ static struct ima_template_desc *lookup_template_desc(const char *name)
>  	return NULL;
>  }
> 
> +static struct ima_template_desc *lookup_template_desc_by_fmt(const char *fmt)
> +{
> +	struct ima_template_desc *desc;
> +
> +	list_for_each_entry(desc, &defined_templates[0].list, list) {
> +		if (strcmp(desc->fmt, fmt) == 0)
> +			return desc;
> +	}
> +
> +	return NULL;
> +}
> +
>  static struct ima_template_field *lookup_template_field(const char *field_id)
>  {
>  	int i;
> @@ -232,7 +244,7 @@ struct ima_template_desc *ima_template_desc_current(void)
>  {
>  	if (!ima_template)
>  		ima_template =
> -		    lookup_template_desc(CONFIG_IMA_DEFAULT_TEMPLATE);
> +		    lookup_template_desc_by_name(CONFIG_IMA_DEFAULT_TEMPLATE);
>  	return ima_template;
>  }
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-v2 3/6] ima: added ima_get_template_desc() for templates dynamic registration
  2013-11-19 12:33 ` [PATCH-v2 3/6] ima: added ima_get_template_desc() for templates dynamic registration Roberto Sassu
@ 2013-12-09 13:35   ` Mimi Zohar
  0 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2013-12-09 13:35 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-security-module, linux-kernel, linux-ima-devel, zohar,
	d.kasatkin, james.l.morris

On Tue, 2013-11-19 at 13:33 +0100, Roberto Sassu wrote: 
> This patch introduces the ima_get_template_desc() function which returns
> a template descriptor depending on the template name and format passed
> as arguments (at least one argument should be not NULL). If the first
> argument is not NULL, the new function searches an existing template
> descriptor by name among those defined and returns it to the caller.
> Instead, if the second argument is not NULL and the first is NULL,
> it does a template lookup by format and, if not found, creates a new one
> before returning the pointer to the caller. Newly created templates
> are cached to avoid duplicates.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> ---
>  security/integrity/ima/ima.h          |  2 ++
>  security/integrity/ima/ima_template.c | 45 +++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 8b4a4f3..632d92e 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -109,6 +109,8 @@ int ima_init_crypto(void);
>  void ima_putc(struct seq_file *m, void *data, int datalen);
>  void ima_print_digest(struct seq_file *m, u8 *digest, int size);
>  struct ima_template_desc *ima_template_desc_current(void);
> +struct ima_template_desc *ima_get_template_desc(char *template_name,
> +						char *template_fmt);
>  int ima_init_template(void);
> 
>  int ima_init_template(void);
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index c849723..9bec7d4 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -41,6 +41,8 @@ static struct ima_template_desc *ima_template;
>  static struct ima_template_desc *lookup_template_desc_by_name(const char *name);
>  static struct ima_template_field *lookup_template_field(const char *field_id);
> 
> +static DEFINE_MUTEX(ima_templates_mutex);
> +
>  static int __init ima_template_setup(char *str)
>  {
>  	struct ima_template_desc *template_desc;
> @@ -248,6 +250,49 @@ struct ima_template_desc *ima_template_desc_current(void)
>  	return ima_template;
>  }
> 
> +struct ima_template_desc *ima_get_template_desc(char *template_name,
> +						char *template_fmt)
> +{
> +	struct ima_template_desc *desc;
> +	int result;
> +
> +	if (template_name == NULL && template_fmt == NULL)
> +		return NULL;

Originally, before using a variable, we checked that it wasn't NULL.
This changed, at some point. (Can someone supply a reference, please?)
As the only code calling ima_get_template_desc() will be IMA, this check
shouldn't be required.

> +
> +	if (template_name)
> +		desc = lookup_template_desc_by_name(template_name);
> +	else {
> +		mutex_lock(&ima_templates_mutex);
> +		desc = lookup_template_desc_by_fmt(template_fmt);
> +		if (desc == NULL) {
> +			desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> +			if (desc == NULL)
> +				goto out_unlock;
> +		}
> +		desc->name = "";

What's this?

> +		desc->fmt = kstrdup(template_fmt, GFP_KERNEL);
> +		if (desc->fmt == NULL)
> +			goto out_free;
> +
> +		result = template_desc_init_fields(desc->fmt, &(desc->fields),
> +						   &(desc->num_fields));
> +		if (result < 0)
> +			goto out_free_fmt;
> +

This patch set was posted before the new cleanup functions were defined.
Reminder, to use your new cleanup function.

> +		list_add_tail(&desc->list, &defined_templates[0].list);
> +		mutex_unlock(&ima_templates_mutex);

Right, as new template formats can only be added, not deleted, locking
just here, to prevent duplicates, should be fine.  Perhaps add a short
comment, indicating this is safe, because templates are not ever
removed.

thanks,

Mimi

> +	}
> +
> +	return desc;
> +out_free_fmt:
> +	kfree(desc->fmt);
> +out_free:
> +	kfree(desc);
> +out_unlock:
> +	mutex_unlock(&ima_templates_mutex);
> +	return NULL;
> +}
> +
>  int ima_init_template(void)
>  {
>  	int result;



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-v2 4/6] ima: added ima_template and ima_template_fmt new policy options
  2013-11-19 12:33 ` [PATCH-v2 4/6] ima: added ima_template and ima_template_fmt new policy options Roberto Sassu
@ 2013-12-09 13:52   ` Mimi Zohar
  0 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2013-12-09 13:52 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-security-module, linux-kernel, linux-ima-devel, zohar,
	d.kasatkin, james.l.morris

On Tue, 2013-11-19 at 13:33 +0100, Roberto Sassu wrote: 
> This patch adds the support for 'ima_template' and 'ima_template_fmt'
> policy options. They allow to define which template should be used
> and, thus, which information should be included in measurements entries
> generated from events that match other rules' criteria.
> 
> With this feature, it is possible to include for each measurement entry
> only relevant information. For example, while measurements that report
> the execution of the execve() system call may contain the credentials
> being installed on the current process (stored in the 'cred' field of the
> 'linux_binprm' structure), others should not include it (also because
> the pointer to the above structure is not available from other IMA hooks).
> 
> A sample policy to add to measurement entries the LSM label in the
> 'linux_binprm' structure only for file execution events should be:

As mentioned for the cover letter, please simplify the example.

> ---
> measure func=BPRM_CHECK mask=MAY_EXEC \
>     ima_template_fmt=d-ng|n-ng|target-subj
> measure func=FILE_MMAP mask=MAY_EXEC
> ---
> 
> where 'target-subj' is the identifier of a new field (whose code is not yet
> upstreamed) which displays the additional information.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> ---
>  Documentation/ABI/testing/ima_policy     |  6 +++++-
>  Documentation/security/IMA-templates.txt | 19 +++++++++++--------
>  security/integrity/ima/ima_policy.c      | 32 +++++++++++++++++++++++++++++++-
>  3 files changed, 47 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index f1c5cc9..7fbe47d 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -23,7 +23,7 @@ Description:
>  				 [fowner]]
>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
> -			option:	[[appraise_type=]]
> +			option:	[[appraise_type=] [ima_template=] [ima_template_fmt=]]
> 
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
>  			mask:= [MAY_READ] [MAY_WRITE] [MAY_APPEND] [MAY_EXEC]
> @@ -33,6 +33,10 @@ Description:
>  			fowner:=decimal value
>  		lsm:  	are LSM specific
>  		option:	appraise_type:= [imasig]
> +			ima_template:= an already defined template
> +			ima_template_fmt:= a custom template format
> +					   (see Documentation/security/IMA-templates.txt
> +					   for more details)
> 
>  		default policy:
>  			# PROC_SUPER_MAGIC
> diff --git a/Documentation/security/IMA-templates.txt b/Documentation/security/IMA-templates.txt
> index 08ea2da..61d9f0d 100644
> --- a/Documentation/security/IMA-templates.txt
> +++ b/Documentation/security/IMA-templates.txt
> @@ -36,13 +36,14 @@ from the set of the supported ones.
>  After the initialization step, IMA will call ima_alloc_init_template()
>  (new function defined within the patches for the new template management
>  mechanism) to generate a new measurement entry by using the template
> -descriptor chosen through the kernel configuration or through the newly
> -introduced 'ima_template' and 'ima_template_fmt' kernel command line parameters.
> -It is during this phase that the advantages of the new architecture are
> -clearly shown: the latter function will not contain specific code to handle
> -a given template but, instead, it simply calls the init() method of the template
> -fields associated to the chosen template descriptor and store the result
> -(pointer to allocated data and data length) in the measurement entry structure.
> +descriptor chosen through the kernel configuration, the newly introduced
> +'ima_template' and 'ima_template_fmt' kernel command line parameters and
> +new policy options with the same names. It is during this phase that the
> +advantages of the new architecture are clearly shown: the latter function
> +will not contain specific code to handle a given template but, instead, it
> +simply calls the init() method of the template fields associated to the
> +chosen template descriptor and store the result (pointer to allocated data
> +and data length) in the measurement entry structure.
> 
>  The same mechanism is employed to display measurements entries.
>  The functions ima[_ascii]_measurements_show() retrieve, for each entry,
> @@ -83,4 +84,6 @@ currently the following methods are supported:
>   - specify a template descriptor name from the kernel command line through
>     the 'ima_template=' parameter;
>   - register a new template descriptor with custom format through the kernel
> -   command line parameter 'ima_template_fmt='.
> +   command line parameter 'ima_template_fmt=';
> + - provide desired template name or custom format for specific events through
> +   the new policy options 'ima_template=' and 'ima_template_fmt='.
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index a9c3d3c..f4b3fd0 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -50,6 +50,7 @@ struct ima_rule_entry {
>  	u8 fsuuid[16];
>  	kuid_t uid;
>  	kuid_t fowner;
> +	struct ima_template_desc *desc;
>  	struct {
>  		void *rule;	/* LSM file metadata specific */
>  		void *args_p;	/* audit value */
> @@ -351,7 +352,8 @@ enum {
>  	Opt_obj_user, Opt_obj_role, Opt_obj_type,
>  	Opt_subj_user, Opt_subj_role, Opt_subj_type,
>  	Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
> -	Opt_appraise_type, Opt_fsuuid
> +	Opt_appraise_type, Opt_fsuuid,
> +	Opt_ima_template, Opt_ima_template_fmt
>  };
> 
>  static match_table_t policy_tokens = {
> @@ -373,6 +375,8 @@ static match_table_t policy_tokens = {
>  	{Opt_uid, "uid=%s"},
>  	{Opt_fowner, "fowner=%s"},
>  	{Opt_appraise_type, "appraise_type=%s"},
> +	{Opt_ima_template, "ima_template=%s"},
> +	{Opt_ima_template_fmt, "ima_template_fmt=%s"},
>  	{Opt_err, NULL}
>  };
> 
> @@ -621,6 +625,32 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			else
>  				result = -EINVAL;
>  			break;
> +		case Opt_ima_template:
> +			ima_log_string(ab, "ima_template", args[0].from);
> +
> +			if (entry->desc) {
> +				result = -EINVAL;
> +				break;
> +			}
> +

Right, this prevents overwriting a prior defined template.  Perhaps add
a short comment on the 'if (entry->desc) {' statement.

thanks,

Mimi

> +			entry->desc = ima_get_template_desc(args[0].from, NULL);
> +			if (entry->desc == NULL)
> +				result = -EINVAL;
> +
> +			break;
> +		case Opt_ima_template_fmt:
> +			ima_log_string(ab, "ima_template_fmt", args[0].from);
> +
> +			if (entry->desc) {
> +				result = -EINVAL;
> +				break;
> +			}
> +
> +			entry->desc = ima_get_template_desc(NULL, args[0].from);
> +			if (entry->desc == NULL)
> +				result = -EINVAL;
> +
> +			break;
>  		case Opt_err:
>  			ima_log_string(ab, "UNKNOWN", p);
>  			result = -EINVAL;



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-12-09 13:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-19 12:33 [PATCH-v2 0/6] ima: support per-measurement templates Roberto Sassu
2013-11-19 12:33 ` [PATCH-v2 1/6] ima: connect defined IMA templates through a linked list Roberto Sassu
2013-11-19 12:33 ` [PATCH-v2 2/6] ima: added new template helper lookup_template_desc_by_fmt() Roberto Sassu
2013-12-09 13:34   ` Mimi Zohar
2013-11-19 12:33 ` [PATCH-v2 3/6] ima: added ima_get_template_desc() for templates dynamic registration Roberto Sassu
2013-12-09 13:35   ` Mimi Zohar
2013-11-19 12:33 ` [PATCH-v2 4/6] ima: added ima_template and ima_template_fmt new policy options Roberto Sassu
2013-12-09 13:52   ` Mimi Zohar
2013-11-19 12:33 ` [PATCH-v2 5/6] ima: pass template descriptor to ima_alloc_init_template() Roberto Sassu
2013-11-19 12:33 ` [PATCH-v2 6/6] ima: use custom template obtained from a matched policy rule Roberto Sassu
2013-11-21 14:11 ` [Linux-ima-devel] [PATCH-v2 0/6] ima: support per-measurement templates Mimi Zohar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.