Re: [PATCH 1/4] libsepol/cil: Provide option to allow qualified names in declarations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 24, 2021 at 9:59 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> Qualified names have "dots" in them. They are generated when a CIL
> policy is compiled and come from declarations in blocks. If a kernel
> policy is decompiled into a CIL policy, the resulting policy could
> have decarations that use qualified names. Compiling this policy would

Misspelling: decarations -> declarations

> result in an error because "dots" in declarations are not allowed.
>
> Qualified names in a policy are normally used to refer to the name of
> identifiers, blocks, macros, or optionals that are declared in a
> different block (that is not a parent). Name resolution is based on
> splitting a name based on the "dots", searching the parents up to the
> global namespace for the first block using the first part of the name,
> using the second part of the name to lookup the next block using the
> first block's symbol tables, looking up the third block in the second's
> symbol tables, and so on.
>
> To allow the option of using qualified names in declarations:
>
> 1) Create a field in the struct cil_db called "qualified_names" which
> is set to CIL_TRUE when qualified names are to be used. This field is
> checked in cil_verify_name() and "dots" are allowed if qualified names
> are being allowed.
>
> 2) Only allow the direct lookup of the whole name in the global symbol
> table. This means that blocks, blockinherits, blockabstracts, and in-
> statements cannot be allowed. Use the "qualified_names" field of the
> cil_db to know when using one of these should result in an error.
>
> 3) Create the function cil_set_qualified_names() that is used to set
> the "qualified_names" field. Export the function in libsepol.
>
> Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> ---
>  libsepol/cil/include/cil/cil.h     |  1 +
>  libsepol/cil/src/cil.c             |  6 ++++++
>  libsepol/cil/src/cil_build_ast.c   | 24 ++++++++++++++++++++++--
>  libsepol/cil/src/cil_internal.h    |  1 +
>  libsepol/cil/src/cil_resolve_ast.c |  4 ++--
>  libsepol/cil/src/cil_verify.c      | 19 ++++++++++++++-----
>  libsepol/cil/src/cil_verify.h      |  2 +-
>  libsepol/src/libsepol.map.in       |  1 +
>  8 files changed, 48 insertions(+), 10 deletions(-)
>
[...]
> diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> index 59397f70..9cb1a6f6 100644
> --- a/libsepol/cil/src/cil_verify.c
> +++ b/libsepol/cil/src/cil_verify.c
> @@ -92,7 +92,7 @@ static int __cil_is_reserved_name(const char *name, enum cil_flavor flavor)
>         return CIL_FALSE;
>  }
>
> -int cil_verify_name(const char *name, enum cil_flavor flavor)
> +int cil_verify_name(struct cil_db *db, const char *name, enum cil_flavor flavor)
>  {
>         int rc = SEPOL_ERR;
>         int len;
> @@ -116,10 +116,19 @@ int cil_verify_name(const char *name, enum cil_flavor flavor)
>                         goto exit;
>         }
>
> -       for (i = 1; i < len; i++) {
> -               if (!isalnum(name[i]) && name[i] != '_' && name[i] != '-') {
> -                       cil_log(CIL_ERR, "Invalid character \"%c\" in %s\n", name[i], name);
> -                       goto exit;
> +       if (db->qualified_names == CIL_FALSE) {
> +               for (i = 1; i < len; i++) {
> +                       if (!isalnum(name[i]) && name[i] != '_' && name[i] != '-') {
> +                               cil_log(CIL_ERR, "Invalid character \"%c\" in %s\n", name[i], name);
> +                               goto exit;
> +                       }
> +               }
> +       } else {
> +               for (i = 1; i < len; i++) {
> +                       if (!isalnum(name[i]) && name[i] != '_' && name[i] != '-' && name[i] != '.') {
> +                               cil_log(CIL_ERR, "Invalid character \"%c\" in %s\n", name[i], name);
> +                               goto exit;
> +                       }
>                 }
>         }

As cil_verify_name does not modify db (and would be seen as a function
which does not modify anything and only checks some rules), it would
be better to add a const qualifier:

    int cil_verify_name(const struct cil_db *db, const char *name,
enum cil_flavor flavor)

Other than that, the documentation of the new option,
"--qualified-names  Use qualified names." make me feel like the
wording can be improved. More precisely, a commit message contains
"Using qualified names means that declaration names can have dots in
them" and this definition should also be in the documentation. So I am
suggesting to replace the documentation with:

    Allow names containing dots (qualified names). Blocks,
blockinherits, blockabstracts, and in-statements will not be allowed.

Other than that, when I tested "secil2tree -A resolve -o test.cil
secilc/test/policy.cil && secilc -Q test.cil" I encountered other
errors, which are not related to these patches. When modifying the
resulting "test.cil" to make it compile (by removing blocks and
renaming some objects), option -Q worked fine. It would be nice if
such a command was integrated in secilc's tests, in order to prevent
future regressions. Nevertheless such work can be done once this
series is merged.

Cheers,
Nicolas




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux