[PATCH 0/3] Fix problems with CIL's handling of anonymous call arguments

[PATCH 0/3] Fix problems with CIL's handling of anonymous call arguments

[James Carter]

For more information on anonymous call arguments, see the third patch.

The first two patches fix a couple of bugs in the handling of anonymous
call arguments.

The last patch adds a test policy that can be used to test the handling
of anonymouse call arguments.

James Carter (3):
  libsepol/cil: Fix anonymous IP address call arguments
  libsepol/cil: Account for anonymous category sets in an expression
  secilc/test: Add test for anonymous args

 libsepol/cil/src/cil_build_ast.c   |   4 --
 libsepol/cil/src/cil_resolve_ast.c |  47 +++++++------
 secilc/test/anonymous_arg_test.cil | 106 +++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 24 deletions(-)
 create mode 100644 secilc/test/anonymous_arg_test.cil

-- 
2.26.3

[PATCH 1/3] libsepol/cil: Fix anonymous IP address call arguments

[James Carter]

A named IP address (using an ipaddr rule) could be passed as an
argument, but trying to pass an actual IP address caused an error.

As an exmample, consider the following portion of a policy.
  (macro m4 ((ipaddr ip)(ipaddr nm))
    (nodecon ip nm (USER ROLE TYPE ((s0) (s0))))
  )
  (ipaddr nm1 255.255.255.0)
  (ipaddr ip1 1.2.3.4)
  (call m4 (ip1 nm1)) ; This works
  (call m4 (1.2.3.4 255.255.255.0)) ; This doesn't

Allow actual IP addresses to be passed as a call argument. Now the
second call works as well.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_build_ast.c   |  4 ----
 libsepol/cil/src/cil_resolve_ast.c | 23 ++++++++++-------------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 71f14e20..538df279 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -5642,10 +5642,6 @@ int cil_fill_ipaddr(struct cil_tree_node *addr_node, struct cil_ipaddr *addr)
 		goto exit;
 	}
 
-	if (addr_node->cl_head != NULL ||  addr_node->next != NULL) {
-		goto exit;
-	}
-
 	if (strchr(addr_node->data, '.') != NULL) {
 		addr->family = AF_INET;
 	} else {
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index 77ffe0ff..16c8c753 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -3024,14 +3024,18 @@ static int cil_build_call_args(struct cil_tree_node *call_node, struct cil_call
 			break;
 		}
 		case CIL_IPADDR: {
-			if (arg_node->cl_head != NULL) {
+			if (arg_node->data == NULL) {
+				cil_tree_log(call_node, CIL_ERR, "Invalid macro parameter");
+				cil_destroy_args(arg);
+				rc = SEPOL_ERR;
+				goto exit;
+			} else if (strchr(arg_node->data, '.') || strchr(arg_node->data, ':')) {
 				struct cil_ipaddr *ipaddr = NULL;
 				struct cil_tree_node *addr_node = NULL;
 				cil_ipaddr_init(&ipaddr);
-
-				rc = cil_fill_ipaddr(arg_node->cl_head, ipaddr);
+				rc = cil_fill_ipaddr(arg_node, ipaddr);
 				if (rc != SEPOL_OK) {
-					cil_log(CIL_ERR, "Failed to create anonymous ip address, rc: %d\n", rc);
+					cil_tree_log(call_node, CIL_ERR, "Failed to create anonymous ip address");
 					cil_destroy_ipaddr(ipaddr);
 					cil_destroy_args(arg);
 					goto exit;
@@ -3039,18 +3043,11 @@ static int cil_build_call_args(struct cil_tree_node *call_node, struct cil_call
 				cil_tree_node_init(&addr_node);
 				addr_node->flavor = CIL_IPADDR;
 				addr_node->data = ipaddr;
-				cil_list_append(((struct cil_symtab_datum*)ipaddr)->nodes,
-								CIL_LIST_ITEM, addr_node);
-				arg->arg = (struct cil_symtab_datum*)ipaddr;
-			} else if (arg_node->data == NULL) {
-				cil_tree_log(call_node, CIL_ERR, "Invalid macro parameter");
-				cil_destroy_args(arg);
-				rc = SEPOL_ERR;
-				goto exit;
+				cil_list_append(DATUM(ipaddr)->nodes, CIL_LIST_ITEM, addr_node);
+				arg->arg = DATUM(ipaddr);
 			} else {
 				arg->arg_str = arg_node->data;
 			}
-
 			break;
 		}
 		case CIL_CLASS:
-- 
2.26.3

[PATCH 2/3] libsepol/cil: Account for anonymous category sets in an expression

[James Carter]

It is possible for anonymous category sets to be in a category
expression if the expression has a macro parameter in it.
Unfortunately, anonymous category sets are not looked for when
resolving category expressions and a segfault will occur during
later processing if there was one.

As an example, consider the following portion of a policy.
  (macro m1 ((categoryset cs))
    (userlevel USER (s0 (cs)))
  )
  (call m1 ((c0 c1)))
This policy will cause a segault, because the categoryset datum
for the parameter cs is not seen as a categoryset and is treated
as a plain category.

When resolving an expression, check whether or not the datum that
is found is actually an anonymous category set associated with a
macro parameter. If it is, then resolve the category set if it
has not already been resolved and treat its categories as a sub
expression.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_resolve_ast.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index 16c8c753..42a58468 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -3346,6 +3346,7 @@ int cil_resolve_expr(enum cil_flavor expr_type, struct cil_list *str_expr, struc
 	struct cil_list_item *curr;
 	struct cil_symtab_datum *res_datum = NULL;
 	enum cil_sym_index sym_index =  CIL_SYM_UNKNOWN;
+	struct cil_list *datum_sub_expr;
 
 	switch (str_expr->flavor) {
 	case CIL_BOOL:
@@ -3379,18 +3380,26 @@ int cil_resolve_expr(enum cil_flavor expr_type, struct cil_list *str_expr, struc
 			if (rc != SEPOL_OK) {
 				goto exit;
 			}
-
-			if (sym_index == CIL_SYM_TYPES && (expr_type == CIL_CONSTRAIN || expr_type == CIL_VALIDATETRANS)) {
-				cil_type_used(res_datum, CIL_ATTR_CONSTRAINT);
+			if (sym_index == CIL_SYM_CATS && NODE(res_datum)->flavor == CIL_CATSET) {
+				struct cil_catset *catset = (struct cil_catset *)res_datum;
+				if (!catset->cats->datum_expr) {
+					rc = cil_resolve_expr(expr_type, catset->cats->str_expr, &catset->cats->datum_expr, parent, extra_args);
+					if (rc != SEPOL_OK) {
+						goto exit;
+					}
+				}
+				cil_copy_list(catset->cats->datum_expr, &datum_sub_expr);
+				cil_list_append(*datum_expr, CIL_LIST, datum_sub_expr);
+			} else {
+				if (sym_index == CIL_SYM_TYPES && (expr_type == CIL_CONSTRAIN || expr_type == CIL_VALIDATETRANS)) {
+					cil_type_used(res_datum, CIL_ATTR_CONSTRAINT);
+				}
+				cil_list_append(*datum_expr, CIL_DATUM, res_datum);
 			}
-
-			cil_list_append(*datum_expr, CIL_DATUM, res_datum);
 			break;
 		case CIL_LIST: {
-			struct cil_list *datum_sub_expr;
 			rc = cil_resolve_expr(expr_type, curr->data, &datum_sub_expr, parent, extra_args);
 			if (rc != SEPOL_OK) {
-				cil_list_destroy(&datum_sub_expr, CIL_TRUE);
 				goto exit;
 			}
 			cil_list_append(*datum_expr, CIL_LIST, datum_sub_expr);
@@ -3404,6 +3413,7 @@ int cil_resolve_expr(enum cil_flavor expr_type, struct cil_list *str_expr, struc
 	return SEPOL_OK;
 
 exit:
+	cil_list_destroy(datum_expr, CIL_FALSE);
 	return rc;
 }
 
-- 
2.26.3

[PATCH 3/3] secilc/test: Add test for anonymous args

[James Carter]

CIL has rules that allow names to be assigned to certain objects
like MLS category sets, MLS levels, MLS ranges, IP addresses, and
class permission sets. These objects can also be named as parameters
for a macro. A call may pass in a name for one of these objects, but
it also may pass in one of the actual objects. These objects are
referred as anonymous arguments.

Add CIL policy that can be used to test whether or not anonymous
arguments are being handled properly in macros. Also test the
equivalent named arguments to help determine if the problem is with
that argument type or just with an anonymous argument of that type.

The anonymouse arguments that are tested are categoryset, level,
levelrange, ipaddr, and classpermission.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 secilc/test/anonymous_arg_test.cil | 106 +++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 secilc/test/anonymous_arg_test.cil

diff --git a/secilc/test/anonymous_arg_test.cil b/secilc/test/anonymous_arg_test.cil
new file mode 100644
index 00000000..46f8ce73
--- /dev/null
+++ b/secilc/test/anonymous_arg_test.cil
@@ -0,0 +1,106 @@
+;; Test anonymous args
+
+(mls true)
+(class CLASS (PERM))
+(classorder (CLASS))
+(sid SID)
+(sidorder (SID))
+(user USER)
+(role ROLE)
+(type TYPE)
+(category CAT)
+(categoryorder (CAT))
+(sensitivity SENS)
+(sensitivityorder (SENS))
+(sensitivitycategory SENS (CAT))
+(allow TYPE self (CLASS (PERM)))
+(roletype ROLE TYPE)
+(userrole USER ROLE)
+(userlevel USER (SENS))
+(userrange USER ((SENS)(SENS (CAT))))
+(sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))
+
+(category c0)
+(category c1)
+(category c2)
+(category c3)
+(categoryorder (CAT c0 c1 c2 c3))
+(categoryset cs01 (c0 c1))
+(categoryset cs03 (range c0 c3))
+
+(sensitivity s0)
+(sensitivity s1)
+(sensitivity s2)
+(sensitivity s3)
+(sensitivityorder (SENS s0 s1 s2 s3))
+
+(sensitivitycategory s0 (cs01 c2 c3))
+(sensitivitycategory s1 (c0 c1 c2 c3))
+(sensitivitycategory s2 (c0 c1 c2 c3))
+(sensitivitycategory s3 (range c0 c3))
+
+(level lvl (s0 (c0)))
+(level lvl0 (s0))
+(level lvl3 (s3 (range c0 c3)))
+
+(levelrange rng ((s0) (s3 (c0 c1 c2 c3))))
+
+(user u1)
+(user u2)
+(user u3)
+(user u4)
+
+(userrole u1 ROLE)
+(userrole u2 ROLE)
+(userrole u3 ROLE)
+(userrole u4 ROLE)
+
+; Test categoryset
+(macro m1 ((user u)(sensitivity s)(categoryset cs))
+  (userlevel u (s (cs)))
+)
+(call m1 (u1 s1 (c0 c1)))
+(call m1 (u2 s2 cs01))
+
+; Test level
+(macro m2 ((user u)(level l))
+  (userlevel u l)
+)
+(call m2 (u3 (s3 (c2))))
+(call m2 (u4 lvl))
+
+; Test levelrange
+(macro m3 ((user u)(levelrange lr))
+  (userrange u lr)
+)
+(call m3 (u1 ((s0) (s3 (range c0 c3)))))
+(call m3 (u2 (lvl0 (s3 (cs03)))))
+(call m3 (u3 (lvl0 lvl3)))
+(call m3 (u4 rng))
+
+; Test ipaddr
+(macro m4 ((user u)(ipaddr nm)(ipaddr ip))
+  (nodecon ip nm (u ROLE TYPE ((s0) (s0))))
+)
+(ipaddr nm1 255.255.255.0)
+(ipaddr ip4 1.2.3.4)
+(call m4 (u1 nm1 192.25.35.200))
+(call m4 (u2 255.255.255.0 ip4))
+
+; Test classpermission
+(type t1)
+(type t2)
+(type t3)
+
+(classpermission cp1)
+(classpermissionset cp1 (CLASS (PERM)))
+
+(classmap cm1 (cm1p))
+(classmapping cm1 cm1p (CLASS (PERM)))
+
+(macro m5 ((type t)(classpermission cp))
+  (allow t self cp)
+)
+(call m5 (t1 (CLASS (PERM))))
+(call m5 (t2 cp1))
+(call m5 (t3 (cm1 (cm1p))))
-- 
2.26.3

Re: [PATCH 0/3] Fix problems with CIL's handling of anonymous call arguments

[Nicolas Iooss]

On Tue, Jun 15, 2021 at 8:57 PM James Carter <jwcart2@gmail.com> wrote:
>
> For more information on anonymous call arguments, see the third patch.
>
> The first two patches fix a couple of bugs in the handling of anonymous
> call arguments.
>
> The last patch adds a test policy that can be used to test the handling
> of anonymouse call arguments.
>
> James Carter (3):
>   libsepol/cil: Fix anonymous IP address call arguments
>   libsepol/cil: Account for anonymous category sets in an expression
>   secilc/test: Add test for anonymous args
>
>  libsepol/cil/src/cil_build_ast.c   |   4 --
>  libsepol/cil/src/cil_resolve_ast.c |  47 +++++++------
>  secilc/test/anonymous_arg_test.cil | 106 +++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+), 24 deletions(-)
>  create mode 100644 secilc/test/anonymous_arg_test.cil
>
> --
> 2.26.3

For these 3 patches:

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks!
Nicolas

Re: [PATCH 0/3] Fix problems with CIL's handling of anonymous call arguments

[James Carter]

On Sat, Jun 19, 2021 at 10:19 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Tue, Jun 15, 2021 at 8:57 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > For more information on anonymous call arguments, see the third patch.
> >
> > The first two patches fix a couple of bugs in the handling of anonymous
> > call arguments.
> >
> > The last patch adds a test policy that can be used to test the handling
> > of anonymouse call arguments.
> >
> > James Carter (3):
> >   libsepol/cil: Fix anonymous IP address call arguments
> >   libsepol/cil: Account for anonymous category sets in an expression
> >   secilc/test: Add test for anonymous args
> >
> >  libsepol/cil/src/cil_build_ast.c   |   4 --
> >  libsepol/cil/src/cil_resolve_ast.c |  47 +++++++------
> >  secilc/test/anonymous_arg_test.cil | 106 +++++++++++++++++++++++++++++
> >  3 files changed, 133 insertions(+), 24 deletions(-)
> >  create mode 100644 secilc/test/anonymous_arg_test.cil
> >
> > --
> > 2.26.3
>
> For these 3 patches:
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>

This series has been merged.
Jim

> Thanks!
> Nicolas
>