Re: [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

Re: [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Randy MacLeod]

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

Add the oe-core list where patches are usually discussed.

On 2021-04-17 10:41 a.m., Shachar Menashe wrote:
> This adds proper TLS verification to wget

I think you should add some of the comments you made in the bugzilla here:

---

By enabling the busybox feature: WGET_OPENSSL it means that in builds 
WITH openssl (ex. sato)
the issue will be completely fixed, and in builds WITHOUT openssl, 
busybox will fallback
to using the internal (insecure) client which will print out a message
"note: TLS certificate validation not implemented" Note that busybox 
does not rely in any way on the OpenSSL library
(it just executes the standalone binary, if it is found) so
we shouldn't have linkage issues is CONFIG_FEATURE_WGET_OPENSSL is 
enabled but OpenSSL is not getting built.

---

Thanks for the explanation.
We could add a RSUGGESTS make the coupling more clear:

http://docs.yoctoproject.org/ref-manual/variables.html?highlight=rrecommends#term-RSUGGESTS

I don't use that feature at all and it's not widely used in oe-core so 
hopefully someone
opinionated will reply and help us out.

../Randy



> Signed-off-by: Shachar Menashe <shachar@vdoo.com>
> ---
>  meta/recipes-core/busybox/busybox.inc | 1 +
>  1 file changed, 1 insertion(+)
> diff --git a/meta/recipes-core/busybox/busybox.inc 
> b/meta/recipes-core/busybox/busybox.inc
> index 47fcb59302..8f274bd263 100644
> --- a/meta/recipes-core/busybox/busybox.inc
> +++ b/meta/recipes-core/busybox/busybox.inc
> @@ -77,6 +77,7 @@ def features_to_busybox_settings(d):
>      busybox_cfg(bb.utils.contains('DISTRO_FEATURES', 'ipv4', True, 
> False, d), 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem)
>      busybox_cfg(bb.utils.contains('DISTRO_FEATURES', 'ipv6', True, 
> False, d), 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem)
>      busybox_cfg(bb.utils.contains_any('DISTRO_FEATURES', 'bluetooth 
> wifi', True, False, d), 'CONFIG_RFKILL', cnf, rem)
> +    busybox_cfg(True, 'CONFIG_FEATURE_WGET_OPENSSL', cnf, rem)
>      return "\n".join(cnf), "\n".join(rem)
>  # X, Y = ${@features_to_busybox_settings(d)}
> --
> 2.17.1
>
> 
>

-- 
# Randy MacLeod
# Wind River Linux


[-- Attachment #2: Type: text/html, Size: 3912 bytes --]

Re: [OE-core] [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Andre McCurdy]

On Tue, Apr 20, 2021 at 10:23 AM Randy MacLeod
<randy.macleod@windriver.com> wrote:
>
> Add the oe-core list where patches are usually discussed.
>
> On 2021-04-17 10:41 a.m., Shachar Menashe wrote:
>
> This adds proper TLS verification to wget
>
> I think you should add some of the comments you made in the bugzilla here:
>
> ---
>
> By enabling the busybox feature: WGET_OPENSSL it means that in builds WITH openssl (ex. sato)
> the issue will be completely fixed, and in builds WITHOUT openssl, busybox will fallback
> to using the internal (insecure) client which will print out a message
> "note: TLS certificate validation not implemented" Note that busybox does not rely in any way on the OpenSSL library
> (it just executes the standalone binary, if it is found) so
> we shouldn't have linkage issues is CONFIG_FEATURE_WGET_OPENSSL is enabled but OpenSSL is not getting built.
>
> ---
>
> Thanks for the explanation.
> We could add a RSUGGESTS make the coupling more clear:
>
> http://docs.yoctoproject.org/ref-manual/variables.html?highlight=rrecommends#term-RSUGGESTS
>
> I don't use that feature at all and it's not widely used in oe-core so hopefully someone
> opinionated will reply and help us out.
>
> ../Randy


> Signed-off-by: Shachar Menashe <shachar@vdoo.com>
> ---
>  meta/recipes-core/busybox/busybox.inc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
> index 47fcb59302..8f274bd263 100644
> --- a/meta/recipes-core/busybox/busybox.inc
> +++ b/meta/recipes-core/busybox/busybox.inc
> @@ -77,6 +77,7 @@ def features_to_busybox_settings(d):
>      busybox_cfg(bb.utils.contains('DISTRO_FEATURES', 'ipv4', True, False, d), 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem)
>      busybox_cfg(bb.utils.contains('DISTRO_FEATURES', 'ipv6', True, False, d), 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem)
>      busybox_cfg(bb.utils.contains_any('DISTRO_FEATURES', 'bluetooth wifi', True, False, d), 'CONFIG_RFKILL', cnf, rem)
> +    busybox_cfg(True, 'CONFIG_FEATURE_WGET_OPENSSL', cnf, rem)
>      return "\n".join(cnf), "\n".join(rem)
>
>  # X, Y = ${@features_to_busybox_settings(d)}
> --
> 2.17.1
>

This was discussed on the list last year. The conclusion was that
FEATURE_WGET_HTTPS should be disabled by default (ie giving anyone who
needs to fetch from https URLs to clear hint that that should be using
full featured wget or curl) rather than enabling a hacky solution to
have busybox call out to the openssl command line tool. Has something
changed since then?

Re: [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Shachar Menashe]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

Last time we talked about this I thought we would need to change something in openssl build settings to make the openssl binary get built just for this solution, and that was what got rejected.
But actually now I see (or perhaps it got changed) that the openssl binary is built anyways, in any build that already relies on openssl.
So my suggestion is to enable this feature. Like I said in builds with openssl it will make everything more secure in a transparent manner, and in builds without openssl it will display a warning just like today.
I wouldn't consider it a hacky solution since this is the official solution for this issue.
This is also exacerbated due to the fact that there are no other alternatives for secure download from CLI (ex. the sato build doesn't contain the "curl" standalone binary)

[-- Attachment #2: Type: text/html, Size: 831 bytes --]

Re: [OE-core] [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Khem Raj]

On Tue, Apr 20, 2021 at 1:46 PM Shachar Menashe <shachar@vdoo.com> wrote:
>
> Last time we talked about this I thought we would need to change something in openssl build settings to make the openssl binary get built just for this solution, and that was what got rejected.
> But actually now I see (or perhaps it got changed) that the openssl binary is built anyways, in any build that already relies on openssl.
> So my suggestion is to enable this feature. Like I said in builds with openssl it will make everything more secure in a transparent manner, and in builds without openssl it will display a warning just like today.

How much does busybox size grow with this? I think we will have to add
openssl dependency on it, or else default wget behavious will be less
than ideal. right now perhaps using gnu wget is a standalone solution
but I do understand that it may not be usable in some cases.

> I wouldn't consider it a hacky solution since this is the official solution for this issue.
> This is also exacerbated due to the fact that there are no other alternatives for secure download from CLI (ex. the sato build doesn't contain the "curl" standalone binary)

certainly, add curl to default reference images would be fine.

> 
>

Re: [OE-core] [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Andre McCurdy]

On Tue, Apr 20, 2021 at 1:46 PM Shachar Menashe <shachar@vdoo.com> wrote:
>
> Last time we talked about this I thought we would need to change something in openssl build settings to make the openssl binary get built just for this solution, and that was what got rejected.
> But actually now I see (or perhaps it got changed) that the openssl binary is built anyways, in any build that already relies on openssl.
> So my suggestion is to enable this feature. Like I said in builds with openssl it will make everything more secure in a transparent manner, and in builds without openssl it will display a warning just like today.
> I wouldn't consider it a hacky solution since this is the official solution for this issue.

It's very clearly a hack. Maybe it's the "official solution" for
supporting https with busybox wget, but OE has a wider scope - we're
not limited to busybox wget if a better overall solution is available.

> This is also exacerbated due to the fact that there are no other alternatives for secure download from CLI (ex. the sato build doesn't contain the "curl" standalone binary)

I don't see an issue with adding curl to any OE reference image which
needs an https client.

> 
>

Re: [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Shachar Menashe]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

On Tue, Apr 20, 2021 at 1:46 PM Shachar Menashe <shachar@vdoo.com> wrote:

> 
> 
> Last time we talked about this I thought we would need to change something
> in openssl build settings to make the openssl binary get built just for
> this solution, and that was what got rejected.
> But actually now I see (or perhaps it got changed) that the openssl binary
> is built anyways, in any build that already relies on openssl.
> So my suggestion is to enable this feature. Like I said in builds with
> openssl it will make everything more secure in a transparent manner, and
> in builds without openssl it will display a warning just like today.
> I wouldn't consider it a hacky solution since this is the official
> solution for this issue.

It's very clearly a hack. Maybe it's the "official solution" for
supporting https with busybox wget, but OE has a wider scope - we're
not limited to busybox wget if a better overall solution is available.

> 
> This is also exacerbated due to the fact that there are no other
> alternatives for secure download from CLI (ex. the sato build doesn't
> contain the "curl" standalone binary)

I don't see an issue with adding curl to any OE reference image which
needs an https client.

> 
> OK, so do you suggest adding curl and removing wget? (that would be a
> patch with a configuration change to busybox)

[-- Attachment #2: Type: text/html, Size: 1451 bytes --]

Re: [OE-core] [yocto-security] [PATCH] busybox: use openssl for TLS connections whenever possible

[Andre McCurdy]

On Wed, Apr 21, 2021 at 2:22 AM Shachar Menashe <shachar@vdoo.com> wrote:
> On Tue, Apr 20, 2021 at 1:46 PM Shachar Menashe <shachar@vdoo.com> wrote:
>
> Last time we talked about this I thought we would need to change something in openssl build settings to make the openssl binary get built just for this solution, and that was what got rejected.
> But actually now I see (or perhaps it got changed) that the openssl binary is built anyways, in any build that already relies on openssl.
> So my suggestion is to enable this feature. Like I said in builds with openssl it will make everything more secure in a transparent manner, and in builds without openssl it will display a warning just like today.
> I wouldn't consider it a hacky solution since this is the official solution for this issue.
>
> It's very clearly a hack. Maybe it's the "official solution" for
> supporting https with busybox wget, but OE has a wider scope - we're
> not limited to busybox wget if a better overall solution is available.
>
> This is also exacerbated due to the fact that there are no other alternatives for secure download from CLI (ex. the sato build doesn't contain the "curl" standalone binary)
>
> I don't see an issue with adding curl to any OE reference image which
> needs an https client.
>
> OK, so do you suggest adding curl and removing wget? (that would be a patch with a configuration change to busybox)

Yes, sounds good to me.