Petr, Daniel, Have you had time to verify this issue yet? Any comments to add?
Thanks, Joe On Tue, Mar 20, 2018 at 8:14 AM Stephen Smalley <[email protected]> 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:* [email protected] <[email protected]> *M:* 1.604.365.2823 <https://app.salesforceiq.com/r?target=5a6243eae4b0af727465cb94&t=AFwhZf1CQsv7FDLhSeW59giQrg545clQ2ksOeCxqTY5CK4AoaKjZJqLqF4FBhplxxtKw68VNNcp3ThHgNAMyfYlitWTAFUx2WymfWC2lJKWbtcBVzXz7rzKynmKi0AIVRaiN70T6bDHU>
