* [PATCH] libsepol/cil: Allow some duplicate macro and block declarations
@ 2021-08-11 21:04 James Carter
2021-08-12 7:53 ` Dominick Grift
2021-08-16 9:31 ` Nicolas Iooss
0 siblings, 2 replies; 6+ messages in thread
From: James Carter @ 2021-08-11 21:04 UTC (permalink / raw)
To: selinux; +Cc: dominick.grift, James Carter
The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
Check for duplicate blocks, optionals, and macros) added checks when
copying blocks, macros, and optionals so that a duplicate would cause
an exit with an error. Unfortunately, some policies exist that depend
on this behavior when using inheritance.
The behavior is as follows.
For macros only the first declared macro matters.
;
(macro m ((type ARG1))
(allow ARG1 self (CLASS (PERM1)))
)
(block b
(macro m ((type ARG1))
(allow ARG1 self (CLASS (PERM2)))
)
)
(blockinherit b)
(type t)
(call m (t))
;
For this policy segment, the macro m in block b will not be called.
Only the original macro m will be.
This behavior has been used to override macros that are going to
be inherited. Only the inherited macros that have not already been
declared in the destination namespace will be used.
Blocks seem to work fine even though there are two of them
;
(block b1
(blockinherit b2)
(block b
(type t1)
(allow t1 self (CLASS (PERM)))
)
)
(block b2
(block b
(type t2)
(allow t2 self (CLASS (PERM)))
)
)
(blockinherit b1)
;
In this example, the blockinherit of b2 will cause there to be
two block b's in block b1. Note that if both block b's tried to
declare the same type, then that would be an error. The blockinherit
of b1 will copy both block b's.
This behavior has been used to allow the use of in-statements for
a block that is being inherited. Since the in-statements are resolved
before block inheritance, this only works if a block with the same
name as the block to be inherited is declared in the namespace.
To support the use of these two behaviors, allow duplicate blocks
and macros when they occur as the result of block inheritance. In
any other circumstances and error for a redeclaration will be given.
Since the duplicate macro is not going to be used it is just skipped.
The duplicate block will use the datum of the original block. In both
cases a warning message will be produced (it will only be seen if
"-v" is used when compiling the policy).
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_copy_ast.c | 69 ++++++++++++++++++++++++---------
1 file changed, 50 insertions(+), 19 deletions(-)
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 9c0231f2..67be2ec8 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -43,6 +43,7 @@
#include "cil_verify.h"
struct cil_args_copy {
+ struct cil_tree_node *orig_dest;
struct cil_tree_node *dest;
struct cil_db *db;
};
@@ -101,17 +102,23 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
struct cil_block *orig = data;
char *key = orig->datum.name;
struct cil_symtab_datum *datum = NULL;
- struct cil_block *new;
cil_symtab_get_datum(symtab, key, &datum);
if (datum != NULL) {
- cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
- return SEPOL_ERR;
+ if (FLAVOR(datum) != CIL_BLOCK) {
+ cil_tree_log(NODE(orig), CIL_ERR, "Block %s being copied", key);
+ cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
+ return SEPOL_ERR;
+ }
+ cil_tree_log(NODE(orig), CIL_WARN, "Block %s being copied", key);
+ cil_tree_log(NODE(datum), CIL_WARN, " Previously declared", key);
+ *copy = datum;
+ } else {
+ struct cil_block *new;
+ cil_block_init(&new);
+ *copy = new;
}
- cil_block_init(&new);
- *copy = new;
-
return SEPOL_OK;
}
@@ -1511,21 +1518,26 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
struct cil_macro *orig = data;
char *key = orig->datum.name;
struct cil_symtab_datum *datum = NULL;
- struct cil_macro *new;
cil_symtab_get_datum(symtab, key, &datum);
if (datum != NULL) {
- cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
- return SEPOL_ERR;
- }
-
- cil_macro_init(&new);
- if (orig->params != NULL) {
- cil_copy_list(orig->params, &new->params);
+ if (FLAVOR(datum) != CIL_MACRO) {
+ cil_tree_log(NODE(orig), CIL_ERR, "Macro %s being copied", key);
+ cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
+ return SEPOL_ERR;
+ }
+ cil_tree_log(NODE(orig), CIL_WARN, "Skipping macro %s", key);
+ cil_tree_log(NODE(datum), CIL_WARN, " Previously declared");
+ *copy = NULL;
+ } else {
+ struct cil_macro *new;
+ cil_macro_init(&new);
+ if (orig->params != NULL) {
+ cil_copy_list(orig->params, &new->params);
+ }
+ *copy = new;
}
- *copy = new;
-
return SEPOL_OK;
}
@@ -1700,7 +1712,7 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
return SEPOL_OK;
}
-int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) uint32_t *finished, void *extra_args)
+int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void *extra_args)
{
int rc = SEPOL_ERR;
struct cil_tree_node *parent = NULL;
@@ -2005,6 +2017,16 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
rc = (*copy_func)(db, orig->data, &data, symtab);
if (rc == SEPOL_OK) {
+ if (orig->flavor == CIL_MACRO && data == NULL) {
+ /* Skipping macro re-declaration */
+ if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
+ cil_log(CIL_ERR, " Re-declaration of macro is only allowed when inheriting a block\n");
+ return SEPOL_ERR;
+ }
+ *finished = CIL_TREE_SKIP_HEAD;
+ return SEPOL_OK;
+ }
+
cil_tree_node_init(&new);
new->parent = parent;
@@ -2013,7 +2035,15 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
new->flavor = orig->flavor;
new->data = data;
- if (orig->flavor >= CIL_MIN_DECLARATIVE) {
+ if (orig->flavor == CIL_BLOCK && DATUM(data)->nodes->head != NULL) {
+ /* Duplicate block */
+ if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
+ cil_log(CIL_ERR, " Re-declaration of block is only allowed when inheriting a block\n");
+ rc = SEPOL_ERR;
+ goto exit;
+ }
+ cil_list_append(DATUM(new->data)->nodes, CIL_NODE, new);
+ } else if (orig->flavor >= CIL_MIN_DECLARATIVE) {
/* Check the flavor of data if was found in the destination symtab */
if (DATUM(data)->nodes->head && FLAVOR(data) != orig->flavor) {
cil_tree_log(orig, CIL_ERR, "Incompatible flavor when trying to copy %s", DATUM(data)->name);
@@ -2098,12 +2128,13 @@ int cil_copy_ast(struct cil_db *db, struct cil_tree_node *orig, struct cil_tree_
int rc = SEPOL_ERR;
struct cil_args_copy extra_args;
+ extra_args.orig_dest = dest;
extra_args.dest = dest;
extra_args.db = db;
rc = cil_tree_walk(orig, __cil_copy_node_helper, NULL, __cil_copy_last_child_helper, &extra_args);
if (rc != SEPOL_OK) {
- cil_log(CIL_INFO, "cil_tree_walk failed, rc: %d\n", rc);
+ cil_tree_log(dest, CIL_ERR, "Failed to copy %s to %s", cil_node_to_string(orig), cil_node_to_string(dest));
goto exit;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libsepol/cil: Allow some duplicate macro and block declarations
2021-08-11 21:04 [PATCH] libsepol/cil: Allow some duplicate macro and block declarations James Carter
@ 2021-08-12 7:53 ` Dominick Grift
2021-08-12 13:20 ` James Carter
2021-08-16 9:31 ` Nicolas Iooss
1 sibling, 1 reply; 6+ messages in thread
From: Dominick Grift @ 2021-08-12 7:53 UTC (permalink / raw)
To: James Carter; +Cc: selinux
James Carter <jwcart2@gmail.com> writes:
> The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
> Check for duplicate blocks, optionals, and macros) added checks when
> copying blocks, macros, and optionals so that a duplicate would cause
> an exit with an error. Unfortunately, some policies exist that depend
> on this behavior when using inheritance.
Thanks. This resolved my issues.
We should probably document this behavior.
>
> The behavior is as follows.
>
> For macros only the first declared macro matters.
> ;
> (macro m ((type ARG1))
> (allow ARG1 self (CLASS (PERM1)))
> )
> (block b
> (macro m ((type ARG1))
> (allow ARG1 self (CLASS (PERM2)))
> )
> )
> (blockinherit b)
> (type t)
> (call m (t))
> ;
> For this policy segment, the macro m in block b will not be called.
> Only the original macro m will be.
>
> This behavior has been used to override macros that are going to
> be inherited. Only the inherited macros that have not already been
> declared in the destination namespace will be used.
>
> Blocks seem to work fine even though there are two of them
> ;
> (block b1
> (blockinherit b2)
> (block b
> (type t1)
> (allow t1 self (CLASS (PERM)))
> )
> )
> (block b2
> (block b
> (type t2)
> (allow t2 self (CLASS (PERM)))
> )
> )
> (blockinherit b1)
> ;
> In this example, the blockinherit of b2 will cause there to be
> two block b's in block b1. Note that if both block b's tried to
> declare the same type, then that would be an error. The blockinherit
> of b1 will copy both block b's.
>
> This behavior has been used to allow the use of in-statements for
> a block that is being inherited. Since the in-statements are resolved
> before block inheritance, this only works if a block with the same
> name as the block to be inherited is declared in the namespace.
>
> To support the use of these two behaviors, allow duplicate blocks
> and macros when they occur as the result of block inheritance. In
> any other circumstances and error for a redeclaration will be given.
>
> Since the duplicate macro is not going to be used it is just skipped.
> The duplicate block will use the datum of the original block. In both
> cases a warning message will be produced (it will only be seen if
> "-v" is used when compiling the policy).
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil_copy_ast.c | 69 ++++++++++++++++++++++++---------
> 1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index 9c0231f2..67be2ec8 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -43,6 +43,7 @@
> #include "cil_verify.h"
>
> struct cil_args_copy {
> + struct cil_tree_node *orig_dest;
> struct cil_tree_node *dest;
> struct cil_db *db;
> };
> @@ -101,17 +102,23 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
> struct cil_block *orig = data;
> char *key = orig->datum.name;
> struct cil_symtab_datum *datum = NULL;
> - struct cil_block *new;
>
> cil_symtab_get_datum(symtab, key, &datum);
> if (datum != NULL) {
> - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> - return SEPOL_ERR;
> + if (FLAVOR(datum) != CIL_BLOCK) {
> + cil_tree_log(NODE(orig), CIL_ERR, "Block %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> + return SEPOL_ERR;
> + }
> + cil_tree_log(NODE(orig), CIL_WARN, "Block %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared", key);
> + *copy = datum;
> + } else {
> + struct cil_block *new;
> + cil_block_init(&new);
> + *copy = new;
> }
>
> - cil_block_init(&new);
> - *copy = new;
> -
> return SEPOL_OK;
> }
>
> @@ -1511,21 +1518,26 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
> struct cil_macro *orig = data;
> char *key = orig->datum.name;
> struct cil_symtab_datum *datum = NULL;
> - struct cil_macro *new;
>
> cil_symtab_get_datum(symtab, key, &datum);
> if (datum != NULL) {
> - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> - return SEPOL_ERR;
> - }
> -
> - cil_macro_init(&new);
> - if (orig->params != NULL) {
> - cil_copy_list(orig->params, &new->params);
> + if (FLAVOR(datum) != CIL_MACRO) {
> + cil_tree_log(NODE(orig), CIL_ERR, "Macro %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> + return SEPOL_ERR;
> + }
> + cil_tree_log(NODE(orig), CIL_WARN, "Skipping macro %s", key);
> + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared");
> + *copy = NULL;
> + } else {
> + struct cil_macro *new;
> + cil_macro_init(&new);
> + if (orig->params != NULL) {
> + cil_copy_list(orig->params, &new->params);
> + }
> + *copy = new;
> }
>
> - *copy = new;
> -
> return SEPOL_OK;
> }
>
> @@ -1700,7 +1712,7 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
> return SEPOL_OK;
> }
>
> -int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) uint32_t *finished, void *extra_args)
> +int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void *extra_args)
> {
> int rc = SEPOL_ERR;
> struct cil_tree_node *parent = NULL;
> @@ -2005,6 +2017,16 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
>
> rc = (*copy_func)(db, orig->data, &data, symtab);
> if (rc == SEPOL_OK) {
> + if (orig->flavor == CIL_MACRO && data == NULL) {
> + /* Skipping macro re-declaration */
> + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> + cil_log(CIL_ERR, " Re-declaration of macro is only allowed when inheriting a block\n");
> + return SEPOL_ERR;
> + }
> + *finished = CIL_TREE_SKIP_HEAD;
> + return SEPOL_OK;
> + }
> +
> cil_tree_node_init(&new);
>
> new->parent = parent;
> @@ -2013,7 +2035,15 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> new->flavor = orig->flavor;
> new->data = data;
>
> - if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> + if (orig->flavor == CIL_BLOCK && DATUM(data)->nodes->head != NULL) {
> + /* Duplicate block */
> + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> + cil_log(CIL_ERR, " Re-declaration of block is only allowed when inheriting a block\n");
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> + cil_list_append(DATUM(new->data)->nodes, CIL_NODE, new);
> + } else if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> /* Check the flavor of data if was found in the destination symtab */
> if (DATUM(data)->nodes->head && FLAVOR(data) != orig->flavor) {
> cil_tree_log(orig, CIL_ERR, "Incompatible flavor when trying to copy %s", DATUM(data)->name);
> @@ -2098,12 +2128,13 @@ int cil_copy_ast(struct cil_db *db, struct cil_tree_node *orig, struct cil_tree_
> int rc = SEPOL_ERR;
> struct cil_args_copy extra_args;
>
> + extra_args.orig_dest = dest;
> extra_args.dest = dest;
> extra_args.db = db;
>
> rc = cil_tree_walk(orig, __cil_copy_node_helper, NULL, __cil_copy_last_child_helper, &extra_args);
> if (rc != SEPOL_OK) {
> - cil_log(CIL_INFO, "cil_tree_walk failed, rc: %d\n", rc);
> + cil_tree_log(dest, CIL_ERR, "Failed to copy %s to %s", cil_node_to_string(orig), cil_node_to_string(dest));
> goto exit;
> }
--
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6 E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libsepol/cil: Allow some duplicate macro and block declarations
2021-08-12 7:53 ` Dominick Grift
@ 2021-08-12 13:20 ` James Carter
0 siblings, 0 replies; 6+ messages in thread
From: James Carter @ 2021-08-12 13:20 UTC (permalink / raw)
To: Dominick Grift; +Cc: SElinux list
On Thu, Aug 12, 2021 at 3:53 AM Dominick Grift
<dominick.grift@defensec.nl> wrote:
>
> James Carter <jwcart2@gmail.com> writes:
>
> > The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
> > Check for duplicate blocks, optionals, and macros) added checks when
> > copying blocks, macros, and optionals so that a duplicate would cause
> > an exit with an error. Unfortunately, some policies exist that depend
> > on this behavior when using inheritance.
>
> Thanks. This resolved my issues.
>
> We should probably document this behavior.
>
Yes, you are right, we do need to document this behavior.
Thanks for testing the patch.
Jim
> >
> > The behavior is as follows.
> >
> > For macros only the first declared macro matters.
> > ;
> > (macro m ((type ARG1))
> > (allow ARG1 self (CLASS (PERM1)))
> > )
> > (block b
> > (macro m ((type ARG1))
> > (allow ARG1 self (CLASS (PERM2)))
> > )
> > )
> > (blockinherit b)
> > (type t)
> > (call m (t))
> > ;
> > For this policy segment, the macro m in block b will not be called.
> > Only the original macro m will be.
> >
> > This behavior has been used to override macros that are going to
> > be inherited. Only the inherited macros that have not already been
> > declared in the destination namespace will be used.
> >
> > Blocks seem to work fine even though there are two of them
> > ;
> > (block b1
> > (blockinherit b2)
> > (block b
> > (type t1)
> > (allow t1 self (CLASS (PERM)))
> > )
> > )
> > (block b2
> > (block b
> > (type t2)
> > (allow t2 self (CLASS (PERM)))
> > )
> > )
> > (blockinherit b1)
> > ;
> > In this example, the blockinherit of b2 will cause there to be
> > two block b's in block b1. Note that if both block b's tried to
> > declare the same type, then that would be an error. The blockinherit
> > of b1 will copy both block b's.
> >
> > This behavior has been used to allow the use of in-statements for
> > a block that is being inherited. Since the in-statements are resolved
> > before block inheritance, this only works if a block with the same
> > name as the block to be inherited is declared in the namespace.
> >
> > To support the use of these two behaviors, allow duplicate blocks
> > and macros when they occur as the result of block inheritance. In
> > any other circumstances and error for a redeclaration will be given.
> >
> > Since the duplicate macro is not going to be used it is just skipped.
> > The duplicate block will use the datum of the original block. In both
> > cases a warning message will be produced (it will only be seen if
> > "-v" is used when compiling the policy).
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> > libsepol/cil/src/cil_copy_ast.c | 69 ++++++++++++++++++++++++---------
> > 1 file changed, 50 insertions(+), 19 deletions(-)
> >
> > diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> > index 9c0231f2..67be2ec8 100644
> > --- a/libsepol/cil/src/cil_copy_ast.c
> > +++ b/libsepol/cil/src/cil_copy_ast.c
> > @@ -43,6 +43,7 @@
> > #include "cil_verify.h"
> >
> > struct cil_args_copy {
> > + struct cil_tree_node *orig_dest;
> > struct cil_tree_node *dest;
> > struct cil_db *db;
> > };
> > @@ -101,17 +102,23 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
> > struct cil_block *orig = data;
> > char *key = orig->datum.name;
> > struct cil_symtab_datum *datum = NULL;
> > - struct cil_block *new;
> >
> > cil_symtab_get_datum(symtab, key, &datum);
> > if (datum != NULL) {
> > - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> > - return SEPOL_ERR;
> > + if (FLAVOR(datum) != CIL_BLOCK) {
> > + cil_tree_log(NODE(orig), CIL_ERR, "Block %s being copied", key);
> > + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> > + return SEPOL_ERR;
> > + }
> > + cil_tree_log(NODE(orig), CIL_WARN, "Block %s being copied", key);
> > + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared", key);
> > + *copy = datum;
> > + } else {
> > + struct cil_block *new;
> > + cil_block_init(&new);
> > + *copy = new;
> > }
> >
> > - cil_block_init(&new);
> > - *copy = new;
> > -
> > return SEPOL_OK;
> > }
> >
> > @@ -1511,21 +1518,26 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
> > struct cil_macro *orig = data;
> > char *key = orig->datum.name;
> > struct cil_symtab_datum *datum = NULL;
> > - struct cil_macro *new;
> >
> > cil_symtab_get_datum(symtab, key, &datum);
> > if (datum != NULL) {
> > - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> > - return SEPOL_ERR;
> > - }
> > -
> > - cil_macro_init(&new);
> > - if (orig->params != NULL) {
> > - cil_copy_list(orig->params, &new->params);
> > + if (FLAVOR(datum) != CIL_MACRO) {
> > + cil_tree_log(NODE(orig), CIL_ERR, "Macro %s being copied", key);
> > + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> > + return SEPOL_ERR;
> > + }
> > + cil_tree_log(NODE(orig), CIL_WARN, "Skipping macro %s", key);
> > + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared");
> > + *copy = NULL;
> > + } else {
> > + struct cil_macro *new;
> > + cil_macro_init(&new);
> > + if (orig->params != NULL) {
> > + cil_copy_list(orig->params, &new->params);
> > + }
> > + *copy = new;
> > }
> >
> > - *copy = new;
> > -
> > return SEPOL_OK;
> > }
> >
> > @@ -1700,7 +1712,7 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
> > return SEPOL_OK;
> > }
> >
> > -int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) uint32_t *finished, void *extra_args)
> > +int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void *extra_args)
> > {
> > int rc = SEPOL_ERR;
> > struct cil_tree_node *parent = NULL;
> > @@ -2005,6 +2017,16 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> >
> > rc = (*copy_func)(db, orig->data, &data, symtab);
> > if (rc == SEPOL_OK) {
> > + if (orig->flavor == CIL_MACRO && data == NULL) {
> > + /* Skipping macro re-declaration */
> > + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> > + cil_log(CIL_ERR, " Re-declaration of macro is only allowed when inheriting a block\n");
> > + return SEPOL_ERR;
> > + }
> > + *finished = CIL_TREE_SKIP_HEAD;
> > + return SEPOL_OK;
> > + }
> > +
> > cil_tree_node_init(&new);
> >
> > new->parent = parent;
> > @@ -2013,7 +2035,15 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> > new->flavor = orig->flavor;
> > new->data = data;
> >
> > - if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> > + if (orig->flavor == CIL_BLOCK && DATUM(data)->nodes->head != NULL) {
> > + /* Duplicate block */
> > + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> > + cil_log(CIL_ERR, " Re-declaration of block is only allowed when inheriting a block\n");
> > + rc = SEPOL_ERR;
> > + goto exit;
> > + }
> > + cil_list_append(DATUM(new->data)->nodes, CIL_NODE, new);
> > + } else if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> > /* Check the flavor of data if was found in the destination symtab */
> > if (DATUM(data)->nodes->head && FLAVOR(data) != orig->flavor) {
> > cil_tree_log(orig, CIL_ERR, "Incompatible flavor when trying to copy %s", DATUM(data)->name);
> > @@ -2098,12 +2128,13 @@ int cil_copy_ast(struct cil_db *db, struct cil_tree_node *orig, struct cil_tree_
> > int rc = SEPOL_ERR;
> > struct cil_args_copy extra_args;
> >
> > + extra_args.orig_dest = dest;
> > extra_args.dest = dest;
> > extra_args.db = db;
> >
> > rc = cil_tree_walk(orig, __cil_copy_node_helper, NULL, __cil_copy_last_child_helper, &extra_args);
> > if (rc != SEPOL_OK) {
> > - cil_log(CIL_INFO, "cil_tree_walk failed, rc: %d\n", rc);
> > + cil_tree_log(dest, CIL_ERR, "Failed to copy %s to %s", cil_node_to_string(orig), cil_node_to_string(dest));
> > goto exit;
> > }
>
> --
> gpg --locate-keys dominick.grift@defensec.nl
> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6 E0FF DA7E 521F 10F6 4098
> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
> Dominick Grift
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] libsepol/cil: Allow some duplicate macro and block declarations
@ 2021-08-13 11:51 James Carter
2021-08-13 11:52 ` James Carter
0 siblings, 1 reply; 6+ messages in thread
From: James Carter @ 2021-08-13 11:51 UTC (permalink / raw)
To: selinux; +Cc: dominick.grift, James Carter
The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
Check for duplicate blocks, optionals, and macros) added checks when
copying blocks, macros, and optionals so that a duplicate would cause
an exit with an error. Unfortunately, some policies exist that depend
on this behavior when using inheritance.
The behavior is as follows.
For macros only the first declared macro matters.
;
(macro m ((type ARG1))
(allow ARG1 self (CLASS (PERM1)))
)
(block b
(macro m ((type ARG1))
(allow ARG1 self (CLASS (PERM2)))
)
)
(blockinherit b)
(type t)
(call m (t))
;
For this policy segment, the macro m in block b will not be called.
Only the original macro m will be.
This behavior has been used to override macros that are going to
be inherited. Only the inherited macros that have not already been
declared in the destination namespace will be used.
Blocks seem to work fine even though there are two of them
;
(block b1
(blockinherit b2)
(block b
(type t1)
(allow t1 self (CLASS (PERM)))
)
)
(block b2
(block b
(type t2)
(allow t2 self (CLASS (PERM)))
)
)
(blockinherit b1)
;
In this example, the blockinherit of b2 will cause there to be
two block b's in block b1. Note that if both block b's tried to
declare the same type, then that would be an error. The blockinherit
of b1 will copy both block b's.
This behavior has been used to allow the use of in-statements for
a block that is being inherited. Since the in-statements are resolved
before block inheritance, this only works if a block with the same
name as the block to be inherited is declared in the namespace.
To support the use of these two behaviors, allow duplicate blocks
and macros when they occur as the result of block inheritance. In
any other circumstances and error for a redeclaration will be given.
Since the duplicate macro is not going to be used it is just skipped.
The duplicate block will use the datum of the original block. In both
cases a warning message will be produced (it will only be seen if
"-v" is used when compiling the policy).
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_copy_ast.c | 69 ++++++++++++++++++++++++---------
1 file changed, 50 insertions(+), 19 deletions(-)
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 9c0231f2..67be2ec8 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -43,6 +43,7 @@
#include "cil_verify.h"
struct cil_args_copy {
+ struct cil_tree_node *orig_dest;
struct cil_tree_node *dest;
struct cil_db *db;
};
@@ -101,17 +102,23 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
struct cil_block *orig = data;
char *key = orig->datum.name;
struct cil_symtab_datum *datum = NULL;
- struct cil_block *new;
cil_symtab_get_datum(symtab, key, &datum);
if (datum != NULL) {
- cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
- return SEPOL_ERR;
+ if (FLAVOR(datum) != CIL_BLOCK) {
+ cil_tree_log(NODE(orig), CIL_ERR, "Block %s being copied", key);
+ cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
+ return SEPOL_ERR;
+ }
+ cil_tree_log(NODE(orig), CIL_WARN, "Block %s being copied", key);
+ cil_tree_log(NODE(datum), CIL_WARN, " Previously declared", key);
+ *copy = datum;
+ } else {
+ struct cil_block *new;
+ cil_block_init(&new);
+ *copy = new;
}
- cil_block_init(&new);
- *copy = new;
-
return SEPOL_OK;
}
@@ -1511,21 +1518,26 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
struct cil_macro *orig = data;
char *key = orig->datum.name;
struct cil_symtab_datum *datum = NULL;
- struct cil_macro *new;
cil_symtab_get_datum(symtab, key, &datum);
if (datum != NULL) {
- cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
- return SEPOL_ERR;
- }
-
- cil_macro_init(&new);
- if (orig->params != NULL) {
- cil_copy_list(orig->params, &new->params);
+ if (FLAVOR(datum) != CIL_MACRO) {
+ cil_tree_log(NODE(orig), CIL_ERR, "Macro %s being copied", key);
+ cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
+ return SEPOL_ERR;
+ }
+ cil_tree_log(NODE(orig), CIL_WARN, "Skipping macro %s", key);
+ cil_tree_log(NODE(datum), CIL_WARN, " Previously declared");
+ *copy = NULL;
+ } else {
+ struct cil_macro *new;
+ cil_macro_init(&new);
+ if (orig->params != NULL) {
+ cil_copy_list(orig->params, &new->params);
+ }
+ *copy = new;
}
- *copy = new;
-
return SEPOL_OK;
}
@@ -1700,7 +1712,7 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
return SEPOL_OK;
}
-int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) uint32_t *finished, void *extra_args)
+int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void *extra_args)
{
int rc = SEPOL_ERR;
struct cil_tree_node *parent = NULL;
@@ -2005,6 +2017,16 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
rc = (*copy_func)(db, orig->data, &data, symtab);
if (rc == SEPOL_OK) {
+ if (orig->flavor == CIL_MACRO && data == NULL) {
+ /* Skipping macro re-declaration */
+ if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
+ cil_log(CIL_ERR, " Re-declaration of macro is only allowed when inheriting a block\n");
+ return SEPOL_ERR;
+ }
+ *finished = CIL_TREE_SKIP_HEAD;
+ return SEPOL_OK;
+ }
+
cil_tree_node_init(&new);
new->parent = parent;
@@ -2013,7 +2035,15 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
new->flavor = orig->flavor;
new->data = data;
- if (orig->flavor >= CIL_MIN_DECLARATIVE) {
+ if (orig->flavor == CIL_BLOCK && DATUM(data)->nodes->head != NULL) {
+ /* Duplicate block */
+ if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
+ cil_log(CIL_ERR, " Re-declaration of block is only allowed when inheriting a block\n");
+ rc = SEPOL_ERR;
+ goto exit;
+ }
+ cil_list_append(DATUM(new->data)->nodes, CIL_NODE, new);
+ } else if (orig->flavor >= CIL_MIN_DECLARATIVE) {
/* Check the flavor of data if was found in the destination symtab */
if (DATUM(data)->nodes->head && FLAVOR(data) != orig->flavor) {
cil_tree_log(orig, CIL_ERR, "Incompatible flavor when trying to copy %s", DATUM(data)->name);
@@ -2098,12 +2128,13 @@ int cil_copy_ast(struct cil_db *db, struct cil_tree_node *orig, struct cil_tree_
int rc = SEPOL_ERR;
struct cil_args_copy extra_args;
+ extra_args.orig_dest = dest;
extra_args.dest = dest;
extra_args.db = db;
rc = cil_tree_walk(orig, __cil_copy_node_helper, NULL, __cil_copy_last_child_helper, &extra_args);
if (rc != SEPOL_OK) {
- cil_log(CIL_INFO, "cil_tree_walk failed, rc: %d\n", rc);
+ cil_tree_log(dest, CIL_ERR, "Failed to copy %s to %s", cil_node_to_string(orig), cil_node_to_string(dest));
goto exit;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libsepol/cil: Allow some duplicate macro and block declarations
2021-08-13 11:51 James Carter
@ 2021-08-13 11:52 ` James Carter
0 siblings, 0 replies; 6+ messages in thread
From: James Carter @ 2021-08-13 11:52 UTC (permalink / raw)
To: SElinux list; +Cc: Dominick Grift
Sorry, didn't mean to send this patch again.
Jim
On Fri, Aug 13, 2021 at 7:52 AM James Carter <jwcart2@gmail.com> wrote:
>
> The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
> Check for duplicate blocks, optionals, and macros) added checks when
> copying blocks, macros, and optionals so that a duplicate would cause
> an exit with an error. Unfortunately, some policies exist that depend
> on this behavior when using inheritance.
>
> The behavior is as follows.
>
> For macros only the first declared macro matters.
> ;
> (macro m ((type ARG1))
> (allow ARG1 self (CLASS (PERM1)))
> )
> (block b
> (macro m ((type ARG1))
> (allow ARG1 self (CLASS (PERM2)))
> )
> )
> (blockinherit b)
> (type t)
> (call m (t))
> ;
> For this policy segment, the macro m in block b will not be called.
> Only the original macro m will be.
>
> This behavior has been used to override macros that are going to
> be inherited. Only the inherited macros that have not already been
> declared in the destination namespace will be used.
>
> Blocks seem to work fine even though there are two of them
> ;
> (block b1
> (blockinherit b2)
> (block b
> (type t1)
> (allow t1 self (CLASS (PERM)))
> )
> )
> (block b2
> (block b
> (type t2)
> (allow t2 self (CLASS (PERM)))
> )
> )
> (blockinherit b1)
> ;
> In this example, the blockinherit of b2 will cause there to be
> two block b's in block b1. Note that if both block b's tried to
> declare the same type, then that would be an error. The blockinherit
> of b1 will copy both block b's.
>
> This behavior has been used to allow the use of in-statements for
> a block that is being inherited. Since the in-statements are resolved
> before block inheritance, this only works if a block with the same
> name as the block to be inherited is declared in the namespace.
>
> To support the use of these two behaviors, allow duplicate blocks
> and macros when they occur as the result of block inheritance. In
> any other circumstances and error for a redeclaration will be given.
>
> Since the duplicate macro is not going to be used it is just skipped.
> The duplicate block will use the datum of the original block. In both
> cases a warning message will be produced (it will only be seen if
> "-v" is used when compiling the policy).
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil_copy_ast.c | 69 ++++++++++++++++++++++++---------
> 1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index 9c0231f2..67be2ec8 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -43,6 +43,7 @@
> #include "cil_verify.h"
>
> struct cil_args_copy {
> + struct cil_tree_node *orig_dest;
> struct cil_tree_node *dest;
> struct cil_db *db;
> };
> @@ -101,17 +102,23 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
> struct cil_block *orig = data;
> char *key = orig->datum.name;
> struct cil_symtab_datum *datum = NULL;
> - struct cil_block *new;
>
> cil_symtab_get_datum(symtab, key, &datum);
> if (datum != NULL) {
> - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> - return SEPOL_ERR;
> + if (FLAVOR(datum) != CIL_BLOCK) {
> + cil_tree_log(NODE(orig), CIL_ERR, "Block %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> + return SEPOL_ERR;
> + }
> + cil_tree_log(NODE(orig), CIL_WARN, "Block %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared", key);
> + *copy = datum;
> + } else {
> + struct cil_block *new;
> + cil_block_init(&new);
> + *copy = new;
> }
>
> - cil_block_init(&new);
> - *copy = new;
> -
> return SEPOL_OK;
> }
>
> @@ -1511,21 +1518,26 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
> struct cil_macro *orig = data;
> char *key = orig->datum.name;
> struct cil_symtab_datum *datum = NULL;
> - struct cil_macro *new;
>
> cil_symtab_get_datum(symtab, key, &datum);
> if (datum != NULL) {
> - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> - return SEPOL_ERR;
> - }
> -
> - cil_macro_init(&new);
> - if (orig->params != NULL) {
> - cil_copy_list(orig->params, &new->params);
> + if (FLAVOR(datum) != CIL_MACRO) {
> + cil_tree_log(NODE(orig), CIL_ERR, "Macro %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> + return SEPOL_ERR;
> + }
> + cil_tree_log(NODE(orig), CIL_WARN, "Skipping macro %s", key);
> + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared");
> + *copy = NULL;
> + } else {
> + struct cil_macro *new;
> + cil_macro_init(&new);
> + if (orig->params != NULL) {
> + cil_copy_list(orig->params, &new->params);
> + }
> + *copy = new;
> }
>
> - *copy = new;
> -
> return SEPOL_OK;
> }
>
> @@ -1700,7 +1712,7 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
> return SEPOL_OK;
> }
>
> -int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) uint32_t *finished, void *extra_args)
> +int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void *extra_args)
> {
> int rc = SEPOL_ERR;
> struct cil_tree_node *parent = NULL;
> @@ -2005,6 +2017,16 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
>
> rc = (*copy_func)(db, orig->data, &data, symtab);
> if (rc == SEPOL_OK) {
> + if (orig->flavor == CIL_MACRO && data == NULL) {
> + /* Skipping macro re-declaration */
> + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> + cil_log(CIL_ERR, " Re-declaration of macro is only allowed when inheriting a block\n");
> + return SEPOL_ERR;
> + }
> + *finished = CIL_TREE_SKIP_HEAD;
> + return SEPOL_OK;
> + }
> +
> cil_tree_node_init(&new);
>
> new->parent = parent;
> @@ -2013,7 +2035,15 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> new->flavor = orig->flavor;
> new->data = data;
>
> - if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> + if (orig->flavor == CIL_BLOCK && DATUM(data)->nodes->head != NULL) {
> + /* Duplicate block */
> + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> + cil_log(CIL_ERR, " Re-declaration of block is only allowed when inheriting a block\n");
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> + cil_list_append(DATUM(new->data)->nodes, CIL_NODE, new);
> + } else if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> /* Check the flavor of data if was found in the destination symtab */
> if (DATUM(data)->nodes->head && FLAVOR(data) != orig->flavor) {
> cil_tree_log(orig, CIL_ERR, "Incompatible flavor when trying to copy %s", DATUM(data)->name);
> @@ -2098,12 +2128,13 @@ int cil_copy_ast(struct cil_db *db, struct cil_tree_node *orig, struct cil_tree_
> int rc = SEPOL_ERR;
> struct cil_args_copy extra_args;
>
> + extra_args.orig_dest = dest;
> extra_args.dest = dest;
> extra_args.db = db;
>
> rc = cil_tree_walk(orig, __cil_copy_node_helper, NULL, __cil_copy_last_child_helper, &extra_args);
> if (rc != SEPOL_OK) {
> - cil_log(CIL_INFO, "cil_tree_walk failed, rc: %d\n", rc);
> + cil_tree_log(dest, CIL_ERR, "Failed to copy %s to %s", cil_node_to_string(orig), cil_node_to_string(dest));
> goto exit;
> }
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libsepol/cil: Allow some duplicate macro and block declarations
2021-08-11 21:04 [PATCH] libsepol/cil: Allow some duplicate macro and block declarations James Carter
2021-08-12 7:53 ` Dominick Grift
@ 2021-08-16 9:31 ` Nicolas Iooss
1 sibling, 0 replies; 6+ messages in thread
From: Nicolas Iooss @ 2021-08-16 9:31 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list, Dominick Grift
On Wed, Aug 11, 2021 at 11:04 PM James Carter <jwcart2@gmail.com> wrote:
>
> The commit d155b410d4bbc90d28f361b966f0429598da8188 (libsepol/cil:
> Check for duplicate blocks, optionals, and macros) added checks when
> copying blocks, macros, and optionals so that a duplicate would cause
> an exit with an error. Unfortunately, some policies exist that depend
> on this behavior when using inheritance.
>
> The behavior is as follows.
>
> For macros only the first declared macro matters.
> ;
> (macro m ((type ARG1))
> (allow ARG1 self (CLASS (PERM1)))
> )
> (block b
> (macro m ((type ARG1))
> (allow ARG1 self (CLASS (PERM2)))
> )
> )
> (blockinherit b)
> (type t)
> (call m (t))
> ;
> For this policy segment, the macro m in block b will not be called.
> Only the original macro m will be.
>
> This behavior has been used to override macros that are going to
> be inherited. Only the inherited macros that have not already been
> declared in the destination namespace will be used.
>
> Blocks seem to work fine even though there are two of them
> ;
> (block b1
> (blockinherit b2)
> (block b
> (type t1)
> (allow t1 self (CLASS (PERM)))
> )
> )
> (block b2
> (block b
> (type t2)
> (allow t2 self (CLASS (PERM)))
> )
> )
> (blockinherit b1)
> ;
> In this example, the blockinherit of b2 will cause there to be
> two block b's in block b1. Note that if both block b's tried to
> declare the same type, then that would be an error. The blockinherit
> of b1 will copy both block b's.
>
> This behavior has been used to allow the use of in-statements for
> a block that is being inherited. Since the in-statements are resolved
> before block inheritance, this only works if a block with the same
> name as the block to be inherited is declared in the namespace.
>
> To support the use of these two behaviors, allow duplicate blocks
> and macros when they occur as the result of block inheritance. In
> any other circumstances and error for a redeclaration will be given.
>
> Since the duplicate macro is not going to be used it is just skipped.
> The duplicate block will use the datum of the original block. In both
> cases a warning message will be produced (it will only be seen if
> "-v" is used when compiling the policy).
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
Hello,
The patch looks good to me but a format string issue (see comment
below). If you fix this:
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Thanks,
Nicolas
> ---
> libsepol/cil/src/cil_copy_ast.c | 69 ++++++++++++++++++++++++---------
> 1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index 9c0231f2..67be2ec8 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -43,6 +43,7 @@
> #include "cil_verify.h"
>
> struct cil_args_copy {
> + struct cil_tree_node *orig_dest;
> struct cil_tree_node *dest;
> struct cil_db *db;
> };
> @@ -101,17 +102,23 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
> struct cil_block *orig = data;
> char *key = orig->datum.name;
> struct cil_symtab_datum *datum = NULL;
> - struct cil_block *new;
>
> cil_symtab_get_datum(symtab, key, &datum);
> if (datum != NULL) {
> - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> - return SEPOL_ERR;
> + if (FLAVOR(datum) != CIL_BLOCK) {
> + cil_tree_log(NODE(orig), CIL_ERR, "Block %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> + return SEPOL_ERR;
> + }
> + cil_tree_log(NODE(orig), CIL_WARN, "Block %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared", key);
Here there is an extra format argument "key", which needs to be removed.
> + *copy = datum;
> + } else {
> + struct cil_block *new;
> + cil_block_init(&new);
> + *copy = new;
> }
>
> - cil_block_init(&new);
> - *copy = new;
> -
> return SEPOL_OK;
> }
>
> @@ -1511,21 +1518,26 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
> struct cil_macro *orig = data;
> char *key = orig->datum.name;
> struct cil_symtab_datum *datum = NULL;
> - struct cil_macro *new;
>
> cil_symtab_get_datum(symtab, key, &datum);
> if (datum != NULL) {
> - cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> - return SEPOL_ERR;
> - }
> -
> - cil_macro_init(&new);
> - if (orig->params != NULL) {
> - cil_copy_list(orig->params, &new->params);
> + if (FLAVOR(datum) != CIL_MACRO) {
> + cil_tree_log(NODE(orig), CIL_ERR, "Macro %s being copied", key);
> + cil_tree_log(NODE(datum), CIL_ERR, " Conflicts with %s already declared", cil_node_to_string(NODE(datum)));
> + return SEPOL_ERR;
> + }
> + cil_tree_log(NODE(orig), CIL_WARN, "Skipping macro %s", key);
> + cil_tree_log(NODE(datum), CIL_WARN, " Previously declared");
> + *copy = NULL;
> + } else {
> + struct cil_macro *new;
> + cil_macro_init(&new);
> + if (orig->params != NULL) {
> + cil_copy_list(orig->params, &new->params);
> + }
> + *copy = new;
> }
>
> - *copy = new;
> -
> return SEPOL_OK;
> }
>
> @@ -1700,7 +1712,7 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
> return SEPOL_OK;
> }
>
> -int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) uint32_t *finished, void *extra_args)
> +int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void *extra_args)
> {
> int rc = SEPOL_ERR;
> struct cil_tree_node *parent = NULL;
> @@ -2005,6 +2017,16 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
>
> rc = (*copy_func)(db, orig->data, &data, symtab);
> if (rc == SEPOL_OK) {
> + if (orig->flavor == CIL_MACRO && data == NULL) {
> + /* Skipping macro re-declaration */
> + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> + cil_log(CIL_ERR, " Re-declaration of macro is only allowed when inheriting a block\n");
> + return SEPOL_ERR;
> + }
> + *finished = CIL_TREE_SKIP_HEAD;
> + return SEPOL_OK;
> + }
> +
> cil_tree_node_init(&new);
>
> new->parent = parent;
> @@ -2013,7 +2035,15 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> new->flavor = orig->flavor;
> new->data = data;
>
> - if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> + if (orig->flavor == CIL_BLOCK && DATUM(data)->nodes->head != NULL) {
> + /* Duplicate block */
> + if (args->orig_dest->flavor != CIL_BLOCKINHERIT) {
> + cil_log(CIL_ERR, " Re-declaration of block is only allowed when inheriting a block\n");
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> + cil_list_append(DATUM(new->data)->nodes, CIL_NODE, new);
> + } else if (orig->flavor >= CIL_MIN_DECLARATIVE) {
> /* Check the flavor of data if was found in the destination symtab */
> if (DATUM(data)->nodes->head && FLAVOR(data) != orig->flavor) {
> cil_tree_log(orig, CIL_ERR, "Incompatible flavor when trying to copy %s", DATUM(data)->name);
> @@ -2098,12 +2128,13 @@ int cil_copy_ast(struct cil_db *db, struct cil_tree_node *orig, struct cil_tree_
> int rc = SEPOL_ERR;
> struct cil_args_copy extra_args;
>
> + extra_args.orig_dest = dest;
> extra_args.dest = dest;
> extra_args.db = db;
>
> rc = cil_tree_walk(orig, __cil_copy_node_helper, NULL, __cil_copy_last_child_helper, &extra_args);
> if (rc != SEPOL_OK) {
> - cil_log(CIL_INFO, "cil_tree_walk failed, rc: %d\n", rc);
> + cil_tree_log(dest, CIL_ERR, "Failed to copy %s to %s", cil_node_to_string(orig), cil_node_to_string(dest));
> goto exit;
> }
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-16 9:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-11 21:04 [PATCH] libsepol/cil: Allow some duplicate macro and block declarations James Carter
2021-08-12 7:53 ` Dominick Grift
2021-08-12 13:20 ` James Carter
2021-08-16 9:31 ` Nicolas Iooss
-- strict thread matches above, loose matches on Subject: below --
2021-08-13 11:51 James Carter
2021-08-13 11:52 ` James Carter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.