Re: [address@hidden: Re: Bash 5 change in behavior and SELinux]

[Chet Ramey]

On 2/24/19 3:50 PM, Dominick Grift wrote:
> I noticed that Bash5 requires some additional permissions that I found 
> questionable.
> 
> Mostly the listing of / and /home
> 
> I am wondering whether there is a way to avoid the requirement for above 
> permissions.

There is, at least in the most frequent cases, and that change is in the
devel branch to be evaluated.

> 
> After some digging with the help of the community we came up with the 
> following analysis:
> 
> ----- Forwarded message from Nicolas Iooss <nicolas.iooss@m4x.org> -----
> 
> Date: Sun, 24 Feb 2019 21:32:13 +0100
> From: Nicolas Iooss <nicolas.iooss@m4x.org>
> To: Dominick Grift <dominick.grift@defensec.nl>
> Cc: selinux@vger.kernel.org
> Subject: Re: Bash 5 change in behavior and SELinux
> 
> On Sun, Feb 24, 2019 at 7:37 PM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
>>
>> On Sun, Feb 24, 2019 at 07:17:33PM +0100, Nicolas Iooss wrote:
>>> On Sun, Feb 24, 2019 at 6:39 PM Dominick Grift
>>> <dominick.grift@defensec.nl> wrote:
>>>>
>>>> On Sun, Feb 24, 2019 at 05:59:19PM +0100, Dominick Grift wrote:
>>>>> Recently Bash-5 appeared in the Fedora repositories and i instantly 
>>>>> noticed an inpleasant change (for the record: this did not happen before):
>>>>
>>>> I suppose this is just a "feature" or a "bug" in Bash-5 and that i will 
>>>> just have to deal with it. Just seems a bit unnecessary access to me.
>>>>
>>>>>
>>>>> [kcinimod@brutus ~]$ touch mytest1.test
>>>>> [kcinimod@brutus ~]$ rm ~/*.test
>>>>> rm: cannot remove '/home/kcinimod/*.test': No such file or directory
>>>>> [kcinimod@brutus ~]$ rm ~/mytest1.test
>>>>> [kcinimod@brutus ~]$ echo $?
>>>>> 0
>>>>>
>>>>> After running `semodule -DB` the following AVC denials surfaced:
>>>>>
>>>>> avc:  denied  { read } for  pid=2178 comm="bash" name="/" dev="dm-3" 
>>>>> ino=2 scontext=wheel.id:wheel.role:wheel.subj:s0 
>>>>> tcontext=sys.id:sys.role:files.home.file:s0 tclass=dir permissive=1
>>>>> avc:  denied  { read } for  pid=2178 comm="bash" name="/" dev="dm-1" 
>>>>> ino=2 scontext=wheel.id:wheel.role:wheel.subj:s0 
>>>>> tcontext=sys.id:sys.role:fs.rootfs.fs:s0 tclass=dir permissive=1
>>>
>>> [For other readers: these are the labels of /home and /, the parent
>>> directories of /home/kcinimod/]
>>>
>>>>> So I took to #bash and they told me:
>>>>>
>>>>> 17:43 <_abc_> grift: that is exactly what you see on android and is
>>>>>               a direct result of the missing x bit equivalent in
>>>>>               the selinux policy
>>>>>
>>>>> 17:44 <_abc_> grift: rephrased: globbing the * requires the x bit
>>>>>               set
>>>>> 17:44 <_abc_> (it's equivalent in selinux policy)
>>>>>
>>>>> So why does this show up as a "read"? Its allowed to "search" "/" and 
>>>>> "/home", but since Bash 5 this no longer is enough.

The idea is that since a backslash can be used to escape characters in glob
patterns, and the pattern matching engine is required to discard the
backslash ("A <backslash> character shall escape the following character.
The escaping <backslash> shall be discarded."), patterns like 'a\b' should
match a filename like `ab'. The different shells do this inconsistently, to
say the least, and there is an ongoing discussion about this on the POSIX
working group mailing list.

So if the backslash is effectively a pattern character, since the matching
engine has to understand and discard it when processing the pattern, the
initial implementation in bash-5.0 fell under this clause in POSIX:

"Each component that contains a pattern character shall require read
permission in the directory containing that component. Any component,
except the last, that does not contain a pattern character shall require
search permission."

However, you can do things more efficiently and avoid some of this.


> Here is what happens, as far I as understand:
> * bash runs execute_simple_command(), which expands the command arguments.
> * It expands the ~ in expand_word_internal()
> (https://git.savannah.gnu.org/cgit/bash.git/tree/subst.c?h=bash-5.0#n9959)
> * glob_expand_word_list() calls
> shell_glob_filename(pathname="\001/\001h\001o\001m\001e\001/\001l\001i\001v\001e\001u\001s\001e\001r/*.test")

The directory name is quoted internally because the results of tilde
expansion are not supposed to be subject to the other word expansions,
like pathname expansion (this came in a long time ago as the result of a
POSIX  interpretation).

> (frame #6 in the debug backtrace)
> * shell_glob_filename() starts by calling quote_string_for_globbing()
> (https://git.savannah.gnu.org/cgit/bash.git/tree/pathexp.c?h=bash-5.0#n385).
> This function replaces CTLESC (=\001) with backslashes and returns
> "/\\h\\o\\m\\e/\\l\\i\\v\\e\\u\\s\\e\\r/*.test".
> * shell_glob_filename() calls
> glob_filename(pathname="/\\h\\o\\m\\e/\\l\\i\\v\\e\\u\\s\\e\\r/*.test",
> flags=0) (frame #5)
> * This function calls itself with the directory,
> glob_filename(pathname="/\\h\\o\\m\\e/\\l\\i\\v\\e\\u\\s\\e\\r",
> flags=0) (frame #4)
> * This function calls itself with the directory,
> glob_filename(pathname="/\\h\\o\\m\\e", flags=0) (frame #3)
> * This function calls glob_vector(pat="\\h\\o\\m\\e", dir="/",
> flags=0) (frame #2), implemented in
> https://git.savannah.gnu.org/cgit/bash.git/tree/lib/glob/glob.c?h=bash-5.0#n577
> * This function checks whether pat is a pattern, by calling
> glob_pattern_p(pat). As variable pat contains backslashes, the answer
> is yes (cf. 
> https://git.savannah.gnu.org/cgit/bash.git/tree/lib/glob/glob_loop.c?h=bash-5.0#n56).

This is where you can make efficiency improvements. If you have a pattern
that has backslashes, but the backslashes are the only special pattern
characters, you can just dequote the pattern and go on, instead of
recursively processing each directory component.

That makes the first call to glob_filename dequote the home directory
and return it.

The attached patch should fix the problem and only require search
permission on the directory if the only special characters in the
pattern are backslashes.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    chet@case.edu    http://tiswww.cwru.edu/~chet/

glob-pattern-backslashes.patch
Description: Text document