[PATCH userspace 0/2] Refine semantics of libsemanage's check_ext_changes

[PATCH userspace 0/2] Refine semantics of libsemanage's check_ext_changes

[Ondrej Mosnacek]

Make it always do the "do_write_kernel" step so that the semantic is
essentially "--build but with the module content checksum optimization",
as otherwise the final policy doesn't get updated when there are no
changes in modules, but other customizations are applied.

Since the name --rebuild-if-modules-changed no longer matches the
semantics, also change that to just --refresh, which fits better.

Also update man pages and code comments.

Ondrej Mosnacek (2):
  libsemanage: always write kernel policy when check_ext_changes is
    specified
  semodule: rename --rebuild-if-modules-changed to --refresh

 libsemanage/include/semanage/handle.h |  2 +-
 libsemanage/src/direct_api.c          |  8 +++++---
 policycoreutils/semodule/semodule.8   | 12 ++++++------
 policycoreutils/semodule/semodule.c   | 13 ++++++++++---
 4 files changed, 22 insertions(+), 13 deletions(-)

-- 
2.36.1

[PATCH userspace 1/2] libsemanage: always write kernel policy when check_ext_changes is specified

[Ondrej Mosnacek]

For the use case of rebuilding the policy afte package updates, we need
the check_ext_changes operation to always do at least the do_write_kernel
step, because the various semanage dbs may have also changed content
relative to the current binary policy. As this step is itself relatively
fast, we can do it unconditionally.

Fixes: 286a679fadc4 ("libsemanage: optionally rebuild policy when modules are changed externally")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libsemanage/include/semanage/handle.h | 2 +-
 libsemanage/src/direct_api.c          | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h
index 0157be4f..4cf30815 100644
--- a/libsemanage/include/semanage/handle.h
+++ b/libsemanage/include/semanage/handle.h
@@ -67,7 +67,7 @@ extern void semanage_set_reload(semanage_handle_t * handle, int do_reload);
 extern void semanage_set_rebuild(semanage_handle_t * handle, int do_rebuild);
 
 /* set whether to rebuild the policy on commit when potential changes
- * to module files since last rebuild are detected,
+ * to store files since last rebuild are detected,
  * 1 for yes (default), 0 for no */
 extern void semanage_set_check_ext_changes(semanage_handle_t * handle, int do_check);
 
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 7206483a..7aa081ab 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -1437,13 +1437,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 	 * Determine what else needs to be done.
 	 * We need to write the kernel policy if we are rebuilding
 	 * or if any other policy component that lives in the kernel
-	 * policy has been modified.
+	 * policy has been modified. We also want to force it when
+	 * check_ext_changes was specified as the various dbases may have
+	 * changes as well.
 	 * We need to install the policy files if any of the managed files
 	 * that live under /etc/selinux (kernel policy, seusers, file contexts)
 	 * will be modified.
 	 */
-	do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified |
-		ibendports_modified |
+	do_write_kernel = do_rebuild | sh->check_ext_changes |
+		ports_modified | ibpkeys_modified | ibendports_modified |
 		bools->dtable->is_modified(bools->dbase) |
 		ifaces->dtable->is_modified(ifaces->dbase) |
 		nodes->dtable->is_modified(nodes->dbase) |
-- 
2.36.1

[PATCH userspace 2/2] semodule: rename --rebuild-if-modules-changed to --refresh

[Ondrej Mosnacek]

After the last commit this option's name and description no longer
matches the semantic, so give it a new one and update the descriptions.
The old name is still recognized and aliased to the new one for
backwards compatibility.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 policycoreutils/semodule/semodule.8 | 12 ++++++------
 policycoreutils/semodule/semodule.c | 13 ++++++++++---
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/policycoreutils/semodule/semodule.8 b/policycoreutils/semodule/semodule.8
index d1735d21..c56e580f 100644
--- a/policycoreutils/semodule/semodule.8
+++ b/policycoreutils/semodule/semodule.8
@@ -23,12 +23,12 @@ force a reload of policy
 .B \-B, \-\-build
 force a rebuild of policy (also reloads unless \-n is used)
 .TP
-.B \-\-rebuild-if-modules-changed
-Force a rebuild of the policy if any changes to module content are detected
-(by comparing with checksum from the last transaction).  One can use this
-instead of \-B to ensure that any changes to the module store done by an
-external tool (e.g. a package manager) are applied, while automatically
-skipping the rebuild if there are no new changes.
+.B \-\-refresh
+Like \-\-build, but reuses existing linked policy if no changes to module
+files are detected (by comparing with checksum from the last transaction).
+One can use this instead of \-B to ensure that any changes to the module
+store done by an external tool (e.g. a package manager) are applied, while
+automatically skipping the module re-linking if there are no module changes.
 .TP
 .B \-D, \-\-disable_dontaudit
 Temporarily remove dontaudits from policy.  Reverts whenever policy is rebuilt
diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
index 1ed8e690..ec079486 100644
--- a/policycoreutils/semodule/semodule.c
+++ b/policycoreutils/semodule/semodule.c
@@ -150,9 +150,12 @@ static void usage(char *progname)
 	printf("  -c, --cil extract module as cil. This only affects module extraction.\n");
 	printf("  -H, --hll extract module as hll. This only affects module extraction.\n");
 	printf("  -m, --checksum   print module checksum (SHA256).\n");
-	printf("      --rebuild-if-modules-changed\n"
-	       "                   force policy rebuild if module content changed since\n"
-	       "                   last rebuild (based on checksum)\n");
+	printf("      --refresh    like --build, but reuses existing linked policy if no\n"
+	       "                   changes to module files are detected (via checksum)\n");
+	printf("Deprecated options:\n");
+	printf("  -b,--base	   same as --install\n");
+	printf("  --rebuild-if-modules-changed\n"
+	       "                   same as --refresh\n");
 }
 
 /* Sets the global mode variable to new_mode, but only if no other
@@ -185,6 +188,7 @@ static void parse_command_line(int argc, char **argv)
 {
 	static struct option opts[] = {
 		{"rebuild-if-modules-changed", 0, NULL, '\0'},
+		{"refresh", 0, NULL, '\0'},
 		{"store", required_argument, NULL, 's'},
 		{"base", required_argument, NULL, 'b'},
 		{"help", 0, NULL, 'h'},
@@ -225,6 +229,9 @@ static void parse_command_line(int argc, char **argv)
 		case '\0':
 			switch(longind) {
 			case 0: /* --rebuild-if-modules-changed */
+				fprintf(stderr, "The --rebuild-if-modules-changed option is deprecated. Use --refresh instead.\n");
+				/* fallthrough */
+			case 1: /* --refresh */
 				check_ext_changes = 1;
 				break;
 			default:
-- 
2.36.1

Re: [PATCH userspace 1/2] libsemanage: always write kernel policy when check_ext_changes is specified

[Nicolas Iooss]

On Wed, Jun 8, 2022 at 7:23 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> For the use case of rebuilding the policy afte package updates, we need
> the check_ext_changes operation to always do at least the do_write_kernel
> step, because the various semanage dbs may have also changed content
> relative to the current binary policy. As this step is itself relatively
> fast, we can do it unconditionally.
>
> Fixes: 286a679fadc4 ("libsemanage: optionally rebuild policy when modules are changed externally")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

Hello,
This patch and the next one ("semodule: rename
--rebuild-if-modules-changed to --refresh") look good to me. Has
anyone also taken a look at them?

If nobody objects, I will merge it tomorrow, with a small misspelling
fix in the commit message (afte -> after).

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks,
Nicolas

> ---
>  libsemanage/include/semanage/handle.h | 2 +-
>  libsemanage/src/direct_api.c          | 8 +++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h
> index 0157be4f..4cf30815 100644
> --- a/libsemanage/include/semanage/handle.h
> +++ b/libsemanage/include/semanage/handle.h
> @@ -67,7 +67,7 @@ extern void semanage_set_reload(semanage_handle_t * handle, int do_reload);
>  extern void semanage_set_rebuild(semanage_handle_t * handle, int do_rebuild);
>
>  /* set whether to rebuild the policy on commit when potential changes
> - * to module files since last rebuild are detected,
> + * to store files since last rebuild are detected,
>   * 1 for yes (default), 0 for no */
>  extern void semanage_set_check_ext_changes(semanage_handle_t * handle, int do_check);
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index 7206483a..7aa081ab 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -1437,13 +1437,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>          * Determine what else needs to be done.
>          * We need to write the kernel policy if we are rebuilding
>          * or if any other policy component that lives in the kernel
> -        * policy has been modified.
> +        * policy has been modified. We also want to force it when
> +        * check_ext_changes was specified as the various dbases may have
> +        * changes as well.
>          * We need to install the policy files if any of the managed files
>          * that live under /etc/selinux (kernel policy, seusers, file contexts)
>          * will be modified.
>          */
> -       do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified |
> -               ibendports_modified |
> +       do_write_kernel = do_rebuild | sh->check_ext_changes |
> +               ports_modified | ibpkeys_modified | ibendports_modified |
>                 bools->dtable->is_modified(bools->dbase) |
>                 ifaces->dtable->is_modified(ifaces->dbase) |
>                 nodes->dtable->is_modified(nodes->dbase) |
> --
> 2.36.1
>

Re: [PATCH userspace 1/2] libsemanage: always write kernel policy when check_ext_changes is specified

[James Carter]

On Tue, Jun 28, 2022 at 5:08 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Wed, Jun 8, 2022 at 7:23 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > For the use case of rebuilding the policy afte package updates, we need
> > the check_ext_changes operation to always do at least the do_write_kernel
> > step, because the various semanage dbs may have also changed content
> > relative to the current binary policy. As this step is itself relatively
> > fast, we can do it unconditionally.
> >
> > Fixes: 286a679fadc4 ("libsemanage: optionally rebuild policy when modules are changed externally")
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Hello,
> This patch and the next one ("semodule: rename
> --rebuild-if-modules-changed to --refresh") look good to me. Has
> anyone also taken a look at them?
>
> If nobody objects, I will merge it tomorrow, with a small misspelling
> fix in the commit message (afte -> after).
>

These look good to me as well.
Thanks,
Jim


> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Thanks,
> Nicolas
>
> > ---
> >  libsemanage/include/semanage/handle.h | 2 +-
> >  libsemanage/src/direct_api.c          | 8 +++++---
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h
> > index 0157be4f..4cf30815 100644
> > --- a/libsemanage/include/semanage/handle.h
> > +++ b/libsemanage/include/semanage/handle.h
> > @@ -67,7 +67,7 @@ extern void semanage_set_reload(semanage_handle_t * handle, int do_reload);
> >  extern void semanage_set_rebuild(semanage_handle_t * handle, int do_rebuild);
> >
> >  /* set whether to rebuild the policy on commit when potential changes
> > - * to module files since last rebuild are detected,
> > + * to store files since last rebuild are detected,
> >   * 1 for yes (default), 0 for no */
> >  extern void semanage_set_check_ext_changes(semanage_handle_t * handle, int do_check);
> >
> > diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> > index 7206483a..7aa081ab 100644
> > --- a/libsemanage/src/direct_api.c
> > +++ b/libsemanage/src/direct_api.c
> > @@ -1437,13 +1437,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> >          * Determine what else needs to be done.
> >          * We need to write the kernel policy if we are rebuilding
> >          * or if any other policy component that lives in the kernel
> > -        * policy has been modified.
> > +        * policy has been modified. We also want to force it when
> > +        * check_ext_changes was specified as the various dbases may have
> > +        * changes as well.
> >          * We need to install the policy files if any of the managed files
> >          * that live under /etc/selinux (kernel policy, seusers, file contexts)
> >          * will be modified.
> >          */
> > -       do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified |
> > -               ibendports_modified |
> > +       do_write_kernel = do_rebuild | sh->check_ext_changes |
> > +               ports_modified | ibpkeys_modified | ibendports_modified |
> >                 bools->dtable->is_modified(bools->dbase) |
> >                 ifaces->dtable->is_modified(ifaces->dbase) |
> >                 nodes->dtable->is_modified(nodes->dbase) |
> > --
> > 2.36.1
> >
>

Re: [PATCH userspace 1/2] libsemanage: always write kernel policy when check_ext_changes is specified

[Nicolas Iooss]

On Wed, Jun 29, 2022 at 6:55 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, Jun 28, 2022 at 5:08 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > On Wed, Jun 8, 2022 at 7:23 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > For the use case of rebuilding the policy afte package updates, we need
> > > the check_ext_changes operation to always do at least the do_write_kernel
> > > step, because the various semanage dbs may have also changed content
> > > relative to the current binary policy. As this step is itself relatively
> > > fast, we can do it unconditionally.
> > >
> > > Fixes: 286a679fadc4 ("libsemanage: optionally rebuild policy when modules are changed externally")
> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> >
> > Hello,
> > This patch and the next one ("semodule: rename
> > --rebuild-if-modules-changed to --refresh") look good to me. Has
> > anyone also taken a look at them?
> >
> > If nobody objects, I will merge it tomorrow, with a small misspelling
> > fix in the commit message (afte -> after).
> >
>
> These look good to me as well.
> Thanks,
> Jim

Thanks. I merged both patches.
Nicolas

> > Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> >
> > Thanks,
> > Nicolas
> >
> > > ---
> > >  libsemanage/include/semanage/handle.h | 2 +-
> > >  libsemanage/src/direct_api.c          | 8 +++++---
> > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h
> > > index 0157be4f..4cf30815 100644
> > > --- a/libsemanage/include/semanage/handle.h
> > > +++ b/libsemanage/include/semanage/handle.h
> > > @@ -67,7 +67,7 @@ extern void semanage_set_reload(semanage_handle_t * handle, int do_reload);
> > >  extern void semanage_set_rebuild(semanage_handle_t * handle, int do_rebuild);
> > >
> > >  /* set whether to rebuild the policy on commit when potential changes
> > > - * to module files since last rebuild are detected,
> > > + * to store files since last rebuild are detected,
> > >   * 1 for yes (default), 0 for no */
> > >  extern void semanage_set_check_ext_changes(semanage_handle_t * handle, int do_check);
> > >
> > > diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> > > index 7206483a..7aa081ab 100644
> > > --- a/libsemanage/src/direct_api.c
> > > +++ b/libsemanage/src/direct_api.c
> > > @@ -1437,13 +1437,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> > >          * Determine what else needs to be done.
> > >          * We need to write the kernel policy if we are rebuilding
> > >          * or if any other policy component that lives in the kernel
> > > -        * policy has been modified.
> > > +        * policy has been modified. We also want to force it when
> > > +        * check_ext_changes was specified as the various dbases may have
> > > +        * changes as well.
> > >          * We need to install the policy files if any of the managed files
> > >          * that live under /etc/selinux (kernel policy, seusers, file contexts)
> > >          * will be modified.
> > >          */
> > > -       do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified |
> > > -               ibendports_modified |
> > > +       do_write_kernel = do_rebuild | sh->check_ext_changes |
> > > +               ports_modified | ibpkeys_modified | ibendports_modified |
> > >                 bools->dtable->is_modified(bools->dbase) |
> > >                 ifaces->dtable->is_modified(ifaces->dbase) |
> > >                 nodes->dtable->is_modified(nodes->dbase) |
> > > --
> > > 2.36.1
> > >
> >