[RFC PATCH 0/2] userspace: Allow changing version of kernel policy built by semodule

[RFC PATCH 0/2] userspace: Allow changing version of kernel policy built by semodule

[Ondrej Mosnacek]

See the second patch for motivation.

Passed Travis: https://travis-ci.org/WOnder93/selinux/builds/646845127

Ondrej Mosnacek (2):
  libsemanage: support changing policy version via API
  semodule: support changing policyvers via command line

 libsemanage/include/semanage/handle.h |  6 +++
 libsemanage/src/direct_api.c          |  9 ++++-
 libsemanage/src/handle.c              | 24 ++++++++++++
 libsemanage/src/handle.h              |  1 +
 libsemanage/src/libsemanage.map       |  6 +++
 libsemanage/src/semanage_store.c      | 54 ++++++++++++++++-----------
 libsemanage/src/semanage_store.h      |  6 ++-
 policycoreutils/semodule/semodule.8   |  3 ++
 policycoreutils/semodule/semodule.c   | 12 +++++-
 9 files changed, 95 insertions(+), 26 deletions(-)

-- 
2.24.1

[RFC PATCH 1/2] libsemanage: support changing policy version via API

[Ondrej Mosnacek]

This change will be needed to support explicly specifying the policy
version in semodule (in a subsequent patch).

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libsemanage/include/semanage/handle.h |  6 +++
 libsemanage/src/direct_api.c          |  9 ++++-
 libsemanage/src/handle.c              | 24 ++++++++++++
 libsemanage/src/handle.h              |  1 +
 libsemanage/src/libsemanage.map       |  6 +++
 libsemanage/src/semanage_store.c      | 54 ++++++++++++++++-----------
 libsemanage/src/semanage_store.h      |  6 ++-
 7 files changed, 81 insertions(+), 25 deletions(-)

diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h
index 946d69bc..70b37863 100644
--- a/libsemanage/include/semanage/handle.h
+++ b/libsemanage/include/semanage/handle.h
@@ -85,6 +85,12 @@ extern void semanage_set_disable_dontaudit(semanage_handle_t * handle, int disab
 /* Set whether or not to execute setfiles to check file contexts upon commit */
 extern void semanage_set_check_contexts(semanage_handle_t * sh, int do_check_contexts);
 
+/* Get the kernel policy version. */
+extern unsigned semanage_get_policyvers(semanage_handle_t *sh);
+
+/* Set the kernel policy version. */
+extern int semanage_set_policyvers(semanage_handle_t *sh, unsigned policyvers);
+
 /* Get the default priority. */
 extern uint16_t semanage_get_default_priority(semanage_handle_t *sh);
 
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 1088a0ac..78c40018 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -1204,6 +1204,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	size_t fc_buffer_len = 0;
 	const char *ofilename = NULL;
 	const char *path;
+	char kernel_path[PATH_MAX];
 	int retval = -1, num_modinfos = 0, i;
 	sepol_policydb_t *out = NULL;
 	struct cil_db *cildb = NULL;
@@ -1593,9 +1594,13 @@ rebuild:
 	if (retval < 0)
 		goto cleanup;
 
+	if (semanage_get_full_kernel_path(sh, SEMANAGE_FINAL_TMP, kernel_path)) {
+		ERR(sh, "Unable to build path to kernel policy.");
+		goto cleanup;
+	}
+
 	retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL),
-			semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_KERNEL),
-			sh->conf->file_mode);
+			kernel_path, sh->conf->file_mode);
 	if (retval < 0) {
 		goto cleanup;
 	}
diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c
index 5e59aef7..78818930 100644
--- a/libsemanage/src/handle.c
+++ b/libsemanage/src/handle.c
@@ -81,6 +81,9 @@ semanage_handle_t *semanage_handle_create(void)
 		goto err;
 	sepol_msg_set_callback(sh->sepolh, semanage_msg_relay_handler, sh);
 
+	/* Default policy version is taken from config */
+	sh->policyvers = sh->conf->policyvers;
+
 	/* Default priority is 400 */
 	sh->priority = 400;
 
@@ -246,6 +249,27 @@ void semanage_set_check_contexts(semanage_handle_t * sh, int do_check_contexts)
 	return;
 }
 
+unsigned semanage_get_policyvers(semanage_handle_t *sh)
+{
+	assert(sh != NULL);
+	return sh->policyvers;
+}
+
+int semanage_set_policyvers(semanage_handle_t *sh, unsigned policyvers)
+{
+	assert(sh != NULL);
+
+	/* Verify policy version */
+	if (   policyvers < POLICYDB_VERSION_MIN
+	    || policyvers > POLICYDB_VERSION_MAX) {
+		ERR(sh, "Policy version %u is invalid.", policyvers);
+		return -1;
+	}
+
+	sh->policyvers = policyvers;
+	return 0;
+}
+
 uint16_t semanage_get_default_priority(semanage_handle_t *sh)
 {
 	assert(sh != NULL);
diff --git a/libsemanage/src/handle.h b/libsemanage/src/handle.h
index a91907b0..ee389226 100644
--- a/libsemanage/src/handle.h
+++ b/libsemanage/src/handle.h
@@ -57,6 +57,7 @@ struct semanage_handle {
 
 	semanage_conf_t *conf;
 
+	unsigned policyvers;
 	uint16_t priority;
 	int is_connected;
 	int is_in_transaction;
diff --git a/libsemanage/src/libsemanage.map b/libsemanage/src/libsemanage.map
index 02036696..8c05b9ad 100644
--- a/libsemanage/src/libsemanage.map
+++ b/libsemanage/src/libsemanage.map
@@ -63,3 +63,9 @@ LIBSEMANAGE_1.1 {
 	  semanage_module_remove_key;
 	  semanage_set_store_root;
 } LIBSEMANAGE_1.0;
+
+LIBSEMANAGE_1.2 {
+  global:
+	  semanage_get_policyvers;
+	  semanage_set_policyvers;
+} LIBSEMANAGE_1.1;
diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 58dded6e..52217be7 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -277,9 +277,7 @@ cleanup:
 
 static int semanage_init_final_suffix(semanage_handle_t *sh)
 {
-	int ret = 0;
 	int status = 0;
-	char path[PATH_MAX];
 	size_t offset = strlen(selinux_policy_root());
 
 	semanage_final_suffix[SEMANAGE_FINAL_TOPLEVEL] = strdup("");
@@ -350,19 +348,9 @@ static int semanage_init_final_suffix(semanage_handle_t *sh)
 		goto cleanup;
 	}
 
-	ret = snprintf(path,
-		       sizeof(path),
-		       "%s.%d",
-		       selinux_binary_policy_path() + offset,
-		       sh->conf->policyvers);
-	if (ret < 0 || ret >= (int)sizeof(path)) {
-		ERR(sh, "Unable to compose policy binary path.");
-		status = -1;
-		goto cleanup;
-	}
-
-	semanage_final_suffix[SEMANAGE_KERNEL] = strdup(path);
-	if (semanage_final_suffix[SEMANAGE_KERNEL] == NULL) {
+	semanage_final_suffix[SEMANAGE_KERNEL_PREFIX] =
+		strdup(selinux_binary_policy_path() + offset);
+	if (semanage_final_suffix[SEMANAGE_KERNEL_PREFIX] == NULL) {
 		ERR(sh, "Unable to allocate space for policy binary path.");
 		status = -1;
 		goto cleanup;
@@ -503,6 +491,20 @@ const char *semanage_final_path(enum semanage_final_defs store,
 	return semanage_final_paths[store][path_name];
 }
 
+/* Return a fully-qualified path + filename to kernel policy for the given
+ * semanage store.
+ */
+int semanage_get_full_kernel_path(semanage_handle_t * sh,
+				  enum semanage_final_defs root,
+				  char out[PATH_MAX])
+{
+	int ret = snprintf(out, PATH_MAX, "%s.%u",
+			   semanage_final_path(root, SEMANAGE_KERNEL_PREFIX),
+			   sh->policyvers);
+
+	return ret < 0 || ret >= PATH_MAX ? -1 : 0;
+}
+
 /* Return a fully-qualified path + filename to the semanage
  * configuration file. If semanage.conf file in the semanage
  * root is cannot be read, use the default semanage.conf as a
@@ -1568,12 +1570,16 @@ static int semanage_validate_and_compile_fcontexts(semanage_handle_t * sh)
 	int status = -1;
 
 	if (sh->do_check_contexts) {
+		char path[PATH_MAX];
 		int ret;
+
+		if (semanage_get_full_kernel_path(sh, SEMANAGE_FINAL_TMP, path)) {
+			ERR(sh, "Unable to build path to kernel policy.");
+			goto cleanup;
+		}
+
 		ret = semanage_exec_prog(
-			sh,
-			sh->conf->setfiles,
-			semanage_final_path(SEMANAGE_FINAL_TMP,
-					    SEMANAGE_KERNEL),
+			sh, sh->conf->setfiles, path,
 			semanage_final_path(SEMANAGE_FINAL_TMP,
 					    SEMANAGE_FC));
 		if (ret != 0) {
@@ -2233,15 +2239,19 @@ int semanage_verify_linked(semanage_handle_t * sh)
 int semanage_verify_kernel(semanage_handle_t * sh)
 {
 	int retval = -1;
-	const char *kernel_filename =
-	    semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_KERNEL);
+	char path[PATH_MAX];
 	semanage_conf_t *conf = sh->conf;
 	external_prog_t *e;
+
 	if (conf->kernel_prog == NULL) {
 		return 0;
 	}
+	if (semanage_get_full_kernel_path(sh, SEMANAGE_FINAL_TMP, path)) {
+		ERR(sh, "Unable to build path to kernel policy.");
+		goto cleanup;
+	}
 	for (e = conf->kernel_prog; e != NULL; e = e->next) {
-		if (semanage_exec_prog(sh, e, kernel_filename, "$<") != 0) {
+		if (semanage_exec_prog(sh, e, path, "$<") != 0) {
 			goto cleanup;
 		}
 	}
diff --git a/libsemanage/src/semanage_store.h b/libsemanage/src/semanage_store.h
index 34bf8523..d5567782 100644
--- a/libsemanage/src/semanage_store.h
+++ b/libsemanage/src/semanage_store.h
@@ -81,7 +81,7 @@ enum semanage_final_path_defs {
 	SEMANAGE_FC_HOMEDIRS_BIN,
 	SEMANAGE_FC_LOCAL,
 	SEMANAGE_FC_LOCAL_BIN,
-	SEMANAGE_KERNEL,
+	SEMANAGE_KERNEL_PREFIX,
 	SEMANAGE_NC,
 	SEMANAGE_SEUSERS,
 	SEMANAGE_FINAL_PATH_NUM
@@ -102,6 +102,10 @@ extern const char *semanage_path(enum semanage_store_defs store,
 extern const char *semanage_final_path(enum semanage_final_defs root,
 				       enum semanage_final_path_defs suffix);
 
+int semanage_get_full_kernel_path(semanage_handle_t * sh,
+				  enum semanage_final_defs root,
+				  char out[PATH_MAX]);
+
 int semanage_create_store(semanage_handle_t * sh, int create);
 
 int semanage_store_access_check(void);
-- 
2.24.1

[RFC PATCH 2/2] semodule: support changing policyvers via command line

[Ondrej Mosnacek]

When using semodule for building a distribution policy package (as
Fedora does), the environment might not have selinuxfs available and
provide no way to modify semanage.conf. When we want to build a policy
with version X (because our kernel doesn't support X+1 and above yet),
but our libsepol already has support for X+1, then we currently have no
way to do so.

To resolve this, add a new command-line argument to semodule, which
allows to override the system-wide configured version to a different
one.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 policycoreutils/semodule/semodule.8 |  3 +++
 policycoreutils/semodule/semodule.c | 12 +++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/policycoreutils/semodule/semodule.8 b/policycoreutils/semodule/semodule.8
index 18d4f708..88e027fd 100644
--- a/policycoreutils/semodule/semodule.8
+++ b/policycoreutils/semodule/semodule.8
@@ -64,6 +64,9 @@ A module is extracted as HLL by default. The name of the module written is
 <module-name>.<lang_ext>
 .SH "OPTIONS"
 .TP
+.B  \-V,\-\-policyvers
+force specific kernel policy version
+.TP
 .B  \-s,\-\-store
 name of the store to operate on
 .TP
diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
index a1f75e16..30c4495b 100644
--- a/policycoreutils/semodule/semodule.c
+++ b/policycoreutils/semodule/semodule.c
@@ -50,6 +50,8 @@ static int build;
 static int disable_dontaudit;
 static int preserve_tunables;
 static int ignore_module_cache;
+static unsigned policyvers;
+static int policyvers_set = 0;
 static uint16_t priority;
 static int priority_set = 0;
 
@@ -137,6 +139,7 @@ static void usage(char *progname)
 	printf("  -d,--disable=MODULE_NAME  disable module\n");
 	printf("  -E,--extract=MODULE_NAME  extract module\n");
 	printf("Options:\n");
+	printf("  -V,--policyvers  force specific kernel policy version\n");
 	printf("  -s,--store	   name of the store to operate on\n");
 	printf("  -N,-n,--noreload do not reload policy after commit\n");
 	printf("  -h,--help        print this message and quit\n");
@@ -210,7 +213,7 @@ static void parse_command_line(int argc, char **argv)
 	no_reload = 0;
 	priority = 400;
 	while ((i =
-		getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cH", opts,
+		getopt_long(argc, argv, "V:s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cH", opts,
 			    NULL)) != -1) {
 		switch (i) {
 		case 'b':
@@ -248,6 +251,10 @@ static void parse_command_line(int argc, char **argv)
 			fprintf(stderr, "The --upgrade option is deprecated. Use --install instead.\n");
 			set_mode(INSTALL_M, optarg);
 			break;
+		case 'V':
+			policyvers = (unsigned)strtoul(optarg, NULL, 10);
+			policyvers_set = 1;
+			break;
 		case 's':
 			set_store(optarg);
 			break;
@@ -363,6 +370,9 @@ int main(int argc, char *argv[])
 		goto cleanup_nohandle;
 	}
 
+	if (policyvers_set)
+		semanage_set_policyvers(sh, policyvers);
+
 	if (store) {
 		/* Set the store we want to connect to, before connecting.
 		 * this will always set a direct connection now, an additional
-- 
2.24.1

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Stephen Smalley]

On 2/6/20 8:12 AM, Ondrej Mosnacek wrote:
> When using semodule for building a distribution policy package (as
> Fedora does), the environment might not have selinuxfs available and
> provide no way to modify semanage.conf. When we want to build a policy
> with version X (because our kernel doesn't support X+1 and above yet),
> but our libsepol already has support for X+1, then we currently have no
> way to do so.

Not fundamentally opposed, but unclear on the motivation.  The current 
approach is to generate the highest policy version supported by our 
libsepol at build time, then libselinux selinux_mkload_policy() uses 
libsepol functions (sepol_policydb_set_vers(), 
sepol_policydb_to_image()) to automatically downgrade the policy in 
memory to whatever version is supported by the kernel before writing it 
to the kernel.  This works as long as one uses the same or newer 
libsepol at load time as at build time and as long as policydb_write() 
supports writing older policy versions (generally discarding newer 
features).

> 
> To resolve this, add a new command-line argument to semodule, which
> allows to override the system-wide configured version to a different
> one.
> 
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>   policycoreutils/semodule/semodule.8 |  3 +++
>   policycoreutils/semodule/semodule.c | 12 +++++++++++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/policycoreutils/semodule/semodule.8 b/policycoreutils/semodule/semodule.8
> index 18d4f708..88e027fd 100644
> --- a/policycoreutils/semodule/semodule.8
> +++ b/policycoreutils/semodule/semodule.8
> @@ -64,6 +64,9 @@ A module is extracted as HLL by default. The name of the module written is
>   <module-name>.<lang_ext>
>   .SH "OPTIONS"
>   .TP
> +.B  \-V,\-\-policyvers
> +force specific kernel policy version
> +.TP
>   .B  \-s,\-\-store
>   name of the store to operate on
>   .TP
> diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> index a1f75e16..30c4495b 100644
> --- a/policycoreutils/semodule/semodule.c
> +++ b/policycoreutils/semodule/semodule.c
> @@ -50,6 +50,8 @@ static int build;
>   static int disable_dontaudit;
>   static int preserve_tunables;
>   static int ignore_module_cache;
> +static unsigned policyvers;
> +static int policyvers_set = 0;
>   static uint16_t priority;
>   static int priority_set = 0;
>   
> @@ -137,6 +139,7 @@ static void usage(char *progname)
>   	printf("  -d,--disable=MODULE_NAME  disable module\n");
>   	printf("  -E,--extract=MODULE_NAME  extract module\n");
>   	printf("Options:\n");
> +	printf("  -V,--policyvers  force specific kernel policy version\n");
>   	printf("  -s,--store	   name of the store to operate on\n");
>   	printf("  -N,-n,--noreload do not reload policy after commit\n");
>   	printf("  -h,--help        print this message and quit\n");
> @@ -210,7 +213,7 @@ static void parse_command_line(int argc, char **argv)
>   	no_reload = 0;
>   	priority = 400;
>   	while ((i =
> -		getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cH", opts,
> +		getopt_long(argc, argv, "V:s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cH", opts,
>   			    NULL)) != -1) {
>   		switch (i) {
>   		case 'b':
> @@ -248,6 +251,10 @@ static void parse_command_line(int argc, char **argv)
>   			fprintf(stderr, "The --upgrade option is deprecated. Use --install instead.\n");
>   			set_mode(INSTALL_M, optarg);
>   			break;
> +		case 'V':
> +			policyvers = (unsigned)strtoul(optarg, NULL, 10);
> +			policyvers_set = 1;
> +			break;
>   		case 's':
>   			set_store(optarg);
>   			break;
> @@ -363,6 +370,9 @@ int main(int argc, char *argv[])
>   		goto cleanup_nohandle;
>   	}
>   
> +	if (policyvers_set)
> +		semanage_set_policyvers(sh, policyvers);
> +
>   	if (store) {
>   		/* Set the store we want to connect to, before connecting.
>   		 * this will always set a direct connection now, an additional
> 

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Ondrej Mosnacek]

On Thu, Feb 6, 2020 at 2:44 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 2/6/20 8:12 AM, Ondrej Mosnacek wrote:
> > When using semodule for building a distribution policy package (as
> > Fedora does), the environment might not have selinuxfs available and
> > provide no way to modify semanage.conf. When we want to build a policy
> > with version X (because our kernel doesn't support X+1 and above yet),
> > but our libsepol already has support for X+1, then we currently have no
> > way to do so.
>
> Not fundamentally opposed, but unclear on the motivation.  The current
> approach is to generate the highest policy version supported by our
> libsepol at build time, then libselinux selinux_mkload_policy() uses
> libsepol functions (sepol_policydb_set_vers(),
> sepol_policydb_to_image()) to automatically downgrade the policy in
> memory to whatever version is supported by the kernel before writing it
> to the kernel.  This works as long as one uses the same or newer
> libsepol at load time as at build time and as long as policydb_write()
> supports writing older policy versions (generally discarding newer
> features).

The problem is that:
1. selinux-policy expects that the generated /etc/selinux/.../policy.X
file will be generated with a specific (hard-coded) value X, so if the
userspace is updated in buildroot, the selinux-policy build fails.
2. If we fix the above by expecting any value X and ship that, then
the build passes in such case, but if a user updates selinux-policy
without updating userspace and reboots, the system will not boot. So
even if we stop incrementing the expected policy version manually, we
would still need to manually increment the minimum required userspace
version each time the policy is rebuilt with userspace that has
incremented its max policyvers.

With these patches we can call semodule -V %{POLICYVER} ... and new
rebuilds of selinux-policy wouldn't be disrupted by userspace
upgrades. The only downside is that we would need to remember to
increment the specfile versions from time to time.

OTOH, maybe the build failure is actually a good thing in that it
serves as a reminder to bump all the hard-coded versions whenever
userspace bumps max policyvers...

>
> >
> > To resolve this, add a new command-line argument to semodule, which
> > allows to override the system-wide configured version to a different
> > one.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >   policycoreutils/semodule/semodule.8 |  3 +++
> >   policycoreutils/semodule/semodule.c | 12 +++++++++++-
> >   2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/policycoreutils/semodule/semodule.8 b/policycoreutils/semodule/semodule.8
> > index 18d4f708..88e027fd 100644
> > --- a/policycoreutils/semodule/semodule.8
> > +++ b/policycoreutils/semodule/semodule.8
> > @@ -64,6 +64,9 @@ A module is extracted as HLL by default. The name of the module written is
> >   <module-name>.<lang_ext>
> >   .SH "OPTIONS"
> >   .TP
> > +.B  \-V,\-\-policyvers
> > +force specific kernel policy version
> > +.TP
> >   .B  \-s,\-\-store
> >   name of the store to operate on
> >   .TP
> > diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> > index a1f75e16..30c4495b 100644
> > --- a/policycoreutils/semodule/semodule.c
> > +++ b/policycoreutils/semodule/semodule.c
> > @@ -50,6 +50,8 @@ static int build;
> >   static int disable_dontaudit;
> >   static int preserve_tunables;
> >   static int ignore_module_cache;
> > +static unsigned policyvers;
> > +static int policyvers_set = 0;
> >   static uint16_t priority;
> >   static int priority_set = 0;
> >
> > @@ -137,6 +139,7 @@ static void usage(char *progname)
> >       printf("  -d,--disable=MODULE_NAME  disable module\n");
> >       printf("  -E,--extract=MODULE_NAME  extract module\n");
> >       printf("Options:\n");
> > +     printf("  -V,--policyvers  force specific kernel policy version\n");
> >       printf("  -s,--store       name of the store to operate on\n");
> >       printf("  -N,-n,--noreload do not reload policy after commit\n");
> >       printf("  -h,--help        print this message and quit\n");
> > @@ -210,7 +213,7 @@ static void parse_command_line(int argc, char **argv)
> >       no_reload = 0;
> >       priority = 400;
> >       while ((i =
> > -             getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cH", opts,
> > +             getopt_long(argc, argv, "V:s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cH", opts,
> >                           NULL)) != -1) {
> >               switch (i) {
> >               case 'b':
> > @@ -248,6 +251,10 @@ static void parse_command_line(int argc, char **argv)
> >                       fprintf(stderr, "The --upgrade option is deprecated. Use --install instead.\n");
> >                       set_mode(INSTALL_M, optarg);
> >                       break;
> > +             case 'V':
> > +                     policyvers = (unsigned)strtoul(optarg, NULL, 10);
> > +                     policyvers_set = 1;
> > +                     break;
> >               case 's':
> >                       set_store(optarg);
> >                       break;
> > @@ -363,6 +370,9 @@ int main(int argc, char *argv[])
> >               goto cleanup_nohandle;
> >       }
> >
> > +     if (policyvers_set)
> > +             semanage_set_policyvers(sh, policyvers);
> > +
> >       if (store) {
> >               /* Set the store we want to connect to, before connecting.
> >                * this will always set a direct connection now, an additional
> >
>

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Stephen Smalley]

On 2/6/20 9:19 AM, Ondrej Mosnacek wrote:
> On Thu, Feb 6, 2020 at 2:44 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> On 2/6/20 8:12 AM, Ondrej Mosnacek wrote:
>>> When using semodule for building a distribution policy package (as
>>> Fedora does), the environment might not have selinuxfs available and
>>> provide no way to modify semanage.conf. When we want to build a policy
>>> with version X (because our kernel doesn't support X+1 and above yet),
>>> but our libsepol already has support for X+1, then we currently have no
>>> way to do so.
>>
>> Not fundamentally opposed, but unclear on the motivation.  The current
>> approach is to generate the highest policy version supported by our
>> libsepol at build time, then libselinux selinux_mkload_policy() uses
>> libsepol functions (sepol_policydb_set_vers(),
>> sepol_policydb_to_image()) to automatically downgrade the policy in
>> memory to whatever version is supported by the kernel before writing it
>> to the kernel.  This works as long as one uses the same or newer
>> libsepol at load time as at build time and as long as policydb_write()
>> supports writing older policy versions (generally discarding newer
>> features).
> 
> The problem is that:
> 1. selinux-policy expects that the generated /etc/selinux/.../policy.X
> file will be generated with a specific (hard-coded) value X, so if the
> userspace is updated in buildroot, the selinux-policy build fails.
> 2. If we fix the above by expecting any value X and ship that, then
> the build passes in such case, but if a user updates selinux-policy
> without updating userspace and reboots, the system will not boot. So
> even if we stop incrementing the expected policy version manually, we
> would still need to manually increment the minimum required userspace
> version each time the policy is rebuilt with userspace that has
> incremented its max policyvers.

Seems like you could just have selinux-policy depend on the version of 
libsepol used to build it.

The problem with both your current approach and your proposed one is 
that it means that if a user or package does a semodule -B (or any other 
semodule/semanage command) on their system, that will generate the 
latest policy.N version supported by their libsepol, and libselinux will 
give precedence to that policy at load time.  So if they then later 
update their selinux-policy package, and it only installs a prebuilt 
policy.(N-1), that won't actually get loaded - libselinux 
selinux_mkload_policy() will keep using the policy.N file (which may be 
older).  Unless I'm missing something.

> With these patches we can call semodule -V %{POLICYVER} ... and new
> rebuilds of selinux-policy wouldn't be disrupted by userspace
> upgrades. The only downside is that we would need to remember to
> increment the specfile versions from time to time.
> 
> OTOH, maybe the build failure is actually a good thing in that it
> serves as a reminder to bump all the hard-coded versions whenever
> userspace bumps max policyvers...

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Ondrej Mosnacek]

On Thu, Feb 6, 2020 at 3:52 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 2/6/20 9:19 AM, Ondrej Mosnacek wrote:
> > On Thu, Feb 6, 2020 at 2:44 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >> On 2/6/20 8:12 AM, Ondrej Mosnacek wrote:
> >>> When using semodule for building a distribution policy package (as
> >>> Fedora does), the environment might not have selinuxfs available and
> >>> provide no way to modify semanage.conf. When we want to build a policy
> >>> with version X (because our kernel doesn't support X+1 and above yet),
> >>> but our libsepol already has support for X+1, then we currently have no
> >>> way to do so.
> >>
> >> Not fundamentally opposed, but unclear on the motivation.  The current
> >> approach is to generate the highest policy version supported by our
> >> libsepol at build time, then libselinux selinux_mkload_policy() uses
> >> libsepol functions (sepol_policydb_set_vers(),
> >> sepol_policydb_to_image()) to automatically downgrade the policy in
> >> memory to whatever version is supported by the kernel before writing it
> >> to the kernel.  This works as long as one uses the same or newer
> >> libsepol at load time as at build time and as long as policydb_write()
> >> supports writing older policy versions (generally discarding newer
> >> features).
> >
> > The problem is that:
> > 1. selinux-policy expects that the generated /etc/selinux/.../policy.X
> > file will be generated with a specific (hard-coded) value X, so if the
> > userspace is updated in buildroot, the selinux-policy build fails.
> > 2. If we fix the above by expecting any value X and ship that, then
> > the build passes in such case, but if a user updates selinux-policy
> > without updating userspace and reboots, the system will not boot. So
> > even if we stop incrementing the expected policy version manually, we
> > would still need to manually increment the minimum required userspace
> > version each time the policy is rebuilt with userspace that has
> > incremented its max policyvers.
>
> Seems like you could just have selinux-policy depend on the version of
> libsepol used to build it.
>
> The problem with both your current approach and your proposed one is
> that it means that if a user or package does a semodule -B (or any other
> semodule/semanage command) on their system, that will generate the
> latest policy.N version supported by their libsepol, and libselinux will
> give precedence to that policy at load time.  So if they then later
> update their selinux-policy package, and it only installs a prebuilt
> policy.(N-1), that won't actually get loaded - libselinux
> selinux_mkload_policy() will keep using the policy.N file (which may be
> older).  Unless I'm missing something.

Hm, yes, you're right... It seems we have no other choice than to
better handle the dependency between selinux-policy and libsepol.
Please disregard this patch series.

>
> > With these patches we can call semodule -V %{POLICYVER} ... and new
> > rebuilds of selinux-policy wouldn't be disrupted by userspace
> > upgrades. The only downside is that we would need to remember to
> > increment the specfile versions from time to time.
> >
> > OTOH, maybe the build failure is actually a good thing in that it
> > serves as a reminder to bump all the hard-coded versions whenever
> > userspace bumps max policyvers...
>

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Stephen Smalley]

On 2/6/20 10:28 AM, Ondrej Mosnacek wrote:
> On Thu, Feb 6, 2020 at 3:52 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>
>> On 2/6/20 9:19 AM, Ondrej Mosnacek wrote:
>>> On Thu, Feb 6, 2020 at 2:44 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>> On 2/6/20 8:12 AM, Ondrej Mosnacek wrote:
>>>>> When using semodule for building a distribution policy package (as
>>>>> Fedora does), the environment might not have selinuxfs available and
>>>>> provide no way to modify semanage.conf. When we want to build a policy
>>>>> with version X (because our kernel doesn't support X+1 and above yet),
>>>>> but our libsepol already has support for X+1, then we currently have no
>>>>> way to do so.
>>>>
>>>> Not fundamentally opposed, but unclear on the motivation.  The current
>>>> approach is to generate the highest policy version supported by our
>>>> libsepol at build time, then libselinux selinux_mkload_policy() uses
>>>> libsepol functions (sepol_policydb_set_vers(),
>>>> sepol_policydb_to_image()) to automatically downgrade the policy in
>>>> memory to whatever version is supported by the kernel before writing it
>>>> to the kernel.  This works as long as one uses the same or newer
>>>> libsepol at load time as at build time and as long as policydb_write()
>>>> supports writing older policy versions (generally discarding newer
>>>> features).
>>>
>>> The problem is that:
>>> 1. selinux-policy expects that the generated /etc/selinux/.../policy.X
>>> file will be generated with a specific (hard-coded) value X, so if the
>>> userspace is updated in buildroot, the selinux-policy build fails.
>>> 2. If we fix the above by expecting any value X and ship that, then
>>> the build passes in such case, but if a user updates selinux-policy
>>> without updating userspace and reboots, the system will not boot. So
>>> even if we stop incrementing the expected policy version manually, we
>>> would still need to manually increment the minimum required userspace
>>> version each time the policy is rebuilt with userspace that has
>>> incremented its max policyvers.
>>
>> Seems like you could just have selinux-policy depend on the version of
>> libsepol used to build it.
>>
>> The problem with both your current approach and your proposed one is
>> that it means that if a user or package does a semodule -B (or any other
>> semodule/semanage command) on their system, that will generate the
>> latest policy.N version supported by their libsepol, and libselinux will
>> give precedence to that policy at load time.  So if they then later
>> update their selinux-policy package, and it only installs a prebuilt
>> policy.(N-1), that won't actually get loaded - libselinux
>> selinux_mkload_policy() will keep using the policy.N file (which may be
>> older).  Unless I'm missing something.
> 
> Hm, yes, you're right... It seems we have no other choice than to
> better handle the dependency between selinux-policy and libsepol.
> Please disregard this patch series.

Historically, I think we got to this point because originally 
selinux-policy would run semodule from %post to generate the policy.N 
file at install time, thereby always generating the latest version 
supported, and then later switched to pre-building policy.N at package 
build time and just dropping it in place at install time to avoid the 
runtime and memory overhead.  Particularly because it could otherwise 
fail at install time on low-memory systems/VMs.

As a separate matter, one could possibly argue that libselinux 
selinux_mkload_policy() should give preference to the newest file (i.e. 
timestamp-based) rather than the latest policy version.  But even if we 
were to make that change going forward, it won't help with existing 
distro releases.

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Stephen Smalley]

On 2/6/20 10:35 AM, Stephen Smalley wrote:
> On 2/6/20 10:28 AM, Ondrej Mosnacek wrote:
>> On Thu, Feb 6, 2020 at 3:52 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>
>>> On 2/6/20 9:19 AM, Ondrej Mosnacek wrote:
>>>> On Thu, Feb 6, 2020 at 2:44 PM Stephen Smalley <sds@tycho.nsa.gov> 
>>>> wrote:
>>> Seems like you could just have selinux-policy depend on the version of
>>> libsepol used to build it.
>>>
>>> The problem with both your current approach and your proposed one is
>>> that it means that if a user or package does a semodule -B (or any other
>>> semodule/semanage command) on their system, that will generate the
>>> latest policy.N version supported by their libsepol, and libselinux will
>>> give precedence to that policy at load time.  So if they then later
>>> update their selinux-policy package, and it only installs a prebuilt
>>> policy.(N-1), that won't actually get loaded - libselinux
>>> selinux_mkload_policy() will keep using the policy.N file (which may be
>>> older).  Unless I'm missing something.
>>
>> Hm, yes, you're right... It seems we have no other choice than to
>> better handle the dependency between selinux-policy and libsepol.
>> Please disregard this patch series.
> 
> Historically, I think we got to this point because originally 
> selinux-policy would run semodule from %post to generate the policy.N 
> file at install time, thereby always generating the latest version 
> supported, and then later switched to pre-building policy.N at package 
> build time and just dropping it in place at install time to avoid the 
> runtime and memory overhead.  Particularly because it could otherwise 
> fail at install time on low-memory systems/VMs.
> 
> As a separate matter, one could possibly argue that libselinux 
> selinux_mkload_policy() should give preference to the newest file (i.e. 
> timestamp-based) rather than the latest policy version.  But even if we 
> were to make that change going forward, it won't help with existing 
> distro releases.

I guess that doesn't help either since the timestamp of the policy.N 
file generated at package build may still be older than that of any 
locally generated one, even if the package was installed later.

Re: [RFC PATCH 2/2] semodule: support changing policyvers via command line

[Stephen Smalley]

On 2/6/20 1:47 PM, Stephen Smalley wrote:
> On 2/6/20 10:35 AM, Stephen Smalley wrote:
>> On 2/6/20 10:28 AM, Ondrej Mosnacek wrote:
>>> On Thu, Feb 6, 2020 at 3:52 PM Stephen Smalley <sds@tycho.nsa.gov> 
>>> wrote:
>>>>
>>>> On 2/6/20 9:19 AM, Ondrej Mosnacek wrote:
>>>>> On Thu, Feb 6, 2020 at 2:44 PM Stephen Smalley <sds@tycho.nsa.gov> 
>>>>> wrote:
>>>> Seems like you could just have selinux-policy depend on the version of
>>>> libsepol used to build it.
>>>>
>>>> The problem with both your current approach and your proposed one is
>>>> that it means that if a user or package does a semodule -B (or any 
>>>> other
>>>> semodule/semanage command) on their system, that will generate the
>>>> latest policy.N version supported by their libsepol, and libselinux 
>>>> will
>>>> give precedence to that policy at load time.  So if they then later
>>>> update their selinux-policy package, and it only installs a prebuilt
>>>> policy.(N-1), that won't actually get loaded - libselinux
>>>> selinux_mkload_policy() will keep using the policy.N file (which may be
>>>> older).  Unless I'm missing something.
>>>
>>> Hm, yes, you're right... It seems we have no other choice than to
>>> better handle the dependency between selinux-policy and libsepol.
>>> Please disregard this patch series.
>>
>> Historically, I think we got to this point because originally 
>> selinux-policy would run semodule from %post to generate the policy.N 
>> file at install time, thereby always generating the latest version 
>> supported, and then later switched to pre-building policy.N at package 
>> build time and just dropping it in place at install time to avoid the 
>> runtime and memory overhead.  Particularly because it could otherwise 
>> fail at install time on low-memory systems/VMs.
>>
>> As a separate matter, one could possibly argue that libselinux 
>> selinux_mkload_policy() should give preference to the newest file 
>> (i.e. timestamp-based) rather than the latest policy version.  But 
>> even if we were to make that change going forward, it won't help with 
>> existing distro releases.
> 
> I guess that doesn't help either since the timestamp of the policy.N 
> file generated at package build may still be older than that of any 
> locally generated one, even if the package was installed later.

Looks like selinux-policy.spec has preInstall and postInstall macros 
that are triggering a rebuild of policy (semodule -B) if there are any 
/etc/selinux/targeted/policy/policy.* files that differ from the 
packaged one.  So if the user did a semodule -B or any other 
semodule/semanage command previously that generated a newer policy 
version (or the same version but with different contents) then the 
package install is going to run semodule -B and re-generate it anyway.
I guess it isn't broken then.