| Message ID | 20171019234915.21820-1-jaekyun@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Please hold off on submission. We're discussing if this is really necessary. On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux <selinux@tycho.nsa.gov> wrote: > Performs exact match if a property key of property contexts ends with '$' > instead of prefix match. > > This will enable to define an exact rule which can avoid unexpected > context assignment. > > Signed-off-by: Jaekyun Seok <jaekyun@google.com> > --- > libselinux/src/label_backends_android.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libselinux/src/label_backends_android.c b/libselinux/src/label_backends_android.c > index cb8aae26..4611d396 100644 > --- a/libselinux/src/label_backends_android.c > +++ b/libselinux/src/label_backends_android.c > @@ -258,8 +258,13 @@ static struct selabel_lookup_rec *property_lookup(struct selabel_handle *rec, > } > > for (i = 0; i < data->nspec; i++) { > - if (strncmp(spec_arr[i].property_key, key, > - strlen(spec_arr[i].property_key)) == 0) { > + size_t property_key_len = strlen(spec_arr[i].property_key); > + if (spec_arr[i].property_key[property_key_len - 1] == '$' && > + strlen(key) == property_key_len - 1 && > + strncmp(spec_arr[i].property_key, key, property_key_len - 1) == 0) { > + break; > + } > + if (strncmp(spec_arr[i].property_key, key, property_key_len) == 0) { > break; > } > if (strncmp(spec_arr[i].property_key, "*", 1) == 0) > -- > 2.15.0.rc0.271.g36b669edcc-goog > >
On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux <selinux@tycho.nsa.gov> wrote: > Please hold off on submission. We're discussing if this is really necessary. Yeah I'd like to hear about what issues the current longest match logic was causing in the commit message. > > On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux > <selinux@tycho.nsa.gov> wrote: >> Performs exact match if a property key of property contexts ends with '$' >> instead of prefix match. This seems like an overly verbose way to accomplish exact match. The property_contexts file has things like: * <-- match everything foo.bar. <- match prefix foo.bar. properties foo.bar.baz <-- currently matches foo.bar.baz, foo.bar.bazbaz, etc. No trailing . could be changed to mean exact match. Really what you would want is that if it doesn't end with a dot, don't do a prefix match. No need to add the $ semantic AFAICT. >> >> This will enable to define an exact rule which can avoid unexpected >> context assignment. >> >> Signed-off-by: Jaekyun Seok <jaekyun@google.com> >> --- >> libselinux/src/label_backends_android.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/libselinux/src/label_backends_android.c b/libselinux/src/label_backends_android.c >> index cb8aae26..4611d396 100644 >> --- a/libselinux/src/label_backends_android.c >> +++ b/libselinux/src/label_backends_android.c >> @@ -258,8 +258,13 @@ static struct selabel_lookup_rec *property_lookup(struct selabel_handle *rec, >> } >> >> for (i = 0; i < data->nspec; i++) { >> - if (strncmp(spec_arr[i].property_key, key, >> - strlen(spec_arr[i].property_key)) == 0) { >> + size_t property_key_len = strlen(spec_arr[i].property_key); >> + if (spec_arr[i].property_key[property_key_len - 1] == '$' && >> + strlen(key) == property_key_len - 1 && >> + strncmp(spec_arr[i].property_key, key, property_key_len - 1) == 0) { >> + break; >> + } >> + if (strncmp(spec_arr[i].property_key, key, property_key_len) == 0) { >> break; >> } >> if (strncmp(spec_arr[i].property_key, "*", 1) == 0) >> -- >> 2.15.0.rc0.271.g36b669edcc-goog >> >> >
Please see my inline comments. Thanks, On Sat, Oct 21, 2017 at 1:02 AM, William Roberts <bill.c.roberts@gmail.com> wrote: > On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux > <selinux@tycho.nsa.gov> wrote: > > Please hold off on submission. We're discussing if this is really > necessary. > > Yeah I'd like to hear about what issues the current longest match > logic was causing > in the commit message. > I am working to whitelist properties which should be restricted from being accessed by some components. To do that, exact match should be supported. > > > > > On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux > > <selinux@tycho.nsa.gov> wrote: > >> Performs exact match if a property key of property contexts ends with > '$' > >> instead of prefix match. > > This seems like an overly verbose way to accomplish exact match. The > property_contexts > file has things like: > > * <-- match everything > foo.bar. <- match prefix foo.bar. properties > foo.bar.baz <-- currently matches foo.bar.baz, foo.bar.bazbaz, etc. No > trailing . > could be changed to mean exact match. > > Really what you would want is that if it doesn't end with a dot, don't > do a prefix > match. No need to add the $ semantic AFAICT. > Sounds good to me. I will discuss this way internally. > > >> > >> This will enable to define an exact rule which can avoid unexpected > >> context assignment. > >> > >> Signed-off-by: Jaekyun Seok <jaekyun@google.com> > >> --- > >> libselinux/src/label_backends_android.c | 9 +++++++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/libselinux/src/label_backends_android.c > b/libselinux/src/label_backends_android.c > >> index cb8aae26..4611d396 100644 > >> --- a/libselinux/src/label_backends_android.c > >> +++ b/libselinux/src/label_backends_android.c > >> @@ -258,8 +258,13 @@ static struct selabel_lookup_rec > *property_lookup(struct selabel_handle *rec, > >> } > >> > >> for (i = 0; i < data->nspec; i++) { > >> - if (strncmp(spec_arr[i].property_key, key, > >> - strlen(spec_arr[i].property_key)) == 0) { > >> + size_t property_key_len = strlen(spec_arr[i].property_ > key); > >> + if (spec_arr[i].property_key[property_key_len - 1] == > '$' && > >> + strlen(key) == property_key_len - 1 && > >> + strncmp(spec_arr[i].property_key, key, > property_key_len - 1) == 0) { > >> + break; > >> + } > >> + if (strncmp(spec_arr[i].property_key, key, > property_key_len) == 0) { > >> break; > >> } > >> if (strncmp(spec_arr[i].property_key, "*", 1) == 0) > >> -- > >> 2.15.0.rc0.271.g36b669edcc-goog > >> > >> > > > > > > -- > Respectfully, > > William C Roberts >
FYI, I've uploaded https://android-review.googlesource.com/#/c/platform/external/selinux/+/517958/ for public discussion. Thanks, On Mon, Oct 23, 2017 at 6:56 AM, Jaekyun Seok <jaekyun@google.com> wrote: > Please see my inline comments. > > Thanks, > > On Sat, Oct 21, 2017 at 1:02 AM, William Roberts <bill.c.roberts@gmail.com > > wrote: > >> On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux >> <selinux@tycho.nsa.gov> wrote: >> > Please hold off on submission. We're discussing if this is really >> necessary. >> >> Yeah I'd like to hear about what issues the current longest match >> logic was causing >> in the commit message. >> > > I am working to whitelist properties which should be restricted from being > accessed by some components. > > To do that, exact match should be supported. > > >> >> > >> > On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux >> > <selinux@tycho.nsa.gov> wrote: >> >> Performs exact match if a property key of property contexts ends with >> '$' >> >> instead of prefix match. >> >> This seems like an overly verbose way to accomplish exact match. The >> property_contexts >> file has things like: >> >> * <-- match everything >> foo.bar. <- match prefix foo.bar. properties >> foo.bar.baz <-- currently matches foo.bar.baz, foo.bar.bazbaz, etc. No >> trailing . >> could be changed to mean exact match. >> >> Really what you would want is that if it doesn't end with a dot, don't >> do a prefix >> match. No need to add the $ semantic AFAICT. >> > > Sounds good to me. I will discuss this way internally. > > >> >> >> >> >> This will enable to define an exact rule which can avoid unexpected >> >> context assignment. >> >> >> >> Signed-off-by: Jaekyun Seok <jaekyun@google.com> >> >> --- >> >> libselinux/src/label_backends_android.c | 9 +++++++-- >> >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/libselinux/src/label_backends_android.c >> b/libselinux/src/label_backends_android.c >> >> index cb8aae26..4611d396 100644 >> >> --- a/libselinux/src/label_backends_android.c >> >> +++ b/libselinux/src/label_backends_android.c >> >> @@ -258,8 +258,13 @@ static struct selabel_lookup_rec >> *property_lookup(struct selabel_handle *rec, >> >> } >> >> >> >> for (i = 0; i < data->nspec; i++) { >> >> - if (strncmp(spec_arr[i].property_key, key, >> >> - strlen(spec_arr[i].property_key)) == 0) { >> >> + size_t property_key_len = >> strlen(spec_arr[i].property_key); >> >> + if (spec_arr[i].property_key[property_key_len - 1] == >> '$' && >> >> + strlen(key) == property_key_len - 1 && >> >> + strncmp(spec_arr[i].property_key, key, >> property_key_len - 1) == 0) { >> >> + break; >> >> + } >> >> + if (strncmp(spec_arr[i].property_key, key, >> property_key_len) == 0) { >> >> break; >> >> } >> >> if (strncmp(spec_arr[i].property_key, "*", 1) == 0) >> >> -- >> >> 2.15.0.rc0.271.g36b669edcc-goog >> >> >> >> >> > >> >> >> >> -- >> Respectfully, >> >> William C Roberts >> > > > > -- > Jaekyun Seok | Software Engineer | jaekyun@google.com | +82 2 531 9235 > <+82%202-531-9235> >
diff --git a/libselinux/src/label_backends_android.c b/libselinux/src/label_backends_android.c index cb8aae26..4611d396 100644 --- a/libselinux/src/label_backends_android.c +++ b/libselinux/src/label_backends_android.c @@ -258,8 +258,13 @@ static struct selabel_lookup_rec *property_lookup(struct selabel_handle *rec, } for (i = 0; i < data->nspec; i++) { - if (strncmp(spec_arr[i].property_key, key, - strlen(spec_arr[i].property_key)) == 0) { + size_t property_key_len = strlen(spec_arr[i].property_key); + if (spec_arr[i].property_key[property_key_len - 1] == '$' && + strlen(key) == property_key_len - 1 && + strncmp(spec_arr[i].property_key, key, property_key_len - 1) == 0) { + break; + } + if (strncmp(spec_arr[i].property_key, key, property_key_len) == 0) { break; } if (strncmp(spec_arr[i].property_key, "*", 1) == 0)
Performs exact match if a property key of property contexts ends with '$' instead of prefix match. This will enable to define an exact rule which can avoid unexpected context assignment. Signed-off-by: Jaekyun Seok <jaekyun@google.com> --- libselinux/src/label_backends_android.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)