diff mbox series

package/refpolicy: Treat all modules as custom

Message ID 20210830114531.2285178-1-jose.pekkarinen@unikie.com
State Superseded
Headers show
Series package/refpolicy: Treat all modules as custom | expand

Commit Message

José Pekkarinen Aug. 30, 2021, 11:45 a.m. UTC
The current processing of the modules doesn't work for
custom made policies appended through the extra dir mechanism,
since sed won't find a match for custom modules, it will
continue without triggering and error. This patch removes
all the modules from modules.conf and add them one by
one using REFPOLICY_MODULES values.

Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com>
---
 package/refpolicy/refpolicy.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni Aug. 30, 2021, 9:14 p.m. UTC | #1
Hello,

On Mon, 30 Aug 2021 14:45:31 +0300
José Pekkarinen <jose.pekkarinen@unikie.com> wrote:

> The current processing of the modules doesn't work for
> custom made policies appended through the extra dir mechanism,
> since sed won't find a match for custom modules, it will
> continue without triggering and error. This patch removes
> all the modules from modules.conf and add them one by
> one using REFPOLICY_MODULES values.
> 
> Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com>
> ---
>  package/refpolicy/refpolicy.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I've added Antoine Ténart, Maxime Chevallier and Matt Weber in Cc for a
review of this patch.

Best regards,

Thomas Petazzoni

> diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
> index 0194708b37..1c0a2c3385 100644
> --- a/package/refpolicy/refpolicy.mk
> +++ b/package/refpolicy/refpolicy.mk
> @@ -85,9 +85,9 @@ endef
>  # In the context of a monolithic policy enabling a piece of the policy as
>  # 'base' or 'module' is equivalent, so we enable them as 'base'.
>  define REFPOLICY_CONFIGURE_MODULES
> -	$(SED) "s/ = module/ = no/g" $(@D)/policy/modules.conf
> +	$(SED) "/ = module/d" $(@D)/policy/modules.conf
>  	$(foreach m,$(sort $(REFPOLICY_MODULES)),
> -		$(SED) "/^$(m) =/c\$(m) = base" $(@D)/policy/modules.conf
> +		$(SED) "/^# Module: $(m)/a\$(m) = base" $(@D)/policy/modules.conf
>  	)
>  endef
>
Antoine Tenart Sept. 17, 2021, 5:22 p.m. UTC | #2
Hello José,

Quoting José Pekkarinen (2021-08-30 13:45:31)
> The current processing of the modules doesn't work for
> custom made policies appended through the extra dir mechanism,
> since sed won't find a match for custom modules, it will
> continue without triggering and error. This patch removes
> all the modules from modules.conf and add them one by
> one using REFPOLICY_MODULES values.

I'm failing to see what particular setup the change below would fix.

Could you elaborate on the above? Maybe including configuration
snippets and example of such a module (with the file tree, starting from
REFPOLICY_EXTRA_MODULES_DIRS).

Thanks!
Antoine

> diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
> index 0194708b37..1c0a2c3385 100644
> --- a/package/refpolicy/refpolicy.mk
> +++ b/package/refpolicy/refpolicy.mk
> @@ -85,9 +85,9 @@ endef
>  # In the context of a monolithic policy enabling a piece of the policy as
>  # 'base' or 'module' is equivalent, so we enable them as 'base'.
>  define REFPOLICY_CONFIGURE_MODULES
> -       $(SED) "s/ = module/ = no/g" $(@D)/policy/modules.conf
> +       $(SED) "/ = module/d" $(@D)/policy/modules.conf
>         $(foreach m,$(sort $(REFPOLICY_MODULES)),
> -               $(SED) "/^$(m) =/c\$(m) = base" $(@D)/policy/modules.conf
> +               $(SED) "/^# Module: $(m)/a\$(m) = base" $(@D)/policy/modules.conf
>         )
>  endef
>  
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
José Pekkarinen Sept. 20, 2021, 6:01 a.m. UTC | #3
On Fri, Sep 17, 2021 at 8:22 PM Antoine Tenart <atenart@kernel.org> wrote:

> Hello José,
>
> Quoting José Pekkarinen (2021-08-30 13:45:31)
> > The current processing of the modules doesn't work for
> > custom made policies appended through the extra dir mechanism,
> > since sed won't find a match for custom modules, it will
> > continue without triggering and error. This patch removes
> > all the modules from modules.conf and add them one by
> > one using REFPOLICY_MODULES values.
>
> I'm failing to see what particular setup the change below would fix.
>
> Could you elaborate on the above? Maybe including configuration
> snippets and example of such a module (with the file tree, starting from
> REFPOLICY_EXTRA_MODULES_DIRS).
>
> Thanks!
> Antoine
>

Absolutely, in the security section of my .config we can read the

following:

BR2_PACKAGE_POLICYCOREUTILS=y
BR2_PACKAGE_REFPOLICY=y
BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"
BR2_PACKAGE_REFPOLICY_POLICY_STATE_ENFORCING=y

In $OUTPUT_DIR/selinux I have the following files:

$ ls
secure.fc  secure.if  secure.te
$ cat secure.fc
$ cat secure.if
$  cat secure.te
policy_module(secure, 1.0);

#============= bin_t ==============
allow bin_t user_home_t:dir { getattr open read };
allow bin_t user_home_t:file { getattr open read };

#============= chkpwd_t ==============
allow chkpwd_t device_t:chr_file { read write };
allow chkpwd_t proc_t:filesystem getattr;
allow chkpwd_t sysctl_kernel_t:dir search;
allow chkpwd_t sysctl_kernel_t:file { open read };
allow chkpwd_t tmpfs_t:dir search;
[...]

The problem arises because the secure module is not

known to refpolicy, and at the time of building, policy/modules.conf
requires a line that the recipe sed, but no reference of secure
module is in the file beforehand. Since then, the sed modifies
nothing and the policies I have in the file doesn't turn up if
I execute sesearch on the policy file that in my case is policy.32
since I'm working with buildroot 2021.02.x. Please let me know
if there is any further question.

Best regards.

José.


> diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/
> refpolicy.mk
> > index 0194708b37..1c0a2c3385 100644
> > --- a/package/refpolicy/refpolicy.mk
> > +++ b/package/refpolicy/refpolicy.mk
> > @@ -85,9 +85,9 @@ endef
> >  # In the context of a monolithic policy enabling a piece of the policy
> as
> >  # 'base' or 'module' is equivalent, so we enable them as 'base'.
> >  define REFPOLICY_CONFIGURE_MODULES
> > -       $(SED) "s/ = module/ = no/g" $(@D)/policy/modules.conf
> > +       $(SED) "/ = module/d" $(@D)/policy/modules.conf
> >         $(foreach m,$(sort $(REFPOLICY_MODULES)),
> > -               $(SED) "/^$(m) =/c\$(m) = base" $(@D)/policy/modules.conf
> > +               $(SED) "/^# Module: $(m)/a\$(m) = base"
> $(@D)/policy/modules.conf
> >         )
> >  endef
> >
> > --
> > 2.25.1
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
>
Antoine Tenart Sept. 20, 2021, 9:30 a.m. UTC | #4
Quoting José Pekkarinen (2021-09-20 08:01:27)
>    On Fri, Sep 17, 2021 at 8:22 PM Antoine Tenart <[1]atenart@kernel.org>
>    wrote:
>      Quoting José Pekkarinen (2021-08-30 13:45:31)
>      > The current processing of the modules doesn't work for
>      > custom made policies appended through the extra dir mechanism,
>      > since sed won't find a match for custom modules, it will
>      > continue without triggering and error. This patch removes
>      > all the modules from modules.conf and add them one by
>      > one using REFPOLICY_MODULES values.
> 
>      I'm failing to see what particular setup the change below would fix.
> 
>      Could you elaborate on the above? Maybe including configuration
>      snippets and example of such a module (with the file tree, starting from
>      REFPOLICY_EXTRA_MODULES_DIRS).
> 
>    Absolutely, in the security section of my .config we can read the
>    following:
>    BR2_PACKAGE_POLICYCOREUTILS=y
>    BR2_PACKAGE_REFPOLICY=y
>    BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"
>    BR2_PACKAGE_REFPOLICY_POLICY_STATE_ENFORCING=y

This should work. Did you check the content of your module show up after
applying this patch?

I'm wondering if this has to do with:

  BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"

What is the value of $OUTPUT_DIR? Where does this come from? Could you
try without using a variable in BR2_REFPOLICY_EXTRA_MODULES_DIRS?

Thanks,
Antoine
José Pekkarinen Sept. 20, 2021, 9:44 a.m. UTC | #5
On Mon, Sep 20, 2021 at 12:30 PM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-20 08:01:27)
> >    On Fri, Sep 17, 2021 at 8:22 PM Antoine Tenart <[1]atenart@kernel.org
> >
> >    wrote:
> >      Quoting José Pekkarinen (2021-08-30 13:45:31)
> >      > The current processing of the modules doesn't work for
> >      > custom made policies appended through the extra dir mechanism,
> >      > since sed won't find a match for custom modules, it will
> >      > continue without triggering and error. This patch removes
> >      > all the modules from modules.conf and add them one by
> >      > one using REFPOLICY_MODULES values.
> >
> >      I'm failing to see what particular setup the change below would fix.
> >
> >      Could you elaborate on the above? Maybe including configuration
> >      snippets and example of such a module (with the file tree, starting
> from
> >      REFPOLICY_EXTRA_MODULES_DIRS).
> >
> >    Absolutely, in the security section of my .config we can read the
> >    following:
> >    BR2_PACKAGE_POLICYCOREUTILS=y
> >    BR2_PACKAGE_REFPOLICY=y
> >    BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"
> >    BR2_PACKAGE_REFPOLICY_POLICY_STATE_ENFORCING=y
>
> This should work. Did you check the content of your module show up after
> applying this patch?
>

Hi,

Yes, after the patch I can see the module copied in the folder:

build/refpolicy-2.20200818$ ls policy/modules/buildroot/
base.fc  base.if  base.te  metadata.xml  secure.fc  secure.if  secure.te

And:

/build/refpolicy-2.20200818$ grep secure policy/modules.conf
# Module: secure
secure = base
# Small and secure DNS daemon.

I'm wondering if this has to do with:
>
>   BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"
>
> What is the value of $OUTPUT_DIR? Where does this come from? Could you
> try without using a variable in BR2_REFPOLICY_EXTRA_MODULES_DIRS?
>

I put that to try to make your life easier, in fact we use more

variables in this line that are modified on the fly by a makefile. The
line translates to something like:

$ ls /output/secure/output_x86_qemu/selinux/
base.fc  base.if  base.te  secure.fc  secure.if  secure.te

Best regards.

José.
Antoine Tenart Sept. 20, 2021, 1:21 p.m. UTC | #6
Quoting José Pekkarinen (2021-09-20 11:44:42)
>    On Mon, Sep 20, 2021 at 12:30 PM Antoine Tenart <[1]atenart@kernel.org>
>    wrote:
> 
>      Quoting José Pekkarinen (2021-09-20 08:01:27)
>      >
>      >    Absolutely, in the security section of my .config we can read the
>      >    following:
>      >    BR2_PACKAGE_POLICYCOREUTILS=y
>      >    BR2_PACKAGE_REFPOLICY=y
>      >    BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"
>      >    BR2_PACKAGE_REFPOLICY_POLICY_STATE_ENFORCING=y
> 
>      This should work. Did you check the content of your module show up after
>      applying this patch?
> 
>    Yes, after the patch I can see the module copied in the folder:
>    build/refpolicy-2.20200818$ ls policy/modules/buildroot/
>    base.fc  base.if  base.te  metadata.xml  secure.fc  secure.if  secure.te
> 
>      And:
> 
>    /build/refpolicy-2.20200818$ grep secure policy/modules.conf
>    # Module: secure
>    secure = base
>    # Small and secure DNS daemon.

I'm missing something here. I did the test and using the module and
configuration snippets you provided (replacing $OUTPUT_DIR/selinux with
something else; and adding a <summary> to secure.if[1]). It worked. The
'secure' module was found and enabled.

The logic is the following in Buildroot for extra modules:

1. The modules are rsynced in policy/modules/buildrood/.
2. If not already there, a metadata.xml file is added.
3. The refpolicy build system is used[2] to generate modules.conf using
   all modules matching 'policy/modules/*/*.te'.
4. All modules in modules.conf are disabled and then only the ones in
   REFPOLICY_MODULES are enabled.

It looks like more of a refpolicy/module issue than a Buildroot one:
steps 1 and 2 seem to work, but not step 3. If you retrieve the
refpolicy project outside of Builroot and mimic the above steps, are
your modules listed in modules.conf? If not that might be a good
starting point. I don't have a better idea for now...

Antoine

[1] Which I guess is not your issue as otherwise the configuration step
    fails and the build stops.
[2] `make -j1 bare conf`
José Pekkarinen Sept. 20, 2021, 1:39 p.m. UTC | #7
On Mon, Sep 20, 2021 at 4:21 PM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-20 11:44:42)
> >    On Mon, Sep 20, 2021 at 12:30 PM Antoine Tenart <[1]
> atenart@kernel.org>
> >    wrote:
> >
> >      Quoting José Pekkarinen (2021-09-20 08:01:27)
> >      >
> >      >    Absolutely, in the security section of my .config we can read
> the
> >      >    following:
> >      >    BR2_PACKAGE_POLICYCOREUTILS=y
> >      >    BR2_PACKAGE_REFPOLICY=y
> >      >    BR2_REFPOLICY_EXTRA_MODULES_DIRS="$OUTPUT_DIR/selinux"
> >      >    BR2_PACKAGE_REFPOLICY_POLICY_STATE_ENFORCING=y
> >
> >      This should work. Did you check the content of your module show up
> after
> >      applying this patch?
> >
> >    Yes, after the patch I can see the module copied in the folder:
> >    build/refpolicy-2.20200818$ ls policy/modules/buildroot/
> >    base.fc  base.if  base.te  metadata.xml  secure.fc  secure.if
>  secure.te
> >
> >      And:
> >
> >    /build/refpolicy-2.20200818$ grep secure policy/modules.conf
> >    # Module: secure
> >    secure = base
> >    # Small and secure DNS daemon.
>
> I'm missing something here. I did the test and using the module and
> configuration snippets you provided (replacing $OUTPUT_DIR/selinux with
> something else; and adding a <summary> to secure.if[1]). It worked. The
> 'secure' module was found and enabled.
>
> The logic is the following in Buildroot for extra modules:
>
> 1. The modules are rsynced in policy/modules/buildrood/.
> 2. If not already there, a metadata.xml file is added.
> 3. The refpolicy build system is used[2] to generate modules.conf using
>    all modules matching 'policy/modules/*/*.te'.
> 4. All modules in modules.conf are disabled and then only the ones in
>    REFPOLICY_MODULES are enabled.
>
> It looks like more of a refpolicy/module issue than a Buildroot one:
> steps 1 and 2 seem to work, but not step 3. If you retrieve the
> refpolicy project outside of Builroot and mimic the above steps, are
> your modules listed in modules.conf? If not that might be a good
> starting point. I don't have a better idea for now...
>

Hi,

I did, and this is how modules.conf looks like when

it comes to the section of my module:

[...]
# Module: xscreensaver
#
# Modular screen saver and locker for X11.
#
xscreensaver = module

# Layer: buildroot
# Module: secure
#
# Layer: kernel
# Module: storage
[...]

Now, reading the INSTALL file, it says the following:

If you do not have a modules.conf, one can be generated:

   make conf

This will create a *default modules.conf*.

This default makes me think it implies you'd need to

activate your own modules if they are there, and why I believe
buildroot would require that extra logic. refpolicy project may
stand for letting users add their own, but not taking part on
it theirselves.

Best regards.

José.



>
> Antoine
>
> [1] Which I guess is not your issue as otherwise the configuration step
>     fails and the build stops.
> [2] `make -j1 bare conf`
>
Antoine Tenart Sept. 20, 2021, 1:52 p.m. UTC | #8
Quoting José Pekkarinen (2021-09-20 15:39:13)
>    On Mon, Sep 20, 2021 at 4:21 PM Antoine Tenart <[1]atenart@kernel.org>
>    wrote:
> 
>      The logic is the following in Buildroot for extra modules:
> 
>      1. The modules are rsynced in policy/modules/buildrood/.
>      2. If not already there, a metadata.xml file is added.
>      3. The refpolicy build system is used[2] to generate modules.conf using
>         all modules matching 'policy/modules/*/*.te'.
>      4. All modules in modules.conf are disabled and then only the ones in
>         REFPOLICY_MODULES are enabled.
> 
>      It looks like more of a refpolicy/module issue than a Buildroot one:
>      steps 1 and 2 seem to work, but not step 3. If you retrieve the
>      refpolicy project outside of Builroot and mimic the above steps, are
>      your modules listed in modules.conf? If not that might be a good
>      starting point. I don't have a better idea for now...
> 
>    I did, and this is how modules.conf looks like when
>    it comes to the section of my module:
>    [...]
>    # Module: xscreensaver
>    #
>    # Modular screen saver and locker for X11.
>    #  
>    xscreensaver = module
> 
>    # Layer: buildroot
>    # Module: secure
>    #
>    # Layer: kernel
>    # Module: storage
>    [...] 
> 
>    Now, reading the INSTALL file, it says the following:
>    If you do not have a modules.conf, one can be generated:
> 
>       make conf
> 
>    This will create a default modules.conf.
> 
>    This default makes me think it implies you'd need to
>    activate your own modules if they are there, and why I believe
>    buildroot would require that extra logic. refpolicy project may
>    stand for letting users add their own, but not taking part on
>    it theirselves.

Reproducing locally the modules were correctly listed and enabled.
However looking at the modules.conf generated on your machine, your
modules' documentation is included but the modules aren't enabled (as
modules) by default. There might be some rules in the refpolicy build
system that can explain such a difference.

I think the issue comes down to understanding how modules are selected
to be enabled by default (or not enabled), and why your modules are
impacted. (Then there might be something to improve in Buildroot).

Thanks!
Antoine
José Pekkarinen Sept. 21, 2021, 6:29 a.m. UTC | #9
On Mon, Sep 20, 2021 at 4:52 PM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-20 15:39:13)
> >    On Mon, Sep 20, 2021 at 4:21 PM Antoine Tenart <[1]atenart@kernel.org
> >
> >    wrote:
> >
> >      The logic is the following in Buildroot for extra modules:
> >
> >      1. The modules are rsynced in policy/modules/buildrood/.
> >      2. If not already there, a metadata.xml file is added.
> >      3. The refpolicy build system is used[2] to generate modules.conf
> using
> >         all modules matching 'policy/modules/*/*.te'.
> >      4. All modules in modules.conf are disabled and then only the ones
> in
> >         REFPOLICY_MODULES are enabled.
> >
> >      It looks like more of a refpolicy/module issue than a Buildroot one:
> >      steps 1 and 2 seem to work, but not step 3. If you retrieve the
> >      refpolicy project outside of Builroot and mimic the above steps, are
> >      your modules listed in modules.conf? If not that might be a good
> >      starting point. I don't have a better idea for now...
> >
> >    I did, and this is how modules.conf looks like when
> >    it comes to the section of my module:
> >    [...]
> >    # Module: xscreensaver
> >    #
> >    # Modular screen saver and locker for X11.
> >    #
> >    xscreensaver = module
> >
> >    # Layer: buildroot
> >    # Module: secure
> >    #
> >    # Layer: kernel
> >    # Module: storage
> >    [...]
> >
> >    Now, reading the INSTALL file, it says the following:
> >    If you do not have a modules.conf, one can be generated:
> >
> >       make conf
> >
> >    This will create a default modules.conf.
> >
> >    This default makes me think it implies you'd need to
> >    activate your own modules if they are there, and why I believe
> >    buildroot would require that extra logic. refpolicy project may
> >    stand for letting users add their own, but not taking part on
> >    it theirselves.
>
> Reproducing locally the modules were correctly listed and enabled.
> However looking at the modules.conf generated on your machine, your
> modules' documentation is included but the modules aren't enabled (as
> modules) by default. There might be some rules in the refpolicy build
> system that can explain such a difference.
>
> I think the issue comes down to understanding how modules are selected
> to be enabled by default (or not enabled), and why your modules are
> impacted. (Then there might be something to improve in Buildroot).
>

Can this be that I'm working with an out of date version of buildroot?

My project was started with 2021.02 and I observe the refpolicy dates from
August last year. I plan to start the works on fixing this, since it impacts
any sort of upstreaming.

Best regards.


José.
Antoine Tenart Sept. 21, 2021, 7:12 a.m. UTC | #10
Quoting José Pekkarinen (2021-09-21 08:29:38)
>    On Mon, Sep 20, 2021 at 4:52 PM Antoine Tenart <[1]atenart@kernel.org>
>    wrote:
> 
>      Reproducing locally the modules were correctly listed and enabled.
>      However looking at the modules.conf generated on your machine, your
>      modules' documentation is included but the modules aren't enabled (as
>      modules) by default. There might be some rules in the refpolicy build
>      system that can explain such a difference.
> 
>      I think the issue comes down to understanding how modules are selected
>      to be enabled by default (or not enabled), and why your modules are
>      impacted. (Then there might be something to improve in Buildroot).
> 
>    Can this be that I'm working with an out of date version of buildroot?
> 
>    My project was started with 2021.02 and I observe the refpolicy dates from
>    August last year. I plan to start the works on fixing this, since it
>    impacts any sort of upstreaming.

You can try with a newer version of refpolicy, in case they did fix a
bug related to this; it's a good first test to make. But there are
chances this is due to how modules are selected by default, there might
be some extra logic we don't know about.

Thanks,
Antoine
José Pekkarinen Sept. 21, 2021, 1:32 p.m. UTC | #11
On Tue, Sep 21, 2021 at 10:12 AM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-21 08:29:38)
> >    On Mon, Sep 20, 2021 at 4:52 PM Antoine Tenart <[1]atenart@kernel.org
> >
> >    wrote:
> >
> >      Reproducing locally the modules were correctly listed and enabled.
> >      However looking at the modules.conf generated on your machine, your
> >      modules' documentation is included but the modules aren't enabled
> (as
> >      modules) by default. There might be some rules in the refpolicy
> build
> >      system that can explain such a difference.
> >
> >      I think the issue comes down to understanding how modules are
> selected
> >      to be enabled by default (or not enabled), and why your modules are
> >      impacted. (Then there might be something to improve in Buildroot).
> >
> >    Can this be that I'm working with an out of date version of buildroot?
> >
> >    My project was started with 2021.02 and I observe the refpolicy dates
> from
> >    August last year. I plan to start the works on fixing this, since it
> >    impacts any sort of upstreaming.
>
> You can try with a newer version of refpolicy, in case they did fix a
> bug related to this; it's a good first test to make. But there are
> chances this is due to how modules are selected by default, there might
> be some extra logic we don't know about.
>

Hi,

I tested today to build the system with buildroot 2021.05.2(without the

patch) and it reproduces exactly the same behaviour, policy/modules.conf
doesn't
receive the line to activate the secure module, and if I search in
policy.conf or
policy.32 through sesearch I find no sign of the policies defined in the
module.
I'll attempt the upgrade to 2021.08, but that will require a bit more time.

Best regards.


José.
Antoine Tenart Sept. 21, 2021, 1:42 p.m. UTC | #12
Quoting José Pekkarinen (2021-09-21 15:32:32)
> On Tue, Sep 21, 2021 at 10:12 AM Antoine Tenart <[1]atenart@kernel.org>
> wrote:
> 
> I tested today to build the system with buildroot 2021.05.2(without
> the patch) and it reproduces exactly the same behaviour,
> policy/modules.conf doesn't receive the line to activate the secure
> module, and if I search in policy.conf or policy.32 through sesearch I
> find no sign of the policies defined in the module.  I'll attempt the
> upgrade to 2021.08, but that will require a bit more time.

Alternatively you can just test with newer refpolicy versions, outside
of Buildroot and look at the generated modules.conf. This will give the
same information and should be easier to do. (My feeling is this won't
change and we'll have to dive into the refpolicy logic for enabling
modules when running 'make conf').

Antoine
José Pekkarinen Sept. 22, 2021, 2 p.m. UTC | #13
On Tue, Sep 21, 2021 at 4:42 PM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-21 15:32:32)
> > On Tue, Sep 21, 2021 at 10:12 AM Antoine Tenart <[1]atenart@kernel.org>
> > wrote:
> >
> > I tested today to build the system with buildroot 2021.05.2(without
> > the patch) and it reproduces exactly the same behaviour,
> > policy/modules.conf doesn't receive the line to activate the secure
> > module, and if I search in policy.conf or policy.32 through sesearch I
> > find no sign of the policies defined in the module.  I'll attempt the
> > upgrade to 2021.08, but that will require a bit more time.
>
> Alternatively you can just test with newer refpolicy versions, outside
> of Buildroot and look at the generated modules.conf. This will give the
> same information and should be easier to do. (My feeling is this won't
> change and we'll have to dive into the refpolicy logic for enabling
> modules when running 'make conf').
>

The config generator requires a summary line in the module.if file

to be added in policy/modules.conf, otherwise it doesn't process any
further.
It seems to be something tricky to address, in your end developing a check
the summary is in place doesn't make sense, in their end, not using that
hook to learn the modules from the xml make be also complicated. All
in all, thanks for the comments, at least I have a way out without this
patch. If there is something I can address for you in this topic, feel free
to ask.

Best regards.

José.
Antoine Tenart Sept. 22, 2021, 2:23 p.m. UTC | #14
Quoting José Pekkarinen (2021-09-22 16:00:19)
>    On Tue, Sep 21, 2021 at 4:42 PM Antoine Tenart <[1]atenart@kernel.org>
>    wrote:
>      Quoting José Pekkarinen (2021-09-21 15:32:32)
>      > On Tue, Sep 21, 2021 at 10:12 AM Antoine Tenart
>      <[1][2]atenart@kernel.org>
>      > wrote:
>      >
>      > I tested today to build the system with buildroot 2021.05.2(without
>      > the patch) and it reproduces exactly the same behaviour,
>      > policy/modules.conf doesn't receive the line to activate the secure
>      > module, and if I search in policy.conf or policy.32 through sesearch I
>      > find no sign of the policies defined in the module.  I'll attempt the
>      > upgrade to 2021.08, but that will require a bit more time.
> 
>      Alternatively you can just test with newer refpolicy versions, outside
>      of Buildroot and look at the generated modules.conf. This will give the
>      same information and should be easier to do. (My feeling is this won't
>      change and we'll have to dive into the refpolicy logic for enabling
>      modules when running 'make conf').
> 
>    The config generator requires a summary line in the module.if file
>    to be added in policy/modules.conf, otherwise it doesn't process
>    any further.  It seems to be something tricky to address, in your
>    end developing a check the summary is in place doesn't make sense,
>    in their end, not using that hook to learn the modules from the xml
>    make be also complicated.

I agree, having a check for the summary would be outside of Buildroot's
scope. It's linked to how SELinux modules should be written.

However I'm surprised as my understanding was the summary was required
for the refpolicy configuration step to succeed (I did use a summary
for all my tests because of this). When removing a summary from a module
I always get the following error, and the Buildroot build stops.

  doc/policy.xml:8376: element module: validity error : Element module content does not follow the DTD, expecting (summary , desc? , required? , (interface | template)* , (bool | tunable)*), got ()
  Document doc/policy.xml does not validate against doc/policy.dtd

Do you have an idea what made your build to succeed even though you did
not have a summary in your module?

Antoine
José Pekkarinen Sept. 23, 2021, 6:26 a.m. UTC | #15
On Wed, Sep 22, 2021 at 5:23 PM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-22 16:00:19)
> >    On Tue, Sep 21, 2021 at 4:42 PM Antoine Tenart <[1]atenart@kernel.org
> >
> >    wrote:
> >      Quoting José Pekkarinen (2021-09-21 15:32:32)
> >      > On Tue, Sep 21, 2021 at 10:12 AM Antoine Tenart
> >      <[1][2]atenart@kernel.org>
> >      > wrote:
> >      >
> >      > I tested today to build the system with buildroot
> 2021.05.2(without
> >      > the patch) and it reproduces exactly the same behaviour,
> >      > policy/modules.conf doesn't receive the line to activate the
> secure
> >      > module, and if I search in policy.conf or policy.32 through
> sesearch I
> >      > find no sign of the policies defined in the module.  I'll attempt
> the
> >      > upgrade to 2021.08, but that will require a bit more time.
> >
> >      Alternatively you can just test with newer refpolicy versions,
> outside
> >      of Buildroot and look at the generated modules.conf. This will give
> the
> >      same information and should be easier to do. (My feeling is this
> won't
> >      change and we'll have to dive into the refpolicy logic for enabling
> >      modules when running 'make conf').
> >
> >    The config generator requires a summary line in the module.if file
> >    to be added in policy/modules.conf, otherwise it doesn't process
> >    any further.  It seems to be something tricky to address, in your
> >    end developing a check the summary is in place doesn't make sense,
> >    in their end, not using that hook to learn the modules from the xml
> >    make be also complicated.
>
> I agree, having a check for the summary would be outside of Buildroot's
> scope. It's linked to how SELinux modules should be written.
>
> However I'm surprised as my understanding was the summary was required
> for the refpolicy configuration step to succeed (I did use a summary
> for all my tests because of this). When removing a summary from a module
> I always get the following error, and the Buildroot build stops.
>
>   doc/policy.xml:8376: element module: validity error : Element module
> content does not follow the DTD, expecting (summary , desc? , required? ,
> (interface | template)* , (bool | tunable)*), got ()
>   Document doc/policy.xml does not validate against doc/policy.dtd
>
> Do you have an idea what made your build to succeed even though you did
> not have a summary in your module?
>

I believe it is validating to the summary prior to the module,

the one you put in metadata.xml, but not any internal summary for
the interface. This is how policy.xml looks like in a case where I didn't
apply the mitigation:

<layer name="buildroot">
<summary>Buildroot extra modules</summary>
<module name="base" filename="policy/modules/buildroot/base.if">
</module>
<module name="secure" filename="policy/modules/buildroot/secure.if">
</module>
</layer>

With this the modules.conf comes as:

# Layer: buildroot
# Module: base
#
# Layer: buildroot
# Module: secure
#

There is a summary followed by a module, validation pass, but

the module is not built. If I add the following lines in the build folder
modules[1]
and run make.conf:

[1] refpolicy-2.20200818/policy/modules/buildroot/secure.if: ##
<summary>External secure module.</summary>
refpolicy-2.20200818/policy/modules/buildroot/base.if: ## <summary>External
base module.</summary>

The policy.xml looks like:

<layer name="buildroot">
<summary>Buildroot extra modules</summary>
<module name="base" filename="policy/modules/buildroot/base.if">
<summary>External base modules.</summary>
</module>
<module name="secure" filename="policy/modules/buildroot/secure.if">
<summary>External secure os vm module.</summary>
</module>
</layer>

Then policy/modules.conf looks this way:

# Layer: buildroot
# Module: base
#
# External base modules.
#
base = module

# Layer: buildroot
# Module: secure
#
# External secure os vm module.
#
secure = module

And this produces the modules to get into the policy.32 file.

Does it makes any sense on your end?


Best regards.

José.
Antoine Tenart Sept. 23, 2021, 7:59 a.m. UTC | #16
Quoting José Pekkarinen (2021-09-23 08:26:02)
>  On Wed, Sep 22, 2021 at 5:23 PM Antoine Tenart <[1]atenart@kernel.org>
>  wrote:
>
>    However I'm surprised as my understanding was the summary was required
>    for the refpolicy configuration step to succeed (I did use a summary
>    for all my tests because of this). When removing a summary from a module
>    I always get the following error, and the Buildroot build stops.
>
>      doc/policy.xml:8376: element module: validity error : Element module
>    content does not follow the DTD, expecting (summary , desc? , required?
>    , (interface | template)* , (bool | tunable)*), got ()
>      Document doc/policy.xml does not validate against doc/policy.dtd
>
>    Do you have an idea what made your build to succeed even though you did
>    not have a summary in your module?
>
>  I believe it is validating to the summary prior to the module,
>  the one you put in metadata.xml, but not any internal summary for
>  the interface. This is how policy.xml looks like in a case where I didn't
>  apply the mitigation:
>  <layer name="buildroot">
>  <summary>Buildroot extra modules</summary>
>  <module name="base" filename="policy/modules/buildroot/base.if">
>  </module>
>  <module name="secure" filename="policy/modules/buildroot/secure.if">
>  </module>
>  </layer>
>
>  With this the modules.conf comes as:
>
>  # Layer: buildroot
>  # Module: base
>  #
>  # Layer: buildroot
>  # Module: secure
>  #
>
>  There is a summary followed by a module, validation pass, but
>
>  the module is not built. If I add the following lines in the build folder
>  modules[1]
>  and run make.conf:
>  [1] refpolicy-2.20200818/policy/modules/buildroot/secure.if: ##
>  <summary>External secure module.</summary>
>  refpolicy-2.20200818/policy/modules/buildroot/base.if: ##
>  <summary>External base module.</summary>
>
>  The policy.xml looks like:
>
>  <layer name="buildroot">
>  <summary>Buildroot extra modules</summary>
>  <module name="base" filename="policy/modules/buildroot/base.if">
>  <summary>External base modules.</summary>
>  </module>
>  <module name="secure" filename="policy/modules/buildroot/secure.if">
>  <summary>External secure os vm module.</summary>
>  </module>
>  </layer>
>
>  Then policy/modules.conf looks this way:
>
>  # Layer: buildroot
>  # Module: base
>  #
>  # External base modules.
>  #  
>  base = module
>
>  # Layer: buildroot
>  # Module: secure
>  #
>  # External secure os vm module.
>  #  
>  secure = module
>
>  And this produces the modules to get into the policy.32 file.
>  Does it makes any sense on your end?

The above does not reproduce for me. But I might know what's going on:
do you have xmllint installed on your machine?

If not, the validation step is skipped but the build is not stopped,
which would explain the difference in behaviour we have between our
tests:

  Makefile:453:
  $(verbose) if test -x $(XMLLINT) && test -f $(xmldtd); then \
          $(XMLLINT) --noout --path $(dir $(xmldtd)) --dtdvalid $(xmldtd) $@ ;\
          else \
          echo "$@ XML validation not run. Please install the xmllint tool." ;\
  fi

I believe we should make refpolicy depend on host-libxml2 and force it
to use the Buildroot version of xmllint by setting XMLLINT in the
configuration step.

Do the following fixes the issue[1] on your side?

  diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
  index 1180f0d38bae..ecd8cf226b45 100644
  --- a/package/refpolicy/refpolicy.mk
  +++ b/package/refpolicy/refpolicy.mk
  @@ -14,7 +14,8 @@ REFPOLICY_DEPENDENCIES = \
          host-policycoreutils \
          host-python3 \
          host-setools \
  -       host-gawk
  +       host-gawk \
  +       host-libxml2

   ifeq ($(BR2_PACKAGE_REFPOLICY_CUSTOM_GIT),y)
   REFPOLICY_VERSION = $(call qstrip,$(BR2_PACKAGE_REFPOLICY_CUSTOM_REPO_VERSION))
  @@ -30,6 +31,7 @@ endif
   # Cannot use multiple threads to build the reference policy
   REFPOLICY_MAKE = \
          PYTHON=$(HOST_DIR)/usr/bin/python3 \
  +       XMLLINT=$(LIBXML2_HOST_BINARY) \
          TEST_TOOLCHAIN=$(HOST_DIR) \
          $(TARGET_MAKE_ENV) \
          $(MAKE1)

(I also checked for other `test -x` conditions in the refpolicy
Makefile; xmllint seems to be the only one).

[1] "fix the issue" aka throw an error while adding modules without a
    summary.

Thanks,
Antoine
Antoine Tenart Sept. 23, 2021, 8:33 a.m. UTC | #17
Quoting Antoine Tenart (2021-09-23 09:59:46)
> Quoting José Pekkarinen (2021-09-23 08:26:02)
> >  On Wed, Sep 22, 2021 at 5:23 PM Antoine Tenart <[1]atenart@kernel.org>
> >  wrote:
> >
> >    However I'm surprised as my understanding was the summary was required
> >    for the refpolicy configuration step to succeed (I did use a summary
> >    for all my tests because of this). When removing a summary from a module
> >    I always get the following error, and the Buildroot build stops.
> >
> >      doc/policy.xml:8376: element module: validity error : Element module
> >    content does not follow the DTD, expecting (summary , desc? , required?
> >    , (interface | template)* , (bool | tunable)*), got ()
> >      Document doc/policy.xml does not validate against doc/policy.dtd
> >
> >    Do you have an idea what made your build to succeed even though you did
> >    not have a summary in your module?
> >
> >  I believe it is validating to the summary prior to the module,
> >  the one you put in metadata.xml, but not any internal summary for
> >  the interface. This is how policy.xml looks like in a case where I didn't
> >  apply the mitigation:
> >  <layer name="buildroot">
> >  <summary>Buildroot extra modules</summary>
> >  <module name="base" filename="policy/modules/buildroot/base.if">
> >  </module>
> >  <module name="secure" filename="policy/modules/buildroot/secure.if">
> >  </module>
> >  </layer>
> >
> >  With this the modules.conf comes as:
> >
> >  # Layer: buildroot
> >  # Module: base
> >  #
> >  # Layer: buildroot
> >  # Module: secure
> >  #
> >
> >  There is a summary followed by a module, validation pass, but
> >
> >  the module is not built. If I add the following lines in the build folder
> >  modules[1]
> >  and run make.conf:
> >  [1] refpolicy-2.20200818/policy/modules/buildroot/secure.if: ##
> >  <summary>External secure module.</summary>
> >  refpolicy-2.20200818/policy/modules/buildroot/base.if: ##
> >  <summary>External base module.</summary>
> >
> >  The policy.xml looks like:
> >
> >  <layer name="buildroot">
> >  <summary>Buildroot extra modules</summary>
> >  <module name="base" filename="policy/modules/buildroot/base.if">
> >  <summary>External base modules.</summary>
> >  </module>
> >  <module name="secure" filename="policy/modules/buildroot/secure.if">
> >  <summary>External secure os vm module.</summary>
> >  </module>
> >  </layer>
> >
> >  Then policy/modules.conf looks this way:
> >
> >  # Layer: buildroot
> >  # Module: base
> >  #
> >  # External base modules.
> >  #  
> >  base = module
> >
> >  # Layer: buildroot
> >  # Module: secure
> >  #
> >  # External secure os vm module.
> >  #  
> >  secure = module
> >
> >  And this produces the modules to get into the policy.32 file.
> >  Does it makes any sense on your end?
> 
> The above does not reproduce for me. But I might know what's going on:
> do you have xmllint installed on your machine?

Or not at /usr/bin/xmllint

> If not, the validation step is skipped but the build is not stopped,
> which would explain the difference in behaviour we have between our
> tests:
> 
>   Makefile:453:
>   $(verbose) if test -x $(XMLLINT) && test -f $(xmldtd); then \
>           $(XMLLINT) --noout --path $(dir $(xmldtd)) --dtdvalid $(xmldtd) $@ ;\
>           else \
>           echo "$@ XML validation not run. Please install the xmllint tool." ;\
>   fi
> 
> I believe we should make refpolicy depend on host-libxml2 and force it
> to use the Buildroot version of xmllint by setting XMLLINT in the
> configuration step.
> 
> Do the following fixes the issue[1] on your side?
> 
>   diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
>   index 1180f0d38bae..ecd8cf226b45 100644
>   --- a/package/refpolicy/refpolicy.mk
>   +++ b/package/refpolicy/refpolicy.mk
>   @@ -14,7 +14,8 @@ REFPOLICY_DEPENDENCIES = \
>           host-policycoreutils \
>           host-python3 \
>           host-setools \
>   -       host-gawk
>   +       host-gawk \
>   +       host-libxml2
> 
>    ifeq ($(BR2_PACKAGE_REFPOLICY_CUSTOM_GIT),y)
>    REFPOLICY_VERSION = $(call qstrip,$(BR2_PACKAGE_REFPOLICY_CUSTOM_REPO_VERSION))
>   @@ -30,6 +31,7 @@ endif
>    # Cannot use multiple threads to build the reference policy
>    REFPOLICY_MAKE = \
>           PYTHON=$(HOST_DIR)/usr/bin/python3 \
>   +       XMLLINT=$(LIBXML2_HOST_BINARY) \
>           TEST_TOOLCHAIN=$(HOST_DIR) \
>           $(TARGET_MAKE_ENV) \
>           $(MAKE1)
> 
> (I also checked for other `test -x` conditions in the refpolicy
> Makefile; xmllint seems to be the only one).
> 
> [1] "fix the issue" aka throw an error while adding modules without a
>     summary.
> 
> Thanks,
> Antoine
>
José Pekkarinen Sept. 23, 2021, 8:47 a.m. UTC | #18
On Thu, Sep 23, 2021 at 11:33 AM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting Antoine Tenart (2021-09-23 09:59:46)
> > Quoting José Pekkarinen (2021-09-23 08:26:02)
> > >  On Wed, Sep 22, 2021 at 5:23 PM Antoine Tenart <[1]atenart@kernel.org
> >
> > >  wrote:
> > >
> > >    However I'm surprised as my understanding was the summary was
> required
> > >    for the refpolicy configuration step to succeed (I did use a summary
> > >    for all my tests because of this). When removing a summary from a
> module
> > >    I always get the following error, and the Buildroot build stops.
> > >
> > >      doc/policy.xml:8376: element module: validity error : Element
> module
> > >    content does not follow the DTD, expecting (summary , desc? ,
> required?
> > >    , (interface | template)* , (bool | tunable)*), got ()
> > >      Document doc/policy.xml does not validate against doc/policy.dtd
> > >
> > >    Do you have an idea what made your build to succeed even though you
> did
> > >    not have a summary in your module?
> > >
> > >  I believe it is validating to the summary prior to the module,
> > >  the one you put in metadata.xml, but not any internal summary for
> > >  the interface. This is how policy.xml looks like in a case where I
> didn't
> > >  apply the mitigation:
> > >  <layer name="buildroot">
> > >  <summary>Buildroot extra modules</summary>
> > >  <module name="base" filename="policy/modules/buildroot/base.if">
> > >  </module>
> > >  <module name="secure" filename="policy/modules/buildroot/secure.if">
> > >  </module>
> > >  </layer>
> > >
> > >  With this the modules.conf comes as:
> > >
> > >  # Layer: buildroot
> > >  # Module: base
> > >  #
> > >  # Layer: buildroot
> > >  # Module: secure
> > >  #
> > >
> > >  There is a summary followed by a module, validation pass, but
> > >
> > >  the module is not built. If I add the following lines in the build
> folder
> > >  modules[1]
> > >  and run make.conf:
> > >  [1] refpolicy-2.20200818/policy/modules/buildroot/secure.if: ##
> > >  <summary>External secure module.</summary>
> > >  refpolicy-2.20200818/policy/modules/buildroot/base.if: ##
> > >  <summary>External base module.</summary>
> > >
> > >  The policy.xml looks like:
> > >
> > >  <layer name="buildroot">
> > >  <summary>Buildroot extra modules</summary>
> > >  <module name="base" filename="policy/modules/buildroot/base.if">
> > >  <summary>External base modules.</summary>
> > >  </module>
> > >  <module name="secure" filename="policy/modules/buildroot/secure.if">
> > >  <summary>External secure os vm module.</summary>
> > >  </module>
> > >  </layer>
> > >
> > >  Then policy/modules.conf looks this way:
> > >
> > >  # Layer: buildroot
> > >  # Module: base
> > >  #
> > >  # External base modules.
> > >  #
> > >  base = module
> > >
> > >  # Layer: buildroot
> > >  # Module: secure
> > >  #
> > >  # External secure os vm module.
> > >  #
> > >  secure = module
> > >
> > >  And this produces the modules to get into the policy.32 file.
> > >  Does it makes any sense on your end?
> >
> > The above does not reproduce for me. But I might know what's going on:
> > do you have xmllint installed on your machine?
>
> Or not at /usr/bin/xmllint
>

It was built in a container without it, I'm testing the patch, bear

for a bit.

José.



> > If not, the validation step is skipped but the build is not stopped,
> > which would explain the difference in behaviour we have between our
> > tests:
> >
> >   Makefile:453:
> >   $(verbose) if test -x $(XMLLINT) && test -f $(xmldtd); then \
> >           $(XMLLINT) --noout --path $(dir $(xmldtd)) --dtdvalid
> $(xmldtd) $@ ;\
> >           else \
> >           echo "$@ XML validation not run. Please install the xmllint
> tool." ;\
> >   fi
> >
> > I believe we should make refpolicy depend on host-libxml2 and force it
> > to use the Buildroot version of xmllint by setting XMLLINT in the
> > configuration step.
> >
> > Do the following fixes the issue[1] on your side?
> >
> >   diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/
> refpolicy.mk
> >   index 1180f0d38bae..ecd8cf226b45 100644
> >   --- a/package/refpolicy/refpolicy.mk
> >   +++ b/package/refpolicy/refpolicy.mk
> >   @@ -14,7 +14,8 @@ REFPOLICY_DEPENDENCIES = \
> >           host-policycoreutils \
> >           host-python3 \
> >           host-setools \
> >   -       host-gawk
> >   +       host-gawk \
> >   +       host-libxml2
> >
> >    ifeq ($(BR2_PACKAGE_REFPOLICY_CUSTOM_GIT),y)
> >    REFPOLICY_VERSION = $(call
> qstrip,$(BR2_PACKAGE_REFPOLICY_CUSTOM_REPO_VERSION))
> >   @@ -30,6 +31,7 @@ endif
> >    # Cannot use multiple threads to build the reference policy
> >    REFPOLICY_MAKE = \
> >           PYTHON=$(HOST_DIR)/usr/bin/python3 \
> >   +       XMLLINT=$(LIBXML2_HOST_BINARY) \
> >           TEST_TOOLCHAIN=$(HOST_DIR) \
> >           $(TARGET_MAKE_ENV) \
> >           $(MAKE1)
> >
> > (I also checked for other `test -x` conditions in the refpolicy
> > Makefile; xmllint seems to be the only one).
> >
> > [1] "fix the issue" aka throw an error while adding modules without a
> >     summary.
> >
> > Thanks,
> > Antoine
> >
>
José Pekkarinen Sept. 23, 2021, 9:08 a.m. UTC | #19
On Thu, Sep 23, 2021 at 10:59 AM Antoine Tenart <atenart@kernel.org> wrote:

> Quoting José Pekkarinen (2021-09-23 08:26:02)
> >  On Wed, Sep 22, 2021 at 5:23 PM Antoine Tenart <[1]atenart@kernel.org>
> >  wrote:
> >
> >    However I'm surprised as my understanding was the summary was required
> >    for the refpolicy configuration step to succeed (I did use a summary
> >    for all my tests because of this). When removing a summary from a
> module
> >    I always get the following error, and the Buildroot build stops.
> >
> >      doc/policy.xml:8376: element module: validity error : Element module
> >    content does not follow the DTD, expecting (summary , desc? ,
> required?
> >    , (interface | template)* , (bool | tunable)*), got ()
> >      Document doc/policy.xml does not validate against doc/policy.dtd
> >
> >    Do you have an idea what made your build to succeed even though you
> did
> >    not have a summary in your module?
> >
> >  I believe it is validating to the summary prior to the module,
> >  the one you put in metadata.xml, but not any internal summary for
> >  the interface. This is how policy.xml looks like in a case where I
> didn't
> >  apply the mitigation:
> >  <layer name="buildroot">
> >  <summary>Buildroot extra modules</summary>
> >  <module name="base" filename="policy/modules/buildroot/base.if">
> >  </module>
> >  <module name="secure" filename="policy/modules/buildroot/secure.if">
> >  </module>
> >  </layer>
> >
> >  With this the modules.conf comes as:
> >
> >  # Layer: buildroot
> >  # Module: base
> >  #
> >  # Layer: buildroot
> >  # Module: secure
> >  #
> >
> >  There is a summary followed by a module, validation pass, but
> >
> >  the module is not built. If I add the following lines in the build
> folder
> >  modules[1]
> >  and run make.conf:
> >  [1] refpolicy-2.20200818/policy/modules/buildroot/secure.if: ##
> >  <summary>External secure module.</summary>
> >  refpolicy-2.20200818/policy/modules/buildroot/base.if: ##
> >  <summary>External base module.</summary>
> >
> >  The policy.xml looks like:
> >
> >  <layer name="buildroot">
> >  <summary>Buildroot extra modules</summary>
> >  <module name="base" filename="policy/modules/buildroot/base.if">
> >  <summary>External base modules.</summary>
> >  </module>
> >  <module name="secure" filename="policy/modules/buildroot/secure.if">
> >  <summary>External secure os vm module.</summary>
> >  </module>
> >  </layer>
> >
> >  Then policy/modules.conf looks this way:
> >
> >  # Layer: buildroot
> >  # Module: base
> >  #
> >  # External base modules.
> >  #
> >  base = module
> >
> >  # Layer: buildroot
> >  # Module: secure
> >  #
> >  # External secure os vm module.
> >  #
> >  secure = module
> >
> >  And this produces the modules to get into the policy.32 file.
> >  Does it makes any sense on your end?
>
> The above does not reproduce for me. But I might know what's going on:
> do you have xmllint installed on your machine?
>
> If not, the validation step is skipped but the build is not stopped,
> which would explain the difference in behaviour we have between our
> tests:
>
>   Makefile:453:
>   $(verbose) if test -x $(XMLLINT) && test -f $(xmldtd); then \
>           $(XMLLINT) --noout --path $(dir $(xmldtd)) --dtdvalid $(xmldtd)
> $@ ;\
>           else \
>           echo "$@ XML validation not run. Please install the xmllint
> tool." ;\
>   fi
>
> I believe we should make refpolicy depend on host-libxml2 and force it
> to use the Buildroot version of xmllint by setting XMLLINT in the
> configuration step.
>
> Do the following fixes the issue[1] on your side?
>
>   diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/
> refpolicy.mk
>   index 1180f0d38bae..ecd8cf226b45 100644
>   --- a/package/refpolicy/refpolicy.mk
>   +++ b/package/refpolicy/refpolicy.mk
>   @@ -14,7 +14,8 @@ REFPOLICY_DEPENDENCIES = \
>           host-policycoreutils \
>           host-python3 \
>           host-setools \
>   -       host-gawk
>   +       host-gawk \
>   +       host-libxml2
>
>    ifeq ($(BR2_PACKAGE_REFPOLICY_CUSTOM_GIT),y)
>    REFPOLICY_VERSION = $(call
> qstrip,$(BR2_PACKAGE_REFPOLICY_CUSTOM_REPO_VERSION))
>   @@ -30,6 +31,7 @@ endif
>    # Cannot use multiple threads to build the reference policy
>    REFPOLICY_MAKE = \
>           PYTHON=$(HOST_DIR)/usr/bin/python3 \
>   +       XMLLINT=$(LIBXML2_HOST_BINARY) \
>           TEST_TOOLCHAIN=$(HOST_DIR) \
>           $(TARGET_MAKE_ENV) \
>           $(MAKE1)
>
>
Confirmed, the patch *works*:


Creating policy.xml
echo '<?xml version="1.0" encoding="ISO-8859-1" standalone="no"?>' >
doc/policy.xml
echo '<!DOCTYPE policy SYSTEM "policy.dtd">' >> doc/policy.xml
echo '<policy>' >> doc/policy.xml
for i in admin apps buildroot kernel roles services system; do echo "<layer
name=\"$i\">" >> doc/policy.xml; cat doc/tmp/$i.xml >> doc/policy.xml; echo
"</layer>" >> doc/policy.xml; done
cat doc/global_tunables.xml doc/global_booleans.xml >> doc/policy.xml
echo '</policy>' >> doc/policy.xml
if test -x /output/br_admin/output_x86_qemu/host/bin/xmllint && test -f
doc/policy.dtd; then \
       /output/br_admin/output_x86_qemu/host/bin/xmllint --noout --path
doc/ --dtdvalid doc/policy.dtd doc/policy.xml ;\
       else \
       echo "doc/policy.xml XML validation not run. Please install the
xmllint tool." ;\
fi
doc/policy.xml:8373: element module: validity error : Element module
content does not follow the DTD, expecting (summary , desc? , required? ,
(interface | template)* , (bool | tunable)*), got ()
doc/policy.xml:8375: element module: validity error : Element module
content does not follow the DTD, expecting (summary , desc? , required? ,
(interface | template)* , (bool | tunable)*), got ()

Thanks!


José.
Antoine Tenart Sept. 23, 2021, 9:17 a.m. UTC | #20
Quoting José Pekkarinen (2021-09-23 11:08:28)
>  On Thu, Sep 23, 2021 at 10:59 AM Antoine Tenart <[1]atenart@kernel.org>
>  wrote:
>
>    Do the following fixes the issue[1] on your side?
>
>      diff --git a/package/refpolicy/[3]refpolicy.mk
>    b/package/refpolicy/[4]refpolicy.mk
>      index 1180f0d38bae..ecd8cf226b45 100644
>      --- a/package/refpolicy/[5]refpolicy.mk
>      +++ b/package/refpolicy/[6]refpolicy.mk
>      @@ -14,7 +14,8 @@ REFPOLICY_DEPENDENCIES = \
>              host-policycoreutils \
>              host-python3 \
>              host-setools \
>      -       host-gawk
>      +       host-gawk \
>      +       host-libxml2
>
>       ifeq ($(BR2_PACKAGE_REFPOLICY_CUSTOM_GIT),y)
>       REFPOLICY_VERSION = $(call
>    qstrip,$(BR2_PACKAGE_REFPOLICY_CUSTOM_REPO_VERSION))
>      @@ -30,6 +31,7 @@ endif
>       # Cannot use multiple threads to build the reference policy
>       REFPOLICY_MAKE = \
>              PYTHON=$(HOST_DIR)/usr/bin/python3 \
>      +       XMLLINT=$(LIBXML2_HOST_BINARY) \
>              TEST_TOOLCHAIN=$(HOST_DIR) \
>              $(TARGET_MAKE_ENV) \
>              $(MAKE1)
>
>  Confirmed, the patch *works*:

Great! I'll send this patch with your Tested-by then.

Thanks,
Antoine
diff mbox series

Patch

diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
index 0194708b37..1c0a2c3385 100644
--- a/package/refpolicy/refpolicy.mk
+++ b/package/refpolicy/refpolicy.mk
@@ -85,9 +85,9 @@  endef
 # In the context of a monolithic policy enabling a piece of the policy as
 # 'base' or 'module' is equivalent, so we enable them as 'base'.
 define REFPOLICY_CONFIGURE_MODULES
-	$(SED) "s/ = module/ = no/g" $(@D)/policy/modules.conf
+	$(SED) "/ = module/d" $(@D)/policy/modules.conf
 	$(foreach m,$(sort $(REFPOLICY_MODULES)),
-		$(SED) "/^$(m) =/c\$(m) = base" $(@D)/policy/modules.conf
+		$(SED) "/^# Module: $(m)/a\$(m) = base" $(@D)/policy/modules.conf
 	)
 endef