Re: [Buildroot] [PATCH] package/chrony: add default unprivileged user option

["Yann E. MORIN" <yann.morin.1998@free.fr>]

James, All,

On 2023-02-07 17:10 +0000, James Kent spake thusly:
> Configurable option to define and enable by default an unprivileged
> user which the Chrony daemon will assume once initialised. Where libcap
> is not enabled a comment indicates the dependency requirement to make
> the option available.
> 
> This option supports the good security practice of dropping elevated
> privileges for daemon runtime.

Thanks, thjis is=ndeed makes sense. See below for some comments...

> Signed-off-by: James Kent <james.kent@orchestrated-technology.com>
> ---
>  package/chrony/Config.in | 10 ++++++++++
>  package/chrony/chrony.mk |  8 ++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/package/chrony/Config.in b/package/chrony/Config.in
> index 158dc20530..8b053ed7e2 100644
> --- a/package/chrony/Config.in
> +++ b/package/chrony/Config.in
> @@ -14,3 +14,13 @@ config BR2_PACKAGE_CHRONY_DEBUG_LOGGING
>  	  Enable support for debug logging output from Chrony when
>  	  enabled at runtime. If disabled, code for debug logging will
>  	  not be compiled in.
> +
> +config BR2_PACKAGE_CHRONY_USER
> +	bool "chrony default unprivileged user"
> +	depends on BR2_PACKAGE_CHRONY && BR2_PACKAGE_LIBCAP

First, we now try to enclose package sub-options in an if-endif block,
rather than have each option depend on the package. I.e.:

    config BR2_PACKAGE_CHRONY
        bool "chrony"

    if BR2_PACKAGE_CHRONY

    config BR2_PACKAGE_CHRONY_FOO
        bool "foo"

    config BR2_PACKAGE_CHRONY_BAR
        bool "bar"

    endif

Yes, the existing option doesnot follow that coding style, so just fix
it up while at it.

Second, chrony and libcap both need an MMU, so jut select libcap when
BR2_PACKAGE_CHRONY_USER is enabled.

> +	help
> +	  Define and enable default unprivileged user for the Chrony
> +	  daemon to run as.
> +
> +comment "chrony default unprivileged user requires libcap"
> +	depends on BR2_PACKAGE_CHRONY && !BR2_PACKAGE_LIBCAP

And thus, that comment is no longer needed.

> diff --git a/package/chrony/chrony.mk b/package/chrony/chrony.mk
> index 379e95a778..16f8f082a3 100644
> --- a/package/chrony/chrony.mk
> +++ b/package/chrony/chrony.mk
> @@ -21,6 +21,14 @@ CHRONY_CONF_OPTS = \
>  
>  ifeq ($(BR2_PACKAGE_LIBCAP),y)
>  CHRONY_DEPENDENCIES += libcap
> +
> +ifeq ($(BR2_PACKAGE_CHRONY_USER),y)
> +CHRONY_CONF_OPTS += --with-user=chrony
> +define CHRONY_USERS
> +	chrony -1 chrony -1 * /run/chrony - - Time daemon
> +endef
> +endif

This part is correct.

Can you fix and respin, please?

Regards,
Yann E. MORIN.

>  else
>  CHRONY_CONF_OPTS += --without-libcap
>  endif
> -- 
> 2.35.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot