Re: [RFC PATCH v2] libsepol/cil: raise default attrs_expand_size to 2

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

 



On Mon, May 11, 2020 at 9:25 AM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Mon, May 11, 2020 at 8:27 AM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
> >
> > On Thu, Mar 05, 2020 at 02:53:37PM +0100, Ondrej Mosnacek wrote:
> > > The value attrs_expand_size == 1 removes all empty attributes, but it
> > > also makes sense to expand all attributes that have only one type. This
> > > removes some redundant rules (there is sometimes the same rule for the
> > > type and the attribute) and reduces the number of attributes that the
> > > kernel has to go through when looking up rules.
> > >
> > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > > ---
> > >
> > > v2: fix typos (Tne -> The; cointains -> contains)
> > >
> > >  libsepol/cil/src/cil.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
> > > index d222ad3a..c010ca2a 100644
> > > --- a/libsepol/cil/src/cil.c
> > > +++ b/libsepol/cil/src/cil.c
> > > @@ -452,7 +452,8 @@ void cil_db_init(struct cil_db **db)
> > >       (*db)->disable_dontaudit = CIL_FALSE;
> > >       (*db)->disable_neverallow = CIL_FALSE;
> > >       (*db)->attrs_expand_generated = CIL_FALSE;
> > > -     (*db)->attrs_expand_size = 1;
> > > +     /* 2 == remove attributes that contain none or just 1 type */
> > > +     (*db)->attrs_expand_size = 2;
> > >       (*db)->preserve_tunables = CIL_FALSE;
> > >       (*db)->handle_unknown = -1;
> > >       (*db)->mls = -1;
> > > --
> > > 2.24.1
> > >
> >
> >
> > This patch broke `semanage node -l` on Fedora [1]
> >
> > :: [ 21:25:25 ] :: [  BEGIN   ] :: Running 'make LIBDIR=/usr/lib64 SHLIBDIR=/lib64 test'
> > ...
> > test_list (__main__.SemanageTests) ... Traceback (most recent call last):
> >   File "/usr/sbin/semanage", line 967, in <module>
> >     do_parser()
> >   File "/usr/sbin/semanage", line 946, in do_parser
> >     args.func(args)
> >   File "/usr/sbin/semanage", line 649, in handleNode
> >     OBJECT = object_dict['node'](args)
> >   File "/usr/lib/python3.8/site-packages/seobject.py", line 1849, in __init__
> >     self.valid_types = list(list(sepolicy.info(sepolicy.ATTRIBUTE, "node_type"))[0]["types"])
> > IndexError: list index out of range
> >
> > While the `IndexError: list index out of range` error can be simply fixed, it
> > uncovered the problem that semanage uses attibutes to list certain records -
> > node_type, port_type, file_type, device_node, ... and these attributes can disappear when
> > there's only 1 type assigned.
> >
> > I guess it should be reverted as there's no other way how to find out that a
> > type node_t is node_type.
> >
> > [1] https://jenkins-continuous-infra.apps.ci.centos.org/job/fedora-rawhide-pr-pipeline/3462/artifact/package-tests/logs/FAIL-upstream-err.log
> >

I see now.  python/semanage/seobject.py and
python/semanage/semanage-bash-completion.sh both assume that node_type
is always defined as an attribute in a policy. There seems to be quite
a lot that is assumed about policy in the python directory.

This is not a bug in CIL or libsepol. Ideally the tests should have
their own policy and the python code should gracefully handle the
situation when its assumptions are wrong. If we need to revert this
patch in the short-term than I am ok with that.

Jim

> > Petr
>
> This suggests that there is a problem with how type attributes are
> being expanded. This patch is just causing that problem to be exposed.
> I'll have to investigate.
>
> Thanks for the report.
>
> Jim



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

  Powered by Linux