* [PATCH] libselinux: selinux_set_mapping: fix handling of unknown classes/perms
@ 2019-02-22 14:41 Stephen Smalley
2019-02-22 16:11 ` Stephen Smalley
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Smalley @ 2019-02-22 14:41 UTC (permalink / raw)
To: paul; +Cc: dominick.grift, selinux, Stephen Smalley
The libselinux selinux_set_mapping() implementation was never updated
to handle unknown classes/permissions based on the policy handle_unknown
flag. Update it and the internal mapping functions to gracefully
handle unknown classes/permissions. Add a security_reject_unknown()
interface to expose the corresponding selinuxfs node and use it when
creating a mapping to decide whether to fail immediately or proceed.
This enables dbus-daemon and XSELinux, which use selinux_set_mapping(),
to continue working with the dummy policy or other policies that lack
their userspace class/permission definitions as long as the policy
was built with -U allow.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
libselinux/include/selinux/selinux.h | 5 +-
libselinux/src/compute_av.c | 16 +++++--
libselinux/src/mapping.c | 71 ++++++++++++++++++++++------
libselinux/src/mapping.h | 4 +-
libselinux/src/reject_unknown.c | 40 ++++++++++++++++
libselinux/src/selinux_internal.h | 1 +
6 files changed, 117 insertions(+), 20 deletions(-)
create mode 100644 libselinux/src/reject_unknown.c
diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h
index 01201eee..a34d54fc 100644
--- a/libselinux/include/selinux/selinux.h
+++ b/libselinux/include/selinux/selinux.h
@@ -328,7 +328,10 @@ extern int security_getenforce(void);
/* Set the enforce flag value. */
extern int security_setenforce(int value);
-/* Get the behavior for undefined classes/permissions */
+/* Get the load-time behavior for undefined classes/permissions */
+extern int security_reject_unknown(void);
+
+/* Get the runtime behavior for undefined classes/permissions */
extern int security_deny_unknown(void);
/* Get the checkreqprot value */
diff --git a/libselinux/src/compute_av.c b/libselinux/src/compute_av.c
index 1d05e7b6..d5741637 100644
--- a/libselinux/src/compute_av.c
+++ b/libselinux/src/compute_av.c
@@ -20,6 +20,7 @@ int security_compute_av_flags_raw(const char * scon,
char *buf;
size_t len;
int fd, ret;
+ security_class_t kclass;
if (!selinux_mnt) {
errno = ENOENT;
@@ -38,8 +39,9 @@ int security_compute_av_flags_raw(const char * scon,
goto out;
}
+ kclass = unmap_class(tclass);
snprintf(buf, len, "%s %s %hu %x", scon, tcon,
- unmap_class(tclass), unmap_perm(tclass, requested));
+ kclass, unmap_perm(tclass, requested));
ret = write(fd, buf, strlen(buf));
if (ret < 0)
@@ -60,9 +62,15 @@ int security_compute_av_flags_raw(const char * scon,
} else if (ret < 6)
avd->flags = 0;
- /* If tclass invalid, kernel sets avd according to deny_unknown flag */
- if (tclass != 0)
- map_decision(tclass, avd);
+ /*
+ * If the tclass could not be mapped to a kernel class at all, the
+ * kernel will have already set avd according to the
+ * handle_unknown flag and we do not need to do anything further.
+ * Otherwise, we must map the permissions within the returned
+ * avd to the userspace permission values.
+ */
+ if (kclass != 0)
+ map_decision(tclass, avd, !security_deny_unknown());
ret = 0;
out2:
diff --git a/libselinux/src/mapping.c b/libselinux/src/mapping.c
index f205804b..892f887e 100644
--- a/libselinux/src/mapping.c
+++ b/libselinux/src/mapping.c
@@ -8,7 +8,9 @@
#include <stdarg.h>
#include <selinux/selinux.h>
#include <selinux/avc.h>
+#include "callbacks.h"
#include "mapping.h"
+#include "selinux_internal.h"
/*
* Class and permission mappings
@@ -33,6 +35,9 @@ selinux_set_mapping(struct security_class_mapping *map)
size_t size = sizeof(struct selinux_mapping);
security_class_t i, j;
unsigned k;
+ bool print_unknown_handle = false;
+ bool reject = (security_reject_unknown() == 1);
+ bool deny = (security_deny_unknown() == 1);
free(current_mapping);
current_mapping = NULL;
@@ -62,8 +67,16 @@ selinux_set_mapping(struct security_class_mapping *map)
struct selinux_mapping *p_out = current_mapping + j;
p_out->value = string_to_security_class(p_in->name);
- if (!p_out->value)
- goto err2;
+ if (!p_out->value) {
+ selinux_log(SELINUX_INFO,
+ "SELinux: Class %s not defined in policy.\n",
+ p_in->name);
+ if (reject)
+ goto err2;
+ p_out->num_perms = 0;
+ print_unknown_handle = true;
+ continue;
+ }
k = 0;
while (p_in->perms[k]) {
@@ -74,13 +87,24 @@ selinux_set_mapping(struct security_class_mapping *map)
}
p_out->perms[k] = string_to_av_perm(p_out->value,
p_in->perms[k]);
- if (!p_out->perms[k])
- goto err2;
+ if (!p_out->perms[k]) {
+ selinux_log(SELINUX_INFO,
+ "SELinux: Permission %s in class %s not defined in policy.\n",
+ p_in->perms[k], p_in->name);
+ if (reject)
+ goto err2;
+ print_unknown_handle = true;
+ }
k++;
}
p_out->num_perms = k;
}
+ if (print_unknown_handle)
+ selinux_log(SELINUX_INFO,
+ "SELinux: the above unknown classes and permissions will be %s\n",
+ deny ? "denied" : "allowed");
+
/* Set the mapping size here so the above lookups are "raw" */
current_mapping_size = i;
return 0;
@@ -181,30 +205,49 @@ map_perm(security_class_t tclass, access_vector_t kperm)
}
void
-map_decision(security_class_t tclass, struct av_decision *avd)
+map_decision(security_class_t tclass, struct av_decision *avd,
+ bool allow_unknown)
{
if (tclass < current_mapping_size) {
- unsigned i;
+ struct selinux_mapping *mapping = ¤t_mapping[tclass];
+ unsigned int i, n = mapping->num_perms;
access_vector_t result;
- for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
- if (avd->allowed & current_mapping[tclass].perms[i])
+ for (i = 0, result = 0; i < n; i++) {
+ if (avd->allowed & mapping->perms[i])
+ result |= 1<<i;
+ if (allow_unknown && !mapping->perms[i])
result |= 1<<i;
+ }
avd->allowed = result;
- for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
- if (avd->decided & current_mapping[tclass].perms[i])
+ for (i = 0, result = 0; i < n; i++) {
+ if (avd->decided & mapping->perms[i])
+ result |= 1<<i;
+ if (allow_unknown && !mapping->perms[i])
result |= 1<<i;
+ }
avd->decided = result;
- for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
- if (avd->auditallow & current_mapping[tclass].perms[i])
+ for (i = 0, result = 0; i < n; i++)
+ if (avd->auditallow & mapping->perms[i])
result |= 1<<i;
avd->auditallow = result;
- for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
- if (avd->auditdeny & current_mapping[tclass].perms[i])
+ for (i = 0, result = 0; i < n; i++) {
+ if (avd->auditdeny & mapping->perms[i])
result |= 1<<i;
+ if (!allow_unknown && !mapping->perms[i])
+ result |= 1<<i;
+ }
+
+ /*
+ * Make sure we audit denials for any permission check
+ * beyond the mapping->num_perms since this indicates
+ * a bug in the object manager.
+ */
+ for (; i < (sizeof(result)*8); i++)
+ result |= 1<<i;
avd->auditdeny = result;
}
}
diff --git a/libselinux/src/mapping.h b/libselinux/src/mapping.h
index 22a4ccaf..12ebb89f 100644
--- a/libselinux/src/mapping.h
+++ b/libselinux/src/mapping.h
@@ -7,6 +7,7 @@
#define _SELINUX_MAPPING_H_
#include <selinux/selinux.h>
+#include <stdbool.h>
/*
* Get real, kernel values from mapped values
@@ -29,6 +30,7 @@ extern access_vector_t
map_perm(security_class_t tclass, access_vector_t kperm);
extern void
-map_decision(security_class_t tclass, struct av_decision *avd);
+map_decision(security_class_t tclass, struct av_decision *avd,
+ bool allow_unknown);
#endif /* _SELINUX_MAPPING_H_ */
diff --git a/libselinux/src/reject_unknown.c b/libselinux/src/reject_unknown.c
new file mode 100644
index 00000000..5c1d3605
--- /dev/null
+++ b/libselinux/src/reject_unknown.c
@@ -0,0 +1,40 @@
+#include <unistd.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include "selinux_internal.h"
+#include "policy.h"
+#include <stdio.h>
+#include <limits.h>
+
+int security_reject_unknown(void)
+{
+ int fd, ret, reject_unknown = 0;
+ char path[PATH_MAX];
+ char buf[20];
+
+ if (!selinux_mnt) {
+ errno = ENOENT;
+ return -1;
+ }
+
+ snprintf(path, sizeof(path), "%s/reject_unknown", selinux_mnt);
+ fd = open(path, O_RDONLY | O_CLOEXEC);
+ if (fd < 0)
+ return -1;
+
+ memset(buf, 0, sizeof(buf));
+ ret = read(fd, buf, sizeof(buf) - 1);
+ close(fd);
+ if (ret < 0)
+ return -1;
+
+ if (sscanf(buf, "%d", &reject_unknown) != 1)
+ return -1;
+
+ return reject_unknown;
+}
+
+hidden_def(security_reject_unknown);
diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
index dfc421cc..70b5025d 100644
--- a/libselinux/src/selinux_internal.h
+++ b/libselinux/src/selinux_internal.h
@@ -59,6 +59,7 @@ hidden_proto(selinux_mkload_policy)
hidden_proto(security_getenforce)
hidden_proto(security_setenforce)
hidden_proto(security_deny_unknown)
+ hidden_proto(security_reject_unknown)
hidden_proto(security_get_checkreqprot)
hidden_proto(selinux_boolean_sub)
hidden_proto(selinux_current_policy_path)
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] libselinux: selinux_set_mapping: fix handling of unknown classes/perms
2019-02-22 14:41 [PATCH] libselinux: selinux_set_mapping: fix handling of unknown classes/perms Stephen Smalley
@ 2019-02-22 16:11 ` Stephen Smalley
2019-02-22 16:14 ` Stephen Smalley
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Smalley @ 2019-02-22 16:11 UTC (permalink / raw)
To: paul; +Cc: dominick.grift, selinux
On 2/22/19 9:41 AM, Stephen Smalley wrote:
> The libselinux selinux_set_mapping() implementation was never updated
> to handle unknown classes/permissions based on the policy handle_unknown
> flag. Update it and the internal mapping functions to gracefully
> handle unknown classes/permissions. Add a security_reject_unknown()
> interface to expose the corresponding selinuxfs node and use it when
> creating a mapping to decide whether to fail immediately or proceed.
>
> This enables dbus-daemon and XSELinux, which use selinux_set_mapping(),
> to continue working with the dummy policy or other policies that lack
> their userspace class/permission definitions as long as the policy
> was built with -U allow.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> libselinux/include/selinux/selinux.h | 5 +-
> libselinux/src/compute_av.c | 16 +++++--
> libselinux/src/mapping.c | 71 ++++++++++++++++++++++------
> libselinux/src/mapping.h | 4 +-
> libselinux/src/reject_unknown.c | 40 ++++++++++++++++
> libselinux/src/selinux_internal.h | 1 +
> 6 files changed, 117 insertions(+), 20 deletions(-)
> create mode 100644 libselinux/src/reject_unknown.c
>
> diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h
> index 01201eee..a34d54fc 100644
> --- a/libselinux/include/selinux/selinux.h
> +++ b/libselinux/include/selinux/selinux.h
> @@ -328,7 +328,10 @@ extern int security_getenforce(void);
> /* Set the enforce flag value. */
> extern int security_setenforce(int value);
>
> -/* Get the behavior for undefined classes/permissions */
> +/* Get the load-time behavior for undefined classes/permissions */
> +extern int security_reject_unknown(void);
> +
> +/* Get the runtime behavior for undefined classes/permissions */
> extern int security_deny_unknown(void);
>
> /* Get the checkreqprot value */
> diff --git a/libselinux/src/compute_av.c b/libselinux/src/compute_av.c
> index 1d05e7b6..d5741637 100644
> --- a/libselinux/src/compute_av.c
> +++ b/libselinux/src/compute_av.c
> @@ -20,6 +20,7 @@ int security_compute_av_flags_raw(const char * scon,
> char *buf;
> size_t len;
> int fd, ret;
> + security_class_t kclass;
>
> if (!selinux_mnt) {
> errno = ENOENT;
> @@ -38,8 +39,9 @@ int security_compute_av_flags_raw(const char * scon,
> goto out;
> }
>
> + kclass = unmap_class(tclass);
> snprintf(buf, len, "%s %s %hu %x", scon, tcon,
> - unmap_class(tclass), unmap_perm(tclass, requested));
> + kclass, unmap_perm(tclass, requested));
>
> ret = write(fd, buf, strlen(buf));
> if (ret < 0)
> @@ -60,9 +62,15 @@ int security_compute_av_flags_raw(const char * scon,
> } else if (ret < 6)
> avd->flags = 0;
>
> - /* If tclass invalid, kernel sets avd according to deny_unknown flag */
> - if (tclass != 0)
> - map_decision(tclass, avd);
> + /*
> + * If the tclass could not be mapped to a kernel class at all, the
> + * kernel will have already set avd according to the
> + * handle_unknown flag and we do not need to do anything further.
> + * Otherwise, we must map the permissions within the returned
> + * avd to the userspace permission values.
> + */
> + if (kclass != 0)
> + map_decision(tclass, avd, !security_deny_unknown());
We probably want to cache security_deny_unknown() and refresh it upon a
policy reload. If the callers were to use selinux_status_open() during
setup, they could get the current value via
selinux_status_deny_unknown() without requiring any further system
calls. Or if using netlink selinux notifications, they could call
security_deny_unknown() again only when they receive a
SELNL_MSG_POLICYLOAD message. Not clear how to best integrate it here
since security_compute_av*() can be called directly or via the AVC.
>
> ret = 0;
> out2:
> diff --git a/libselinux/src/mapping.c b/libselinux/src/mapping.c
> index f205804b..892f887e 100644
> --- a/libselinux/src/mapping.c
> +++ b/libselinux/src/mapping.c
> @@ -8,7 +8,9 @@
> #include <stdarg.h>
> #include <selinux/selinux.h>
> #include <selinux/avc.h>
> +#include "callbacks.h"
> #include "mapping.h"
> +#include "selinux_internal.h"
>
> /*
> * Class and permission mappings
> @@ -33,6 +35,9 @@ selinux_set_mapping(struct security_class_mapping *map)
> size_t size = sizeof(struct selinux_mapping);
> security_class_t i, j;
> unsigned k;
> + bool print_unknown_handle = false;
> + bool reject = (security_reject_unknown() == 1);
> + bool deny = (security_deny_unknown() == 1);
>
> free(current_mapping);
> current_mapping = NULL;
> @@ -62,8 +67,16 @@ selinux_set_mapping(struct security_class_mapping *map)
> struct selinux_mapping *p_out = current_mapping + j;
>
> p_out->value = string_to_security_class(p_in->name);
> - if (!p_out->value)
> - goto err2;
> + if (!p_out->value) {
> + selinux_log(SELINUX_INFO,
> + "SELinux: Class %s not defined in policy.\n",
> + p_in->name);
> + if (reject)
> + goto err2;
> + p_out->num_perms = 0;
> + print_unknown_handle = true;
> + continue;
> + }
>
> k = 0;
> while (p_in->perms[k]) {
> @@ -74,13 +87,24 @@ selinux_set_mapping(struct security_class_mapping *map)
> }
> p_out->perms[k] = string_to_av_perm(p_out->value,
> p_in->perms[k]);
> - if (!p_out->perms[k])
> - goto err2;
> + if (!p_out->perms[k]) {
> + selinux_log(SELINUX_INFO,
> + "SELinux: Permission %s in class %s not defined in policy.\n",
> + p_in->perms[k], p_in->name);
> + if (reject)
> + goto err2;
> + print_unknown_handle = true;
> + }
> k++;
> }
> p_out->num_perms = k;
> }
>
> + if (print_unknown_handle)
> + selinux_log(SELINUX_INFO,
> + "SELinux: the above unknown classes and permissions will be %s\n",
> + deny ? "denied" : "allowed");
> +
> /* Set the mapping size here so the above lookups are "raw" */
> current_mapping_size = i;
> return 0;
> @@ -181,30 +205,49 @@ map_perm(security_class_t tclass, access_vector_t kperm)
> }
>
> void
> -map_decision(security_class_t tclass, struct av_decision *avd)
> +map_decision(security_class_t tclass, struct av_decision *avd,
> + bool allow_unknown)
> {
> if (tclass < current_mapping_size) {
> - unsigned i;
> + struct selinux_mapping *mapping = ¤t_mapping[tclass];
> + unsigned int i, n = mapping->num_perms;
> access_vector_t result;
>
> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
> - if (avd->allowed & current_mapping[tclass].perms[i])
> + for (i = 0, result = 0; i < n; i++) {
> + if (avd->allowed & mapping->perms[i])
> + result |= 1<<i;
> + if (allow_unknown && !mapping->perms[i])
> result |= 1<<i;
> + }
> avd->allowed = result;
>
> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
> - if (avd->decided & current_mapping[tclass].perms[i])
> + for (i = 0, result = 0; i < n; i++) {
> + if (avd->decided & mapping->perms[i])
> + result |= 1<<i;
> + if (allow_unknown && !mapping->perms[i])
> result |= 1<<i;
> + }
> avd->decided = result;
>
> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
> - if (avd->auditallow & current_mapping[tclass].perms[i])
> + for (i = 0, result = 0; i < n; i++)
> + if (avd->auditallow & mapping->perms[i])
> result |= 1<<i;
> avd->auditallow = result;
>
> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
> - if (avd->auditdeny & current_mapping[tclass].perms[i])
> + for (i = 0, result = 0; i < n; i++) {
> + if (avd->auditdeny & mapping->perms[i])
> result |= 1<<i;
> + if (!allow_unknown && !mapping->perms[i])
> + result |= 1<<i;
> + }
> +
> + /*
> + * Make sure we audit denials for any permission check
> + * beyond the mapping->num_perms since this indicates
> + * a bug in the object manager.
> + */
> + for (; i < (sizeof(result)*8); i++)
> + result |= 1<<i;
> avd->auditdeny = result;
> }
> }
> diff --git a/libselinux/src/mapping.h b/libselinux/src/mapping.h
> index 22a4ccaf..12ebb89f 100644
> --- a/libselinux/src/mapping.h
> +++ b/libselinux/src/mapping.h
> @@ -7,6 +7,7 @@
> #define _SELINUX_MAPPING_H_
>
> #include <selinux/selinux.h>
> +#include <stdbool.h>
>
> /*
> * Get real, kernel values from mapped values
> @@ -29,6 +30,7 @@ extern access_vector_t
> map_perm(security_class_t tclass, access_vector_t kperm);
>
> extern void
> -map_decision(security_class_t tclass, struct av_decision *avd);
> +map_decision(security_class_t tclass, struct av_decision *avd,
> + bool allow_unknown);
>
> #endif /* _SELINUX_MAPPING_H_ */
> diff --git a/libselinux/src/reject_unknown.c b/libselinux/src/reject_unknown.c
> new file mode 100644
> index 00000000..5c1d3605
> --- /dev/null
> +++ b/libselinux/src/reject_unknown.c
> @@ -0,0 +1,40 @@
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <string.h>
> +#include "selinux_internal.h"
> +#include "policy.h"
> +#include <stdio.h>
> +#include <limits.h>
> +
> +int security_reject_unknown(void)
> +{
> + int fd, ret, reject_unknown = 0;
> + char path[PATH_MAX];
> + char buf[20];
> +
> + if (!selinux_mnt) {
> + errno = ENOENT;
> + return -1;
> + }
> +
> + snprintf(path, sizeof(path), "%s/reject_unknown", selinux_mnt);
> + fd = open(path, O_RDONLY | O_CLOEXEC);
> + if (fd < 0)
> + return -1;
> +
> + memset(buf, 0, sizeof(buf));
> + ret = read(fd, buf, sizeof(buf) - 1);
> + close(fd);
> + if (ret < 0)
> + return -1;
> +
> + if (sscanf(buf, "%d", &reject_unknown) != 1)
> + return -1;
> +
> + return reject_unknown;
> +}
> +
> +hidden_def(security_reject_unknown);
> diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
> index dfc421cc..70b5025d 100644
> --- a/libselinux/src/selinux_internal.h
> +++ b/libselinux/src/selinux_internal.h
> @@ -59,6 +59,7 @@ hidden_proto(selinux_mkload_policy)
> hidden_proto(security_getenforce)
> hidden_proto(security_setenforce)
> hidden_proto(security_deny_unknown)
> + hidden_proto(security_reject_unknown)
> hidden_proto(security_get_checkreqprot)
> hidden_proto(selinux_boolean_sub)
> hidden_proto(selinux_current_policy_path)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libselinux: selinux_set_mapping: fix handling of unknown classes/perms
2019-02-22 16:11 ` Stephen Smalley
@ 2019-02-22 16:14 ` Stephen Smalley
0 siblings, 0 replies; 3+ messages in thread
From: Stephen Smalley @ 2019-02-22 16:14 UTC (permalink / raw)
To: paul; +Cc: dominick.grift, selinux
On 2/22/19 11:11 AM, Stephen Smalley wrote:
> On 2/22/19 9:41 AM, Stephen Smalley wrote:
>> The libselinux selinux_set_mapping() implementation was never updated
>> to handle unknown classes/permissions based on the policy handle_unknown
>> flag. Update it and the internal mapping functions to gracefully
>> handle unknown classes/permissions. Add a security_reject_unknown()
>> interface to expose the corresponding selinuxfs node and use it when
>> creating a mapping to decide whether to fail immediately or proceed.
>>
>> This enables dbus-daemon and XSELinux, which use selinux_set_mapping(),
>> to continue working with the dummy policy or other policies that lack
>> their userspace class/permission definitions as long as the policy
>> was built with -U allow.
>>
>> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
>> ---
>> libselinux/include/selinux/selinux.h | 5 +-
>> libselinux/src/compute_av.c | 16 +++++--
>> libselinux/src/mapping.c | 71 ++++++++++++++++++++++------
>> libselinux/src/mapping.h | 4 +-
>> libselinux/src/reject_unknown.c | 40 ++++++++++++++++
>> libselinux/src/selinux_internal.h | 1 +
>> 6 files changed, 117 insertions(+), 20 deletions(-)
>> create mode 100644 libselinux/src/reject_unknown.c
>>
>> diff --git a/libselinux/include/selinux/selinux.h
>> b/libselinux/include/selinux/selinux.h
>> index 01201eee..a34d54fc 100644
>> --- a/libselinux/include/selinux/selinux.h
>> +++ b/libselinux/include/selinux/selinux.h
>> @@ -328,7 +328,10 @@ extern int security_getenforce(void);
>> /* Set the enforce flag value. */
>> extern int security_setenforce(int value);
>> -/* Get the behavior for undefined classes/permissions */
>> +/* Get the load-time behavior for undefined classes/permissions */
>> +extern int security_reject_unknown(void);
>> +
>> +/* Get the runtime behavior for undefined classes/permissions */
>> extern int security_deny_unknown(void);
>> /* Get the checkreqprot value */
>> diff --git a/libselinux/src/compute_av.c b/libselinux/src/compute_av.c
>> index 1d05e7b6..d5741637 100644
>> --- a/libselinux/src/compute_av.c
>> +++ b/libselinux/src/compute_av.c
>> @@ -20,6 +20,7 @@ int security_compute_av_flags_raw(const char * scon,
>> char *buf;
>> size_t len;
>> int fd, ret;
>> + security_class_t kclass;
>> if (!selinux_mnt) {
>> errno = ENOENT;
>> @@ -38,8 +39,9 @@ int security_compute_av_flags_raw(const char * scon,
>> goto out;
>> }
>> + kclass = unmap_class(tclass);
>> snprintf(buf, len, "%s %s %hu %x", scon, tcon,
>> - unmap_class(tclass), unmap_perm(tclass, requested));
>> + kclass, unmap_perm(tclass, requested));
>> ret = write(fd, buf, strlen(buf));
>> if (ret < 0)
>> @@ -60,9 +62,15 @@ int security_compute_av_flags_raw(const char * scon,
>> } else if (ret < 6)
>> avd->flags = 0;
>> - /* If tclass invalid, kernel sets avd according to deny_unknown
>> flag */
>> - if (tclass != 0)
>> - map_decision(tclass, avd);
>> + /*
>> + * If the tclass could not be mapped to a kernel class at all, the
>> + * kernel will have already set avd according to the
>> + * handle_unknown flag and we do not need to do anything further.
>> + * Otherwise, we must map the permissions within the returned
>> + * avd to the userspace permission values.
>> + */
>> + if (kclass != 0)
>> + map_decision(tclass, avd, !security_deny_unknown());
>
> We probably want to cache security_deny_unknown() and refresh it upon a
> policy reload. If the callers were to use selinux_status_open() during
> setup, they could get the current value via
> selinux_status_deny_unknown() without requiring any further system
> calls. Or if using netlink selinux notifications, they could call
> security_deny_unknown() again only when they receive a
> SELNL_MSG_POLICYLOAD message. Not clear how to best integrate it here
> since security_compute_av*() can be called directly or via the AVC.
Actually, I should probably move the security_deny_unknown() call inside
of map_decision(). Then we can omit it entirely if there is no mapping,
i.e. caller is not using selinux_set_mapping(). That avoids any extra
overhead on selinux_check_access() users.
>
>> ret = 0;
>> out2:
>> diff --git a/libselinux/src/mapping.c b/libselinux/src/mapping.c
>> index f205804b..892f887e 100644
>> --- a/libselinux/src/mapping.c
>> +++ b/libselinux/src/mapping.c
>> @@ -8,7 +8,9 @@
>> #include <stdarg.h>
>> #include <selinux/selinux.h>
>> #include <selinux/avc.h>
>> +#include "callbacks.h"
>> #include "mapping.h"
>> +#include "selinux_internal.h"
>> /*
>> * Class and permission mappings
>> @@ -33,6 +35,9 @@ selinux_set_mapping(struct security_class_mapping *map)
>> size_t size = sizeof(struct selinux_mapping);
>> security_class_t i, j;
>> unsigned k;
>> + bool print_unknown_handle = false;
>> + bool reject = (security_reject_unknown() == 1);
>> + bool deny = (security_deny_unknown() == 1);
>> free(current_mapping);
>> current_mapping = NULL;
>> @@ -62,8 +67,16 @@ selinux_set_mapping(struct security_class_mapping
>> *map)
>> struct selinux_mapping *p_out = current_mapping + j;
>> p_out->value = string_to_security_class(p_in->name);
>> - if (!p_out->value)
>> - goto err2;
>> + if (!p_out->value) {
>> + selinux_log(SELINUX_INFO,
>> + "SELinux: Class %s not defined in policy.\n",
>> + p_in->name);
>> + if (reject)
>> + goto err2;
>> + p_out->num_perms = 0;
>> + print_unknown_handle = true;
>> + continue;
>> + }
>> k = 0;
>> while (p_in->perms[k]) {
>> @@ -74,13 +87,24 @@ selinux_set_mapping(struct security_class_mapping
>> *map)
>> }
>> p_out->perms[k] = string_to_av_perm(p_out->value,
>> p_in->perms[k]);
>> - if (!p_out->perms[k])
>> - goto err2;
>> + if (!p_out->perms[k]) {
>> + selinux_log(SELINUX_INFO,
>> + "SELinux: Permission %s in class %s not
>> defined in policy.\n",
>> + p_in->perms[k], p_in->name);
>> + if (reject)
>> + goto err2;
>> + print_unknown_handle = true;
>> + }
>> k++;
>> }
>> p_out->num_perms = k;
>> }
>> + if (print_unknown_handle)
>> + selinux_log(SELINUX_INFO,
>> + "SELinux: the above unknown classes and permissions
>> will be %s\n",
>> + deny ? "denied" : "allowed");
>> +
>> /* Set the mapping size here so the above lookups are "raw" */
>> current_mapping_size = i;
>> return 0;
>> @@ -181,30 +205,49 @@ map_perm(security_class_t tclass,
>> access_vector_t kperm)
>> }
>> void
>> -map_decision(security_class_t tclass, struct av_decision *avd)
>> +map_decision(security_class_t tclass, struct av_decision *avd,
>> + bool allow_unknown)
>> {
>> if (tclass < current_mapping_size) {
>> - unsigned i;
>> + struct selinux_mapping *mapping = ¤t_mapping[tclass];
>> + unsigned int i, n = mapping->num_perms;
>> access_vector_t result;
>> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
>> - if (avd->allowed & current_mapping[tclass].perms[i])
>> + for (i = 0, result = 0; i < n; i++) {
>> + if (avd->allowed & mapping->perms[i])
>> + result |= 1<<i;
>> + if (allow_unknown && !mapping->perms[i])
>> result |= 1<<i;
>> + }
>> avd->allowed = result;
>> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
>> - if (avd->decided & current_mapping[tclass].perms[i])
>> + for (i = 0, result = 0; i < n; i++) {
>> + if (avd->decided & mapping->perms[i])
>> + result |= 1<<i;
>> + if (allow_unknown && !mapping->perms[i])
>> result |= 1<<i;
>> + }
>> avd->decided = result;
>> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
>> - if (avd->auditallow & current_mapping[tclass].perms[i])
>> + for (i = 0, result = 0; i < n; i++)
>> + if (avd->auditallow & mapping->perms[i])
>> result |= 1<<i;
>> avd->auditallow = result;
>> - for (i=0, result=0; i<current_mapping[tclass].num_perms; i++)
>> - if (avd->auditdeny & current_mapping[tclass].perms[i])
>> + for (i = 0, result = 0; i < n; i++) {
>> + if (avd->auditdeny & mapping->perms[i])
>> result |= 1<<i;
>> + if (!allow_unknown && !mapping->perms[i])
>> + result |= 1<<i;
>> + }
>> +
>> + /*
>> + * Make sure we audit denials for any permission check
>> + * beyond the mapping->num_perms since this indicates
>> + * a bug in the object manager.
>> + */
>> + for (; i < (sizeof(result)*8); i++)
>> + result |= 1<<i;
>> avd->auditdeny = result;
>> }
>> }
>> diff --git a/libselinux/src/mapping.h b/libselinux/src/mapping.h
>> index 22a4ccaf..12ebb89f 100644
>> --- a/libselinux/src/mapping.h
>> +++ b/libselinux/src/mapping.h
>> @@ -7,6 +7,7 @@
>> #define _SELINUX_MAPPING_H_
>> #include <selinux/selinux.h>
>> +#include <stdbool.h>
>> /*
>> * Get real, kernel values from mapped values
>> @@ -29,6 +30,7 @@ extern access_vector_t
>> map_perm(security_class_t tclass, access_vector_t kperm);
>> extern void
>> -map_decision(security_class_t tclass, struct av_decision *avd);
>> +map_decision(security_class_t tclass, struct av_decision *avd,
>> + bool allow_unknown);
>> #endif /* _SELINUX_MAPPING_H_ */
>> diff --git a/libselinux/src/reject_unknown.c
>> b/libselinux/src/reject_unknown.c
>> new file mode 100644
>> index 00000000..5c1d3605
>> --- /dev/null
>> +++ b/libselinux/src/reject_unknown.c
>> @@ -0,0 +1,40 @@
>> +#include <unistd.h>
>> +#include <sys/types.h>
>> +#include <fcntl.h>
>> +#include <stdlib.h>
>> +#include <errno.h>
>> +#include <string.h>
>> +#include "selinux_internal.h"
>> +#include "policy.h"
>> +#include <stdio.h>
>> +#include <limits.h>
>> +
>> +int security_reject_unknown(void)
>> +{
>> + int fd, ret, reject_unknown = 0;
>> + char path[PATH_MAX];
>> + char buf[20];
>> +
>> + if (!selinux_mnt) {
>> + errno = ENOENT;
>> + return -1;
>> + }
>> +
>> + snprintf(path, sizeof(path), "%s/reject_unknown", selinux_mnt);
>> + fd = open(path, O_RDONLY | O_CLOEXEC);
>> + if (fd < 0)
>> + return -1;
>> +
>> + memset(buf, 0, sizeof(buf));
>> + ret = read(fd, buf, sizeof(buf) - 1);
>> + close(fd);
>> + if (ret < 0)
>> + return -1;
>> +
>> + if (sscanf(buf, "%d", &reject_unknown) != 1)
>> + return -1;
>> +
>> + return reject_unknown;
>> +}
>> +
>> +hidden_def(security_reject_unknown);
>> diff --git a/libselinux/src/selinux_internal.h
>> b/libselinux/src/selinux_internal.h
>> index dfc421cc..70b5025d 100644
>> --- a/libselinux/src/selinux_internal.h
>> +++ b/libselinux/src/selinux_internal.h
>> @@ -59,6 +59,7 @@ hidden_proto(selinux_mkload_policy)
>> hidden_proto(security_getenforce)
>> hidden_proto(security_setenforce)
>> hidden_proto(security_deny_unknown)
>> + hidden_proto(security_reject_unknown)
>> hidden_proto(security_get_checkreqprot)
>> hidden_proto(selinux_boolean_sub)
>> hidden_proto(selinux_current_policy_path)
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-22 16:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-22 14:41 [PATCH] libselinux: selinux_set_mapping: fix handling of unknown classes/perms Stephen Smalley
2019-02-22 16:11 ` Stephen Smalley
2019-02-22 16:14 ` Stephen Smalley
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).