Re: [PATCH v2 2/2] libselinux: echo line number of bad label in selabel_fini()

[Yuli Khodorkovskiy <ykhodo@xxxxxxxxx>]

On Thu, Mar 29, 2018 at 9:49 AM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:

On 03/28/2018 11:40 PM, Yuli Khodorkovskiy wrote:
> Keep track of line numbers for each file context in
> selabel_handle. If an error occurs in selabel_fini(), the
> line number of an invalid file context is echoed to the user.
>
> Signed-off-by: Yuli Khodorkovskiy <ykhodo@xxxxxxxxx>
> ---
>  libselinux/src/label.c          | 2 +-
>  libselinux/src/label_file.h     | 1 +
>  libselinux/src/label_internal.h | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libselinux/src/label.c b/libselinux/src/label.c
> index e642a97b..d9a58ce9 100644
> --- a/libselinux/src/label.c
> +++ b/libselinux/src/label.c
> @@ -143,7 +143,7 @@ static int selabel_fini(struct selabel_handle *rec,
>                           struct selabel_lookup_rec *lr,
>                           int translating)
>  {
> -     if (compat_validate(rec, lr, rec->spec_file, 0))
> +     if (compat_validate(rec, lr, rec->spec_file, lr->lineno))
>               return -1;
>
>       if (translating && !lr->ctx_trans &&
> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
> index aa576d8e..4780ae48 100644
> --- a/libselinux/src/label_file.h
> +++ b/libselinux/src/label_file.h
> @@ -472,6 +472,7 @@ static inline int process_line(struct selabel_handle *rec,
>       spec_arr[nspec].mode = 0;
>
>       spec_arr[nspec].lr.ctx_raw = context;
> +     spec_arr[nspec].lr.lineno = lineno;
>
>       /*
>        * bump data->nspecs to cause closef() to cover it in its free
> diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h
> index c55efb75..0e020557 100644
> --- a/libselinux/src/label_internal.h
> +++ b/libselinux/src/label_internal.h
> @@ -73,6 +73,7 @@ struct selabel_lookup_rec {
>       char * ctx_raw;
>       char * ctx_trans;
>       int validated;
> +     unsigned lineno;
>  };
>
>  struct selabel_handle {
>

I think this is ok, but wanted to double check: does this work correctly when file contexts are loaded from
file_contexts.bin instead?  It looks to me as if the lineno will be left as 0 in that case and the
code will handle that correctly.

Compiling a file_contexts.bin with sefcontext_compile will give you the line number:

sefcontext_compile /etc/selinux/targeted/contexts/files/file_contexts -o file_contexts.bin -p /etc/selinux/targeted/policy/policy.31

libsepol.sepol_context_from_string: malformed context "test"

libsepol.sepol_context_from_string: could not construct context from string

libsepol.context_from_string: could not create context structure

libsepol.sepol_context_to_sid: could not convert test to sid

/etc/selinux/targeted/contexts/files/file_contexts: line 6132 has invalid context test

sefcontext_compile: process_file failed

Using file_contexts.bin for relabeling that I generated with no validation will not report a line number:

restorecon: /etc/selinux/targeted/contexts/files/file_contexts: has invalid context test

I'll see if I can associate the line number with each regex in sefcontext_compile.

 

The other question is whether we correctly report the file name when the entry comes from a file other
than file_contexts itself, e.g. file_contexts.local, .homedirs, ...  Not your bug if we don't but wondered.

It is reporting the line number correctly, but the filename is incorrect. I'll update this.