selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Fix some cil_gen_policy() bugs.
@ 2016-09-08 20:30 Daniel Cashman
  2016-09-08 20:30 ` [PATCH 1/5] libsepol: cil: Add userrole mapping to cil_gen_policy() Daniel Cashman
  2016-09-08 20:37 ` [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:30 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv, dcashman

From: dcashman <dcashman@android.com>

cil_gen_policy() appears to exist to generate a policy.conf corresponding to the
original SELinux HLL from a cil_db struct.  All of libsepol/cil/src/cil_policy.c
appears to exist to support this functionality.  This patchset provides some
fixes for issues encountered when trying to go from android's policy.conf to a
CIL representation (via checkpolicy) and then back to the HLL representation via
cil_gen_policy().

dcashman (5):
  libsepol: cil: Add userrole mapping to cil_gen_policy().
  libsepol: cil: Remove duplicate sid policy declaration.
  libsepol: cil: Replace sensitivityorder statement.
  libsepol: cil: Fix CIL_OP data assignment.
  libsepol: cil: Add cil_constraint_expr_to_policy()

 libsepol/cil/src/cil_policy.c | 235 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 224 insertions(+), 11 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 1/5] libsepol: cil: Add userrole mapping to cil_gen_policy().
  2016-09-08 20:30 [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
@ 2016-09-08 20:30 ` Daniel Cashman
  2016-09-08 20:30   ` [PATCH 2/5] libsepol: cil: Remove duplicate sid policy declaration Daniel Cashman
  2016-09-08 20:37 ` [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:30 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv, dcashman

From: dcashman <dcashman@android.com>

Avoid a "No roles associated with user" error produced by
cil_userrole_to_policy() when a userrole mapping is present in CIL policy.

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 libsepol/cil/src/cil_policy.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_policy.c b/libsepol/cil/src/cil_policy.c
index 382129b..324becc 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -1155,7 +1155,12 @@ int __cil_gen_policy_node_helper(struct cil_tree_node *node, uint32_t *finished,
 	} else {
 		switch (node->flavor) {
 		case CIL_USER:
-			cil_multimap_insert(users, node->data, NULL, CIL_USERROLE, CIL_NONE);
+			cil_multimap_insert(users, node->data, NULL, CIL_USER, CIL_NONE);
+			break;
+		case CIL_USERROLE: {
+			struct cil_userrole *userrole = node->data;
+			cil_multimap_insert(users, userrole->user, userrole->role, CIL_USER, CIL_ROLE);
+		}
 			break;
 		case CIL_CATALIAS: {
 			struct cil_alias *alias = node->data;
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 2/5] libsepol: cil: Remove duplicate sid policy declaration.
  2016-09-08 20:30 ` [PATCH 1/5] libsepol: cil: Add userrole mapping to cil_gen_policy() Daniel Cashman
@ 2016-09-08 20:30   ` Daniel Cashman
  2016-09-08 20:30     ` [PATCH 3/5] libsepol: cil: Replace sensitivityorder statement Daniel Cashman
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:30 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv, dcashman

From: dcashman <dcashman@android.com>

cil_gen_policy() creates two sets of sid declarations from CIL policy
due to the combination of sidorder iteration and the CIL_SID case in the
AST walk.  Remove the sidorder iteration.

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 libsepol/cil/src/cil_policy.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libsepol/cil/src/cil_policy.c b/libsepol/cil/src/cil_policy.c
index 324becc..d8ef151 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -1272,10 +1272,6 @@ int cil_gen_policy(struct cil_db *db)
 
 	policy_file = fopen("policy.conf", "w+");
 
-	cil_list_for_each(item, db->sidorder) {
-		fprintf(file_arr[ISIDS], "sid %s ", ((struct cil_sid*)item->data)->datum.name);
-	}
-
 	cil_list_for_each(item, db->classorder) {
 		struct cil_class *class = item->data;
 		struct cil_tree_node *node = class->datum.nodes->head->data;
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 3/5] libsepol: cil: Replace sensitivityorder statement.
  2016-09-08 20:30   ` [PATCH 2/5] libsepol: cil: Remove duplicate sid policy declaration Daniel Cashman
@ 2016-09-08 20:30     ` Daniel Cashman
  2016-09-08 20:30       ` [PATCH 4/5] libsepol: cil: Fix CIL_OP data assignment Daniel Cashman
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:30 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv, dcashman

From: dcashman <dcashman@android.com>

cil_gen_policy() prints a sensitivityorder{}; output statement when
generating its policy.conf file from CIL policy.  This omits the
sensitivity declarations, however, and should instead be represented as
a sid declaration block followed by a dominance statement.

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 libsepol/cil/src/cil_policy.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libsepol/cil/src/cil_policy.c b/libsepol/cil/src/cil_policy.c
index d8ef151..78b135e 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -1301,11 +1301,14 @@ int cil_gen_policy(struct cil_db *db)
 	}
 
 	if (db->sensitivityorder->head != NULL) {
-		fprintf(file_arr[SENS], "sensitivityorder { ");
+		cil_list_for_each(item, db->sensitivityorder) {
+			fprintf(file_arr[SENS], "sensitivity %s;\n", ((struct cil_sens*)item->data)->datum.name);
+		}
+		fprintf(file_arr[SENS], "dominance { ");
 		cil_list_for_each(item, db->sensitivityorder) {
 			fprintf(file_arr[SENS], "%s ", ((struct cil_sens*)item->data)->datum.name);
 		}
-		fprintf(file_arr[SENS], "};\n");
+		fprintf(file_arr[SENS], "}\n");
 	}
 
 	extra_args.users = users;
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 4/5] libsepol: cil: Fix CIL_OP data assignment.
  2016-09-08 20:30     ` [PATCH 3/5] libsepol: cil: Replace sensitivityorder statement Daniel Cashman
@ 2016-09-08 20:30       ` Daniel Cashman
  2016-09-08 20:30         ` [PATCH 5/5] libsepol: cil: Add cil_constraint_expr_to_policy() Daniel Cashman
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:30 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv, dcashman

From: dcashman <dcashman@android.com>

cil_flavor enums stored in cil_list_items are not pointers, but rather
the actual enum value.  Remove pointer dereferences on this value to
avoid segfaults.

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 libsepol/cil/src/cil_policy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libsepol/cil/src/cil_policy.c b/libsepol/cil/src/cil_policy.c
index 78b135e..32b6b41 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -470,7 +470,7 @@ void cil_perms_to_policy(FILE **file_arr, uint32_t file_index, struct cil_list *
 			fprintf(file_arr[file_index], " %s", ((struct cil_symtab_datum *)curr->data)->name);
 			break;
 		case CIL_OP: {
-			enum cil_flavor op_flavor = *((enum cil_flavor *)curr->data);
+			enum cil_flavor op_flavor = (enum cil_flavor)curr->data;
 			char *op_str = NULL;
 
 			switch (op_flavor) {
@@ -673,7 +673,7 @@ static int cil_expr_to_string(struct cil_list *expr, char **out)
 		case CIL_OP: {
 			int len;
 			char *expr_str;
-			enum cil_flavor op_flavor = *((enum cil_flavor *)curr->data);
+			enum cil_flavor op_flavor = (enum cil_flavor)curr->data;
 			char *op_str = NULL;
 
 			if (pos == 0) {
@@ -742,7 +742,7 @@ static int cil_expr_to_string(struct cil_list *expr, char **out)
 			break;
 		}
 		case CIL_CONS_OPERAND: {
-			enum cil_flavor operand_flavor = *((enum cil_flavor *)curr->data);
+			enum cil_flavor operand_flavor = (enum cil_flavor)curr->data;
 			char *operand_str = NULL;
 			switch (operand_flavor) {
 			case CIL_CONS_U1:
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 5/5] libsepol: cil: Add cil_constraint_expr_to_policy()
  2016-09-08 20:30       ` [PATCH 4/5] libsepol: cil: Fix CIL_OP data assignment Daniel Cashman
@ 2016-09-08 20:30         ` Daniel Cashman
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:30 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv, dcashman

From: dcashman <dcashman@android.com>

The current cil_expr_to_policy() does not properly hanlde the case where
CIL_OP is at the beginning of an expression.  Create a new function,
cil_constraint_expr_to_policy() rather than modifying the original,
since the expression syntax for constraint expressions requires this
ability, but the existing cil_expr_to_policy() function has many other
consumers.

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 libsepol/cil/src/cil_policy.c | 211 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 210 insertions(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_policy.c b/libsepol/cil/src/cil_policy.c
index 32b6b41..e5ca091 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -84,6 +84,8 @@ struct cil_args_booleanif {
 
 int cil_expr_to_policy(FILE **file_arr, uint32_t file_index, struct cil_list *expr);
 
+int cil_constraint_expr_to_policy(FILE **file_arr, uint32_t file_index, struct cil_list *expr);
+
 int cil_combine_policy(FILE **file_arr, FILE *policy_file)
 {
 	char temp[BUFFER];
@@ -515,7 +517,7 @@ void cil_constrain_to_policy_helper(FILE **file_arr, char *kind, struct cil_list
 				fprintf(file_arr[CONSTRAINS], "%s %s", kind, cp->class->datum.name);
 				cil_perms_to_policy(file_arr, CONSTRAINS, cp->perms);
 				fprintf(file_arr[CONSTRAINS], "\n\t");
-				cil_expr_to_policy(file_arr, CONSTRAINS, expr);
+				cil_constraint_expr_to_policy(file_arr, CONSTRAINS, expr);
 				fprintf(file_arr[CONSTRAINS], ";\n");
 			} else { /* MAP */
 				struct cil_list_item *i = NULL;
@@ -811,6 +813,195 @@ exit:
 	return rc;
 }
 
+static int cil_constraint_expr_to_string(struct cil_list *expr, char **out)
+{
+	int rc = SEPOL_ERR;
+	struct cil_list_item *curr;
+	const size_t CONS_DEPTH = 3;
+	char *stack[CONS_DEPTH] = {}; // 1 operator + 1 - 2 operands
+	size_t pos = 0;
+	size_t i;
+
+	cil_list_for_each(curr, expr) {
+		if (pos >= CONS_DEPTH) {
+			rc = SEPOL_ERR;
+			goto exit;
+		}
+		switch (curr->flavor) {
+		case CIL_LIST:
+			rc = cil_constraint_expr_to_string(curr->data, &stack[pos]);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+			pos++;
+			break;
+		case CIL_STRING:
+			stack[pos] = strdup(curr->data);
+			if (!stack[pos]) {
+				cil_log(CIL_ERR, "OOM. Unable to convert cons_expr to string\n");
+				rc = SEPOL_ERR;
+				goto exit;
+			}
+			pos++;
+			break;
+		case CIL_DATUM:
+			stack[pos] = strdup(((struct cil_symtab_datum *)curr->data)->name);
+			if (!stack[pos]) {
+				cil_log(CIL_ERR, "OOM. Unable to convert cons_expr to string\n");
+				rc = SEPOL_ERR;
+				goto exit;
+			}
+			pos++;
+			break;
+		case CIL_OP: {
+			enum cil_flavor op_flavor = (enum cil_flavor)curr->data;
+			char *op_str = NULL;
+
+			if (pos != 0) {
+				/* ops come before the operand(s) */
+				cil_log(CIL_ERR, "CIL_OP encountered at incorrect offset\n");
+				rc = SEPOL_ERR;
+				goto exit;
+			}
+			switch (op_flavor) {
+			case CIL_AND:
+				op_str = CIL_KEY_AND;
+				break;
+			case CIL_OR:
+				op_str = CIL_KEY_OR;
+				break;
+			case CIL_NOT:
+				op_str = CIL_KEY_NOT;
+				break;
+			case CIL_ALL:
+				op_str = CIL_KEY_ALL;
+				break;
+			case CIL_EQ:
+				op_str = CIL_KEY_EQ;
+				break;
+			case CIL_NEQ:
+				op_str = CIL_KEY_NEQ;
+				break;
+			case CIL_XOR:
+				op_str = CIL_KEY_XOR;
+				break;
+			case CIL_CONS_DOM:
+				op_str = CIL_KEY_CONS_DOM;
+				break;
+			case CIL_CONS_DOMBY:
+				op_str = CIL_KEY_CONS_DOMBY;
+				break;
+			case CIL_CONS_INCOMP:
+				op_str = CIL_KEY_CONS_INCOMP;
+				break;
+			default:
+				cil_log(CIL_ERR, "Unknown operator in expression\n");
+				rc = SEPOL_ERR;
+				goto exit;
+				break;
+			}
+			stack[pos] = strdup(op_str);
+			if (!stack[pos]) {
+				cil_log(CIL_ERR, "OOM. Unable to convert cons_expr to string\n");
+				rc = SEPOL_ERR;
+				goto exit;
+			}
+			pos++;
+			break;
+		}
+		case CIL_CONS_OPERAND: {
+			enum cil_flavor operand_flavor = (enum cil_flavor)curr->data;
+			char *operand_str = NULL;
+			switch (operand_flavor) {
+			case CIL_CONS_U1:
+				operand_str = CIL_KEY_CONS_U1;
+				break;
+			case CIL_CONS_U2:
+				operand_str = CIL_KEY_CONS_U2;
+				break;
+			case CIL_CONS_U3:
+				operand_str = CIL_KEY_CONS_U3;
+				break;
+			case CIL_CONS_T1:
+				operand_str = CIL_KEY_CONS_T1;
+				break;
+			case CIL_CONS_T2:
+				operand_str = CIL_KEY_CONS_T2;
+				break;
+			case CIL_CONS_T3:
+				operand_str = CIL_KEY_CONS_T3;
+				break;
+			case CIL_CONS_R1:
+				operand_str = CIL_KEY_CONS_R1;
+				break;
+			case CIL_CONS_R2:
+				operand_str = CIL_KEY_CONS_R2;
+				break;
+			case CIL_CONS_R3:
+				operand_str = CIL_KEY_CONS_R3;
+				break;
+			case CIL_CONS_L1:
+				operand_str = CIL_KEY_CONS_L1;
+				break;
+			case CIL_CONS_L2:
+				operand_str = CIL_KEY_CONS_L2;
+				break;
+			case CIL_CONS_H1:
+				operand_str = CIL_KEY_CONS_H1;
+				break;
+			case CIL_CONS_H2:
+				operand_str = CIL_KEY_CONS_H2;
+				break;
+			default:
+				cil_log(CIL_ERR, "Unknown operand in expression\n");
+				goto exit;
+				break;
+			}
+			stack[pos] = strdup(operand_str);
+			if (!stack[pos]) {
+				cil_log(CIL_ERR, "OOM. Unable to convert cons_expr to string\n");
+				rc = SEPOL_ERR;
+				goto exit;
+			}
+			pos++;
+			break;
+		}
+		default:
+			cil_log(CIL_ERR, "Unknown flavor in expression\n");
+			rc = SEPOL_ERR;
+			goto exit;
+			break;
+		}
+	}
+	if (pos > 3) {
+		cil_log(CIL_ERR, "Illegal CIL expr: more than 3 components\n");
+		rc = SEPOL_ERR;
+		goto exit;
+	}
+	size_t len;
+	char *expr_str;
+	if (!strcmp(stack[0], CIL_KEY_NOT)) {
+		/* All ops take 2 operands except for CIL_KEY_NOT */
+		len = strlen(stack[0]) + strlen(stack[1]) + 4;
+		expr_str = cil_malloc(len);
+		snprintf(expr_str, len, "(%s %s)", stack[0], stack[1]);
+		/* free() done below */
+	} else {
+		len = strlen(stack[0]) + strlen(stack[1]) + strlen(stack[2]) + 5;
+		expr_str = cil_malloc(len);
+		snprintf(expr_str, len, "(%s %s %s)", stack[1], stack[0], stack[2]);
+		/* free() done below */
+	}
+	*out = expr_str;
+	rc = SEPOL_OK;
+exit:
+	for (i = 0; i < pos; i++) {
+		free(stack[i]);
+		stack[i] = NULL;
+	}
+	return rc;
+}
+
 int cil_expr_to_policy(FILE **file_arr, uint32_t file_index, struct cil_list *expr)
 {
 	int rc = SEPOL_ERR;
@@ -829,6 +1020,24 @@ out:
 	return rc;
 }
 
+int cil_constraint_expr_to_policy(FILE **file_arr, uint32_t file_index, struct cil_list *expr)
+{
+	int rc = SEPOL_ERR;
+	char *str_out;
+
+	rc = cil_constraint_expr_to_string(expr, &str_out);
+	if (rc != SEPOL_OK) {
+		goto out;
+	}
+	fprintf(file_arr[file_index], "%s", str_out);
+	free(str_out);
+
+	return SEPOL_OK;
+
+out:
+	return rc;
+}
+
 int __cil_booleanif_node_helper(struct cil_tree_node *node, __attribute__((unused)) uint32_t *finished, void *extra_args)
 {
 	int rc = SEPOL_ERR;
-- 
2.8.0.rc3.226.g39d4020

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

* Re: [PATCH 0/5] Fix some cil_gen_policy() bugs.
  2016-09-08 20:30 [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
  2016-09-08 20:30 ` [PATCH 1/5] libsepol: cil: Add userrole mapping to cil_gen_policy() Daniel Cashman
@ 2016-09-08 20:37 ` Daniel Cashman
  2016-09-09 12:29   ` James Carter
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel Cashman @ 2016-09-08 20:37 UTC (permalink / raw)
  To: selinux; +Cc: sds, jwcart2, jeffv

On 09/08/2016 01:30 PM, Daniel Cashman wrote:
> From: dcashman <dcashman@android.com>
> 
> cil_gen_policy() appears to exist to generate a policy.conf corresponding to the
> original SELinux HLL from a cil_db struct.  All of libsepol/cil/src/cil_policy.c
> appears to exist to support this functionality.  This patchset provides some
> fixes for issues encountered when trying to go from android's policy.conf to a
> CIL representation (via checkpolicy) and then back to the HLL representation via
> cil_gen_policy().
> 
> dcashman (5):
>   libsepol: cil: Add userrole mapping to cil_gen_policy().
>   libsepol: cil: Remove duplicate sid policy declaration.
>   libsepol: cil: Replace sensitivityorder statement.
>   libsepol: cil: Fix CIL_OP data assignment.
>   libsepol: cil: Add cil_constraint_expr_to_policy()
> 
>  libsepol/cil/src/cil_policy.c | 235 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 224 insertions(+), 11 deletions(-)
> 

I suspect that the "proper" fix here is to just remove all of
libsepol/cil/src/cil_policy.c, so I can put that patch together too if
desired.

The patches in this patchset do not address all of the bugs I
encountered trying to go from HLL -> CIL -> HLL. Since I was using this
as a temporary work-around, I decided to move on and submit these, in
case rescuing cil_gen_policy() is desired; the additional changes needed
were becoming more invasive (similar to the 5th patch in this set) and
less bug-fix-like.

Thank You,
Dan

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

* Re: [PATCH 0/5] Fix some cil_gen_policy() bugs.
  2016-09-08 20:37 ` [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
@ 2016-09-09 12:29   ` James Carter
  2016-09-09 14:35     ` James Carter
  0 siblings, 1 reply; 10+ messages in thread
From: James Carter @ 2016-09-09 12:29 UTC (permalink / raw)
  To: Daniel Cashman, selinux; +Cc: sds, jeffv

On 09/08/2016 04:37 PM, Daniel Cashman wrote:
> On 09/08/2016 01:30 PM, Daniel Cashman wrote:
>> From: dcashman <dcashman@android.com>
>>
>> cil_gen_policy() appears to exist to generate a policy.conf corresponding to the
>> original SELinux HLL from a cil_db struct.  All of libsepol/cil/src/cil_policy.c
>> appears to exist to support this functionality.  This patchset provides some
>> fixes for issues encountered when trying to go from android's policy.conf to a
>> CIL representation (via checkpolicy) and then back to the HLL representation via
>> cil_gen_policy().
>>
>> dcashman (5):
>>   libsepol: cil: Add userrole mapping to cil_gen_policy().
>>   libsepol: cil: Remove duplicate sid policy declaration.
>>   libsepol: cil: Replace sensitivityorder statement.
>>   libsepol: cil: Fix CIL_OP data assignment.
>>   libsepol: cil: Add cil_constraint_expr_to_policy()
>>
>>  libsepol/cil/src/cil_policy.c | 235 ++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 224 insertions(+), 11 deletions(-)
>>
>
> I suspect that the "proper" fix here is to just remove all of
> libsepol/cil/src/cil_policy.c, so I can put that patch together too if
> desired.
>

Yes, that code was used early on to help with debugging the CIL compiler, but 
hasn't been maintained. I've wanted to go back and fix it, but there didn't seem 
to be any use case needing it before now.

If that functionality would be valuable to you, I would be glad to work on this.

I think the right course would be to move this out of libsepol like secilc is.

Jim

> The patches in this patchset do not address all of the bugs I
> encountered trying to go from HLL -> CIL -> HLL. Since I was using this
> as a temporary work-around, I decided to move on and submit these, in
> case rescuing cil_gen_policy() is desired; the additional changes needed
> were becoming more invasive (similar to the 5th patch in this set) and
> less bug-fix-like.
>
> Thank You,
> Dan
>


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

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

* Re: [PATCH 0/5] Fix some cil_gen_policy() bugs.
  2016-09-09 12:29   ` James Carter
@ 2016-09-09 14:35     ` James Carter
  2016-09-09 17:10       ` Daniel Cashman
  0 siblings, 1 reply; 10+ messages in thread
From: James Carter @ 2016-09-09 14:35 UTC (permalink / raw)
  To: Daniel Cashman, selinux; +Cc: sds

On 09/09/2016 08:29 AM, James Carter wrote:
> On 09/08/2016 04:37 PM, Daniel Cashman wrote:
>> On 09/08/2016 01:30 PM, Daniel Cashman wrote:
>>> From: dcashman <dcashman@android.com>
>>>
>>> cil_gen_policy() appears to exist to generate a policy.conf corresponding to the
>>> original SELinux HLL from a cil_db struct.  All of libsepol/cil/src/cil_policy.c
>>> appears to exist to support this functionality.  This patchset provides some
>>> fixes for issues encountered when trying to go from android's policy.conf to a
>>> CIL representation (via checkpolicy) and then back to the HLL representation via
>>> cil_gen_policy().
>>>
>>> dcashman (5):
>>>   libsepol: cil: Add userrole mapping to cil_gen_policy().
>>>   libsepol: cil: Remove duplicate sid policy declaration.
>>>   libsepol: cil: Replace sensitivityorder statement.
>>>   libsepol: cil: Fix CIL_OP data assignment.
>>>   libsepol: cil: Add cil_constraint_expr_to_policy()
>>>
>>>  libsepol/cil/src/cil_policy.c | 235 ++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 224 insertions(+), 11 deletions(-)
>>>
>>
>> I suspect that the "proper" fix here is to just remove all of
>> libsepol/cil/src/cil_policy.c, so I can put that patch together too if
>> desired.
>>
>
> Yes, that code was used early on to help with debugging the CIL compiler, but
> hasn't been maintained. I've wanted to go back and fix it, but there didn't seem
> to be any use case needing it before now.
>
> If that functionality would be valuable to you, I would be glad to work on this.
>
> I think the right course would be to move this out of libsepol like secilc is.
>

The caffeine hadn't kicked in yet. cil_policy.c is like cil_binary.c and should 
stay where it is.

Jim

> Jim
>
>> The patches in this patchset do not address all of the bugs I
>> encountered trying to go from HLL -> CIL -> HLL. Since I was using this
>> as a temporary work-around, I decided to move on and submit these, in
>> case rescuing cil_gen_policy() is desired; the additional changes needed
>> were becoming more invasive (similar to the 5th patch in this set) and
>> less bug-fix-like.
>>
>> Thank You,
>> Dan
>>
>
>


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

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

* Re: [PATCH 0/5] Fix some cil_gen_policy() bugs.
  2016-09-09 14:35     ` James Carter
@ 2016-09-09 17:10       ` Daniel Cashman
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Cashman @ 2016-09-09 17:10 UTC (permalink / raw)
  To: James Carter, selinux; +Cc: sds

On 09/09/2016 07:35 AM, James Carter wrote:
> On 09/09/2016 08:29 AM, James Carter wrote:
>> On 09/08/2016 04:37 PM, Daniel Cashman wrote:
>>> On 09/08/2016 01:30 PM, Daniel Cashman wrote:
>>>> From: dcashman <dcashman@android.com>
>>>>
>>>> cil_gen_policy() appears to exist to generate a policy.conf
>>>> corresponding to the
>>>> original SELinux HLL from a cil_db struct.  All of
>>>> libsepol/cil/src/cil_policy.c
>>>> appears to exist to support this functionality.  This patchset
>>>> provides some
>>>> fixes for issues encountered when trying to go from android's
>>>> policy.conf to a
>>>> CIL representation (via checkpolicy) and then back to the HLL
>>>> representation via
>>>> cil_gen_policy().
>>>>
>>>> dcashman (5):
>>>>   libsepol: cil: Add userrole mapping to cil_gen_policy().
>>>>   libsepol: cil: Remove duplicate sid policy declaration.
>>>>   libsepol: cil: Replace sensitivityorder statement.
>>>>   libsepol: cil: Fix CIL_OP data assignment.
>>>>   libsepol: cil: Add cil_constraint_expr_to_policy()
>>>>
>>>>  libsepol/cil/src/cil_policy.c | 235
>>>> ++++++++++++++++++++++++++++++++++++++++--
>>>>  1 file changed, 224 insertions(+), 11 deletions(-)
>>>>
>>>
>>> I suspect that the "proper" fix here is to just remove all of
>>> libsepol/cil/src/cil_policy.c, so I can put that patch together too if
>>> desired.
>>>
>>
>> Yes, that code was used early on to help with debugging the CIL
>> compiler, but
>> hasn't been maintained. I've wanted to go back and fix it, but there
>> didn't seem
>> to be any use case needing it before now.
>>
>> If that functionality would be valuable to you, I would be glad to
>> work on this.
>>
>> I think the right course would be to move this out of libsepol like
>> secilc is.
>>
> 
> The caffeine hadn't kicked in yet. cil_policy.c is like cil_binary.c and
> should stay where it is.
> 
> Jim
> 

Yes, it requires access to the cil_db internals, most-importantly the
ast.  I'm trying to do similar processing to replace types and
attributes to new attributes (what I'm calling 'attributizing') for
portions of policy.  Thus, I think any changes I make will also have to
live in libsepol, although we'll see eventually how acceptable they are
for upstream.

As for the usefulness of cil_gen_policy(), my actual desire was to get
some CIL -> CIL code, perhaps a cil_write_ast() used as part of a
cil_gen_cil() function, that would allow me to make some AST
modifications and then produce transformed CIL policy.  I noticed
cil_gen_policy() as a potential shortcut to allow me to postpone that
further.  I don't currently see a need for cil_gen_policy() outside of
testing other changes, so I submitted the fixes I'd come up with before
deciding to continue with another approach.

Dan
>> Jim
>>
>>> The patches in this patchset do not address all of the bugs I
>>> encountered trying to go from HLL -> CIL -> HLL. Since I was using this
>>> as a temporary work-around, I decided to move on and submit these, in
>>> case rescuing cil_gen_policy() is desired; the additional changes needed
>>> were becoming more invasive (similar to the 5th patch in this set) and
>>> less bug-fix-like.
>>>
>>> Thank You,
>>> Dan
>>>
>>
>>
> 
> 

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

end of thread, other threads:[~2016-09-09 17:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-08 20:30 [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
2016-09-08 20:30 ` [PATCH 1/5] libsepol: cil: Add userrole mapping to cil_gen_policy() Daniel Cashman
2016-09-08 20:30   ` [PATCH 2/5] libsepol: cil: Remove duplicate sid policy declaration Daniel Cashman
2016-09-08 20:30     ` [PATCH 3/5] libsepol: cil: Replace sensitivityorder statement Daniel Cashman
2016-09-08 20:30       ` [PATCH 4/5] libsepol: cil: Fix CIL_OP data assignment Daniel Cashman
2016-09-08 20:30         ` [PATCH 5/5] libsepol: cil: Add cil_constraint_expr_to_policy() Daniel Cashman
2016-09-08 20:37 ` [PATCH 0/5] Fix some cil_gen_policy() bugs Daniel Cashman
2016-09-09 12:29   ` James Carter
2016-09-09 14:35     ` James Carter
2016-09-09 17:10       ` Daniel Cashman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).