Re: [PATCH 0/2] userspace: Implement new format of filename trans rules

[James Carter <jwcart2@gmail.com>]

On Mon, Mar 30, 2020 at 9:06 AM Chris PeBenito <pebenito@ieee.org> wrote:
>
> On 3/27/20 3:21 PM, Stephen Smalley wrote:
> > On 3/27/20 11:21 AM, Ondrej Mosnacek wrote:
> >> These patches are the userspace side of the kernel change posted at [1].
> >>
> >> The first patch changes libsepol's internal representation of filename
> >> transition rules in a way similar to kernel commit c3a276111ea2
> >> ("selinux: optimize storage of filename transitions") [2].
> >>
> >> The second patch then builds upon that and implements reading and
> >> writing of a new binary policy format that uses this representation also
> >> in the data layout.
> >>
> >> See individual patches for more details.
> >>
> >> NOTE: This series unfortunately breaks the build of setools. Moreover,
> >> when an existing build of setools dynamically links against the new
> >> libsepol, it segfaults. Sadly, there doesn't seem to be a nice way of
> >> handling this, since setools relies on non-public libsepol policydb
> >> API/ABI.
> >
> > I think this has happened before a few years ago when we made a
> > different change to those structures, and required updates on the
> > setools side.
> >
> > Maybe we need to figure out what setools needs to be encapsulated and
> > exported as part of the libsepol public ABI/API, and then stop having it
> > peer into libsepol internals?
>
> I'd be fine with that :)
>

I am a little confused. I would consider peering into the libsepol
internals to be something like assuming the memory layout of
structures and pulling things out based on those assumptions, but
setools is just using public header files and functions. It seems to
me to be using the public API. Now I think that too much of the
internals are being exposed, but I am not sure we can do anything
about that now.

It doesn't seem like it would be too hard to update setools to work
with this change, unless there is a requirement that it still work
with older versions of libsepol. I am not very familiar with cython
and I don't know how to make it work differently depending on what
version of libsepol is on the system.

Speaking of versions, since we are actually modifying structs in the
header files instead of just adding new things, don't we need change
the version of libsepol or something?

> Ultimately SETools does 2 thing for the policy access:
> * iterate over the entire policy, reading data out of the various objects
> * use linkages between objects in the policy, e.g. get the
> type/attributes from an AV rule, get types from an attribute, etc. using
> the stuff like class_val_to_struct as needed.
>
> So if sufficient functionality to do dispol was exported, that would get
> close to all that was needed.  There are some optimizations I made by
> digging at the structs a bit more than the above, but that could
> potentially be dropped if libsepol has sufficient support.  See
> <https://github.com/SELinuxProject/setools/blob/master/setools/policyrep/selinuxpolicy.pxi#L673>
> for the policy loading code.
>

I think the fact that the CIL, kernel to CIL, kernel to conf, and
module to CIL code is all in libsepol speaks to the fact of how
tightly integrated they are to the rest of libsepol. One argument that
could be made is that the policyrep stuff in setools really belongs in
libsepol.

Thinking about how libsepol could be encapsulated leads me to a couple
of possibilities. One way would be functions that could return lists
of rules. The policy module code gives us avrule_t, role_trans_rule_t,
role_allow_t, filename_trans_rule_t, range_trans_rule_t, and others.
Those structures are probably unlikely to change and, at least in this
case, creating a function that walks the filename_trans hashtable and
returns a list of filename_trans_rule_t certainly seems like it
wouldn't be too hard. Another possible way to encapsulate would be
create a way to walk the various hashtables element by element (I
think hashtab_map() requires too much knowledge of the internal
structures), returning an opaque structure to track where you are in
the hashtable and functions that allow you to get each part of the
rule being stored. There are other ways that it could be done, but I
could rewrite kernel to and module to stuff with either of those. CIL
itself would require some functions to insert rules into the policydb
which probably wouldn't be too hard. None of this would be too hard,
but it would take some time. The real question is would it really be
valuable?

Jim