Re: [PATCH v2] isar-bootstrap: Run gpg-agent before starting apt-key

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

Am Tue, 19 Jan 2021 14:20:01 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> From: Yuri Adamov <yadamov@ilbers.de>
> 
> Building rpi-stretch natively (under qemu) sometimes fails with:
> 
> gpg: can't connect to the agent: IPC connect call failed
> 
> gpg starts gpg-agent and times out after 5 s. This value is
> hard-coded.
> 
> Besides, leaving running gpg-agent processes is not clean and prevents
> unmounting of filesystems.
> 
> This patch starts and stops the agent manually.
> 
> gnupg now appended to package list unconditionally because gpg-agent
> is used in every isar_bootstrap run.
> 
> Signed-off-by: Yuri Adamov <yadamov@ilbers.de>
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
> Changes since v1:
> - Removed unnecessary sleeping.
> - Removed -9 in kill.
> - Commented unconditionally gnupg package append.
> - Removed unused OVERRIDES_append and get_distro_needs_gpg_support().
> ---
>  .../isar-bootstrap/isar-bootstrap.inc         | 22
> +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> 8f5f727..751980f 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -24,7 +24,7
> @@ DISTRO_BOOTSTRAP_KEYFILES = "" THIRD_PARTY_APT_KEYFILES = ""
>  DEPLOY_ISAR_BOOTSTRAP ?= ""
>  DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_gnupg = ",gnupg"
> +DISTRO_BOOTSTRAP_BASE_PACKAGES_append = ",gnupg"
>  DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support =
> "${@https_support(d)}" 
>  inherit deb-dl-dir
> @@ -175,16 +175,6 @@ def get_distro_needs_https_support(d,
> is_host=False): else:
>          return ""
>  
> -def get_distro_needs_gpg_support(d):
> -    apt_keys = d.getVar("DISTRO_BOOTSTRAP_KEYS") or ""
> -    apt_keys += " " + (d.getVar("THIRD_PARTY_APT_KEYS") or "")
> -    apt_keys += " " + (d.getVar("BASE_REPO_KEY") or "")
> -    if apt_keys != " ":
> -        return "gnupg"
> -    return ""
> -
> -OVERRIDES_append = ":${@get_distro_needs_gpg_support(d)}"
> -
>  def get_distro_source(d, is_host):
>      return get_distro_primary_source_entry(d, is_host)[0]
>  
> @@ -309,14 +299,22 @@ isar_bootstrap() {
>              mkdir -p "${ROOTFSDIR}/etc/apt/apt.conf.d"
>              install -v -m644 "${WORKDIR}/isar-apt.conf" \
>                               "${ROOTFSDIR}/etc/apt/apt.conf.d/50isar.conf"
> +            MY_GPGHOME=$(chroot "${ROOTFSDIR}" mktemp -d
> /tmp/gpghomeXXXXXXXXXX)
> +            echo "Created temporary directory ${MY_GPGHOME} for
> gpg-agent"

It is probably better to "export GNUPGHOME" and skip the extra argument
to all the calls, this way we can not forget that argument on some
calls.

> +            chroot "${ROOTFSDIR}" gpg-agent --homedir
> "${MY_GPGHOME}" --daemon find ${APT_KEYS_DIR}/ -type f | while read
> keyfile do
>                  kfn="$(basename $keyfile)"
>                  cp $keyfile "${ROOTFSDIR}/tmp/$kfn"
>                  chroot "${ROOTFSDIR}" /usr/bin/apt-key \
> -                   --keyring ${THIRD_PARTY_APT_KEYRING} add
> "/tmp/$kfn"
> +                    --keyring ${THIRD_PARTY_APT_KEYRING} \
> +                    --homedir ${MY_GPGHOME} add "/tmp/$kfn"
>                  rm "${ROOTFSDIR}/tmp/$kfn"
>              done
> +            GPG_AGENT_PID=$(ps -aux | grep
> "gpg-agent.*${MY_GPGHOME}" | grep -v grep | awk  '{print $2}')
> +            echo "Killing gpg-agent with pid $GPG_AGENT_PID"
> +            /bin/kill ${GPG_AGENT_PID}

That kill is better done with "gpgconf --kill gpg-agent". In fact you
should kill always before the first use, in case something fails and a
second run finds a running agent.

> +            chroot "${ROOTFSDIR}" /bin/rm -rf "${MY_GPGHOME}"

this should be guarded making sure MY_GPGHOME is indeed a directory
with a matching name ... would be a pity if for some reason it would be
i.e. "/usr" or even only "/tmp"

Henning

>              if [ "${@get_distro_suite(d, True)}" = "stretch" ] && [
> "${@get_host_release().split('.')[0]}" -lt "4" ]; then install -v
> -m644 "${WORKDIR}/isar-apt-fallback.conf" \