All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] package/refpolicy: Add option to disable "dontaudit" rules
@ 2021-01-28 12:52 Maxime Chevallier
  2021-01-28 14:24 ` Antoine Tenart
  2022-01-19 22:23 ` [Buildroot] [PATCH v3] " Giulio Benetti
  0 siblings, 2 replies; 14+ messages in thread
From: Maxime Chevallier @ 2021-01-28 12:52 UTC (permalink / raw)
  To: buildroot

Some rules in the refpolicy are declared with "dontaudit", effectively
suppressing any AVC violation log, while still denying the actions.

This is useful in some cases, where denied actions are to be expected
but won't prevent the system from operating.

However in some other cases, the suppressed logs are important to
troubleshoot some issues.

Disabling the "dontaudit" rules can be done either from the running
system by rebuilding the policy with "semodules -DB", or when initialy
building the policy by using the "enableaudit" make target.

This commit allows building the refpolicy with the "enableaudit" target
prior to installing it, thanks to a dedicated config option.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
v1 -> v2: Use POST_BUILD_HOOKS to summon make enableaudit, as per
Antoine Tenart adnd Thomas petazzoni's reviews

 package/refpolicy/Config.in    | 14 ++++++++++++++
 package/refpolicy/refpolicy.mk |  8 ++++++++
 2 files changed, 22 insertions(+)

diff --git a/package/refpolicy/Config.in b/package/refpolicy/Config.in
index c529b85e1d..d6e195e8f8 100644
--- a/package/refpolicy/Config.in
+++ b/package/refpolicy/Config.in
@@ -111,6 +111,20 @@ config BR2_REFPOLICY_EXTRA_MODULES
 
 endif
 
+config BR2_REFPOLICY_DISABLE_DONTAUDIT
+	bool "Disable dontaudit"
+	help
+	  Builds the refpolicy with the "dontaudit" rules disabled.
+	  This will trigger unseen, and probably unharmful audit logs that are
+	  explicitely silenced otherwise. This option can be helpful for
+	  debugging purposes, should a silenced message cause a real issue
+	  that would otherwise be hard to troubleshoot.
+
+	  This option should be used for debugging purposes only, due to
+	  the amount of avc logs it generates.
+
+	  If unsure, select n.
+
 endif
 
 comment "refpolicy needs a toolchain w/ threads"
diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
index 0194708b37..4dfa9e914a 100644
--- a/package/refpolicy/refpolicy.mk
+++ b/package/refpolicy/refpolicy.mk
@@ -116,6 +116,14 @@ define REFPOLICY_BUILD_CMDS
 	$(REFPOLICY_MAKE) -C $(@D) policy
 endef
 
+ifeq ($(BR2_REFPOLICY_DISABLE_DONTAUDIT),y)
+define REFPOLICY_DISABLE_DONTAUDIT_CMDS
+	$(REFPOLICY_MAKE) -C $(@D) enableaudit
+endef
+endif
+
+REFPOLICY_POST_BUILD_HOOKS += REFPOLICY_DISABLE_DONTAUDIT_CMDS
+
 define REFPOLICY_INSTALL_STAGING_CMDS
 	$(REFPOLICY_MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) \
 		install-src install-headers
-- 
2.25.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] package/refpolicy: Add option to disable "dontaudit" rules
  2021-01-28 12:52 [Buildroot] [PATCH v2] package/refpolicy: Add option to disable "dontaudit" rules Maxime Chevallier
@ 2021-01-28 14:24 ` Antoine Tenart
  2022-01-19 22:23 ` [Buildroot] [PATCH v3] " Giulio Benetti
  1 sibling, 0 replies; 14+ messages in thread
From: Antoine Tenart @ 2021-01-28 14:24 UTC (permalink / raw)
  To: buildroot

Quoting Maxime Chevallier (2021-01-28 13:52:56)
> --- a/package/refpolicy/refpolicy.mk
> +++ b/package/refpolicy/refpolicy.mk
> @@ -116,6 +116,14 @@ define REFPOLICY_BUILD_CMDS
>         $(REFPOLICY_MAKE) -C $(@D) policy
>  endef
>  
> +ifeq ($(BR2_REFPOLICY_DISABLE_DONTAUDIT),y)
> +define REFPOLICY_DISABLE_DONTAUDIT_CMDS
> +       $(REFPOLICY_MAKE) -C $(@D) enableaudit
> +endef
> +endif
> +
> +REFPOLICY_POST_BUILD_HOOKS += REFPOLICY_DISABLE_DONTAUDIT_CMDS

I think this could be in the ifeq block as well.

Otherwise, LGTM.

Thanks!
Antoine

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2021-01-28 12:52 [Buildroot] [PATCH v2] package/refpolicy: Add option to disable "dontaudit" rules Maxime Chevallier
  2021-01-28 14:24 ` Antoine Tenart
@ 2022-01-19 22:23 ` Giulio Benetti
  2022-01-19 22:39   ` Thomas Petazzoni
  1 sibling, 1 reply; 14+ messages in thread
From: Giulio Benetti @ 2022-01-19 22:23 UTC (permalink / raw)
  To: buildroot
  Cc: Antoine Tenart, Giulio Benetti, Thomas Petazzoni, Maxime Chevallier

From: Maxime Chevallier <maxime.chevallier@bootlin.com>

Some rules in the refpolicy are declared with "dontaudit", effectively
suppressing any AVC violation log, while still denying the actions.

This is useful in some cases, where denied actions are to be expected
but won't prevent the system from operating.

However in some other cases, the suppressed logs are important to
troubleshoot some issues.

Disabling the "dontaudit" rules can be done either from the running
system by rebuilding the policy with "semodules -DB", or when initialy
building the policy by using the "enableaudit" make target.

This commit allows building the refpolicy with the "enableaudit" target
prior to installing it, thanks to a dedicated config option.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
[Giulio: moved REFPOLICY_POST_BUILD_HOOKS inside ifeq/endef]
---
Maxime:
V1->V2:
* Use POST_BUILD_HOOKS to summon make enableaudit, as per Antoine Tenart
and Thomas petazzoni's reviews
Giulio:
V2->V3:
* moved REFPOLICY_POST_BUILD_HOOKS into ifeq/endef as suggested by Antoine
Tenart

NOTE: this patch superseeds V2:
https://patchwork.ozlabs.org/project/buildroot/patch/20210128125256.1419587-1-maxime.chevallier@bootlin.com/
---
 package/refpolicy/Config.in    | 14 ++++++++++++++
 package/refpolicy/refpolicy.mk |  7 +++++++
 2 files changed, 21 insertions(+)

diff --git a/package/refpolicy/Config.in b/package/refpolicy/Config.in
index 0e72b895df..caba147feb 100644
--- a/package/refpolicy/Config.in
+++ b/package/refpolicy/Config.in
@@ -113,6 +113,20 @@ config BR2_REFPOLICY_EXTRA_MODULES
 
 endif
 
+config BR2_REFPOLICY_DISABLE_DONTAUDIT
+	bool "Disable dontaudit"
+	help
+	  Builds the refpolicy with the "dontaudit" rules disabled.
+	  This will trigger unseen, and probably unharmful audit logs that are
+	  explicitely silenced otherwise. This option can be helpful for
+	  debugging purposes, should a silenced message cause a real issue
+	  that would otherwise be hard to troubleshoot.
+
+	  This option should be used for debugging purposes only, due to
+	  the amount of avc logs it generates.
+
+	  If unsure, select n.
+
 endif
 
 comment "refpolicy needs a toolchain w/ threads, gcc >= 5, host gcc >= 5"
diff --git a/package/refpolicy/refpolicy.mk b/package/refpolicy/refpolicy.mk
index 44c50af278..e113c3496e 100644
--- a/package/refpolicy/refpolicy.mk
+++ b/package/refpolicy/refpolicy.mk
@@ -118,6 +118,13 @@ define REFPOLICY_BUILD_CMDS
 	$(REFPOLICY_MAKE) -C $(@D) policy
 endef
 
+ifeq ($(BR2_REFPOLICY_DISABLE_DONTAUDIT),y)
+define REFPOLICY_DISABLE_DONTAUDIT_CMDS
+	$(REFPOLICY_MAKE) -C $(@D) enableaudit
+endef
+REFPOLICY_POST_BUILD_HOOKS += REFPOLICY_DISABLE_DONTAUDIT_CMDS
+endif
+
 define REFPOLICY_INSTALL_STAGING_CMDS
 	$(REFPOLICY_MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) \
 		install-src install-headers
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-19 22:23 ` [Buildroot] [PATCH v3] " Giulio Benetti
@ 2022-01-19 22:39   ` Thomas Petazzoni
  2022-01-19 23:56     ` Giulio Benetti
  2022-01-20  7:48     ` Maxime Chevallier
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2022-01-19 22:39 UTC (permalink / raw)
  To: Giulio Benetti; +Cc: Antoine Tenart, Maxime Chevallier, buildroot

On Wed, 19 Jan 2022 23:23:32 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> +config BR2_REFPOLICY_DISABLE_DONTAUDIT
> +	bool "Disable dontaudit"

I am still extremely confused by the name of option, with its double
negative.

When enabled, this option will disable something that doesn't audit.
Meh.

Is it possible to find a better name / description that doesn't make
one's brain segfault when trying to understand what it does ?

The make target that gets triggered is "enableaudit". Would it make
sense to call this option BR2_PACKAGE_REFPOLICY_ENABLE_AUDIT ?

It would be nice to get the feedback from Antoine and/or Maxime on this.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-19 22:39   ` Thomas Petazzoni
@ 2022-01-19 23:56     ` Giulio Benetti
  2022-01-20  7:48     ` Maxime Chevallier
  1 sibling, 0 replies; 14+ messages in thread
From: Giulio Benetti @ 2022-01-19 23:56 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: Antoine Tenart, buildroot, Maxime Chevallier

On 19/01/22 23:39, Thomas Petazzoni wrote:
> On Wed, 19 Jan 2022 23:23:32 +0100
> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> 
>> +config BR2_REFPOLICY_DISABLE_DONTAUDIT
>> +	bool "Disable dontaudit"
> 
> I am still extremely confused by the name of option, with its double
> negative.
> 
> When enabled, this option will disable something that doesn't audit.
> Meh.
> 
> Is it possible to find a better name / description that doesn't make
> one's brain segfault when trying to understand what it does ?
> 
> The make target that gets triggered is "enableaudit". Would it make
> sense to call this option BR2_PACKAGE_REFPOLICY_ENABLE_AUDIT ?

I didn't check well the make argument..... Sorry. I agree with Thomas on 
changing the option variable to BR2_PACKAGE_REFPOLICY_ENABLE_AUDIT and 
name to:
"Enable audit"

And I would change the help section from:
```
Builds the refpolicy with the "dontaudit" rules disabled.
This will trigger unseen, and ...
```
to:
```
Remove all dontaudit rules from policy.conf.
This will trigger unseen, and ...
```
as reported in refpolicy README:
https://github.com/SELinuxProject/refpolicy/blob/master/README#L78

> It would be nice to get the feedback from Antoine and/or Maxime on this.

Following this since I'm not used to this package.

Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-19 22:39   ` Thomas Petazzoni
  2022-01-19 23:56     ` Giulio Benetti
@ 2022-01-20  7:48     ` Maxime Chevallier
  2022-01-20  9:29       ` Antoine Tenart
  1 sibling, 1 reply; 14+ messages in thread
From: Maxime Chevallier @ 2022-01-20  7:48 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: Giulio Benetti, Antoine Tenart, buildroot

Hello Thomas,

On Wed, 19 Jan 2022 23:39:44 +0100
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

>On Wed, 19 Jan 2022 23:23:32 +0100
>Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
>
>> +config BR2_REFPOLICY_DISABLE_DONTAUDIT
>> +	bool "Disable dontaudit"  
>
>I am still extremely confused by the name of option, with its double
>negative.
>
>When enabled, this option will disable something that doesn't audit.
>Meh.

I agree about the confusing double-negative, but it follows the SELinux
terminology from the rules syntax. My personal view is that the "make
enableaudit" target is a bit confusing already :)

>Is it possible to find a better name / description that doesn't make
>one's brain segfault when trying to understand what it does ?

Maybe we can think of an option name like
"BR2_REFPOLICY_VERBOSE_DONTAUDIT", suggesting that we're not silencing
these 'dontaudit' rules anymore ? The only actual effect is what gets
printed in the AVC logs.

>The make target that gets triggered is "enableaudit". Would it make
>sense to call this option BR2_PACKAGE_REFPOLICY_ENABLE_AUDIT ?

The more I think about that, the more I think that using
"enable/disable" here is misleading, the behaviour stays the same with
regard to what gets denied/allow, only the logs are going to change.

Thanks,

Maxime

>It would be nice to get the feedback from Antoine and/or Maxime on this.
>
>Thomas



-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-20  7:48     ` Maxime Chevallier
@ 2022-01-20  9:29       ` Antoine Tenart
  2022-01-23 22:21         ` Giulio Benetti
  0 siblings, 1 reply; 14+ messages in thread
From: Antoine Tenart @ 2022-01-20  9:29 UTC (permalink / raw)
  To: Maxime Chevallier, Thomas Petazzoni; +Cc: Giulio Benetti, buildroot

Quoting Maxime Chevallier (2022-01-20 08:48:04)
> On Wed, 19 Jan 2022 23:39:44 +0100
> Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> >On Wed, 19 Jan 2022 23:23:32 +0100
> >Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> >
> >> +config BR2_REFPOLICY_DISABLE_DONTAUDIT
> >> +    bool "Disable dontaudit"  
> >
> >I am still extremely confused by the name of option, with its double
> >negative.
> >
> >When enabled, this option will disable something that doesn't audit.
> >Meh.
> 
> I agree about the confusing double-negative, but it follows the SELinux
> terminology from the rules syntax. My personal view is that the "make
> enableaudit" target is a bit confusing already :)

Agreed.

> >Is it possible to find a better name / description that doesn't make
> >one's brain segfault when trying to understand what it does ?
> 
> Maybe we can think of an option name like
> "BR2_REFPOLICY_VERBOSE_DONTAUDIT", suggesting that we're not silencing
> these 'dontaudit' rules anymore ? The only actual effect is what gets
> printed in the AVC logs.
> 
> >The make target that gets triggered is "enableaudit". Would it make
> >sense to call this option BR2_PACKAGE_REFPOLICY_ENABLE_AUDIT ?
> 
> The more I think about that, the more I think that using
> "enable/disable" here is misleading, the behaviour stays the same with
> regard to what gets denied/allow, only the logs are going to change.

I would suggest using BR2_PACKAGE_REFPOLICY_WITH_DONTAUDIT, defaulting
to y (using _WITHOUT_ would be less clear IMHO).

And in Kconfig something along the lines:

  bool "Build with dontaudit rules"
  default y
  help
    The refpolicy comes with 'dontaudit' rules suppressing audit logs
    for known and expected violations of the policy. These rules are
    enabled by default. Building without the 'dontaudit' rules will lead
    in more denied actions being logged, which can be useful for
    debugging purposes. Note that running 'semodules -DB' from a running
    system will have the same effect.

    Say y if unsure.

(Please check the semodules part, never tried it).

WDYT?

Antoine
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-20  9:29       ` Antoine Tenart
@ 2022-01-23 22:21         ` Giulio Benetti
  2022-01-24  8:44           ` Antoine Tenart
  0 siblings, 1 reply; 14+ messages in thread
From: Giulio Benetti @ 2022-01-23 22:21 UTC (permalink / raw)
  To: Antoine Tenart, Maxime Chevallier, Thomas Petazzoni; +Cc: buildroot

Hi Antoine, Maxime, Thomas, All,

On 20/01/22 10:29, Antoine Tenart wrote:
> Quoting Maxime Chevallier (2022-01-20 08:48:04)
>> On Wed, 19 Jan 2022 23:39:44 +0100
>> Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
>>> On Wed, 19 Jan 2022 23:23:32 +0100
>>> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
>>>
>>>> +config BR2_REFPOLICY_DISABLE_DONTAUDIT
>>>> +    bool "Disable dontaudit"
>>>
>>> I am still extremely confused by the name of option, with its double
>>> negative.
>>>
>>> When enabled, this option will disable something that doesn't audit.
>>> Meh.
>>
>> I agree about the confusing double-negative, but it follows the SELinux
>> terminology from the rules syntax. My personal view is that the "make
>> enableaudit" target is a bit confusing already :)
> 
> Agreed.
> 
>>> Is it possible to find a better name / description that doesn't make
>>> one's brain segfault when trying to understand what it does ?
>>
>> Maybe we can think of an option name like
>> "BR2_REFPOLICY_VERBOSE_DONTAUDIT", suggesting that we're not silencing
>> these 'dontaudit' rules anymore ? The only actual effect is what gets
>> printed in the AVC logs.
>>
>>> The make target that gets triggered is "enableaudit". Would it make
>>> sense to call this option BR2_PACKAGE_REFPOLICY_ENABLE_AUDIT ?
>>
>> The more I think about that, the more I think that using
>> "enable/disable" here is misleading, the behaviour stays the same with
>> regard to what gets denied/allow, only the logs are going to change.
> 
> I would suggest using BR2_PACKAGE_REFPOLICY_WITH_DONTAUDIT, defaulting
> to y (using _WITHOUT_ would be less clear IMHO).
> 
> And in Kconfig something along the lines:
> 
>    bool "Build with dontaudit rules"
>    default y
>    help
>      The refpolicy comes with 'dontaudit' rules suppressing audit logs
>      for known and expected violations of the policy. These rules are
>      enabled by default. Building without the 'dontaudit' rules will lead
>      in more denied actions being logged, which can be useful for
>      debugging purposes. Note that running 'semodules -DB' from a running
>      system will have the same effect.
> 
>      Say y if unsure.
> 
> (Please check the semodules part, never tried it).
> 
> WDYT?

Re-reading this after some day it looks to me like the simple
BR2_PACKAGE_REFPOLICY_ENABLEAUDIT is correct. Because it "Removes all 
dontaudit rules from policy.conf", so it's true that it enables audit 
logs in the end. It's a very specific options and the one who will use 
it will know what he will enable and most of all we can specify in the 
help section that:
"Removes all dontaudit rules from policy.conf"

It changes the perspective we are looking at it, but in the end, it 
"Removes all dontaudit rules from policy.conf" and subsequentially this 
leads to show the audits logs. So it enables audits.

Does it work for you?

Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-23 22:21         ` Giulio Benetti
@ 2022-01-24  8:44           ` Antoine Tenart
  2022-01-24  8:59             ` Giulio Benetti
  0 siblings, 1 reply; 14+ messages in thread
From: Antoine Tenart @ 2022-01-24  8:44 UTC (permalink / raw)
  To: Giulio Benetti, Maxime Chevallier, Thomas Petazzoni; +Cc: buildroot

Quoting Giulio Benetti (2022-01-23 23:21:50)
> 
> Re-reading this after some day it looks to me like the simple
> BR2_PACKAGE_REFPOLICY_ENABLEAUDIT is correct. Because it "Removes all 
> dontaudit rules from policy.conf", so it's true that it enables audit 
> logs in the end. It's a very specific options and the one who will use 
> it will know what he will enable and most of all we can specify in the 
> help section that:
> "Removes all dontaudit rules from policy.conf"

> Does it work for you?

As long as the help text is clear enough for the user, either config
option name should be acceptable. Using the above is good to have an 1:1
mapping with the target name, at the expense of having the help text not
matching it and an unclear option name from the user perspective (it's a
bit weird not to enable it, at first). For _WITH_DONTAUDIT_, the
opposite is true. I'd say there is no perfect solution, we can either
stay close to the refpolicy internals or have the Buildroot specifics
more consistent; not sure which is better.

Thanks,
Antoine
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-24  8:44           ` Antoine Tenart
@ 2022-01-24  8:59             ` Giulio Benetti
  2022-01-24  9:06               ` Antoine Tenart
  0 siblings, 1 reply; 14+ messages in thread
From: Giulio Benetti @ 2022-01-24  8:59 UTC (permalink / raw)
  To: Antoine Tenart; +Cc: buildroot, Thomas Petazzoni, Maxime Chevallier


> Il giorno 24 gen 2022, alle ore 09:45, Antoine Tenart <atenart@kernel.org> ha scritto:
> 
> Quoting Giulio Benetti (2022-01-23 23:21:50)
>> 
>> Re-reading this after some day it looks to me like the simple
>> BR2_PACKAGE_REFPOLICY_ENABLEAUDIT is correct. Because it "Removes all 
>> dontaudit rules from policy.conf", so it's true that it enables audit 
>> logs in the end. It's a very specific options and the one who will use 
>> it will know what he will enable and most of all we can specify in the 
>> help section that:
>> "Removes all dontaudit rules from policy.conf"
> 
>> Does it work for you?
> 
> As long as the help text is clear enough for the user, either config
> option name should be acceptable. Using the above is good to have an 1:1
> mapping with the target name, at the expense of having the help text not
> matching it and an unclear option name from the user perspective (it's a
> bit weird not to enable it, at first). For _WITH_DONTAUDIT_, the
> opposite is true. I'd say there is no perfect solution, we can either
> stay close to the refpolicy internals or have the Buildroot specifics
> more consistent; not sure which is better.

I’d go for Buildroot consistency since we’re in Buildroot. If someone enables refpolicy package and enables “Enable audit”, I expect to see audit log. Right?

