[PATCH v3 1/2] libsepol: Fix type alias handling in kernel_to_cil

[PATCH v3 1/2] libsepol: Fix type alias handling in kernel_to_cil

[James Carter]

Type alias rules are not written out when converting a binary kernel
policy to CIL. The problem is that type aliases are not in the
type_val_to_struct array and that is what is being used to find the
aliases.

Since type aliases are only in the types hashtable, walk that to
find the type aliases.

Fixes: 70a480bfcd46214a ("libsepol: Add ability to convert binary
       policy to CIL")

Signed-off-by: James Carter <jwcart2@gmail.com>
---
v2: No changes
v3: Add "__attribute__((unused))" to unused parameters as suggested by
    Nicolas Iooss

 libsepol/src/kernel_to_cil.c | 44 +++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
index ede78a20..9acf2ec2 100644
--- a/libsepol/src/kernel_to_cil.c
+++ b/libsepol/src/kernel_to_cil.c
@@ -1367,33 +1367,55 @@ exit:
 	return rc;
 }
 
+static int map_count_type_aliases(__attribute__((unused)) char *key, void *data, void *args)
+{
+	type_datum_t *datum = data;
+	unsigned *count = args;
+
+	if (datum->primary == 0 && datum->flavor == TYPE_TYPE)
+		(*count)++;
+
+	return SEPOL_OK;
+}
+
+static int map_type_aliases_to_strs(char *key, void *data, void *args)
+{
+	type_datum_t *datum = data;
+	struct strs *strs = args;
+	int rc = 0;
+
+	if (datum->primary == 0 && datum->flavor == TYPE_TYPE)
+		rc = strs_add(strs, key);
+
+	return rc;
+}
+
 static int write_type_alias_rules_to_cil(FILE *out, struct policydb *pdb)
 {
 	type_datum_t *alias;
 	struct strs *strs;
 	char *name;
 	char *type;
-	unsigned i, num;
+	unsigned i, num = 0;
 	int rc = 0;
 
-	rc = strs_init(&strs, pdb->p_types.nprim);
+	rc = hashtab_map(pdb->p_types.table, map_count_type_aliases, &num);
 	if (rc != 0) {
 		goto exit;
 	}
 
-	for (i=0; i < pdb->p_types.nprim; i++) {
-		alias = pdb->type_val_to_struct[i];
-		if (!alias->primary) {
-			rc = strs_add(strs, pdb->p_type_val_to_name[i]);
-			if (rc != 0) {
-				goto exit;
-			}
-		}
+	rc = strs_init(&strs, num);
+	if (rc != 0) {
+		goto exit;
+	}
+
+	rc = hashtab_map(pdb->p_types.table, map_type_aliases_to_strs, strs);
+	if (rc != 0) {
+		goto exit;
 	}
 
 	strs_sort(strs);
 
-	num = strs_num_items(strs);
 	for (i=0; i<num; i++) {
 		name = strs_read_at_index(strs, i);
 		if (!name) {
-- 
2.25.4

[PATCH v3 2/2] libsepol: Fix type alias handling in kernel_to_conf

[James Carter]

Type alias rules are not written out when converting a binary kernel
policy to a policy.conf. The problem is that type aliases are not in
the type_val_to_struct array and that is what is being used to find
the aliases.

Since type aliases are only in the types hashtable, walk that to
find the type aliases.

Fixed the syntax of the typalias rule which requires "alias" to come
between the type and the aliases (ex/ typealias TYPE alias ALIAS;).

Fixes: 0a08fd1e69797d6a ("libsepol: Add ability to convert binary
       policy to policy.conf file")

Signed-off-by: James Carter <jwcart2@gmail.com>
---
v2: Fix typealias syntax
v3: Add "__attribute__((unused))" to unused parameters as suggested by
    Nicolas Iooss

 libsepol/src/kernel_to_conf.c | 47 +++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index 9de64832..046d76a4 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -1353,34 +1353,55 @@ exit:
 	return rc;
 }
 
+static int map_count_type_aliases(__attribute__((unused)) char *key, void *data, void *args)
+{
+	type_datum_t *datum = data;
+	unsigned *count = args;
+
+	if (datum->primary == 0 && datum->flavor == TYPE_TYPE)
+		(*count)++;
+
+	return SEPOL_OK;
+}
+
+static int map_type_aliases_to_strs(char *key, void *data, void *args)
+{
+	type_datum_t *datum = data;
+	struct strs *strs = args;
+	int rc = 0;
+
+	if (datum->primary == 0 && datum->flavor == TYPE_TYPE)
+		rc = strs_add(strs, key);
+
+	return rc;
+}
+
 static int write_type_alias_rules_to_conf(FILE *out, struct policydb *pdb)
 {
 	type_datum_t *alias;
 	struct strs *strs;
 	char *name;
 	char *type;
-	unsigned i, num;
+	unsigned i, num = 0;
 	int rc = 0;
 
-	rc = strs_init(&strs, pdb->p_types.nprim);
+	rc = hashtab_map(pdb->p_types.table, map_count_type_aliases, &num);
 	if (rc != 0) {
 		goto exit;
 	}
 
-	for (i=0; i < pdb->p_types.nprim; i++) {
-		alias = pdb->type_val_to_struct[i];
-		if (!alias->primary) {
-			rc = strs_add(strs, pdb->p_type_val_to_name[i]);
-			if (rc != 0) {
-				goto exit;
-			}
-		}
+	rc = strs_init(&strs, num);
+	if (rc != 0) {
+		goto exit;
 	}
 
+	rc = hashtab_map(pdb->p_types.table, map_type_aliases_to_strs, strs);
+	if (rc != 0) {
+		goto exit;
+	}
+	
 	strs_sort(strs);
 
-	num = strs_num_items(strs);
-
 	for (i=0; i<num; i++) {
 		name = strs_read_at_index(strs, i);
 		if (!name) {
@@ -1393,7 +1414,7 @@ static int write_type_alias_rules_to_conf(FILE *out, struct policydb *pdb)
 			goto exit;
 		}
 		type = pdb->p_type_val_to_name[alias->s.value - 1];
-		sepol_printf(out, "typealias %s %s;\n", type, name);
+		sepol_printf(out, "typealias %s alias %s;\n", type, name);
 	}
 
 exit:
-- 
2.25.4

Re: [PATCH v3 2/2] libsepol: Fix type alias handling in kernel_to_conf

[Stephen Smalley]

On Fri, May 22, 2020 at 10:55 AM James Carter <jwcart2@gmail.com> wrote:
>
> Type alias rules are not written out when converting a binary kernel
> policy to a policy.conf. The problem is that type aliases are not in
> the type_val_to_struct array and that is what is being used to find
> the aliases.
>
> Since type aliases are only in the types hashtable, walk that to
> find the type aliases.
>
> Fixed the syntax of the typalias rule which requires "alias" to come
> between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
>
> Fixes: 0a08fd1e69797d6a ("libsepol: Add ability to convert binary
>        policy to policy.conf file")
>
> Signed-off-by: James Carter <jwcart2@gmail.com>

This fixes the missing alias problem, so:
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

However, in testing these, I noticed that if I do the following:
checkpolicy -MF -o policy.conf -b /etc/selinux/targeted/policy/policy.32
checkpolicy -MC -o policy.cil -b /etc/selinux/targeted/policy/policy.32
checkpolicy -M -o policyfromconf policy.conf
secilc -o policyfromcil policy.cil
checkpolicy -M -o policyfromkernel -b /etc/selinux/targeted/policy.32

then the three policyfrom* files differ in length and contents.
Decompiling them all via checkpolicy -MF (or -MC) and diff'ing those
(since sediff takes too long) appears to suggest differences from
attribute removal (odd since I thought you reverted that), redundant
rule removal (isn't that off by default too?), and portcon ordering
(by protocol).
Optimally we should able to regenerate the same kernel policy from all
three (although we might need to run the kernel policy through
checkpolicy to normalize ordering).

Re: [PATCH v3 2/2] libsepol: Fix type alias handling in kernel_to_conf

[James Carter]

On Wed, May 27, 2020 at 10:23 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 10:55 AM James Carter <jwcart2@gmail.com> wrote:
> >
> > Type alias rules are not written out when converting a binary kernel
> > policy to a policy.conf. The problem is that type aliases are not in
> > the type_val_to_struct array and that is what is being used to find
> > the aliases.
> >
> > Since type aliases are only in the types hashtable, walk that to
> > find the type aliases.
> >
> > Fixed the syntax of the typalias rule which requires "alias" to come
> > between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
> >
> > Fixes: 0a08fd1e69797d6a ("libsepol: Add ability to convert binary
> >        policy to policy.conf file")
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
>
> This fixes the missing alias problem, so:
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
> However, in testing these, I noticed that if I do the following:
> checkpolicy -MF -o policy.conf -b /etc/selinux/targeted/policy/policy.32
> checkpolicy -MC -o policy.cil -b /etc/selinux/targeted/policy/policy.32
> checkpolicy -M -o policyfromconf policy.conf
> secilc -o policyfromcil policy.cil
> checkpolicy -M -o policyfromkernel -b /etc/selinux/targeted/policy.32
>
> then the three policyfrom* files differ in length and contents.
> Decompiling them all via checkpolicy -MF (or -MC) and diff'ing those
> (since sediff takes too long) appears to suggest differences from
> attribute removal (odd since I thought you reverted that), redundant
> rule removal (isn't that off by default too?), and portcon ordering
> (by protocol).
> Optimally we should able to regenerate the same kernel policy from all
> three (although we might need to run the kernel policy through
> checkpolicy to normalize ordering).

Interesting. I thought that we had mostly fixed the ordering issues. I
will take a look at this.
Jim

Re: [PATCH v3 2/2] libsepol: Fix type alias handling in kernel_to_conf

[James Carter]

On Wed, May 27, 2020 at 10:23 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 10:55 AM James Carter <jwcart2@gmail.com> wrote:
> >
> > Type alias rules are not written out when converting a binary kernel
> > policy to a policy.conf. The problem is that type aliases are not in
> > the type_val_to_struct array and that is what is being used to find
> > the aliases.
> >
> > Since type aliases are only in the types hashtable, walk that to
> > find the type aliases.
> >
> > Fixed the syntax of the typalias rule which requires "alias" to come
> > between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
> >
> > Fixes: 0a08fd1e69797d6a ("libsepol: Add ability to convert binary
> >        policy to policy.conf file")
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
>
> This fixes the missing alias problem, so:
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
> However, in testing these, I noticed that if I do the following:
> checkpolicy -MF -o policy.conf -b /etc/selinux/targeted/policy/policy.32
> checkpolicy -MC -o policy.cil -b /etc/selinux/targeted/policy/policy.32
> checkpolicy -M -o policyfromconf policy.conf
> secilc -o policyfromcil policy.cil
> checkpolicy -M -o policyfromkernel -b /etc/selinux/targeted/policy.32
>
> then the three policyfrom* files differ in length and contents.
> Decompiling them all via checkpolicy -MF (or -MC) and diff'ing those
> (since sediff takes too long) appears to suggest differences from
> attribute removal (odd since I thought you reverted that), redundant
> rule removal (isn't that off by default too?), and portcon ordering
> (by protocol).
> Optimally we should able to regenerate the same kernel policy from all
> three (although we might need to run the kernel policy through
> checkpolicy to normalize ordering).

Starting with a  policy.31 file and running the following:
$PREFIX/usr/bin/checkpolicy -C -M -b -o policy.cil policy.31
$PREFIX/usr/bin/secilc -M 1 -o policy.cil.bin policy.cil
$PREFIX/usr/bin/checkpolicy -C -M -b -o policy.2.cil policy.cil.bin
$PREFIX/usr/bin/secilc -M 1 -o policy.2.cil.bin policy.2.cil
$PREFIX/usr/bin/checkpolicy -C -M -b -o policy.3.cil policy.2.cil.bin
$PREFIX/usr/bin/secilc -M 1 -o policy.3.cil.bin policy.3.cil
$PREFIX/usr/bin/checkpolicy -C -M -b -o policy.4.cil policy.3.cil.bin
$PREFIX/usr/bin/checkpolicy -M -b -o policy.bin policy.31
$PREFIX/usr/bin/checkpolicy -F -M -b -o policy.conf policy.31
$PREFIX/usr/bin/checkpolicy -M -o policy.conf.bin policy.conf
$PREFIX/usr/bin/checkpolicy -F -M -b -o policy.2.conf policy.conf.bin
$PREFIX/usr/bin/checkpolicy -M -o policy.2.conf.bin policy.2.conf
$PREFIX/usr/bin/checkpolicy -F -M -b -o policy.3.conf policy.2.conf.bin
$PREFIX/usr/bin/checkpolicy -M -o policy.3.conf.bin policy.3.conf
$PREFIX/usr/bin/checkpolicy -F -M -b -o policy.4.conf policy.3.conf.bin

After the first conversion to policy.cil, all CIL binaries are the
same in everyway.
After the first conversion to policy.conf, every conf binary is the
same size and every other conf binary is the same. (sctp and udp
portcon rules for the same port range swap positions.)

All CIL source policies after the initial policy.cil are the same. The
policy.cil has attributes that are in no rules, all other CIL binaries
and source have those removed.

After the initial policy.conf, every other conf file is the same.
(Again, it is the portcon rules for scp and udp that are swapped.)

The initial policy.conf has a bunch of duplicate rules.

I thought CIL sorted portcon rules like checkpolicy, so I am not sure
why the behavior is different.
I don't know why there are duplicate rules in the initial conversion
to a policy.conf file.
I am not sure about the attribute behavior. Why are they even in the
original binary if secilc is producing it?

Jim

Re: [PATCH v3 2/2] libsepol: Fix type alias handling in kernel_to_conf

[Stephen Smalley]

On Wed, May 27, 2020 at 10:22 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 10:55 AM James Carter <jwcart2@gmail.com> wrote:
> >
> > Type alias rules are not written out when converting a binary kernel
> > policy to a policy.conf. The problem is that type aliases are not in
> > the type_val_to_struct array and that is what is being used to find
> > the aliases.
> >
> > Since type aliases are only in the types hashtable, walk that to
> > find the type aliases.
> >
> > Fixed the syntax of the typalias rule which requires "alias" to come
> > between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
> >
> > Fixes: 0a08fd1e69797d6a ("libsepol: Add ability to convert binary
> >        policy to policy.conf file")
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
>
> This fixes the missing alias problem, so:
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Applied.

Re: [PATCH v3 2/2] libsepol: Fix type alias handling in kernel_to_conf

[Stephen Smalley]

On Wed, May 27, 2020 at 5:16 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Wed, May 27, 2020 at 10:23 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, May 22, 2020 at 10:55 AM James Carter <jwcart2@gmail.com> wrote:
> > >
> > > Type alias rules are not written out when converting a binary kernel
> > > policy to a policy.conf. The problem is that type aliases are not in
> > > the type_val_to_struct array and that is what is being used to find
> > > the aliases.
> > >
> > > Since type aliases are only in the types hashtable, walk that to
> > > find the type aliases.
> > >
> > > Fixed the syntax of the typalias rule which requires "alias" to come
> > > between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
> > >
> > > Fixes: 0a08fd1e69797d6a ("libsepol: Add ability to convert binary
> > >        policy to policy.conf file")
> > >
> > > Signed-off-by: James Carter <jwcart2@gmail.com>
> >
> > This fixes the missing alias problem, so:
> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> >
> > However, in testing these, I noticed that if I do the following:
> > checkpolicy -MF -o policy.conf -b /etc/selinux/targeted/policy/policy.32
> > checkpolicy -MC -o policy.cil -b /etc/selinux/targeted/policy/policy.32
> > checkpolicy -M -o policyfromconf policy.conf
> > secilc -o policyfromcil policy.cil
> > checkpolicy -M -o policyfromkernel -b /etc/selinux/targeted/policy.32
> >
> > then the three policyfrom* files differ in length and contents.
> > Decompiling them all via checkpolicy -MF (or -MC) and diff'ing those
> > (since sediff takes too long) appears to suggest differences from
> > attribute removal (odd since I thought you reverted that), redundant
> > rule removal (isn't that off by default too?), and portcon ordering
> > (by protocol).
> > Optimally we should able to regenerate the same kernel policy from all
> > three (although we might need to run the kernel policy through
> > checkpolicy to normalize ordering).
>
> Starting with a  policy.31 file and running the following:
> $PREFIX/usr/bin/checkpolicy -C -M -b -o policy.cil policy.31
> $PREFIX/usr/bin/secilc -M 1 -o policy.cil.bin policy.cil
> $PREFIX/usr/bin/checkpolicy -C -M -b -o policy.2.cil policy.cil.bin
> $PREFIX/usr/bin/secilc -M 1 -o policy.2.cil.bin policy.2.cil
> $PREFIX/usr/bin/checkpolicy -C -M -b -o policy.3.cil policy.2.cil.bin
> $PREFIX/usr/bin/secilc -M 1 -o policy.3.cil.bin policy.3.cil
> $PREFIX/usr/bin/checkpolicy -C -M -b -o policy.4.cil policy.3.cil.bin
> $PREFIX/usr/bin/checkpolicy -M -b -o policy.bin policy.31
> $PREFIX/usr/bin/checkpolicy -F -M -b -o policy.conf policy.31
> $PREFIX/usr/bin/checkpolicy -M -o policy.conf.bin policy.conf
> $PREFIX/usr/bin/checkpolicy -F -M -b -o policy.2.conf policy.conf.bin
> $PREFIX/usr/bin/checkpolicy -M -o policy.2.conf.bin policy.2.conf
> $PREFIX/usr/bin/checkpolicy -F -M -b -o policy.3.conf policy.2.conf.bin
> $PREFIX/usr/bin/checkpolicy -M -o policy.3.conf.bin policy.3.conf
> $PREFIX/usr/bin/checkpolicy -F -M -b -o policy.4.conf policy.3.conf.bin
>
> After the first conversion to policy.cil, all CIL binaries are the
> same in everyway.
> After the first conversion to policy.conf, every conf binary is the
> same size and every other conf binary is the same. (sctp and udp
> portcon rules for the same port range swap positions.)
>
> All CIL source policies after the initial policy.cil are the same. The
> policy.cil has attributes that are in no rules, all other CIL binaries
> and source have those removed.
>
> After the initial policy.conf, every other conf file is the same.
> (Again, it is the portcon rules for scp and udp that are swapped.)
>
> The initial policy.conf has a bunch of duplicate rules.
>
> I thought CIL sorted portcon rules like checkpolicy, so I am not sure
> why the behavior is different.
> I don't know why there are duplicate rules in the initial conversion
> to a policy.conf file.
> I am not sure about the attribute behavior. Why are they even in the
> original binary if secilc is producing it?

Possible difference in default behaviors for secilc versus libsemanage
direct usage of cil_compile()?
Or something to do with the whole checkmodule -> semodule_package ->
/usr/libexec/selinux/hll/pp translation?