v2 Discard unused tunables from raw policy

v2 Discard unused tunables from raw policy

[Harry Ciao]

Special Notes
---------------
1. So far the latest master toolchain seems to have a segfault problem related
   with the commit of "libsemanage: patch for MCS/MLS in user files", which
   would be gone if we revert that commit or specify
   "disable-genhomedircon = true" in semanage.conf.


Differences from v1
--------------------

The v2 patchset just provides two minor changes from v1:

1. 0008-Preserve-tunables-when-required-by-semodule-program.patch
   Extract the codes to make use of sh->preserve_tunables flag in discard_tunable()
   from the 0007 patch in v1, and present them in this separate 0008 patch;

2. 0007-Create-a-new-preserve_tunables-flag-in-sepol_handle_.patch
   Fix an obvious error to mistakenly set sh->preserve_tunables flag in
   semanage_direct_connect(), which should be solely set according to the options
   passed for the semodule program.

   Then the preserve_tunables flag file in the module store could be properly
   removed if no "-P/--preserve_tunables" option used for semodule.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 1/8] Indicate when boolean is indeed a tunable.

[Harry Ciao]

Add flags to cond_bool_datum_t and cond_node_t structures to differentiate
the tunables' identifiers and conditionals from those of booleans.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsepol/include/sepol/policydb/conditional.h |    6 ++++--
 libsepol/include/sepol/policydb/policydb.h    |    5 ++++-
 libsepol/src/policydb.c                       |   14 ++++++++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/libsepol/include/sepol/policydb/conditional.h b/libsepol/include/sepol/policydb/conditional.h
index a8ed694..48ec106 100644
--- a/libsepol/include/sepol/policydb/conditional.h
+++ b/libsepol/include/sepol/policydb/conditional.h
@@ -77,15 +77,17 @@ typedef struct cond_node {
 	/* these true/false lists point into te_avtab when that is used */
 	cond_av_list_t *true_list;
 	cond_av_list_t *false_list;
-	/* and these are using during parsing and for modules */
+	/* and these are used during parsing and for modules */
 	avrule_t *avtrue_list;
 	avrule_t *avfalse_list;
 	/* these fields are not written to binary policy */
 	unsigned int nbools;
 	uint32_t bool_ids[COND_MAX_BOOLS];
 	uint32_t expr_pre_comp;
-	/*                                               */
 	struct cond_node *next;
+	/* a tunable conditional, calculated and used at expansion */
+#define	COND_NODE_FLAGS_TUNABLE	0x01
+	uint32_t flags;
 } cond_node_t;
 
 extern int cond_evaluate_expr(policydb_t * p, cond_expr_t * expr);
diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
index 5320bc8..1848a7b 100644
--- a/libsepol/include/sepol/policydb/policydb.h
+++ b/libsepol/include/sepol/policydb/policydb.h
@@ -210,6 +210,8 @@ typedef struct range_trans {
 typedef struct cond_bool_datum {
 	symtab_datum_t s;
 	int state;
+#define COND_BOOL_FLAGS_TUNABLE	0x01	/* is this a tunable? */
+	uint32_t flags;
 } cond_bool_datum_t;
 
 struct cond_node;
@@ -683,9 +685,10 @@ extern int policydb_set_target_platform(policydb_t *p, int platform);
 #define MOD_POLICYDB_VERSION_FILENAME_TRANS	11
 #define MOD_POLICYDB_VERSION_ROLETRANS		12
 #define MOD_POLICYDB_VERSION_ROLEATTRIB		13
+#define MOD_POLICYDB_VERSION_TUNABLE_SEP	14
 
 #define MOD_POLICYDB_VERSION_MIN MOD_POLICYDB_VERSION_BASE
-#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_ROLEATTRIB
+#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_TUNABLE_SEP
 
 #define POLICYDB_CONFIG_MLS    1
 
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 017aeca..136b450 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -221,6 +221,13 @@ static struct policydb_compat_info policydb_compat[] = {
 	 .target_platform = SEPOL_TARGET_SELINUX,
 	},
 	{
+	 .type = POLICY_BASE,
+	 .version = MOD_POLICYDB_VERSION_TUNABLE_SEP,
+	 .sym_num = SYM_NUM,
+	 .ocon_num = OCON_NODE6 + 1,
+	 .target_platform = SEPOL_TARGET_SELINUX,
+	},
+	{
 	 .type = POLICY_MOD,
 	 .version = MOD_POLICYDB_VERSION_BASE,
 	 .sym_num = SYM_NUM,
@@ -290,6 +297,13 @@ static struct policydb_compat_info policydb_compat[] = {
 	 .ocon_num = 0,
 	 .target_platform = SEPOL_TARGET_SELINUX,
 	},
+	{
+	 .type = POLICY_MOD,
+	 .version = MOD_POLICYDB_VERSION_TUNABLE_SEP,
+	 .sym_num = SYM_NUM,
+	 .ocon_num = 0,
+	 .target_platform = SEPOL_TARGET_SELINUX,
+	},
 };
 
 #if 0
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 2/8] Separate tunable from boolean during compile.

[Harry Ciao]

Both boolean and tunable keywords are processed by define_bool_tunable(),
argument 0 and 1 would be passed for boolean and tunable respectively.
For tunable, a TUNABLE flag would be set in cond_bool_datum_t.flags.

Note, when creating an if-else conditional we can not know if the
tunable identifier is indeed a tunable(for example, a boolean may be
misused in tunable_policy() or vice versa), thus the TUNABLE flag
for cond_node_t would be calculated and used in expansion when all
booleans/tunables copied during link.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 checkpolicy/module_compiler.c |   16 +++++++++++++++-
 checkpolicy/module_compiler.h |    1 +
 checkpolicy/policy_define.c   |    4 +++-
 checkpolicy/policy_define.h   |    2 +-
 checkpolicy/policy_parse.y    |    8 +++++++-
 checkpolicy/policy_scan.l     |    2 ++
 libsepol/src/conditional.c    |    1 +
 7 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
index 1c1d1d5..ffffaf1 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -1045,7 +1045,7 @@ int require_user(int pass)
 	}
 }
 
-int require_bool(int pass)
+static int require_bool_tunable(int pass, int is_tunable)
 {
 	char *id = queue_remove(id_queue);
 	cond_bool_datum_t *booldatum = NULL;
@@ -1063,6 +1063,8 @@ int require_bool(int pass)
 		yyerror("Out of memory!");
 		return -1;
 	}
+	if (is_tunable)
+		booldatum->flags |= COND_BOOL_FLAGS_TUNABLE;
 	retval =
 	    require_symbol(SYM_BOOLS, id, (hashtab_datum_t *) booldatum,
 			   &booldatum->s.value, &booldatum->s.value);
@@ -1094,6 +1096,16 @@ int require_bool(int pass)
 	}
 }
 
+int require_bool(int pass)
+{
+	return require_bool_tunable(pass, 0);
+}
+
+int require_tunable(int pass)
+{
+	return require_bool_tunable(pass, 1);
+}
+
 int require_sens(int pass)
 {
 	char *id = queue_remove(id_queue);
@@ -1328,6 +1340,8 @@ void append_cond_list(cond_list_t * cond)
 		     tmp = tmp->next) ;
 		tmp->next = cond->avfalse_list;
 	}