This is me at least that I’ve never used refpolicy so I’m agnostic in the topic, so maybe a neutral “use case“ :-)

Best regards
Giulio

> 
> Thanks,
> Antoine
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-24  8:59             ` Giulio Benetti
@ 2022-01-24  9:06               ` Antoine Tenart
  2022-01-24  9:20                 ` Giulio Benetti
  0 siblings, 1 reply; 14+ messages in thread
From: Antoine Tenart @ 2022-01-24  9:06 UTC (permalink / raw)
  To: Giulio Benetti; +Cc: Maxime Chevallier, Thomas Petazzoni, buildroot

Quoting Giulio Benetti (2022-01-24 09:59:45)
> 
> > Il giorno 24 gen 2022, alle ore 09:45, Antoine Tenart <atenart@kernel.org> ha scritto:
> > 
> > Quoting Giulio Benetti (2022-01-23 23:21:50)
> >> 
> >> Re-reading this after some day it looks to me like the simple
> >> BR2_PACKAGE_REFPOLICY_ENABLEAUDIT is correct. Because it "Removes all 
> >> dontaudit rules from policy.conf", so it's true that it enables audit 
> >> logs in the end. It's a very specific options and the one who will use 
> >> it will know what he will enable and most of all we can specify in the 
> >> help section that:
> >> "Removes all dontaudit rules from policy.conf"
> > 
> >> Does it work for you?
> > 
> > As long as the help text is clear enough for the user, either config
> > option name should be acceptable. Using the above is good to have an 1:1
> > mapping with the target name, at the expense of having the help text not
> > matching it and an unclear option name from the user perspective (it's a
> > bit weird not to enable it, at first). For _WITH_DONTAUDIT_, the
> > opposite is true. I'd say there is no perfect solution, we can either
> > stay close to the refpolicy internals or have the Buildroot specifics
> > more consistent; not sure which is better.
> 
> I’d go for Buildroot consistency since we’re in Buildroot.

So _WITH_DONTAUDIT_?

> If someone enables refpolicy package and enables “Enable audit”, I
> expect to see audit log. Right?

Extra audit log*

It's not about enabling audit logs, it's about having some known rules
to not be silenced in the audit logs as they're known to be problematic.

> This is me at least that I’ve never used refpolicy so I’m agnostic in
> the topic, so maybe a neutral “use case“ :-)

I don't really have a preference for either solution tbh.

Antoine
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-24  9:06               ` Antoine Tenart
@ 2022-01-24  9:20                 ` Giulio Benetti
  2022-01-24  9:29                   ` Antoine Tenart
  0 siblings, 1 reply; 14+ messages in thread
From: Giulio Benetti @ 2022-01-24  9:20 UTC (permalink / raw)
  To: Antoine Tenart; +Cc: Maxime Chevallier, Thomas Petazzoni, buildroot


> Il giorno 24 gen 2022, alle ore 10:07, Antoine Tenart <atenart@kernel.org> ha scritto:
> 
> Quoting Giulio Benetti (2022-01-24 09:59:45)
>> 
>>>> Il giorno 24 gen 2022, alle ore 09:45, Antoine Tenart <atenart@kernel.org> ha scritto:
>>> 
>>> Quoting Giulio Benetti (2022-01-23 23:21:50)
>>>> 
>>>> Re-reading this after some day it looks to me like the simple
>>>> BR2_PACKAGE_REFPOLICY_ENABLEAUDIT is correct. Because it "Removes all 
>>>> dontaudit rules from policy.conf", so it's true that it enables audit 
>>>> logs in the end. It's a very specific options and the one who will use 
>>>> it will know what he will enable and most of all we can specify in the 
>>>> help section that:
>>>> "Removes all dontaudit rules from policy.conf"
>>> 
>>>> Does it work for you?
>>> 
>>> As long as the help text is clear enough for the user, either config
>>> option name should be acceptable. Using the above is good to have an 1:1
>>> mapping with the target name, at the expense of having the help text not
>>> matching it and an unclear option name from the user perspective (it's a
>>> bit weird not to enable it, at first). For _WITH_DONTAUDIT_, the
>>> opposite is true. I'd say there is no perfect solution, we can either
>>> stay close to the refpolicy internals or have the Buildroot specifics
>>> more consistent; not sure which is better.
>> 
>> I’d go for Buildroot consistency since we’re in Buildroot.
> 
> So _WITH_DONTAUDIT_?
> 
>> If someone enables refpolicy package and enables “Enable audit”, I
>> expect to see audit log. Right?
> 
> Extra audit log*

