selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] checkpolicy: drop unused token CLONE
@ 2023-05-12  9:23 Christian Göttsche
  2023-05-12  9:23 ` [PATCH 2/4] checkpolicy: reject condition with bool and tunable in expression Christian Göttsche
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Christian Göttsche @ 2023-05-12  9:23 UTC (permalink / raw)
  To: selinux

The token CLONE is never used in the grammar; drop it.

As side effect `clone` and `CLONE` become available as identifier names.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_parse.y | 1 -
 checkpolicy/policy_scan.l  | 2 --
 2 files changed, 3 deletions(-)

diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
index 45f973ff..da32a776 100644
--- a/checkpolicy/policy_parse.y
+++ b/checkpolicy/policy_parse.y
@@ -85,7 +85,6 @@ typedef int (* require_func_t)(int pass);
 %token PATH
 %token QPATH
 %token FILENAME
-%token CLONE
 %token COMMON
 %token CLASS
 %token CONSTRAIN
diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
index 9fefea7b..2c025b61 100644
--- a/checkpolicy/policy_scan.l
+++ b/checkpolicy/policy_scan.l
@@ -77,8 +77,6 @@ hexval	[0-9A-Fa-f]
 				      source_lineno++;
 				  yyless(1);
 				}
-CLONE |
-clone				{ return(CLONE); }
 COMMON |
 common				{ return(COMMON); }
 CLASS |
-- 
2.40.1


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

* [PATCH 2/4] checkpolicy: reject condition with bool and tunable in expression
  2023-05-12  9:23 [PATCH 1/4] checkpolicy: drop unused token CLONE Christian Göttsche
@ 2023-05-12  9:23 ` Christian Göttsche
  2023-05-12  9:23 ` [PATCH 3/4] checkpolicy: only set declared permission bits for wildcards Christian Göttsche
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Christian Göttsche @ 2023-05-12  9:23 UTC (permalink / raw)
  To: selinux

If tunables are not preserved (the mode unconditionally used by
checkpolicy) an expression must not consist of booleans and tunables,
since such expressions are not supported during expansion (see expand.c:
discard_tunables()).

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index d4e376ad..95cd5c85 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -3964,8 +3964,9 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2)
 int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
 {
 	cond_expr_t *e;
-	int depth;
+	int depth, booleans, tunables;
 	cond_node_t cn, *cn_old;
+	const cond_bool_datum_t *bool_var;
 
 	/* expression cannot be NULL */
 	if (!expr) {
@@ -3990,6 +3991,8 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
 
 	/* verify expression */
 	depth = -1;
+	booleans = 0;
+	tunables = 0;
 	for (e = expr; e; e = e->next) {
 		switch (e->expr_type) {
 		case COND_NOT:
@@ -4018,6 +4021,14 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
 				return -1;
 			}
 			depth++;
+
+			bool_var = policydbp->bool_val_to_struct[e->boolean - 1];
+			if (bool_var->flags & COND_BOOL_FLAGS_TUNABLE) {
+				tunables = 1;
+			} else {
+				booleans = 1;
+			}
+
 			break;
 		default:
 			yyerror("illegal conditional expression");
@@ -4028,6 +4039,10 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
 		yyerror("illegal conditional expression");
 		return -1;
 	}
+	if (booleans && tunables) {
+		yyerror("illegal conditional expression; Contains boolean and tunable");
+		return -1;
+	}
 
 	/*  use tmp conditional node to partially build new node */
 	memset(&cn, 0, sizeof(cn));
-- 
2.40.1


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

* [PATCH 3/4] checkpolicy: only set declared permission bits for wildcards
  2023-05-12  9:23 [PATCH 1/4] checkpolicy: drop unused token CLONE Christian Göttsche
  2023-05-12  9:23 ` [PATCH 2/4] checkpolicy: reject condition with bool and tunable in expression Christian Göttsche
@ 2023-05-12  9:23 ` Christian Göttsche
  2023-05-12  9:23 ` [PATCH 4/4] libsepol: dump non-mls validatetrans rules as such Christian Göttsche
  2023-05-25 18:48 ` [PATCH 1/4] checkpolicy: drop unused token CLONE James Carter
  3 siblings, 0 replies; 6+ messages in thread
From: Christian Göttsche @ 2023-05-12  9:23 UTC (permalink / raw)
  To: selinux

When setting permission bits from a wildcard or complement only set the
bits for permissions actually declared for the associated class.  This
helps optimizing the policy later, since only rules are dropped with a
complete empty permission bitset.  Example policy:

    class CLASS1
    sid kernel
    class CLASS1 { PERM1 }
    type TYPE1;
    bool BOOL1 true;
    allow TYPE1 self : CLASS1 { PERM1 };
    role ROLE1;
    role ROLE1 types { TYPE1 };
    if ! BOOL1 { allow TYPE1 self: CLASS1 *; }
    user USER1 roles ROLE1;
    sid kernel USER1:ROLE1:TYPE1

Also emit a warning if a rule will have an empty permission bitset due
to an exhausting complement.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 95cd5c85..cef8f3c4 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -2511,6 +2511,8 @@ int define_te_avtab_extended_perms(int which)
 	return rc;
 }
 
+#define PERMISSION_MASK(nprim) ((nprim) == PERM_SYMTAB_SIZE ? (~UINT32_C(0)) : ((UINT32_C(1) << (nprim)) - 1))
+
 static int define_te_avtab_helper(int which, avrule_t ** rule)
 {
 	char *id;
@@ -2616,8 +2618,8 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 			cladatum = policydbp->class_val_to_struct[i];
 
 			if (strcmp(id, "*") == 0) {
-				/* set all permissions in the class */
-				cur_perms->data = ~0U;
+				/* set all declared permissions in the class */
+				cur_perms->data = PERMISSION_MASK(cladatum->permissions.nprim);
 				goto next;
 			}
 
@@ -2625,7 +2627,16 @@ static int define_te_avtab_helper(int which, avrule_t ** rule)
 				/* complement the set */
 				if (which == AVRULE_DONTAUDIT)
 					yywarn("dontaudit rule with a ~?");
-				cur_perms->data = ~cur_perms->data;
+				cur_perms->data = ~cur_perms->data & PERMISSION_MASK(cladatum->permissions.nprim);
+				if (cur_perms->data == 0) {
+					class_perm_node_t *tmp = cur_perms;
+					yywarn("omitting avrule with no permission set");
+					if (perms == cur_perms)
+						perms = cur_perms->next;
+					cur_perms = cur_perms->next;
+					free(tmp);
+					continue;
+				}
 				goto next;
 			}
 
@@ -3549,8 +3560,6 @@ static constraint_expr_t *constraint_expr_clone(const constraint_expr_t * expr)
 	return NULL;
 }
 
-#define PERMISSION_MASK(nprim) ((nprim) == PERM_SYMTAB_SIZE ? (~UINT32_C(0)) : ((UINT32_C(1) << (nprim)) - 1))
-
 int define_constraint(constraint_expr_t * expr)
 {
 	struct constraint_node *node;
-- 
2.40.1


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

* [PATCH 4/4] libsepol: dump non-mls validatetrans rules as such
  2023-05-12  9:23 [PATCH 1/4] checkpolicy: drop unused token CLONE Christian Göttsche
  2023-05-12  9:23 ` [PATCH 2/4] checkpolicy: reject condition with bool and tunable in expression Christian Göttsche
  2023-05-12  9:23 ` [PATCH 3/4] checkpolicy: only set declared permission bits for wildcards Christian Göttsche
@ 2023-05-12  9:23 ` Christian Göttsche
  2023-05-25 18:48 ` [PATCH 1/4] checkpolicy: drop unused token CLONE James Carter
  3 siblings, 0 replies; 6+ messages in thread
From: Christian Göttsche @ 2023-05-12  9:23 UTC (permalink / raw)
  To: selinux

The functions constraint_expr_to_str() prepare a string representation
for validatetrans and mlsvalidatetrans rules.  To decide what keyword to
use the type of expression is consulted.  Currently the extra target
type (CEXPR_XTARGET) is considered to be an MLS statement while its not,
e.g.:

    validatetrans CLASS1 t3 == ATTR1;

Actually check for MLS expression types only.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/kernel_to_cil.c  | 2 +-
 libsepol/src/kernel_to_conf.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
index e9cd89c2..7e279e3f 100644
--- a/libsepol/src/kernel_to_cil.c
+++ b/libsepol/src/kernel_to_cil.c
@@ -172,7 +172,7 @@ static char *constraint_expr_to_str(struct policydb *pdb, struct constraint_expr
 				goto exit;
 			}
 
-			if (curr->attr >= CEXPR_XTARGET) {
+			if (curr->attr >= CEXPR_L1L2) {
 				*use_mls = 1;
 			}
 
diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index c48a7114..4c93cc10 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -169,7 +169,7 @@ static char *constraint_expr_to_str(struct policydb *pdb, struct constraint_expr
 				goto exit;
 			}
 
-			if (curr->attr >= CEXPR_XTARGET) {
+			if (curr->attr >= CEXPR_L1L2) {
 				*use_mls = 1;
 			}
 
-- 
2.40.1


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

* Re: [PATCH 1/4] checkpolicy: drop unused token CLONE
  2023-05-12  9:23 [PATCH 1/4] checkpolicy: drop unused token CLONE Christian Göttsche
                   ` (2 preceding siblings ...)
  2023-05-12  9:23 ` [PATCH 4/4] libsepol: dump non-mls validatetrans rules as such Christian Göttsche
@ 2023-05-25 18:48 ` James Carter
  2023-06-05 20:08   ` James Carter
  3 siblings, 1 reply; 6+ messages in thread
From: James Carter @ 2023-05-25 18:48 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Fri, May 12, 2023 at 5:24 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> The token CLONE is never used in the grammar; drop it.
>
> As side effect `clone` and `CLONE` become available as identifier names.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

For these four patches:
Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  checkpolicy/policy_parse.y | 1 -
>  checkpolicy/policy_scan.l  | 2 --
>  2 files changed, 3 deletions(-)
>
> diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
> index 45f973ff..da32a776 100644
> --- a/checkpolicy/policy_parse.y
> +++ b/checkpolicy/policy_parse.y
> @@ -85,7 +85,6 @@ typedef int (* require_func_t)(int pass);
>  %token PATH
>  %token QPATH
>  %token FILENAME
> -%token CLONE
>  %token COMMON
>  %token CLASS
>  %token CONSTRAIN
> diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
> index 9fefea7b..2c025b61 100644
> --- a/checkpolicy/policy_scan.l
> +++ b/checkpolicy/policy_scan.l
> @@ -77,8 +77,6 @@ hexval        [0-9A-Fa-f]
>                                       source_lineno++;
>                                   yyless(1);
>                                 }
> -CLONE |
> -clone                          { return(CLONE); }
>  COMMON |
>  common                         { return(COMMON); }
>  CLASS |
> --
> 2.40.1
>

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

* Re: [PATCH 1/4] checkpolicy: drop unused token CLONE
  2023-05-25 18:48 ` [PATCH 1/4] checkpolicy: drop unused token CLONE James Carter
