diff mbox series

[v2] libsepol: Populate and use policy name

Message ID 20220216005326.1899481-1-tweek@google.com (mailing list archive)
State Accepted
Commit c900816e93e4
Headers show
Series [v2] libsepol: Populate and use policy name | expand

Commit Message

Thiébaud Weksteen Feb. 16, 2022, 12:53 a.m. UTC
When an assertion fails, the error message refers to a generic
"policy.conf" file. When parsing a policy in checkpolicy, populate its
name using the original filename (source_filename is still build using
the #line directives within the policy).

Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
v1 -> v2: Fix leak reported by Christian Göttsche

 checkpolicy/module_compiler.c |  1 +
 checkpolicy/parse_util.c      |  1 +
 libsepol/src/assertion.c      | 20 ++++++++++++++------
 libsepol/src/expand.c         |  3 +++
 4 files changed, 19 insertions(+), 6 deletions(-)

Comments

James Carter Feb. 18, 2022, 9:15 p.m. UTC | #1
On Wed, Feb 16, 2022 at 2:27 AM Thiébaud Weksteen <tweek@google.com> wrote:
>
> When an assertion fails, the error message refers to a generic
> "policy.conf" file. When parsing a policy in checkpolicy, populate its
> name using the original filename (source_filename is still build using
> the #line directives within the policy).
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
> v1 -> v2: Fix leak reported by Christian Göttsche
>
>  checkpolicy/module_compiler.c |  1 +
>  checkpolicy/parse_util.c      |  1 +
>  libsepol/src/assertion.c      | 20 ++++++++++++++------
>  libsepol/src/expand.c         |  3 +++
>  4 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
> index 5f5b0b19..129650fa 100644
> --- a/checkpolicy/module_compiler.c
> +++ b/checkpolicy/module_compiler.c
> @@ -99,6 +99,7 @@ int define_policy(int pass, int module_header_given)
>                                 yyerror("no module name");
>                                 return -1;
>                         }
> +                       free(policydbp->name);
>                         policydbp->name = id;
>                         if ((policydbp->version =
>                              queue_remove(id_queue)) == NULL) {
> diff --git a/checkpolicy/parse_util.c b/checkpolicy/parse_util.c
> index 8c1f393c..f2d1e04d 100644
> --- a/checkpolicy/parse_util.c
> +++ b/checkpolicy/parse_util.c
> @@ -47,6 +47,7 @@ int read_source_policy(policydb_t * p, const char *file, const char *progname)
>         }
>
>         policydbp = p;
> +       policydbp->name = strdup(file);
>         mlspol = p->mls;
>
>         init_parser(1);
> diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
> index dd2749a0..74f6d9c0 100644
> --- a/libsepol/src/assertion.c
> +++ b/libsepol/src/assertion.c
> @@ -36,13 +36,21 @@ struct avtab_match_args {
>         unsigned long errors;
>  };
>
> +static const char* policy_name(policydb_t *p) {
> +       const char *policy_file = "policy.conf";
> +       if (p->name) {
> +               policy_file = p->name;
> +       }
> +       return policy_file;
> +}
> +
>  static void report_failure(sepol_handle_t *handle, policydb_t *p, const avrule_t *avrule,
>                            unsigned int stype, unsigned int ttype,
>                            const class_perm_node_t *curperm, uint32_t perms)
>  {
>         if (avrule->source_filename) {
> -               ERR(handle, "neverallow on line %lu of %s (or line %lu of policy.conf) violated by allow %s %s:%s {%s };",
> -                   avrule->source_line, avrule->source_filename, avrule->line,
> +               ERR(handle, "neverallow on line %lu of %s (or line %lu of %s) violated by allow %s %s:%s {%s };",
> +                   avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
>                     p->p_type_val_to_name[stype],
>                     p->p_type_val_to_name[ttype],
>                     p->p_class_val_to_name[curperm->tclass - 1],
> @@ -173,9 +181,9 @@ static int report_assertion_extended_permissions(sepol_handle_t *handle,
>                                 /* failure on the extended permission check_extended_permissions */
>                                 if (rc) {
>                                         extended_permissions_violated(&error, avrule->xperms, xperms);
> -                                       ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
> +                                       ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
>                                                         "allowxperm %s %s:%s %s;",
> -                                                       avrule->source_line, avrule->source_filename, avrule->line,
> +                                                       avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
>                                                         p->p_type_val_to_name[i],
>                                                         p->p_type_val_to_name[j],
>                                                         p->p_class_val_to_name[curperm->tclass - 1],
> @@ -190,9 +198,9 @@ static int report_assertion_extended_permissions(sepol_handle_t *handle,
>
>         /* failure on the regular permissions */
>         if (rc) {
> -               ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
> +               ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
>                                 "allow %s %s:%s {%s };",
> -                               avrule->source_line, avrule->source_filename, avrule->line,
> +                               avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
>                                 p->p_type_val_to_name[stype],
>                                 p->p_type_val_to_name[ttype],
>                                 p->p_class_val_to_name[curperm->tclass - 1],
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index 8667f2ed..7da51a40 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -2970,6 +2970,9 @@ int expand_module(sepol_handle_t * handle,
>
>         state.out->policy_type = POLICY_KERN;
>         state.out->policyvers = POLICYDB_VERSION_MAX;
> +       if (state.base->name) {
> +               state.out->name = strdup(state.base->name);
> +       }
>
>         /* Copy mls state from base to out */
>         out->mls = base->mls;
> --
> 2.35.1.265.g69c8d7142f-goog
>
James Carter Feb. 24, 2022, 9:06 p.m. UTC | #2
On Fri, Feb 18, 2022 at 4:15 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Wed, Feb 16, 2022 at 2:27 AM Thiébaud Weksteen <tweek@google.com> wrote:
> >
> > When an assertion fails, the error message refers to a generic
> > "policy.conf" file. When parsing a policy in checkpolicy, populate its
> > name using the original filename (source_filename is still build using
> > the #line directives within the policy).
> >
> > Signed-off-by: Thiébaud Weksteen <tweek@google.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>

Merged.
Thanks,
Jim

> > ---
> > v1 -> v2: Fix leak reported by Christian Göttsche
> >
> >  checkpolicy/module_compiler.c |  1 +
> >  checkpolicy/parse_util.c      |  1 +
> >  libsepol/src/assertion.c      | 20 ++++++++++++++------
> >  libsepol/src/expand.c         |  3 +++
> >  4 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
> > index 5f5b0b19..129650fa 100644
> > --- a/checkpolicy/module_compiler.c
> > +++ b/checkpolicy/module_compiler.c
> > @@ -99,6 +99,7 @@ int define_policy(int pass, int module_header_given)
> >                                 yyerror("no module name");
> >                                 return -1;
> >                         }
> > +                       free(policydbp->name);
> >                         policydbp->name = id;
> >                         if ((policydbp->version =
> >                              queue_remove(id_queue)) == NULL) {
> > diff --git a/checkpolicy/parse_util.c b/checkpolicy/parse_util.c
> > index 8c1f393c..f2d1e04d 100644
> > --- a/checkpolicy/parse_util.c
> > +++ b/checkpolicy/parse_util.c
> > @@ -47,6 +47,7 @@ int read_source_policy(policydb_t * p, const char *file, const char *progname)
> >         }
> >
> >         policydbp = p;
> > +       policydbp->name = strdup(file);
> >         mlspol = p->mls;
> >
> >         init_parser(1);
> > diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
> > index dd2749a0..74f6d9c0 100644
> > --- a/libsepol/src/assertion.c
> > +++ b/libsepol/src/assertion.c
> > @@ -36,13 +36,21 @@ struct avtab_match_args {
> >         unsigned long errors;
> >  };
> >
> > +static const char* policy_name(policydb_t *p) {
> > +       const char *policy_file = "policy.conf";
> > +       if (p->name) {
> > +               policy_file = p->name;
> > +       }
> > +       return policy_file;
> > +}
> > +
> >  static void report_failure(sepol_handle_t *handle, policydb_t *p, const avrule_t *avrule,
> >                            unsigned int stype, unsigned int ttype,
> >                            const class_perm_node_t *curperm, uint32_t perms)
> >  {
> >         if (avrule->source_filename) {
> > -               ERR(handle, "neverallow on line %lu of %s (or line %lu of policy.conf) violated by allow %s %s:%s {%s };",
> > -                   avrule->source_line, avrule->source_filename, avrule->line,
> > +               ERR(handle, "neverallow on line %lu of %s (or line %lu of %s) violated by allow %s %s:%s {%s };",
> > +                   avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
> >                     p->p_type_val_to_name[stype],
> >                     p->p_type_val_to_name[ttype],
> >                     p->p_class_val_to_name[curperm->tclass - 1],
> > @@ -173,9 +181,9 @@ static int report_assertion_extended_permissions(sepol_handle_t *handle,
> >                                 /* failure on the extended permission check_extended_permissions */
> >                                 if (rc) {
> >                                         extended_permissions_violated(&error, avrule->xperms, xperms);
> > -                                       ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
> > +                                       ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
> >                                                         "allowxperm %s %s:%s %s;",
> > -                                                       avrule->source_line, avrule->source_filename, avrule->line,
> > +                                                       avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
> >                                                         p->p_type_val_to_name[i],
> >                                                         p->p_type_val_to_name[j],
> >                                                         p->p_class_val_to_name[curperm->tclass - 1],
> > @@ -190,9 +198,9 @@ static int report_assertion_extended_permissions(sepol_handle_t *handle,
> >
> >         /* failure on the regular permissions */
> >         if (rc) {
> > -               ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
> > +               ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
> >                                 "allow %s %s:%s {%s };",
> > -                               avrule->source_line, avrule->source_filename, avrule->line,
> > +                               avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
> >                                 p->p_type_val_to_name[stype],
> >                                 p->p_type_val_to_name[ttype],
> >                                 p->p_class_val_to_name[curperm->tclass - 1],
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> > index 8667f2ed..7da51a40 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -2970,6 +2970,9 @@ int expand_module(sepol_handle_t * handle,
> >
> >         state.out->policy_type = POLICY_KERN;
> >         state.out->policyvers = POLICYDB_VERSION_MAX;
> > +       if (state.base->name) {
> > +               state.out->name = strdup(state.base->name);
> > +       }
> >
> >         /* Copy mls state from base to out */
> >         out->mls = base->mls;
> > --
> > 2.35.1.265.g69c8d7142f-goog
> >
diff mbox series

Patch

diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
index 5f5b0b19..129650fa 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -99,6 +99,7 @@  int define_policy(int pass, int module_header_given)
 				yyerror("no module name");
 				return -1;
 			}
+			free(policydbp->name);
 			policydbp->name = id;
 			if ((policydbp->version =
 			     queue_remove(id_queue)) == NULL) {
diff --git a/checkpolicy/parse_util.c b/checkpolicy/parse_util.c
index 8c1f393c..f2d1e04d 100644
--- a/checkpolicy/parse_util.c
+++ b/checkpolicy/parse_util.c
@@ -47,6 +47,7 @@  int read_source_policy(policydb_t * p, const char *file, const char *progname)
 	}
 
 	policydbp = p;
+	policydbp->name = strdup(file);
 	mlspol = p->mls;
 
 	init_parser(1);
diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index dd2749a0..74f6d9c0 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -36,13 +36,21 @@  struct avtab_match_args {
 	unsigned long errors;
 };
 
+static const char* policy_name(policydb_t *p) {
+	const char *policy_file = "policy.conf";
+	if (p->name) {
+		policy_file = p->name;
+	}
+	return policy_file;
+}
+
 static void report_failure(sepol_handle_t *handle, policydb_t *p, const avrule_t *avrule,
 			   unsigned int stype, unsigned int ttype,
 			   const class_perm_node_t *curperm, uint32_t perms)
 {
 	if (avrule->source_filename) {
-		ERR(handle, "neverallow on line %lu of %s (or line %lu of policy.conf) violated by allow %s %s:%s {%s };",
-		    avrule->source_line, avrule->source_filename, avrule->line,
+		ERR(handle, "neverallow on line %lu of %s (or line %lu of %s) violated by allow %s %s:%s {%s };",
+		    avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
 		    p->p_type_val_to_name[stype],
 		    p->p_type_val_to_name[ttype],
 		    p->p_class_val_to_name[curperm->tclass - 1],
@@ -173,9 +181,9 @@  static int report_assertion_extended_permissions(sepol_handle_t *handle,
 				/* failure on the extended permission check_extended_permissions */
 				if (rc) {
 					extended_permissions_violated(&error, avrule->xperms, xperms);
-					ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
+					ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
 							"allowxperm %s %s:%s %s;",
-							avrule->source_line, avrule->source_filename, avrule->line,
+							avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
 							p->p_type_val_to_name[i],
 							p->p_type_val_to_name[j],
 							p->p_class_val_to_name[curperm->tclass - 1],
@@ -190,9 +198,9 @@  static int report_assertion_extended_permissions(sepol_handle_t *handle,
 
 	/* failure on the regular permissions */
 	if (rc) {
-		ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of policy.conf) violated by\n"
+		ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
 				"allow %s %s:%s {%s };",
-				avrule->source_line, avrule->source_filename, avrule->line,
+				avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
 				p->p_type_val_to_name[stype],
 				p->p_type_val_to_name[ttype],
 				p->p_class_val_to_name[curperm->tclass - 1],
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 8667f2ed..7da51a40 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -2970,6 +2970,9 @@  int expand_module(sepol_handle_t * handle,
 
 	state.out->policy_type = POLICY_KERN;
 	state.out->policyvers = POLICYDB_VERSION_MAX;
+	if (state.base->name) {
+		state.out->name = strdup(state.base->name);
+	}
 
 	/* Copy mls state from base to out */
 	out->mls = base->mls;