* [PATCH] libselinux: Strip spaces before values in config
@ 2022-01-13 20:43 Vit Mojzis
2022-01-15 17:28 ` Christian Göttsche
0 siblings, 1 reply; 9+ messages in thread
From: Vit Mojzis @ 2022-01-13 20:43 UTC (permalink / raw)
To: selinux
Spaces before values in /etc/selinux/config should be ignored just as
spaces after them are.
E.g. "SELINUXTYPE= targeted" should be a valid value.
Fixes:
# sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
# dnf install <any_package>
...
RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
RPM: error: Plugin selinux: hook tsm_pre failed
...
Error: Could not run transaction.
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libselinux/src/selinux_config.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
index 97f81a8b..99cd6124 100644
--- a/libselinux/src/selinux_config.c
+++ b/libselinux/src/selinux_config.c
@@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
FILE *cfg = fopen(SELINUXCONFIG, "re");
if (cfg) {
char *buf;
+ char *tag;
int len = sizeof(SELINUXTAG) - 1;
buf = malloc(selinux_page_size);
if (!buf) {
@@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
while (fgets_unlocked(buf, selinux_page_size, cfg)) {
if (strncmp(buf, SELINUXTAG, len))
continue;
+ tag = buf+len;
+ while (isspace(*tag))
+ tag++;
if (!strncasecmp
- (buf + len, "enforcing", sizeof("enforcing") - 1)) {
+ (tag, "enforcing", sizeof("enforcing") - 1)) {
*enforce = 1;
ret = 0;
break;
} else
if (!strncasecmp
- (buf + len, "permissive",
+ (tag, "permissive",
sizeof("permissive") - 1)) {
*enforce = 0;
ret = 0;
break;
} else
if (!strncasecmp
- (buf + len, "disabled",
+ (tag, "disabled",
sizeof("disabled") - 1)) {
*enforce = -1;
ret = 0;
@@ -176,7 +180,10 @@ static void init_selinux_config(void)
if (!strncasecmp(buf_p, SELINUXTYPETAG,
sizeof(SELINUXTYPETAG) - 1)) {
- type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
+ buf_p += sizeof(SELINUXTYPETAG) - 1;
+ while (isspace(*buf_p))
+ buf_p++;
+ type = strdup(buf_p);
if (!type) {
free(line_buf);
fclose(fp);
@@ -199,6 +206,8 @@ static void init_selinux_config(void)
} else if (!strncmp(buf_p, REQUIRESEUSERS,
sizeof(REQUIRESEUSERS) - 1)) {
value = buf_p + sizeof(REQUIRESEUSERS) - 1;
+ while (isspace(*value))
+ buf_p++;
intptr = &require_seusers;
} else {
continue;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] libselinux: Strip spaces before values in config
2022-01-13 20:43 [PATCH] libselinux: Strip spaces before values in config Vit Mojzis
@ 2022-01-15 17:28 ` Christian Göttsche
2022-01-17 9:42 ` Vit Mojzis
0 siblings, 1 reply; 9+ messages in thread
From: Christian Göttsche @ 2022-01-15 17:28 UTC (permalink / raw)
To: Vit Mojzis; +Cc: SElinux list
On Thu, 13 Jan 2022 at 21:44, Vit Mojzis <vmojzis@redhat.com> wrote:
>
> Spaces before values in /etc/selinux/config should be ignored just as
> spaces after them are.
>
> E.g. "SELINUXTYPE= targeted" should be a valid value.
>
> Fixes:
> # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
> # dnf install <any_package>
> ...
> RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
> RPM: error: Plugin selinux: hook tsm_pre failed
> ...
> Error: Could not run transaction.
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libselinux/src/selinux_config.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> index 97f81a8b..99cd6124 100644
> --- a/libselinux/src/selinux_config.c
> +++ b/libselinux/src/selinux_config.c
> @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
> FILE *cfg = fopen(SELINUXCONFIG, "re");
> if (cfg) {
> char *buf;
> + char *tag;
> int len = sizeof(SELINUXTAG) - 1;
> buf = malloc(selinux_page_size);
> if (!buf) {
> @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
> while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> if (strncmp(buf, SELINUXTAG, len))
> continue;
> + tag = buf+len;
> + while (isspace(*tag))
> + tag++;
> if (!strncasecmp
> - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
> + (tag, "enforcing", sizeof("enforcing") - 1)) {
> *enforce = 1;
> ret = 0;
> break;
> } else
> if (!strncasecmp
> - (buf + len, "permissive",
> + (tag, "permissive",
> sizeof("permissive") - 1)) {
> *enforce = 0;
> ret = 0;
> break;
> } else
> if (!strncasecmp
> - (buf + len, "disabled",
> + (tag, "disabled",
> sizeof("disabled") - 1)) {
> *enforce = -1;
> ret = 0;
> @@ -176,7 +180,10 @@ static void init_selinux_config(void)
>
> if (!strncasecmp(buf_p, SELINUXTYPETAG,
> sizeof(SELINUXTYPETAG) - 1)) {
> - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> + buf_p += sizeof(SELINUXTYPETAG) - 1;
> + while (isspace(*buf_p))
> + buf_p++;
> + type = strdup(buf_p);
> if (!type) {
> free(line_buf);
> fclose(fp);
> @@ -199,6 +206,8 @@ static void init_selinux_config(void)
> } else if (!strncmp(buf_p, REQUIRESEUSERS,
> sizeof(REQUIRESEUSERS) - 1)) {
> value = buf_p + sizeof(REQUIRESEUSERS) - 1;
> + while (isspace(*value))
> + buf_p++;
This looks wrong.
> intptr = &require_seusers;
> } else {
> continue;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] libselinux: Strip spaces before values in config
2022-01-15 17:28 ` Christian Göttsche
@ 2022-01-17 9:42 ` Vit Mojzis
2022-02-17 13:14 ` [PATCH v2] " Vit Mojzis
0 siblings, 1 reply; 9+ messages in thread
From: Vit Mojzis @ 2022-01-17 9:42 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On 15. 01. 22 18:28, Christian Göttsche wrote:
> On Thu, 13 Jan 2022 at 21:44, Vit Mojzis <vmojzis@redhat.com> wrote:
>> Spaces before values in /etc/selinux/config should be ignored just as
>> spaces after them are.
>>
>> E.g. "SELINUXTYPE= targeted" should be a valid value.
>>
>> Fixes:
>> # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
>> # dnf install <any_package>
>> ...
>> RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
>> RPM: error: Plugin selinux: hook tsm_pre failed
>> ...
>> Error: Could not run transaction.
>>
>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>> ---
>> libselinux/src/selinux_config.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
>> index 97f81a8b..99cd6124 100644
>> --- a/libselinux/src/selinux_config.c
>> +++ b/libselinux/src/selinux_config.c
>> @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
>> FILE *cfg = fopen(SELINUXCONFIG, "re");
>> if (cfg) {
>> char *buf;
>> + char *tag;
>> int len = sizeof(SELINUXTAG) - 1;
>> buf = malloc(selinux_page_size);
>> if (!buf) {
>> @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
>> while (fgets_unlocked(buf, selinux_page_size, cfg)) {
>> if (strncmp(buf, SELINUXTAG, len))
>> continue;
>> + tag = buf+len;
>> + while (isspace(*tag))
>> + tag++;
>> if (!strncasecmp
>> - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
>> + (tag, "enforcing", sizeof("enforcing") - 1)) {
>> *enforce = 1;
>> ret = 0;
>> break;
>> } else
>> if (!strncasecmp
>> - (buf + len, "permissive",
>> + (tag, "permissive",
>> sizeof("permissive") - 1)) {
>> *enforce = 0;
>> ret = 0;
>> break;
>> } else
>> if (!strncasecmp
>> - (buf + len, "disabled",
>> + (tag, "disabled",
>> sizeof("disabled") - 1)) {
>> *enforce = -1;
>> ret = 0;
>> @@ -176,7 +180,10 @@ static void init_selinux_config(void)
>>
>> if (!strncasecmp(buf_p, SELINUXTYPETAG,
>> sizeof(SELINUXTYPETAG) - 1)) {
>> - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
>> + buf_p += sizeof(SELINUXTYPETAG) - 1;
>> + while (isspace(*buf_p))
>> + buf_p++;
>> + type = strdup(buf_p);
>> if (!type) {
>> free(line_buf);
>> fclose(fp);
>> @@ -199,6 +206,8 @@ static void init_selinux_config(void)
>> } else if (!strncmp(buf_p, REQUIRESEUSERS,
>> sizeof(REQUIRESEUSERS) - 1)) {
>> value = buf_p + sizeof(REQUIRESEUSERS) - 1;
>> + while (isspace(*value))
>> + buf_p++;
> This looks wrong.
Yes, sorry, stupid mistake.
>
>> intptr = &require_seusers;
>> } else {
>> continue;
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] libselinux: Strip spaces before values in config
2022-01-17 9:42 ` Vit Mojzis
@ 2022-02-17 13:14 ` Vit Mojzis
2022-02-17 14:16 ` Christian Göttsche
2022-02-28 20:22 ` James Carter
0 siblings, 2 replies; 9+ messages in thread
From: Vit Mojzis @ 2022-02-17 13:14 UTC (permalink / raw)
To: selinux
Spaces before values in /etc/selinux/config should be ignored just as
spaces after them are.
E.g. "SELINUXTYPE= targeted" should be a valid value.
Fixes:
# sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
# dnf install <any_package>
...
RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
RPM: error: Plugin selinux: hook tsm_pre failed
...
Error: Could not run transaction.
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
Sorry for the delay. I sent the fixed patch to a wrong mailing list.
libselinux/src/selinux_config.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
index 97f81a8b..d2e49ee1 100644
--- a/libselinux/src/selinux_config.c
+++ b/libselinux/src/selinux_config.c
@@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
FILE *cfg = fopen(SELINUXCONFIG, "re");
if (cfg) {
char *buf;
+ char *tag;
int len = sizeof(SELINUXTAG) - 1;
buf = malloc(selinux_page_size);
if (!buf) {
@@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
while (fgets_unlocked(buf, selinux_page_size, cfg)) {
if (strncmp(buf, SELINUXTAG, len))
continue;
+ tag = buf+len;
+ while (isspace(*tag))
+ tag++;
if (!strncasecmp
- (buf + len, "enforcing", sizeof("enforcing") - 1)) {
+ (tag, "enforcing", sizeof("enforcing") - 1)) {
*enforce = 1;
ret = 0;
break;
} else
if (!strncasecmp
- (buf + len, "permissive",
+ (tag, "permissive",
sizeof("permissive") - 1)) {
*enforce = 0;
ret = 0;
break;
} else
if (!strncasecmp
- (buf + len, "disabled",
+ (tag, "disabled",
sizeof("disabled") - 1)) {
*enforce = -1;
ret = 0;
@@ -176,7 +180,10 @@ static void init_selinux_config(void)
if (!strncasecmp(buf_p, SELINUXTYPETAG,
sizeof(SELINUXTYPETAG) - 1)) {
- type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
+ buf_p += sizeof(SELINUXTYPETAG) - 1;
+ while (isspace(*buf_p))
+ buf_p++;
+ type = strdup(buf_p);
if (!type) {
free(line_buf);
fclose(fp);
@@ -199,6 +206,8 @@ static void init_selinux_config(void)
} else if (!strncmp(buf_p, REQUIRESEUSERS,
sizeof(REQUIRESEUSERS) - 1)) {
value = buf_p + sizeof(REQUIRESEUSERS) - 1;
+ while (isspace(*value))
+ value++;
intptr = &require_seusers;
} else {
continue;
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libselinux: Strip spaces before values in config
2022-02-17 13:14 ` [PATCH v2] " Vit Mojzis
@ 2022-02-17 14:16 ` Christian Göttsche
2022-02-21 10:31 ` Vit Mojzis
2022-02-28 20:22 ` James Carter
1 sibling, 1 reply; 9+ messages in thread
From: Christian Göttsche @ 2022-02-17 14:16 UTC (permalink / raw)
To: Vit Mojzis; +Cc: SElinux list
On Thu, 17 Feb 2022 at 14:14, Vit Mojzis <vmojzis@redhat.com> wrote:
>
> Spaces before values in /etc/selinux/config should be ignored just as
> spaces after them are.
>
> E.g. "SELINUXTYPE= targeted" should be a valid value.
>
> Fixes:
> # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
> # dnf install <any_package>
> ...
> RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
> RPM: error: Plugin selinux: hook tsm_pre failed
> ...
> Error: Could not run transaction.
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
>
> Sorry for the delay. I sent the fixed patch to a wrong mailing list.
>
> libselinux/src/selinux_config.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> index 97f81a8b..d2e49ee1 100644
> --- a/libselinux/src/selinux_config.c
> +++ b/libselinux/src/selinux_config.c
> @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
> FILE *cfg = fopen(SELINUXCONFIG, "re");
> if (cfg) {
> char *buf;
> + char *tag;
> int len = sizeof(SELINUXTAG) - 1;
> buf = malloc(selinux_page_size);
> if (!buf) {
> @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
> while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> if (strncmp(buf, SELINUXTAG, len))
> continue;
> + tag = buf+len;
> + while (isspace(*tag))
> + tag++;
> if (!strncasecmp
> - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
> + (tag, "enforcing", sizeof("enforcing") - 1)) {
> *enforce = 1;
> ret = 0;
> break;
> } else
> if (!strncasecmp
> - (buf + len, "permissive",
> + (tag, "permissive",
> sizeof("permissive") - 1)) {
> *enforce = 0;
> ret = 0;
> break;
> } else
> if (!strncasecmp
> - (buf + len, "disabled",
> + (tag, "disabled",
> sizeof("disabled") - 1)) {
> *enforce = -1;
> ret = 0;
> @@ -176,7 +180,10 @@ static void init_selinux_config(void)
>
> if (!strncasecmp(buf_p, SELINUXTYPETAG,
> sizeof(SELINUXTYPETAG) - 1)) {
> - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> + buf_p += sizeof(SELINUXTYPETAG) - 1;
> + while (isspace(*buf_p))
Strictly speaking one should cast to unsigned char to avoid UB, see
[1], but that
seems to be not done in SElinux userspace as
grep -REw "(isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower|isprint|ispunct|isspace|isupper|isxdigit|toascii|toupper|tolower)"
shows 87 usages.
[1]: https://wiki.sei.cmu.edu/confluence/display/c/STR37-C.+Arguments+to+character-handling+functions+must+be+representable+as+an+unsigned+char
> + buf_p++;
> + type = strdup(buf_p);
> if (!type) {
> free(line_buf);
> fclose(fp);
> @@ -199,6 +206,8 @@ static void init_selinux_config(void)
> } else if (!strncmp(buf_p, REQUIRESEUSERS,
> sizeof(REQUIRESEUSERS) - 1)) {
> value = buf_p + sizeof(REQUIRESEUSERS) - 1;
> + while (isspace(*value))
> + value++;
> intptr = &require_seusers;
> } else {
> continue;
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libselinux: Strip spaces before values in config
2022-02-17 14:16 ` Christian Göttsche
@ 2022-02-21 10:31 ` Vit Mojzis
2022-02-28 20:25 ` James Carter
0 siblings, 1 reply; 9+ messages in thread
From: Vit Mojzis @ 2022-02-21 10:31 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On 17. 02. 22 15:16, Christian Göttsche wrote:
> On Thu, 17 Feb 2022 at 14:14, Vit Mojzis <vmojzis@redhat.com> wrote:
>>
>> Spaces before values in /etc/selinux/config should be ignored just as
>> spaces after them are.
>>
>> E.g. "SELINUXTYPE= targeted" should be a valid value.
>>
>> Fixes:
>> # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
>> # dnf install <any_package>
>> ...
>> RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
>> RPM: error: Plugin selinux: hook tsm_pre failed
>> ...
>> Error: Could not run transaction.
>>
>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>> ---
>>
>> Sorry for the delay. I sent the fixed patch to a wrong mailing list.
>>
>> libselinux/src/selinux_config.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
>> index 97f81a8b..d2e49ee1 100644
>> --- a/libselinux/src/selinux_config.c
>> +++ b/libselinux/src/selinux_config.c
>> @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
>> FILE *cfg = fopen(SELINUXCONFIG, "re");
>> if (cfg) {
>> char *buf;
>> + char *tag;
>> int len = sizeof(SELINUXTAG) - 1;
>> buf = malloc(selinux_page_size);
>> if (!buf) {
>> @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
>> while (fgets_unlocked(buf, selinux_page_size, cfg)) {
>> if (strncmp(buf, SELINUXTAG, len))
>> continue;
>> + tag = buf+len;
>> + while (isspace(*tag))
>> + tag++;
>> if (!strncasecmp
>> - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
>> + (tag, "enforcing", sizeof("enforcing") - 1)) {
>> *enforce = 1;
>> ret = 0;
>> break;
>> } else
>> if (!strncasecmp
>> - (buf + len, "permissive",
>> + (tag, "permissive",
>> sizeof("permissive") - 1)) {
>> *enforce = 0;
>> ret = 0;
>> break;
>> } else
>> if (!strncasecmp
>> - (buf + len, "disabled",
>> + (tag, "disabled",
>> sizeof("disabled") - 1)) {
>> *enforce = -1;
>> ret = 0;
>> @@ -176,7 +180,10 @@ static void init_selinux_config(void)
>>
>> if (!strncasecmp(buf_p, SELINUXTYPETAG,
>> sizeof(SELINUXTYPETAG) - 1)) {
>> - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
>> + buf_p += sizeof(SELINUXTYPETAG) - 1;
>> + while (isspace(*buf_p))
>
> Strictly speaking one should cast to unsigned char to avoid UB, see
> [1], but that
> seems to be not done in SElinux userspace as
>
> grep -REw "(isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower|isprint|ispunct|isspace|isupper|isxdigit|toascii|toupper|tolower)"
>
> shows 87 usages.
>
> [1]: https://wiki.sei.cmu.edu/confluence/display/c/STR37-C.+Arguments+to+character-handling+functions+must+be+representable+as+an+unsigned+char
>
Right, thank you. So, should I fix the patch (which would make the use
of the cast inconsistent in selinux_config.c), or leave it as is?
If this is something worth fixing I can make a new patch adding the cast
to all the calls (except for cases where EOF can be expected? -- not
sure if we need to watch for that).
Vit
>> + buf_p++;
>> + type = strdup(buf_p);
>> if (!type) {
>> free(line_buf);
>> fclose(fp);
>> @@ -199,6 +206,8 @@ static void init_selinux_config(void)
>> } else if (!strncmp(buf_p, REQUIRESEUSERS,
>> sizeof(REQUIRESEUSERS) - 1)) {
>> value = buf_p + sizeof(REQUIRESEUSERS) - 1;
>> + while (isspace(*value))
>> + value++;
>> intptr = &require_seusers;
>> } else {
>> continue;
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libselinux: Strip spaces before values in config
2022-02-17 13:14 ` [PATCH v2] " Vit Mojzis
2022-02-17 14:16 ` Christian Göttsche
@ 2022-02-28 20:22 ` James Carter
2022-03-03 18:18 ` James Carter
1 sibling, 1 reply; 9+ messages in thread
From: James Carter @ 2022-02-28 20:22 UTC (permalink / raw)
To: Vit Mojzis; +Cc: SElinux list
On Thu, Feb 17, 2022 at 1:24 PM Vit Mojzis <vmojzis@redhat.com> wrote:
>
> Spaces before values in /etc/selinux/config should be ignored just as
> spaces after them are.
>
> E.g. "SELINUXTYPE= targeted" should be a valid value.
>
> Fixes:
> # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
> # dnf install <any_package>
> ...
> RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
> RPM: error: Plugin selinux: hook tsm_pre failed
> ...
> Error: Could not run transaction.
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
> ---
>
> Sorry for the delay. I sent the fixed patch to a wrong mailing list.
>
> libselinux/src/selinux_config.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> index 97f81a8b..d2e49ee1 100644
> --- a/libselinux/src/selinux_config.c
> +++ b/libselinux/src/selinux_config.c
> @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
> FILE *cfg = fopen(SELINUXCONFIG, "re");
> if (cfg) {
> char *buf;
> + char *tag;
> int len = sizeof(SELINUXTAG) - 1;
> buf = malloc(selinux_page_size);
> if (!buf) {
> @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
> while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> if (strncmp(buf, SELINUXTAG, len))
> continue;
> + tag = buf+len;
> + while (isspace(*tag))
> + tag++;
> if (!strncasecmp
> - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
> + (tag, "enforcing", sizeof("enforcing") - 1)) {
> *enforce = 1;
> ret = 0;
> break;
> } else
> if (!strncasecmp
> - (buf + len, "permissive",
> + (tag, "permissive",
> sizeof("permissive") - 1)) {
> *enforce = 0;
> ret = 0;
> break;
> } else
> if (!strncasecmp
> - (buf + len, "disabled",
> + (tag, "disabled",
> sizeof("disabled") - 1)) {
> *enforce = -1;
> ret = 0;
> @@ -176,7 +180,10 @@ static void init_selinux_config(void)
>
> if (!strncasecmp(buf_p, SELINUXTYPETAG,
> sizeof(SELINUXTYPETAG) - 1)) {
> - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> + buf_p += sizeof(SELINUXTYPETAG) - 1;
> + while (isspace(*buf_p))
> + buf_p++;
> + type = strdup(buf_p);
> if (!type) {
> free(line_buf);
> fclose(fp);
> @@ -199,6 +206,8 @@ static void init_selinux_config(void)
> } else if (!strncmp(buf_p, REQUIRESEUSERS,
> sizeof(REQUIRESEUSERS) - 1)) {
> value = buf_p + sizeof(REQUIRESEUSERS) - 1;
> + while (isspace(*value))
> + value++;
> intptr = &require_seusers;
> } else {
> continue;
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libselinux: Strip spaces before values in config
2022-02-21 10:31 ` Vit Mojzis
@ 2022-02-28 20:25 ` James Carter
0 siblings, 0 replies; 9+ messages in thread
From: James Carter @ 2022-02-28 20:25 UTC (permalink / raw)
To: Vit Mojzis; +Cc: Christian Göttsche, SElinux list
On Mon, Feb 21, 2022 at 8:04 AM Vit Mojzis <vmojzis@redhat.com> wrote:
>
> On 17. 02. 22 15:16, Christian Göttsche wrote:
> > On Thu, 17 Feb 2022 at 14:14, Vit Mojzis <vmojzis@redhat.com> wrote:
> >>
> >> Spaces before values in /etc/selinux/config should be ignored just as
> >> spaces after them are.
> >>
> >> E.g. "SELINUXTYPE= targeted" should be a valid value.
> >>
> >> Fixes:
> >> # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
> >> # dnf install <any_package>
> >> ...
> >> RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
> >> RPM: error: Plugin selinux: hook tsm_pre failed
> >> ...
> >> Error: Could not run transaction.
> >>
> >> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> >> ---
> >>
> >> Sorry for the delay. I sent the fixed patch to a wrong mailing list.
> >>
> >> libselinux/src/selinux_config.c | 17 +++++++++++++----
> >> 1 file changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> >> index 97f81a8b..d2e49ee1 100644
> >> --- a/libselinux/src/selinux_config.c
> >> +++ b/libselinux/src/selinux_config.c
> >> @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
> >> FILE *cfg = fopen(SELINUXCONFIG, "re");
> >> if (cfg) {
> >> char *buf;
> >> + char *tag;
> >> int len = sizeof(SELINUXTAG) - 1;
> >> buf = malloc(selinux_page_size);
> >> if (!buf) {
> >> @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
> >> while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> >> if (strncmp(buf, SELINUXTAG, len))
> >> continue;
> >> + tag = buf+len;
> >> + while (isspace(*tag))
> >> + tag++;
> >> if (!strncasecmp
> >> - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
> >> + (tag, "enforcing", sizeof("enforcing") - 1)) {
> >> *enforce = 1;
> >> ret = 0;
> >> break;
> >> } else
> >> if (!strncasecmp
> >> - (buf + len, "permissive",
> >> + (tag, "permissive",
> >> sizeof("permissive") - 1)) {
> >> *enforce = 0;
> >> ret = 0;
> >> break;
> >> } else
> >> if (!strncasecmp
> >> - (buf + len, "disabled",
> >> + (tag, "disabled",
> >> sizeof("disabled") - 1)) {
> >> *enforce = -1;
> >> ret = 0;
> >> @@ -176,7 +180,10 @@ static void init_selinux_config(void)
> >>
> >> if (!strncasecmp(buf_p, SELINUXTYPETAG,
> >> sizeof(SELINUXTYPETAG) - 1)) {
> >> - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> >> + buf_p += sizeof(SELINUXTYPETAG) - 1;
> >> + while (isspace(*buf_p))
> >
> > Strictly speaking one should cast to unsigned char to avoid UB, see
> > [1], but that
> > seems to be not done in SElinux userspace as
> >
> > grep -REw "(isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower|isprint|ispunct|isspace|isupper|isxdigit|toascii|toupper|tolower)"
> >
> > shows 87 usages.
> >
> > [1]: https://wiki.sei.cmu.edu/confluence/display/c/STR37-C.+Arguments+to+character-handling+functions+must+be+representable+as+an+unsigned+char
> >
>
>
> Right, thank you. So, should I fix the patch (which would make the use
> of the cast inconsistent in selinux_config.c), or leave it as is?
> If this is something worth fixing I can make a new patch adding the cast
> to all the calls (except for cases where EOF can be expected? -- not
> sure if we need to watch for that).
>
> Vit
>
Since, we aren't casting it to unsigned char anywhere else, I think
the patch is fine as it is. If we feel the need to fix up the other 87
usages someday, then we can fix this one then. It seems unlikely to
cause any problems.
Jim
>
> >> + buf_p++;
> >> + type = strdup(buf_p);
> >> if (!type) {
> >> free(line_buf);
> >> fclose(fp);
> >> @@ -199,6 +206,8 @@ static void init_selinux_config(void)
> >> } else if (!strncmp(buf_p, REQUIRESEUSERS,
> >> sizeof(REQUIRESEUSERS) - 1)) {
> >> value = buf_p + sizeof(REQUIRESEUSERS) - 1;
> >> + while (isspace(*value))
> >> + value++;
> >> intptr = &require_seusers;
> >> } else {
> >> continue;
> >> --
> >> 2.30.2
> >>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libselinux: Strip spaces before values in config
2022-02-28 20:22 ` James Carter
@ 2022-03-03 18:18 ` James Carter
0 siblings, 0 replies; 9+ messages in thread
From: James Carter @ 2022-03-03 18:18 UTC (permalink / raw)
To: Vit Mojzis; +Cc: SElinux list
On Mon, Feb 28, 2022 at 3:22 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Feb 17, 2022 at 1:24 PM Vit Mojzis <vmojzis@redhat.com> wrote:
> >
> > Spaces before values in /etc/selinux/config should be ignored just as
> > spaces after them are.
> >
> > E.g. "SELINUXTYPE= targeted" should be a valid value.
> >
> > Fixes:
> > # sed -i 's/^SELINUXTYPE=/SELINUXTYPE= /g' /etc/selinux/config
> > # dnf install <any_package>
> > ...
> > RPM: error: selabel_open: (/etc/selinux/ targeted/contexts/files/file_contexts) No such file or directory
> > RPM: error: Plugin selinux: hook tsm_pre failed
> > ...
> > Error: Could not run transaction.
> >
> > Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>
Merged.
Thanks,
Jim
> > ---
> >
> > Sorry for the delay. I sent the fixed patch to a wrong mailing list.
> >
> > libselinux/src/selinux_config.c | 17 +++++++++++++----
> > 1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> > index 97f81a8b..d2e49ee1 100644
> > --- a/libselinux/src/selinux_config.c
> > +++ b/libselinux/src/selinux_config.c
> > @@ -92,6 +92,7 @@ int selinux_getenforcemode(int *enforce)
> > FILE *cfg = fopen(SELINUXCONFIG, "re");
> > if (cfg) {
> > char *buf;
> > + char *tag;
> > int len = sizeof(SELINUXTAG) - 1;
> > buf = malloc(selinux_page_size);
> > if (!buf) {
> > @@ -101,21 +102,24 @@ int selinux_getenforcemode(int *enforce)
> > while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> > if (strncmp(buf, SELINUXTAG, len))
> > continue;
> > + tag = buf+len;
> > + while (isspace(*tag))
> > + tag++;
> > if (!strncasecmp
> > - (buf + len, "enforcing", sizeof("enforcing") - 1)) {
> > + (tag, "enforcing", sizeof("enforcing") - 1)) {
> > *enforce = 1;
> > ret = 0;
> > break;
> > } else
> > if (!strncasecmp
> > - (buf + len, "permissive",
> > + (tag, "permissive",
> > sizeof("permissive") - 1)) {
> > *enforce = 0;
> > ret = 0;
> > break;
> > } else
> > if (!strncasecmp
> > - (buf + len, "disabled",
> > + (tag, "disabled",
> > sizeof("disabled") - 1)) {
> > *enforce = -1;
> > ret = 0;
> > @@ -176,7 +180,10 @@ static void init_selinux_config(void)
> >
> > if (!strncasecmp(buf_p, SELINUXTYPETAG,
> > sizeof(SELINUXTYPETAG) - 1)) {
> > - type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> > + buf_p += sizeof(SELINUXTYPETAG) - 1;
> > + while (isspace(*buf_p))
> > + buf_p++;
> > + type = strdup(buf_p);
> > if (!type) {
> > free(line_buf);
> > fclose(fp);
> > @@ -199,6 +206,8 @@ static void init_selinux_config(void)
> > } else if (!strncmp(buf_p, REQUIRESEUSERS,
> > sizeof(REQUIRESEUSERS) - 1)) {
> > value = buf_p + sizeof(REQUIRESEUSERS) - 1;
> > + while (isspace(*value))
> > + value++;
> > intptr = &require_seusers;
> > } else {
> > continue;
> > --
> > 2.30.2
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-03 18:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 20:43 [PATCH] libselinux: Strip spaces before values in config Vit Mojzis
2022-01-15 17:28 ` Christian Göttsche
2022-01-17 9:42 ` Vit Mojzis
2022-02-17 13:14 ` [PATCH v2] " Vit Mojzis
2022-02-17 14:16 ` Christian Göttsche
2022-02-21 10:31 ` Vit Mojzis
2022-02-28 20:25 ` James Carter
2022-02-28 20:22 ` James Carter
2022-03-03 18:18 ` James Carter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).