[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Mircea Gliga]

libssh supports mbedtls as a crypto backend. Allow selection of crypto
backend libssh will use through a choice in the package config. The
default will be to select the first enabled crypto backend provider in
the same preference order as is used now, i.e. no change from current
behavior.

Signed-off-by: Mircea Gliga <gliga.mircea@gmail.com>
---
 package/libssh/Config.in | 28 ++++++++++++++++++++++++++--
 package/libssh/libssh.mk | 10 +++++-----
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/package/libssh/Config.in b/package/libssh/Config.in
index 3dbfa7d561..d74c821d06 100644
--- a/package/libssh/Config.in
+++ b/package/libssh/Config.in
@@ -3,8 +3,7 @@ config BR2_PACKAGE_LIBSSH
 	depends on BR2_USE_MMU # fork()
 	depends on !BR2_STATIC_LIBS
 	depends on BR2_TOOLCHAIN_HAS_THREADS
-	# Either OpenSSL or libgcrypt are mandatory
-	select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_LIBGCRYPT
+	select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_MBEDTLS || BR2_PACKAGE_LIBGCRYPT)
 	help
 	  libssh is a multiplatform C library implementing the SSHv2
 	  and SSHv1 protocol on client and server side. With libssh,
@@ -13,6 +12,31 @@ config BR2_PACKAGE_LIBSSH
 
 	  http://www.libssh.org/
 
+if BR2_PACKAGE_LIBSSH
+
+choice
+	prompt "Crypto Backend"
+	default BR2_PACKAGE_LIBSSH_LIBGCRYPT if BR2_PACKAGE_LIBGCRYPT
+	default BR2_PACKAGE_LIBSSH_OPENSSL if BR2_PACKAGE_OPENSSL
+	default BR2_PACKAGE_LIBSSH_MBEDTLS if BR2_PACKAGE_MBEDTLS
+	help
+	  Select crypto library to be used in libssh.
+
+config BR2_PACKAGE_LIBSSH_MBEDTLS
+	bool "mbedtls"
+	depends on BR2_PACKAGE_MBEDTLS
+
+config BR2_PACKAGE_LIBSSH_LIBGCRYPT
+	bool "gcrypt"
+	depends on BR2_PACKAGE_LIBGCRYPT
+
+config BR2_PACKAGE_LIBSSH_OPENSSL
+	bool "openssl"
+	depends on BR2_PACKAGE_OPENSSL
+
+endchoice
+endif
+
 comment "libssh needs a toolchain w/ dynamic library, threads"
 	depends on BR2_USE_MMU
 	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/libssh/libssh.mk b/package/libssh/libssh.mk
index d5f22c29a0..7ee23ca6ba 100644
--- a/package/libssh/libssh.mk
+++ b/package/libssh/libssh.mk
@@ -27,13 +27,13 @@ else
 LIBSSH_CONF_OPTS += -DWITH_ZLIB=OFF
 endif
 
-# Dependency is either on libgcrypt or openssl, guaranteed in Config.in.
-# Favour libgcrypt.
-ifeq ($(BR2_PACKAGE_LIBGCRYPT),y)
+ifeq ($(BR2_PACKAGE_LIBSSH_MBEDTLS),y)
+LIBSSH_CONF_OPTS += -DWITH_MBEDTLS=ON
+LIBSSH_DEPENDENCIES += mbedtls
+else ifeq ($(BR2_PACKAGE_LIBSSH_LIBGCRYPT),y)
 LIBSSH_CONF_OPTS += -DWITH_GCRYPT=ON
 LIBSSH_DEPENDENCIES += libgcrypt
-else
-LIBSSH_CONF_OPTS += -DWITH_GCRYPT=OFF
+else ifeq ($(BR2_PACKAGE_LIBSSH_OPENSSL),y)
 LIBSSH_DEPENDENCIES += openssl
 endif
 
-- 
2.23.0

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Thomas Petazzoni]

Hello Mircea,

Thanks for this contribution!

On Wed, 18 Sep 2019 14:06:09 +0300
Mircea Gliga <gliga.mircea@gmail.com> wrote:

> +choice
> +	prompt "Crypto Backend"
> +	default BR2_PACKAGE_LIBSSH_LIBGCRYPT if BR2_PACKAGE_LIBGCRYPT
> +	default BR2_PACKAGE_LIBSSH_OPENSSL if BR2_PACKAGE_OPENSSL
> +	default BR2_PACKAGE_LIBSSH_MBEDTLS if BR2_PACKAGE_MBEDTLS
> +	help
> +	  Select crypto library to be used in libssh.
> +
> +config BR2_PACKAGE_LIBSSH_MBEDTLS
> +	bool "mbedtls"
> +	depends on BR2_PACKAGE_MBEDTLS
> +
> +config BR2_PACKAGE_LIBSSH_LIBGCRYPT
> +	bool "gcrypt"
> +	depends on BR2_PACKAGE_LIBGCRYPT
> +
> +config BR2_PACKAGE_LIBSSH_OPENSSL
> +	bool "openssl"
> +	depends on BR2_PACKAGE_OPENSSL
> +
> +endchoice
> +endif

I am wondering if we really need an explicit choice option here. Can't
we just use one of the three crypto libraries, depending on which one
is available ? Is there a good benefit in being able to explicitly
select which crypto backend to use ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Mircea Gliga]

Hi Thomas and thanks for the review,

The Config.in also has this part that automatically selects the OpenSSL
package if no crypto backend is already available:

> -       select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_LIBGCRYPT
> +       select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_MBEDTLS ||
BR2_PACKAGE_LIBGCRYPT)

This is in sync with the old behavior, when mbedtls was not in scope.
And then in case there are multiple crypto backends available it will
prefer Libgcrypt, then OpenSSL and in the end mbedTLS:

> +     default BR2_PACKAGE_LIBSSH_LIBGCRYPT if BR2_PACKAGE_LIBGCRYPT
> +     default BR2_PACKAGE_LIBSSH_OPENSSL if BR2_PACKAGE_OPENSSL
> +     default BR2_PACKAGE_LIBSSH_MBEDTLS if BR2_PACKAGE_MBEDTLS

The default will be to select the first enabled crypto backend provider
in the same preference order as is used now, i.e. no change from current
behavior.

Thanks and regards
Mircea
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190923/fdf120f2/attachment.html>

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Thomas Petazzoni]

Hello,

On Mon, 23 Sep 2019 09:11:40 +0300
Mircea Gliga <gliga.mircea@gmail.com> wrote:

> Hi Thomas and thanks for the review,
> 
> The Config.in also has this part that automatically selects the OpenSSL
> package if no crypto backend is already available:
> 
> > -       select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_LIBGCRYPT
> > +       select BR2_PACKAGE_OPENSSL if !(BR2_PACKAGE_MBEDTLS ||  
> BR2_PACKAGE_LIBGCRYPT)
> 
> This is in sync with the old behavior, when mbedtls was not in scope.
> And then in case there are multiple crypto backends available it will
> prefer Libgcrypt, then OpenSSL and in the end mbedTLS:
> 
> > +     default BR2_PACKAGE_LIBSSH_LIBGCRYPT if BR2_PACKAGE_LIBGCRYPT
> > +     default BR2_PACKAGE_LIBSSH_OPENSSL if BR2_PACKAGE_OPENSSL
> > +     default BR2_PACKAGE_LIBSSH_MBEDTLS if BR2_PACKAGE_MBEDTLS  
> 
> The default will be to select the first enabled crypto backend provider
> in the same preference order as is used now, i.e. no change from current
> behavior.

Thanks, but that was not my point. My point was: is it really useful to
provide an explicit choice vs. simply selecting a preferred crypto
provider automatically, with no explicit choice in Config.in.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Mircea Gliga]

Hi Thomas,

Sorry for the misunderstanding, I re-read your question.

> My point was: is it really useful to
> provide an explicit choice vs. simply selecting a preferred crypto
> provider automatically, with no explicit choice in Config.in.

Yes it's useful if for some reason you have multiple backends already
present: for example one package has a hard dependency on openSSL and I
want mine to use libssh with mbedTLS. In this case you have to manually
choose your crypto backend as the default is not suitable for your needs.

Best regards
Mircea
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190923/23a09665/attachment.html>

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Arnout Vandecappelle]

On 23/09/2019 12:22, Mircea Gliga wrote:
> Hi Thomas,
> 
> Sorry for the misunderstanding, I re-read your question.
> 
>> My point was: is it really useful to
>> provide an explicit choice vs. simply selecting a preferred crypto
>> provider automatically, with no explicit choice in Config.in.
> 
> Yes it's useful if for some reason you have multiple backends already
> present: for example one package has a hard dependency on openSSL and I
> want mine to use libssh with mbedTLS. In this case you have to manually
> choose your crypto backend as the default is not suitable for your needs.

 Yes, but the real question then is: why would mbedTLS ever be more suitable for
your needs than openssl (when used as a backend for libssh)?

 I remember there was a patch doing something similar for another package at
some point, and there the explanation was that (IIRC) with gnutls you could use
a crypto token but with openssl you couldn't. So, if there is something like
that at play here as well, please explain it in the commit message.

 Regards,
 Arnout

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Mircea Gliga]

Hello,

I did the patch to be consistent with the already existing libssh2
package, using choice. It's exactly the same use case.

1. at this point Buildroot doesn't allow to use mbedTLS backend even
though libssh supports it perfectly, this is _the root of the problem_

2. in case of *fully statically linked* ELF executables the size
difference between OpenSSL and mbedTLS is huge: it matters for embedded
targets with _very_ limited storage

3. for systems where it's impossible to use shared libraries this size
difference counts

An example:
a. ELF 1 -> libcurl -> libssh2 -> OpenSSL
b. ELF 2 -> libssh (NOT libssh2) -> mbedTLS

In case of a. OpenSSL is required for libssh2. Otherwise eliptic curve
KEX cannot be used [1] and we need them. For ECDSA/ED25519 with the
mbedTLS backend, the glue code is missing. The slow diffie hellman is
used instead.

On the otherside, ELF 2 from case b. is perfectly happy with mbedTLS and
ECDSA/ED25519 KEX, but currently this cannot be achieved.

Regards,

-- 
Mircea

[1] https://github.com/libssh2/libssh2/blob/master/RELEASE-NOTES#L5
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190923/1847245a/attachment.html>

[Buildroot] [PATCH 1/1] package/libssh: add support for mbedtls crypto backend

[Arnout Vandecappelle]

On 23/09/2019 16:20, Mircea Gliga wrote:
> Hello,
> 
> I did the patch to be consistent with the already existing libssh2
> package, using choice. It's exactly the same use case.

 Yeah, but I'm not sure if it's the best example. I think strongswan is a better
example, i.e. the choice selects the crypto provider package.

 I would also put openssl as the default, since that's what people are more
likely to want.


> 1. at this point Buildroot doesn't allow to use mbedTLS backend even
> though libssh supports it perfectly, this is _the root of the problem_
> 
> 2. in case of *fully statically linked* ELF executables the size
> difference between OpenSSL and mbedTLS is huge: it matters for embedded
> targets with _very_ limited storage

 Good point! That would be something nice to mention in the commit message.

 Refer to the commit message of 04a1031d3429ff8e5a2ae4d820702c50519243a8 for an
example of what we like to see.

 Regards,
 Arnout

> 
> 3. for systems where it's impossible to use shared libraries this size
> difference counts
> 
> An example:?
> a. ELF 1 -> libcurl -> libssh2 -> OpenSSL
> b. ELF 2 -> libssh (NOT libssh2) -> mbedTLS
> 
> In case of a. OpenSSL is required for libssh2. Otherwise eliptic curve
> KEX cannot be used [1] and we need them. For ECDSA/ED25519 with the
> mbedTLS backend, the glue code is missing. The slow diffie hellman is
> used instead.
> 
> On the otherside, ELF 2 from case b. is perfectly happy with mbedTLS and
> ECDSA/ED25519 KEX, but currently this cannot be achieved.
> 
> Regards,
> 
> --?
> Mircea
> 
> [1] https://github.com/libssh2/libssh2/blob/master/RELEASE-NOTES#L5

[Buildroot] [PATCH v2] package/libssh: add support for mbedtls crypto backend

[Mircea Gliga]

At this point Buildroot doesn't allow to use mbedTLS crypto
backend even though libssh supports it. In case of fully statically
linked ELF executables the size difference between OpenSSL and mbedTLS
is significant: it matters for embedded targets with very limited
storage.

This patch adds support for compiling libssh with mbedTLS as a
crypto backend. It also allows the selection of the crypto backend
libssh will use through a choice in the package config.

Currently, the selection of the backend is based on a priority order,
which is not always desirable, as in some cases multiple backends
can exists at the same time for various reasons.

Switch to OpenSSL as the default crypto backend, instead of libgcrypt,
since OpenSSL is more commonly used.

Signed-off-by: Mircea Gliga <gliga.mircea@gmail.com>

---
Changes V1->V2:
* choice now selects the crypto provider package
* more detailed description in commit message
* switch default crypto backend to OpenSSL
---
 package/libssh/Config.in | 26 ++++++++++++++++++++++++--
 package/libssh/libssh.mk | 10 +++++-----
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/package/libssh/Config.in b/package/libssh/Config.in
index 3dbfa7d561..f31b35f9ab 100644
--- a/package/libssh/Config.in
+++ b/package/libssh/Config.in
@@ -3,8 +3,6 @@ config BR2_PACKAGE_LIBSSH
 	depends on BR2_USE_MMU # fork()
 	depends on !BR2_STATIC_LIBS
 	depends on BR2_TOOLCHAIN_HAS_THREADS
-	# Either OpenSSL or libgcrypt are mandatory
-	select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_LIBGCRYPT
 	help
 	  libssh is a multiplatform C library implementing the SSHv2
 	  and SSHv1 protocol on client and server side. With libssh,
@@ -13,6 +11,30 @@ config BR2_PACKAGE_LIBSSH
 
 	  http://www.libssh.org/
 
+if BR2_PACKAGE_LIBSSH
+
+choice
+	prompt "Crypto Backend"
+	default BR2_PACKAGE_LIBSSH_OPENSSL
+	help
+	  Select crypto library to be used in libssh.
+
+config BR2_PACKAGE_LIBSSH_MBEDTLS
+	bool "mbedtls"
+	select BR2_PACKAGE_MBEDTLS
+
+config BR2_PACKAGE_LIBSSH_LIBGCRYPT
+	bool "gcrypt"
+	depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libgcrypt
+	select BR2_PACKAGE_LIBGCRYPT
+
+config BR2_PACKAGE_LIBSSH_OPENSSL
+	bool "openssl"
+	select BR2_PACKAGE_OPENSSL
+
+endchoice
+endif
+
 comment "libssh needs a toolchain w/ dynamic library, threads"
 	depends on BR2_USE_MMU
 	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/libssh/libssh.mk b/package/libssh/libssh.mk
index d5f22c29a0..7ee23ca6ba 100644
--- a/package/libssh/libssh.mk
+++ b/package/libssh/libssh.mk
@@ -27,13 +27,13 @@ else
 LIBSSH_CONF_OPTS += -DWITH_ZLIB=OFF
 endif
 
-# Dependency is either on libgcrypt or openssl, guaranteed in Config.in.
-# Favour libgcrypt.
-ifeq ($(BR2_PACKAGE_LIBGCRYPT),y)
+ifeq ($(BR2_PACKAGE_LIBSSH_MBEDTLS),y)
+LIBSSH_CONF_OPTS += -DWITH_MBEDTLS=ON
+LIBSSH_DEPENDENCIES += mbedtls
+else ifeq ($(BR2_PACKAGE_LIBSSH_LIBGCRYPT),y)
 LIBSSH_CONF_OPTS += -DWITH_GCRYPT=ON
 LIBSSH_DEPENDENCIES += libgcrypt
-else
-LIBSSH_CONF_OPTS += -DWITH_GCRYPT=OFF
+else ifeq ($(BR2_PACKAGE_LIBSSH_OPENSSL),y)
 LIBSSH_DEPENDENCIES += openssl
 endif
 
-- 
2.23.0

[Buildroot] [PATCH v2] package/libssh: add support for mbedtls crypto backend

[Mircea Gliga]

Hello,

I didn't got any feedback in regards to this and I was wondering if this
v2 patch is better and if it has any chances to go to master.
In this version, the choice selects the crypto provider package, and
as a default it uses OpenSSL.
Also I improved the commit message.

Thanks and regards
Mircea


On Tue, Sep 24, 2019 at 4:26 PM Mircea Gliga <gliga.mircea@gmail.com> wrote:

> At this point Buildroot doesn't allow to use mbedTLS crypto
> backend even though libssh supports it. In case of fully statically
> linked ELF executables the size difference between OpenSSL and mbedTLS
> is significant: it matters for embedded targets with very limited
> storage.
>
> This patch adds support for compiling libssh with mbedTLS as a
> crypto backend. It also allows the selection of the crypto backend
> libssh will use through a choice in the package config.
>
> Currently, the selection of the backend is based on a priority order,
> which is not always desirable, as in some cases multiple backends
> can exists at the same time for various reasons.
>
> Switch to OpenSSL as the default crypto backend, instead of libgcrypt,
> since OpenSSL is more commonly used.
>
> Signed-off-by: Mircea Gliga <gliga.mircea@gmail.com>
>
> ---
> Changes V1->V2:
> * choice now selects the crypto provider package
> * more detailed description in commit message
> * switch default crypto backend to OpenSSL
> ---
>  package/libssh/Config.in | 26 ++++++++++++++++++++++++--
>  package/libssh/libssh.mk | 10 +++++-----
>  2 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/package/libssh/Config.in b/package/libssh/Config.in
> index 3dbfa7d561..f31b35f9ab 100644
> --- a/package/libssh/Config.in
> +++ b/package/libssh/Config.in
> @@ -3,8 +3,6 @@ config BR2_PACKAGE_LIBSSH
>         depends on BR2_USE_MMU # fork()
>         depends on !BR2_STATIC_LIBS
>         depends on BR2_TOOLCHAIN_HAS_THREADS
> -       # Either OpenSSL or libgcrypt are mandatory
> -       select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_LIBGCRYPT
>         help
>           libssh is a multiplatform C library implementing the SSHv2
>           and SSHv1 protocol on client and server side. With libssh,
> @@ -13,6 +11,30 @@ config BR2_PACKAGE_LIBSSH
>
>           http://www.libssh.org/
>
> +if BR2_PACKAGE_LIBSSH
> +
> +choice
> +       prompt "Crypto Backend"
> +       default BR2_PACKAGE_LIBSSH_OPENSSL
> +       help
> +         Select crypto library to be used in libssh.
> +
> +config BR2_PACKAGE_LIBSSH_MBEDTLS
> +       bool "mbedtls"
> +       select BR2_PACKAGE_MBEDTLS
> +
> +config BR2_PACKAGE_LIBSSH_LIBGCRYPT
> +       bool "gcrypt"
> +       depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libgcrypt
> +       select BR2_PACKAGE_LIBGCRYPT
> +
> +config BR2_PACKAGE_LIBSSH_OPENSSL
> +       bool "openssl"
> +       select BR2_PACKAGE_OPENSSL
> +
> +endchoice
> +endif
> +
>  comment "libssh needs a toolchain w/ dynamic library, threads"
>         depends on BR2_USE_MMU
>         depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
> diff --git a/package/libssh/libssh.mk b/package/libssh/libssh.mk
> index d5f22c29a0..7ee23ca6ba 100644
> --- a/package/libssh/libssh.mk
> +++ b/package/libssh/libssh.mk
> @@ -27,13 +27,13 @@ else
>  LIBSSH_CONF_OPTS += -DWITH_ZLIB=OFF
>  endif
>
> -# Dependency is either on libgcrypt or openssl, guaranteed in Config.in.
> -# Favour libgcrypt.
> -ifeq ($(BR2_PACKAGE_LIBGCRYPT),y)
> +ifeq ($(BR2_PACKAGE_LIBSSH_MBEDTLS),y)
> +LIBSSH_CONF_OPTS += -DWITH_MBEDTLS=ON
> +LIBSSH_DEPENDENCIES += mbedtls
> +else ifeq ($(BR2_PACKAGE_LIBSSH_LIBGCRYPT),y)
>  LIBSSH_CONF_OPTS += -DWITH_GCRYPT=ON
>  LIBSSH_DEPENDENCIES += libgcrypt
> -else
> -LIBSSH_CONF_OPTS += -DWITH_GCRYPT=OFF
> +else ifeq ($(BR2_PACKAGE_LIBSSH_OPENSSL),y)
>  LIBSSH_DEPENDENCIES += openssl
>  endif
>
> --
> 2.23.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20191002/b4d2290a/attachment.html>

[Buildroot] [PATCH v2] package/libssh: add support for mbedtls crypto backend

[Peter Korsgaard]

>>>>> "Mircea" == Mircea Gliga <gliga.mircea@gmail.com> writes:

 > At this point Buildroot doesn't allow to use mbedTLS crypto
 > backend even though libssh supports it. In case of fully statically
 > linked ELF executables the size difference between OpenSSL and mbedTLS
 > is significant: it matters for embedded targets with very limited
 > storage.

 > This patch adds support for compiling libssh with mbedTLS as a
 > crypto backend. It also allows the selection of the crypto backend
 > libssh will use through a choice in the package config.

 > Currently, the selection of the backend is based on a priority order,
 > which is not always desirable, as in some cases multiple backends
 > can exists at the same time for various reasons.

 > Switch to OpenSSL as the default crypto backend, instead of libgcrypt,
 > since OpenSSL is more commonly used.

 > Signed-off-by: Mircea Gliga <gliga.mircea@gmail.com>

 > ---
 > Changes V1->V2:
 > * choice now selects the crypto provider package
 > * more detailed description in commit message
 > * switch default crypto backend to OpenSSL

Committed after changing the selects to depends on to match libssh2,
thanks and sorry for the slow response.

-- 
Bye, Peter Korsgaard