| Message ID | 20200511223108.4184-1-edo.rus@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/openvpn: add option to use mbed TLS instead of OpenSSL | expand |
On Tue, May 12, 2020 at 1:31 AM Ed Spiridonov <edo.rus@gmail.com> wrote: > > Since 2.4 version, OpenVPN can be built using mbeb TLS as it's > crypto backend, instead of OpenSSL. > > About 2 Mb of uncompressed image size can be saved by replacing > OpenSSL with mbed TLS. Is anybody interested in this patch? I'm going to publish another patch to make some features optional (server side, multihome, etc), but I guess it will be ignored like this one.
Hi Ed, Am Fr., 15. Mai 2020 um 19:30 Uhr schrieb Ed Spiridonov <edo.rus@gmail.com>: > On Tue, May 12, 2020 at 1:31 AM Ed Spiridonov <edo.rus@gmail.com> wrote: > > > > Since 2.4 version, OpenVPN can be built using mbeb TLS as it's > > crypto backend, instead of OpenSSL. > > > > About 2 Mb of uncompressed image size can be saved by replacing > > OpenSSL with mbed TLS. > > Is anybody interested in this patch? > I'm going to publish another patch to make some features optional > (server side, multihome, etc), but I guess it will be ignored like > this one. > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot I think the maintainers are busy preparing the upcoming release. And on the other hand the backlog [1] is full with more than 400 pending patches. So please be patient ;-/ [1] https://patchwork.ozlabs.org/project/buildroot/list/
Hello Ed, On Tue, 12 May 2020 01:31:07 +0300 Ed Spiridonov <edo.rus@gmail.com> wrote: > Since 2.4 version, OpenVPN can be built using mbeb TLS as it's > crypto backend, instead of OpenSSL. > > About 2 Mb of uncompressed image size can be saved by replacing > OpenSSL with mbed TLS. > > Signed-off-by: Ed Spiridonov <edo.rus@gmail.com> Thanks for your contribution! See below for some comments. > diff --git a/package/openvpn/Config.in b/package/openvpn/Config.in > index 0a16755..254fe74 100644 > --- a/package/openvpn/Config.in > +++ b/package/openvpn/Config.in > @@ -1,7 +1,6 @@ > config BR2_PACKAGE_OPENVPN > bool "openvpn" > depends on BR2_USE_MMU # fork() > - select BR2_PACKAGE_OPENSSL Could you change this to: select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_MBEDTLS > +choice > + prompt "crypto backend" > + default BR2_PACKAGE_OPENVPN_OPENSSL > + help > + Select crypto backend (OpenSSL/LibreSSL or mbed TLS) > + > +config BR2_PACKAGE_OPENVPN_OPENSSL > + bool "openssl" > + select BR2_PACKAGE_OPENSSL > + help > + OpenSSL/LibreSSL is a default crypto backend > + > +config BR2_PACKAGE_OPENVPN_MBEDTLS > + bool "mbedtls" > + select BR2_PACKAGE_MBEDTLS > + help > + mbed TLS is a compact crypto backend > + > + https://community.openvpn.net/openvpn/wiki/Using-mbedtls > + > +endchoice Drop this new choice. > diff --git a/package/openvpn/openvpn.mk b/package/openvpn/openvpn.mk > index 4234675..20cebf0 100644 > --- a/package/openvpn/openvpn.mk > +++ b/package/openvpn/openvpn.mk > @@ -7,18 +7,31 @@ > OPENVPN_VERSION = 2.4.9 > OPENVPN_SOURCE = openvpn-$(OPENVPN_VERSION).tar.xz > OPENVPN_SITE = http://swupdate.openvpn.net/community/releases > -OPENVPN_DEPENDENCIES = host-pkgconf openssl > +OPENVPN_DEPENDENCIES = host-pkgconf > +ifeq ($(BR2_PACKAGE_OPENVPN_MBEDTLS),y) > +OPENVPN_DEPENDENCIES += mbedtls > +else > +OPENVPN_DEPENDENCIES += openssl > +endif > + > OPENVPN_LICENSE = GPL-2.0 > OPENVPN_LICENSE_FILES = COPYRIGHT.GPL > OPENVPN_CONF_OPTS = \ > --enable-iproute2 \ > - --with-crypto-library=openssl \ > $(if $(BR2_STATIC_LIBS),--disable-plugins) > OPENVPN_CONF_ENV = IFCONFIG=/sbin/ifconfig \ > NETSTAT=/bin/netstat \ > ROUTE=/sbin/route \ > IPROUTE=/sbin/ip > > +ifeq ($(BR2_PACKAGE_OPENVPN_MBEDTLS),y) Use BR2_PACKAGE_MBEDTLS here > +OPENVPN_CONF_OPTS += \ > + --with-crypto-library=mbedtls > +else > +OPENVPN_CONF_OPTS += \ > + --with-crypto-library=openssl > +endif This way, we use mbedtls if available, otherwise we use OpenSSL. Could you send an updated version that implements this? Thanks a lot! Thomas
On Fri, May 15, 2020 at 11:10 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Could you change this to: > > select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_MBEDTLS Ok. But I would rather use mbed TLS as the default option: select BR2_PACKAGE_MBEDTLS if !BR2_PACKAGE_OPENSSL OpenVPN + mbed TLS combination is well tested (Android/iOS builds use mbed TLS). > This way, we use mbedtls if available, otherwise we use OpenSSL. BTW, there is "--disable-crypto" build option, but I'm unsure if anyone use it. P. S. What about other options? OpenWRT has lot of them https://github.com/openwrt/openwrt/blob/master/package/network/services/openvpn/Config-openssl.in This helps make the binary more compact.
Hello Ed, On Fri, 15 May 2020 23:39:09 +0300 Ed Spiridonov <edo.rus@gmail.com> wrote: > On Fri, May 15, 2020 at 11:10 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > Could you change this to: > > > > select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_MBEDTLS > > Ok. But I would rather use mbed TLS as the default option: > select BR2_PACKAGE_MBEDTLS if !BR2_PACKAGE_OPENSSL > > OpenVPN + mbed TLS combination is well tested (Android/iOS builds use mbed TLS). The idea of using select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_MBEDTLS was to keep the current behavior, i.e be backward compatible. > > This way, we use mbedtls if available, otherwise we use OpenSSL. > > BTW, there is "--disable-crypto" build option, but I'm unsure if anyone use it. > > P. S. What about other options? OpenWRT has lot of them > https://github.com/openwrt/openwrt/blob/master/package/network/services/openvpn/Config-openssl.in > This helps make the binary more compact. Feel free to add support for more options, patches welcome. Thanks! Thomas
Hello, On Sat, 16 May 2020 00:03:10 +0300 Ed Spiridonov <edo.rus@gmail.com> wrote: > > The idea of using select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_MBEDTLS > > was to keep the current behavior, i.e be backward compatible. > > Does it make sense? > If OpenSSL is selected, it will be used as a crypto backend. So any > build based on an existing .config remains the same. What you say will work if: (1) Your .mk file tests BR2_PACKAGE_OPENSSL and uses openssl if set, before using mbedtls (2) Users are using full .config and not defconfig files. Indeed, a defconfig file today that has BR2_PACKAGE_OPENVPN=y will not have BR2_PACKAGE_OPENSSL=y, because this is implied by BR2_PACKAGE_OPENVPN=y. So such users would transition from using OpenSSL as the crypto backend for openvpn to mbedtls. I don't have a very strong feeling on this. I agree that on the other hand, it's good to use a smaller crypto library by default if possible. Thomas
On Fri, May 15, 2020 at 11:10 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Could you change this to: > > select BR2_PACKAGE_OPENSSL if !BR2_PACKAGE_MBEDTLS > > > +choice > > + prompt "crypto backend" > > + default BR2_PACKAGE_OPENVPN_OPENSSL > > + help > > + Select crypto backend (OpenSSL/LibreSSL or mbed TLS) > > + > > +config BR2_PACKAGE_OPENVPN_OPENSSL > > + bool "openssl" > > + select BR2_PACKAGE_OPENSSL > > + help > > + OpenSSL/LibreSSL is a default crypto backend > > + > > +config BR2_PACKAGE_OPENVPN_MBEDTLS > > + bool "mbedtls" > > + select BR2_PACKAGE_MBEDTLS > > + help > > + mbed TLS is a compact crypto backend > > + > > + https://community.openvpn.net/openvpn/wiki/Using-mbedtls > > + > > +endchoice > > Drop this new choice. I thought a bit about this. Any argument against the choice? IMHO the choice is clear and understandable. The user explicitly selects the crypto backend. He knows what is going on. In the case proposed by you, OpenSSL is selected automagically without ability to unselect (until mbed TLS is selected manually). How can the user find out that mbed TLS could be used instead of OpenSSL? Press help on OpenVPN item? Kconfig shows dependencies here, but not a condition (if !BR2_PACKAGE_MBEDTLS). Look into package/openvpn/Config.in? Of course, detailed explanation could be added into OpenVPN help. But nobody reads help for *all* selected items.
Hello Ed, On Mon, 18 May 2020 04:45:35 +0300 Ed Spiridonov <edo.rus@gmail.com> wrote: > In the case proposed by you, OpenSSL is selected automagically without > ability to unselect (until mbed TLS is selected manually). > How can the user find out that mbed TLS could be used instead of OpenSSL? > Press help on OpenVPN item? Kconfig shows dependencies here, but not a > condition (if !BR2_PACKAGE_MBEDTLS). > Look into package/openvpn/Config.in? > Of course, detailed explanation could be added into OpenVPN help. But > nobody reads help for *all* selected items. All what you're saying here is valid for a huge number of packages where Buildroot automatically makes use of optional dependencies if they are available. For example: ifeq ($(BR2_PACKAGE_XLIB_LIBXRANDR),y) LIBGTK3_CONF_OPTS += --enable-xrandr LIBGTK3_DEPENDENCIES += xlib_libXrandr else LIBGTK3_CONF_OPTS += --disable-xrandr endif libgtk3 will automatically have xrandr support when BR2_PACKAGE_XLIB_LIBXRANDR is enabled. The user has no way to "know" about it, except by reading libgtk3.mk. But this allows to limit the number of Config.in options we have. We already have thousands of them, and if we were to add such options for each optional dependency of all packages, we would have tens of thousands of Config.in options. There are however some packages where we add explicit sub-options, for various reasons, but that is not the general case. Best regards, Thomas
On Mon, May 18, 2020 at 8:15 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > All what you're saying here is valid for a huge number of packages > where Buildroot automatically makes use of optional dependencies if > they are available. > > For example: > > ifeq ($(BR2_PACKAGE_XLIB_LIBXRANDR),y) > LIBGTK3_CONF_OPTS += --enable-xrandr > LIBGTK3_DEPENDENCIES += xlib_libXrandr > else > LIBGTK3_CONF_OPTS += --disable-xrandr > endif > > libgtk3 will automatically have xrandr support when > BR2_PACKAGE_XLIB_LIBXRANDR is enabled. The user has no way to "know" > about it, except by reading libgtk3.mk. I agree, that looks reasonable. If someone wants xrandr support, he selects BR2_PACKAGE_XLIB_LIBXRANDR. But in the case of choosing openvpn crypto backend, I would prefer an explicit choice. IMO mbed TLS should be the default option because it is much more compact. But someone could choose OpenSSL because it is more mature. BTW, I tried "select BR2_PACKAGE_MBEDTLS if !BR2_PACKAGE_OPENSSL", it doesn't work. https://www.spinics.net/lists/linux-kbuild/msg25569.html So I do not know how to make mbed TLS the default choice.
On Mon, May 18, 2020 at 8:15 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > There are however some packages where we add explicit sub-options, for > various reasons, but that is not the general case. 1. Is eliminating the explicit choice of a crypto backend required for the patch approval? 2. Is it ok to provide Mbed TLS as the default choice?
On Fri, May 15, 2020 at 10:17 PM Heiko Thiery <heiko.thiery@gmail.com> wrote: > > Hi Ed, >> > About 2 Mb of uncompressed image size can be saved by replacing >> > OpenSSL with mbed TLS. >> >> Is anybody interested in this patch? >> I'm going to publish another patch to make some features optional >> (server side, multihome, etc), but I guess it will be ignored like > > I think the maintainers are busy preparing the upcoming release. And on the other hand the backlog [1] is full with more than 400 pending patches. > > So please be patient ;-/ The situation doesn't seem to be improving. Now there are 500+ pending patches.
I'm still waiting for some reply. On Fri, Jul 10, 2020 at 9:39 PM Ed Spiridonov <edo.rus@gmail.com> wrote: > > On Mon, May 18, 2020 at 8:15 AM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > There are however some packages where we add explicit sub-options, for > > various reasons, but that is not the general case. > > 1. Is eliminating the explicit choice of a crypto backend required for > the patch approval? > 2. Is it ok to provide Mbed TLS as the default choice?
diff --git a/DEVELOPERS b/DEVELOPERS index e427ab1..3437f31 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -719,6 +719,9 @@ F: package/unscd/ N: Dushara Jayasinghe <nidujay@gmail.com> F: package/prosody/ +N: Ed Spiridonov <edo.rus@gmail.com> +F: package/openvpn/ + N: Eloi Bail <eloi.bail@savoirfairelinux.com> F: package/bayer2rgb-neon/ F: package/gstreamer1/gst1-plugins-bayer2rgb-neon/ diff --git a/package/openvpn/Config.in b/package/openvpn/Config.in index 0a16755..254fe74 100644 --- a/package/openvpn/Config.in +++ b/package/openvpn/Config.in @@ -1,7 +1,6 @@ config BR2_PACKAGE_OPENVPN bool "openvpn" depends on BR2_USE_MMU # fork() - select BR2_PACKAGE_OPENSSL help OpenVPN is a full-featured SSL VPN solution which can accomodate a wide range of configurations, including road @@ -14,6 +13,29 @@ config BR2_PACKAGE_OPENVPN if BR2_PACKAGE_OPENVPN +choice + prompt "crypto backend" + default BR2_PACKAGE_OPENVPN_OPENSSL + help + Select crypto backend (OpenSSL/LibreSSL or mbed TLS) + +config BR2_PACKAGE_OPENVPN_OPENSSL + bool "openssl" + select BR2_PACKAGE_OPENSSL + help + OpenSSL/LibreSSL is a default crypto backend + +config BR2_PACKAGE_OPENVPN_MBEDTLS + bool "mbedtls" + select BR2_PACKAGE_MBEDTLS + help + mbed TLS is a compact crypto backend + + https://community.openvpn.net/openvpn/wiki/Using-mbedtls + +endchoice + + config BR2_PACKAGE_OPENVPN_LZ4 bool "LZ4 compression" default y diff --git a/package/openvpn/openvpn.mk b/package/openvpn/openvpn.mk index 4234675..20cebf0 100644 --- a/package/openvpn/openvpn.mk +++ b/package/openvpn/openvpn.mk @@ -7,18 +7,31 @@ OPENVPN_VERSION = 2.4.9 OPENVPN_SOURCE = openvpn-$(OPENVPN_VERSION).tar.xz OPENVPN_SITE = http://swupdate.openvpn.net/community/releases -OPENVPN_DEPENDENCIES = host-pkgconf openssl +OPENVPN_DEPENDENCIES = host-pkgconf +ifeq ($(BR2_PACKAGE_OPENVPN_MBEDTLS),y) +OPENVPN_DEPENDENCIES += mbedtls +else +OPENVPN_DEPENDENCIES += openssl +endif + OPENVPN_LICENSE = GPL-2.0 OPENVPN_LICENSE_FILES = COPYRIGHT.GPL OPENVPN_CONF_OPTS = \ --enable-iproute2 \ - --with-crypto-library=openssl \ $(if $(BR2_STATIC_LIBS),--disable-plugins) OPENVPN_CONF_ENV = IFCONFIG=/sbin/ifconfig \ NETSTAT=/bin/netstat \ ROUTE=/sbin/route \ IPROUTE=/sbin/ip +ifeq ($(BR2_PACKAGE_OPENVPN_MBEDTLS),y) +OPENVPN_CONF_OPTS += \ + --with-crypto-library=mbedtls +else +OPENVPN_CONF_OPTS += \ + --with-crypto-library=openssl +endif + ifeq ($(BR2_PACKAGE_OPENVPN_SMALL),y) OPENVPN_CONF_OPTS += \ --enable-small \
Since 2.4 version, OpenVPN can be built using mbeb TLS as it's crypto backend, instead of OpenSSL. About 2 Mb of uncompressed image size can be saved by replacing OpenSSL with mbed TLS. Signed-off-by: Ed Spiridonov <edo.rus@gmail.com> --- DEVELOPERS | 3 +++ package/openvpn/Config.in | 24 +++++++++++++++++++++++- package/openvpn/openvpn.mk | 17 +++++++++++++++-- 3 files changed, 41 insertions(+), 3 deletions(-)