On Mon, Apr 23, 2018 at 04:21:22PM +0000, Joe Kirwin wrote:
> Petr, Daniel,
>
> Have you had time to verify this issue yet?
> Any comments to add?
>
I consider this as the expected behavior.
It's defined as "Substitute target path with sourcepath when generating default
label." It means that /apple is substituted for /banana and the lookup is made
for /banana/orange/foo.
On the other hand, `semanage-fcontext` man page and `semanage fcontext -h`
output could be misleading a bit as they use words "EQUAL" and "equivalent"
while it's not a symmetric relation, it's just a substitution.
I don't have an opinion about proposed change to have a real equivalence. It
could complicate some things a lot and the benefit is not clear to me right now.
Petr
>
> On Tue, Mar 20, 2018 at 8:14 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
>
> > On 03/19/2018 10:29 PM, Joe Kirwin wrote:
> > > *_Empirical Observations _*
> > > *
> > > *
> > > If I was to create an SELinux policy containing the following
> > file_contexts (fruits.fc)
> > > ```
> > > /apple/orange/.* --
> > gen_context(system_u:object_r:atype_t,s0)
> > > /banana/.* --
> > gen_context(system_u:object_r:btype_t,s0)
> > > ```
> > >
> > > If I then take the file
> > > /etc/selinux/default/contexts/files/file_contexts.subs_dist and append
> > to it the alias
> > > ```
> > > /apple /banana
> > > ```
> > >
> > > The resulting behavior is that when running:
> > > ```
> > > $ ./libselinux/utils/selabel_lookup_best_match -p /apple/orange/foo
> > > Best match context: system_u:object_r:btype_t:s0
> > >
> > > But the expected behavior is to match `atype_t` as that is a
> > "more-specific" match pattern
> >
> > I don't think this is a bug based on the documented behavior for
> > file_contexts.subs. That said,
> > that support was added by Red Hat so I'll let them speak to it.
> >
> > >
> > > *_Looking into why_*
> > >
> > > From the method in `libselinux/src/label_file.c` :
> > > lookup_common(struct selabel_handle *rec, const
> > char *key, int type, bool partial)
> > >
> > > we encounter a call to :
> > > selabel_sub_key(struct saved_data *data, const char
> > *key)
> > >
> > > In the example above the candidate path we're trying to match (referred
> > to as the key in the code) is "canonicalized" to the /banana alias but the
> > regex being evaluated is not
> > >
> > > *_A proposed fix_*
> > > *
> > > *
> > > /Also attached (label_file.patch), if the patch formatting is off on
> > this thread, apologies./
> > > *
> > > *
> > > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> > > index 560d8c3..98a8d1b 100644
> > > --- a/libselinux/src/label_file.c
> > > +++ b/libselinux/src/label_file.c
> > > @@ -848,7 +848,7 @@ static struct spec *lookup_common(struct
> > selabel_handle *rec,
> > > {
> > > struct saved_data *data = (struct saved_data *)rec->data;
> > > struct spec *spec_arr = data->spec_arr;
> > > - int i, rc, file_stem;
> > > + int i, rc, file_stem, orig_file_stem;
> > > mode_t mode = (mode_t)type;
> > > const char *buf;
> > > struct spec *ret = NULL;
> > > @@ -879,8 +879,12 @@ static struct spec *lookup_common(struct
> > selabel_handle *rec,
> > > }
> > >
> > > sub = selabel_sub_key(data, key);
> > > - if (sub)
> > > + orig_file_stem = -1;
> > > + if (sub) {
> > > + orig_file_stem = find_stem_from_file(data, &key);
> > > key = sub;
> > > + }
> > >
> > > buf = key;
> > > file_stem = find_stem_from_file(data, &buf);
> > > @@ -896,7 +900,8 @@ static struct spec *lookup_common(struct
> > selabel_handle *rec,
> > > * stem as the file AND if the spec in question has no mode
> > > * specified or if the mode matches the file mode then we do
> > > * a regex check */
> > > - if ((spec->stem_id == -1 || spec->stem_id == file_stem) &&
> > > + if ((spec->stem_id == -1 || spec->stem_id == file_stem ||
> > > + spec->stem_id == orig_file_stem) &&
> > > (!mode || !spec->mode || mode == spec->mode)) {
> > > if (compile_regex(data, spec, NULL) < 0)
> > > goto finish;
> > >
> > >
> > >
> > > I think there is still some simplification that could be done with
> > aliases, in that they really shouldn't have a direction (e.g. alias ->
> > original) instead they should go both ways and if there is a tie it should
> > go by the ordering of the specs.
> > > Reason for this is that a developer of an SELinux policy, may not know
> > the contents or directionality of file_contexts.subs_dist ahead of time or
> > when it might change.
> > >
> > > Thanks,
> > > Joe Kirwin and Travis Szucs
> > >
> >
> > --
> --
> *Joe Kirwin* | *Senior Security Developer_*
> *E:* jpk@xxxxxxx <bam@xxxxxxx> *M:* 1.604.365.2823
>
> <https://app.salesforceiq.com/r?target=5a6243eae4b0af727465cb94&t=AFwhZf1CQsv7FDLhSeW59giQrg545clQ2ksOeCxqTY5CK4AoaKjZJqLqF4FBhplxxtKw68VNNcp3ThHgNAMyfYlitWTAFUx2WymfWC2lJKWbtcBVzXz7rzKynmKi0AIVRaiN70T6bDHU>
Attachment:
signature.asc
Description: PGP signature
