[PATCH 0/2 v2] Warn if module name different than output filename

[PATCH 0/2 v2] Warn if module name different than output filename

[James Carter]

Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is not the same as its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.

Because of this, provide a warning message when converting a policy
package to CIL or compiling a module and the output filename is different
than the module name.

Changes from v1:
- Added a "Warning:" prefix
- Removed checks against the input filename
- Since there are now only two checks and the base filename is used in the
warning message, it no longer made sense to create common helper functions
in libsepol.

James Carter (2):
  policycoreutils/hll/pp: Warn if module name different than output
    filename
  checkpolicy: Warn if module name different than output filename

 checkpolicy/checkmodule.c   | 15 +++++++++++++++
 policycoreutils/hll/pp/pp.c | 28 ++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

-- 
2.5.5

[PATCH 1/2 v2] policycoreutils/hll/pp: Warn if module name different than output filename

[James Carter]

Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is not the same as its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.

Because of this, provide a warning message when converting a policy
package to CIL and the output filename is different than the module
name.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 policycoreutils/hll/pp/pp.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
index 866734f..8621b50 100644
--- a/policycoreutils/hll/pp/pp.c
+++ b/policycoreutils/hll/pp/pp.c
@@ -28,6 +28,7 @@
 
 #include <sepol/module.h>
 #include <sepol/module_to_cil.h>
+#include <sepol/policydb/module.h>
 
 char *progname;
 
@@ -68,6 +69,8 @@ int main(int argc, char **argv)
 		{ NULL, 0, NULL, 0 }
 	};
 	struct sepol_module_package *mod_pkg = NULL;
+	char *ifile = NULL;
+	char *ofile = NULL;
 	FILE *in = NULL;
 	FILE *out = NULL;
 	int outfd = -1;
@@ -89,20 +92,23 @@ int main(int argc, char **argv)
 	}
 
 	if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
-		in = fopen(argv[1], "rb");
+		ifile = argv[1];
+		in = fopen(ifile, "rb");
 		if (in == NULL) {
-			log_err("Failed to open %s: %s", argv[1], strerror(errno));
+			log_err("Failed to open %s: %s", ifile, strerror(errno));
 			rc = -1;
 			goto exit;
 		}
 	} else {
+		ifile = "stdin";
 		in = stdin;
 	}
 
 	if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
-		out = fopen(argv[2], "w");
+		ofile = argv[2];
+		out = fopen(ofile, "w");
 		if (out == NULL) {
-			log_err("Failed to open %s: %s", argv[2], strerror(errno));
+			log_err("Failed to open %s: %s", ofile, strerror(errno));
 			rc = -1;
 			goto exit;
 		}
@@ -122,6 +128,20 @@ int main(int argc, char **argv)
 	fclose(in);
 	in = NULL;
 
