android-security-selinux@google.com

android-security-selinux@google.com

[Jiyong Park]

Hi all,

I am Jiyong Park from Google. I have a question regarding the behavior
of selabel_open.

selabel_open() returns -1 when it is given with file_contexts.bin
compiled from following:

(/.*)?                u:object_r:system_file:s0
/lib                   u:object_r:system_dir:s0

, but it runs okay when the file has at least one regex without meta
characters and having '/' after the first character (i.e., when there
is a stem in it). So, selabel_open() accepts this:

(/.*)?                u:object_r:system_file:s0
/lib/                  u:object_r:system_dir:s0

It looks like, when compiling file_contexts, num_stems becomes 0 when
there is no line having a stem in the regex [1]. And selabel_open()
fails when the length of the stems is found to be 0 [2].

[1] https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/label_file.h#L320
[2] https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/label_file.c#L235

I would like to first know whether or not this is intended behavior.
If intended, could any of you tell me the rationale behind it? If not,
what would be the best way to fix it?

Thanks,

Jiyong

[PATCH] libselinux: fix overly strict validation of file_contexts.bin

[Stephen Smalley]

load_mmap and regex_load_mmap (in the !USE_PCRE2 case) were
incorrectly treating the absence of any fixed stems or study data
as an error, rejecting valid file_contexts.bin files.  Remove
the extraneous validation checks.

Test:
$ cat > file_contexts <<EOF
(/.*)?                u:object_r:system_file:s0
/lib                   u:object_r:system_dir:s0
EOF
$ sefcontext_compile file_contexts
$ selabel_lookup -b file -k /lib -f file_contexts.bin

Before:
ERROR: selabel_open - Could not obtain handle.

After:
Default context: u:object_r:system_dir:s0

Reported-by: Jiyong Park <jiyong@google.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libselinux/src/label_file.c | 2 +-
 libselinux/src/regex.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
index 560d8c3d..dbf51a93 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -232,7 +232,7 @@ end_arch_check:
 
 	/* allocate the stems_data array */
 	rc = next_entry(&stem_map_len, mmap_area, sizeof(uint32_t));
-	if (rc < 0 || !stem_map_len)
+	if (rc < 0)
 		return -1;
 
 	/*
diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
index dfc15d63..a6fcbbfe 100644
--- a/libselinux/src/regex.c
+++ b/libselinux/src/regex.c
@@ -348,7 +348,7 @@ int regex_load_mmap(struct mmap_area *mmap_area, struct regex_data **regex,
 		goto err;
 
 	rc = next_entry(&entry_len, mmap_area, sizeof(uint32_t));
-	if (rc < 0 || !entry_len)
+	if (rc < 0)
 		goto err;
 
 	if (entry_len) {
-- 
2.14.5

Re: [PATCH] libselinux: fix overly strict validation of file_contexts.bin

[Stephen Smalley]

On 10/31/18 10:21 AM, Stephen Smalley wrote:
> load_mmap and regex_load_mmap (in the !USE_PCRE2 case) were
> incorrectly treating the absence of any fixed stems or study data
> as an error, rejecting valid file_contexts.bin files.  Remove
> the extraneous validation checks.
> 
> Test:
> $ cat > file_contexts <<EOF
> (/.*)?                u:object_r:system_file:s0
> /lib                   u:object_r:system_dir:s0
> EOF
> $ sefcontext_compile file_contexts
> $ selabel_lookup -b file -k /lib -f file_contexts.bin
> 
> Before:
> ERROR: selabel_open - Could not obtain handle.
> 
> After:
> Default context: u:object_r:system_dir:s0
> 
> Reported-by: Jiyong Park <jiyong@google.com>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>

This has been merged.  Let us know if it didn't resolve the issue for you.

> ---
>   libselinux/src/label_file.c | 2 +-
>   libselinux/src/regex.c      | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 560d8c3d..dbf51a93 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -232,7 +232,7 @@ end_arch_check:
>   
>   	/* allocate the stems_data array */
>   	rc = next_entry(&stem_map_len, mmap_area, sizeof(uint32_t));
> -	if (rc < 0 || !stem_map_len)
> +	if (rc < 0)
>   		return -1;
>   
>   	/*
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> index dfc15d63..a6fcbbfe 100644
> --- a/libselinux/src/regex.c
> +++ b/libselinux/src/regex.c
> @@ -348,7 +348,7 @@ int regex_load_mmap(struct mmap_area *mmap_area, struct regex_data **regex,
>   		goto err;
>   
>   	rc = next_entry(&entry_len, mmap_area, sizeof(uint32_t));
> -	if (rc < 0 || !entry_len)
> +	if (rc < 0)
>   		goto err;
>   
>   	if (entry_len) {
> 

Re: [PATCH] libselinux: fix overly strict validation of file_contexts.bin

[Jiyong Park]

Hi Stephen,

I've just confirmed that the issue was resolved with the patch. Thank you!

On Mon, Nov 5, 2018 at 6:34 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 10/31/18 10:21 AM, Stephen Smalley wrote:
> > load_mmap and regex_load_mmap (in the !USE_PCRE2 case) were
> > incorrectly treating the absence of any fixed stems or study data
> > as an error, rejecting valid file_contexts.bin files.  Remove
> > the extraneous validation checks.
> >
> > Test:
> > $ cat > file_contexts <<EOF
> > (/.*)?                u:object_r:system_file:s0
> > /lib                   u:object_r:system_dir:s0
> > EOF
> > $ sefcontext_compile file_contexts
> > $ selabel_lookup -b file -k /lib -f file_contexts.bin
> >
> > Before:
> > ERROR: selabel_open - Could not obtain handle.
> >
> > After:
> > Default context: u:object_r:system_dir:s0
> >
> > Reported-by: Jiyong Park <jiyong@google.com>
> > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
>
> This has been merged.  Let us know if it didn't resolve the issue for you.
>
> > ---
> >   libselinux/src/label_file.c | 2 +-
> >   libselinux/src/regex.c      | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> > index 560d8c3d..dbf51a93 100644
> > --- a/libselinux/src/label_file.c
> > +++ b/libselinux/src/label_file.c
> > @@ -232,7 +232,7 @@ end_arch_check:
> >
> >       /* allocate the stems_data array */
> >       rc = next_entry(&stem_map_len, mmap_area, sizeof(uint32_t));
> > -     if (rc < 0 || !stem_map_len)
> > +     if (rc < 0)
> >               return -1;
> >
> >       /*
> > diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> > index dfc15d63..a6fcbbfe 100644
> > --- a/libselinux/src/regex.c
> > +++ b/libselinux/src/regex.c
> > @@ -348,7 +348,7 @@ int regex_load_mmap(struct mmap_area *mmap_area, struct regex_data **regex,
> >               goto err;
> >
> >       rc = next_entry(&entry_len, mmap_area, sizeof(uint32_t));
> > -     if (rc < 0 || !entry_len)
> > +     if (rc < 0)
> >               goto err;
> >
> >       if (entry_len) {
> >
>