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

[James Kent]

Yann,

thanks for the feedback.  Yes I'll rework the change with your input and 
submit a revised patch.

Regards,
James

On 07/02/2023 21:50, Yann E. MORIN wrote:
> 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 at 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 at buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot