[PATCH] libselinux: android: support exact match for a property key

[Jaekyun Seok via Selinux]

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 <***@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)

[Jeffrey Vander Stoep via Selinux]

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

Post by Jaekyun Seok via Selinux
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.
---
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

[William Roberts]

On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux

Post by Jeffrey Vander Stoep via Selinux
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.

Post by Jeffrey Vander Stoep via Selinux
On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux

Post by Jaekyun Seok via Selinux
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.

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
This will enable to define an exact rule which can avoid unexpected
context assignment.
---
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

[Jaekyun Seok via Selinux]

Please see my inline comments.

Thanks,

Post by William Roberts
On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux

Post by Jeffrey Vander Stoep via Selinux
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.

Post by William Roberts

Post by Jeffrey Vander Stoep via Selinux
On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux

Post by Jaekyun Seok via Selinux
Performs exact match if a property key of property contexts ends with

'$'

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
instead of prefix match.

This seems like an overly verbose way to accomplish exact match. The
property_contexts
* <-- 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.

Post by William Roberts

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
This will enable to define an exact rule which can avoid unexpected
context assignment.
---
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

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
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,

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
}
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);

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
+ if (spec_arr[i].property_key[property_key_len - 1] ==

'$' &&

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
+ strlen(key) == property_key_len - 1 &&
+ strncmp(spec_arr[i].property_key, key,

property_key_len - 1) == 0) {

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
+ break;
+ }
+ if (strncmp(spec_arr[i].property_key, key,

property_key_len) == 0) {

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
break;
}
if (strncmp(spec_arr[i].property_key, "*", 1) == 0)
--
2.15.0.rc0.271.g36b669edcc-goog

--
Respectfully,
William C Roberts

[Jaekyun Seok via Selinux]

FYI, I've uploaded
https://android-review.googlesource.com/#/c/platform/external/selinux/+/517958/
for public discussion.

Thanks,

Post by Jaekyun Seok via Selinux
Please see my inline comments.
Thanks,

Post by William Roberts
On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux

Post by Jeffrey Vander Stoep via Selinux
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.

Post by William Roberts

Post by Jeffrey Vander Stoep via Selinux
On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux

Post by Jaekyun Seok via Selinux
Performs exact match if a property key of property contexts ends with

'$'

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
instead of prefix match.

This seems like an overly verbose way to accomplish exact match. The
property_contexts
* <-- 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.

Post by William Roberts

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
This will enable to define an exact rule which can avoid unexpected
context assignment.
---
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

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
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,

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
}
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);

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
+ if (spec_arr[i].property_key[property_key_len - 1] ==

'$' &&

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
+ strlen(key) == property_key_len - 1 &&
+ strncmp(spec_arr[i].property_key, key,

property_key_len - 1) == 0) {

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
+ break;
+ }
+ if (strncmp(spec_arr[i].property_key, key,

property_key_len) == 0) {

Post by Jeffrey Vander Stoep via Selinux

Post by Jaekyun Seok via Selinux
break;
}
if (strncmp(spec_arr[i].property_key, "*", 1) == 0)
--
2.15.0.rc0.271.g36b669edcc-goog

--
Respectfully,
William C Roberts

--
<+82%202-531-9235>