HTTPS performance issue with opkg (Was: [PATCH 3/3] build: switch VERSION_REPO to HTTPS)

[Paul Spooren]

On Mon Sep 14, 2020 at 10:23 PM HST, Daniel Golle wrote:
> On Tue, Sep 15, 2020 at 08:49:51AM +0200, Baptiste Jonglez wrote:
> > On 27-08-20, Paul Spooren wrote:
> > > The variable VERSION_REPO is used by opkg to download package(list)s.
> > > Now that the default installation support encrypted HTTP opkg should
> > > make use of it.
> > 
> > I wonder what is the performance impact of this?  Opkg forks a new wget
> > process to download each package, so the HTTPS connection is never reused.
> > 
> > Running a simple "time make image
> > PROFILE=mikrotik_routerboard-921gs-5hpacd-15s" with the ath79/mikrotik
> > imagebuilder results in a 50% increase of running time:
> > 
> > - with HTTPS: 32 seconds
> > - with HTTP: 22 seconds
> > 
> > (timing for the second run is shown, and dl/ is cleaned up before each run)
> > 
> > The overhead might be even more significant on a device, and the download
> > server probably sees more load from the large number of key exchange.
> > Anybody got any figures?
> > 
> > With HTTPS, opkg would really need connection reuse.  I don't think the
> > current situation is acceptable for a stable release, if only to avoid
> > high load on the download server.
>
> I suggest to revert that change as HTTPS doesn't do us a favour here.
> It prevents Web-Caches (squid and such), hurts performance and
> integrity should (at least now) anyway be ensured by SHA256 of the
> package contained in the ed25519 signed package list.
> In my opinion HTTPS even gives users a false sense of security, as it
> is up to a few hundred certification authorities to not compromise
> rather than just to a few keys shipped with OpenWrt.
> Plus the the complexity of the handshake, large choice of ciphers
> (rather than pinning SHA256 + ed25519), ...
> Even major distributions like Debian and ArchLinux make HTTPS optional
> and opt-in for their package download servers.
> Imho the disadvantages clearly outweight the benifits here.

I see some advantage in not sharing what packages (incl. versions) are
installed in images or running machines, however that maybe doesn't
outweighs the issues stated above.

Removing HTTPS right now means insecure package installations in
ImageBuilder created images, I suggest to avoid that.

Enabling signature checks right now in ImageBuilder doesn't work because
the local folder packages/ contains kmods, kernel and libc which are
required for image creation. This folder is dynamically put together on
ImageBuilder creation and the index is created during ImageBuilder
runtime locally, meaning no signatures are available.

My initial idea (mocked in a very poor patch) was to disable signature
checking for the special case "file:packages". Given a second thought a
per feed option could be used for those special cases, like
"src/trusted" instead of "src". Adding signature check exceptions is
however bad, therefore I'd like to suggest a third solution:

Instead of shipping the ImageBuilder with the packages/ folder
(containing kmods etc.) another (signed) feed should be added to the
ImageBuilders repositories.conf, similar to what's already added to
snapshot images:

src/gz openwrt_kmods http://downloads.openwrt.org/snapshots/targets/armvirt/64/kmods/5.4.65-1-397bac86dd45c4d97d5f09fd9def9671/

This folder contains all available kmods. The special packages `kernel`
and `libc` should be added there, which requires a little buildbot
patch.

Instead of installing the local available kernel_*.ipk[1] the kernel and
libc revision could be hardcoded, something which is already done for
e.g. the revision[2] in ImageBuilder and SDK.

In combination this allows smaller ImageBuilders only using remote
packages which are all signed.

When choosing to have a standalone ImageBuilder with all packages
available locally (CONFIG_IB_STANDALONE) then all feeds except the
packages/ folder are disabled, meaning the `option check_signature` can
be disabled independently of `CONFIG_SIGNED_PACKAGES`.

If this seems a feasible approach I'd submit the patches for Buildbot
and ImageBuilder.

[1]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/imagebuilder/files/Makefile;h=27d3cfa8dfd5e5b7de0465db2ebff277e2040a14;hb=HEAD#l156
[2]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/imagebuilder/Makefile;h=ad19ab2b53ffd08d91ffb86817e54fdb31279660;hb=HEAD#l79


>
>
> > 
> > Baptiste
> > 
> > > Suggested-by: Petr Štetiar <ynezz at true.cz>
> > > Suggested-by: Baptiste Jonglez <baptiste at bitsofnetworks.org>
> > > Signed-off-by: Paul Spooren <mail at aparcar.org>
> > > ---
> > >  include/version.mk                 | 2 +-
> > >  package/base-files/image-config.in | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/version.mk b/include/version.mk
> > > index 7d3c1ad640..b7f42e13bb 100644
> > > --- a/include/version.mk
> > > +++ b/include/version.mk
> > > @@ -32,7 +32,7 @@ VERSION_CODE:=$(call qstrip,$(CONFIG_VERSION_CODE))
> > >  VERSION_CODE:=$(if $(VERSION_CODE),$(VERSION_CODE),$(REVISION))
> > >  
> > >  VERSION_REPO:=$(call qstrip,$(CONFIG_VERSION_REPO))
> > > -VERSION_REPO:=$(if $(VERSION_REPO),$(VERSION_REPO),http://downloads.openwrt.org/snapshots)
> > > +VERSION_REPO:=$(if $(VERSION_REPO),$(VERSION_REPO),https://downloads.openwrt.org/snapshots)
> > >  
> > >  VERSION_DIST:=$(call qstrip,$(CONFIG_VERSION_DIST))
> > >  VERSION_DIST:=$(if $(VERSION_DIST),$(VERSION_DIST),OpenWrt)
> > > diff --git a/package/base-files/image-config.in b/package/base-files/image-config.in
> > > index 4bace77db0..bfa3055cc8 100644
> > > --- a/package/base-files/image-config.in
> > > +++ b/package/base-files/image-config.in
> > > @@ -183,7 +183,7 @@ if VERSIONOPT
> > >  	config VERSION_REPO
> > >  		string
> > >  		prompt "Release repository"
> > > -		default "http://downloads.openwrt.org/snapshots"
> > > +		default "https://downloads.openwrt.org/snapshots"
> > >  		help
> > >  			This is the repository address embedded in the image, it defaults
> > >  			to the trunk snapshot repo; the url may contain the following placeholders:
>
>
>
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel at lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel