[PATCH] libsepol: support policy modules when roletrans rules not supported

[PATCH] libsepol: support policy modules when roletrans rules not supported

[Eric Paris]

Although the role trans code had support to handle the kernel policy
when the version was less that roletrans such support was not in the
module read/write code.  This patch adds proper support for role trans
in modules.

Signed-off-by: Eric Paris <eparis@redhat.com>
---
 libsepol/src/policydb.c |   14 ++++++++++----
 libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 2ecb636..6d8ff91 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
 	return 0;
 }
 
-static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
+static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
+				struct policy_file *fp)
 {
 	uint32_t buf[1], nel;
 	unsigned int i;
@@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
 		if (type_set_read(&tr->types, fp))
 			return -1;
 
-		if (ebitmap_read(&tr->classes, fp))
-			return -1;
+		if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
+			if (ebitmap_read(&tr->classes, fp))
+				return -1;
+		} else {
+			if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
+				return -1;
+		}
 
 		rc = next_entry(buf, fp, sizeof(uint32_t));
 		if (rc < 0)
@@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
 	decl->enabled = le32_to_cpu(buf[1]);
 	if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
 	    avrule_read_list(p, &decl->avrules, fp) == -1 ||
-	    role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
+	    role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
 	    role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
 		return -1;
 	}
diff --git a/libsepol/src/write.c b/libsepol/src/write.c
index c4f5035..78b3aa6 100644
--- a/libsepol/src/write.c
+++ b/libsepol/src/write.c
@@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
 	return POLICYDB_SUCCESS;
 }
 
-static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
+static int only_process(ebitmap_t *in)
+{
+	unsigned int i;
+	ebitmap_node_t *node;
+
+	ebitmap_for_each_bit(in, node, i) {
+		if (ebitmap_node_get_bit(node, i) &&
+		    i != SECCLASS_PROCESS - 1)
+			return 0;
+	}
+	return 1;
+}
+
+static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
+				 struct policy_file *fp)
 {
 	int nel = 0;
 	size_t items;
 	uint32_t buf[1];
 	role_trans_rule_t *tr;
+	int warned = 0;
+	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
 
 	for (tr = t; tr; tr = tr->next)
-		nel++;
+		if (new_role || only_process(&tr->classes))
+			nel++;
+
 	buf[0] = cpu_to_le32(nel);
 	items = put_entry(buf, sizeof(uint32_t), 1, fp);
 	if (items != 1)
 		return POLICYDB_ERROR;
 	for (tr = t; tr; tr = tr->next) {
+		if (!new_role && !only_process(&tr->classes)) {
+			if (!warned)
+				WARN(fp->handle, "Discarding role_transition "
+					"rules for security classes other than "
+					"\"process\"");
+			warned = 1;
+			continue;
+		}
 		if (role_set_write(&tr->roles, fp))
 			return POLICYDB_ERROR;
 		if (type_set_write(&tr->types, fp))
 			return POLICYDB_ERROR;
-		if (ebitmap_write(&tr->classes, fp))
-			return POLICYDB_ERROR;
+		if (new_role)
+			if (ebitmap_write(&tr->classes, fp))
+				return POLICYDB_ERROR;
 		buf[0] = cpu_to_le32(tr->new_role);
 		items = put_entry(buf, sizeof(uint32_t), 1, fp);
 		if (items != 1)
@@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
 	}
 	if (cond_write_list(p, decl->cond_list, fp) == -1 ||
 	    avrule_write_list(decl->avrules, fp) == -1 ||
-	    role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
+	    role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
 	    role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
 		return POLICYDB_ERROR;
 	}
-- 
1.7.1


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Harry Ciao]

Hi Eric,

Just one minor comment, I think the variable name "new_roletr" would be
better than "new_role" in role_trans_list_write(), we are writing the
tr->classes bitmap for the new kind of role_transition rule anyway:

-	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
+	int new_roletr = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;


I am a newbie as for policy module compile/link/expansion process. From
my own commits now I can see the gap that the same logics to support
class in role_transition rule for the kernel policy have not been
properly applied to modules. Since we are permanently adding the classes
ebitmap to the role_trans_rule_t structure, we should always properly
set it up no matter for kernel policy or modules or the version they may
have, write it out only if the target version supports it.

I think your patch has complemented these logic for modules, thanks again!

Best regards,
Harry


Eric Paris 写道:
> Although the role trans code had support to handle the kernel policy
> when the version was less that roletrans such support was not in the
> module read/write code.  This patch adds proper support for role trans
> in modules.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
>  libsepol/src/policydb.c |   14 ++++++++++----
>  libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index 2ecb636..6d8ff91 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
>  	return 0;
>  }
>  
> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
> +				struct policy_file *fp)
>  {
>  	uint32_t buf[1], nel;
>  	unsigned int i;
> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>  		if (type_set_read(&tr->types, fp))
>  			return -1;
>  
> -		if (ebitmap_read(&tr->classes, fp))
> -			return -1;
> +		if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
> +			if (ebitmap_read(&tr->classes, fp))
> +				return -1;
> +		} else {
> +			if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
> +				return -1;
> +		}
>  
>  		rc = next_entry(buf, fp, sizeof(uint32_t));
>  		if (rc < 0)
> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>  	decl->enabled = le32_to_cpu(buf[1]);
>  	if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
>  	    avrule_read_list(p, &decl->avrules, fp) == -1 ||
> -	    role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
> +	    role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
>  	    role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
>  		return -1;
>  	}
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index c4f5035..78b3aa6 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
>  	return POLICYDB_SUCCESS;
>  }
>  
> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
> +static int only_process(ebitmap_t *in)
> +{
> +	unsigned int i;
> +	ebitmap_node_t *node;
> +
> +	ebitmap_for_each_bit(in, node, i) {
> +		if (ebitmap_node_get_bit(node, i) &&
> +		    i != SECCLASS_PROCESS - 1)
> +			return 0;
> +	}
> +	return 1;
> +}
> +
> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
> +				 struct policy_file *fp)
>  {
>  	int nel = 0;
>  	size_t items;
>  	uint32_t buf[1];
>  	role_trans_rule_t *tr;
> +	int warned = 0;
> +	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
>  
>  	for (tr = t; tr; tr = tr->next)
> -		nel++;
> +		if (new_role || only_process(&tr->classes))
> +			nel++;
> +
>  	buf[0] = cpu_to_le32(nel);
>  	items = put_entry(buf, sizeof(uint32_t), 1, fp);
>  	if (items != 1)
>  		return POLICYDB_ERROR;
>  	for (tr = t; tr; tr = tr->next) {
> +		if (!new_role && !only_process(&tr->classes)) {
> +			if (!warned)
> +				WARN(fp->handle, "Discarding role_transition "
> +					"rules for security classes other than "
> +					"\"process\"");
> +			warned = 1;
> +			continue;
> +		}
>  		if (role_set_write(&tr->roles, fp))
>  			return POLICYDB_ERROR;
>  		if (type_set_write(&tr->types, fp))
>  			return POLICYDB_ERROR;
> -		if (ebitmap_write(&tr->classes, fp))
> -			return POLICYDB_ERROR;
> +		if (new_role)
> +			if (ebitmap_write(&tr->classes, fp))
> +				return POLICYDB_ERROR;
>  		buf[0] = cpu_to_le32(tr->new_role);
>  		items = put_entry(buf, sizeof(uint32_t), 1, fp);
>  		if (items != 1)
> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>  	}
>  	if (cond_write_list(p, decl->cond_list, fp) == -1 ||
>  	    avrule_write_list(decl->avrules, fp) == -1 ||
> -	    role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
> +	    role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
>  	    role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
>  		return POLICYDB_ERROR;
>  	}
>   


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Steve Lawrence]

Thanks for the patch. This solves the problem, but I'm still seeing
issues that seems related to support of older version of policy when
using role_transition with non-process classes. I'm now seeing an mls
range overflow error:

libsepol.role_trans_write: Discarding role_transition rules for security
class other than "process"
libsepol.mls_read_range_helper: range overflow
libsepol.context_read_and_validate: error reading MLS range of context
libsepol.policydb_to_image: new policy image is invalid

I'm still looking into it, but I'm not too familiar this part of the code.


On 04/12/2011 05:11 PM, Eric Paris wrote:
> Although the role trans code had support to handle the kernel policy
> when the version was less that roletrans such support was not in the
> module read/write code.  This patch adds proper support for role trans
> in modules.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
>  libsepol/src/policydb.c |   14 ++++++++++----
>  libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index 2ecb636..6d8ff91 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
>  	return 0;
>  }
>  
> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
> +				struct policy_file *fp)
>  {
>  	uint32_t buf[1], nel;
>  	unsigned int i;
> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>  		if (type_set_read(&tr->types, fp))
>  			return -1;
>  
> -		if (ebitmap_read(&tr->classes, fp))
> -			return -1;
> +		if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
> +			if (ebitmap_read(&tr->classes, fp))
> +				return -1;
> +		} else {
> +			if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
> +				return -1;
> +		}
>  
>  		rc = next_entry(buf, fp, sizeof(uint32_t));
>  		if (rc < 0)
> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>  	decl->enabled = le32_to_cpu(buf[1]);
>  	if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
>  	    avrule_read_list(p, &decl->avrules, fp) == -1 ||
> -	    role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
> +	    role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
>  	    role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
>  		return -1;
>  	}
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index c4f5035..78b3aa6 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
>  	return POLICYDB_SUCCESS;
>  }
>  
> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
> +static int only_process(ebitmap_t *in)
> +{
> +	unsigned int i;
> +	ebitmap_node_t *node;
> +
> +	ebitmap_for_each_bit(in, node, i) {
> +		if (ebitmap_node_get_bit(node, i) &&
> +		    i != SECCLASS_PROCESS - 1)
> +			return 0;
> +	}
> +	return 1;
> +}
> +
> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
> +				 struct policy_file *fp)
>  {
>  	int nel = 0;
>  	size_t items;
>  	uint32_t buf[1];
>  	role_trans_rule_t *tr;
> +	int warned = 0;
> +	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
>  
>  	for (tr = t; tr; tr = tr->next)
> -		nel++;
> +		if (new_role || only_process(&tr->classes))
> +			nel++;
> +
>  	buf[0] = cpu_to_le32(nel);
>  	items = put_entry(buf, sizeof(uint32_t), 1, fp);
>  	if (items != 1)
>  		return POLICYDB_ERROR;
>  	for (tr = t; tr; tr = tr->next) {
> +		if (!new_role && !only_process(&tr->classes)) {
> +			if (!warned)
> +				WARN(fp->handle, "Discarding role_transition "
> +					"rules for security classes other than "
> +					"\"process\"");
> +			warned = 1;
> +			continue;
> +		}
>  		if (role_set_write(&tr->roles, fp))
>  			return POLICYDB_ERROR;
>  		if (type_set_write(&tr->types, fp))
>  			return POLICYDB_ERROR;
> -		if (ebitmap_write(&tr->classes, fp))
> -			return POLICYDB_ERROR;
> +		if (new_role)
> +			if (ebitmap_write(&tr->classes, fp))
> +				return POLICYDB_ERROR;
>  		buf[0] = cpu_to_le32(tr->new_role);
>  		items = put_entry(buf, sizeof(uint32_t), 1, fp);
>  		if (items != 1)
> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>  	}
>  	if (cond_write_list(p, decl->cond_list, fp) == -1 ||
>  	    avrule_write_list(decl->avrules, fp) == -1 ||
> -	    role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
> +	    role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
>  	    role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
>  		return POLICYDB_ERROR;
>  	}


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Steve Lawrence]

On 04/13/2011 11:23 AM, Steve Lawrence wrote:
> Thanks for the patch. This solves the problem, but I'm still seeing
> issues that seems related to support of older version of policy when
> using role_transition with non-process classes. I'm now seeing an mls
> range overflow error:
> 
> libsepol.role_trans_write: Discarding role_transition rules for security
> class other than "process"
> libsepol.mls_read_range_helper: range overflow
> libsepol.context_read_and_validate: error reading MLS range of context
> libsepol.policydb_to_image: new policy image is invalid
> 
> I'm still looking into it, but I'm not too familiar this part of the code.
> 
> 
> On 04/12/2011 05:11 PM, Eric Paris wrote:
>> Although the role trans code had support to handle the kernel policy
>> when the version was less that roletrans such support was not in the
>> module read/write code.  This patch adds proper support for role trans
>> in modules.
>>
>> Signed-off-by: Eric Paris <eparis@redhat.com>
>> ---
>>  libsepol/src/policydb.c |   14 ++++++++++----
>>  libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
>>  2 files changed, 42 insertions(+), 9 deletions(-)
>>
>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>> index 2ecb636..6d8ff91 100644
>> --- a/libsepol/src/policydb.c
>> +++ b/libsepol/src/policydb.c
>> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
>>  	return 0;
>>  }
>>  
>> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
>> +				struct policy_file *fp)
>>  {
>>  	uint32_t buf[1], nel;
>>  	unsigned int i;
>> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>>  		if (type_set_read(&tr->types, fp))
>>  			return -1;
>>  
>> -		if (ebitmap_read(&tr->classes, fp))
>> -			return -1;
>> +		if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
>> +			if (ebitmap_read(&tr->classes, fp))
>> +				return -1;
>> +		} else {
>> +			if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
>> +				return -1;
>> +		}
>>  
>>  		rc = next_entry(buf, fp, sizeof(uint32_t));
>>  		if (rc < 0)
>> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>>  	decl->enabled = le32_to_cpu(buf[1]);
>>  	if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
>>  	    avrule_read_list(p, &decl->avrules, fp) == -1 ||
>> -	    role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
>> +	    role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
>>  	    role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
>>  		return -1;
>>  	}
>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>> index c4f5035..78b3aa6 100644
>> --- a/libsepol/src/write.c
>> +++ b/libsepol/src/write.c
>> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
>>  	return POLICYDB_SUCCESS;
>>  }
>>  
>> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
>> +static int only_process(ebitmap_t *in)
>> +{
>> +	unsigned int i;
>> +	ebitmap_node_t *node;
>> +
>> +	ebitmap_for_each_bit(in, node, i) {
>> +		if (ebitmap_node_get_bit(node, i) &&
>> +		    i != SECCLASS_PROCESS - 1)
>> +			return 0;
>> +	}
>> +	return 1;
>> +}
>> +
>> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
>> +				 struct policy_file *fp)
>>  {
>>  	int nel = 0;
>>  	size_t items;
>>  	uint32_t buf[1];
>>  	role_trans_rule_t *tr;
>> +	int warned = 0;
>> +	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
>>  
>>  	for (tr = t; tr; tr = tr->next)
>> -		nel++;
>> +		if (new_role || only_process(&tr->classes))
>> +			nel++;
>> +
>>  	buf[0] = cpu_to_le32(nel);
>>  	items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>  	if (items != 1)
>>  		return POLICYDB_ERROR;
>>  	for (tr = t; tr; tr = tr->next) {
>> +		if (!new_role && !only_process(&tr->classes)) {
>> +			if (!warned)
>> +				WARN(fp->handle, "Discarding role_transition "
>> +					"rules for security classes other than "
>> +					"\"process\"");
>> +			warned = 1;
>> +			continue;
>> +		}
>>  		if (role_set_write(&tr->roles, fp))
>>  			return POLICYDB_ERROR;
>>  		if (type_set_write(&tr->types, fp))
>>  			return POLICYDB_ERROR;
>> -		if (ebitmap_write(&tr->classes, fp))
>> -			return POLICYDB_ERROR;
>> +		if (new_role)
>> +			if (ebitmap_write(&tr->classes, fp))
>> +				return POLICYDB_ERROR;
>>  		buf[0] = cpu_to_le32(tr->new_role);
>>  		items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>  		if (items != 1)
>> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>>  	}
>>  	if (cond_write_list(p, decl->cond_list, fp) == -1 ||
>>  	    avrule_write_list(decl->avrules, fp) == -1 ||
>> -	    role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
>> +	    role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
>>  	    role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
>>  		return POLICYDB_ERROR;
>>  	}
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.

I think the below patch fixes the problem I was seeing. Some rules were
getting discarded in role_trans_write during a policy downgrade, but the
count was not being decreased. This fix is similar to your fix in
role_trans_rule_write.

---
diff --git a/libsepol/src/write.c b/libsepol/src/write.c
index 78b3aa6..3a9c35a 100644
--- a/libsepol/src/write.c
+++ b/libsepol/src/write.c
@@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct
policy_file *fp)

    nel = 0;
    for (tr = r; tr; tr = tr->next)
-       nel++;
+       if(new_roletr || tr->class == SECCLASS_PROCESS)
+           nel++;
+
    buf[0] = cpu_to_le32(nel);
    items = put_entry(buf, sizeof(uint32_t), 1, fp);
    if (items != 1)

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Eric Paris]

On Wed, 2011-04-13 at 13:35 -0400, Steve Lawrence wrote:
> On 04/13/2011 11:23 AM, Steve Lawrence wrote:
> > Thanks for the patch. This solves the problem, but I'm still seeing
> > issues that seems related to support of older version of policy when
> > using role_transition with non-process classes. I'm now seeing an mls
> > range overflow error:
> > 
> > libsepol.role_trans_write: Discarding role_transition rules for security
> > class other than "process"
> > libsepol.mls_read_range_helper: range overflow
> > libsepol.context_read_and_validate: error reading MLS range of context
> > libsepol.policydb_to_image: new policy image is invalid
> > 
> > I'm still looking into it, but I'm not too familiar this part of the code.
> > 
> > 
> > On 04/12/2011 05:11 PM, Eric Paris wrote:
> >> Although the role trans code had support to handle the kernel policy
> >> when the version was less that roletrans such support was not in the
> >> module read/write code.  This patch adds proper support for role trans
> >> in modules.
> >>
> >> Signed-off-by: Eric Paris <eparis@redhat.com>
> >> ---
> >>  libsepol/src/policydb.c |   14 ++++++++++----
> >>  libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
> >>  2 files changed, 42 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> >> index 2ecb636..6d8ff91 100644
> >> --- a/libsepol/src/policydb.c
> >> +++ b/libsepol/src/policydb.c
> >> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
> >>  	return 0;
> >>  }
> >>  
> >> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
> >> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
> >> +				struct policy_file *fp)
> >>  {
> >>  	uint32_t buf[1], nel;
> >>  	unsigned int i;
> >> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
> >>  		if (type_set_read(&tr->types, fp))
> >>  			return -1;
> >>  
> >> -		if (ebitmap_read(&tr->classes, fp))
> >> -			return -1;
> >> +		if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
> >> +			if (ebitmap_read(&tr->classes, fp))
> >> +				return -1;
> >> +		} else {
> >> +			if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
> >> +				return -1;
> >> +		}
> >>  
> >>  		rc = next_entry(buf, fp, sizeof(uint32_t));
> >>  		if (rc < 0)
> >> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
> >>  	decl->enabled = le32_to_cpu(buf[1]);
> >>  	if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
> >>  	    avrule_read_list(p, &decl->avrules, fp) == -1 ||
> >> -	    role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
> >> +	    role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
> >>  	    role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
> >>  		return -1;
> >>  	}
> >> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> >> index c4f5035..78b3aa6 100644
> >> --- a/libsepol/src/write.c
> >> +++ b/libsepol/src/write.c
> >> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
> >>  	return POLICYDB_SUCCESS;
> >>  }
> >>  
> >> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
> >> +static int only_process(ebitmap_t *in)
> >> +{
> >> +	unsigned int i;
> >> +	ebitmap_node_t *node;
> >> +
> >> +	ebitmap_for_each_bit(in, node, i) {
> >> +		if (ebitmap_node_get_bit(node, i) &&
> >> +		    i != SECCLASS_PROCESS - 1)
> >> +			return 0;
> >> +	}
> >> +	return 1;
> >> +}
> >> +
> >> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
> >> +				 struct policy_file *fp)
> >>  {
> >>  	int nel = 0;
> >>  	size_t items;
> >>  	uint32_t buf[1];
> >>  	role_trans_rule_t *tr;
> >> +	int warned = 0;
> >> +	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
> >>  
> >>  	for (tr = t; tr; tr = tr->next)
> >> -		nel++;
> >> +		if (new_role || only_process(&tr->classes))
> >> +			nel++;
> >> +
> >>  	buf[0] = cpu_to_le32(nel);
> >>  	items = put_entry(buf, sizeof(uint32_t), 1, fp);
> >>  	if (items != 1)
> >>  		return POLICYDB_ERROR;
> >>  	for (tr = t; tr; tr = tr->next) {
> >> +		if (!new_role && !only_process(&tr->classes)) {
> >> +			if (!warned)
> >> +				WARN(fp->handle, "Discarding role_transition "
> >> +					"rules for security classes other than "
> >> +					"\"process\"");
> >> +			warned = 1;
> >> +			continue;
> >> +		}
> >>  		if (role_set_write(&tr->roles, fp))
> >>  			return POLICYDB_ERROR;
> >>  		if (type_set_write(&tr->types, fp))
> >>  			return POLICYDB_ERROR;
> >> -		if (ebitmap_write(&tr->classes, fp))
> >> -			return POLICYDB_ERROR;
> >> +		if (new_role)
> >> +			if (ebitmap_write(&tr->classes, fp))
> >> +				return POLICYDB_ERROR;
> >>  		buf[0] = cpu_to_le32(tr->new_role);
> >>  		items = put_entry(buf, sizeof(uint32_t), 1, fp);
> >>  		if (items != 1)
> >> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
> >>  	}
> >>  	if (cond_write_list(p, decl->cond_list, fp) == -1 ||
> >>  	    avrule_write_list(decl->avrules, fp) == -1 ||
> >> -	    role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
> >> +	    role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
> >>  	    role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
> >>  		return POLICYDB_ERROR;
> >>  	}
> > 
> > 
> > --
> > This message was distributed to subscribers of the selinux mailing list.
> > If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> > the words "unsubscribe selinux" without quotes as the message.
> 
> I think the below patch fixes the problem I was seeing. Some rules were
> getting discarded in role_trans_write during a policy downgrade, but the
> count was not being decreased. This fix is similar to your fix in
> role_trans_rule_write.

Looks as appropriate to me as when range trans does it.

-Eric

> ---
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index 78b3aa6..3a9c35a 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct
> policy_file *fp)
> 
>     nel = 0;
>     for (tr = r; tr; tr = tr->next)
> -       nel++;
> +       if(new_roletr || tr->class == SECCLASS_PROCESS)
> +           nel++;
> +
>     buf[0] = cpu_to_le32(nel);
>     items = put_entry(buf, sizeof(uint32_t), 1, fp);
>     if (items != 1)



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Steve Lawrence]

On 04/13/2011 02:07 PM, Eric Paris wrote:
> On Wed, 2011-04-13 at 13:35 -0400, Steve Lawrence wrote:
>> On 04/13/2011 11:23 AM, Steve Lawrence wrote:
>>> Thanks for the patch. This solves the problem, but I'm still seeing
>>> issues that seems related to support of older version of policy when
>>> using role_transition with non-process classes. I'm now seeing an mls
>>> range overflow error:
>>>
>>> libsepol.role_trans_write: Discarding role_transition rules for security
>>> class other than "process"
>>> libsepol.mls_read_range_helper: range overflow
>>> libsepol.context_read_and_validate: error reading MLS range of context
>>> libsepol.policydb_to_image: new policy image is invalid
>>>
>>> I'm still looking into it, but I'm not too familiar this part of the code.
>>>
>>>
>>> On 04/12/2011 05:11 PM, Eric Paris wrote:
>>>> Although the role trans code had support to handle the kernel policy
>>>> when the version was less that roletrans such support was not in the
>>>> module read/write code.  This patch adds proper support for role trans
>>>> in modules.
>>>>
>>>> Signed-off-by: Eric Paris <eparis@redhat.com>
>>>> ---
>>>>  libsepol/src/policydb.c |   14 ++++++++++----
>>>>  libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
>>>>  2 files changed, 42 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>>>> index 2ecb636..6d8ff91 100644
>>>> --- a/libsepol/src/policydb.c
>>>> +++ b/libsepol/src/policydb.c
>>>> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>>>> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
>>>> +				struct policy_file *fp)
>>>>  {
>>>>  	uint32_t buf[1], nel;
>>>>  	unsigned int i;
>>>> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>>>>  		if (type_set_read(&tr->types, fp))
>>>>  			return -1;
>>>>  
>>>> -		if (ebitmap_read(&tr->classes, fp))
>>>> -			return -1;
>>>> +		if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
>>>> +			if (ebitmap_read(&tr->classes, fp))
>>>> +				return -1;
>>>> +		} else {
>>>> +			if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
>>>> +				return -1;
>>>> +		}
>>>>  
>>>>  		rc = next_entry(buf, fp, sizeof(uint32_t));
>>>>  		if (rc < 0)
>>>> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>>>>  	decl->enabled = le32_to_cpu(buf[1]);
>>>>  	if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
>>>>  	    avrule_read_list(p, &decl->avrules, fp) == -1 ||
>>>> -	    role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
>>>> +	    role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
>>>>  	    role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
>>>>  		return -1;
>>>>  	}
>>>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>>>> index c4f5035..78b3aa6 100644
>>>> --- a/libsepol/src/write.c
>>>> +++ b/libsepol/src/write.c
>>>> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
>>>>  	return POLICYDB_SUCCESS;
>>>>  }
>>>>  
>>>> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
>>>> +static int only_process(ebitmap_t *in)
>>>> +{
>>>> +	unsigned int i;
>>>> +	ebitmap_node_t *node;
>>>> +
>>>> +	ebitmap_for_each_bit(in, node, i) {
>>>> +		if (ebitmap_node_get_bit(node, i) &&
>>>> +		    i != SECCLASS_PROCESS - 1)
>>>> +			return 0;
>>>> +	}
>>>> +	return 1;
>>>> +}
>>>> +
>>>> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
>>>> +				 struct policy_file *fp)
>>>>  {
>>>>  	int nel = 0;
>>>>  	size_t items;
>>>>  	uint32_t buf[1];
>>>>  	role_trans_rule_t *tr;
>>>> +	int warned = 0;
>>>> +	int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
>>>>  
>>>>  	for (tr = t; tr; tr = tr->next)
>>>> -		nel++;
>>>> +		if (new_role || only_process(&tr->classes))
>>>> +			nel++;
>>>> +
>>>>  	buf[0] = cpu_to_le32(nel);
>>>>  	items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>>>  	if (items != 1)
>>>>  		return POLICYDB_ERROR;
>>>>  	for (tr = t; tr; tr = tr->next) {
>>>> +		if (!new_role && !only_process(&tr->classes)) {
>>>> +			if (!warned)
>>>> +				WARN(fp->handle, "Discarding role_transition "
>>>> +					"rules for security classes other than "
>>>> +					"\"process\"");
>>>> +			warned = 1;
>>>> +			continue;
>>>> +		}
>>>>  		if (role_set_write(&tr->roles, fp))
>>>>  			return POLICYDB_ERROR;
>>>>  		if (type_set_write(&tr->types, fp))
>>>>  			return POLICYDB_ERROR;
>>>> -		if (ebitmap_write(&tr->classes, fp))
>>>> -			return POLICYDB_ERROR;
>>>> +		if (new_role)
>>>> +			if (ebitmap_write(&tr->classes, fp))
>>>> +				return POLICYDB_ERROR;
>>>>  		buf[0] = cpu_to_le32(tr->new_role);
>>>>  		items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>>>  		if (items != 1)
>>>> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>>>>  	}
>>>>  	if (cond_write_list(p, decl->cond_list, fp) == -1 ||
>>>>  	    avrule_write_list(decl->avrules, fp) == -1 ||
>>>> -	    role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
>>>> +	    role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
>>>>  	    role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
>>>>  		return POLICYDB_ERROR;
>>>>  	}
>>>
>>>
>>> --
>>> This message was distributed to subscribers of the selinux mailing list.
>>> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
>>> the words "unsubscribe selinux" without quotes as the message.
>>
>> I think the below patch fixes the problem I was seeing. Some rules were
>> getting discarded in role_trans_write during a policy downgrade, but the
>> count was not being decreased. This fix is similar to your fix in
>> role_trans_rule_write.
> 
> Looks as appropriate to me as when range trans does it.
> 
> -Eric
> 
>> ---
>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>> index 78b3aa6..3a9c35a 100644
>> --- a/libsepol/src/write.c
>> +++ b/libsepol/src/write.c
>> @@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct
>> policy_file *fp)
>>
>>     nel = 0;
>>     for (tr = r; tr; tr = tr->next)
>> -       nel++;
>> +       if(new_roletr || tr->class == SECCLASS_PROCESS)
>> +           nel++;
>> +
>>     buf[0] = cpu_to_le32(nel);
>>     items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>     if (items != 1)
> 
> 

Both patches applied in libsepol-2.0.44.

Thanks.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Now that we have an updated libsepol lets get the checkpolicy patch to match in.

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Here is the latest Fedora Patch for checkpolicy that we are using to add
filename transitions to F16.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2m7sAACgkQrlYvE4MpobPiYwCgwig1M5fvx7xhewHi7H/7V3aU
GnwAoNjwfqG6l5LbHlxs++xyXO48Hnql
=xwYU
-----END PGP SIGNATURE-----

[-- Attachment #2: checkpolicy-rhat.patch --]
[-- Type: text/plain, Size: 18614 bytes --]

diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
index d6ebd78..0946ff6 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -1313,6 +1313,18 @@ void append_role_allow(role_allow_rule_t * role_allow_rules)
 }
 
 /* this doesn't actually append, but really prepends it */
+void append_filename_trans(filename_trans_rule_t * filename_trans_rules)
+{
+	avrule_decl_t *decl = stack_top->decl;
+
+	/* filename transitions are not allowed within conditionals */
+	assert(stack_top->type == 1);
+
+	filename_trans_rules->next = decl->filename_trans_rules;
+	decl->filename_trans_rules = filename_trans_rules;
+}
+
+/* this doesn't actually append, but really prepends it */
 void append_range_trans(range_trans_rule_t * range_tr_rules)
 {
 	avrule_decl_t *decl = stack_top->decl;
diff --git a/checkpolicy/module_compiler.h b/checkpolicy/module_compiler.h
index fa91400..ae33753 100644
--- a/checkpolicy/module_compiler.h
+++ b/checkpolicy/module_compiler.h
@@ -80,6 +80,7 @@ void append_avrule(avrule_t * avrule);
 void append_role_trans(role_trans_rule_t * role_tr_rules);
 void append_role_allow(role_allow_rule_t * role_allow_rules);
 void append_range_trans(range_trans_rule_t * range_tr_rules);
+void append_filename_trans(filename_trans_rule_t * filename_trans_rules);
 
 /* Create a new optional block and add it to the global policy.
  * During the second pass resolve the block's requirements.  Return 0
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 5e99b30..f75a682 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -2241,6 +2241,190 @@ int define_role_allow(void)
 	return 0;
 }
 
+avrule_t *define_cond_filename_trans(void)
+{
+	yyerror("type transitions with a filename not allowed inside "
+		"conditionals\n");
+	return COND_ERR;
+}
+
+int define_filename_trans(void)
+{
+	char *id, *name = NULL;
+	type_set_t stypes, ttypes;
+	ebitmap_t e_stypes, e_ttypes;
+	ebitmap_t e_tclasses;
+	ebitmap_node_t *snode, *tnode, *cnode;
+	filename_trans_t *ft;
+	filename_trans_rule_t *ftr;
+	class_datum_t *cladatum;
+	type_datum_t *typdatum;
+	uint32_t otype;
+	unsigned int c, s, t;
+	int add;
+
+	if (pass == 1) {
+		/* stype */
+		while ((id = queue_remove(id_queue)))
+			free(id);
+		/* ttype */
+		while ((id = queue_remove(id_queue)))
+			free(id);
+		/* tclass */
+		while ((id = queue_remove(id_queue)))
+			free(id);
+		/* otype */
+		id = queue_remove(id_queue);
+		free(id);
+		/* name */
+		id = queue_remove(id_queue);
+		free(id);
+		return 0;
+	}
+
+
+	add = 1;
+	type_set_init(&stypes);
+	while ((id = queue_remove(id_queue))) {
+		if (set_types(&stypes, id, &add, 0))
+			goto bad;
+	}
+
+	add =1;
+	type_set_init(&ttypes);
+	while ((id = queue_remove(id_queue))) {
+		if (set_types(&ttypes, id, &add, 0))
+			goto bad;
+	}
+
+	ebitmap_init(&e_tclasses);
+	while ((id = queue_remove(id_queue))) {
+		if (!is_id_in_scope(SYM_CLASSES, id)) {
+			yyerror2("class %s is not within scope", id);
+			free(id);
+			goto bad;
+		}
+		cladatum = hashtab_search(policydbp->p_classes.table, id);
+		if (!cladatum) {
+			yyerror2("unknown class %s", id);
+			goto bad;
+		}
+		if (ebitmap_set_bit(&e_tclasses, cladatum->s.value - 1, TRUE)) {
+			yyerror("Out of memory");
+			goto bad;
+		}
+		free(id);
+	}
+
+	id = (char *)queue_remove(id_queue);
+	if (!id) {
+		yyerror("no otype in transition definition?");
+		goto bad;
+	}
+	if (!is_id_in_scope(SYM_TYPES, id)) {
+		yyerror2("type %s is not within scope", id);
+		free(id);
+		goto bad;
+	}
+	typdatum = hashtab_search(policydbp->p_types.table, id);
+	if (!typdatum) {
+		yyerror2("unknown type %s used in transition definition", id);
+		goto bad;
+	}
+	free(id);
+	otype = typdatum->s.value;
+
+	name = queue_remove(id_queue);
+	if (!name) {
+		yyerror("no pathname specified in filename_trans definition?");
+		goto bad;
+	}
+
+	/* We expand the class set into seperate rules.  We expand the types
+	 * just to make sure there are not duplicates.  They will get turned
+	 * into seperate rules later */
+	ebitmap_init(&e_stypes);
+	if (type_set_expand(&stypes, &e_stypes, policydbp, 1))
+		goto bad;
+
+	ebitmap_init(&e_ttypes);
+	if (type_set_expand(&ttypes, &e_ttypes, policydbp, 1))
+		goto bad;
+
+	ebitmap_for_each_bit(&e_tclasses, cnode, c) {
+		if (!ebitmap_node_get_bit(cnode, c))
+			continue;
+		ebitmap_for_each_bit(&e_stypes, snode, s) {
+			if (!ebitmap_node_get_bit(snode, s))
+				continue;
+			ebitmap_for_each_bit(&e_ttypes, tnode, t) {
+				if (!ebitmap_node_get_bit(tnode, t))
+					continue;
+	
+				for (ft = policydbp->filename_trans; ft; ft = ft->next) {
+					if (ft->stype == (s + 1) &&
+					    ft->ttype == (t + 1) &&
+					    ft->tclass == (c + 1) &&
+					    !strcmp(ft->name, name)) {
+						yyerror2("duplicate filename transition for: filename_trans %s %s %s:%s",
+							 name, 
+							 policydbp->p_type_val_to_name[s],
+							 policydbp->p_type_val_to_name[t],
+							 policydbp->p_class_val_to_name[c]);
+						goto bad;
+					}
+				}
+	
+				ft = malloc(sizeof(*ft));
+				if (!ft) {
+					yyerror("out of memory");
+					goto bad;
+				}
+				memset(ft, 0, sizeof(*ft));
+	
+				ft->next = policydbp->filename_trans;
+				policydbp->filename_trans = ft;
+	
+				ft->name = strdup(name);
+				if (!ft->name) {
+					yyerror("out of memory");
+					goto bad;
+				}
+				ft->stype = s + 1;
+				ft->ttype = t + 1;
+				ft->tclass = c + 1;
+				ft->otype = otype;
+			}
+		}
+	
+		/* Now add the real rule since we didn't find any duplicates */
+		ftr = malloc(sizeof(*ftr));
+		if (!ftr) {
+			yyerror("out of memory");
+			goto bad;
+		}
+		filename_trans_rule_init(ftr);
+		append_filename_trans(ftr);
+
+		ftr->name = strdup(name);
+		ftr->stypes = stypes;
+		ftr->ttypes = ttypes;
+		ftr->tclass = c + 1;
+		ftr->otype = otype;
+	}
+
+	free(name);
+	ebitmap_destroy(&e_stypes);
+	ebitmap_destroy(&e_ttypes);
+	ebitmap_destroy(&e_tclasses);
+
+	return 0;
+
+bad:
+	free(name);
+	return -1;
+}
+
 static constraint_expr_t *constraint_expr_clone(constraint_expr_t * expr)
 {
 	constraint_expr_t *h = NULL, *l = NULL, *e, *newe;
diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h
index 2f7a78f..890a6af 100644
--- a/checkpolicy/policy_define.h
+++ b/checkpolicy/policy_define.h
@@ -16,6 +16,7 @@
 avrule_t *define_cond_compute_type(int which);
 avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *stmt);
 avrule_t *define_cond_te_avtab(int which);
+avrule_t *define_cond_filename_trans(void);
 cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void* arg2);
 int define_attrib(void);
 int define_av_perms(int inherits);
@@ -47,6 +48,7 @@ int define_range_trans(int class_specified);
 int define_role_allow(void);
 int define_role_trans(int class_specified);
 int define_role_types(void);
+int define_filename_trans(void);
 int define_sens(void);
 int define_te_avtab(int which);
 int define_typealias(void);
diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
index 8c29e2b..8274d36 100644
--- a/checkpolicy/policy_parse.y
+++ b/checkpolicy/policy_parse.y
@@ -81,6 +81,7 @@ typedef int (* require_func_t)();
 %type <require_func> require_decl_def
 
 %token PATH
+%token FILENAME
 %token CLONE
 %token COMMON
 %token CLASS
@@ -341,7 +342,10 @@ cond_rule_def           : cond_transition_def
 			| require_block
 			{ $$ = NULL; }
                         ;
-cond_transition_def	: TYPE_TRANSITION names names ':' names identifier ';'
+cond_transition_def	: TYPE_TRANSITION names names ':' names identifier filename ';'
+                        { $$ = define_cond_filename_trans() ;
+                          if ($$ == COND_ERR) return -1;}
+			| TYPE_TRANSITION names names ':' names identifier ';'
                         { $$ = define_cond_compute_type(AVRULE_TRANSITION) ;
                           if ($$ == COND_ERR) return -1;}
                         | TYPE_MEMBER names names ':' names identifier ';'
@@ -376,7 +380,9 @@ cond_dontaudit_def	: DONTAUDIT names names ':' names names ';'
 			{ $$ = define_cond_te_avtab(AVRULE_DONTAUDIT);
                           if ($$ == COND_ERR) return -1; }
 		        ;
-transition_def		: TYPE_TRANSITION names names ':' names identifier ';'
+transition_def		: TYPE_TRANSITION names names ':' names identifier filename';'
+                        {if (define_filename_trans()) return -1; }
+                        |TYPE_TRANSITION names names ':' names identifier ';'
                         {if (define_compute_type(AVRULE_TRANSITION)) return -1;}
                         | TYPE_MEMBER names names ':' names identifier ';'
                         {if (define_compute_type(AVRULE_MEMBER)) return -1;}
@@ -639,7 +645,7 @@ opt_fs_uses             : fs_uses
 fs_uses                 : fs_use_def
                         | fs_uses fs_use_def
                         ;
-fs_use_def              : FSUSEXATTR identifier security_context_def ';'
+fs_use_def              : FSUSEXATTR filename security_context_def ';'
                         {if (define_fs_use(SECURITY_FS_USE_XATTR)) return -1;}
                         | FSUSETASK identifier security_context_def ';'
                         {if (define_fs_use(SECURITY_FS_USE_TASK)) return -1;}
@@ -652,11 +658,11 @@ opt_genfs_contexts      : genfs_contexts
 genfs_contexts          : genfs_context_def
                         | genfs_contexts genfs_context_def
                         ;
-genfs_context_def	: GENFSCON identifier path '-' identifier security_context_def
+genfs_context_def	: GENFSCON filename path '-' identifier security_context_def
 			{if (define_genfs_context(1)) return -1;}
-			| GENFSCON identifier path '-' '-' {insert_id("-", 0);} security_context_def
+			| GENFSCON filename path '-' '-' {insert_id("-", 0);} security_context_def
 			{if (define_genfs_context(1)) return -1;}
-                        | GENFSCON identifier path security_context_def
+                        | GENFSCON filename path security_context_def
 			{if (define_genfs_context(0)) return -1;}
 			;
 ipv4_addr_def		: IPV4_ADDR
@@ -733,6 +739,17 @@ identifier		: IDENTIFIER
 path     		: PATH
 			{ if (insert_id(yytext,0)) return -1; }
 			;
+filename     		: FILENAME
+			{ if (insert_id(yytext,0)) return -1; }
+                        | NUMBER
+			{ if (insert_id(yytext,0)) return -1; }
+                        | IPV4_ADDR
+			{ if (insert_id(yytext,0)) return -1; }
+                        | VERSION_IDENTIFIER
+			{ if (insert_id(yytext,0)) return -1; }
+                        | IDENTIFIER
+			{ if (insert_id(yytext,0)) return -1; }
+                        ;
 number			: NUMBER 
 			{ $$ = strtoul(yytext,NULL,0); }
 			;
@@ -757,6 +774,8 @@ module_def              : MODULE identifier version_identifier ';'
                         ;
 version_identifier      : VERSION_IDENTIFIER
                         { if (insert_id(yytext,0)) return -1; }
+			| number
+                        { if (insert_id(yytext,0)) return -1; }
                         | ipv4_addr_def /* version can look like ipv4 address */
                         ;
 avrules_block           : avrule_decls avrule_user_defs
diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
index 48128a8..427c189 100644
--- a/checkpolicy/policy_scan.l
+++ b/checkpolicy/policy_scan.l
@@ -218,9 +218,12 @@ PERMISSIVE			{ return(PERMISSIVE); }
 "/"({alnum}|[_\.\-/])*	        { return(PATH); }
 {letter}({alnum}|[_\-])*([\.]?({alnum}|[_\-]))*	{ return(IDENTIFIER); }
 {digit}+|0x{hexval}+            { return(NUMBER); }
+{alnum}*                        { return(FILENAME); }
 {digit}{1,3}(\.{digit}{1,3}){3}    { return(IPV4_ADDR); }
 {hexval}{0,4}":"{hexval}{0,4}":"({hexval}|[:.])*  { return(IPV6_ADDR); }
 {digit}+(\.({alnum}|[_.])*)?    { return(VERSION_IDENTIFIER); }
+{alnum}+([_\.]|{alnum})+      { return(FILENAME); }
+([_\.]){alnum}+                 { return(FILENAME); }
 #line[ ]1[ ]\"[^\n]*\"		{ set_source_file(yytext+9); }
 #line[ ]{digit}+	        { source_lineno = atoi(yytext+6)-1; }
 #[^\n]*                         { /* delete comments */ }
diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index 33a29e4..66f976f 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -45,6 +45,15 @@
 #define le32_to_cpu(x) bswap_32(x)
 #endif
 
+#define DISPLAY_AVBLOCK_COND_AVTAB	0
+#define DISPLAY_AVBLOCK_UNCOND_AVTAB	1
+#define DISPLAY_AVBLOCK_ROLE_TYPE_NODE	2 /* unused? */
+#define DISPLAY_AVBLOCK_ROLE_TRANS	3
+#define DISPLAY_AVBLOCK_ROLE_ALLOW	4
+#define DISPLAY_AVBLOCK_REQUIRES	5
+#define DISPLAY_AVBLOCK_DECLARES	6
+#define DISPLAY_AVBLOCK_FILENAME_TRANS	7
+
 static policydb_t policydb;
 extern unsigned int ss_initialized;
 
@@ -497,6 +506,18 @@ void display_role_allow(role_allow_rule_t * ra, policydb_t * p, FILE * fp)
 	}
 }
 
+void display_filename_trans(filename_trans_rule_t * tr, policydb_t * p, FILE * fp)
+{
+	for (; tr; tr = tr->next) {
+		fprintf(fp, "filename transition %s", tr->name);
+		display_type_set(&tr->stypes, 0, p, fp);
+		display_type_set(&tr->ttypes, 0, p, fp);
+		display_id(p, fp, SYM_CLASSES, tr->tclass - 1, ":");
+		display_id(p, fp, SYM_TYPES, tr->otype - 1, "");
+		fprintf(fp, "\n");
+	}
+}
+
 int role_display_callback(hashtab_key_t key, hashtab_datum_t datum, void *data)
 {
 	role_datum_t *role;
@@ -596,7 +617,7 @@ int display_avdecl(avrule_decl_t * decl, int field, uint32_t what,
 	fprintf(out_fp, "decl %u:%s\n", decl->decl_id,
 		(decl->enabled ? " [enabled]" : ""));
 	switch (field) {
-	case 0:{
+	case DISPLAY_AVBLOCK_COND_AVTAB:{
 			cond_list_t *cond = decl->cond_list;
 			avrule_t *avrule;
 			while (cond) {
@@ -624,7 +645,7 @@ int display_avdecl(avrule_decl_t * decl, int field, uint32_t what,
 			}
 			break;
 		}
-	case 1:{
+	case DISPLAY_AVBLOCK_UNCOND_AVTAB:{
 			avrule_t *avrule = decl->avrules;
 			if (avrule == NULL) {
 				fprintf(out_fp, "  <empty>\n");
@@ -638,32 +659,37 @@ int display_avdecl(avrule_decl_t * decl, int field, uint32_t what,
 			}
 			break;
 		}
-	case 2:{		/* role_type_node */
+	case DISPLAY_AVBLOCK_ROLE_TYPE_NODE:{	/* role_type_node */
 			break;
 		}
-	case 3:{
+	case DISPLAY_AVBLOCK_ROLE_TRANS:{
 			display_role_trans(decl->role_tr_rules, policy, out_fp);
 			break;
 		}
-	case 4:{
+	case DISPLAY_AVBLOCK_ROLE_ALLOW:{
 			display_role_allow(decl->role_allow_rules, policy,
 					   out_fp);
 			break;
 		}
-	case 5:{
+	case DISPLAY_AVBLOCK_REQUIRES:{
 			if (display_scope_index
 			    (&decl->required, policy, out_fp)) {
 				return -1;
 			}
 			break;
 		}
-	case 6:{
+	case DISPLAY_AVBLOCK_DECLARES:{
 			if (display_scope_index
 			    (&decl->declared, policy, out_fp)) {
 				return -1;
 			}
 			break;
 		}
+	case DISPLAY_AVBLOCK_FILENAME_TRANS:
+		display_filename_trans(decl->filename_trans_rules, policy,
+				       out_fp);
+			return -1;
+		break;
 	default:{
 			assert(0);
 		}
@@ -829,6 +855,7 @@ int menu()
 	printf("c)  Display policy capabilities\n");
 	printf("l)  Link in a module\n");
 	printf("u)  Display the unknown handling setting\n");
+	printf("F)  Display filename_trans rules\n");
 	printf("\n");
 	printf("f)  set output file\n");
 	printf("m)  display menu\n");
@@ -886,15 +913,16 @@ int main(int argc, char **argv)
 		fgets(ans, sizeof(ans), stdin);
 		switch (ans[0]) {
 
-		case '1':{
-				fprintf(out_fp, "unconditional avtab:\n");
-				display_avblock(1, RENDER_UNCONDITIONAL,
-						&policydb, out_fp);
-				break;
-			}
+		case '1':
+			fprintf(out_fp, "unconditional avtab:\n");
+			display_avblock(DISPLAY_AVBLOCK_UNCOND_AVTAB,
+					RENDER_UNCONDITIONAL, &policydb,
+					out_fp);
+			break;
 		case '2':
 			fprintf(out_fp, "conditional avtab:\n");
-			display_avblock(0, RENDER_UNCONDITIONAL, &policydb,
+			display_avblock(DISPLAY_AVBLOCK_COND_AVTAB,
+					RENDER_UNCONDITIONAL, &policydb,
 					out_fp);
 			break;
 		case '3':
@@ -917,11 +945,13 @@ int main(int argc, char **argv)
 			break;
 		case '7':
 			fprintf(out_fp, "role transitions:\n");
-			display_avblock(3, 0, &policydb, out_fp);
+			display_avblock(DISPLAY_AVBLOCK_ROLE_TRANS, 0,
+					&policydb, out_fp);
 			break;
 		case '8':
 			fprintf(out_fp, "role allows:\n");
-			display_avblock(4, 0, &policydb, out_fp);
+			display_avblock(DISPLAY_AVBLOCK_ROLE_ALLOW, 0,
+					&policydb, out_fp);
 			break;
 		case '9':
 			display_policycon(&policydb, out_fp);
@@ -931,11 +961,13 @@ int main(int argc, char **argv)
 			break;
 		case 'a':
 			fprintf(out_fp, "avrule block requirements:\n");
-			display_avblock(5, 0, &policydb, out_fp);
+			display_avblock(DISPLAY_AVBLOCK_REQUIRES, 0,
+					&policydb, out_fp);
 			break;
 		case 'b':
 			fprintf(out_fp, "avrule block declarations:\n");
-			display_avblock(6, 0, &policydb, out_fp);
+			display_avblock(DISPLAY_AVBLOCK_DECLARES, 0,
+					&policydb, out_fp);
 			break;
 		case 'c':
 			display_policycaps(&policydb, out_fp);
@@ -959,6 +991,11 @@ int main(int argc, char **argv)
 			if (out_fp != stdout)
 				printf("\nOutput to file: %s\n", OutfileName);
 			break;
+		case 'F':
+			fprintf(out_fp, "filename_trans rules:\n");
+			display_avblock(DISPLAY_AVBLOCK_FILENAME_TRANS,
+					0, &policydb, out_fp);
+			break;
 		case 'l':
 			link_module(&policydb, out_fp);
 			break;
diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
index f8c05e6..ee2cf02 100644
--- a/checkpolicy/test/dispol.c
+++ b/checkpolicy/test/dispol.c
@@ -341,6 +341,21 @@ static void display_permissive(policydb_t *p, FILE *fp)
 	}
 }
 
+static void display_filename_trans(policydb_t *p, FILE *fp)
+{
+	filename_trans_t *ft;
+
+	fprintf(fp, "filename_trans rules:\n");
+	for (ft = p->filename_trans; ft; ft = ft->next) {
+		fprintf(fp, "%s\n", ft->name);
+		display_id(p, fp, SYM_TYPES, ft->stype - 1, "");
+		display_id(p, fp, SYM_TYPES, ft->ttype - 1, "");
+		display_id(p, fp, SYM_CLASSES, ft->tclass - 1, ":");
+		display_id(p, fp, SYM_TYPES, ft->otype - 1, "");
+		fprintf(fp, "\n");
+	}
+}
+
 int menu()
 {
 	printf("\nSelect a command:\n");
@@ -355,6 +370,8 @@ int menu()
 	printf("c)  display policy capabilities\n");
 	printf("p)  display the list of permissive types\n");
 	printf("u)  display unknown handling setting\n");
+	printf("F)  display filename_trans rules\n");
+	printf("\n");
 	printf("f)  set output file\n");
 	printf("m)  display menu\n");
 	printf("q)  quit\n");
@@ -492,6 +509,9 @@ int main(int argc, char **argv)
 			if (out_fp != stdout)
 				printf("\nOutput to file: %s\n", OutfileName);
 			break;
+		case 'F':
+			display_filename_trans(&policydb, out_fp);
+			break;
 		case 'q':
 			policydb_destroy(&policydb);
 			exit(0);

[-- Attachment #3: checkpolicy-rhat.patch.sig --]
[-- Type: application/pgp-signature, Size: 72 bytes --]

Re: Now that we have an updated libsepol lets get the checkpolicy patch to match in.

[Steve Lawrence]

On 04/14/2011 08:55 AM, Daniel J Walsh wrote:
> Here is the latest Fedora Patch for checkpolicy that we are using to add
> filename transitions to F16.

I don't have anymore time to look at patches this week, and I'll be on
vacation next week. But when I get back, I'll take a look at this patch,
as well as the others in the patch queue.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Now that we have an updated libsepol lets get the checkpolicy patch to match in.

[Steve Lawrence]

On 04/14/2011 08:55 AM, Daniel J Walsh wrote:
> Here is the latest Fedora Patch for checkpolicy that we are using to add
> filename transitions to F16.

This has all been applied to checkpolicy-2.0.25.

Thanks.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Eric Paris]

So I'm questioning the correctness of the range_transition and
role_transition rules in libsepol.  My main problem is that libsepol
defines SECCLASS_* at all.  Right now if the policy reads in one of
these rules types without a tclass it will set SECCLASS_PROCESS in the
tclasses bitmap.  But we never had any that would declare what the
means.  At link time when we have to map "process" in a module to
"process" in the base policy.  But if the module didn't require
"process" it won't have the mapping.  So the fact that we set the
SECCLASS_PROCESS bit could cause it to get mapped to random crap (or
nothing at all)  Right?

I know it's ugly but I think we need to do a couple of things.  #1 on
that list is get SECCLASS_* out of libsepol altogether.  Those are
COMPLETELY a construct of policy.  Not libsepol.  After we remove all
of those we need to change the logic of everything that uses them to
instead make sure that the "process" class exists in it's definitions
and if not declare it.  Then set the bitmap for that new object.

Am I not understanding something about how using SECCLASS_PROCESS
could ever be a good idea?

-Eric

On Wed, Apr 13, 2011 at 2:07 PM, Eric Paris <eparis@redhat.com> wrote:
> On Wed, 2011-04-13 at 13:35 -0400, Steve Lawrence wrote:
>> On 04/13/2011 11:23 AM, Steve Lawrence wrote:
>> > Thanks for the patch. This solves the problem, but I'm still seeing
>> > issues that seems related to support of older version of policy when
>> > using role_transition with non-process classes. I'm now seeing an mls
>> > range overflow error:
>> >
>> > libsepol.role_trans_write: Discarding role_transition rules for security
>> > class other than "process"
>> > libsepol.mls_read_range_helper: range overflow
>> > libsepol.context_read_and_validate: error reading MLS range of context
>> > libsepol.policydb_to_image: new policy image is invalid
>> >
>> > I'm still looking into it, but I'm not too familiar this part of the code.
>> >
>> >
>> > On 04/12/2011 05:11 PM, Eric Paris wrote:
>> >> Although the role trans code had support to handle the kernel policy
>> >> when the version was less that roletrans such support was not in the
>> >> module read/write code.  This patch adds proper support for role trans
>> >> in modules.
>> >>
>> >> Signed-off-by: Eric Paris <eparis@redhat.com>
>> >> ---
>> >>  libsepol/src/policydb.c |   14 ++++++++++----
>> >>  libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
>> >>  2 files changed, 42 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>> >> index 2ecb636..6d8ff91 100644
>> >> --- a/libsepol/src/policydb.c
>> >> +++ b/libsepol/src/policydb.c
>> >> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
>> >>    return 0;
>> >>  }
>> >>
>> >> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>> >> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
>> >> +                          struct policy_file *fp)
>> >>  {
>> >>    uint32_t buf[1], nel;
>> >>    unsigned int i;
>> >> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>> >>            if (type_set_read(&tr->types, fp))
>> >>                    return -1;
>> >>
>> >> -          if (ebitmap_read(&tr->classes, fp))
>> >> -                  return -1;
>> >> +          if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) {
>> >> +                  if (ebitmap_read(&tr->classes, fp))
>> >> +                          return -1;
>> >> +          } else {
>> >> +                  if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
>> >> +                          return -1;
>> >> +          }
>> >>
>> >>            rc = next_entry(buf, fp, sizeof(uint32_t));
>> >>            if (rc < 0)
>> >> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>> >>    decl->enabled = le32_to_cpu(buf[1]);
>> >>    if (cond_read_list(p, &decl->cond_list, fp) == -1 ||
>> >>        avrule_read_list(p, &decl->avrules, fp) == -1 ||
>> >> -      role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
>> >> +      role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 ||
>> >>        role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
>> >>            return -1;
>> >>    }
>> >> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>> >> index c4f5035..78b3aa6 100644
>> >> --- a/libsepol/src/write.c
>> >> +++ b/libsepol/src/write.c
>> >> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
>> >>    return POLICYDB_SUCCESS;
>> >>  }
>> >>
>> >> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
>> >> +static int only_process(ebitmap_t *in)
>> >> +{
>> >> +  unsigned int i;
>> >> +  ebitmap_node_t *node;
>> >> +
>> >> +  ebitmap_for_each_bit(in, node, i) {
>> >> +          if (ebitmap_node_get_bit(node, i) &&
>> >> +              i != SECCLASS_PROCESS - 1)
>> >> +                  return 0;
>> >> +  }
>> >> +  return 1;
>> >> +}
>> >> +
>> >> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
>> >> +                           struct policy_file *fp)
>> >>  {
>> >>    int nel = 0;
>> >>    size_t items;
>> >>    uint32_t buf[1];
>> >>    role_trans_rule_t *tr;
>> >> +  int warned = 0;
>> >> +  int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS;
>> >>
>> >>    for (tr = t; tr; tr = tr->next)
>> >> -          nel++;
>> >> +          if (new_role || only_process(&tr->classes))
>> >> +                  nel++;
>> >> +
>> >>    buf[0] = cpu_to_le32(nel);
>> >>    items = put_entry(buf, sizeof(uint32_t), 1, fp);
>> >>    if (items != 1)
>> >>            return POLICYDB_ERROR;
>> >>    for (tr = t; tr; tr = tr->next) {
>> >> +          if (!new_role && !only_process(&tr->classes)) {
>> >> +                  if (!warned)
>> >> +                          WARN(fp->handle, "Discarding role_transition "
>> >> +                                  "rules for security classes other than "
>> >> +                                  "\"process\"");
>> >> +                  warned = 1;
>> >> +                  continue;
>> >> +          }
>> >>            if (role_set_write(&tr->roles, fp))
>> >>                    return POLICYDB_ERROR;
>> >>            if (type_set_write(&tr->types, fp))
>> >>                    return POLICYDB_ERROR;
>> >> -          if (ebitmap_write(&tr->classes, fp))
>> >> -                  return POLICYDB_ERROR;
>> >> +          if (new_role)
>> >> +                  if (ebitmap_write(&tr->classes, fp))
>> >> +                          return POLICYDB_ERROR;
>> >>            buf[0] = cpu_to_le32(tr->new_role);
>> >>            items = put_entry(buf, sizeof(uint32_t), 1, fp);
>> >>            if (items != 1)
>> >> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>> >>    }
>> >>    if (cond_write_list(p, decl->cond_list, fp) == -1 ||
>> >>        avrule_write_list(decl->avrules, fp) == -1 ||
>> >> -      role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
>> >> +      role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
>> >>        role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
>> >>            return POLICYDB_ERROR;
>> >>    }
>> >
>> >
>> > --
>> > This message was distributed to subscribers of the selinux mailing list.
>> > If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
>> > the words "unsubscribe selinux" without quotes as the message.
>>
>> I think the below patch fixes the problem I was seeing. Some rules were
>> getting discarded in role_trans_write during a policy downgrade, but the
>> count was not being decreased. This fix is similar to your fix in
>> role_trans_rule_write.
>
> Looks as appropriate to me as when range trans does it.
>
> -Eric
>
>> ---
>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>> index 78b3aa6..3a9c35a 100644
>> --- a/libsepol/src/write.c
>> +++ b/libsepol/src/write.c
>> @@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct
>> policy_file *fp)
>>
>>     nel = 0;
>>     for (tr = r; tr; tr = tr->next)
>> -       nel++;
>> +       if(new_roletr || tr->class == SECCLASS_PROCESS)
>> +           nel++;
>> +
>>     buf[0] = cpu_to_le32(nel);
>>     items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>     if (items != 1)
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Joshua Brindle]

Eric Paris wrote:
> So I'm questioning the correctness of the range_transition and
> role_transition rules in libsepol.  My main problem is that libsepol
> defines SECCLASS_* at all.  Right now if the policy reads in one of
> these rules types without a tclass it will set SECCLASS_PROCESS in the
> tclasses bitmap.  But we never had any that would declare what the
> means.  At link time when we have to map "process" in a module to
> "process" in the base policy.  But if the module didn't require
> "process" it won't have the mapping.  So the fact that we set the
> SECCLASS_PROCESS bit could cause it to get mapped to random crap (or
> nothing at all)  Right?
>
> I know it's ugly but I think we need to do a couple of things.  #1 on
> that list is get SECCLASS_* out of libsepol altogether.  Those are
> COMPLETELY a construct of policy.  Not libsepol.  After we remove all
> of those we need to change the logic of everything that uses them to
> instead make sure that the "process" class exists in it's definitions
> and if not declare it.  Then set the bitmap for that new object.
>
> Am I not understanding something about how using SECCLASS_PROCESS
> could ever be a good idea?

It isn't the first time we've had hardcoded symbols that have to be mapped in 
the code. See OBJECT_R_VAL as an example.

It is a bit wonky though, suppose a type_transition rule were in a XenSE policy, 
I don't think they have a process object class.

>
> -Eric
>
> On Wed, Apr 13, 2011 at 2:07 PM, Eric Paris<eparis@redhat.com>  wrote:
>> On Wed, 2011-04-13 at 13:35 -0400, Steve Lawrence wrote:
>>> On 04/13/2011 11:23 AM, Steve Lawrence wrote:
>>>> Thanks for the patch. This solves the problem, but I'm still seeing
>>>> issues that seems related to support of older version of policy when
>>>> using role_transition with non-process classes. I'm now seeing an mls
>>>> range overflow error:
>>>>
>>>> libsepol.role_trans_write: Discarding role_transition rules for security
>>>> class other than "process"
>>>> libsepol.mls_read_range_helper: range overflow
>>>> libsepol.context_read_and_validate: error reading MLS range of context
>>>> libsepol.policydb_to_image: new policy image is invalid
>>>>
>>>> I'm still looking into it, but I'm not too familiar this part of the code.
>>>>
>>>>
>>>> On 04/12/2011 05:11 PM, Eric Paris wrote:
>>>>> Although the role trans code had support to handle the kernel policy
>>>>> when the version was less that roletrans such support was not in the
>>>>> module read/write code.  This patch adds proper support for role trans
>>>>> in modules.
>>>>>
>>>>> Signed-off-by: Eric Paris<eparis@redhat.com>
>>>>> ---
>>>>>   libsepol/src/policydb.c |   14 ++++++++++----
>>>>>   libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
>>>>>   2 files changed, 42 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>>>>> index 2ecb636..6d8ff91 100644
>>>>> --- a/libsepol/src/policydb.c
>>>>> +++ b/libsepol/src/policydb.c
>>>>> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
>>>>>     return 0;
>>>>>   }
>>>>>
>>>>> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>>>>> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
>>>>> +                          struct policy_file *fp)
>>>>>   {
>>>>>     uint32_t buf[1], nel;
>>>>>     unsigned int i;
>>>>> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
>>>>>             if (type_set_read(&tr->types, fp))
>>>>>                     return -1;
>>>>>
>>>>> -          if (ebitmap_read(&tr->classes, fp))
>>>>> -                  return -1;
>>>>> +          if (p->policyvers>= MOD_POLICYDB_VERSION_ROLETRANS) {
>>>>> +                  if (ebitmap_read(&tr->classes, fp))
>>>>> +                          return -1;
>>>>> +          } else {
>>>>> +                  if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
>>>>> +                          return -1;
>>>>> +          }
>>>>>
>>>>>             rc = next_entry(buf, fp, sizeof(uint32_t));
>>>>>             if (rc<  0)
>>>>> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
>>>>>     decl->enabled = le32_to_cpu(buf[1]);
>>>>>     if (cond_read_list(p,&decl->cond_list, fp) == -1 ||
>>>>>         avrule_read_list(p,&decl->avrules, fp) == -1 ||
>>>>> -      role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
>>>>> +      role_trans_rule_read(p,&decl->role_tr_rules, fp) == -1 ||
>>>>>         role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
>>>>>             return -1;
>>>>>     }
>>>>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>>>>> index c4f5035..78b3aa6 100644
>>>>> --- a/libsepol/src/write.c
>>>>> +++ b/libsepol/src/write.c
>>>>> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
>>>>>     return POLICYDB_SUCCESS;
>>>>>   }
>>>>>
>>>>> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
>>>>> +static int only_process(ebitmap_t *in)
>>>>> +{
>>>>> +  unsigned int i;
>>>>> +  ebitmap_node_t *node;
>>>>> +
>>>>> +  ebitmap_for_each_bit(in, node, i) {
>>>>> +          if (ebitmap_node_get_bit(node, i)&&
>>>>> +              i != SECCLASS_PROCESS - 1)
>>>>> +                  return 0;
>>>>> +  }
>>>>> +  return 1;
>>>>> +}
>>>>> +
>>>>> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
>>>>> +                           struct policy_file *fp)
>>>>>   {
>>>>>     int nel = 0;
>>>>>     size_t items;
>>>>>     uint32_t buf[1];
>>>>>     role_trans_rule_t *tr;
>>>>> +  int warned = 0;
>>>>> +  int new_role = p->policyvers>= MOD_POLICYDB_VERSION_ROLETRANS;
>>>>>
>>>>>     for (tr = t; tr; tr = tr->next)
>>>>> -          nel++;
>>>>> +          if (new_role || only_process(&tr->classes))
>>>>> +                  nel++;
>>>>> +
>>>>>     buf[0] = cpu_to_le32(nel);
>>>>>     items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>>>>     if (items != 1)
>>>>>             return POLICYDB_ERROR;
>>>>>     for (tr = t; tr; tr = tr->next) {
>>>>> +          if (!new_role&&  !only_process(&tr->classes)) {
>>>>> +                  if (!warned)
>>>>> +                          WARN(fp->handle, "Discarding role_transition "
>>>>> +                                  "rules for security classes other than "
>>>>> +                                  "\"process\"");
>>>>> +                  warned = 1;
>>>>> +                  continue;
>>>>> +          }
>>>>>             if (role_set_write(&tr->roles, fp))
>>>>>                     return POLICYDB_ERROR;
>>>>>             if (type_set_write(&tr->types, fp))
>>>>>                     return POLICYDB_ERROR;
>>>>> -          if (ebitmap_write(&tr->classes, fp))
>>>>> -                  return POLICYDB_ERROR;
>>>>> +          if (new_role)
>>>>> +                  if (ebitmap_write(&tr->classes, fp))
>>>>> +                          return POLICYDB_ERROR;
>>>>>             buf[0] = cpu_to_le32(tr->new_role);
>>>>>             items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>>>>             if (items != 1)
>>>>> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
>>>>>     }
>>>>>     if (cond_write_list(p, decl->cond_list, fp) == -1 ||
>>>>>         avrule_write_list(decl->avrules, fp) == -1 ||
>>>>> -      role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
>>>>> +      role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
>>>>>         role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
>>>>>             return POLICYDB_ERROR;
>>>>>     }
>>>>
>>>> --
>>>> This message was distributed to subscribers of the selinux mailing list.
>>>> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
>>>> the words "unsubscribe selinux" without quotes as the message.
>>> I think the below patch fixes the problem I was seeing. Some rules were
>>> getting discarded in role_trans_write during a policy downgrade, but the
>>> count was not being decreased. This fix is similar to your fix in
>>> role_trans_rule_write.
>> Looks as appropriate to me as when range trans does it.
>>
>> -Eric
>>
>>> ---
>>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
>>> index 78b3aa6..3a9c35a 100644
>>> --- a/libsepol/src/write.c
>>> +++ b/libsepol/src/write.c
>>> @@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct
>>> policy_file *fp)
>>>
>>>      nel = 0;
>>>      for (tr = r; tr; tr = tr->next)
>>> -       nel++;
>>> +       if(new_roletr || tr->class == SECCLASS_PROCESS)
>>> +           nel++;
>>> +
>>>      buf[0] = cpu_to_le32(nel);
>>>      items = put_entry(buf, sizeof(uint32_t), 1, fp);
>>>      if (items != 1)
>>
>>
>> --
>> This message was distributed to subscribers of the selinux mailing list.
>> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
>> the words "unsubscribe selinux" without quotes as the message.
>>
>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Eric Paris]

On Thu, 2011-04-14 at 09:58 -0400, Joshua Brindle wrote:
> Eric Paris wrote:
> > So I'm questioning the correctness of the range_transition and
> > role_transition rules in libsepol.  My main problem is that libsepol
> > defines SECCLASS_* at all.  Right now if the policy reads in one of
> > these rules types without a tclass it will set SECCLASS_PROCESS in the
> > tclasses bitmap.  But we never had any that would declare what the
> > means.  At link time when we have to map "process" in a module to
> > "process" in the base policy.  But if the module didn't require
> > "process" it won't have the mapping.  So the fact that we set the
> > SECCLASS_PROCESS bit could cause it to get mapped to random crap (or
> > nothing at all)  Right?
> >
> > I know it's ugly but I think we need to do a couple of things.  #1 on
> > that list is get SECCLASS_* out of libsepol altogether.  Those are
> > COMPLETELY a construct of policy.  Not libsepol.  After we remove all
> > of those we need to change the logic of everything that uses them to
> > instead make sure that the "process" class exists in it's definitions
> > and if not declare it.  Then set the bitmap for that new object.
> >
> > Am I not understanding something about how using SECCLASS_PROCESS
> > could ever be a good idea?
> 
> It isn't the first time we've had hardcoded symbols that have to be mapped in 
> the code. See OBJECT_R_VAL as an example.
> 
> It is a bit wonky though, suppose a type_transition rule were in a XenSE policy, 
> I don't think they have a process object class.

So what it sounds like is that I probably want to look up the "process"
class every time I would have used SECCLASS_PROCESS and if I don't find
it, create it, and then look it up.  In this manor a policy which
doesn't have process at all wouldn't have it forcibly jammed upon it
(like we do in roles_init for OBJECT_R_VAL) and we wouldn't be mapping
rules to garbage.  Sound right/ok to others?

-Eric
> 
> >
> > -Eric
> >
> > On Wed, Apr 13, 2011 at 2:07 PM, Eric Paris<eparis@redhat.com>  wrote:
> >> On Wed, 2011-04-13 at 13:35 -0400, Steve Lawrence wrote:
> >>> On 04/13/2011 11:23 AM, Steve Lawrence wrote:
> >>>> Thanks for the patch. This solves the problem, but I'm still seeing
> >>>> issues that seems related to support of older version of policy when
> >>>> using role_transition with non-process classes. I'm now seeing an mls
> >>>> range overflow error:
> >>>>
> >>>> libsepol.role_trans_write: Discarding role_transition rules for security
> >>>> class other than "process"
> >>>> libsepol.mls_read_range_helper: range overflow
> >>>> libsepol.context_read_and_validate: error reading MLS range of context
> >>>> libsepol.policydb_to_image: new policy image is invalid
> >>>>
> >>>> I'm still looking into it, but I'm not too familiar this part of the code.
> >>>>
> >>>>
> >>>> On 04/12/2011 05:11 PM, Eric Paris wrote:
> >>>>> Although the role trans code had support to handle the kernel policy
> >>>>> when the version was less that roletrans such support was not in the
> >>>>> module read/write code.  This patch adds proper support for role trans
> >>>>> in modules.
> >>>>>
> >>>>> Signed-off-by: Eric Paris<eparis@redhat.com>
> >>>>> ---
> >>>>>   libsepol/src/policydb.c |   14 ++++++++++----
> >>>>>   libsepol/src/write.c    |   37 ++++++++++++++++++++++++++++++++-----
> >>>>>   2 files changed, 42 insertions(+), 9 deletions(-)
> >>>>>
> >>>>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> >>>>> index 2ecb636..6d8ff91 100644
> >>>>> --- a/libsepol/src/policydb.c
> >>>>> +++ b/libsepol/src/policydb.c
> >>>>> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules,
> >>>>>     return 0;
> >>>>>   }
> >>>>>
> >>>>> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
> >>>>> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r,
> >>>>> +                          struct policy_file *fp)
> >>>>>   {
> >>>>>     uint32_t buf[1], nel;
> >>>>>     unsigned int i;
> >>>>> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp)
> >>>>>             if (type_set_read(&tr->types, fp))
> >>>>>                     return -1;
> >>>>>
> >>>>> -          if (ebitmap_read(&tr->classes, fp))
> >>>>> -                  return -1;
> >>>>> +          if (p->policyvers>= MOD_POLICYDB_VERSION_ROLETRANS) {
> >>>>> +                  if (ebitmap_read(&tr->classes, fp))
> >>>>> +                          return -1;
> >>>>> +          } else {
> >>>>> +                  if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1))
> >>>>> +                          return -1;
> >>>>> +          }
> >>>>>
> >>>>>             rc = next_entry(buf, fp, sizeof(uint32_t));
> >>>>>             if (rc<  0)
> >>>>> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
> >>>>>     decl->enabled = le32_to_cpu(buf[1]);
> >>>>>     if (cond_read_list(p,&decl->cond_list, fp) == -1 ||
> >>>>>         avrule_read_list(p,&decl->avrules, fp) == -1 ||
> >>>>> -      role_trans_rule_read(&decl->role_tr_rules, fp) == -1 ||
> >>>>> +      role_trans_rule_read(p,&decl->role_tr_rules, fp) == -1 ||
> >>>>>         role_allow_rule_read(&decl->role_allow_rules, fp) == -1) {
> >>>>>             return -1;
> >>>>>     }
> >>>>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> >>>>> index c4f5035..78b3aa6 100644
> >>>>> --- a/libsepol/src/write.c
> >>>>> +++ b/libsepol/src/write.c
> >>>>> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp)
> >>>>>     return POLICYDB_SUCCESS;
> >>>>>   }
> >>>>>
> >>>>> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp)
> >>>>> +static int only_process(ebitmap_t *in)
> >>>>> +{
> >>>>> +  unsigned int i;
> >>>>> +  ebitmap_node_t *node;
> >>>>> +
> >>>>> +  ebitmap_for_each_bit(in, node, i) {
> >>>>> +          if (ebitmap_node_get_bit(node, i)&&
> >>>>> +              i != SECCLASS_PROCESS - 1)
> >>>>> +                  return 0;
> >>>>> +  }
> >>>>> +  return 1;
> >>>>> +}
> >>>>> +
> >>>>> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t,
> >>>>> +                           struct policy_file *fp)
> >>>>>   {
> >>>>>     int nel = 0;
> >>>>>     size_t items;
> >>>>>     uint32_t buf[1];
> >>>>>     role_trans_rule_t *tr;
> >>>>> +  int warned = 0;
> >>>>> +  int new_role = p->policyvers>= MOD_POLICYDB_VERSION_ROLETRANS;
> >>>>>
> >>>>>     for (tr = t; tr; tr = tr->next)
> >>>>> -          nel++;
> >>>>> +          if (new_role || only_process(&tr->classes))
> >>>>> +                  nel++;
> >>>>> +
> >>>>>     buf[0] = cpu_to_le32(nel);
> >>>>>     items = put_entry(buf, sizeof(uint32_t), 1, fp);
> >>>>>     if (items != 1)
> >>>>>             return POLICYDB_ERROR;
> >>>>>     for (tr = t; tr; tr = tr->next) {
> >>>>> +          if (!new_role&&  !only_process(&tr->classes)) {
> >>>>> +                  if (!warned)
> >>>>> +                          WARN(fp->handle, "Discarding role_transition "
> >>>>> +                                  "rules for security classes other than "
> >>>>> +                                  "\"process\"");
> >>>>> +                  warned = 1;
> >>>>> +                  continue;
> >>>>> +          }
> >>>>>             if (role_set_write(&tr->roles, fp))
> >>>>>                     return POLICYDB_ERROR;
> >>>>>             if (type_set_write(&tr->types, fp))
> >>>>>                     return POLICYDB_ERROR;
> >>>>> -          if (ebitmap_write(&tr->classes, fp))
> >>>>> -                  return POLICYDB_ERROR;
> >>>>> +          if (new_role)
> >>>>> +                  if (ebitmap_write(&tr->classes, fp))
> >>>>> +                          return POLICYDB_ERROR;
> >>>>>             buf[0] = cpu_to_le32(tr->new_role);
> >>>>>             items = put_entry(buf, sizeof(uint32_t), 1, fp);
> >>>>>             if (items != 1)
> >>>>> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
> >>>>>     }
> >>>>>     if (cond_write_list(p, decl->cond_list, fp) == -1 ||
> >>>>>         avrule_write_list(decl->avrules, fp) == -1 ||
> >>>>> -      role_trans_rule_write(decl->role_tr_rules, fp) == -1 ||
> >>>>> +      role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 ||
> >>>>>         role_allow_rule_write(decl->role_allow_rules, fp) == -1) {
> >>>>>             return POLICYDB_ERROR;
> >>>>>     }
> >>>>
> >>>> --
> >>>> This message was distributed to subscribers of the selinux mailing list.
> >>>> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> >>>> the words "unsubscribe selinux" without quotes as the message.
> >>> I think the below patch fixes the problem I was seeing. Some rules were
> >>> getting discarded in role_trans_write during a policy downgrade, but the
> >>> count was not being decreased. This fix is similar to your fix in
> >>> role_trans_rule_write.
> >> Looks as appropriate to me as when range trans does it.
> >>
> >> -Eric
> >>
> >>> ---
> >>> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> >>> index 78b3aa6..3a9c35a 100644
> >>> --- a/libsepol/src/write.c
> >>> +++ b/libsepol/src/write.c
> >>> @@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct
> >>> policy_file *fp)
> >>>
> >>>      nel = 0;
> >>>      for (tr = r; tr; tr = tr->next)
> >>> -       nel++;
> >>> +       if(new_roletr || tr->class == SECCLASS_PROCESS)
> >>> +           nel++;
> >>> +
> >>>      buf[0] = cpu_to_le32(nel);
> >>>      items = put_entry(buf, sizeof(uint32_t), 1, fp);
> >>>      if (items != 1)
> >>
> >>
> >> --
> >> This message was distributed to subscribers of the selinux mailing list.
> >> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> >> the words "unsubscribe selinux" without quotes as the message.
> >>
> >



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Joshua Brindle]

Eric Paris wrote:
> On Thu, 2011-04-14 at 09:58 -0400, Joshua Brindle wrote:
>> Eric Paris wrote:
>>> So I'm questioning the correctness of the range_transition and
>>> role_transition rules in libsepol.  My main problem is that libsepol
>>> defines SECCLASS_* at all.  Right now if the policy reads in one of
>>> these rules types without a tclass it will set SECCLASS_PROCESS in the
>>> tclasses bitmap.  But we never had any that would declare what the
>>> means.  At link time when we have to map "process" in a module to
>>> "process" in the base policy.  But if the module didn't require
>>> "process" it won't have the mapping.  So the fact that we set the
>>> SECCLASS_PROCESS bit could cause it to get mapped to random crap (or
>>> nothing at all)  Right?
>>>
>>> I know it's ugly but I think we need to do a couple of things.  #1 on
>>> that list is get SECCLASS_* out of libsepol altogether.  Those are
>>> COMPLETELY a construct of policy.  Not libsepol.  After we remove all
>>> of those we need to change the logic of everything that uses them to
>>> instead make sure that the "process" class exists in it's definitions
>>> and if not declare it.  Then set the bitmap for that new object.
>>>
>>> Am I not understanding something about how using SECCLASS_PROCESS
>>> could ever be a good idea?
>> It isn't the first time we've had hardcoded symbols that have to be mapped in
>> the code. See OBJECT_R_VAL as an example.
>>
>> It is a bit wonky though, suppose a type_transition rule were in a XenSE policy,
>> I don't think they have a process object class.
>
> So what it sounds like is that I probably want to look up the "process"
> class every time I would have used SECCLASS_PROCESS and if I don't find
> it, create it, and then look it up.  In this manor a policy which
> doesn't have process at all wouldn't have it forcibly jammed upon it
> (like we do in roles_init for OBJECT_R_VAL) and we wouldn't be mapping
> rules to garbage.  Sound right/ok to others?
>

It isn't the most elegant but there is precedence. I'm fine with it.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Stephen Smalley]

On Thu, 2011-04-14 at 09:55 -0400, Eric Paris wrote:
> So I'm questioning the correctness of the range_transition and
> role_transition rules in libsepol.  My main problem is that libsepol
> defines SECCLASS_* at all.  Right now if the policy reads in one of
> these rules types without a tclass it will set SECCLASS_PROCESS in the
> tclasses bitmap.  But we never had any that would declare what the
> means.  At link time when we have to map "process" in a module to
> "process" in the base policy.  But if the module didn't require
> "process" it won't have the mapping.  So the fact that we set the
> SECCLASS_PROCESS bit could cause it to get mapped to random crap (or
> nothing at all)  Right?
> 
> I know it's ugly but I think we need to do a couple of things.  #1 on
> that list is get SECCLASS_* out of libsepol altogether.  Those are
> COMPLETELY a construct of policy.  Not libsepol.  After we remove all
> of those we need to change the logic of everything that uses them to
> instead make sure that the "process" class exists in it's definitions
> and if not declare it.  Then set the bitmap for that new object.
> 
> Am I not understanding something about how using SECCLASS_PROCESS
> could ever be a good idea?

Until we introduced the kernel support for dynamic class/perm mapping,
the kernel classes were guaranteed to remain stable in value and thus
both libsepol and the kernel security server could (and did) directly
use SECCLASS_PROCESS.  It is still presently valid since we have yet to
add any classes to the secclass_map[] in the kernel before the process
class, so the policy value and the kernel index are still
identity-mapped, but this could of course change going forward.  So
libsepol needs to be fixed, but that shouldn't cause a bug at present.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] libsepol: support policy modules when roletrans rules not supported

[Eric Paris]

On Thu, 2011-04-14 at 09:58 -0400, Stephen Smalley wrote:
> On Thu, 2011-04-14 at 09:55 -0400, Eric Paris wrote:
> > So I'm questioning the correctness of the range_transition and
> > role_transition rules in libsepol.  My main problem is that libsepol
> > defines SECCLASS_* at all.  Right now if the policy reads in one of
> > these rules types without a tclass it will set SECCLASS_PROCESS in the
> > tclasses bitmap.  But we never had any that would declare what the
> > means.  At link time when we have to map "process" in a module to
> > "process" in the base policy.  But if the module didn't require
> > "process" it won't have the mapping.  So the fact that we set the
> > SECCLASS_PROCESS bit could cause it to get mapped to random crap (or
> > nothing at all)  Right?
> > 
> > I know it's ugly but I think we need to do a couple of things.  #1 on
> > that list is get SECCLASS_* out of libsepol altogether.  Those are
> > COMPLETELY a construct of policy.  Not libsepol.  After we remove all
> > of those we need to change the logic of everything that uses them to
> > instead make sure that the "process" class exists in it's definitions
> > and if not declare it.  Then set the bitmap for that new object.
> > 
> > Am I not understanding something about how using SECCLASS_PROCESS
> > could ever be a good idea?
> 
> Until we introduced the kernel support for dynamic class/perm mapping,
> the kernel classes were guaranteed to remain stable in value and thus
> both libsepol and the kernel security server could (and did) directly
> use SECCLASS_PROCESS.  It is still presently valid since we have yet to
> add any classes to the secclass_map[] in the kernel before the process
> class, so the policy value and the kernel index are still
> identity-mapped, but this could of course change going forward.  So
> libsepol needs to be fixed, but that shouldn't cause a bug at present.

So lets say I define a module with something like:

require {
	type unconfined_t;
	type kernel_t;
	class file read;
	class socket read;
	sensitivity s0;
}

allow unconfined_t kernel_t : file read;
allow unconfined_t kernel_t : socket read;
range_transition unconfined_t kernel_t s0;

The module is going to be created setting file as class #1 and socket as
class #2.  Since SECCLASS_PROCESS == 2 we are going to implicitly set
bit 2 when we create the range_transition rule.  Right?  Now when we map
from "bit 2" in the module (which is "socket") to the base.pp we are
actually going to put the rule

range_transition unconfined_t kernel_t : socket s0;

In the final policy.  Right?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.