+
+	old_cond->flags |= cond->flags;
 }
 
 void append_avrule(avrule_t * avrule)
diff --git a/checkpolicy/module_compiler.h b/checkpolicy/module_compiler.h
index 45a21cd..72c2d9b 100644
--- a/checkpolicy/module_compiler.h
+++ b/checkpolicy/module_compiler.h
@@ -58,6 +58,7 @@ int require_attribute(int pass);
 int require_attribute_role(int pass);
 int require_user(int pass);
 int require_bool(int pass);
+int require_tunable(int pass);
 int require_sens(int pass);
 int require_cat(int pass);
 
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index ded27f7..1bf669c 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1494,7 +1494,7 @@ avrule_t *define_cond_compute_type(int which)
 	return avrule;
 }
 
-int define_bool(void)
+int define_bool_tunable(int is_tunable)
 {
 	char *id, *bool_value;
 	cond_bool_datum_t *datum;
@@ -1524,6 +1524,8 @@ int define_bool(void)
 		return -1;
 	}
 	memset(datum, 0, sizeof(cond_bool_datum_t));
+	if (is_tunable)
+		datum->flags |= COND_BOOL_FLAGS_TUNABLE;
 	ret = declare_symbol(SYM_BOOLS, id, datum, &value, &value);
 	switch (ret) {
 	case -3:{
diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h
index fc8cd4d..92a9be7 100644
--- a/checkpolicy/policy_define.h
+++ b/checkpolicy/policy_define.h
@@ -21,7 +21,7 @@ cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void* arg2);
 int define_attrib(void);
 int define_attrib_role(void);
 int define_av_perms(int inherits);
-int define_bool(void);
+int define_bool_tunable(int is_tunable);
 int define_category(void);
 int define_class(void);
 int define_common_perms(void);
diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
index 0a17bdc..49ac15f 100644
--- a/checkpolicy/policy_parse.y
+++ b/checkpolicy/policy_parse.y
@@ -101,6 +101,7 @@ typedef int (* require_func_t)();
 %token ALIAS
 %token ATTRIBUTE
 %token BOOL
+%token TUNABLE
 %token IF
 %token ELSE
 %token TYPE_TRANSITION
@@ -269,6 +270,7 @@ te_decl			: attribute_def
                         | typeattribute_def
                         | typebounds_def
                         | bool_def
+			| tunable_def
                         | transition_def
                         | range_trans_def
                         | te_avtab_def
@@ -295,8 +297,11 @@ opt_attr_list           : ',' id_comma_list
 			| 
 			;
 bool_def                : BOOL identifier bool_val ';'
-                        {if (define_bool()) return -1;}
+                        { if (define_bool_tunable(0)) return -1; }
                         ;
+tunable_def		: TUNABLE identifier bool_val ';'
+			{ if (define_bool_tunable(1)) return -1; }
+			;
 bool_val                : CTRUE
  			{ if (insert_id("T",0)) return -1; }
                         | CFALSE
@@ -820,6 +825,7 @@ require_decl_def        : ROLE        { $$ = require_role; }
                         | ATTRIBUTE_ROLE   { $$ = require_attribute_role; }
                         | USER        { $$ = require_user; }
                         | BOOL        { $$ = require_bool; }
+			| TUNABLE     { $$ = require_tunable; }
                         | SENSITIVITY { $$ = require_sens; }
                         | CATEGORY    { $$ = require_cat; }
                         ;
diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
index ed27bbe..a61e0db 100644
--- a/checkpolicy/policy_scan.l
+++ b/checkpolicy/policy_scan.l
@@ -92,6 +92,8 @@ TYPE |
 type				{ return(TYPE); }
 BOOL |
 bool                            { return(BOOL); }
+TUNABLE |
+tunable				{ return(TUNABLE); }
 IF |
 if				{ return(IF); }
 ELSE |
diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
index 1482387..efdedb0 100644
--- a/libsepol/src/conditional.c
+++ b/libsepol/src/conditional.c
@@ -160,6 +160,7 @@ cond_node_t *cond_node_create(policydb_t * p, cond_node_t * node)
 		for (i = 0; i < min(node->nbools, COND_MAX_BOOLS); i++)
 			new_node->bool_ids[i] = node->bool_ids[i];
 		new_node->expr_pre_comp = node->expr_pre_comp;
+		new_node->flags = node->flags;
 	}
 
 	return new_node;
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 3/8] Write and read TUNABLE flags in related data structures.

[Harry Ciao]

All flags in cond_bool_datum_t and cond_node_t structures are written
or read for policy modules which version is no less than
MOD_POLICYDB_VERSION_TUNABLE_SEP.

Note, for cond_node_t the TUNABLE flag bit would be used only at expand,
however, it won't hurt to read/write this field for modules(potentially
for future usage).

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsepol/src/conditional.c |   21 +++++++++++++++++++--
 libsepol/src/write.c       |   18 ++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
index efdedb0..d9d4fee 100644
--- a/libsepol/src/conditional.c
+++ b/libsepol/src/conditional.c
@@ -564,8 +564,8 @@ static int bool_isvalid(cond_bool_datum_t * b)
 	return 1;
 }
 
-int cond_read_bool(policydb_t * p
-		   __attribute__ ((unused)), hashtab_t h,
+int cond_read_bool(policydb_t * p,
+		   hashtab_t h,
 		   struct policy_file *fp)
 {
 	char *key = 0;
@@ -597,6 +597,15 @@ int cond_read_bool(policydb_t * p
 	if (rc < 0)
 		goto err;
 	key[len] = 0;
+
+	if (p->policy_type != POLICY_KERN &&
+	    p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
+		rc = next_entry(buf, fp, sizeof(uint32_t));
+		if (rc < 0)
+			goto err;
+		booldatum->flags = le32_to_cpu(buf[0]);
+	}
+
 	if (hashtab_insert(h, key, booldatum))
 		goto err;
 
@@ -810,6 +819,14 @@ static int cond_read_node(policydb_t * p, cond_node_t * node, void *fp)
 		if (avrule_read_list(p, &node->avfalse_list, fp))
 			goto err;
 	}
+	
+	if (p->policy_type != POLICY_KERN &&
+	    p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
+		rc = next_entry(buf, fp, sizeof(uint32_t));
+		if (rc < 0)
+			goto err;
+		node->flags = le32_to_cpu(buf[0]);
+	}
 
 	return 0;
       err:
diff --git a/libsepol/src/write.c b/libsepol/src/write.c
index 290e036..4284c93 100644
--- a/libsepol/src/write.c
+++ b/libsepol/src/write.c
@@ -607,6 +607,7 @@ static int cond_write_bool(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
 	unsigned int items, items2;
 	struct policy_data *pd = ptr;
 	struct policy_file *fp = pd->fp;
+	struct policydb *p = pd->p;
 
 	booldatum = (cond_bool_datum_t *) datum;
 
@@ -621,6 +622,15 @@ static int cond_write_bool(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
 	items = put_entry(key, 1, len, fp);
 	if (items != len)
 		return POLICYDB_ERROR;
+
+	if (p->policy_type != POLICY_KERN &&
+	    p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
+		buf[0] = cpu_to_le32(booldatum->flags);
+		items = put_entry(buf, sizeof(uint32_t), 1, fp);
+		if (items != 1)
+			return POLICYDB_ERROR;
+	}
+
 	return POLICYDB_SUCCESS;
 }
 
@@ -727,6 +737,14 @@ static int cond_write_node(policydb_t * p,
 			return POLICYDB_ERROR;
 	}
 
+	if (p->policy_type != POLICY_KERN &&
+	    p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {	
+		buf[0] = cpu_to_le32(node->flags);
+		items = put_entry(buf, sizeof(uint32_t), 1, fp);
+		if (items != 1)
+			return POLICYDB_ERROR;
+	}
+
 	return POLICYDB_SUCCESS;
 }
 
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 4/8] Copy and check the cond_bool_datum_t.flags during link.

[Harry Ciao]

Copy the TUNABLE flag for cond_bool_datum_t during link, and check
if there is a mismatch between boolean/tunable declaration and
usage among modules. If this is the case, bail out with errors.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsepol/src/link.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/libsepol/src/link.c b/libsepol/src/link.c
index 421c47b..ee9675b 100644
--- a/libsepol/src/link.c
+++ b/libsepol/src/link.c
@@ -587,7 +587,18 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
 		}
 		state->base->p_bools.nprim++;
 		base_bool = new_bool;
-
+		base_bool->flags = booldatum->flags;
+	} else if ((booldatum->flags & COND_BOOL_FLAGS_TUNABLE) !=
+		   (base_bool->flags & COND_BOOL_FLAGS_TUNABLE)) {
+			/* A mismatch between boolean/tunable declaration
+			 * and usage(for example a boolean used in the
+			 * tunable_policy() or vice versa).
+			 *
+			 * This is not allowed and bail out with errors */
+			ERR(state->handle,
+			    "%s: Mismatch between boolean/tunable definition "
+			    "and usage for %s", state->cur_mod_name, id);
+			return -1;
 	}
 
 	/* Get the scope info for this boolean to see if this is the declaration, 
@@ -595,9 +606,12 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
 	scope = hashtab_search(state->cur->policy->p_bools_scope.table, id);
 	if (!scope)
 		return SEPOL_ERR;
-	if (scope->scope == SCOPE_DECL)  
+	if (scope->scope == SCOPE_DECL) {
 		base_bool->state = booldatum->state;
-
+		/* Only the declaration rather than requirement
+		 * decides if it is a boolean or tunable. */
+		base_bool->flags = booldatum->flags;
+	}
 	state->cur->map[SYM_BOOLS][booldatum->s.value - 1] = base_bool->s.value;
 	return 0;
 
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 5/8] Permanently discard disabled branches of tunables in expansion.

[Harry Ciao]

For a cond_node_t in one decl->cond_list queue, append its
avtrue_list or avfalse_list to the avrules list of its home decl
depending on its state value, so that these effective rules would
be permanently added to te_avtab hashtab.

On the other hand, the rules on the disabled unused list won't be
expanded and written to the raw policy at all.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsepol/src/expand.c |   98 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 06f11f4..be41243 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -2665,6 +2665,94 @@ int expand_module_avrules(sepol_handle_t * handle, policydb_t * base,
 	return copy_and_expand_avrule_block(&state);
 }
 
+static void discard_tunables(policydb_t *pol)
+{
+	avrule_block_t *block;
+	avrule_decl_t *decl;
+	cond_node_t *cur_node;
+	cond_expr_t *cur_expr;
+	int cur_state;
+	avrule_t *tail, *to_be_appended;
+
+	/* Iterate through all cond_node of all enabled decls, if a cond_node
+	 * is about tunable, caculate its state value and concatenate one of
+	 * its avrule list to the current decl->avrules list.
+	 *
+	 * Note, such tunable cond_node would be skipped over in expansion,
+	 * so we won't have to worry about removing it from decl->cond_list
+	 * here :-)
+	 *
+	 * If tunables and booleans co-exist in the expression of a cond_node,
+	 * then tunables would be "transformed" as booleans.
+	 */
+	for (block = pol->global; block != NULL; block = block->next) {
+		decl = block->enabled;
+		if (decl == NULL || decl->enabled == 0)
+			continue;
+
+		tail = decl->avrules;
+		while (tail && tail->next)
+			tail = tail->next;
+
+		for (cur_node = decl->cond_list; cur_node != NULL;
+		     cur_node = cur_node->next) {
+			int booleans, tunables;
+			cond_bool_datum_t *booldatum;
+
+			booleans = tunables = 0;
+
+			for (cur_expr = cur_node->expr; cur_expr != NULL;
+			     cur_expr = cur_expr->next) {
+				if (cur_expr->expr_type != COND_BOOL)
+					continue;
+				booldatum = pol->bool_val_to_struct[cur_expr->bool - 1];
+				if (booldatum->flags & COND_BOOL_FLAGS_TUNABLE)
+					tunables++;
+				else
+					booleans++;
+			}
+
+			/* bool_copy_callback() at link phase has ensured
+			 * that no mixture of tunables and booleans in one
+			 * expression. */
+			assert(!(booleans && tunables));
+
+			if (booleans) {
+				cur_node->flags &= ~COND_NODE_FLAGS_TUNABLE;
+			} else {
+				cur_node->flags |= COND_NODE_FLAGS_TUNABLE;
+				cur_state = cond_evaluate_expr(pol, cur_node->expr);
+				if (cur_state == -1) {
+					printf("Expression result was "
+					       "undefined, skipping all"
+					       "rules\n");
+					continue;
+				}
+
+				to_be_appended = (cur_state == 1) ?
+					cur_node->avtrue_list : cur_node->avfalse_list;
+
+				if (tail)
+					tail->next = to_be_appended;
+				else
+					tail = decl->avrules = to_be_appended;
+
+				/* Now that the effective branch has been
+				 * appended, neutralize its original pointer */
+				if (cur_state == 1)
+					cur_node->avtrue_list = NULL;
+				else
+					cur_node->avfalse_list = NULL;
+
+				/* Update the tail of decl->avrules for
+				 * further concatenation */
+				while (tail && tail->next)
+					tail = tail->next;
+			}
+		}
+	}
+}
+
 /* Linking should always be done before calling expand, even if
  * there is only a base since all optionals are dealt with at link time
  * the base passed in should be indexed and avrule blocks should be 
@@ -2678,6 +2766,16 @@ int expand_module(sepol_handle_t * handle,
 	expand_state_t state;
 	avrule_block_t *curblock;
 
+	/* Append tunable's avtrue_list or avfalse_list to the avrules list
+	 * of its home decl depending on its state value, so that the effect
+	 * rules of a tunable would be added to te_avtab permanently. Whereas
+	 * the disabled unused branch would be discarded.
+	 *
+	 * Originally this function is called at the very end of link phase,
+	 * however, we need to keep the linked policy intact for analysis
+	 * purpose. */
+	discard_tunables(base);
+
 	expand_state_init(&state);
 
 	state.verbose = verbose;
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 6/8] Skip tunable identifier and cond_node_t in expansion.

