Re: [Buildroot] [RFC PATCH v2] package/uacme: requires TLS support in libcurl

[Baruch Siach via buildroot <buildroot@buildroot.org>]

Hi Yann,

On Sun, Jul 17 2022, Yann E. MORIN wrote:
> On 2022-07-14 08:49 +0300, Baruch Siach via buildroot spake thusly:
>> uacme configure script fails when libcurl does not support TLS. This
>> means that BR2_PACKAGE_LIBCURL_TLS_NONE is incompatible with uacme. But
>> there is no way to change the choice to something other than
>> BR2_PACKAGE_LIBCURL_TLS_NONE. So instead make uacme depend on libcurl
>> and !BR2_PACKAGE_LIBCURL_TLS_NONE.
>> 
>> As a result we can no longer select BR2_PACKAGE_OPENSSL since it causes
>> recursive dependency. Use 'depend on' instead, and add a comment to
>> explain this uncommon choice.
>> 
>> Fixes:
>> http://autobuild.buildroot.net/results/4e16f1d958ac3d30e26e7f17bdffc47834b0e2bd/
>> http://autobuild.buildroot.net/results/4e16f1d958ac3d30e26e7f17bdffc47834b0e2bd/
>> http://autobuild.buildroot.net/results/25280409b32282b4dd40b1e88127051439380f3d/
>> 
>> Cc: Nicola Di Lieto <nicola.dilieto@gmail.com>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> v2:
>>   Add dependency on crypto back end for uacme itself (Nicola Di Lieto)
>> ---
>>  package/uacme/Config.in | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>> 
>> diff --git a/package/uacme/Config.in b/package/uacme/Config.in
>> index 58b7c534e73d..815ab5da7d61 100644
>> --- a/package/uacme/Config.in
>> +++ b/package/uacme/Config.in
>> @@ -1,8 +1,9 @@
>>  config BR2_PACKAGE_UACME
>>  	bool "uacme"
>>  	depends on BR2_USE_MMU # fork()
>> -	select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_GNUTLS || BR2_PACKAGE_MBEDTLS)
>> -	select BR2_PACKAGE_LIBCURL
>> +	# We can not use select here as it causes recursive dependency
>> +	depends on BR2_PACKAGE_OPENSSL || BR2_PACKAGE_GNUTLS || BR2_PACKAGE_MBEDTLS
>> +	depends on BR2_PACKAGE_LIBCURL && !BR2_PACKAGE_LIBCURL_TLS_NONE
>
> I don't think this is correct. Indeed, even with one of those packages
> enabled, there is nothing that prevents libcurl to be linked with
> another TLS provider, as that is decided with the choice entries, not
> with the packages being enabled.
>
> Instead, what about:
>
>     depends on BR2_PACKAGE_LIBCURL_OPENSSL \
>             || BR2_PACKAGE_LIBCURL_GNUTLS \
>             || BR2_PACKAGE_LIBCURL_MBEDTLS
>
> That way, it encodes both the fact that libcurl is enabled, *and* that
> is has the proper TLS support enabled.

As Nicola explained on v1, uacme does not care which crypto back end
libcurl uses, as long as there is one.

  https://lore.kernel.org/all/Ys5vPCrxDXWvj+ok@einstein.dilieto.eu/

Regardless of that, uacme requires one of these crypt back ends for its
own use. So I think these dependencies are correct.

>>  	help
>>  	  uacme is a client for the ACMEv2 protocol described in
>>  	  RFC8555, written in plain C with minimal dependencies
>> @@ -14,6 +15,13 @@ config BR2_PACKAGE_UACME
>>  
>>  	  https://github.com/ndilieto/uacme
>>  
>> +comment "uacme needs one of openssl, gnutls or mbedtls"
>> +	depends on !BR2_PACKAGE_OPENSSL && !BR2_PACKAGE_GNUTLS && !BR2_PACKAGE_MBEDTLS
>
> That's not correct. It would be better to phrase it, as Nicola suggested
> in their review of v1:
>
>     comment "uacme needs libcurl with openssl, gnutls or mbedtls"
>             depends on BR2_USE_MMU
>             depends on !BR2_PACKAGE_LIBCURL_OPENSSL \
>                     && !BR2_PACKAGE_LIBCURL_GNUTLS \
>                     && !BR2_PACKAGE_LIBCURL_MBEDTLS

This is overly restrictive. See above.

>> +comment "uacme needs libcurl with TLS support"
>> +	depends on BR2_USE_MMU
>> +	depends on !BR2_PACKAGE_LIBCURL || BR2_PACKAGE_LIBCURL_TLS_NONE
>
> ... then this comment is no longer needed.
>
> Also, comments about packages being not available should go either
> before the main symbol, or after the conditional options. Otherwise, the
> sub-options are not indented below the main symbol. With your code:
>
>     [*] uacme
>     [ ] enable ualpn
>
> while we want:
>
>     [*] uacme
>     [ ]   enable ualpn
>
> I'd have fixed that when applying, but I prefer to get some feedback
> about my proposal on the dependendcy condition.

I'll fix that if I send another iteration.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot