An idea of thread/child-domain assignment

From the viewpoint of SELinux, we cannot apply inidividual access controls

Hi,

I'm now considering the way to apply SELinux from the bottom of LAPP stack to the head. But there is a fundamental matter. Web/Application server can execute a request come from users in a worker thread.

This idea enables to assign an indivisual domain on a thread under the paticular condition.

I'm sorry for a long description, but I thought it need to describe the background of this idea for well understanding.

Please comment anything.

Backend of the idea
-------------------
How do you define a user?

I consider a user is a human. In the most cases, a human is not good at communication with electronic circuit of computer system, so he makes a substitution of himself to obtain a result which he want.

A process on operating system is a typical substitution of the user. Thus, it has a user identifier and a security context for proper access controls as a substitution of a user.
I'm considering database instances, web backends and others are also the substitution of users, because they works according to the request come from users.

SELinux makes its decison on access controls based on security context of subject and various kind of objects. Now we have three options to assign a security context on a subject, as follows:

1. Authentication ... Operating System
2. Labeled Networking ... SE-PostgreSQL, X/SELinux, Xinetd
3. Do nothing ... Apache, Samba

However, the third option is uncomfortable for system security. It execute a user's request as a part of the server process, even if it is a substitution of the user.

LAPP stack and SELinux
----------------------
Nowaday, numerous of online services are available and most of them are implemented as a web application using PHP, Jservlet and so on. LAPP is a term to represent its system architecture. It shows web applications based on Linux, Apache, PostgreSQL and PHP is a popular combination.

Quoted 1 line

From the viewpoint of SELinux, we cannot apply inidividual access controls

for every request come from clients, because web/application server handle these requests without individual processes (except for CGI invocation). Apache typically assigns a pre-created process/thread with "httpd_t" domain to execute the request, independent from the peer's context.

Now SELinux can cover the lowest two layer of the LAPP stack (OS, RDBMS). It enables to apply consistent/mandatory access controls for them. I think it should be also expanded to whole of the stack to improve web-application security.
But we have a fundamental matter to achieve it.

[1] LAPP Stack and SELinux coverage

 +----------------------------+   <-----------------+
 |  Application (PHP, Tomcat) |                     |
 +----------------------------+                     |
 |    Web Server (Apache)     |                  Future
 +----------------------------+   <-----------+     |
 |  Database (SE-PostgreSQL)  |               |     |
 +----------------------------+   <-----+   Today   |
 | Operating System (SELinux) |       Past    |     |
 +----------------------------+   <-----+-----+-----+

An idea: thread/hierarchical-domain assignment
----------------------------------------------
If possible, Apache should change the security context of web backend during it executes a request come from users.
However, Apache can execute it on a worker thread. It conflicts a wired constraint of SELinux which does not allow a multi-threaded process dynamic domain transition.

It is a reasonable constraint, if the source and destination domains are completely independent. Because it shares a local memory, mixed security context within a process can make a violation in domain separation.

However, a characteristic relationship between hierarchical domains can help the situation. It restricts a child domain (like "httpd_t.staff") cannot over the boundary of its parent ("httpd_t"). It also means a multi-threaded process cannot over the (parent) domain boundary, even if a part of threads has different security context.

My suggestion is the hard-wired constraint should be modified. It allows to assign threads an indivisual domain, if the new domain is a child one of the current domain.

For example:

    httpd_t -> httpd_t.staff           = allowed
    httpd_t -> staff_t                 = disallowed
    httpd_t.staff -> httpd_t.staff.php = allowed

In other words, this idea requires to change our viewpoint for child domain. Previously, these are mutually independent domain, but a child domain will mean a state with restricted operations compared to its parent domain.

Issues: Domain Reverting
------------------------
There is a related issue. To change security context during user request execution also means that we have to revert its domain after executions, because Apache repeatedly uses worker thread/process for some reasons.

I don't have any good idea for this issue yet.
A one time cookie like AppArmor is an approach, but it is basically weak separation compared from execve() transition.

Quoted 6 lines · KaiGai Kohei

Hi,

I'm now considering the way to apply SELinux from the bottom of LAPP stack to the head. But there is a fundamental matter. Web/Application server can execute a request come from users in a worker thread.

Quoted 2 lines

This idea enables to assign an indivisual domain on a thread under the paticular condition.

Yes, this came up recently, e.g.:
http://marc.info/?l=selinux&m=121320242220824&w=2

And James added it to the kernel todo list on selinuxproject.org.

Quoted 22 lines

An idea: thread/hierarchical-domain assignment
----------------------------------------------
If possible, Apache should change the security context of web backend during it executes a request come from users.
However, Apache can execute it on a worker thread. It conflicts a wired constraint of SELinux which does not allow a multi-threaded process dynamic domain transition.

It is a reasonable constraint, if the source and destination domains are completely independent. Because it shares a local memory, mixed security context within a process can make a violation in domain separation.

However, a characteristic relationship between hierarchical domains can help the situation. It restricts a child domain (like "httpd_t.staff") cannot over the boundary of its parent ("httpd_t"). It also means a multi-threaded process cannot over the (parent) domain boundary, even if a part of threads has different security context.

My suggestion is the hard-wired constraint should be modified. It allows to assign threads an indivisual domain, if the new domain is a child one of the current domain.

Quoted 8 lines

For example:

    httpd_t -> httpd_t.staff           = allowed
    httpd_t -> staff_t                 = disallowed
    httpd_t.staff -> httpd_t.staff.php = allowed

In other words, this idea requires to change our viewpoint for child domain. Previously, these are mutually independent domain, but a child domain will mean a state with restricted operations compared to its parent domain.

I tend to agree that we should support such changes in the kernel mechanism and let the use of it be determined by the policy.

In terms of hierarchy though, the plan was to replace the current name-based hierarchy with an explicitly declared hierarchy.

Quoted 6 lines

Issues: Domain Reverting
------------------------
There is a related issue. To change security context during user request execution also means that we have to revert its domain after executions, because Apache repeatedly uses worker thread/process for some reasons.

Quoted 3 lines

I don't have any good idea for this issue yet.
A one time cookie like AppArmor is an approach, but it is basically weak separation compared from execve() transition.

Perhaps it could pre-populate different pools of threads for different client security contexts, and then just draw from the correct pool for each client. Then the thread context doesn't have to be reverted at all and we can make the transition unidirectional.

Quoted 8 lines

Quoted 3 lines

In other words, this idea requires to change our viewpoint for child domain. Previously, these are mutually independent domain, but a child domain will mean a state with restricted operations compared to its parent domain.

I tend to agree that we should support such changes in the kernel mechanism and let the use of it be determined by the policy.

In terms of hierarchy though, the plan was to replace the current name-based hierarchy with an explicitly declared hierarchy.

It needs a new policy statement like:

  TYPE <child domain> DOMBY <parent domain>;
    or
  TYPEDOMINANCE <parent domain> <child domain> [,<child domain>] ;

I guess these relationships are represented as a ebitmap structure in the kernel, and used to mask permissions during AVC entry creation.

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

In other words, this idea requires to change our viewpoint for child domain. Previously, these are mutually independent domain, but a child domain will mean a state with restricted operations compared to its parent domain.

I tend to agree that we should support such changes in the kernel mechanism and let the use of it be determined by the policy.

In terms of hierarchy though, the plan was to replace the current name-based hierarchy with an explicitly declared hierarchy.

Quoted 8 lines

It needs a new policy statement like:

  TYPE <child domain> DOMBY <parent domain>;
    or
  TYPEDOMINANCE <parent domain> <child domain> [,<child domain>] ;

I guess these relationships are represented as a ebitmap structure in the kernel, and used to mask permissions during AVC entry creation.

Should it be possible to have multiple parent domains on the explicitly declared hierarchy?

The current name-based hierarchy only supports tree-shaped structure. (A child domain can have one parent at most.)
It implicitly enables to avoid a matter of loop of relationship between parents and children.

If we can define a domain with multiple parent, it makes the logic to detect the loop complicated.

Quoted 7 lines · Stephen Smalley

Quoted 6 lines · KaiGai Kohei

Hi,

I'm now considering the way to apply SELinux from the bottom of LAPP stack to the head. But there is a fundamental matter. Web/Application server can execute a request come from users in a worker thread.

Quoted 7 lines

Quoted 2 lines

This idea enables to assign an indivisual domain on a thread under the paticular condition.

Yes, this came up recently, e.g.:
http://marc.info/?l=selinux&m=121320242220824&w=2

And James added it to the kernel todo list on selinuxproject.org.

Thanks, I overlooked this message.

Quoted 31 lines

Quoted 22 lines

An idea: thread/hierarchical-domain assignment
----------------------------------------------
If possible, Apache should change the security context of web backend during it executes a request come from users.
However, Apache can execute it on a worker thread. It conflicts a wired constraint of SELinux which does not allow a multi-threaded process dynamic domain transition.

It is a reasonable constraint, if the source and destination domains are completely independent. Because it shares a local memory, mixed security context within a process can make a violation in domain separation.

However, a characteristic relationship between hierarchical domains can help the situation. It restricts a child domain (like "httpd_t.staff") cannot over the boundary of its parent ("httpd_t"). It also means a multi-threaded process cannot over the (parent) domain boundary, even if a part of threads has different security context.

My suggestion is the hard-wired constraint should be modified. It allows to assign threads an indivisual domain, if the new domain is a child one of the current domain.

Quoted 8 lines

For example:

    httpd_t -> httpd_t.staff           = allowed
    httpd_t -> staff_t                 = disallowed
    httpd_t.staff -> httpd_t.staff.php = allowed

In other words, this idea requires to change our viewpoint for child domain. Previously, these are mutually independent domain, but a child domain will mean a state with restricted operations compared to its parent domain.

Quoted 5 lines

I tend to agree that we should support such changes in the kernel mechanism and let the use of it be determined by the policy.

In terms of hierarchy though, the plan was to replace the current name-based hierarchy with an explicitly declared hierarchy.

It needs a new policy statement like:

  TYPE <child domain> DOMBY <parent domain>;
    or
  TYPEDOMINANCE <parent domain> <child domain> [,<child domain>] ;

I guess these relationships are represented as a ebitmap structure in the kernel, and used to mask permissions during AVC entry creation.

Quoted 6 lines

Quoted 6 lines

Issues: Domain Reverting
------------------------
There is a related issue. To change security context during user request execution also means that we have to revert its domain after executions, because Apache repeatedly uses worker thread/process for some reasons.

Quoted 8 lines

Quoted 3 lines

I don't have any good idea for this issue yet.
A one time cookie like AppArmor is an approach, but it is basically weak separation compared from execve() transition.

Perhaps it could pre-populate different pools of threads for different client security contexts, and then just draw from the correct pool for each client. Then the thread context doesn't have to be reverted at all and we can make the transition unidirectional.

I think this approach requires two precondition.

(1) The number of client security context should be enough small. If we want to assign one of MCS categories, it requires 1024 of thread pool in maximum.

(2) We have to know what client security context should be assigned before the communication between client and backend thread. In the case when we use HTTP authentication to decide client security context, the authentication process has to be done in the backend process after the communication started.
It may also require major works in application updates.

In my impression, AA's approach is not perfect, but second best one...

Quoted 41 lines

Quoted 27 lines

Quoted 27 lines

An idea: thread/hierarchical-domain assignment
----------------------------------------------
If possible, Apache should change the security context of web backend during it executes a request come from users.
However, Apache can execute it on a worker thread. It conflicts a wired constraint of SELinux which does not allow a multi-threaded process dynamic domain transition.

It is a reasonable constraint, if the source and destination domains are completely independent. Because it shares a local memory, mixed security context within a process can make a violation in domain separation.

However, a characteristic relationship between hierarchical domains can help the situation. It restricts a child domain (like "httpd_t.staff") cannot over the boundary of its parent ("httpd_t"). It also means a multi-threaded process cannot over the (parent) domain boundary, even if a part of threads has different security context.

My suggestion is the hard-wired constraint should be modified. It allows to assign threads an indivisual domain, if the new domain is a child one of the current domain.

For example:

    httpd_t -> httpd_t.staff           = allowed
    httpd_t -> staff_t                 = disallowed
    httpd_t.staff -> httpd_t.staff.php = allowed

Quoted 8 lines

Quoted 3 lines

In other words, this idea requires to change our viewpoint for child domain. Previously, these are mutually independent domain, but a child domain will mean a state with restricted operations compared to its parent domain.

I tend to agree that we should support such changes in the kernel mechanism and let the use of it be determined by the policy.

In terms of hierarchy though, the plan was to replace the current name-based hierarchy with an explicitly declared hierarchy.

It needs a new policy statement like:

  TYPE <child domain> DOMBY <parent domain>;
    or
  TYPEDOMINANCE <parent domain> <child domain> [,<child domain>] ;

Quoted 2 lines

I guess these relationships are represented as a ebitmap structure in the kernel, and used to mask permissions during AVC entry creation.

You don't necessarily have to represent the hierarchy in the kernel; it can be done entirely in the policy toolchain like the current type hierarchy.

Quoted 15 lines

Quoted 6 lines

Quoted 6 lines

Issues: Domain Reverting
------------------------
There is a related issue. To change security context during user request execution also means that we have to revert its domain after executions, because Apache repeatedly uses worker thread/process for some reasons.

Quoted 7 lines

Quoted 3 lines

I don't have any good idea for this issue yet.
A one time cookie like AppArmor is an approach, but it is basically weak separation compared from execve() transition.

Perhaps it could pre-populate different pools of threads for different client security contexts, and then just draw from the correct pool for each client. Then the thread context doesn't have to be reverted at all and we can make the transition unidirectional.

I think this approach requires two precondition.

Quoted 3 lines

(1) The number of client security context should be enough small. If we want to assign one of MCS categories, it requires 1024 of thread pool in maximum.

The main server thread could lazily create the thread pools as needed to avoid unnecessary pools. And we could possibly use a hybrid scheme (e.g. one pool per sensitivity or per equivalence class of categories, reuse within that pool).

Quoted 6 lines

(2) We have to know what client security context should be assigned before the communication between client and backend thread. In the case when we use HTTP authentication to decide client security context, the authentication process has to be done in the backend process after the communication started.
It may also require major works in application updates.

True, but not an issue when using labeled networking as the basis for determining the client security context.

Quoted 1 line

In my impression, AA's approach is not perfect, but second best one...

I'd have to hear an argument that there is a real security benefit from it. Do you just mean reverting domain transitions or do you specifically mean the cookie-based approach?

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

An idea: thread/hierarchical-domain assignment

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Issues: Domain Reverting

Quoted 7 lines

Quoted 3 lines

(1) The number of client security context should be enough small. If we want to assign one of MCS categories, it requires 1024 of thread pool in maximum.

The main server thread could lazily create the thread pools as needed to avoid unnecessary pools. And we could possibly use a hybrid scheme (e.g. one pool per sensitivity or per equivalence class of categories, reuse within that pool).

I guess it requires massive reworks for Apache itself. :(

If so, it may be better to implement a SELinux specific multi processing module (MPM) which creates a child process with restricted domain per request?
(No need to say, we will get some performance degradation.)

From its documentation, Tomcat create a thread for a single request and

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

Signed-off-by KaiGai Kohei <[email protected]>

----

 security/selinux/hooks.c            |   11 +++++-
 security/selinux/include/security.h |    5 ++-
 security/selinux/ss/policydb.c      |   75 ++++++++++++++++++++++++++++++++--
 security/selinux/ss/policydb.h      |    2 +
 security/selinux/ss/services.c      |   60 ++++++++++++++++++++++++++++
 5 files changed, 146 insertions(+), 7 deletions(-)

security/selinux/hooks.c +10 -1

index bc1c3ae..d4c1c5c 100644
@@ -5181,7 +5181,12 @@ static int selinux_setprocattr(struct task_struct *p,
 		if (sid == 0)
 			return -EINVAL;
 
-		/* Only allow single threaded processes to change context */
+		/*
+		 * SELinux allows to change context in the following case only.
+		 *  - Single threaded processes.
+		 *  - Multi threaded processes intend to change its context into
+		 *    lower or same domain in hierarchy relationship.
+		 */
 		if (atomic_read(&p->mm->mm_users) != 1) {
 			struct task_struct *g, *t;
 			struct mm_struct *mm = p->mm;

@@ -5189,11 +5194,15 @@ static int selinux_setprocattr(struct task_struct *p,
 			do_each_thread(g, t) {
 				if (t->mm == mm && t != p) {
 					read_unlock(&tasklist_lock);
+
+					if (!security_check_hierarchy(tsec->sid, sid))
+						goto hierarchy_ok;
 					return -EPERM;
 				}
 			} while_each_thread(g, t);
 			read_unlock(&tasklist_lock);
 		}
+hierarchy_ok:
 
 		/* Check permissions for the transition. */
 		error = avc_has_perm(tsec->sid, sid, SECCLASS_PROCESS,

security/selinux/include/security.h +4 -1

index ad30ac4..97fa9bb 100644
@@ -27,13 +27,14 @@
 #define POLICYDB_VERSION_RANGETRANS	21
 #define POLICYDB_VERSION_POLCAP		22
 #define POLICYDB_VERSION_PERMISSIVE	23
+#define POLICYDB_VERSION_HIERARCHY	24
 
 /* Range of policy versions we understand*/
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
 #define POLICYDB_VERSION_MAX	CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
 #else
-#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_PERMISSIVE
+#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_HIERARCHY
 #endif
 
 #define CONTEXT_MNT	0x01

@@ -118,6 +119,8 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
 				 u32 xfrm_sid,
 				 u32 *peer_sid);
 
+int security_check_hierarchy(u32 old_sid, u32 new_sid);
+
 int security_get_classes(char ***classes, int *nclasses);
 int security_get_permissions(char *class, char ***perms, int *nperms);
 int security_get_reject_unknown(void);

security/selinux/ss/policydb.c +70 -5

index 84f8cc7..e388f7a 100644
@@ -116,7 +116,12 @@ static struct policydb_compat_info policydb_compat[] = {
 		.version	= POLICYDB_VERSION_PERMISSIVE,
 		.sym_num	= SYM_NUM,
 		.ocon_num	= OCON_NUM,
-	}
+	},
+	{
+		.version	= POLICYDB_VERSION_HIERARCHY,
+		.sym_num	= SYM_NUM,
+		.ocon_num	= OCON_NUM,
+	},
 };
 
 static struct policydb_compat_info *policydb_lookup_compat(int version)

@@ -270,9 +275,12 @@ static int type_index(void *key, void *datum, void *datap)
 	p = datap;
 
 	if (typdatum->primary) {
-		if (!typdatum->value || typdatum->value > p->p_types.nprim)
+		if (!typdatum->value
+		    || typdatum->value > p->p_types.nprim
+		    || typdatum->parent > p->p_types.nprim)
 			return -EINVAL;
 		p->p_type_val_to_name[typdatum->value - 1] = key;
+		p->type_val_to_struct[typdatum->value - 1] = typdatum;
 	}
 
 	return 0;

@@ -397,6 +405,46 @@ static void symtab_hash_eval(struct symtab *s)
 }
 #endif
 
+static int type_hierarchy_sanity_checks(struct policydb *p)
+{
+	struct type_datum *type;
+	struct ebitmap e;
+	int rc = 0, i;
+
+	if (p->policyvers < POLICYDB_VERSION_HIERARCHY)
+		return 0;
+
+	for (i = 0; i < p->p_types.nprim; i++) {
+		type = p->type_val_to_struct[i];
+
+		if (!type || !type->parent)
+			continue;
+
+		ebitmap_init(&e);
+		while (type) {
+			if (ebitmap_get_bit(&e, type->value - 1)) {
+				printk(KERN_ERR "Hierarchy type looped at %s\n",
+				       p->p_type_val_to_name[type->value - 1]);
+				rc = -EINVAL;
+				break;
+			}
+
+			rc = ebitmap_set_bit(&e, type->value - 1, 1);
+			if (rc)
+				break;
+
+			if (!type->parent)
+				break;
+
+			type = p->type_val_to_struct[type->parent - 1];
+		}
+		ebitmap_destroy(&e);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+
 /*
  * Define the other val_to_name and val_to_struct arrays
  * in a policy database structure.

@@ -438,6 +486,14 @@ static int policydb_index_others(struct policydb *p)
 		goto out;
 	}
 
+	p->type_val_to_struct =
+		kzalloc(p->p_types.nprim * sizeof(*(p->type_val_to_struct)),
+			GFP_KERNEL);
+	if (!p->type_val_to_struct) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	if (cond_init_bool_indexes(p)) {
 		rc = -ENOMEM;
 		goto out;

@@ -455,6 +511,7 @@ static int policydb_index_others(struct policydb *p)
 			goto out;
 	}
 
+	rc = type_hierarchy_sanity_checks(p);
 out:
 	return rc;
 }

@@ -625,6 +682,7 @@ void policydb_destroy(struct policydb *p)
 	kfree(p->class_val_to_struct);
 	kfree(p->role_val_to_struct);
 	kfree(p->user_val_to_struct);
+	kfree(p->type_val_to_struct);
 
 	avtab_destroy(&p->te_avtab);
 

@@ -1236,8 +1294,8 @@ static int type_read(struct policydb *p, struct hashtab *h, void *fp)
 {
 	char *key = NULL;
 	struct type_datum *typdatum;
-	int rc;
-	__le32 buf[3];
+	int to_read, rc;
+	__le32 buf[4];
 	u32 len;
 
 	typdatum = kzalloc(sizeof(*typdatum), GFP_KERNEL);

@@ -1246,13 +1304,20 @@ static int type_read(struct policydb *p, struct hashtab *h, void *fp)
 		return rc;
 	}
 
-	rc = next_entry(buf, fp, sizeof buf);
+	if (p->policyvers < POLICYDB_VERSION_HIERARCHY)
+		to_read = sizeof(u32) * 3;
+	else
+		to_read = sizeof(u32) * 4;
+
+	rc = next_entry(buf, fp, to_read);
 	if (rc < 0)
 		goto bad;
 
 	len = le32_to_cpu(buf[0]);
 	typdatum->value = le32_to_cpu(buf[1]);
 	typdatum->primary = le32_to_cpu(buf[2]);
+	if (p->policyvers >= POLICYDB_VERSION_HIERARCHY)
+		typdatum->parent = le32_to_cpu(buf[3]);
 
 	key = kmalloc(len + 1, GFP_KERNEL);
 	if (!key) {

security/selinux/ss/policydb.h +2 -0

index 4253370..f3484d4 100644
@@ -81,6 +81,7 @@ struct role_allow {
 /* Type attributes */
 struct type_datum {
 	u32 value;		/* internal type value */
+	u32 parent;		/* parent type, if hierarchy related */
 	unsigned char primary;	/* primary name? */
 };
 

@@ -209,6 +210,7 @@ struct policydb {
 	struct class_datum **class_val_to_struct;
 	struct role_datum **role_val_to_struct;
 	struct user_datum **user_val_to_struct;
+	struct type_datum **type_val_to_struct;
 
 	/* type enforcement access vectors and transitions */
 	struct avtab te_avtab;

security/selinux/ss/services.c +53 -0

index dcc2e1c..3ecd793 100644
@@ -2160,6 +2160,66 @@ out_slowpath:
 	return rc;
 }
 
+/*
+ * security_check_hierarchy
+ *
+ * It returns 0, if @old_sid is same or upper type of @new_sid in hierarchy
+ * relationship directly/indirectly. 1 means @old_sid is not a child type
+ * of @new_sid, and negative returns are error.
+ */
+int security_check_hierarchy(u32 old_sid, u32 new_sid)
+{
+	struct context *old_context, *new_context;
+	struct type_datum *type;
+	int index;
+	int rc = -EINVAL;
+
+	read_lock(&policy_rwlock);
+
+	old_context = sidtab_search(&sidtab, old_sid);
+	if (!old_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
+		       __func__, old_sid);
+		goto out;
+	}
+
+	new_context = sidtab_search(&sidtab, new_sid);
+	if (!new_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
+		       __func__, new_sid);
+		goto out;
+	}
+
+	/* domain unchanged */
+	if (old_context->type == new_context->type) {
+		rc = 0;
+		goto out;
+	}
+
+	/* check domain hierarchy */
+	index = new_context->type;
+	while (true) {
+		type = policydb.type_val_to_struct[index - 1];
+		if (!type)
+			break;
+
+		/* No hierarchy relationship */
+		if (type->parent == 0) {
+			rc = 1;
+			break;
+		}
+		if (type->parent == old_context->type) {
+			rc = 0;
+			break;
+		}
+		index = type->parent;

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version
24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

And I think both the neverallow checking and the type hierarchy checking needs to move away from needing to do a full expansion in order to check; it is just too expensive these days.

Quoted 7 lines

Signed-off-by KaiGai Kohei <[email protected]>

----

 security/selinux/hooks.c            |   11 +++++-
 security/selinux/include/security.h |    5 ++-
 security/selinux/ss/policydb.c      |   75

++++++++++++++++++++++++++++++++--

Review

Quoted 6 lines

 security/selinux/ss/policydb.h      |    2 +
 security/selinux/ss/services.c      |   60

++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 7
deletions(-)

(quoted diff) +0 -0

b/security/selinux/hooks.c index

security/selinux/hooks.c static int

Quoted 4 lines

bc1c3ae..d4c1c5c 100644

security/selinux/hooks.c +0 -0

@@ -5181,7 +5181,12 @@ static int

selinux_setprocattr(struct task_struct *p,

security/selinux/hooks.c static int selinux_setprocattr(struct task_struct *p,

Quoted 4 lines

security/selinux/hooks.c +0 -1

 		if (sid == 0)
 			return -EINVAL;
 
-		/* Only allow single threaded processes to change

security/selinux/hooks.c static int · line 5184

Quoted 1 line

security/selinux/hooks.c +1 -0

+		 * SELinux allows to change context in the following

security/selinux/hooks.c static int · line 5186

Quoted 2 lines

security/selinux/hooks.c +2 -0

+		 *  - Single threaded processes.
+		 *  - Multi threaded processes intend to change its

security/selinux/hooks.c static int · line 5187

Quoted 1 line

security/selinux/hooks.c +1 -0

+		 *    lower or same domain in hierarchy relationship. +

security/selinux/hooks.c static int

Quoted 4 lines

security/selinux/hooks.c +0 -0

 		if (atomic_read(&p->mm->mm_users) != 1) {
 			struct task_struct *g, *t;
 			struct mm_struct *mm = p->mm;

@@ -5189,11 +5194,15 @@ static int

selinux_setprocattr(struct task_struct *p,

security/selinux/hooks.c static int selinux_setprocattr(struct task_struct *p,

Quoted 5 lines

security/selinux/hooks.c +2 -0

 			do_each_thread(g, t) {
 				if (t->mm == mm && t != p) {
 					read_unlock(&tasklist_lock);
+
+					if

(!security_check_hierarchy(tsec->sid, sid))

Quoted 8 lines

security/selinux/hooks.c +2 -0

+						goto hierarchy_ok;
 					return -EPERM;
 				}
 			} while_each_thread(g, t);
 			read_unlock(&tasklist_lock);
 		}
+hierarchy_ok:
 

security/selinux/hooks.c static int selinux_setprocattr(struct task_struct *p, · line 5208

Quoted 2 lines

security/selinux/hooks.c +0 -0

 		/* Check permissions for the transition. */
 		error = avc_has_perm(tsec->sid, sid, SECCLASS_PROCESS,

Quoted 11 lines

a/security/selinux/include/security.h
b/security/selinux/include/security.h
index ad30ac4..97fa9bb 100644

security/selinux/include/security.h +1 -0

@@ -27,13 +27,14 @@
 #define POLICYDB_VERSION_RANGETRANS	21
 #define POLICYDB_VERSION_POLCAP		22
 #define POLICYDB_VERSION_PERMISSIVE	23
+#define POLICYDB_VERSION_HIERARCHY	24
 

security/selinux/include/security.h

Quoted 4 lines

security/selinux/include/security.h +0 -0

 /* Range of policy versions we understand*/
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
 #define POLICYDB_VERSION_MAX

CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE

security/selinux/include/security.h line 37

Quoted 3 lines

security/selinux/include/security.h +1 -1

 #else
-#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_PERMISSIVE
+#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_HIERARCHY

security/selinux/include/security.h int security_net_peersid_resolve(u32 nlbl_sid, · line 119

Quoted 4 lines

security/selinux/include/security.h +0 -0

 #define CONTEXT_MNT	0x01

@@ -118,6 +119,8 @@ int security_net_peersid_resolve(u32 nlbl_sid,
 				 u32 nlbl_type,

security/selinux/ss/policydb.c static struct policydb_compat_info · line 116

Quoted 11 lines

security/selinux/include/security.h +1 -0

+int security_check_hierarchy(u32 old_sid, u32 new_sid); +
 int security_get_classes(char ***classes, int *nclasses);  int
 security_get_permissions(char *class, char ***perms, int *nperms);
 int security_get_reject_unknown(void);

(quoted diff) +0 -0

b/security/selinux/ss/policydb.c index 84f8cc7..e388f7a 100644

security/selinux/ss/policydb.c +0 -0

@@ -116,7 +116,12 @@ static struct policydb_compat_info
 		policydb_compat[] = { .version	=

Quoted 11 lines

security/selinux/ss/policydb.c +6 -1

 		.sym_num	= SYM_NUM,
 		.ocon_num	= OCON_NUM,
-	}
+	},
+	{
+		.version	= POLICYDB_VERSION_HIERARCHY,
+		.sym_num	= SYM_NUM,
+		.ocon_num	= OCON_NUM,
+	},
 };
 

Quoted 2 lines

security/selinux/ss/policydb.c +0 -0

 static struct policydb_compat_info *policydb_lookup_compat(int

version) @@ -270,9 +275,12 @@ static int type_index(void

*key, void *datum, void *datap)

security/selinux/ss/policydb.c static int type_index(void *key, void *datum, void *datap)

Quoted 4 lines

security/selinux/ss/policydb.c +0 -1

 	p = datap;
 
 	if (typdatum->primary) {
-		if (!typdatum->value || typdatum->value >

Quoted 1 line

(!typdatum->value + || typdatum->value >

security/selinux/ss/policydb.c static int type_index(void *key, void *datum, void *datap) · line 283

Quoted 4 lines

security/selinux/ss/policydb.c +2 -0

+		    || typdatum->parent > p->p_types.nprim)
 			return -EINVAL;
 		p->p_type_val_to_name[typdatum->value - 1] = key;
+		p->type_val_to_struct[typdatum->value - 1] = typdatum;

Quoted 12 lines

security/selinux/ss/policydb.c +7 -0

 	return 0;

@@ -397,6 +405,46 @@ static void symtab_hash_eval(struct symtab *s) 
 } #endif 
 
+static int type_hierarchy_sanity_checks(struct policydb *p) {
+	struct type_datum *type; +	struct ebitmap e;
+	int rc = 0, i;
+
+	if (p->policyvers < POLICYDB_VERSION_HIERARCHY)
+		return 0;
+

security/selinux/ss/policydb.c static void symtab_hash_eval(struct symtab *s) · line 423

Quoted 10 lines

security/selinux/ss/policydb.c +10 -0

+	for (i = 0; i < p->p_types.nprim; i++) {
+		type = p->type_val_to_struct[i];
+
+		if (!type || !type->parent)
+			continue;
+
+		ebitmap_init(&e);
+		while (type) {
+			if (ebitmap_get_bit(&e, type->value - 1)) {
+				printk(KERN_ERR "Hierarchy type

looped at %s\n",

security/selinux/ss/policydb.c static void symtab_hash_eval(struct symtab *s) · line 424

Quoted 1 line

security/selinux/ss/policydb.c +1 -0

+

p->p_type_val_to_name[type->value - 1]);

security/selinux/ss/policydb.c static void symtab_hash_eval(struct symtab *s) · line 439

Quoted 12 lines

security/selinux/ss/policydb.c +12 -0

+				rc = -EINVAL;
+				break;
+			}
+
+			rc = ebitmap_set_bit(&e, type->value - 1, 1);
+			if (rc)
+				break;
+
+			if (!type->parent)
+				break;
+
+			type = p->type_val_to_struct[type->parent - 1];

security/selinux/ss/policydb.c static int

Quoted 11 lines

security/selinux/ss/policydb.c +7 -0

+		ebitmap_destroy(&e);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+
 /*
  * Define the other val_to_name and val_to_struct arrays
  * in a policy database structure.

@@ -438,6 +486,14 @@ static int

policydb_index_others(struct policydb *p)

security/selinux/ss/policydb.c static int policydb_index_others(struct policydb *p)

Quoted 5 lines

security/selinux/ss/policydb.c +2 -0

 		goto out;
 	}
 
+	p->type_val_to_struct =
+		kzalloc(p->p_types.nprim *

sizeof(*(p->type_val_to_struct)),

Quoted 135 lines

security/selinux/ss/policydb.c +19 -3

+			GFP_KERNEL);
+	if (!p->type_val_to_struct) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	if (cond_init_bool_indexes(p)) {
 		rc = -ENOMEM;
 		goto out;

@@ -455,6 +511,7 @@ static int policydb_index_others(struct policydb
 	*p)  			goto out; }
 
+	rc = type_hierarchy_sanity_checks(p);
 out:
 	return rc;
 }

@@ -625,6 +682,7 @@ void policydb_destroy(struct policydb *p)
 	kfree(p->class_val_to_struct);
 	kfree(p->role_val_to_struct);
 	kfree(p->user_val_to_struct);
+	kfree(p->type_val_to_struct);
 
 	avtab_destroy(&p->te_avtab);
 

@@ -1236,8 +1294,8 @@ static int type_read(struct policydb *p,
 	struct hashtab *h, void *fp)  { char *key = NULL;
 	struct type_datum *typdatum;
-	int rc;
-	__le32 buf[3];
+	int to_read, rc;
+	__le32 buf[4];
 	u32 len;
 
 	typdatum = kzalloc(sizeof(*typdatum), GFP_KERNEL); @@ -1246,13
+1304,20 @@ static int type_read(struct policydb *p, struct hashtab
 	*h, void *fp)  		return rc; }
 
-	rc = next_entry(buf, fp, sizeof buf);
+	if (p->policyvers < POLICYDB_VERSION_HIERARCHY)
+		to_read = sizeof(u32) * 3;
+	else
+		to_read = sizeof(u32) * 4;
+
+	rc = next_entry(buf, fp, to_read);
 	if (rc < 0)
 		goto bad;
 
 	len = le32_to_cpu(buf[0]);
 	typdatum->value = le32_to_cpu(buf[1]);
 	typdatum->primary = le32_to_cpu(buf[2]);
+	if (p->policyvers >= POLICYDB_VERSION_HIERARCHY)
+		typdatum->parent = le32_to_cpu(buf[3]);
 
 	key = kmalloc(len + 1, GFP_KERNEL);
 	if (!key) {

(quoted diff) +0 -0

b/security/selinux/ss/policydb.h index 4253370..f3484d4 100644

security/selinux/ss/policydb.h +2 -0

@@ -81,6 +81,7 @@ struct role_allow {
 /* Type attributes */
 struct type_datum {
 	u32 value;		/* internal type value */
+	u32 parent;		/* parent type, if hierarchy related */
 	unsigned char primary;	/* primary name? */
 };
 

@@ -209,6 +210,7 @@ struct policydb {
 	struct class_datum **class_val_to_struct;
 	struct role_datum **role_val_to_struct;
 	struct user_datum **user_val_to_struct;
+	struct type_datum **type_val_to_struct;
 
 	/* type enforcement access vectors and transitions */  	struct

avtab te_avtab; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index dcc2e1c..3ecd793 100644

security/selinux/ss/services.c +5 -0

@@ -2160,6 +2160,66 @@ out_slowpath:
 	return rc;
 }
 
+/*
+ * security_check_hierarchy
+ *
+ * It returns 0, if @old_sid is same or upper type of @new_sid in
+hierarchy + * relationship directly/indirectly. 1 means @old_sid is

not a child +type + * of @new_sid, and negative returns are error.
+ */

security/selinux/ss/services.c +11 -0

+int security_check_hierarchy(u32 old_sid, u32 new_sid) {
+	struct context *old_context, *new_context;
+	struct type_datum *type;
+	int index;
+	int rc = -EINVAL;
+
+	read_lock(&policy_rwlock);
+
+	old_context = sidtab_search(&sidtab, old_sid);
+	if (!old_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n", +

__func__, old_sid); + goto out;

security/selinux/ss/services.c +5 -0

+	}
+
+	new_context = sidtab_search(&sidtab, new_sid);
+	if (!new_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n", +

__func__, new_sid); + goto out;

security/selinux/ss/services.c +26 -0

+	}
+
+	/* domain unchanged */
+	if (old_context->type == new_context->type) {
+		rc = 0;
+		goto out;
+	}
+
+	/* check domain hierarchy */
+	index = new_context->type;
+	while (true) {
+		type = policydb.type_val_to_struct[index - 1];
+		if (!type)
+			break;
+
+		/* No hierarchy relationship */
+		if (type->parent == 0) {
+			rc = 1;
+			break;
+		}
+		if (type->parent == old_context->type) {
+			rc = 0;
+			break;
+		}
+		index = type->parent;
+	}

security/selinux/ss/services.c out_slowpath: · line 2225

Quoted 8 lines

security/selinux/ss/services.c +6 -0

+out:
+	read_unlock(&policy_rwlock);
+
+	return rc;
+}
+
 static int get_classes_callback(void *k, void *d, void *args)  {
 	struct class_datum *datum = d;

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version
24. This patch also enables to read the new version of policy.

Quoted 6 lines

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

If we are going to do this we also might as well implement the explicit hierarchy support and get away from name based hierarchy.

Quoted 4 lines

And I think both the neverallow checking and the type hierarchy checking needs to move away from needing to do a full expansion in order to check; it is just too expensive these days.

Do you think it will be faster to do attribute based lookups? It will certainly use significantly less memory.

Quoted 20 lines

Quoted 7 lines

Signed-off-by KaiGai Kohei <[email protected]>

----

 security/selinux/hooks.c            |   11 +++++-
 security/selinux/include/security.h |    5 ++-
 security/selinux/ss/policydb.c      |   75

++++++++++++++++++++++++++++++++--

Review

Quoted 6 lines

 security/selinux/ss/policydb.h      |    2 +
 security/selinux/ss/services.c      |   60

++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 7
deletions(-)

(quoted diff) +0 -0

b/security/selinux/hooks.c index

security/selinux/hooks.c static int

Quoted 4 lines

bc1c3ae..d4c1c5c 100644

security/selinux/hooks.c +0 -0

@@ -5181,7 +5181,12 @@ static int

selinux_setprocattr(struct task_struct *p,

Quoted 4 lines

security/selinux/hooks.c static int selinux_setprocattr(struct task_struct *p,

Quoted 4 lines

security/selinux/hooks.c +0 -1

 		if (sid == 0)
 			return -EINVAL;
 
-		/* Only allow single threaded processes to change

Quoted 1 line

security/selinux/hooks.c static int · line 5184

Quoted 1 line

security/selinux/hooks.c +1 -0

+		/* +		 * SELinux allows to change context in

Quoted 2 lines

security/selinux/hooks.c

Quoted 2 lines

only. + * - Single threaded processes.

security/selinux/hooks.c +1 -0

+		 *  - Multi threaded processes intend to change its

Quoted 1 line

security/selinux/hooks.c

Quoted 1 line

security/selinux/hooks.c +1 -0

+		 *    lower or same domain in hierarchy relationship. +

Quoted 19 lines

security/selinux/hooks.c static int

Quoted 4 lines

security/selinux/hooks.c +0 -0

 		if (atomic_read(&p->mm->mm_users) != 1) {
 			struct task_struct *g, *t;
 			struct mm_struct *mm = p->mm;

@@ -5189,11 +5194,15 @@ static int

selinux_setprocattr(struct task_struct *p,

security/selinux/hooks.c static int selinux_setprocattr(struct task_struct *p,

Quoted 5 lines

security/selinux/hooks.c +2 -0

 			do_each_thread(g, t) {
 				if (t->mm == mm && t != p) {
 					read_unlock(&tasklist_lock);
+
+					if

(!security_check_hierarchy(tsec->sid, sid))

Quoted 8 lines

security/selinux/hooks.c +2 -0

+						goto hierarchy_ok;
 					return -EPERM;
 				}
 			} while_each_thread(g, t);
 			read_unlock(&tasklist_lock);
 		}
+hierarchy_ok:
 

Quoted 2 lines

security/selinux/hooks.c static int selinux_setprocattr(struct task_struct *p, · line 5208

Quoted 2 lines

security/selinux/hooks.c +0 -0

 		/* Check permissions for the transition. */
 		error = avc_has_perm(tsec->sid, sid, SECCLASS_PROCESS,

Quoted 11 lines

Quoted 11 lines

a/security/selinux/include/security.h
b/security/selinux/include/security.h
index ad30ac4..97fa9bb 100644

security/selinux/include/security.h +1 -0

@@ -27,13 +27,14 @@
 #define POLICYDB_VERSION_RANGETRANS	21
 #define POLICYDB_VERSION_POLCAP		22
 #define POLICYDB_VERSION_PERMISSIVE	23
+#define POLICYDB_VERSION_HIERARCHY	24
 

Quoted 8 lines

security/selinux/include/security.h

Quoted 4 lines

security/selinux/include/security.h +0 -0

 /* Range of policy versions we understand*/
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
 #define POLICYDB_VERSION_MAX

CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE

security/selinux/include/security.h

Quoted 3 lines

security/selinux/include/security.h +1 -1

 #else
-#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_PERMISSIVE
+#define POLICYDB_VERSION_MAX

POLICYDB_VERSION_HIERARCHY #endif

Quoted 4 lines

security/selinux/include/security.h int security_net_peersid_resolve(u32 nlbl_sid, · line 119

Quoted 4 lines

security/selinux/include/security.h +0 -0

 #define CONTEXT_MNT	0x01

@@ -118,6 +119,8 @@ int security_net_peersid_resolve(u32 nlbl_sid,
 				 u32 nlbl_type,

		 u32 xfrm_sid, u32 *peer_sid);

Quoted 1 line

Quoted 1 line

Quoted 10 lines

security/selinux/ss/policydb.c static struct policydb_compat_info · line 116

Quoted 10 lines

+int security_check_hierarchy(u32 old_sid, u32 new_sid); +
int security_get_classes(char ***classes, int *nclasses); int
security_get_permissions(char *class, char ***perms, int *nperms); int security_get_reject_unknown(void);

(quoted diff) +0 -0

b/security/selinux/ss/policydb.c index 84f8cc7..e388f7a 100644

security/selinux/ss/policydb.c +0 -0

@@ -116,7 +116,12 @@ static struct policydb_compat_info
 		policydb_compat[] = { .version	=

Quoted 10 lines

Quoted 10 lines

security/selinux/ss/policydb.c +6 -1

 		.sym_num	= SYM_NUM, .ocon_num	= OCON_NUM,
-	}
+	},
+	{
+		.version	= POLICYDB_VERSION_HIERARCHY,
+		.sym_num	= SYM_NUM,
+		.ocon_num	= OCON_NUM,
+	},
 };
 

Quoted 7 lines

Quoted 2 lines

security/selinux/ss/policydb.c +0 -0

 static struct policydb_compat_info *policydb_lookup_compat(int

version) @@ -270,9 +275,12 @@ static int type_index(void

*key, void *datum, void *datap)

security/selinux/ss/policydb.c static int type_index(void *key, void *datum, void *datap)

Quoted 4 lines

security/selinux/ss/policydb.c +0 -1

 	p = datap;
 
 	if (typdatum->primary) {
-		if (!typdatum->value || typdatum->value >

Quoted 5 lines

security/selinux/ss/policydb.c static int type_index(void *key, void *datum, void *datap) · line 283

Quoted 5 lines

(!typdatum->value + || typdatum->value p->p_types.nprim

security/selinux/ss/policydb.c +2 -0

+		    || typdatum->parent > p->p_types.nprim)
 			return -EINVAL;
 		p->p_type_val_to_name[typdatum->value - 1] = key;
+		p->type_val_to_struct[typdatum->value - 1] = typdatum;

Quoted 25 lines

Quoted 12 lines

security/selinux/ss/policydb.c +7 -0

 	return 0;

@@ -397,6 +405,46 @@ static void symtab_hash_eval(struct symtab *s)
 } #endif 
 
+static int type_hierarchy_sanity_checks(struct policydb *p) {
+	struct type_datum *type; +	struct ebitmap e;
+	int rc = 0, i;
+
+	if (p->policyvers < POLICYDB_VERSION_HIERARCHY)
+		return 0;
+

security/selinux/ss/policydb.c static void symtab_hash_eval(struct symtab *s) · line 423

Quoted 10 lines

security/selinux/ss/policydb.c +10 -0

+	for (i = 0; i < p->p_types.nprim; i++) {
+		type = p->type_val_to_struct[i];
+
+		if (!type || !type->parent)
+			continue;
+
+		ebitmap_init(&e);
+		while (type) {
+			if (ebitmap_get_bit(&e, type->value - 1)) {
+				printk(KERN_ERR "Hierarchy type

looped at %s\n",

security/selinux/ss/policydb.c static void symtab_hash_eval(struct symtab *s) · line 424

Quoted 1 line

security/selinux/ss/policydb.c +1 -0

+

p->p_type_val_to_name[type->value - 1]);

Quoted 12 lines

security/selinux/ss/policydb.c static void symtab_hash_eval(struct symtab *s)

Quoted 12 lines

security/selinux/ss/policydb.c +12 -0

+				rc = -EINVAL;
+				break;
+			}
+
+			rc = ebitmap_set_bit(&e, type->value - 1, 1);
+			if (rc)
+				break;
+
+			if (!type->parent)
+				break;
+
+			type =

p->type_val_to_struct[type->parent - 1]; + }

Quoted 18 lines

security/selinux/ss/policydb.c static int

Quoted 11 lines

security/selinux/ss/policydb.c +7 -0

+		ebitmap_destroy(&e);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+
 /*
  * Define the other val_to_name and val_to_struct arrays
  * in a policy database structure.

@@ -438,6 +486,14 @@ static int

policydb_index_others(struct policydb *p)

security/selinux/ss/policydb.c static int policydb_index_others(struct policydb *p)

Quoted 5 lines

security/selinux/ss/policydb.c +2 -0

 		goto out;
 	}
 
+	p->type_val_to_struct =
+		kzalloc(p->p_types.nprim *

sizeof(*(p->type_val_to_struct)),

Quoted 10 lines

security/selinux/ss/policydb.c static int

Quoted 10 lines

security/selinux/ss/policydb.c +6 -0

+			GFP_KERNEL);
+	if (!p->type_val_to_struct) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	if (cond_init_bool_indexes(p)) {
 		rc = -ENOMEM;
 		goto out;

@@ -455,6 +511,7 @@ static int

policydb_index_others(struct policydb

Quoted 122 lines

Quoted 122 lines

security/selinux/ss/policydb.c +13 -3

 	*p)  			goto out; }
 
+	rc = type_hierarchy_sanity_checks(p);
 out:
 	return rc;
 }

@@ -625,6 +682,7 @@ void policydb_destroy(struct policydb *p)
 	kfree(p->class_val_to_struct);
 	kfree(p->role_val_to_struct);
 	kfree(p->user_val_to_struct);
+	kfree(p->type_val_to_struct);
 
 	avtab_destroy(&p->te_avtab);
 

@@ -1236,8 +1294,8 @@ static int type_read(struct policydb *p,
 	struct hashtab *h, void *fp)  { char *key = NULL;
 	struct type_datum *typdatum;
-	int rc;
-	__le32 buf[3];
+	int to_read, rc;
+	__le32 buf[4];
 	u32 len;
 
 	typdatum = kzalloc(sizeof(*typdatum), GFP_KERNEL); @@ -1246,13
+1304,20 @@ static int type_read(struct policydb *p, struct hashtab
 	*h, void *fp)  		return rc; }
 
-	rc = next_entry(buf, fp, sizeof buf);
+	if (p->policyvers < POLICYDB_VERSION_HIERARCHY)
+		to_read = sizeof(u32) * 3;
+	else
+		to_read = sizeof(u32) * 4;
+
+	rc = next_entry(buf, fp, to_read);
 	if (rc < 0)
 		goto bad;
 
 	len = le32_to_cpu(buf[0]);
 	typdatum->value = le32_to_cpu(buf[1]);
 	typdatum->primary = le32_to_cpu(buf[2]);
+	if (p->policyvers >= POLICYDB_VERSION_HIERARCHY)
+		typdatum->parent = le32_to_cpu(buf[3]);
 
 	key = kmalloc(len + 1, GFP_KERNEL);
 	if (!key) {

(quoted diff) +0 -0

b/security/selinux/ss/policydb.h index 4253370..f3484d4 100644

security/selinux/ss/policydb.h +2 -0

@@ -81,6 +81,7 @@ struct role_allow {
 /* Type attributes */
 struct type_datum {
 	u32 value;		/* internal type value */
+	u32 parent;		/* parent type, if hierarchy related */
 	unsigned char primary;	/* primary name? */
 };
 

@@ -209,6 +210,7 @@ struct policydb {
 	struct class_datum **class_val_to_struct;
 	struct role_datum **role_val_to_struct;
 	struct user_datum **user_val_to_struct;
+	struct type_datum **type_val_to_struct;
 
 	/* type enforcement access vectors and transitions */  	struct

avtab te_avtab; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index dcc2e1c..3ecd793 100644

security/selinux/ss/services.c +5 -0

@@ -2160,6 +2160,66 @@ out_slowpath:
 	return rc;
 }
 
+/*
+ * security_check_hierarchy
+ *
+ * It returns 0, if @old_sid is same or upper type of @new_sid in
+hierarchy + * relationship directly/indirectly. 1 means @old_sid is

not a child +type + * of @new_sid, and negative returns are error.
+ */ +int security_check_hierarchy(u32 old_sid, u32 new_sid) {

security/selinux/ss/services.c +10 -0

+	struct context *old_context, *new_context;
+	struct type_datum *type;
+	int index;
+	int rc = -EINVAL;
+
+	read_lock(&policy_rwlock);
+
+	old_context = sidtab_search(&sidtab, old_sid);
+	if (!old_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n", +

__func__, old_sid); + goto out;

security/selinux/ss/services.c +5 -0

+	}
+
+	new_context = sidtab_search(&sidtab, new_sid);
+	if (!new_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n", +

__func__, new_sid); + goto out;

security/selinux/ss/services.c +26 -0

+	}
+
+	/* domain unchanged */
+	if (old_context->type == new_context->type) {
+		rc = 0;
+		goto out;
+	}
+
+	/* check domain hierarchy */
+	index = new_context->type;
+	while (true) {
+		type = policydb.type_val_to_struct[index - 1];
+		if (!type)
+			break;
+
+		/* No hierarchy relationship */
+		if (type->parent == 0) {
+			rc = 1;
+			break;
+		}
+		if (type->parent == old_context->type) {
+			rc = 0;
+			break;
+		}
+		index = type->parent;
+	}

Quoted 8 lines

security/selinux/ss/services.c out_slowpath: · line 2225

Quoted 8 lines

security/selinux/ss/services.c +6 -0

+out:
+	read_unlock(&policy_rwlock);
+
+	return rc;
+}
+
 static int get_classes_callback(void *k, void *d, void *args)  {
 	struct class_datum *datum = d;

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

And I think both the neverallow checking and the type hierarchy checking needs to move away from needing to do a full expansion in order to check; it is just too expensive these days.

Quoted 6 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version
24. This patch also enables to read the new version of policy.

Quoted 6 lines

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

If we are going to do this we also might as well implement the explicit hierarchy support and get away from name based hierarchy.

Quoted 4 lines

And I think both the neverallow checking and the type hierarchy checking needs to move away from needing to do a full expansion in order to check; it is just too expensive these days.

Do you think it will be faster to do attribute based lookups? It will certainly use significantly less memory.

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

Quoted 5 lines

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Quoted 4 lines

And I think both the neverallow checking and the type hierarchy checking needs to move away from needing to do a full expansion in order to check; it is just too expensive these days.

I agree. expand-check=1 makes slow downed so much. :( It should be done on-demand.

Thanks,

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

Quoted 18 lines · Stephen Smalley

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 8 lines

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

So you want to deny at runtime based on the hierarchy? Won't that create another confusing way SELinux can deny something without any clue why (aside from running the denial through audit2why)?

Further, this is something we can understand and reject ahead of time, I don't know why it makes sense to wait until later when we can tell now that something is wrong with the policy.

Quoted 23 lines · Stephen Smalley

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Quoted 5 lines

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

So you want to deny at runtime based on the hierarchy? Won't that create another confusing way SELinux can deny something without any clue why (aside from running the denial through audit2why)? Further, this is something we can understand and reject ahead of time, I don't know why it makes sense to wait until later when we can tell now that something is wrong with the policy.

Quoted 28 lines · Joshua Brindle

Quoted 18 lines · Stephen Smalley

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 9 lines

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

Quoted 6 lines

So you want to deny at runtime based on the hierarchy? Won't that create another confusing way SELinux can deny something without any clue why (aside from running the denial through audit2why)? Further, this is something we can understand and reject ahead of time, I don't know why it makes sense to wait until later when we can tell now that something is wrong with the policy.

True. On the other hand, if we are going to make the hierarchy visible to the kernel so that it can directly leverage information from it, then it makes sense to let the kernel directly enforce the relationships, whether that happens at policy load time or at compute_av time.

If we can make the checking happen entirely at policy load time in a manner that isn't too expensive, then that is cleaner, I agree. If not, then lazy dynamic checking by the kernel and optional full static checking by the toolchain may be a compromise.

[2/3] thread-context-checkpolicy.1.patch
This patch add a new statement of TYPEDOMINATE for policy language.

    TYPEDOMINATE <parent type>  <chile type> [, <child type> ...] ;

It defines expilct hierarchical relationship between two types. Existing name based hierarchy is dealt as TYPEDOMINATE is described implicitly.

Quoted 41 lines · Stephen Smalley

Quoted 28 lines · Joshua Brindle

Quoted 18 lines · Stephen Smalley

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 9 lines

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

Quoted 6 lines

So you want to deny at runtime based on the hierarchy? Won't that create another confusing way SELinux can deny something without any clue why (aside from running the denial through audit2why)? Further, this is something we can understand and reject ahead of time, I don't know why it makes sense to wait until later when we can tell now that something is wrong with the policy.

True. On the other hand, if we are going to make the hierarchy visible to the kernel so that it can directly leverage information from it, then it makes sense to let the kernel directly enforce the relationships, whether that happens at policy load time or at compute_av time.

Quoted 4 lines

If we can make the checking happen entirely at policy load time in a manner that isn't too expensive, then that is cleaner, I agree. If not, then lazy dynamic checking by the kernel and optional full static checking by the toolchain may be a compromise.

I guess we can reduce cost to check hierarchy constraint, because all we have to do is to scan/check avtabs of related types and ebitmaps of type_attr_maps.
However, I don't have a good idea to boost neverallow constraint so much due to its flexibility. :(

I have measured a performance to check hierarchy/neverallow constraint with patched libsepol as a trial. Unfortunatelly, about 80% of CPU time is consumed by check_assertions() for neverallows.

[root@saba ~]# time -p semodule -r kaigai
hierarchy_check_constraints: 37.32 [s]
check_assertions: 139.05 [s]
real 205.32
user 198.06
sys 3.75

These checks at policy load time gives us negative effect in system bootup time, unless remarkable improvements. So, I prefer lazy dynamic checking approach.

(*) System bootup time is one of the major topics for embedded Linux folks.

Quoted 8 lines · KaiGai Kohei

[2/3] thread-context-checkpolicy.1.patch
This patch add a new statement of TYPEDOMINATE for policy language.

    TYPEDOMINATE <parent type>  <chile type> [, <child type> ...] ;

It defines expilct hierarchical relationship between two types. Existing name based hierarchy is dealt as TYPEDOMINATE is described implicitly.

I reconsidered that the statement should be replaced as follows, because "DOMINATE" is an associated term with MLS and roles/users also have name based hierarchy ideas now.

  HIERARCHY <parent type> TYPES <child type> [, <child type> ...];
  HIERARCHY <parent role> ROLES <child role> [, <child role> ...];
  HIERARCHY <parent user> USERS <child user> [, <child user> ...];

Quoted 10 lines · KaiGai Kohei

Quoted 8 lines · KaiGai Kohei

[2/3] thread-context-checkpolicy.1.patch
This patch add a new statement of TYPEDOMINATE for policy language.

    TYPEDOMINATE <parent type>  <chile type> [, <child type> ...] ;

It defines expilct hierarchical relationship between two types. Existing name based hierarchy is dealt as TYPEDOMINATE is described implicitly.

Quoted 8 lines

I reconsidered that the statement should be replaced as follows, because "DOMINATE" is an associated term with MLS and roles/users also have name based hierarchy ideas now.

  HIERARCHY <parent type> TYPES <child type> [, <child type> ...];
  HIERARCHY <parent role> ROLES <child role> [, <child role> ...];
  HIERARCHY <parent user> USERS <child user> [, <child user> ...];

I agree that TYPEDOMINATE is not a good choice. In dominance, you're saying "whatever privileges x, y, & z have, give me those too."

I like to think of it as the parent type "circumscribing" or limiting its children -- the parent draws a line in the sand and says "<child>, you can grant yourself whatever permissions you want (hopefully no more than you need), but whatever you do, don't cross this line!" The terms used to should make that obvious. "Circumscribe" is fun to say, but awkward to type, so how about:

    TYPELIMIT <parent type> { <child type> [, <child type> ...] } ;
    ROLELIMIT <parent role> { <child role> [, <child role> ...] } ;

It would read better to say:

    TYPE <parent type> LIMITS { <child type> [, <child type> ...] } ;
    ROLE <parent role> LIMITS { <child role> [, <child role> ...] } ;

but that conflicts with existing syntax. Hence the patterning after attributes and aliasing.

I'm not sure what hierarchy for users implies, or why it's needed. If we have adequate support for hierarchy/dominance in roles, doesn't that suffice for users? After all, users can't do anything their roles don't permit them to. I guess I see users as a slightly different case because they are going to be more per-system than roles or types, so if you don't trust system administrators to set a user's roles correctly, you shouldn't make them a sysadmin. Or am I missing something?

Quoted 66 lines · KaiGai Kohei

Quoted 40 lines · Stephen Smalley

Quoted 28 lines · Joshua Brindle

Quoted 18 lines · Stephen Smalley

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 9 lines

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

Quoted 6 lines

So you want to deny at runtime based on the hierarchy? Won't that create another confusing way SELinux can deny something without any clue why (aside from running the denial through audit2why)? Further, this is something we can understand and reject ahead of time, I don't know why it makes sense to wait until later when we can tell now that something is wrong with the policy.

True. On the other hand, if we are going to make the hierarchy visible to the kernel so that it can directly leverage information from it, then it makes sense to let the kernel directly enforce the relationships, whether that happens at policy load time or at compute_av time.

Quoted 4 lines

If we can make the checking happen entirely at policy load time in a manner that isn't too expensive, then that is cleaner, I agree. If not, then lazy dynamic checking by the kernel and optional full static checking by the toolchain may be a compromise.

I guess we can reduce cost to check hierarchy constraint, because all we have to do is to scan/check avtabs of related types and ebitmaps of type_attr_maps.
However, I don't have a good idea to boost neverallow constraint so much due to its flexibility. :(

I have measured a performance to check hierarchy/neverallow constraint with patched libsepol as a trial. Unfortunatelly, about 80% of CPU time is consumed by check_assertions() for neverallows.

[root@saba ~]# time -p semodule -r kaigai
hierarchy_check_constraints: 37.32 [s]
check_assertions: 139.05 [s]
real 205.32
user 198.06
sys 3.75

These checks at policy load time gives us negative effect in system bootup time, unless remarkable improvements. So, I prefer lazy dynamic checking approach.

Quoted 1 line

(*) System bootup time is one of the major topics for embedded Linux folks.

Ok. I'd be interested in seeing that libsepol patch nonetheless, and we may want to retain full checking there as an option for validation at policy build time (via make validate) while introducing lazy dynamic checking in the kernel.

BTW, we may need to revisit the policydb data structures in order to fully address concerns about policy load time.

Quoted 67 lines · Stephen Smalley

Quoted 66 lines · KaiGai Kohei

Quoted 40 lines · Stephen Smalley

Quoted 28 lines · Joshua Brindle

Quoted 18 lines · Stephen Smalley

Quoted 17 lines · KaiGai Kohei

Quoted 11 lines · Stephen Smalley

Quoted 5 lines · KaiGai Kohei

[1/3] thread-context-kernel.1.patch
This patch enables to assign a thread a "weaker" hierarchical domain, only if the destinated domain is a child of the current domain. Hierachy relationships are defined in the policy version 24. This patch also enables to read the new version of policy.

If you are going to take type hierarchy support into the kernel, then it seems like it should be completely taken into the kernel, i.e. the hierarchy checking should be applied by the kernel rather than by the toolchain. That is what the Flask security server did for its extensible policy mechanism.

When the checks are applied in the kernel?
One candidate is policy loading time, and the other is AVC entry creation time. I prefers the later one, because checks in policy loading time will require the expensive checks in kernel space.

Quoted 9 lines

Quoted 4 lines

In my idea, violated permission bits on AVC entries are masked by ones of its parent types, with/without printing logs.
Now hierarchy/neverallow constraint makes abort whole of policy loading. It can be a significant difference.

Yes, it makes sense to apply the hierarchy restrictions at computation time. Neverallows are a bit different though - we want those to flag bugs in policy. Thus, we likely should leave neverallow checking in the userland toolchain.

Quoted 6 lines

So you want to deny at runtime based on the hierarchy? Won't that create another confusing way SELinux can deny something without any clue why (aside from running the denial through audit2why)? Further, this is something we can understand and reject ahead of time, I don't know why it makes sense to wait until later when we can tell now that something is wrong with the policy.

True. On the other hand, if we are going to make the hierarchy visible to the kernel so that it can directly leverage information from it, then it makes sense to let the kernel directly enforce the relationships, whether that happens at policy load time or at compute_av time.

Quoted 4 lines

If we can make the checking happen entirely at policy load time in a manner that isn't too expensive, then that is cleaner, I agree. If not, then lazy dynamic checking by the kernel and optional full static checking by the toolchain may be a compromise.

I guess we can reduce cost to check hierarchy constraint, because all we have to do is to scan/check avtabs of related types and ebitmaps of type_attr_maps.
However, I don't have a good idea to boost neverallow constraint so much due to its flexibility. :(

I have measured a performance to check hierarchy/neverallow constraint with patched libsepol as a trial. Unfortunatelly, about 80% of CPU time is consumed by check_assertions() for neverallows.

[root@saba ~]# time -p semodule -r kaigai
hierarchy_check_constraints: 37.32 [s]
check_assertions: 139.05 [s]
real 205.32
user 198.06
sys 3.75

These checks at policy load time gives us negative effect in system bootup time, unless remarkable improvements. So, I prefer lazy dynamic checking approach.

Quoted 7 lines

Quoted 1 line

(*) System bootup time is one of the major topics for embedded Linux folks.

Ok. I'd be interested in seeing that libsepol patch nonetheless, and we may want to retain full checking there as an option for validation at policy build time (via make validate) while introducing lazy dynamic checking in the kernel.

This sounds reasonable.

Quoted 2 lines

BTW, we may need to revisit the policydb data structures in order to fully address concerns about policy load time.

Do you have ideas here?

Quoted 15 lines

I have measured a performance to check hierarchy/neverallow constraint with patched libsepol as a trial. Unfortunatelly, about 80% of CPU time is consumed by check_assertions() for neverallows.

[root@saba ~]# time -p semodule -r kaigai
hierarchy_check_constraints: 37.32 [s]
check_assertions: 139.05 [s]
real 205.32
user 198.06
sys 3.75

These checks at policy load time gives us negative effect in system bootup time, unless remarkable improvements. So, I prefer lazy dynamic checking approach.

Quoted 1 line

(*) System bootup time is one of the major topics for embedded Linux folks.

Ok. I'd be interested in seeing that libsepol patch nonetheless, and we may want to retain full checking there as an option for validation at policy build time (via make validate) while introducing lazy dynamic checking in the kernel.

[1/3] thread-context-kernel.2.patch
It allows a multithreaded process to assign an individual "more bounded" security context, and it also enables to handle binary policy format version 24 in kernel space.

Signed-off-by KaiGai Kohei <[email protected]>

--

 security/selinux/avc.c              |    2 +-
 security/selinux/hooks.c            |   12 +-
 security/selinux/include/security.h |    6 +-
 security/selinux/ss/policydb.c      |  244 +++++++++++++++++++++++++--
 security/selinux/ss/policydb.h      |    4 +
 security/selinux/ss/services.c      |  324 +++++++++++++++++++++++++++++++----
 6 files changed, 541 insertions(+), 51 deletions(-)

security/selinux/ss/policydb.c +2 -2

@@ -1176,8 +1198,8 @@ static int role_read(struct policydb *p, struct hashtab *h, void *fp)
 {
 	char *key = NULL;
 	struct role_datum *role;
-	int rc;
-	__le32 buf[2];
+	int rc, to_read = 2;
+	__le32 buf[3];
 	u32 len;
 
 	role = kzalloc(sizeof(*role), GFP_KERNEL);

security/selinux/ss/policydb.c +35 -0

 	if (rc < 0)
 		goto bad;
 
 	len = le32_to_cpu(buf[0]);
 	role->value = le32_to_cpu(buf[1]);
+	if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
+		role->bounds = le32_to_cpu(buf[2]);
 
 	key = kmalloc(len + 1, GFP_KERNEL);
 	if (!key) {

@@ -1465,6 +1502,181 @@ static int (*read_f[SYM_NUM]) (struct policydb *p, struct hashtab *h, void *fp)
 	cat_read,
 };
 
+static int user_bounds_sanity_check(void *key, void *datum, void *datap)
+{
+	struct user_datum *upper, *user;
+	struct policydb *p = datap;
+	int rc, depth = 0;
+
+	upper = user = datum;
+	while (upper->bounds) {
+		struct ebitmap_node *node;
+		unsigned long bit;
+
+		if (++depth == POLICYDB_BOUNDS_MAXDEPTH) {
+			printk(KERN_ERR
+			       "SELinux: user %s: too deep bounds\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+
+		upper = p->user_val_to_struct[upper->bounds - 1];
+		if (!upper) {
+			printk(KERN_ERR
+			       "SELinux: user %s: broken boundary\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+		/* drop bounded bit from user->roles */
+retry:
+		ebitmap_for_each_positive_bit(&user->roles, node, bit) {
+			if (ebitmap_get_bit(&upper->roles, bit))
+				continue;
+
+			audit_log(current->audit_context, GFP_KERNEL,
+				  AUDIT_SELINUX_ERR,

security/selinux/ss/policydb.c +38 -0

+		}
+	}
+	return 0;
+}
+
+static int role_bounds_sanity_check(void *key, void *datum, void *poldbp)
+{
+	struct role_datum *upper, *role;
+	struct policydb *p = poldbp;
+	int rc, depth = 0;
+
+	upper = role = datum;
+	while (upper->bounds) {
+		struct ebitmap_node *node;
+		unsigned long bit;
+
+		if (++depth == POLICYDB_BOUNDS_MAXDEPTH) {
+			printk(KERN_ERR
+			       "SELinux: role %s: too deep bounds\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+
+		upper = p->role_val_to_struct[upper->bounds - 1];
+		if (!upper) {
+			printk(KERN_ERR
+			       "SELinux: role %s: broken boundary\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+		/* drop bounded bit from user->roles */
+retry:
+		ebitmap_for_each_positive_bit(&role->types, node, bit) {
+			if (ebitmap_get_bit(&upper->types, bit))
+				continue;
+
+			audit_log(current->audit_context, GFP_KERNEL,
+				  AUDIT_SELINUX_ERR,

security/selinux/ss/policydb.c +44 -0

+		}
+	}
+	return 0;
+}
+
+static int type_bounds_sanity_check(void *key, void *datum, void *poldbp)
+{
+	struct type_datum *upper, *type;
+	struct policydb *p = poldbp;
+	int rc, depth = 0;
+
+	upper = type = datum;
+	while (upper->bounds) {
+		struct ebitmap *type_attrs, *upper_attrs;
+		struct ebitmap_node *node;
+		unsigned long bit;
+
+		if (++depth == POLICYDB_BOUNDS_MAXDEPTH) {
+			printk(KERN_ERR
+			       "SELinux: type %s: too deep boundary\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+
+		upper = p->type_val_to_struct[upper->bounds - 1];
+		if (!upper) {
+			printk(KERN_ERR
+			       "SELinux: type %s: broken boundary\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+		/* drop bounded bit from type_attr_map */
+retry:
+		type_attrs = &p->type_attr_map[type->value - 1];
+		upper_attrs = &p->type_attr_map[upper->value - 1];
+		ebitmap_for_each_positive_bit(type_attrs, node, bit) {
+			if (type->value == bit + 1)
+				continue;
+
+			if (ebitmap_get_bit(upper_attrs, bit))
+				continue;
+
+			audit_log(current->audit_context, GFP_KERNEL,
+				  AUDIT_SELINUX_ERR,

security/selinux/ss/services.c +0 -0

index dcc2e1c..a584c1d 100644

security/selinux/ss/services.c +5 -0

+		bit = ebitmap_get_bit(names, type->value - 1);
+		if (!bit) {
+			if (type->value != type_value)
+				audit_log(current->audit_context,
+					  GFP_ATOMIC, AUDIT_SELINUX_ERR,

security/selinux/ss/services.c +19 -0

+
+/*
+ * security_boundary_permission - drops violated permissions
+ * on boundary constraint.
+ */
+static void security_bounded_permission(u16 source_type,
+					u16 target_type,
+					u16 tclass,
+					struct av_decision *avd)
+{
+	struct type_datum *source
+		= policydb.type_val_to_struct[source_type - 1];
+	struct type_datum *target
+		= policydb.type_val_to_struct[target_type - 1];
+	struct av_decision lo_avd;
+	u32 masked = 0;
+
+	if (source->bounds) {
+		memset(&lo_avd, 0, sizeof(lo_avd));

security/selinux/ss/services.c +31 -0

+ *
+ * @oldsid : current security identifier
+ * @newsid : destinated security identifier
+ */
+int security_bounded_transition(u32 old_sid, u32 new_sid)
+{
+	struct context *old_context, *new_context;
+	struct type_datum *type;
+	int index;
+	int rc = -EINVAL;
+
+	read_lock(&policy_rwlock);
+
+	old_context = sidtab_search(&sidtab, old_sid);
+	if (!old_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
+		       __func__, old_sid);
+		goto out;
+	}
+
+	new_context = sidtab_search(&sidtab, new_sid);
+	if (!new_context) {
+		printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
+		       __func__, new_sid);
+		goto out;
+	}
+
+	/* type/domain unchaned */
+	if (old_context->type == new_context->type) {
+		rc = 0;
+		goto out;

security/selinux/ss/services.c +21 -0

+
+		/* not bounded anymore */
+		if (!type->bounds) {
+			rc = 1;
+			break;
+		}
+
+		/* @newsid is bounded by @oldsid */
+		if (type->bounds == old_context->type) {
+			rc = 0;
+			break;
+		}
+		index = type->bounds;
+	}
+out:
+	read_unlock(&policy_rwlock);
+
+	return rc;
+}
+
+
 /**
  * security_compute_av - Compute access vector decisions.
  * @ssid: source security identifier

Quoted 31 lines · KaiGai Kohei

[1/3] thread-context-kernel.2.patch
It allows a multithreaded process to assign an individual "more bounded" security context, and it also enables to handle binary policy format version 24 in kernel space.

Signed-off-by KaiGai Kohei <[email protected]>

--

 security/selinux/avc.c              |    2 +-
 security/selinux/hooks.c            |   12 +-
 security/selinux/include/security.h |    6 +-
 security/selinux/ss/policydb.c      |  244 +++++++++++++++++++++++++--
 security/selinux/ss/policydb.h      |    4 +
 security/selinux/ss/services.c      |  324 +++++++++++++++++++++++++++++++----
 6 files changed, 541 insertions(+), 51 deletions(-)

security/selinux/ss/policydb.c +2 -2

@@ -1176,8 +1198,8 @@ static int role_read(struct policydb *p, struct hashtab *h, void *fp)
 {
 	char *key = NULL;
 	struct role_datum *role;
-	int rc;
-	__le32 buf[2];
+	int rc, to_read = 2;
+	__le32 buf[3];
 	u32 len;
 
 	role = kzalloc(sizeof(*role), GFP_KERNEL);

@@ -1186,12 +1208,17 @@ static int role_read(struct policydb *p, struct hashtab *h, void *fp)
 		goto out;
 	}
 

security/selinux/ss/policydb.c static int role_read(struct policydb *p, struct hashtab *h, void *fp) · line 1214

Quoted 4 lines

security/selinux/ss/policydb.c +3 -0

+	if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
+		to_read = 3;
+
 	rc = next_entry(buf, fp, sizeof buf);

Doesn't this also need to be changed to pass sizeof(buf[0]) * to_read rather than sizeof buf?
Did you try loading an old policy on your new kernel?

Quoted 53 lines

security/selinux/ss/policydb.c +40 -0

 	if (rc < 0)
 		goto bad;
 
 	len = le32_to_cpu(buf[0]);
 	role->value = le32_to_cpu(buf[1]);
+	if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
+		role->bounds = le32_to_cpu(buf[2]);
 
 	key = kmalloc(len + 1, GFP_KERNEL);
 	if (!key) {

@@ -1465,6 +1502,181 @@ static int (*read_f[SYM_NUM]) (struct policydb *p, struct hashtab *h, void *fp)
 	cat_read,
 };
 
+static int user_bounds_sanity_check(void *key, void *datum, void *datap)
+{
+	struct user_datum *upper, *user;
+	struct policydb *p = datap;
+	int rc, depth = 0;
+
+	upper = user = datum;
+	while (upper->bounds) {
+		struct ebitmap_node *node;
+		unsigned long bit;
+
+		if (++depth == POLICYDB_BOUNDS_MAXDEPTH) {
+			printk(KERN_ERR
+			       "SELinux: user %s: too deep bounds\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+
+		upper = p->user_val_to_struct[upper->bounds - 1];
+		if (!upper) {
+			printk(KERN_ERR
+			       "SELinux: user %s: broken boundary\n",
+			       (char *) key);
+			return -EINVAL;
+		}
+		/* drop bounded bit from user->roles */
+retry:
+		ebitmap_for_each_positive_bit(&user->roles, node, bit) {
+			if (ebitmap_get_bit(&upper->roles, bit))
+				continue;
+
+			audit_log(current->audit_context, GFP_KERNEL,
+				  AUDIT_SELINUX_ERR,
+				  "boundary violation: "
+				  "user=%s role=%s bounds=%s",
+				  p->p_user_val_to_name[user->value - 1],
+				  p->p_role_val_to_name[bit],
+				  p->p_user_val_to_name[upper->value - 1]);

security/selinux/ss/policydb.c static int (*read_f[SYM_NUM]) (struct policydb *p, struct hashtab *h, void *fp) · line 1547

Quoted 5 lines

security/selinux/ss/policydb.c +5 -0

+
+			rc = ebitmap_set_bit(&user->roles, bit, 0);
+			if (rc)
+				return rc;
+			goto retry;

I would just return an error and abort the policy load on any boundary violations detected at load time rather than mutating the policy.

Quoted 10 lines

security/selinux/ss/policydb.h +1 -0

index 4253370..320fa23 100644
@@ -61,6 +61,7 @@ struct class_datum {

@@ -81,12 +82,14 @@ struct role_allow {
 /* Type attributes */
 struct type_datum {
 	u32 value;		/* internal type value */
+	u32 bounds;		/* boundary of type */

security/selinux/ss/policydb.h struct role_allow { · line 87

Quoted 2 lines

security/selinux/ss/policydb.h +0 -0

 	unsigned char primary;	/* primary name? */
 };

An aside: Possibly we should add the attribute flag at the same time and leave the type attributes in the type symtab in the kernel policy, as previously discussed. Then the kernel policy won't be lacking any information needed for analysis. Just need to make sure that context_to_sid doesn't accept attributes in the type field.

Quoted 4 lines

security/selinux/ss/services.c +0 -0

index dcc2e1c..a584c1d 100644

<snip>

security/selinux/ss/services.c static int context_struct_to_string(struct context *context, char **scontext, · line 168

Quoted 7 lines

security/selinux/ss/services.c +7 -0

+static int cexpr_type_names_eval(struct ebitmap *names, u16 type_value)
+{
+	struct type_datum *type
+		= policydb.type_val_to_struct[type_value - 1];
+	int bit;
+
+	while (1) {

You were using while (true) in the prior functions. We should be consistent one way or the other.

Quoted 12 lines

security/selinux/ss/services.c +12 -0

+		bit = ebitmap_get_bit(names, type->value - 1);
+		if (!bit) {
+			if (type->value != type_value)
+				audit_log(current->audit_context,
+					  GFP_ATOMIC, AUDIT_SELINUX_ERR,
+					  "boundary violation: "
+					  "type=%s bounded=%s",
+					  policydb.p_type_val_to_name[type_value - 1],
+					  policydb.p_type_val_to_name[type->value - 1]);
+			break;
+		}
+		if (!type->bounds)

security/selinux/ss/services.c static int context_struct_to_string(struct context *context, char **scontext, · line 183

Quoted 3 lines

security/selinux/ss/services.c +3 -0

+			break;
+		type = policydb.type_val_to_struct[type->bounds - 1];
+	}

I don't think we ought to be doing this checking here, if at all. Either we vet the constraints for internal consistency at load time, or we view the bounds as purely a limit on the TE rules and not necessarily on the constraints.

security/selinux/ss/services.c out: · line 647

Quoted 5 lines

security/selinux/ss/services.c +1 -0

@@ -547,6 +644,167 @@ out:
 	return rc;
 }
 
+extern void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av);

Move to avc.h, please.

Quoted 26 lines

security/selinux/ss/services.c +26 -0

+
+/*
+ * security_boundary_permission - drops violated permissions
+ * on boundary constraint.
+ */
+static void security_bounded_permission(u16 source_type,
+					u16 target_type,
+					u16 tclass,
+					struct av_decision *avd)
+{
+	struct type_datum *source
+		= policydb.type_val_to_struct[source_type - 1];
+	struct type_datum *target
+		= policydb.type_val_to_struct[target_type - 1];
+	struct av_decision lo_avd;
+	u32 masked = 0;
+
+	if (source->bounds) {
+		memset(&lo_avd, 0, sizeof(lo_avd));
+
+		type_attribute_compute_av(source->bounds,
+					  target_type,
+					  tclass,
+					  AVTAB_ALLOWED,
+					  &lo_avd);
+