Aaah, here is the point ^^^.

> 
> It's not about enabling audit logs, it's about having some known rules
> to not be silenced in the audit logs as they're known to be problematic.

Ok, Thanks for the explanation.

> 
>> This is me at least that I’ve never used refpolicy so I’m agnostic in
>> the topic, so maybe a neutral “use case“ :-)
> 
> I don't really have a preference for either solution tbh.

Then I’d go for:
BR2_PACKAGE_REFPOLICY_ENABLE_EXTRA_AUDIT_LOG

“Enable extra audit log”

And in the help section I specify all the warnings.
Defaulted to n.

This way it’s pretty clear to me that I approach the package for the first time. And most of all it’s what it does as you’ve explained me above.

Well. I try to respin it this way. Will I receive a Reviewed-by: you then? :-)

Best regards
Giulio

> 
> Antoine

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-24  9:20                 ` Giulio Benetti
@ 2022-01-24  9:29                   ` Antoine Tenart
  2022-01-24  9:32                     ` Giulio Benetti
  0 siblings, 1 reply; 14+ messages in thread
From: Antoine Tenart @ 2022-01-24  9:29 UTC (permalink / raw)
  To: Giulio Benetti; +Cc: buildroot, Thomas Petazzoni, Maxime Chevallier

Quoting Giulio Benetti (2022-01-24 10:20:59)
> 
> Then I’d go for:
> BR2_PACKAGE_REFPOLICY_ENABLE_EXTRA_AUDIT_LOG
> 
> “Enable extra audit log”
> 
> And in the help section I specify all the warnings.
> Defaulted to n.
> 
> This way it’s pretty clear to me that I approach the package for the
> first time. And most of all it’s what it does as you’ve explained me
> above.

But then it's not close to the refpolicy implementation and I don't
find it particularly clear either. Extra audit could be nice in many
ways :)

Let's wait a bit for other to have time to chime in, they might have an
interesting take, or suggestions.

Thanks,
Antoine
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Buildroot] [PATCH v3] package/refpolicy: Add option to disable "dontaudit" rules
  2022-01-24  9:29                   ` Antoine Tenart
@ 2022-01-24  9:32                     ` Giulio Benetti
  0 siblings, 0 replies; 14+ messages in thread
From: Giulio Benetti @ 2022-01-24  9:32 UTC (permalink / raw)
  To: Antoine Tenart; +Cc: Maxime Chevallier, Thomas Petazzoni, buildroot

On 24/01/22 10:29, Antoine Tenart wrote:
> Quoting Giulio Benetti (2022-01-24 10:20:59)
>>
>> Then I’d go for:
>> BR2_PACKAGE_REFPOLICY_ENABLE_EXTRA_AUDIT_LOG
>>
>> “Enable extra audit log”
>>
>> And in the help section I specify all the warnings.
>> Defaulted to n.
>>
>> This way it’s pretty clear to me that I approach the package for the
>> first time. And most of all it’s what it does as you’ve explained me
>> above.
> 
> But then it's not close to the refpolicy implementation and I don't
> find it particularly clear either. Extra audit could be nice in many
> ways :)
> 
> Let's wait a bit for other to have time to chime in, they might have an
> interesting take, or suggestions.

Ok, good idea

Best regards!
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-01-24  9:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28 12:52 [Buildroot] [PATCH v2] package/refpolicy: Add option to disable "dontaudit" rules Maxime Chevallier
2021-01-28 14:24 ` Antoine Tenart
2022-01-19 22:23 ` [Buildroot] [PATCH v3] " Giulio Benetti
2022-01-19 22:39   ` Thomas Petazzoni
2022-01-19 23:56     ` Giulio Benetti
2022-01-20  7:48     ` Maxime Chevallier
2022-01-20  9:29       ` Antoine Tenart
2022-01-23 22:21         ` Giulio Benetti
2022-01-24  8:44           ` Antoine Tenart
2022-01-24  8:59             ` Giulio Benetti
2022-01-24  9:06               ` Antoine Tenart
2022-01-24  9:20                 ` Giulio Benetti
2022-01-24  9:29                   ` Antoine Tenart
2022-01-24  9:32                     ` Giulio Benetti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.