+	if (ofile) {
+		char *mod_name = mod_pkg->policy->p.name;
+		char *cil_path = strdup(ofile);
+		char *cil_name = basename(cil_path);
+		char *separator = strrchr(cil_name, '.');
+		if (separator) {
+			*separator = '\0';
+		}
+		if (strcmp(mod_name, cil_name) != 0) {
+			fprintf(stderr,	"Warning: SELinux userspace will refer to the module from %s as %s rather than %s\n", ifile, cil_name, mod_name);
+		}
+		free(cil_path);
+	}
+
 	rc = sepol_module_package_to_cil(out, mod_pkg);
 	if (rc != 0) {
 		goto exit;
-- 
2.5.5

[PATCH 2/2 v2] checkpolicy: Warn if module name different than output filename

[James Carter]

Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is different than its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.

Because of this, provide a warning message when compiling a module and
the output filename is different than the module name.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 checkpolicy/checkmodule.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 5957d29..d807620 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -19,6 +19,7 @@
 #include <stdio.h>
 #include <errno.h>
 #include <sys/mman.h>
+#include <libgen.h>
 
 #include <sepol/module_to_cil.h>
 #include <sepol/policydb/policydb.h>
@@ -258,6 +259,20 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (policy_type != POLICY_BASE && outfile) {
+		char *mod_name = modpolicydb.name;
+		char *out_path = strdup(outfile);
+		char *out_name = basename(out_path);
+		char *separator = strrchr(out_name, '.');
+		if (separator) {
+			*separator = '\0';
+		}
+		if (strcmp(mod_name, out_name) != 0) {
+			fprintf(stderr,	"Warning: SELinux userspace will refer to the module from %s as %s rather than as %s\n", file, out_name, mod_name);
+		}
+		free(out_path);
+	}
+
 	if (modpolicydb.policy_type == POLICY_BASE && !cil) {
 		/* Verify that we can successfully expand the base module. */
 		policydb_t kernpolicydb;
-- 
2.5.5

Re: [PATCH 2/2 v2] checkpolicy: Warn if module name different than output filename

[Daniel J Walsh]

On 04/07/2016 11:06 AM, James Carter wrote:
> Since CIL treats files as modules and does not have a separate
> module statement it can cause confusion when a Refpolicy module
> has a name that is different than its base filename because older
> SELinux userspaces will refer to the module by its module name while
> a CIL-based userspace will refer to it by its filename.
>
> Because of this, provide a warning message when compiling a module and
> the output filename is different than the module name.
>
> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
> ---
>   checkpolicy/checkmodule.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
> index 5957d29..d807620 100644
> --- a/checkpolicy/checkmodule.c
> +++ b/checkpolicy/checkmodule.c
> @@ -19,6 +19,7 @@
>   #include <stdio.h>
>   #include <errno.h>
>   #include <sys/mman.h>
> +#include <libgen.h>
>   
>   #include <sepol/module_to_cil.h>
>   #include <sepol/policydb/policydb.h>
> @@ -258,6 +259,20 @@ int main(int argc, char **argv)
>   		}
>   	}
>   
> +	if (policy_type != POLICY_BASE && outfile) {
> +		char *mod_name = modpolicydb.name;
> +		char *out_path = strdup(outfile);
> +		char *out_name = basename(out_path);
> +		char *separator = strrchr(out_name, '.');
> +		if (separator) {
> +			*separator = '\0';
> +		}
> +		if (strcmp(mod_name, out_name) != 0) {
> +			fprintf(stderr,	"Warning: SELinux userspace will refer to the module from %s as %s rather than as %s\n", file, out_name, mod_name);
> +		}
> +		free(out_path);
> +	}
> +
>   	if (modpolicydb.policy_type == POLICY_BASE && !cil) {
>   		/* Verify that we can successfully expand the base module. */
>   		policydb_t kernpolicydb;
Why not fail rather then warn.  Don't let me do stupid things...

Re: [PATCH 1/2 v2] policycoreutils/hll/pp: Warn if module name different than output filename

[Thomas Hurd]

On Thu, Apr 7, 2016 at 11:06 AM, James Carter <jwcart2@tycho.nsa.gov> wrote:
>
> Since CIL treats files as modules and does not have a separate
> module statement it can cause confusion when a Refpolicy module
> has a name that is not the same as its base filename because older
> SELinux userspaces will refer to the module by its module name while
> a CIL-based userspace will refer to it by its filename.
>
> Because of this, provide a warning message when converting a policy
> package to CIL and the output filename is different than the module
> name.
>
> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
> ---
>  policycoreutils/hll/pp/pp.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
> index 866734f..8621b50 100644
> --- a/policycoreutils/hll/pp/pp.c
> +++ b/policycoreutils/hll/pp/pp.c
> @@ -28,6 +28,7 @@
>
>  #include <sepol/module.h>
>  #include <sepol/module_to_cil.h>
> +#include <sepol/policydb/module.h>
>
>  char *progname;
>
> @@ -68,6 +69,8 @@ int main(int argc, char **argv)
>                 { NULL, 0, NULL, 0 }
>         };
>         struct sepol_module_package *mod_pkg = NULL;
> +       char *ifile = NULL;
> +       char *ofile = NULL;
>         FILE *in = NULL;
>         FILE *out = NULL;
>         int outfd = -1;
> @@ -89,20 +92,23 @@ int main(int argc, char **argv)
>         }
>
>         if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
> -               in = fopen(argv[1], "rb");
> +               ifile = argv[1];
> +               in = fopen(ifile, "rb");
>                 if (in == NULL) {
> -                       log_err("Failed to open %s: %s", argv[1], strerror(errno));
> +                       log_err("Failed to open %s: %s", ifile, strerror(errno));
>                         rc = -1;
>                         goto exit;
>                 }
>         } else {
> +               ifile = "stdin";
>                 in = stdin;
>         }
>
>         if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
> -               out = fopen(argv[2], "w");
> +               ofile = argv[2];
> +               out = fopen(ofile, "w");
>                 if (out == NULL) {
> -                       log_err("Failed to open %s: %s", argv[2], strerror(errno));
> +                       log_err("Failed to open %s: %s", ofile, strerror(errno));
>                         rc = -1;
>                         goto exit;
>                 }
> @@ -122,6 +128,20 @@ int main(int argc, char **argv)
>         fclose(in);
>         in = NULL;
>
> +       if (ofile) {
> +               char *mod_name = mod_pkg->policy->p.name;
> +               char *cil_path = strdup(ofile);

Check if strdup fails here and also in the checkmodule patch?

> +               char *cil_name = basename(cil_path);
> +               char *separator = strrchr(cil_name, '.');
> +               if (separator) {
> +                       *separator = '\0';
> +               }
> +               if (strcmp(mod_name, cil_name) != 0) {
> +                       fprintf(stderr, "Warning: SELinux userspace will refer to the module from %s as %s rather than %s\n", ifile, cil_name, mod_name);
> +               }
> +               free(cil_path);
> +       }
> +
>         rc = sepol_module_package_to_cil(out, mod_pkg);
>         if (rc != 0) {
>                 goto exit;
> --
> 2.5.5
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

Re: [PATCH 1/2 v2] policycoreutils/hll/pp: Warn if module name different than output filename

[James Carter]

On 04/07/2016 12:41 PM, Thomas Hurd wrote:
> On Thu, Apr 7, 2016 at 11:06 AM, James Carter <jwcart2@tycho.nsa.gov> wrote:
>>
>> Since CIL treats files as modules and does not have a separate
>> module statement it can cause confusion when a Refpolicy module
>> has a name that is not the same as its base filename because older
>> SELinux userspaces will refer to the module by its module name while
>> a CIL-based userspace will refer to it by its filename.
>>
>> Because of this, provide a warning message when converting a policy
>> package to CIL and the output filename is different than the module
>> name.
>>
>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>> ---
>>   policycoreutils/hll/pp/pp.c | 28 ++++++++++++++++++++++++----
>>   1 file changed, 24 insertions(+), 4 deletions(-)
>>
>> diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
>> index 866734f..8621b50 100644
>> --- a/policycoreutils/hll/pp/pp.c
>> +++ b/policycoreutils/hll/pp/pp.c
>> @@ -28,6 +28,7 @@
>>
>>   #include <sepol/module.h>
>>   #include <sepol/module_to_cil.h>
>> +#include <sepol/policydb/module.h>
>>
>>   char *progname;
>>
>> @@ -68,6 +69,8 @@ int main(int argc, char **argv)
>>                  { NULL, 0, NULL, 0 }
>>          };
>>          struct sepol_module_package *mod_pkg = NULL;
>> +       char *ifile = NULL;
>> +       char *ofile = NULL;
>>          FILE *in = NULL;
>>          FILE *out = NULL;
>>          int outfd = -1;
>> @@ -89,20 +92,23 @@ int main(int argc, char **argv)
>>          }
>>
>>          if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
>> -               in = fopen(argv[1], "rb");
>> +               ifile = argv[1];
>> +               in = fopen(ifile, "rb");
>>                  if (in == NULL) {
>> -                       log_err("Failed to open %s: %s", argv[1], strerror(errno));
>> +                       log_err("Failed to open %s: %s", ifile, strerror(errno));
>>                          rc = -1;
>>                          goto exit;
>>                  }
>>          } else {
>> +               ifile = "stdin";
>>                  in = stdin;
>>          }
>>
>>          if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
>> -               out = fopen(argv[2], "w");
>> +               ofile = argv[2];
>> +               out = fopen(ofile, "w");
>>                  if (out == NULL) {
>> -                       log_err("Failed to open %s: %s", argv[2], strerror(errno));
>> +                       log_err("Failed to open %s: %s", ofile, strerror(errno));
>>                          rc = -1;
>>                          goto exit;
>>                  }
>> @@ -122,6 +128,20 @@ int main(int argc, char **argv)
>>          fclose(in);
>>          in = NULL;
>>
>> +       if (ofile) {
>> +               char *mod_name = mod_pkg->policy->p.name;
>> +               char *cil_path = strdup(ofile);
>
> Check if strdup fails here and also in the checkmodule patch?

Yes, I do need to do that. Thanks.

Jim

>
>> +               char *cil_name = basename(cil_path);
>> +               char *separator = strrchr(cil_name, '.');
>> +               if (separator) {
>> +                       *separator = '\0';
>> +               }
>> +               if (strcmp(mod_name, cil_name) != 0) {
>> +                       fprintf(stderr, "Warning: SELinux userspace will refer to the module from %s as %s rather than %s\n", ifile, cil_name, mod_name);
>> +               }
>> +               free(cil_path);
>> +       }
>> +
>>          rc = sepol_module_package_to_cil(out, mod_pkg);
>>          if (rc != 0) {
>>                  goto exit;
>> --
>> 2.5.5
>>
>> _______________________________________________
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.


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

Re: [PATCH 2/2 v2] checkpolicy: Warn if module name different than output filename

[James Carter]

On 04/07/2016 11:28 AM, Daniel J Walsh wrote:
>
>
> On 04/07/2016 11:06 AM, James Carter wrote:
>> Since CIL treats files as modules and does not have a separate
>> module statement it can cause confusion when a Refpolicy module
>> has a name that is different than its base filename because older
>> SELinux userspaces will refer to the module by its module name while
>> a CIL-based userspace will refer to it by its filename.
>>
>> Because of this, provide a warning message when compiling a module and
>> the output filename is different than the module name.
>>
>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>> ---
>>   checkpolicy/checkmodule.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
>> index 5957d29..d807620 100644
>> --- a/checkpolicy/checkmodule.c
>> +++ b/checkpolicy/checkmodule.c
>> @@ -19,6 +19,7 @@
>>   #include <stdio.h>
>>   #include <errno.h>
>>   #include <sys/mman.h>
>> +#include <libgen.h>
>>   #include <sepol/module_to_cil.h>
>>   #include <sepol/policydb/policydb.h>
>> @@ -258,6 +259,20 @@ int main(int argc, char **argv)
>>           }
>>       }
>> +    if (policy_type != POLICY_BASE && outfile) {
>> +        char *mod_name = modpolicydb.name;
>> +        char *out_path = strdup(outfile);
>> +        char *out_name = basename(out_path);
>> +        char *separator = strrchr(out_name, '.');
>> +        if (separator) {
>> +            *separator = '\0';
>> +        }
>> +        if (strcmp(mod_name, out_name) != 0) {
>> +            fprintf(stderr,    "Warning: SELinux userspace will refer to the
>> module from %s as %s rather than as %s\n", file, out_name, mod_name);
>> +        }
>> +        free(out_path);
>> +    }
>> +
>>       if (modpolicydb.policy_type == POLICY_BASE && !cil) {
>>           /* Verify that we can successfully expand the base module. */
>>           policydb_t kernpolicydb;
> Why not fail rather then warn.  Don't let me do stupid things...

I am willing to do that for checkmodule if that is what everyone wants. I 
wouldn't want to do it for pp since that could cause problems for current 
systems. Just as a note, Fedora's passenger module has "passanger" as the policy 
name in its policy_module statement.

Jim


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

Re: [PATCH 2/2 v2] checkpolicy: Warn if module name different than output filename

[Daniel J Walsh]

On 04/07/2016 03:34 PM, James Carter wrote:
> On 04/07/2016 11:28 AM, Daniel J Walsh wrote:
>>
>>
>> On 04/07/2016 11:06 AM, James Carter wrote:
>>> Since CIL treats files as modules and does not have a separate
>>> module statement it can cause confusion when a Refpolicy module
>>> has a name that is different than its base filename because older
>>> SELinux userspaces will refer to the module by its module name while
>>> a CIL-based userspace will refer to it by its filename.
>>>
>>> Because of this, provide a warning message when compiling a module and
>>> the output filename is different than the module name.
>>>
>>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>>> ---
>>>   checkpolicy/checkmodule.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
>>> index 5957d29..d807620 100644
>>> --- a/checkpolicy/checkmodule.c
>>> +++ b/checkpolicy/checkmodule.c
>>> @@ -19,6 +19,7 @@
>>>   #include <stdio.h>
>>>   #include <errno.h>
>>>   #include <sys/mman.h>
>>> +#include <libgen.h>
>>>   #include <sepol/module_to_cil.h>
>>>   #include <sepol/policydb/policydb.h>
>>> @@ -258,6 +259,20 @@ int main(int argc, char **argv)
>>>           }
>>>       }
>>> +    if (policy_type != POLICY_BASE && outfile) {
>>> +        char *mod_name = modpolicydb.name;
>>> +        char *out_path = strdup(outfile);
>>> +        char *out_name = basename(out_path);
>>> +        char *separator = strrchr(out_name, '.');
>>> +        if (separator) {
>>> +            *separator = '\0';
>>> +        }
>>> +        if (strcmp(mod_name, out_name) != 0) {
>>> +            fprintf(stderr,    "Warning: SELinux userspace will 
>>> refer to the
>>> module from %s as %s rather than as %s\n", file, out_name, mod_name);
>>> +        }
>>> +        free(out_path);
>>> +    }
>>> +
>>>       if (modpolicydb.policy_type == POLICY_BASE && !cil) {
>>>           /* Verify that we can successfully expand the base module. */
>>>           policydb_t kernpolicydb;
>> Why not fail rather then warn.  Don't let me do stupid things...
>
> I am willing to do that for checkmodule if that is what everyone 
> wants. I wouldn't want to do it for pp since that could cause problems 
> for current systems. Just as a note, Fedora's passenger module has 
> "passanger" as the policy name in its policy_module statement.
>
> Jim
>
>
And if it blew up someone would have fixed it.  :^)

Re: [PATCH 0/2 v2] Warn if module name different than output filename

[Petr Lautrbach]

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

On 04/07/2016 05:06 PM, James Carter wrote:
> Since CIL treats files as modules and does not have a separate
> module statement it can cause confusion when a Refpolicy module
> has a name that is not the same as its base filename because older
> SELinux userspaces will refer to the module by its module name while
> a CIL-based userspace will refer to it by its filename.
> 
> Because of this, provide a warning message when converting a policy
> package to CIL or compiling a module and the output filename is different
> than the module name.

Would it be feasible to use a module statement name as a new base name
for CIL module instead of the original base pp filename, and warn users
that the new output filename is different from the original?

It would help to have same installed module names in 'semodule -l' list
in older and current userspace.


> 
> Changes from v1:
> - Added a "Warning:" prefix
> - Removed checks against the input filename
> - Since there are now only two checks and the base filename is used in the
> warning message, it no longer made sense to create common helper functions
> in libsepol.
> 
> James Carter (2):
>   policycoreutils/hll/pp: Warn if module name different than output
>     filename
>   checkpolicy: Warn if module name different than output filename
> 
>  checkpolicy/checkmodule.c   | 15 +++++++++++++++
>  policycoreutils/hll/pp/pp.c | 28 ++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)
> 


-- 
Petr Lautrbach



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]