[PATCH 1/2] libsepol: with pp to CIL, only associate declared roleattributes with in-scope types

[PATCH 1/2] libsepol: with pp to CIL, only associate declared roleattributes with in-scope types

[Steve Lawrence]

When a roleattribute is in a declared scope, CIL roletype statements are
generated for all types associated with it. This incorrectly includes
types that are associated with the roleattribute in optional blocks,
which can result in CIL resolution failures if the optional block is
turned off due to a missing type. So, change the roletype CIL statement
generation with roleattributes to mimic the behavior of roles, ensuring
declared roleattributes are only associated with in-scope types.

Signed-off-by: Steve Lawrence <slawrence@tresys.com>
Reported-by: Miroslav Grepl <mgrepl@redhat.com>
---
 libsepol/src/module_to_cil.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 8d16d3e..1ded21d 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -2091,7 +2091,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
 
 
 		for (i = 0; i < num_types; i++) {
-			cil_println(indent, "(roletype %s %s)", key, types[i]);
+			if (is_id_in_scope(pdb, decl_stack, types[i], SYM_TYPES)) {
+				cil_println(indent, "(roletype %s %s)", key, types[i]);
+			}
 		}
 
 		break;
-- 
2.1.0

[PATCH 2/2] libsepol: with pp to CIL, always write auditadm_r and secadm_r roles to the base module

[Steve Lawrence]

In fedora and refpolicy, the auditadm_r and secadm_r roles can be in
either the base module or a non-base module, or they could be in both.
This means that it is possible for duplicate role declarations to exist.
CIL does not allow duplicate declarations of anything, but there is no
way for the pp compiler to know if the roles are declared in which
module, or if they are in both when compiling a single module. This
means we cannot use the same hack that we use for user_r, staff_r, etc.,
to generate CIL role declarations (i.e. only create role declarations
for these when defined in base).

So only for these two roles, always declare them as part of base,
regardless of where or if they are defined. This means that turning off
the auditadm module will never remove the auditamd_r role (likewise for
secadm), whereas right now, in some cases it would. This also means that
role allow rules will still exist for these roles even with the modules
removed. However, this is okay because the roles would not have any
types associated with them so no access would be allowed.

Signed-off-by: Steve Lawrence <slawrence@tresys.com>
Reported-by: Miroslav Grepl <mgrepl@redhat.com>
---
 libsepol/src/module_to_cil.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 1ded21d..2d8046c 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -2035,12 +2035,22 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
 			// one of these roles in base, the declaration will not appeaer in
 			// the resulting policy, likely resulting in a compilation error in
 			// CIL.
+			//
+			// To make things more complicated, the auditadm_r and secadm_r
+			// roles could actually be in either the base module or a non-base
+			// module, or both. So we can't rely on this same behavior. So for
+			// these roles, don't declare them here, even if they are in a base
+			// or non-base module. Instead we will just declare them in the
+			// base module elsewhere.
 			int is_base_role = (!strcmp(key, "user_r") ||
 			                    !strcmp(key, "staff_r") ||
 			                    !strcmp(key, "sysadm_r") ||
 			                    !strcmp(key, "system_r") ||
 			                    !strcmp(key, "unconfined_r"));
-			if ((is_base_role && pdb->policy_type == SEPOL_POLICY_BASE) || !is_base_role) {
+			int is_builtin_role = (!strcmp(key, "auditadm_r") ||
+			                       !strcmp(key, "secadm_r"));
+			if ((is_base_role && pdb->policy_type == SEPOL_POLICY_BASE) ||
+			    (!is_base_role && !is_builtin_role)) {
 				cil_println(indent, "(role %s)", key);
 			}
 		}
@@ -3717,6 +3727,17 @@ static int generate_default_object(void)
 	return 0;
 }
 
+static int generate_builtin_roles(void)
+{
+	// due to inconsistentencies between policies and CIL not allowing
+	// duplicate roles, some roles are always created, regardless of if they
+	// are declared in modules or not
+	cil_println(0, "(role auditadm_r)");
+	cil_println(0, "(role secadm_r)");
+
+	return 0;
+}
+
 static int generate_gen_require_attribute(void)
 {
 	cil_println(0, "(typeattribute " GEN_REQUIRE_ATTR ")");
@@ -3801,6 +3822,11 @@ int sepol_module_policydb_to_cil(FILE *fp, struct policydb *pdb, int linked)
 			goto exit;
 		}
 
+		rc = generate_builtin_roles();
+		if (rc != 0) {
+			goto exit;
+		}
+
 		// default attribute to be used to mimic gen_require in CIL
 		rc = generate_gen_require_attribute();
 		if (rc != 0) {
-- 
2.1.0

Re: [PATCH 2/2] libsepol: with pp to CIL, always write auditadm_r and secadm_r roles to the base module

[Miroslav Grepl]

On 05/22/2015 07:34 PM, Steve Lawrence wrote:
> In fedora and refpolicy, the auditadm_r and secadm_r roles can be in
> either the base module or a non-base module, or they could be in both.
> This means that it is possible for duplicate role declarations to exist.
> CIL does not allow duplicate declarations of anything, but there is no
> way for the pp compiler to know if the roles are declared in which
> module, or if they are in both when compiling a single module. This
> means we cannot use the same hack that we use for user_r, staff_r, etc.,
> to generate CIL role declarations (i.e. only create role declarations
> for these when defined in base).
> 
> So only for these two roles, always declare them as part of base,
> regardless of where or if they are defined. This means that turning off
> the auditadm module will never remove the auditamd_r role (likewise for
> secadm), whereas right now, in some cases it would. This also means that
> role allow rules will still exist for these roles even with the modules
> removed. However, this is okay because the roles would not have any
> types associated with them so no access would be allowed.
> 
> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
> Reported-by: Miroslav Grepl <mgrepl@redhat.com>
> ---
>  libsepol/src/module_to_cil.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 1ded21d..2d8046c 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -2035,12 +2035,22 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
>  			// one of these roles in base, the declaration will not appeaer in
>  			// the resulting policy, likely resulting in a compilation error in
>  			// CIL.
> +			//
> +			// To make things more complicated, the auditadm_r and secadm_r
> +			// roles could actually be in either the base module or a non-base
> +			// module, or both. So we can't rely on this same behavior. So for
> +			// these roles, don't declare them here, even if they are in a base
> +			// or non-base module. Instead we will just declare them in the
> +			// base module elsewhere.
>  			int is_base_role = (!strcmp(key, "user_r") ||
>  			                    !strcmp(key, "staff_r") ||
>  			                    !strcmp(key, "sysadm_r") ||
>  			                    !strcmp(key, "system_r") ||
>  			                    !strcmp(key, "unconfined_r"));
> -			if ((is_base_role && pdb->policy_type == SEPOL_POLICY_BASE) || !is_base_role) {
> +			int is_builtin_role = (!strcmp(key, "auditadm_r") ||
> +			                       !strcmp(key, "secadm_r"));
> +			if ((is_base_role && pdb->policy_type == SEPOL_POLICY_BASE) ||
> +			    (!is_base_role && !is_builtin_role)) {
>  				cil_println(indent, "(role %s)", key);
>  			}
>  		}
> @@ -3717,6 +3727,17 @@ static int generate_default_object(void)
>  	return 0;
>  }
>  
> +static int generate_builtin_roles(void)
> +{
> +	// due to inconsistentencies between policies and CIL not allowing
> +	// duplicate roles, some roles are always created, regardless of if they
> +	// are declared in modules or not
> +	cil_println(0, "(role auditadm_r)");
> +	cil_println(0, "(role secadm_r)");
> +
> +	return 0;
> +}
> +
>  static int generate_gen_require_attribute(void)
>  {
>  	cil_println(0, "(typeattribute " GEN_REQUIRE_ATTR ")");
> @@ -3801,6 +3822,11 @@ int sepol_module_policydb_to_cil(FILE *fp, struct policydb *pdb, int linked)
>  			goto exit;
>  		}
>  
> +		rc = generate_builtin_roles();
> +		if (rc != 0) {
> +			goto exit;
> +		}
> +
>  		// default attribute to be used to mimic gen_require in CIL
>  		rc = generate_gen_require_attribute();
>  		if (rc != 0) {
> 
Thank you.

-- 
Miroslav Grepl
Software Engineering, SELinux Solutions
Red Hat, Inc.

Re: [PATCH 2/2] libsepol: with pp to CIL, always write auditadm_r and secadm_r roles to the base module

[James Carter]

On 05/22/2015 01:34 PM, Steve Lawrence wrote:
> In fedora and refpolicy, the auditadm_r and secadm_r roles can be in
> either the base module or a non-base module, or they could be in both.
> This means that it is possible for duplicate role declarations to exist.
> CIL does not allow duplicate declarations of anything, but there is no
> way for the pp compiler to know if the roles are declared in which
> module, or if they are in both when compiling a single module. This
> means we cannot use the same hack that we use for user_r, staff_r, etc.,
> to generate CIL role declarations (i.e. only create role declarations
> for these when defined in base).
>
> So only for these two roles, always declare them as part of base,
> regardless of where or if they are defined. This means that turning off
> the auditadm module will never remove the auditamd_r role (likewise for
> secadm), whereas right now, in some cases it would. This also means that
> role allow rules will still exist for these roles even with the modules
> removed. However, this is okay because the roles would not have any
> types associated with them so no access would be allowed.
>
> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
> Reported-by: Miroslav Grepl <mgrepl@redhat.com>

Thanks, applied.

> ---
>   libsepol/src/module_to_cil.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 1ded21d..2d8046c 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -2035,12 +2035,22 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
>   			// one of these roles in base, the declaration will not appeaer in
>   			// the resulting policy, likely resulting in a compilation error in
>   			// CIL.
> +			//
> +			// To make things more complicated, the auditadm_r and secadm_r
> +			// roles could actually be in either the base module or a non-base
> +			// module, or both. So we can't rely on this same behavior. So for
> +			// these roles, don't declare them here, even if they are in a base
> +			// or non-base module. Instead we will just declare them in the
> +			// base module elsewhere.
>   			int is_base_role = (!strcmp(key, "user_r") ||
>   			                    !strcmp(key, "staff_r") ||
>   			                    !strcmp(key, "sysadm_r") ||
>   			                    !strcmp(key, "system_r") ||
>   			                    !strcmp(key, "unconfined_r"));
> -			if ((is_base_role && pdb->policy_type == SEPOL_POLICY_BASE) || !is_base_role) {
> +			int is_builtin_role = (!strcmp(key, "auditadm_r") ||
> +			                       !strcmp(key, "secadm_r"));
> +			if ((is_base_role && pdb->policy_type == SEPOL_POLICY_BASE) ||
> +			    (!is_base_role && !is_builtin_role)) {
>   				cil_println(indent, "(role %s)", key);
>   			}
>   		}
> @@ -3717,6 +3727,17 @@ static int generate_default_object(void)
>   	return 0;
>   }
>
> +static int generate_builtin_roles(void)
> +{
> +	// due to inconsistentencies between policies and CIL not allowing
> +	// duplicate roles, some roles are always created, regardless of if they
> +	// are declared in modules or not
> +	cil_println(0, "(role auditadm_r)");
> +	cil_println(0, "(role secadm_r)");
> +
> +	return 0;
> +}
> +
>   static int generate_gen_require_attribute(void)
>   {
>   	cil_println(0, "(typeattribute " GEN_REQUIRE_ATTR ")");
> @@ -3801,6 +3822,11 @@ int sepol_module_policydb_to_cil(FILE *fp, struct policydb *pdb, int linked)
>   			goto exit;
>   		}
>
> +		rc = generate_builtin_roles();
> +		if (rc != 0) {
> +			goto exit;
> +		}
> +
>   		// default attribute to be used to mimic gen_require in CIL
>   		rc = generate_gen_require_attribute();
>   		if (rc != 0) {
>


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

Re: [PATCH 1/2] libsepol: with pp to CIL, only associate declared roleattributes with in-scope types

[James Carter]

On 05/22/2015 01:34 PM, Steve Lawrence wrote:
> When a roleattribute is in a declared scope, CIL roletype statements are
> generated for all types associated with it. This incorrectly includes
> types that are associated with the roleattribute in optional blocks,
> which can result in CIL resolution failures if the optional block is
> turned off due to a missing type. So, change the roletype CIL statement
> generation with roleattributes to mimic the behavior of roles, ensuring
> declared roleattributes are only associated with in-scope types.
>
> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
> Reported-by: Miroslav Grepl <mgrepl@redhat.com>
> ---
>   libsepol/src/module_to_cil.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 8d16d3e..1ded21d 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -2091,7 +2091,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
>
>
>   		for (i = 0; i < num_types; i++) {
> -			cil_println(indent, "(roletype %s %s)", key, types[i]);
> +			if (is_id_in_scope(pdb, decl_stack, types[i], SYM_TYPES)) {
> +				cil_println(indent, "(roletype %s %s)", key, types[i]);
> +			}
>   		}
>
>   		break;
>

This breaks generating CIL from a policy.conf, or, more likely, exposes a 
problem in what I did to make that work.

A roletype rule will not appear in the generated CIL policy, if
1) It is in an optional block
2) It is for a role attribute that is in the required block of the optional.