[Harry Ciao]

The effective branch of a tunable has been appended to its home
decl->avrules list during link, in expansion we should just skip tunables
from expanding their rules into te_cond_avtab hashtab and adding to the
out->cond_list queue.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsepol/src/expand.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index be41243..d5f10a6 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -1014,6 +1014,11 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
 		return 0;
 	}
 
+	if (bool->flags & COND_BOOL_FLAGS_TUNABLE) {
+		/* Skip tunables */
+		return 0;
+	}
+
 	if (state->verbose)
 		INFO(state->handle, "copying boolean %s", id);
 
@@ -1046,6 +1051,7 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
 	state->boolmap[bool->s.value - 1] = new_bool->s.value;
 
 	new_bool->state = bool->state;
+	new_bool->flags = bool->flags;
 
 	return 0;
 }
@@ -1940,6 +1946,13 @@ static int cond_node_copy(expand_state_t * state, cond_node_t * cn)
 	if (cond_node_copy(state, cn->next)) {
 		return -1;
 	}
+	
+	/* If current cond_node_t is of tunable, its effective branch
+	 * has been appended to its home decl->avrules list during link
+	 * and now we should just skip it. */
+	if (cn->flags & COND_NODE_FLAGS_TUNABLE)
+		return 0;
+
 	if (cond_normalize_expr(state->base, cn)) {
 		ERR(state->handle, "Error while normalizing conditional");
 		return -1;
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 7/8] Create a new preserve_tunables flag in sepol_handle_t.

[Harry Ciao]

By default only the effective branch of a tunable conditional would be
expanded and written to raw policy, while all needless unused branches
would be discarded.

Add a new option '-P' or "--preserve_tunables" to the semodule program.
By default it is 0, if set to 1 then the above preserve_tunables flag
in the sepol_handle_t would be set to 1 accordingly.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsemanage/include/semanage/handle.h |    6 ++++++
 libsemanage/src/direct_api.c          |   29 ++++++++++++++++++++++++++++-
 libsemanage/src/handle.c              |   13 +++++++++++++
 libsemanage/src/libsemanage.map       |    1 +
 libsemanage/src/semanage_store.c      |    1 +
 libsemanage/src/semanage_store.h      |    1 +
 libsepol/include/sepol/handle.h       |    7 +++++++
 libsepol/src/handle.c                 |   15 +++++++++++++++
 libsepol/src/handle.h                 |    2 +-
 libsepol/src/libsepol.map             |    1 +
 policycoreutils/semodule/semodule.c   |   10 +++++++++-
 11 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h
index e303713..c746930 100644
--- a/libsemanage/include/semanage/handle.h
+++ b/libsemanage/include/semanage/handle.h
@@ -129,6 +129,12 @@ int semanage_mls_enabled(semanage_handle_t *sh);
 /* Change to alternate selinux root path */
 int semanage_set_root(const char *path);
 
+/* Get whether or not needless unused branch of tunables would be preserved */
+int semanage_get_preserve_tunables(semanage_handle_t * handle);
+
+/* Set whether or not to preserve the needless unused branch of tunables */
+void semanage_set_preserve_tunables(semanage_handle_t * handle, int preserve_tunables);
+
 /* META NOTES
  *
  * For all functions a non-negative number indicates success. For some
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index aac1974..4eba5dc 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -695,7 +695,8 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 
 	/* Declare some variables */
 	int modified = 0, fcontexts_modified, ports_modified,
-	    seusers_modified, users_extra_modified, dontaudit_modified;
+	    seusers_modified, users_extra_modified, dontaudit_modified,
+	    preserve_tunables_modified;
 	dbase_config_t *users = semanage_user_dbase_local(sh);
 	dbase_config_t *users_base = semanage_user_base_dbase_local(sh);
 	dbase_config_t *pusers_base = semanage_user_base_dbase_policy(sh);
@@ -737,6 +738,31 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 		}
 	}
 
+	/* Create or remove the preserve_tunables flag file. */
+	path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
+	if (access(path, F_OK) == 0)
+		preserve_tunables_modified = !(sepol_get_preserve_tunables(sh->sepolh) == 1);
+	else
+		preserve_tunables_modified = (sepol_get_preserve_tunables(sh->sepolh) == 1);
+	if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
+		FILE *touch;
+		touch = fopen(path, "w");
+		if (touch != NULL) {
+			if (fclose(touch) != 0) {
+				ERR(sh, "Error attempting to create preserve_tunable flag.");
+				goto cleanup;
+			}
+		} else {
+			ERR(sh, "Error attempting to create preserve_tunable flag.");
+			goto cleanup;
+		}
+	} else {
+		if (remove(path) == -1 && errno != ENOENT) {
+			ERR(sh, "Error removing the preserve_tunables flag.");
+			goto cleanup;
+		}
+	}
+
 	/* Before we do anything else, flush the join to its component parts.
 	 * This *does not* flush to disk automatically */
 	if (users->dtable->is_modified(users->dbase)) {
@@ -759,6 +785,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	modified |= ifaces->dtable->is_modified(ifaces->dbase);
 	modified |= nodes->dtable->is_modified(nodes->dbase);
 	modified |= dontaudit_modified;
+	modified |= preserve_tunables_modified;
 
 	/* If there were policy changes, or explicitly requested, rebuild the policy */
 	if (sh->do_rebuild || modified) {
diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c
index 647f0ee..7adc1cc 100644
--- a/libsemanage/src/handle.c
+++ b/libsemanage/src/handle.c
@@ -261,6 +261,19 @@ void semanage_set_disable_dontaudit(semanage_handle_t * sh, int disable_dontaudi
 	return;
 }
 
+int semanage_get_preserve_tunables(semanage_handle_t * sh)
+{
+	assert(sh != NULL);
+	return sepol_get_preserve_tunables(sh->sepolh);
+}
+
+void semanage_set_preserve_tunables(semanage_handle_t * sh,
+				    int preserve_tunables)
+{
+	assert(sh != NULL);
+	sepol_set_preserve_tunables(sh->sepolh, preserve_tunables);
+}
+
 void semanage_set_check_contexts(semanage_handle_t * sh, int do_check_contexts)
 {
 
diff --git a/libsemanage/src/libsemanage.map b/libsemanage/src/libsemanage.map
index 3222e3d..2827abe 100644
--- a/libsemanage/src/libsemanage.map
+++ b/libsemanage/src/libsemanage.map
@@ -22,5 +22,6 @@ LIBSEMANAGE_1.0 {
 	  semanage_is_connected; semanage_get_disable_dontaudit; semanage_set_disable_dontaudit;
 	  semanage_mls_enabled;
 	  semanage_set_check_contexts;
+	  semanage_get_preserve_tunables; semanage_set_preserve_tunables;
   local: *;
 };
diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 8d6ff1c..e5f8234 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -117,6 +117,7 @@ static const char *semanage_sandbox_paths[SEMANAGE_STORE_NUM_PATHS] = {
 	"/netfilter_contexts",
 	"/file_contexts.homedirs",
 	"/disable_dontaudit",
+	"/preserve_tunables",
 };
 
 /* A node used in a linked list of file contexts; used for sorting.
diff --git a/libsemanage/src/semanage_store.h b/libsemanage/src/semanage_store.h
index a0b2dd8..eaae05e 100644
--- a/libsemanage/src/semanage_store.h
+++ b/libsemanage/src/semanage_store.h
@@ -59,6 +59,7 @@ enum semanage_sandbox_defs {
 	SEMANAGE_NC,
 	SEMANAGE_FC_HOMEDIRS,
 	SEMANAGE_DISABLE_DONTAUDIT,
+	SEMANAGE_PRESERVE_TUNABLES,
 	SEMANAGE_STORE_NUM_PATHS
 };
 
diff --git a/libsepol/include/sepol/handle.h b/libsepol/include/sepol/handle.h
index 19be326..115bda1 100644
--- a/libsepol/include/sepol/handle.h
+++ b/libsepol/include/sepol/handle.h
@@ -24,4 +24,11 @@ void sepol_set_expand_consume_base(sepol_handle_t * sh, int consume_base);
 /* Destroy a sepol handle. */
 void sepol_handle_destroy(sepol_handle_t *);
 
+/* Get whether or not needless unused branch of tunables would be preserved */
+int sepol_get_preserve_tunables(sepol_handle_t * sh);
+
+/* Set whether or not to preserve the needless unused branch of tunables,
+ * 0 is default and discard such branch, 1 preserves them */
+void sepol_set_preserve_tunables(sepol_handle_t * sh, int preserve_tunables);
+
 #endif
diff --git a/libsepol/src/handle.c b/libsepol/src/handle.c
index 191ac57..2e9a4ad 100644
--- a/libsepol/src/handle.c
+++ b/libsepol/src/handle.c
@@ -18,9 +18,24 @@ sepol_handle_t *sepol_handle_create(void)
 	sh->disable_dontaudit = 0;
 	sh->expand_consume_base = 0;
 
+	/* by default needless unused branch of tunables would be discarded  */
+	sh->preserve_tunables = 0;
+
 	return sh;
 }
 
+int sepol_get_preserve_tunables(sepol_handle_t *sh)
+{
+	assert(sh != NULL);
+	return sh->preserve_tunables;
+}
+
+void sepol_set_preserve_tunables(sepol_handle_t * sh, int preserve_tunables)
+{
+	assert(sh !=NULL);
+	sh->preserve_tunables = preserve_tunables;
+}
+
 int sepol_get_disable_dontaudit(sepol_handle_t *sh)
 {
 	assert(sh !=NULL);
diff --git a/libsepol/src/handle.h b/libsepol/src/handle.h
index 254fbd8..7728d04 100644
--- a/libsepol/src/handle.h
+++ b/libsepol/src/handle.h
@@ -17,7 +17,7 @@ struct sepol_handle {
 
 	int disable_dontaudit;
 	int expand_consume_base;
-
+	int preserve_tunables;
 };
 
 #endif
diff --git a/libsepol/src/libsepol.map b/libsepol/src/libsepol.map
index 719e5b7..81e0d48 100644
--- a/libsepol/src/libsepol.map
+++ b/libsepol/src/libsepol.map
@@ -15,5 +15,6 @@
 	sepol_get_disable_dontaudit;
 	sepol_set_disable_dontaudit;
 	sepol_set_expand_consume_base;
+	sepol_get_preserve_tunables; sepol_set_preserve_tunables;
   local: *;
 };
diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
index 81d6a3c..5d662e7 100644
--- a/policycoreutils/semodule/semodule.c
+++ b/policycoreutils/semodule/semodule.c
@@ -45,6 +45,7 @@ static int no_reload;
 static int create_store;
 static int build;
 static int disable_dontaudit;
+static int preserve_tunables;
 
 static semanage_handle_t *sh = NULL;
 static char *store;
@@ -117,6 +118,7 @@ static void usage(char *progname)
 	printf("  -h,--help        print this message and quit\n");
 	printf("  -v,--verbose     be verbose\n");
 	printf("  -D,--disable_dontaudit	Remove dontaudits from policy\n");
+	printf("  -P,--preserve_tunables	Preserve tunables in policy\n");
 }
 
 /* Sets the global mode variable to new_mode, but only if no other
@@ -162,6 +164,7 @@ static void parse_command_line(int argc, char **argv)
 		{"noreload", 0, NULL, 'n'},
 		{"build", 0, NULL, 'B'},
 		{"disable_dontaudit", 0, NULL, 'D'},
+		{"preserve_tunables", 0, NULL, 'P'},
 		{"path", required_argument, NULL, 'p'},
 		{NULL, 0, NULL, 0}
 	};
@@ -171,7 +174,7 @@ static void parse_command_line(int argc, char **argv)
 	no_reload = 0;
 	create_store = 0;
 	while ((i =
-		getopt_long(argc, argv, "p:s:b:hi:lvqe:d:r:u:RnBD", opts,
+		getopt_long(argc, argv, "p:s:b:hi:lvqe:d:r:u:RnBDP", opts,
 			    NULL)) != -1) {
 		switch (i) {
 		case 'b':
@@ -220,6 +223,9 @@ static void parse_command_line(int argc, char **argv)
 		case 'D':
 			disable_dontaudit = 1;
 			break;
+		case 'P':
+			preserve_tunables = 1;
+			break;
 		case '?':
 		default:{
 				usage(argv[0]);
@@ -466,6 +472,8 @@ int main(int argc, char *argv[])
 			semanage_set_disable_dontaudit(sh, 1);
 		else if (build)
 			semanage_set_disable_dontaudit(sh, 0);
+		if (preserve_tunables)
+			semanage_set_preserve_tunables(sh, 1);
 
 		result = semanage_commit(sh);
 	}
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[v2 PATCH 8/8] Preserve tunables when required by semodule program.

[Harry Ciao]

If the "-P/--preserve_tunables" option is set for the semodule program,
the preserve_tunables flag in sepol_handle_t would be set, then all tunables
would be treated as booleans by having their TUNABLE flag bit cleared,
resulting in all tunables if-else conditionals preserved for raw policy.

Note, such option would invalidate the logic to double-check if tunables
ever mix with booleans in one expression, so skip the call to assert()
when this option is passed.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 libsepol/src/expand.c |   36 ++++++++++++++++++++++++------------
 1 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index d5f10a6..d67b84c 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -2678,25 +2678,29 @@ int expand_module_avrules(sepol_handle_t * handle, policydb_t * base,
 	return copy_and_expand_avrule_block(&state);
 }
 
-static void discard_tunables(policydb_t *pol)
+static void discard_tunables(sepol_handle_t *sh, policydb_t *pol)
 {
 	avrule_block_t *block;
 	avrule_decl_t *decl;
 	cond_node_t *cur_node;
 	cond_expr_t *cur_expr;
-	int cur_state;
+	int cur_state, preserve_tunables = 0;
 	avrule_t *tail, *to_be_appended;
 
+	if (sh && sh->preserve_tunables)
+		preserve_tunables = 1;
+
 	/* Iterate through all cond_node of all enabled decls, if a cond_node
-	 * is about tunable, caculate its state value and concatenate one of
-	 * its avrule list to the current decl->avrules list.
+	 * is about tunable, calculate its state value and concatenate one of
+	 * its avrule list to the current decl->avrules list. On the other
+	 * hand, the disabled unused branch of a tunable would be discarded.
 	 *
 	 * Note, such tunable cond_node would be skipped over in expansion,
 	 * so we won't have to worry about removing it from decl->cond_list
 	 * here :-)
 	 *
-	 * If tunables and booleans co-exist in the expression of a cond_node,
-	 * then tunables would be "transformed" as booleans.
+	 * If tunables are requested to be preserved then they would be
+	 * "transformed" as booleans by having their TUNABLE flag cleared.
 	 */
 	for (block = pol->global; block != NULL; block = block->next) {
 		decl = block->enabled;
@@ -2709,10 +2713,12 @@ static void discard_tunables(policydb_t *pol)
 
 		for (cur_node = decl->cond_list; cur_node != NULL;
 		     cur_node = cur_node->next) {
-			int booleans, tunables;
+			int booleans, tunables, i;
 			cond_bool_datum_t *booldatum;
+			cond_bool_datum_t *tmp[COND_EXPR_MAXDEPTH];
 
 			booleans = tunables = 0;
+			memset(tmp, 0, sizeof(cond_bool_datum_t *) * COND_EXPR_MAXDEPTH);
 
 			for (cur_expr = cur_node->expr; cur_expr != NULL;
 			     cur_expr = cur_expr->next) {
@@ -2720,18 +2726,24 @@ static void discard_tunables(policydb_t *pol)
 					continue;
 				booldatum = pol->bool_val_to_struct[cur_expr->bool - 1];
 				if (booldatum->flags & COND_BOOL_FLAGS_TUNABLE)
-					tunables++;
+					tmp[tunables++] = booldatum;
 				else
 					booleans++;
 			}
 
 			/* bool_copy_callback() at link phase has ensured
 			 * that no mixture of tunables and booleans in one
-			 * expression. */
-			assert(!(booleans && tunables));
+			 * expression. However, this would be broken by the
+			 * request to preserve tunables */
+			if (!preserve_tunables)
+				assert(!(booleans && tunables));
 
-			if (booleans) {
+			if (booleans || preserve_tunables) {
 				cur_node->flags &= ~COND_NODE_FLAGS_TUNABLE;
+				if (tunables) {
+					for (i = 0; i < tunables; i++)
+						tmp[i]->flags &= ~COND_BOOL_FLAGS_TUNABLE;
+				}
 			} else {
 				cur_node->flags |= COND_NODE_FLAGS_TUNABLE;
 				cur_state = cond_evaluate_expr(pol, cur_node->expr);
@@ -2787,7 +2799,7 @@ int expand_module(sepol_handle_t * handle,
 	 * Originally this function is called at the very end of link phase,
 	 * however, we need to keep the linked policy intact for analysis
 	 * purpose. */
-	discard_tunables(base);
+	discard_tunables(handle, base);
 
 	expand_state_init(&state);
 
-- 
1.7.0.4


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [v2 PATCH 3/8] Write and read TUNABLE flags in related data structures.

[Eric Paris]

This patch contained 2 whitespace errors.  they have been fixed before
applying it to my queue branch, however please fix them if there is a
need to resubmit a v3.

-Eric

On Wed, Aug 31, 2011 at 11:29 PM, Harry Ciao <qingtao.cao@windriver.com> wrote:
> All flags in cond_bool_datum_t and cond_node_t structures are written
> or read for policy modules which version is no less than
> MOD_POLICYDB_VERSION_TUNABLE_SEP.
>
> Note, for cond_node_t the TUNABLE flag bit would be used only at expand,
> however, it won't hurt to read/write this field for modules(potentially
> for future usage).
>
> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
> ---
>  libsepol/src/conditional.c |   21 +++++++++++++++++++--
>  libsepol/src/write.c       |   18 ++++++++++++++++++
>  2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> index efdedb0..d9d4fee 100644
> --- a/libsepol/src/conditional.c
> +++ b/libsepol/src/conditional.c
> @@ -564,8 +564,8 @@ static int bool_isvalid(cond_bool_datum_t * b)
>        return 1;
>  }
>
> -int cond_read_bool(policydb_t * p
> -                  __attribute__ ((unused)), hashtab_t h,
> +int cond_read_bool(policydb_t * p,
> +                  hashtab_t h,
>                   struct policy_file *fp)
>  {
>        char *key = 0;
> @@ -597,6 +597,15 @@ int cond_read_bool(policydb_t * p
>        if (rc < 0)
>                goto err;
>        key[len] = 0;
> +
> +       if (p->policy_type != POLICY_KERN &&
> +           p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> +               rc = next_entry(buf, fp, sizeof(uint32_t));
> +               if (rc < 0)
> +                       goto err;
> +               booldatum->flags = le32_to_cpu(buf[0]);
> +       }
> +
>        if (hashtab_insert(h, key, booldatum))
>                goto err;
>
> @@ -810,6 +819,14 @@ static int cond_read_node(policydb_t * p, cond_node_t * node, void *fp)
>                if (avrule_read_list(p, &node->avfalse_list, fp))
>                        goto err;
>        }
> +
> +       if (p->policy_type != POLICY_KERN &&
> +           p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> +               rc = next_entry(buf, fp, sizeof(uint32_t));
> +               if (rc < 0)
> +                       goto err;
> +               node->flags = le32_to_cpu(buf[0]);
> +       }
>
>        return 0;
>       err:
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index 290e036..4284c93 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -607,6 +607,7 @@ static int cond_write_bool(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
>        unsigned int items, items2;
>        struct policy_data *pd = ptr;
>        struct policy_file *fp = pd->fp;
> +       struct policydb *p = pd->p;
>
>        booldatum = (cond_bool_datum_t *) datum;
>
> @@ -621,6 +622,15 @@ static int cond_write_bool(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
>        items = put_entry(key, 1, len, fp);
>        if (items != len)
>                return POLICYDB_ERROR;
> +
> +       if (p->policy_type != POLICY_KERN &&
> +           p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> +               buf[0] = cpu_to_le32(booldatum->flags);
> +               items = put_entry(buf, sizeof(uint32_t), 1, fp);
> +               if (items != 1)
> +                       return POLICYDB_ERROR;
> +       }
> +
>        return POLICYDB_SUCCESS;
>  }
>
> @@ -727,6 +737,14 @@ static int cond_write_node(policydb_t * p,
>                        return POLICYDB_ERROR;
>        }
>
> +       if (p->policy_type != POLICY_KERN &&
> +           p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> +               buf[0] = cpu_to_le32(node->flags);
> +               items = put_entry(buf, sizeof(uint32_t), 1, fp);
> +               if (items != 1)
> +                       return POLICYDB_ERROR;
> +       }
> +
>        return POLICYDB_SUCCESS;
>  }
>
> --
> 1.7.0.4
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [v2 PATCH 6/8] Skip tunable identifier and cond_node_t in expansion.

[Eric Paris]

This patch contained a whitespace error.  I have fixed it when
applying to my queue branch.  Please fix before resending if there is
a need for a v3.

-Eric

On Wed, Aug 31, 2011 at 11:29 PM, Harry Ciao <qingtao.cao@windriver.com> wrote:
> The effective branch of a tunable has been appended to its home
> decl->avrules list during link, in expansion we should just skip tunables
> from expanding their rules into te_cond_avtab hashtab and adding to the
> out->cond_list queue.
>
> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
> ---
>  libsepol/src/expand.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index be41243..d5f10a6 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -1014,6 +1014,11 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
>                return 0;
>        }
>
> +       if (bool->flags & COND_BOOL_FLAGS_TUNABLE) {
> +               /* Skip tunables */
> +               return 0;
> +       }
> +
>        if (state->verbose)
>                INFO(state->handle, "copying boolean %s", id);
>
> @@ -1046,6 +1051,7 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
>        state->boolmap[bool->s.value - 1] = new_bool->s.value;
>
>        new_bool->state = bool->state;
> +       new_bool->flags = bool->flags;
>
>        return 0;
>  }
> @@ -1940,6 +1946,13 @@ static int cond_node_copy(expand_state_t * state, cond_node_t * cn)
>        if (cond_node_copy(state, cn->next)) {
>                return -1;
>        }
> +
> +       /* If current cond_node_t is of tunable, its effective branch
> +        * has been appended to its home decl->avrules list during link
> +        * and now we should just skip it. */
> +       if (cn->flags & COND_NODE_FLAGS_TUNABLE)
> +               return 0;
> +
>        if (cond_normalize_expr(state->base, cn)) {
>                ERR(state->handle, "Error while normalizing conditional");
>                return -1;
> --
> 1.7.0.4
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [v2 PATCH 6/8] Skip tunable identifier and cond_node_t in expansion.

[Harry Ciao]

Thanks Eric for pointing this out!　I would certainly fix the whitespace
errors in v3 patchset if that is desirable.

Thanks,
Harry

Eric Paris 写道:
> This patch contained a whitespace error.  I have fixed it when
> applying to my queue branch.  Please fix before resending if there is
> a need for a v3.
>
> -Eric
>
> On Wed, Aug 31, 2011 at 11:29 PM, Harry Ciao <qingtao.cao@windriver.com> wrote:
>   
>> The effective branch of a tunable has been appended to its home
>> decl->avrules list during link, in expansion we should just skip tunables
>> from expanding their rules into te_cond_avtab hashtab and adding to the
>> out->cond_list queue.
>>
>> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
>> ---
>>  libsepol/src/expand.c |   13 +++++++++++++
>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
>> index be41243..d5f10a6 100644
>> --- a/libsepol/src/expand.c
>> +++ b/libsepol/src/expand.c
>> @@ -1014,6 +1014,11 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
>>                return 0;
>>        }
>>
>> +       if (bool->flags & COND_BOOL_FLAGS_TUNABLE) {
>> +               /* Skip tunables */
>> +               return 0;
>> +       }
>> +
>>        if (state->verbose)
>>                INFO(state->handle, "copying boolean %s", id);
>>
>> @@ -1046,6 +1051,7 @@ static int bool_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
>>        state->boolmap[bool->s.value - 1] = new_bool->s.value;
>>
>>        new_bool->state = bool->state;
>> +       new_bool->flags = bool->flags;
>>
>>        return 0;
>>  }
>> @@ -1940,6 +1946,13 @@ static int cond_node_copy(expand_state_t * state, cond_node_t * cn)
>>        if (cond_node_copy(state, cn->next)) {
>>                return -1;
>>        }
>> +
>> +       /* If current cond_node_t is of tunable, its effective branch
>> +        * has been appended to its home decl->avrules list during link
>> +        * and now we should just skip it. */
>> +       if (cn->flags & COND_NODE_FLAGS_TUNABLE)
>> +               return 0;
>> +
>>        if (cond_normalize_expr(state->base, cn)) {
>>                ERR(state->handle, "Error while normalizing conditional");
>>                return -1;
>> --
>> 1.7.0.4
>>
>>
>> --
>> This message was distributed to subscribers of the selinux mailing list.
>> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
>> the words "unsubscribe selinux" without quotes as the message.
>>
>>     
>
>   


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [v2 PATCH 3/8] Write and read TUNABLE flags in related data structures.

[HarryCiao]

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


I just realized that I could use the checkpatch.pl in linux source tree to detect any potential whitespace problems in the userspace patches as well. Although we put less strict constraints on the style of userspace patches than kernel patches, it still would be advisable to run checkpatch.pl.

Maybe we could mention this in the "How to Create A Patch Set" section in the wiki of http://oss.tresys.com/projects/refpolicy/wiki/HowToContribute

Thanks,
Harry

> Date: Wed, 7 Sep 2011 17:47:43 -0400
> Subject: Re: [v2 PATCH 3/8] Write and read TUNABLE flags in related data structures.
> From: eparis@parisplace.org
> To: qingtao.cao@windriver.com
> CC: selinux@tycho.nsa.gov
> 
> This patch contained 2 whitespace errors.  they have been fixed before
> applying it to my queue branch, however please fix them if there is a
> need to resubmit a v3.
> 
> -Eric
> 
> On Wed, Aug 31, 2011 at 11:29 PM, Harry Ciao <qingtao.cao@windriver.com> wrote:
> > All flags in cond_bool_datum_t and cond_node_t structures are written
> > or read for policy modules which version is no less than
> > MOD_POLICYDB_VERSION_TUNABLE_SEP.
> >
> > Note, for cond_node_t the TUNABLE flag bit would be used only at expand,
> > however, it won't hurt to read/write this field for modules(potentially
> > for future usage).
> >
> > Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
> > ---
> > álibsepol/src/conditional.c | á 21 +++++++++++++++++++--
> > álibsepol/src/write.c á á á | á 18 ++++++++++++++++++
> > á2 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> > index efdedb0..d9d4fee 100644
> > --- a/libsepol/src/conditional.c
> > +++ b/libsepol/src/conditional.c
> > @@ -564,8 +564,8 @@ static int bool_isvalid(cond_bool_datum_t * b)
> > á á á áreturn 1;
> > á}
> >
> > -int cond_read_bool(policydb_t * p
> > - á á á á á á á á á__attribute__ ((unused)), hashtab_t h,
> > +int cond_read_bool(policydb_t * p,
> > + á á á á á á á á áhashtab_t h,
> > á á á á á á á á á struct policy_file *fp)
> > á{
> > á á á áchar *key = 0;
> > @@ -597,6 +597,15 @@ int cond_read_bool(policydb_t * p
> > á á á áif (rc < 0)
> > á á á á á á á ágoto err;
> > á á á ákey[len] = 0;
> > +
> > + á á á if (p->policy_type != POLICY_KERN &&
> > + á á á á á p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> > + á á á á á á á rc = next_entry(buf, fp, sizeof(uint32_t));
> > + á á á á á á á if (rc < 0)
> > + á á á á á á á á á á á goto err;
> > + á á á á á á á booldatum->flags = le32_to_cpu(buf[0]);
> > + á á á }
> > +
> > á á á áif (hashtab_insert(h, key, booldatum))
> > á á á á á á á ágoto err;
> >
> > @@ -810,6 +819,14 @@ static int cond_read_node(policydb_t * p, cond_node_t * node, void *fp)
> > á á á á á á á áif (avrule_read_list(p, &node->avfalse_list, fp))
> > á á á á á á á á á á á ágoto err;
> > á á á á}
> > +
> > + á á á if (p->policy_type != POLICY_KERN &&
> > + á á á á á p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> > + á á á á á á á rc = next_entry(buf, fp, sizeof(uint32_t));
> > + á á á á á á á if (rc < 0)
> > + á á á á á á á á á á á goto err;
> > + á á á á á á á node->flags = le32_to_cpu(buf[0]);
> > + á á á }
> >
> > á á á áreturn 0;
> > á á á err:
> > diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> > index 290e036..4284c93 100644
> > --- a/libsepol/src/write.c
> > +++ b/libsepol/src/write.c
> > @@ -607,6 +607,7 @@ static int cond_write_bool(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
> > á á á áunsigned int items, items2;
> > á á á ástruct policy_data *pd = ptr;
> > á á á ástruct policy_file *fp = pd->fp;
> > + á á á struct policydb *p = pd->p;
> >
> > á á á ábooldatum = (cond_bool_datum_t *) datum;
> >
> > @@ -621,6 +622,15 @@ static int cond_write_bool(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
> > á á á áitems = put_entry(key, 1, len, fp);
> > á á á áif (items != len)
> > á á á á á á á áreturn POLICYDB_ERROR;
> > +
> > + á á á if (p->policy_type != POLICY_KERN &&
> > + á á á á á p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> > + á á á á á á á buf[0] = cpu_to_le32(booldatum->flags);
> > + á á á á á á á items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > + á á á á á á á if (items != 1)
> > + á á á á á á á á á á á return POLICYDB_ERROR;
> > + á á á }
> > +
> > á á á áreturn POLICYDB_SUCCESS;
> > á}
> >
> > @@ -727,6 +737,14 @@ static int cond_write_node(policydb_t * p,
> > á á á á á á á á á á á áreturn POLICYDB_ERROR;
> > á á á á}
> >
> > + á á á if (p->policy_type != POLICY_KERN &&
> > + á á á á á p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> > + á á á á á á á buf[0] = cpu_to_le32(node->flags);
> > + á á á á á á á items = put_entry(buf, sizeof(uint32_t), 1, fp);
> > + á á á á á á á if (items != 1)
> > + á á á á á á á á á á á return POLICYDB_ERROR;
> > + á á á }
> > +
> > á á á áreturn POLICYDB_SUCCESS;
> > á}
> >
> > --
> > 1.7.0.4
> >
> >
> > --
> > This message was distributed to subscribers of the selinux mailing list.
> > If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> > the words "unsubscribe selinux" without quotes as the message.
> >
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
 		 	   		  

[-- Attachment #2: Type: text/html, Size: 7210 bytes --]

Re: v2 Discard unused tunables from raw policy

[Eric Paris]

On Wed, Aug 31, 2011 at 11:29 PM, Harry Ciao <qingtao.cao@windriver.com> wrote:

> Differences from v1
> --------------------
>
> The v2 patchset just provides two minor changes from v1:
>
> 1. 0008-Preserve-tunables-when-required-by-semodule-program.patch
>   Extract the codes to make use of sh->preserve_tunables flag in discard_tunable()
>   from the 0007 patch in v1, and present them in this separate 0008 patch;
>
> 2. 0007-Create-a-new-preserve_tunables-flag-in-sepol_handle_.patch
>   Fix an obvious error to mistakenly set sh->preserve_tunables flag in
>   semanage_direct_connect(), which should be solely set according to the options
>   passed for the semodule program.
>
>   Then the preserve_tunables flag file in the module store could be properly
>   removed if no "-P/--preserve_tunables" option used for semodule.

I tried to build fedora policy with this patch set and it didn't work
out this time.  It complains about:

libsepol.bool_copy_callback: ppp: Mismatch between boolean/tunable
definition and usage for secure_mode_insmod
/usr/bin/semodule_link:  Error while linking packages

And then stops building.  I know you mentioned this as a known issue,
but previously I don't remember it failing to build.  All I did was
apply the 8 patches in this series, then apply the 4 patches to policy
you sent a long time ago.  Applying those 4 to fedora policy still
meant a couple of minor changes, but nothing directly to ppp.{te,if}

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: v2 Discard unused tunables from raw policy

[Harry Ciao]

Hi Eric,

Eric Paris 写道:
> On Wed, Aug 31, 2011 at 11:29 PM, Harry Ciao <qingtao.cao@windriver.com> wrote:
>
>   
>> Differences from v1
>> --------------------
>>
>> The v2 patchset just provides two minor changes from v1:
>>
>> 1. 0008-Preserve-tunables-when-required-by-semodule-program.patch
>>   Extract the codes to make use of sh->preserve_tunables flag in discard_tunable()
>>   from the 0007 patch in v1, and present them in this separate 0008 patch;
>>
>> 2. 0007-Create-a-new-preserve_tunables-flag-in-sepol_handle_.patch
>>   Fix an obvious error to mistakenly set sh->preserve_tunables flag in
>>   semanage_direct_connect(), which should be solely set according to the options
>>   passed for the semodule program.
>>
>>   Then the preserve_tunables flag file in the module store could be properly
>>   removed if no "-P/--preserve_tunables" option used for semodule.
>>     
>
> I tried to build fedora policy with this patch set and it didn't work
> out this time.  It complains about:
>
> libsepol.bool_copy_callback: ppp: Mismatch between boolean/tunable
> definition and usage for secure_mode_insmod
> /usr/bin/semodule_link:  Error while linking packages
>
>   
The directly reason is the secure_mod_insmod boolean is used along with
pppd_can_insmod tunable in ppp.te in the tunable_policy() macro.
Apparently ppp.pp is linked first with the assumption of
secure_mod_insmod being required as a tunable. Later, when the module
that actually presents the definition of secure_mode_insmod is linked
the link process would find it is declared as a boolean. That's why you
have run into above error message.

I have taken Chris's suggestion to error out rather than just printing
warning messages about the mixture of any tunable with boolean. In order
to support this change Chris has transformed the definition of
pppd_can_insmod from gen_tunable() to gen_bool(), and use an explicit
if-else conditional to replace the call of tunable_policy(). (BTW, we
can use the boolean_policy() once the whole patchset is merged since
both pppd_can_insmod and secure_mode_insmod are booleans now)

Therefore please try to cherry-pick the commit of 68bbbbde to resolve
this problem.

Thanks,
Harry

> And then stops building.  I know you mentioned this as a known issue,
> but previously I don't remember it failing to build.  All I did was
> apply the 8 patches in this series, then apply the 4 patches to policy
> you sent a long time ago.  Applying those 4 to fedora policy still
> meant a couple of minor changes, but nothing directly to ppp.{te,if}
>
> -Eric
>
>   


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.