@ 2023-06-05 20:08   ` James Carter
  0 siblings, 0 replies; 6+ messages in thread
From: James Carter @ 2023-06-05 20:08 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Thu, May 25, 2023 at 2:48 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Fri, May 12, 2023 at 5:24 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > The token CLONE is never used in the grammar; drop it.
> >
> > As side effect `clone` and `CLONE` become available as identifier names.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> For these four patches:
> Acked-by: James Carter <jwcart2@gmail.com>
>
These four patches have been merged.
Thanks,
Jim

> > ---
> >  checkpolicy/policy_parse.y | 1 -
> >  checkpolicy/policy_scan.l  | 2 --
> >  2 files changed, 3 deletions(-)
> >
> > diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
> > index 45f973ff..da32a776 100644
> > --- a/checkpolicy/policy_parse.y
> > +++ b/checkpolicy/policy_parse.y
> > @@ -85,7 +85,6 @@ typedef int (* require_func_t)(int pass);
> >  %token PATH
> >  %token QPATH
> >  %token FILENAME
> > -%token CLONE
> >  %token COMMON
> >  %token CLASS
> >  %token CONSTRAIN
> > diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
> > index 9fefea7b..2c025b61 100644
> > --- a/checkpolicy/policy_scan.l
> > +++ b/checkpolicy/policy_scan.l
> > @@ -77,8 +77,6 @@ hexval        [0-9A-Fa-f]
> >                                       source_lineno++;
> >                                   yyless(1);
> >                                 }
> > -CLONE |
> > -clone                          { return(CLONE); }
> >  COMMON |
> >  common                         { return(COMMON); }
> >  CLASS |
> > --
> > 2.40.1
> >

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

end of thread, other threads:[~2023-06-05 20:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-12  9:23 [PATCH 1/4] checkpolicy: drop unused token CLONE Christian Göttsche
2023-05-12  9:23 ` [PATCH 2/4] checkpolicy: reject condition with bool and tunable in expression Christian Göttsche
2023-05-12  9:23 ` [PATCH 3/4] checkpolicy: only set declared permission bits for wildcards Christian Göttsche
2023-05-12  9:23 ` [PATCH 4/4] libsepol: dump non-mls validatetrans rules as such Christian Göttsche
2023-05-25 18:48 ` [PATCH 1/4] checkpolicy: drop unused token CLONE James Carter
2023-06-05 20:08   ` James Carter

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).