The policydb to CIL code is called after policy linking, so I would have thought 
that everything should work. I am still looking at this.

-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

Re: [PATCH 1/2] libsepol: with pp to CIL, only associate declared roleattributes with in-scope types

[James Carter]

On 05/28/2015 03:08 PM, James Carter wrote:
> On 05/22/2015 01:34 PM, Steve Lawrence wrote:
>> When a roleattribute is in a declared scope, CIL roletype statements are
>> generated for all types associated with it. This incorrectly includes
>> types that are associated with the roleattribute in optional blocks,
>> which can result in CIL resolution failures if the optional block is
>> turned off due to a missing type. So, change the roletype CIL statement
>> generation with roleattributes to mimic the behavior of roles, ensuring
>> declared roleattributes are only associated with in-scope types.
>>
>> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
>> Reported-by: Miroslav Grepl <mgrepl@redhat.com>
>> ---
>>   libsepol/src/module_to_cil.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
>> index 8d16d3e..1ded21d 100644
>> --- a/libsepol/src/module_to_cil.c
>> +++ b/libsepol/src/module_to_cil.c
>> @@ -2091,7 +2091,9 @@ static int role_to_cil(int indent, struct policydb *pdb,
>> struct avrule_block *UN
>>
>>
>>           for (i = 0; i < num_types; i++) {
>> -            cil_println(indent, "(roletype %s %s)", key, types[i]);
>> +            if (is_id_in_scope(pdb, decl_stack, types[i], SYM_TYPES)) {
>> +                cil_println(indent, "(roletype %s %s)", key, types[i]);
>> +            }
>>           }
>>
>>           break;
>>
>
> This breaks generating CIL from a policy.conf, or, more likely, exposes a
> problem in what I did to make that work.
>
> A roletype rule will not appear in the generated CIL policy, if
> 1) It is in an optional block
> 2) It is for a role attribute that is in the required block of the optional.
>
> The policydb to CIL code is called after policy linking, so I would have thought
> that everything should work. I am still looking at this.
>

Actually, it doesn't matter whether or not the role attribute is in a require 
block. The determining factor is whether there is any role rule (CIL roletype 
rule) outside of an optional block. If there is, then all of the roletype rules 
for that role attribute will be in the CIL policy. If not, then none of them 
will be.

-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

Re: [PATCH 1/2] libsepol: with pp to CIL, only associate declared roleattributes with in-scope types

[James Carter]

On 05/28/2015 04:15 PM, James Carter wrote:
> On 05/28/2015 03:08 PM, James Carter wrote:
>> On 05/22/2015 01:34 PM, Steve Lawrence wrote:
>>> When a roleattribute is in a declared scope, CIL roletype statements are
>>> generated for all types associated with it. This incorrectly includes
>>> types that are associated with the roleattribute in optional blocks,
>>> which can result in CIL resolution failures if the optional block is
>>> turned off due to a missing type. So, change the roletype CIL statement
>>> generation with roleattributes to mimic the behavior of roles, ensuring
>>> declared roleattributes are only associated with in-scope types.
>>>
>>> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
>>> Reported-by: Miroslav Grepl <mgrepl@redhat.com>
>>> ---
>>>   libsepol/src/module_to_cil.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
>>> index 8d16d3e..1ded21d 100644
>>> --- a/libsepol/src/module_to_cil.c
>>> +++ b/libsepol/src/module_to_cil.c
>>> @@ -2091,7 +2091,9 @@ static int role_to_cil(int indent, struct policydb *pdb,
>>> struct avrule_block *UN
>>>
>>>
>>>           for (i = 0; i < num_types; i++) {
>>> -            cil_println(indent, "(roletype %s %s)", key, types[i]);
>>> +            if (is_id_in_scope(pdb, decl_stack, types[i], SYM_TYPES)) {
>>> +                cil_println(indent, "(roletype %s %s)", key, types[i]);
>>> +            }
>>>           }
>>>
>>>           break;
>>>
>>
>> This breaks generating CIL from a policy.conf, or, more likely, exposes a
>> problem in what I did to make that work.
>>
>> A roletype rule will not appear in the generated CIL policy, if
>> 1) It is in an optional block
>> 2) It is for a role attribute that is in the required block of the optional.
>>
>> The policydb to CIL code is called after policy linking, so I would have thought
>> that everything should work. I am still looking at this.
>>
>
> Actually, it doesn't matter whether or not the role attribute is in a require
> block. The determining factor is whether there is any role rule (CIL roletype
> rule) outside of an optional block. If there is, then all of the roletype rules
> for that role attribute will be in the CIL policy. If not, then none of them
> will be.
>

I apparently did not have enough caffeine yesterday. I now see that everything 
was working exactly as it should. When converting a policy.conf to CIL, two 
roletype rules were being generated for these rules. This patch correctly 
eliminates one of them.

-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

Re: [PATCH 1/2] libsepol: with pp to CIL, only associate declared roleattributes with in-scope types

[James Carter]

On 05/22/2015 01:34 PM, Steve Lawrence wrote:
> When a roleattribute is in a declared scope, CIL roletype statements are
> generated for all types associated with it. This incorrectly includes
> types that are associated with the roleattribute in optional blocks,
> which can result in CIL resolution failures if the optional block is
> turned off due to a missing type. So, change the roletype CIL statement
> generation with roleattributes to mimic the behavior of roles, ensuring
> declared roleattributes are only associated with in-scope types.
>
> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
> Reported-by: Miroslav Grepl <mgrepl@redhat.com>

Thanks, applied.

> ---
>   libsepol/src/module_to_cil.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 8d16d3e..1ded21d 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -2091,7 +2091,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
>
>
>   		for (i = 0; i < num_types; i++) {
> -			cil_println(indent, "(roletype %s %s)", key, types[i]);
> +			if (is_id_in_scope(pdb, decl_stack, types[i], SYM_TYPES)) {
> +				cil_println(indent, "(roletype %s %s)", key, types[i]);
> +			}
>   		}
>
>   		break;
>


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency