Re: [PATCH v3 6/6] If we use a custom keyring debootstrap may fall to https

[Henning Schild <henning.schild@siemens.com>]

Am Thu, 7 Mar 2019 10:14:39 +0100
schrieb Andreas Reichel <andreas.reichel.ext@siemens.com>:

> On Wed, Mar 06, 2019 at 06:43:11PM +0100, Henning Schild wrote:
> > Am Wed, 6 Mar 2019 17:26:18 +0100
> > schrieb "[ext] Andreas J. Reichel"
> > <andreas.reichel.ext@siemens.com>: 
> > > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > 
> > > See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891009
> > > 
> > > So if we have something in aptkeyring, append https-support to
> > > OVERRIDES.
> > > 
> > > Furthermore, the conditional append for https-support was missing
> > > in qemuamd64-stretch.conf, thus, remove this from all the distros
> > > and put it into the isar-bootstrap.inc.
> > > 
> > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > ---
> > >  meta-isar/conf/multiconfig/qemuamd64-buster.conf    |  1 -
> > >  meta-isar/conf/multiconfig/qemuamd64-jessie.conf    |  1 -
> > >  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 11
> > > +++++++++++ 3 files changed, 11 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/meta-isar/conf/multiconfig/qemuamd64-buster.conf
> > > b/meta-isar/conf/multiconfig/qemuamd64-buster.conf index
> > > 63df75c..da90993 100644 ---
> > > a/meta-isar/conf/multiconfig/qemuamd64-buster.conf +++
> > > b/meta-isar/conf/multiconfig/qemuamd64-buster.conf @@ -18,4 +18,3
> > > @@ QEMU_MACHINE ?= "q35" QEMU_CPU ?= ""
> > >  QEMU_DISK_ARGS ?= "-hda ##ROOTFS_IMAGE##
> > > -bios /usr/local/share/ovmf/OVMF.fd" 
> > > -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "
> > > apt-transport-https ca-certificates" diff --git
> > > a/meta-isar/conf/multiconfig/qemuamd64-jessie.conf
> > > b/meta-isar/conf/multiconfig/qemuamd64-jessie.conf index
> > > d1335ff..42c71df 100644 ---
> > > a/meta-isar/conf/multiconfig/qemuamd64-jessie.conf +++
> > > b/meta-isar/conf/multiconfig/qemuamd64-jessie.conf @@ -15,4 +15,3
> > > @@ QEMU_MACHINE ?= "pc" QEMU_CPU ?= "" QEMU_DISK_ARGS ?= "-hda
> > > ##ROOTFS_IMAGE##" 
> > > -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "
> > > apt-transport-https ca-certificates" diff --git
> > > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > > 8002a53..64cefc6 100644 ---
> > > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -25,6
> > > +25,7 @@ BASEAPTSRCS = "${WORKDIR}/base-apt-sources" APTKEYFILES
> > > = "" DEPLOY_ISAR_BOOTSTRAP ?= "" DISTRO_BOOTSTRAP_BASE_PACKAGES =
> > > "locales gnupg2"
> > > +DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "
> > > apt-transport-https ca-certificates" DISTRO_APT_PREMIRRORS ?=
> > > "${@ "http://ftp\.(\S+\.)?debian.org file:///${REPO_BASE_DIR} \n"
> > > if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO'))
> > > else "" }" @@ -41,6 +42,12 @@ python () { if own_pub_key:
> > >              aptkeys += own_pub_key.split()
> > >  
> > > +    if len(aptkeys) > 0:
> > > +        # debootstrap falls back to https if there is no
> > > +        # 'reliable' keyring, whatever that means, but it
> > > happened
> > > +        #
> > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891009
> > > +        d.setVar("HAVE_CUSTOM_APT_KEYS", "True")  
> > 
> > Why this indirection and the new variable? Can you not just check
> > len(aptkeys) where you check HAVE_CUSTOM_APT_KEYS?  
> Because aptkeys is a local variable. And we don't want that to be
> global.
> 
> > 
> > In fact you probably want to look at DISTRO_APT_KEYS, which seems
> > to be the source feeding all the other variables. Whatever you come
> > up with in v3 4/6.  
> Not quite, becase we have DISTRO_APT_KEYS as well as BASE_REPO_KEY.
> Both can contain several keys and feed aptkeys. (That's why I
> introduced aptkeys, to remove the duplicate code in the keyring
> generation.)

That is not your fault. But DISTRO_APT_KEYS should simply always have
BASE_REPO_KEY appended by default, which gets you back to one variable
that lists all repo keys. Remotes and the cache.

Henning

> >   
> > >      for key in aptkeys:
> > >          d.appendVar("SRC_URI", " %s" % key)
> > >          fetcher = bb.fetch2.Fetch([key], d)
> > > @@ -150,6 +157,10 @@ def get_distro_have_https_source(d,
> > > is_host=False): return any(source[2].startswith("https://") for
> > > source in generate_distro_sources(d, is_host)) 
> > >  def get_distro_needs_https_support(d, is_host=False):
> > > +    apt_keys = d.getVar("HAVE_CUSTOM_APT_KEYS", False)
> > > +    if apt_keys:
> > > +        return "https-support"
> > > +  
> > 
> > Reusing the OVERRIDE seems like a good idea to avoid double adding
> > the packages. So gnupg implies https but does not add the packages
> > again. But i think i would do something like
> > 
> > get_gnupg_overrides() 
> >  ret = "gnupg"
> >  # blabla bug
> >  ret += " https-support"
> >   
> Sure. That makes sense, but I would like to keep the https-support
> part in the other function, because that is there to tell all cases
> where the image needs https-support, no matter wether apt-keys or
> https sources...
> 
> Gnupg then should go into a new function analog to the other one like
> 'get_distro_needs_gnupg_support' to enable a new gnupg OVERRIDE>
> 
> Andreas
> > Henning
> >   
> > >      if get_distro_have_https_source(d, is_host):
> > >          return "https-support"
> > >      else:  
> >   
>