From: James Carter <jwcart2@gmail.com>
To: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Petr Lautrbach <plautrba@redhat.com>,
SElinux list <selinux@vger.kernel.org>,
Ondrej Mosnacek <omosnace@redhat.com>
Subject: Re: [RFC PATCH v2] libsepol/cil: raise default attrs_expand_size to 2
Date: Mon, 11 May 2020 15:01:06 -0400 [thread overview]
Message-ID: <CAP+JOzTX3tFUAeheUUDB4br+nLhqCUP+EPY_XuTugDoqpwNGVg@mail.gmail.com> (raw)
In-Reply-To: <CAEjxPJ7Px-LkZPGk_Oe=G6HZP_Yu1a=nWDBPc6i=oNOMf0nG9A@mail.gmail.com>
On Mon, May 11, 2020 at 2:09 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, May 11, 2020 at 2:03 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > On Mon, May 11, 2020 at 9:25 AM James Carter <jwcart2@gmail.com> wrote:
> > >
> > > On Mon, May 11, 2020 at 8:27 AM Petr Lautrbach <plautrba@redhat.com> 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@redhat.com>
> > > > > ---
> > > > >
> > > > > 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.
>
> We can mark these attributes explicitly in policy to prevent their
> expansion, right? So while we cannot make this change right now
> without breaking compatibility with selinux userspace tools (not just
> tests), we could start marking these attributes on which the tools
> depend in policy and then later we can re-apply this?
Yes, we can add the rule "expandattribute node_type false;" in policy
for node_type and any other attributes that are required to exist.
Jim
next prev parent reply other threads:[~2020-05-11 19:01 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-05 13:53 [RFC PATCH v2] libsepol/cil: raise default attrs_expand_size to 2 Ondrej Mosnacek
2020-03-09 13:17 ` James Carter
2020-03-11 18:29 ` James Carter
2020-05-11 12:27 ` Petr Lautrbach
2020-05-11 13:25 ` James Carter
2020-05-11 18:03 ` James Carter
2020-05-11 18:08 ` Stephen Smalley
2020-05-11 19:01 ` James Carter [this message]
2020-05-12 17:16 ` Petr Lautrbach
2020-05-12 17:27 ` Stephen Smalley
2020-05-12 19:33 ` James Carter
2020-05-12 19:48 ` James Carter
2020-05-12 19:57 ` James Carter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAP+JOzTX3tFUAeheUUDB4br+nLhqCUP+EPY_XuTugDoqpwNGVg@mail.gmail.com \
--to=jwcart2@gmail.com \
--cc=omosnace@redhat.com \
--cc=plautrba@redhat.com \
--cc=selinux@vger.kernel.org \
--cc=stephen.smalley.work@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).