* [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl @ 2022-07-17 19:37 Yann E. MORIN 2022-07-17 19:41 ` Baruch Siach via buildroot 0 siblings, 1 reply; 5+ messages in thread From: Yann E. MORIN @ 2022-07-17 19:37 UTC (permalink / raw) To: buildroot; +Cc: Yann E . MORIN, Nicola Di Lieto From: Baruch Siach <baruch@tkos.co.il> uacme configure script fails when libcurl does not support TLS. This means that BR2_PACKAGE_LIBCURL_TLS_NONE is incompatible with uacme. Add a kconfig knob to libcurl so that no_TLS is not an option. Select that from uacme. 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> [yann.morin.1998@free.fr: - keep the current forward select - add the kconfig knob ] Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> --- v4 (Yann E. MORIN) Restore forward select Add the force-ssl-tls kconfig knob; use it from uacme v3: Move comments up to fix suboption indentation (Yann) Add missing MMU comment dependency (Yann) v2: Add dependency on crypto back end for uacme itself (Nicola Di Lieto) --- package/libcurl/Config.in | 5 +++++ package/uacme/Config.in | 1 + 2 files changed, 6 insertions(+) diff --git a/package/libcurl/Config.in b/package/libcurl/Config.in index 3381decca8..a3148e086d 100644 --- a/package/libcurl/Config.in +++ b/package/libcurl/Config.in @@ -45,6 +45,10 @@ config BR2_PACKAGE_LIBCURL_EXTRA_PROTOCOLS_FEATURES - DICT - Gopher +# Packages must select that if they require a SSL/TLS-enabled libcurl +config BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS + bool + choice prompt "SSL/TLS library to use" @@ -77,6 +81,7 @@ comment "WolfSSL needs a toolchain w/ dynamic library" config BR2_PACKAGE_LIBCURL_TLS_NONE bool "None" + depends on !BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS endchoice diff --git a/package/uacme/Config.in b/package/uacme/Config.in index 58b7c534e7..1458e74d28 100644 --- a/package/uacme/Config.in +++ b/package/uacme/Config.in @@ -3,6 +3,7 @@ config BR2_PACKAGE_UACME depends on BR2_USE_MMU # fork() select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_GNUTLS || BR2_PACKAGE_MBEDTLS) select BR2_PACKAGE_LIBCURL + select BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS help uacme is a client for the ACMEv2 protocol described in RFC8555, written in plain C with minimal dependencies -- 2.25.1 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl 2022-07-17 19:37 [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl Yann E. MORIN @ 2022-07-17 19:41 ` Baruch Siach via buildroot 2022-07-17 20:18 ` Yann E. MORIN 0 siblings, 1 reply; 5+ messages in thread From: Baruch Siach via buildroot @ 2022-07-17 19:41 UTC (permalink / raw) To: Yann E. MORIN; +Cc: Nicola Di Lieto, buildroot Hi Yann, On Sun, Jul 17 2022, Yann E. MORIN wrote: > From: Baruch Siach <baruch@tkos.co.il> > > uacme configure script fails when libcurl does not support TLS. This > means that BR2_PACKAGE_LIBCURL_TLS_NONE is incompatible with uacme. > > Add a kconfig knob to libcurl so that no_TLS is not an option. Select > that from uacme. Looks much more elegant. Thanks. Some comments below. > 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> > [yann.morin.1998@free.fr: > - keep the current forward select > - add the kconfig knob > ] > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > > --- > v4 (Yann E. MORIN) > Restore forward select > Add the force-ssl-tls kconfig knob; use it from uacme > > v3: > Move comments up to fix suboption indentation (Yann) > Add missing MMU comment dependency (Yann) > > v2: > Add dependency on crypto back end for uacme itself (Nicola Di Lieto) > --- > package/libcurl/Config.in | 5 +++++ > package/uacme/Config.in | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/package/libcurl/Config.in b/package/libcurl/Config.in > index 3381decca8..a3148e086d 100644 > --- a/package/libcurl/Config.in > +++ b/package/libcurl/Config.in > @@ -45,6 +45,10 @@ config BR2_PACKAGE_LIBCURL_EXTRA_PROTOCOLS_FEATURES > - DICT > - Gopher > > +# Packages must select that if they require a SSL/TLS-enabled libcurl Said package must also select one of the crypto back ends that libcurl supports. This part is somewhat fragile as libcurl might remove support for any given back end like it recently did for NSS. > +config BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS [Bikeshed] Why not just BR2_PACKAGE_LIBCURL_FORCE_TLS ? baruch > + bool > + > choice > prompt "SSL/TLS library to use" > > @@ -77,6 +81,7 @@ comment "WolfSSL needs a toolchain w/ dynamic library" > > config BR2_PACKAGE_LIBCURL_TLS_NONE > bool "None" > + depends on !BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS > > endchoice > > diff --git a/package/uacme/Config.in b/package/uacme/Config.in > index 58b7c534e7..1458e74d28 100644 > --- a/package/uacme/Config.in > +++ b/package/uacme/Config.in > @@ -3,6 +3,7 @@ config BR2_PACKAGE_UACME > depends on BR2_USE_MMU # fork() > select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_GNUTLS || BR2_PACKAGE_MBEDTLS) > select BR2_PACKAGE_LIBCURL > + select BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS > help > uacme is a client for the ACMEv2 protocol described in > RFC8555, written in plain C with minimal dependencies -- ~. .~ 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl 2022-07-17 19:41 ` Baruch Siach via buildroot @ 2022-07-17 20:18 ` Yann E. MORIN 2022-07-18 3:38 ` Baruch Siach via buildroot 0 siblings, 1 reply; 5+ messages in thread From: Yann E. MORIN @ 2022-07-17 20:18 UTC (permalink / raw) To: Baruch Siach; +Cc: buildroot, Nicola Di Lieto Baruch, All, On 2022-07-17 22:41 +0300, Baruch Siach via buildroot spake thusly: > On Sun, Jul 17 2022, Yann E. MORIN wrote: > > From: Baruch Siach <baruch@tkos.co.il> > > > > uacme configure script fails when libcurl does not support TLS. This > > means that BR2_PACKAGE_LIBCURL_TLS_NONE is incompatible with uacme. > > > > Add a kconfig knob to libcurl so that no_TLS is not an option. Select > > that from uacme. > Looks much more elegant. Thanks. Cool! If you prefer my patch, I'll let you mark yours as superseded in patchwork, then. Otherwise, I'll let another maintainer pick their preferred one. :-) [--SNIP--] > > +# Packages must select that if they require a SSL/TLS-enabled libcurl > Said package must also select one of the crypto back ends that libcurl > supports. Absolutely valid point. > This part is somewhat fragile as libcurl might remove support > for any given back end like it recently did for NSS. I guess openssl will always be a safe default, as it has no architecture dependency. However, that would need further change in libcurl, such as: @@ -47,10 +47,11 @@ config BR2_PACKAGE_LIBCURL_EXTRA_PROTOCOLS_FEATURES choice prompt "SSL/TLS library to use" + default BR2_PACKAGE_LIBCURL_TLS_NONE config BR2_PACKAGE_LIBCURL_OPENSSL bool "OpenSSL" - depends on BR2_PACKAGE_OPENSSL + select BR2_PACKAGE_OPENSSL select BR2_PACKAGE_LIBOPENSSL_ENABLE_DES if BR2_PACKAGE_LIBOPENSSL config BR2_PACKAGE_LIBCURL_BEARSSL which changes the way we handle crypto backends... Maybe just not default the choice to BR2_PACKAGE_LIBCURL_TLS_NONE... After all, we want to promote best practices, and enabled TLS in libcurl is better than not enabling it... Meh... Not sure what's the best, and it is starting to become more complex than my quickly whipped-up patch... > > +config BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS > [Bikeshed] Why not just BR2_PACKAGE_LIBCURL_FORCE_TLS ? The prompt of the choice is "SSL/TLS library to use" so I reflected that in the symbol name, although I do agree that SSL is something we should definitely forget about! :-) If you feel so-inclined, you can grab this patch and adapt it to ensure a crypto backend is always enabled. Otherwise, I'll try to see what I can o a bit later... Thanks for the quick feedback! :-) Regards, Yann E. MORIN. > baruch > > > + bool > > + > > choice > > prompt "SSL/TLS library to use" > > > > @@ -77,6 +81,7 @@ comment "WolfSSL needs a toolchain w/ dynamic library" > > > > config BR2_PACKAGE_LIBCURL_TLS_NONE > > bool "None" > > + depends on !BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS > > > > endchoice > > > > diff --git a/package/uacme/Config.in b/package/uacme/Config.in > > index 58b7c534e7..1458e74d28 100644 > > --- a/package/uacme/Config.in > > +++ b/package/uacme/Config.in > > @@ -3,6 +3,7 @@ config BR2_PACKAGE_UACME > > depends on BR2_USE_MMU # fork() > > select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_GNUTLS || BR2_PACKAGE_MBEDTLS) > > select BR2_PACKAGE_LIBCURL > > + select BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS > > help > > uacme is a client for the ACMEv2 protocol described in > > RFC8555, written in plain C with minimal dependencies > > > -- > ~. .~ 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 -- .-----------------.--------------------.------------------.--------------------. | 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl 2022-07-17 20:18 ` Yann E. MORIN @ 2022-07-18 3:38 ` Baruch Siach via buildroot 2022-07-18 20:29 ` Yann E. MORIN 0 siblings, 1 reply; 5+ messages in thread From: Baruch Siach via buildroot @ 2022-07-18 3:38 UTC (permalink / raw) To: Yann E. MORIN; +Cc: buildroot, Nicola Di Lieto Hi Yann, On Sun, Jul 17 2022, Yann E. MORIN wrote: > On 2022-07-17 22:41 +0300, Baruch Siach via buildroot spake thusly: >> On Sun, Jul 17 2022, Yann E. MORIN wrote: >> > From: Baruch Siach <baruch@tkos.co.il> >> > >> > uacme configure script fails when libcurl does not support TLS. This >> > means that BR2_PACKAGE_LIBCURL_TLS_NONE is incompatible with uacme. >> > >> > Add a kconfig knob to libcurl so that no_TLS is not an option. Select >> > that from uacme. >> Looks much more elegant. Thanks. > > Cool! If you prefer my patch, I'll let you mark yours as superseded in > patchwork, then. Otherwise, I'll let another maintainer pick their > preferred one. :-) > > [--SNIP--] >> > +# Packages must select that if they require a SSL/TLS-enabled libcurl >> Said package must also select one of the crypto back ends that libcurl >> supports. > > Absolutely valid point. > >> This part is somewhat fragile as libcurl might remove support >> for any given back end like it recently did for NSS. > > I guess openssl will always be a safe default, as it has no architecture > dependency. However, that would need further change in libcurl, such as: > > @@ -47,10 +47,11 @@ config BR2_PACKAGE_LIBCURL_EXTRA_PROTOCOLS_FEATURES > > choice > prompt "SSL/TLS library to use" > + default BR2_PACKAGE_LIBCURL_TLS_NONE > > config BR2_PACKAGE_LIBCURL_OPENSSL > bool "OpenSSL" > - depends on BR2_PACKAGE_OPENSSL > + select BR2_PACKAGE_OPENSSL > select BR2_PACKAGE_LIBOPENSSL_ENABLE_DES if BR2_PACKAGE_LIBOPENSSL > > config BR2_PACKAGE_LIBCURL_BEARSSL > > which changes the way we handle crypto backends... Maybe, if we go down this path of 'depends -> select' for all other libcurl crypto backends, we can solve the original uacme problem with a simple !BR2_PACKAGE_LIBCURL_TLS_NONE dependency without recursion. Is that correct? But I'm not sure what can of recursion worms that would open. I only meant to say that the comment above should mention that the package must select a crypto backend. uacme is a special case and it already selects a crypto backend. BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS use is unlikely to become very common in the foreseeable future. So I don't think we need to optimize of this corner case. > Maybe just not default the choice to BR2_PACKAGE_LIBCURL_TLS_NONE... > After all, we want to promote best practices, and enabled TLS in libcurl > is better than not enabling it... Meh... I think this is too heavy handed. TLS takes much more than a crypto backend after all. It should be an explicit user choice. And if we do default to crypto enabled, then a more lightweight option is probably better. > > Not sure what's the best, and it is starting to become more complex than > my quickly whipped-up patch... > >> > +config BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS >> [Bikeshed] Why not just BR2_PACKAGE_LIBCURL_FORCE_TLS ? > > The prompt of the choice is "SSL/TLS library to use" so I reflected that > in the symbol name, although I do agree that SSL is something we should > definitely forget about! :-) > > If you feel so-inclined, you can grab this patch and adapt it to ensure > a crypto backend is always enabled. Otherwise, I'll try to see what I > can o a bit later... I'm fine with the patch as is. baruch > Thanks for the quick feedback! :-) > > Regards, > Yann E. MORIN. > >> baruch >> >> > + bool >> > + >> > choice >> > prompt "SSL/TLS library to use" >> > >> > @@ -77,6 +81,7 @@ comment "WolfSSL needs a toolchain w/ dynamic library" >> > >> > config BR2_PACKAGE_LIBCURL_TLS_NONE >> > bool "None" >> > + depends on !BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS >> > >> > endchoice >> > >> > diff --git a/package/uacme/Config.in b/package/uacme/Config.in >> > index 58b7c534e7..1458e74d28 100644 >> > --- a/package/uacme/Config.in >> > +++ b/package/uacme/Config.in >> > @@ -3,6 +3,7 @@ config BR2_PACKAGE_UACME >> > depends on BR2_USE_MMU # fork() >> > select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_GNUTLS || BR2_PACKAGE_MBEDTLS) >> > select BR2_PACKAGE_LIBCURL >> > + select BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS >> > help >> > uacme is a client for the ACMEv2 protocol described in >> > RFC8555, written in plain C with minimal dependencies -- ~. .~ 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl 2022-07-18 3:38 ` Baruch Siach via buildroot @ 2022-07-18 20:29 ` Yann E. MORIN 0 siblings, 0 replies; 5+ messages in thread From: Yann E. MORIN @ 2022-07-18 20:29 UTC (permalink / raw) To: Baruch Siach; +Cc: Nicola Di Lieto, buildroot Baruch, All, On 2022-07-18 06:38 +0300, Baruch Siach via buildroot spake thusly: > On Sun, Jul 17 2022, Yann E. MORIN wrote: > > On 2022-07-17 22:41 +0300, Baruch Siach via buildroot spake thusly: [--SNIP--] > >> This part is somewhat fragile as libcurl might remove support > >> for any given back end like it recently did for NSS. > > I guess openssl will always be a safe default, as it has no architecture > > dependency. However, that would need further change in libcurl, such as: [--SNIP--] > Maybe, if we go down this path of 'depends -> select' for all other > libcurl crypto backends, we can solve the original uacme problem with a > simple !BR2_PACKAGE_LIBCURL_TLS_NONE dependency without recursion. Is > that correct? Probably. I was wondering why the current choice was using depends on rathwer than select, as that would have been the most logical solution, but the commit message does not explain it. Probably this was done to avoid propagating all the reverse dependencies... > But I'm not sure what can of recursion worms that would open. I was also wondering the same... > I only meant to say that the comment above should mention that the > package must select a crypto backend. Yes, this could also do the trick, but it is not nice that a pakcage that does not have to use crypto for itself be in charge of selecting a crypto backend for libcurl... This does not look nice. > uacme is a special case and it > already selects a crypto backend. Yes, and I indeed leverage that condition to introduce _FORCE_SSL_TLS > BR2_PACKAGE_LIBCURL_FORCE_SSL_TLS use > is unlikely to become very common in the foreseeable future. So I don't > think we need to optimize of this corner case. Valid. [--SNIP--] > > If you feel so-inclined, you can grab this patch and adapt it to ensure > > a crypto backend is always enabled. Otherwise, I'll try to see what I > > can o a bit later... > I'm fine with the patch as is. Ok, thanks for the feedback! :-) I'll send an updated patch that tweaks the comment to also note that a crupto backend needs to be selected. Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-18 20:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-17 19:37 [Buildroot] [PATCHv4] package/uacme: requires TLS support in libcurl Yann E. MORIN 2022-07-17 19:41 ` Baruch Siach via buildroot 2022-07-17 20:18 ` Yann E. MORIN 2022-07-18 3:38 ` Baruch Siach via buildroot 2022-07-18 20:29 ` Yann E. MORIN
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox