[elbe-devel] [PATCH 7/9] Eliminate most apt-key calls

[John Ogness]

On 2023-03-04, Bastian Germann <bage at linutronix.de> wrote:
> apt-key is deprecated and calls to gpg executables, which is especially
> bad in Elbe's target context. It makes sure to add the keyring in its
> argument to apt's trusted keys and unarmors the keyring on the way.
>
> Replace ap-key with unarmor_openpgp_keyring conversions and file writes

          apt-key

> to /etc/apt/trusted.gpg.d/.
>
> Signed-off-by: Bastian Germann <bage at linutronix.de>
> ---
>  elbepack/rfs.py     | 45 ++++++++++++++++++++++-----------------------
>  elbepack/virtapt.py | 18 ++++++++++--------
>  2 files changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/elbepack/rfs.py b/elbepack/rfs.py
> index c8cc07736b..6978fa5d9b 100644
> --- a/elbepack/rfs.py
> +++ b/elbepack/rfs.py
> @@ -212,8 +211,7 @@ class BuildEnv:
>              try:
>                  self.cdrom_mount()
>                  if keyring:
> -                    do(f'apt-key --keyring "{self.rfs.fname("/elbe.keyring")}" '
> -                       f'add "{self.rfs.fname("cdrom")}/targetrepo/repo.pub"')
> +                    self.convert_asc_to_gpg("cdrom/targetrepo/repo.pub", "/elbe.keyring")

The other uses of convert_asc_to_gpg() are specifying absolute paths. I
realize that ELBE is horribly inconsistent with this, but this would be
a chance to make it consistent. Please use "/cdrom/targetrepo/repo.pub".

>                  do(cmd)
>              except CommandError:
>                  cleanup = True
> @@ -238,8 +236,7 @@ class BuildEnv:
>          try:
>              self.cdrom_mount()
>              if keyring:
> -                do(f'apt-key --keyring "{self.rfs.fname("/elbe.keyring")}" '
> -                   f'add "{self.rfs.fname("cdrom")}/targetrepo/repo.pub"')
> +                self.convert_asc_to_gpg("cdrom/targetrepo/repo.pub", "/elbe.keyring")

Ditto. Please use an absolute path.

>              do(cmd)
>  
>              ui = "/usr/share/elbe/qemu-elbe/" + self.xml.defs["userinterpr"]
> @@ -288,10 +287,10 @@ class BuildEnv:
>              # https://github.com/Linutronix/elbe/issues/220
>              #
>              # I could make a none global 'noauth' flag for mirrors
> -            for url in self.xml.node('project/mirror/url-list'):
> +            for i, url in enumerate(self.xml.node('project/mirror/url-list')):
>                  if url.has('raw-key'):
>                      key = "\n".join(line.strip(" \t") for line in url.text('raw-key').splitlines()[1:-1])
> -                    self.add_key(key)
> +                    self.add_key(unarmor_openpgp_keyring(key), f"raw-key{i}.gpg")

I don't expect any collisions, but maybe name the file
"elbe-xml-raw-key{i}.gpg" so that it has an ELBE-specific name? (Just a
suggestion. Feel free to ignore it.)

> diff --git a/elbepack/virtapt.py b/elbepack/virtapt.py
> index 16c20c9845..64e3cecdbb 100644
> --- a/elbepack/virtapt.py
> +++ b/elbepack/virtapt.py
> @@ -140,12 +141,13 @@ class VirtApt:
>          self.downloads = {}
>          self.acquire = apt_pkg.Acquire(self)
>  
> -    def add_key(self, key):
> -        system(f'echo "{key}" > {self.basefs.fname("tmp/key.pub")}')
> -        system(f'fakeroot apt-key '
> -               f'--keyring "{self.basefs.fname("/etc/apt/trusted.gpg")}" '
> -               f'add "{self.basefs.fname("tmp/key.pub")}"')
> -        system(f'rm -f {self.basefs.fname("tmp/key.pub")}')
> +    def add_key(self, key, keyname):
> +        """
> +        Adds the binary OpenPGP keyring 'key' as a trusted apt keyring
> +        with file name 'keyname'.
> +        """
> +        with open(self.basefs.fname(f"/etc/apt/trusted.gpg.d/{keyname}"), "wb") as outfile:
> +            outfile.write(key)

(I'm turning a blind eye to this copy/paste code.)

> @@ -155,10 +157,10 @@ class VirtApt:
>              # https://github.com/Linutronix/elbe/issues/220
>              #
>              # I could make a none global 'noauth' flag for mirrors
> -            for url in self.xml.node('project/mirror/url-list'):
> +            for i, url in enumerate(self.xml.node('project/mirror/url-list')):
>                  if url.has('raw-key'):
>                      key = "\n".join(line.strip(" \t") for line in url.text('raw-key').splitlines()[1:-1])
> -                    self.add_key(key)
> +                    self.add_key(unarmor_openpgp_keyring(key), f"raw-key{i}.gpg")

I wonder if these keys should be named differently. Maybe
"elbe-virtapt-raw-key{i}.gpg"? Technically it isn't necessary, but it
might be useful when debugging an ELBE explosion.

John