public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Fixes usage of additional apt keys
@ 2019-02-27 15:18 Andreas J. Reichel
  2019-02-27 15:18 ` [PATCH v2 1/3] Fix and simplify apt keyring generation Andreas J. Reichel
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Andreas J. Reichel @ 2019-02-27 15:18 UTC (permalink / raw)
  To: isar-users; +Cc: Andreas Reichel

From: Andreas Reichel <andreas.reichel.ext@siemens.com>

If the user does not want to replace the bootstrap source together with
the key, additional keys did not work.

This is solved by not replacing the keyring but to add keys to
/ect/apt/trusted.gpg.d/isar.gpg, where debootstrap and any apt call
can find it.

Furthermore, the code to add keys is simplified a lot by not manually
parsing URIs and guessing about download locations as well as
not manually handling gpg and giving apt config overrides.

It is much simpler by using `apt-key` and default apt keyring paths.

Furthermore, apt-get must not use a given single source list which was
used from debootstrapping. Otherwise, additional packages are always
unauthenticated, which is a quite misleading error. Instead, apt-get
should use all source lists available in the built root.

Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>

Andreas Reichel (3):
  Fix and simplify apt keyring generation
  Use all source lists in target root apt
  Separate apt-key entries from default keyring

 meta/classes/isar-bootstrap-helper.bbclass    | 17 ++++++--
 meta/classes/isar-image.bbclass               |  1 +
 .../isar-bootstrap/isar-bootstrap-host.bb     |  2 +-
 .../isar-bootstrap/isar-bootstrap.inc         | 39 +++++++++----------
 4 files changed, 33 insertions(+), 26 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-02-27 15:18 [PATCH v2 0/3] Fixes usage of additional apt keys Andreas J. Reichel
@ 2019-02-27 15:18 ` Andreas J. Reichel
  2019-02-27 16:12   ` Henning Schild
  2019-02-27 15:18 ` [PATCH v2 2/3] Use all source lists in target root apt Andreas J. Reichel
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Andreas J. Reichel @ 2019-02-27 15:18 UTC (permalink / raw)
  To: isar-users; +Cc: Andreas Reichel

From: Andreas Reichel <andreas.reichel.ext@siemens.com>

Different fetcher stored keys in different sub dirs, and we can never
be sure about where downloaded files go.
To avoid this without making any assumptions, we ask the fetcher where
the file will be after it is downloaded. This way we also don't need to
parse the URL manually.

The code is simplified by removing duplicate code and using apt-key
instead of manually calling gpg.

Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
---
 meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
 .../isar-bootstrap/isar-bootstrap.inc         | 39 +++++++++----------
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/meta/classes/isar-bootstrap-helper.bbclass b/meta/classes/isar-bootstrap-helper.bbclass
index d780b85..b8c41f9 100644
--- a/meta/classes/isar-bootstrap-helper.bbclass
+++ b/meta/classes/isar-bootstrap-helper.bbclass
@@ -119,6 +119,16 @@ setup_root_file_system() {
     export LANG=C
     export LANGUAGE=C
     export LC_ALL=C
+
+    if [ -d ${TMPDIR}/aptkeys ]; then
+        for keyfile in ${TMPDIR}/aptkeys/*
+        do
+            kfn="tmp/$(basename $keyfile)"
+            cp $keyfile "$ROOTFSDIR/$kfn"
+            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add "/$kfn"
+            rm "$ROOTFSDIR/$kfn"
+        done
+    fi
     sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
         -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
         -o Dir::Etc::sourceparts="-" \
@@ -128,6 +138,7 @@ setup_root_file_system() {
         sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg --add-architecture ${DISTRO_ARCH}
         sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update
     fi
+    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
     sudo -E chroot "$ROOTFSDIR" \
         /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
             ${IMAGE_TRANSIENT_PACKAGES}
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index 234d339..2ef3b1e 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -23,35 +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources"
 APTSRCS_INIT = "${WORKDIR}/apt-sources-init"
 BASEAPTSRCS = "${WORKDIR}/base-apt-sources"
 APTKEYFILES = ""
-APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
-DEBOOTSTRAP_KEYRING = ""
 DEPLOY_ISAR_BOOTSTRAP ?= ""
-DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
+DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2 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 "" }"
 
 inherit base-apt-helper
 
 python () {
-    from urllib.parse import urlparse
     distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
-    wd = d.getVar("WORKDIR", True)
+    aptkeys = []
+
     if distro_apt_keys:
-        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
-        for key in distro_apt_keys.split():
-            url = urlparse(key)
-            filename = ''.join([wd, url.path])
-            d.appendVar("SRC_URI", " %s" % key)
-            d.appendVar("APTKEYFILES", " %s" % filename)
+        aptkeys += distro_apt_keys.split()
+
     if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
         own_pub_key = d.getVar("BASE_REPO_KEY", False)
         if own_pub_key:
-            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
-            for key in own_pub_key.split():
-                url = urlparse(key)
-                filename = ''.join([wd, url.path])
-                d.appendVar("SRC_URI", " %s" % key)
-                d.appendVar("APTKEYFILES", " %s" % filename)
+            aptkeys += own_pub_keys.split()
+
+    for key in aptkeys:
+        d.appendVar("SRC_URI", " %s" % key)
+        fetcher = bb.fetch2.Fetch([key], d)
+        filename = fetcher.localpath(key)
+        d.appendVar("APTKEYFILES", " %s" % filename)
 }
 
 def aggregate_files(d, file_list, file_out):
@@ -174,13 +169,17 @@ def get_distro_components_argument(d, is_host):
     else:
         return ""
 
+APTKEYTMPDIR := "${TMPDIR}/aptkeys"
+
+do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
 do_generate_keyring[dirs] = "${DL_DIR}"
 do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
 do_generate_keyring() {
     if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
+        chmod 777 "${APTKEYTMPDIR}"
         for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
-           gpg --no-default-keyring --keyring "${APTKEYRING}" \
-               --no-tty --homedir "${DL_DIR}"  --import "$keyfile"
+           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
+           sudo apt-key add "$keyfile"
         done
     fi
 }
@@ -222,7 +221,6 @@ isar_bootstrap() {
             if [ ${IS_HOST} ]; then
                 ${DEBOOTSTRAP} $debootstrap_args \
                                ${@get_distro_components_argument(d, True)} \
-                               ${DEBOOTSTRAP_KEYRING} \
                                "${@get_distro_suite(d, True)}" \
                                "${ROOTFSDIR}" \
                                "${@get_distro_source(d, True)}"
@@ -231,7 +229,6 @@ isar_bootstrap() {
                  "${DEBOOTSTRAP}" $debootstrap_args \
                                   --arch="${DISTRO_ARCH}" \
                                   ${@get_distro_components_argument(d, False)} \
-                                  ${DEBOOTSTRAP_KEYRING} \
                                   "${@get_distro_suite(d, False)}" \
                                   "${ROOTFSDIR}" \
                                   "${@get_distro_source(d, False)}"
-- 
2.21.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] Use all source lists in target root apt
  2019-02-27 15:18 [PATCH v2 0/3] Fixes usage of additional apt keys Andreas J. Reichel
  2019-02-27 15:18 ` [PATCH v2 1/3] Fix and simplify apt keyring generation Andreas J. Reichel
@ 2019-02-27 15:18 ` Andreas J. Reichel
  2019-02-27 15:18 ` [PATCH v2 3/3] Separate apt-key entries from default keyring Andreas J. Reichel
  2019-02-27 16:15 ` [PATCH v2 0/3] Fixes usage of additional apt keys Henning Schild
  3 siblings, 0 replies; 16+ messages in thread
From: Andreas J. Reichel @ 2019-02-27 15:18 UTC (permalink / raw)
  To: isar-users; +Cc: Andreas Reichel

From: Andreas Reichel <andreas.reichel.ext@siemens.com>

When we only use isar-apt.list, we cannot add additional repositories
since they are listed in the bootstrap list only.

Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
---
 meta/classes/isar-bootstrap-helper.bbclass | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/meta/classes/isar-bootstrap-helper.bbclass b/meta/classes/isar-bootstrap-helper.bbclass
index b8c41f9..26abf62 100644
--- a/meta/classes/isar-bootstrap-helper.bbclass
+++ b/meta/classes/isar-bootstrap-helper.bbclass
@@ -129,10 +129,7 @@ setup_root_file_system() {
             rm "$ROOTFSDIR/$kfn"
         done
     fi
-    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
-        -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
-        -o Dir::Etc::sourceparts="-" \
-        -o APT::Get::List-Cleanup="0"
+    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update
     # Add multiarch for cross-target
     if [ "${ROOTFS_ARCH}" != "${DISTRO_ARCH}" ]; then
         sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg --add-architecture ${DISTRO_ARCH}
-- 
2.21.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] Separate apt-key entries from default keyring
  2019-02-27 15:18 [PATCH v2 0/3] Fixes usage of additional apt keys Andreas J. Reichel
  2019-02-27 15:18 ` [PATCH v2 1/3] Fix and simplify apt keyring generation Andreas J. Reichel
  2019-02-27 15:18 ` [PATCH v2 2/3] Use all source lists in target root apt Andreas J. Reichel
@ 2019-02-27 15:18 ` Andreas J. Reichel
  2019-02-27 16:14   ` Henning Schild
  2019-02-27 16:15 ` [PATCH v2 0/3] Fixes usage of additional apt keys Henning Schild
  3 siblings, 1 reply; 16+ messages in thread
From: Andreas J. Reichel @ 2019-02-27 15:18 UTC (permalink / raw)
  To: isar-users; +Cc: Andreas Reichel

From: Andreas Reichel <andreas.reichel.ext@siemens.com>

Per default, apt-key add adds keys to /etc/apt/trusted.gpg.
However, when building without a container, we don't want to contaminate
the host. Therefore, we specify a keyring file in /etc/apt/trusted.gpg.d
directory named `isar.gpg`. This file can be deleted after the build.

This is necessary because we don't want to specify single keyrings
to debootstrap since we might need a mixture of several keyrings
as per default and would have to find all needed keyrings on the system,
export their keys and reimport into the build keyring, which is more
complicated and unneeded this way.

Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
---
 meta/classes/isar-bootstrap-helper.bbclass              | 1 +
 meta/classes/isar-image.bbclass                         | 1 +
 meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb | 2 +-
 meta/recipes-core/isar-bootstrap/isar-bootstrap.inc     | 2 +-
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/meta/classes/isar-bootstrap-helper.bbclass b/meta/classes/isar-bootstrap-helper.bbclass
index 26abf62..769cbef 100644
--- a/meta/classes/isar-bootstrap-helper.bbclass
+++ b/meta/classes/isar-bootstrap-helper.bbclass
@@ -22,6 +22,7 @@ HOST_DISTRO ?= "debian-stretch"
 HOST_ARCH ?= "${@get_deb_host_arch()}"
 
 HOST_DISTRO_APT_SOURCES += "conf/distro/${HOST_DISTRO}.list"
+ISARKEYRING = "/etc/apt/trusted.gpg.d/isar.gpg"
 
 def reverse_bb_array(d, varname):
     array = d.getVar(varname, True)
diff --git a/meta/classes/isar-image.bbclass b/meta/classes/isar-image.bbclass
index cdd1651..4a89bd7 100644
--- a/meta/classes/isar-image.bbclass
+++ b/meta/classes/isar-image.bbclass
@@ -82,6 +82,7 @@ isar_image_cleanup() {
         fi
         rm -f "${IMAGE_ROOTFS}/etc/apt/sources-list"
     '
+    sudo rm -f "${ISARKEYRING}"
 }
 
 do_rootfs() {
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
index a793585..b70d2a8 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
@@ -11,8 +11,8 @@ WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${HOST_DISTRO}-${HOST_A
 DEPLOY_ISAR_BOOTSTRAP = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}"
 ISAR_BOOTSTRAP_LOCK = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}.lock"
 
-require isar-bootstrap.inc
 inherit isar-bootstrap-helper
+require isar-bootstrap.inc
 
 do_generate_keyring[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
 
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index 2ef3b1e..4613732 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -179,7 +179,7 @@ do_generate_keyring() {
         chmod 777 "${APTKEYTMPDIR}"
         for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
            cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
-           sudo apt-key add "$keyfile"
+           sudo apt-key --keyring "${ISARKEYRING}" add "$keyfile"
         done
     fi
 }
-- 
2.21.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-02-27 15:18 ` [PATCH v2 1/3] Fix and simplify apt keyring generation Andreas J. Reichel
@ 2019-02-27 16:12   ` Henning Schild
  2019-03-01  9:09     ` Andreas Reichel
                       ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Henning Schild @ 2019-02-27 16:12 UTC (permalink / raw)
  To: [ext] Andreas J. Reichel; +Cc: isar-users

Am Wed, 27 Feb 2019 16:18:54 +0100
schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:

> From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> 
> Different fetcher stored keys in different sub dirs, and we can never
> be sure about where downloaded files go.
> To avoid this without making any assumptions, we ask the fetcher where
> the file will be after it is downloaded. This way we also don't need
> to parse the URL manually.
> 
> The code is simplified by removing duplicate code and using apt-key
> instead of manually calling gpg.
> 
> Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> ---
>  meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
>  .../isar-bootstrap/isar-bootstrap.inc         | 39
> +++++++++---------- 2 files changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> b/meta/classes/isar-bootstrap-helper.bbclass index d780b85..b8c41f9
> 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> +++ b/meta/classes/isar-bootstrap-helper.bbclass
> @@ -119,6 +119,16 @@ setup_root_file_system() {
>      export LANG=C
>      export LANGUAGE=C
>      export LC_ALL=C
> +
> +    if [ -d ${TMPDIR}/aptkeys ]; then
> +        for keyfile in ${TMPDIR}/aptkeys/*
> +        do
> +            kfn="tmp/$(basename $keyfile)"
> +            cp $keyfile "$ROOTFSDIR/$kfn"
> +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add "/$kfn"

Would probably be a good idea to work in /tmp/ as well and in a subdir.
Try naming your key "root" or "usr" ...

> +            rm "$ROOTFSDIR/$kfn"
> +        done
> +    fi
>      sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
>          -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
>          -o Dir::Etc::sourceparts="-" \
> @@ -128,6 +138,7 @@ setup_root_file_system() {
>          sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg --add-architecture
> ${DISTRO_ARCH} sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update
>      fi
> +    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
>      sudo -E chroot "$ROOTFSDIR" \
>          /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
>              ${IMAGE_TRANSIENT_PACKAGES}
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> 234d339..2ef3b1e 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -23,35
> +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources" APTSRCS_INIT =
> "${WORKDIR}/apt-sources-init" BASEAPTSRCS =
> "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> -APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
> -DEBOOTSTRAP_KEYRING = ""
>  DEPLOY_ISAR_BOOTSTRAP ?= ""
> -DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> +DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2 apt-transport-https
> ca-certificates"

That looks more like leftovers from an experiment. Are you sure that is
needed?


In general i think that patch should be splittet into two or
even more patches. Because it solves at least two problems.

Henning

>  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 "" }"
> inherit base-apt-helper 
>  python () {
> -    from urllib.parse import urlparse
>      distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
> -    wd = d.getVar("WORKDIR", True)
> +    aptkeys = []
> +
>      if distro_apt_keys:
> -        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
> -        for key in distro_apt_keys.split():
> -            url = urlparse(key)
> -            filename = ''.join([wd, url.path])
> -            d.appendVar("SRC_URI", " %s" % key)
> -            d.appendVar("APTKEYFILES", " %s" % filename)
> +        aptkeys += distro_apt_keys.split()
> +
>      if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
>          own_pub_key = d.getVar("BASE_REPO_KEY", False)
>          if own_pub_key:
> -            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> ${APTKEYRING}")
> -            for key in own_pub_key.split():
> -                url = urlparse(key)
> -                filename = ''.join([wd, url.path])
> -                d.appendVar("SRC_URI", " %s" % key)
> -                d.appendVar("APTKEYFILES", " %s" % filename)
> +            aptkeys += own_pub_keys.split()
> +
> +    for key in aptkeys:
> +        d.appendVar("SRC_URI", " %s" % key)
> +        fetcher = bb.fetch2.Fetch([key], d)
> +        filename = fetcher.localpath(key)
> +        d.appendVar("APTKEYFILES", " %s" % filename)
>  }
>  
>  def aggregate_files(d, file_list, file_out):
> @@ -174,13 +169,17 @@ def get_distro_components_argument(d, is_host):
>      else:
>          return ""
>  
> +APTKEYTMPDIR := "${TMPDIR}/aptkeys"
> +
> +do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
>  do_generate_keyring[dirs] = "${DL_DIR}"
>  do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
>  do_generate_keyring() {
>      if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
> +        chmod 777 "${APTKEYTMPDIR}"
>          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> -           gpg --no-default-keyring --keyring "${APTKEYRING}" \
> -               --no-tty --homedir "${DL_DIR}"  --import "$keyfile"
> +           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
> +           sudo apt-key add "$keyfile"
>          done
>      fi
>  }
> @@ -222,7 +221,6 @@ isar_bootstrap() {
>              if [ ${IS_HOST} ]; then
>                  ${DEBOOTSTRAP} $debootstrap_args \
>                                 ${@get_distro_components_argument(d,
> True)} \
> -                               ${DEBOOTSTRAP_KEYRING} \
>                                 "${@get_distro_suite(d, True)}" \
>                                 "${ROOTFSDIR}" \
>                                 "${@get_distro_source(d, True)}"
> @@ -231,7 +229,6 @@ isar_bootstrap() {
>                   "${DEBOOTSTRAP}" $debootstrap_args \
>                                    --arch="${DISTRO_ARCH}" \
>                                    ${@get_distro_components_argument(d,
> False)} \
> -                                  ${DEBOOTSTRAP_KEYRING} \
>                                    "${@get_distro_suite(d, False)}" \
>                                    "${ROOTFSDIR}" \
>                                    "${@get_distro_source(d, False)}"


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] Separate apt-key entries from default keyring
  2019-02-27 15:18 ` [PATCH v2 3/3] Separate apt-key entries from default keyring Andreas J. Reichel
@ 2019-02-27 16:14   ` Henning Schild
  2019-03-01  9:08     ` Andreas Reichel
  0 siblings, 1 reply; 16+ messages in thread
From: Henning Schild @ 2019-02-27 16:14 UTC (permalink / raw)
  To: [ext] Andreas J. Reichel; +Cc: isar-users

Is this fixing an issue introduced in p1? I guess you should just
squash it ;).

Henning


Am Wed, 27 Feb 2019 16:18:56 +0100
schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:

> From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> 
> Per default, apt-key add adds keys to /etc/apt/trusted.gpg.
> However, when building without a container, we don't want to
> contaminate the host. Therefore, we specify a keyring file
> in /etc/apt/trusted.gpg.d directory named `isar.gpg`. This file can
> be deleted after the build.
> 
> This is necessary because we don't want to specify single keyrings
> to debootstrap since we might need a mixture of several keyrings
> as per default and would have to find all needed keyrings on the
> system, export their keys and reimport into the build keyring, which
> is more complicated and unneeded this way.
> 
> Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> ---
>  meta/classes/isar-bootstrap-helper.bbclass              | 1 +
>  meta/classes/isar-image.bbclass                         | 1 +
>  meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb | 2 +-
>  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc     | 2 +-
>  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> b/meta/classes/isar-bootstrap-helper.bbclass index 26abf62..769cbef
> 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> +++ b/meta/classes/isar-bootstrap-helper.bbclass
> @@ -22,6 +22,7 @@ HOST_DISTRO ?= "debian-stretch"
>  HOST_ARCH ?= "${@get_deb_host_arch()}"
>  
>  HOST_DISTRO_APT_SOURCES += "conf/distro/${HOST_DISTRO}.list"
> +ISARKEYRING = "/etc/apt/trusted.gpg.d/isar.gpg"
>  
>  def reverse_bb_array(d, varname):
>      array = d.getVar(varname, True)
> diff --git a/meta/classes/isar-image.bbclass
> b/meta/classes/isar-image.bbclass index cdd1651..4a89bd7 100644
> --- a/meta/classes/isar-image.bbclass
> +++ b/meta/classes/isar-image.bbclass
> @@ -82,6 +82,7 @@ isar_image_cleanup() {
>          fi
>          rm -f "${IMAGE_ROOTFS}/etc/apt/sources-list"
>      '
> +    sudo rm -f "${ISARKEYRING}"
>  }
>  
>  do_rootfs() {
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb index
> a793585..b70d2a8 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb @@ -11,8
> +11,8 @@ WORKDIR =
> "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${HOST_DISTRO}-${HOST_A
> DEPLOY_ISAR_BOOTSTRAP =
> "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}"
> ISAR_BOOTSTRAP_LOCK =
> "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}.lock" -require
> isar-bootstrap.inc inherit isar-bootstrap-helper +require
> isar-bootstrap.inc do_generate_keyring[stamp-extra-info] =
> "${DISTRO}-${DISTRO_ARCH}" 
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> 2ef3b1e..4613732 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -179,7
> +179,7 @@ do_generate_keyring() { chmod 777 "${APTKEYTMPDIR}"
>          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
>             cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
> -           sudo apt-key add "$keyfile"
> +           sudo apt-key --keyring "${ISARKEYRING}" add "$keyfile"
>          done
>      fi
>  }


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Fixes usage of additional apt keys
  2019-02-27 15:18 [PATCH v2 0/3] Fixes usage of additional apt keys Andreas J. Reichel
                   ` (2 preceding siblings ...)
  2019-02-27 15:18 ` [PATCH v2 3/3] Separate apt-key entries from default keyring Andreas J. Reichel
@ 2019-02-27 16:15 ` Henning Schild
  2019-03-01  9:17   ` Andreas Reichel
  3 siblings, 1 reply; 16+ messages in thread
From: Henning Schild @ 2019-02-27 16:15 UTC (permalink / raw)
  To: [ext] Andreas J. Reichel; +Cc: isar-users

Could you send a list of changes next time, or reply to the reviews.

Otherwise there is no way to tell what you did in response to the
review, which comments you skipped etc.

Henning

Am Wed, 27 Feb 2019 16:18:53 +0100
schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:

> From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> 
> If the user does not want to replace the bootstrap source together
> with the key, additional keys did not work.
> 
> This is solved by not replacing the keyring but to add keys to
> /ect/apt/trusted.gpg.d/isar.gpg, where debootstrap and any apt call
> can find it.
> 
> Furthermore, the code to add keys is simplified a lot by not manually
> parsing URIs and guessing about download locations as well as
> not manually handling gpg and giving apt config overrides.
> 
> It is much simpler by using `apt-key` and default apt keyring paths.
> 
> Furthermore, apt-get must not use a given single source list which was
> used from debootstrapping. Otherwise, additional packages are always
> unauthenticated, which is a quite misleading error. Instead, apt-get
> should use all source lists available in the built root.
> 
> Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> 
> Andreas Reichel (3):
>   Fix and simplify apt keyring generation
>   Use all source lists in target root apt
>   Separate apt-key entries from default keyring
> 
>  meta/classes/isar-bootstrap-helper.bbclass    | 17 ++++++--
>  meta/classes/isar-image.bbclass               |  1 +
>  .../isar-bootstrap/isar-bootstrap-host.bb     |  2 +-
>  .../isar-bootstrap/isar-bootstrap.inc         | 39
> +++++++++---------- 4 files changed, 33 insertions(+), 26 deletions(-)
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] Separate apt-key entries from default keyring
  2019-02-27 16:14   ` Henning Schild
@ 2019-03-01  9:08     ` Andreas Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Andreas Reichel @ 2019-03-01  9:08 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On Wed, Feb 27, 2019 at 05:14:43PM +0100, Henning Schild wrote:
> Is this fixing an issue introduced in p1? I guess you should just
> squash it ;).
> 
> Henning

This is a matter of taste... as discussed for those building in a
container, it is no issue, for others, who don't it MIGHT be,
if they want to keep their keyring exactly as it was.
> 
> 
> Am Wed, 27 Feb 2019 16:18:56 +0100
> schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:
> 
> > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > 
> > Per default, apt-key add adds keys to /etc/apt/trusted.gpg.
> > However, when building without a container, we don't want to
> > contaminate the host. Therefore, we specify a keyring file
> > in /etc/apt/trusted.gpg.d directory named `isar.gpg`. This file can
> > be deleted after the build.
> > 
> > This is necessary because we don't want to specify single keyrings
> > to debootstrap since we might need a mixture of several keyrings
> > as per default and would have to find all needed keyrings on the
> > system, export their keys and reimport into the build keyring, which
> > is more complicated and unneeded this way.
> > 
> > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > ---
> >  meta/classes/isar-bootstrap-helper.bbclass              | 1 +
> >  meta/classes/isar-image.bbclass                         | 1 +
> >  meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb | 2 +-
> >  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc     | 2 +-
> >  4 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> > b/meta/classes/isar-bootstrap-helper.bbclass index 26abf62..769cbef
> > 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> > +++ b/meta/classes/isar-bootstrap-helper.bbclass
> > @@ -22,6 +22,7 @@ HOST_DISTRO ?= "debian-stretch"
> >  HOST_ARCH ?= "${@get_deb_host_arch()}"
> >  
> >  HOST_DISTRO_APT_SOURCES += "conf/distro/${HOST_DISTRO}.list"
> > +ISARKEYRING = "/etc/apt/trusted.gpg.d/isar.gpg"
> >  
> >  def reverse_bb_array(d, varname):
> >      array = d.getVar(varname, True)
> > diff --git a/meta/classes/isar-image.bbclass
> > b/meta/classes/isar-image.bbclass index cdd1651..4a89bd7 100644
> > --- a/meta/classes/isar-image.bbclass
> > +++ b/meta/classes/isar-image.bbclass
> > @@ -82,6 +82,7 @@ isar_image_cleanup() {
> >          fi
> >          rm -f "${IMAGE_ROOTFS}/etc/apt/sources-list"
> >      '
> > +    sudo rm -f "${ISARKEYRING}"
> >  }
> >  
> >  do_rootfs() {
> > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb index
> > a793585..b70d2a8 100644 ---
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb +++
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb @@ -11,8
> > +11,8 @@ WORKDIR =
> > "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${HOST_DISTRO}-${HOST_A
> > DEPLOY_ISAR_BOOTSTRAP =
> > "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}"
> > ISAR_BOOTSTRAP_LOCK =
> > "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}.lock" -require
> > isar-bootstrap.inc inherit isar-bootstrap-helper +require
> > isar-bootstrap.inc do_generate_keyring[stamp-extra-info] =
> > "${DISTRO}-${DISTRO_ARCH}" 
> > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > 2ef3b1e..4613732 100644 ---
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -179,7
> > +179,7 @@ do_generate_keyring() { chmod 777 "${APTKEYTMPDIR}"
> >          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> >             cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
> > -           sudo apt-key add "$keyfile"
> > +           sudo apt-key --keyring "${ISARKEYRING}" add "$keyfile"
> >          done
> >      fi
> >  }
> 

-- 
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas.Reichel@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-02-27 16:12   ` Henning Schild
@ 2019-03-01  9:09     ` Andreas Reichel
  2019-03-01 10:18       ` Henning Schild
  2019-03-01  9:16     ` Andreas Reichel
  2019-03-01 10:22     ` Henning Schild
  2 siblings, 1 reply; 16+ messages in thread
From: Andreas Reichel @ 2019-03-01  9:09 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On Wed, Feb 27, 2019 at 05:12:42PM +0100, Henning Schild wrote:
> Am Wed, 27 Feb 2019 16:18:54 +0100
> schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:
> 
> > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > 
> > Different fetcher stored keys in different sub dirs, and we can never
> > be sure about where downloaded files go.
> > To avoid this without making any assumptions, we ask the fetcher where
> > the file will be after it is downloaded. This way we also don't need
> > to parse the URL manually.
> > 
> > The code is simplified by removing duplicate code and using apt-key
> > instead of manually calling gpg.
> > 
> > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > ---
> >  meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
> >  .../isar-bootstrap/isar-bootstrap.inc         | 39
> > +++++++++---------- 2 files changed, 29 insertions(+), 21 deletions(-)
> > 
> > diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> > b/meta/classes/isar-bootstrap-helper.bbclass index d780b85..b8c41f9
> > 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> > +++ b/meta/classes/isar-bootstrap-helper.bbclass
> > @@ -119,6 +119,16 @@ setup_root_file_system() {
> >      export LANG=C
> >      export LANGUAGE=C
> >      export LC_ALL=C
> > +
> > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > +        for keyfile in ${TMPDIR}/aptkeys/*
> > +        do
> > +            kfn="tmp/$(basename $keyfile)"
> > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add "/$kfn"
> 
> Would probably be a good idea to work in /tmp/ as well and in a subdir.
> Try naming your key "root" or "usr" ...
> 
> > +            rm "$ROOTFSDIR/$kfn"
> > +        done
> > +    fi
> >      sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
> >          -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
> >          -o Dir::Etc::sourceparts="-" \
> > @@ -128,6 +138,7 @@ setup_root_file_system() {
> >          sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg --add-architecture
> > ${DISTRO_ARCH} sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update
> >      fi
> > +    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
> >      sudo -E chroot "$ROOTFSDIR" \
> >          /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
> >              ${IMAGE_TRANSIENT_PACKAGES}
> > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > 234d339..2ef3b1e 100644 ---
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -23,35
> > +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources" APTSRCS_INIT =
> > "${WORKDIR}/apt-sources-init" BASEAPTSRCS =
> > "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> > -APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
> > -DEBOOTSTRAP_KEYRING = ""
> >  DEPLOY_ISAR_BOOTSTRAP ?= ""
> > -DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> > +DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2 apt-transport-https
> > ca-certificates"
> 
> That looks more like leftovers from an experiment. Are you sure that is
> needed?

This is needed because debootstrap can switch to https by itself if it
is unsure about the sources (apt-transport-https, ca-certificates),
 gnupg2 we need for apt-key.
> 
> 
> In general i think that patch should be splittet into two or
> even more patches. Because it solves at least two problems.
> 
> Henning
> 
> >  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 "" }"
> > inherit base-apt-helper 
> >  python () {
> > -    from urllib.parse import urlparse
> >      distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
> > -    wd = d.getVar("WORKDIR", True)
> > +    aptkeys = []
> > +
> >      if distro_apt_keys:
> > -        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
> > -        for key in distro_apt_keys.split():
> > -            url = urlparse(key)
> > -            filename = ''.join([wd, url.path])
> > -            d.appendVar("SRC_URI", " %s" % key)
> > -            d.appendVar("APTKEYFILES", " %s" % filename)
> > +        aptkeys += distro_apt_keys.split()
> > +
> >      if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
> >          own_pub_key = d.getVar("BASE_REPO_KEY", False)
> >          if own_pub_key:
> > -            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > ${APTKEYRING}")
> > -            for key in own_pub_key.split():
> > -                url = urlparse(key)
> > -                filename = ''.join([wd, url.path])
> > -                d.appendVar("SRC_URI", " %s" % key)
> > -                d.appendVar("APTKEYFILES", " %s" % filename)
> > +            aptkeys += own_pub_keys.split()
> > +
> > +    for key in aptkeys:
> > +        d.appendVar("SRC_URI", " %s" % key)
> > +        fetcher = bb.fetch2.Fetch([key], d)
> > +        filename = fetcher.localpath(key)
> > +        d.appendVar("APTKEYFILES", " %s" % filename)
> >  }
> >  
> >  def aggregate_files(d, file_list, file_out):
> > @@ -174,13 +169,17 @@ def get_distro_components_argument(d, is_host):
> >      else:
> >          return ""
> >  
> > +APTKEYTMPDIR := "${TMPDIR}/aptkeys"
> > +
> > +do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
> >  do_generate_keyring[dirs] = "${DL_DIR}"
> >  do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> >  do_generate_keyring() {
> >      if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
> > +        chmod 777 "${APTKEYTMPDIR}"
> >          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> > -           gpg --no-default-keyring --keyring "${APTKEYRING}" \
> > -               --no-tty --homedir "${DL_DIR}"  --import "$keyfile"
> > +           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
> > +           sudo apt-key add "$keyfile"
> >          done
> >      fi
> >  }
> > @@ -222,7 +221,6 @@ isar_bootstrap() {
> >              if [ ${IS_HOST} ]; then
> >                  ${DEBOOTSTRAP} $debootstrap_args \
> >                                 ${@get_distro_components_argument(d,
> > True)} \
> > -                               ${DEBOOTSTRAP_KEYRING} \
> >                                 "${@get_distro_suite(d, True)}" \
> >                                 "${ROOTFSDIR}" \
> >                                 "${@get_distro_source(d, True)}"
> > @@ -231,7 +229,6 @@ isar_bootstrap() {
> >                   "${DEBOOTSTRAP}" $debootstrap_args \
> >                                    --arch="${DISTRO_ARCH}" \
> >                                    ${@get_distro_components_argument(d,
> > False)} \
> > -                                  ${DEBOOTSTRAP_KEYRING} \
> >                                    "${@get_distro_suite(d, False)}" \
> >                                    "${ROOTFSDIR}" \
> >                                    "${@get_distro_source(d, False)}"
> 

-- 
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas.Reichel@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-02-27 16:12   ` Henning Schild
  2019-03-01  9:09     ` Andreas Reichel
@ 2019-03-01  9:16     ` Andreas Reichel
  2019-03-01 10:11       ` Henning Schild
  2019-03-01 10:22     ` Henning Schild
  2 siblings, 1 reply; 16+ messages in thread
From: Andreas Reichel @ 2019-03-01  9:16 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On Wed, Feb 27, 2019 at 05:12:42PM +0100, Henning Schild wrote:
> Am Wed, 27 Feb 2019 16:18:54 +0100
> schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:
> 
> > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > 
> > +
> > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > +        for keyfile in ${TMPDIR}/aptkeys/*
> > +        do
> > +            kfn="tmp/$(basename $keyfile)"
> > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add "/$kfn"
> 
> Would probably be a good idea to work in /tmp/ as well and in a subdir.
> Try naming your key "root" or "usr" ...
> 
I AM working in /tmp/, ... a subdir is not needed since we directly work
with the file names.
This is not " a key ", these are arbitrarily many keys and the name of
the key files don't change between the host, the build and target
chroots.

Andreas

-- 
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas.Reichel@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Fixes usage of additional apt keys
  2019-02-27 16:15 ` [PATCH v2 0/3] Fixes usage of additional apt keys Henning Schild
@ 2019-03-01  9:17   ` Andreas Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Andreas Reichel @ 2019-03-01  9:17 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On Wed, Feb 27, 2019 at 05:15:56PM +0100, Henning Schild wrote:
> Could you send a list of changes next time, or reply to the reviews.
> 
> Otherwise there is no way to tell what you did in response to the
There is a simple way... read the cover letters. But sure, I will
do better next time :)

Andreas

> review, which comments you skipped etc.
> 
> Henning
> 
> Am Wed, 27 Feb 2019 16:18:53 +0100
> schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:
> 
> > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > 
> > If the user does not want to replace the bootstrap source together
> > with the key, additional keys did not work.
> > 
> > This is solved by not replacing the keyring but to add keys to
> > /ect/apt/trusted.gpg.d/isar.gpg, where debootstrap and any apt call
> > can find it.
> > 
> > Furthermore, the code to add keys is simplified a lot by not manually
> > parsing URIs and guessing about download locations as well as
> > not manually handling gpg and giving apt config overrides.
> > 
> > It is much simpler by using `apt-key` and default apt keyring paths.
> > 
> > Furthermore, apt-get must not use a given single source list which was
> > used from debootstrapping. Otherwise, additional packages are always
> > unauthenticated, which is a quite misleading error. Instead, apt-get
> > should use all source lists available in the built root.
> > 
> > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > 
> > Andreas Reichel (3):
> >   Fix and simplify apt keyring generation
> >   Use all source lists in target root apt
> >   Separate apt-key entries from default keyring
> > 
> >  meta/classes/isar-bootstrap-helper.bbclass    | 17 ++++++--
> >  meta/classes/isar-image.bbclass               |  1 +
> >  .../isar-bootstrap/isar-bootstrap-host.bb     |  2 +-
> >  .../isar-bootstrap/isar-bootstrap.inc         | 39
> > +++++++++---------- 4 files changed, 33 insertions(+), 26 deletions(-)
> > 
> 

-- 
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas.Reichel@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-03-01  9:16     ` Andreas Reichel
@ 2019-03-01 10:11       ` Henning Schild
  0 siblings, 0 replies; 16+ messages in thread
From: Henning Schild @ 2019-03-01 10:11 UTC (permalink / raw)
  To: Andreas Reichel; +Cc: isar-users

Am Fri, 1 Mar 2019 10:16:50 +0100
schrieb Andreas Reichel <andreas.reichel.ext@siemens.com>:

> On Wed, Feb 27, 2019 at 05:12:42PM +0100, Henning Schild wrote:
> > Am Wed, 27 Feb 2019 16:18:54 +0100
> > schrieb "[ext] Andreas J. Reichel"
> > <andreas.reichel.ext@siemens.com>: 
> > > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > 
> > > +
> > > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > > +        for keyfile in ${TMPDIR}/aptkeys/*
> > > +        do
> > > +            kfn="tmp/$(basename $keyfile)"
> > > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add
> > > "/$kfn"  
> > 
> > Would probably be a good idea to work in /tmp/ as well and in a
> > subdir. Try naming your key "root" or "usr" ...
> >   
> I AM working in /tmp/, ... a subdir is not needed since we directly
> work with the file names.
> This is not " a key ", these are arbitrarily many keys and the name of
> the key files don't change between the host, the build and target
> chroots.

I see the "tmp/" is in $kfn, might be more readable to have in the the
cp and apt-key.

Henning

> Andreas
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-03-01  9:09     ` Andreas Reichel
@ 2019-03-01 10:18       ` Henning Schild
  2019-03-06 16:29         ` Andreas Reichel
  0 siblings, 1 reply; 16+ messages in thread
From: Henning Schild @ 2019-03-01 10:18 UTC (permalink / raw)
  To: Andreas Reichel; +Cc: isar-users

Am Fri, 1 Mar 2019 10:09:49 +0100
schrieb Andreas Reichel <andreas.reichel.ext@siemens.com>:

> On Wed, Feb 27, 2019 at 05:12:42PM +0100, Henning Schild wrote:
> > Am Wed, 27 Feb 2019 16:18:54 +0100
> > schrieb "[ext] Andreas J. Reichel"
> > <andreas.reichel.ext@siemens.com>: 
> > > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > 
> > > Different fetcher stored keys in different sub dirs, and we can
> > > never be sure about where downloaded files go.
> > > To avoid this without making any assumptions, we ask the fetcher
> > > where the file will be after it is downloaded. This way we also
> > > don't need to parse the URL manually.
> > > 
> > > The code is simplified by removing duplicate code and using
> > > apt-key instead of manually calling gpg.
> > > 
> > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > ---
> > >  meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
> > >  .../isar-bootstrap/isar-bootstrap.inc         | 39
> > > +++++++++---------- 2 files changed, 29 insertions(+), 21
> > > deletions(-)
> > > 
> > > diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> > > b/meta/classes/isar-bootstrap-helper.bbclass index
> > > d780b85..b8c41f9 100644 ---
> > > a/meta/classes/isar-bootstrap-helper.bbclass +++
> > > b/meta/classes/isar-bootstrap-helper.bbclass @@ -119,6 +119,16 @@
> > > setup_root_file_system() { export LANG=C
> > >      export LANGUAGE=C
> > >      export LC_ALL=C
> > > +
> > > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > > +        for keyfile in ${TMPDIR}/aptkeys/*
> > > +        do
> > > +            kfn="tmp/$(basename $keyfile)"
> > > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add
> > > "/$kfn"  
> > 
> > Would probably be a good idea to work in /tmp/ as well and in a
> > subdir. Try naming your key "root" or "usr" ...
> >   
> > > +            rm "$ROOTFSDIR/$kfn"
> > > +        done
> > > +    fi
> > >      sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
> > >          -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
> > >          -o Dir::Etc::sourceparts="-" \
> > > @@ -128,6 +138,7 @@ setup_root_file_system() {
> > >          sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg
> > > --add-architecture ${DISTRO_ARCH} sudo -E chroot
> > > "$ROOTFSDIR" /usr/bin/apt-get update fi
> > > +    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
> > >      sudo -E chroot "$ROOTFSDIR" \
> > >          /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
> > >              ${IMAGE_TRANSIENT_PACKAGES}
> > > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > > 234d339..2ef3b1e 100644 ---
> > > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -23,35
> > > +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources" APTSRCS_INIT =
> > > "${WORKDIR}/apt-sources-init" BASEAPTSRCS =
> > > "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> > > -APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
> > > -DEBOOTSTRAP_KEYRING = ""
> > >  DEPLOY_ISAR_BOOTSTRAP ?= ""
> > > -DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> > > +DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2
> > > apt-transport-https ca-certificates"  
> > 
> > That looks more like leftovers from an experiment. Are you sure
> > that is needed?  
> 
> This is needed because debootstrap can switch to https by itself if it
> is unsure about the sources (apt-transport-https, ca-certificates),
>  gnupg2 we need for apt-key.

Adding the packages should probably depend on whether DISTRO_APT_KEYS
is non-empty. And adding the ones that the https-detector adds should
be tested, not that dual mention makes debootstrap crap out.

I guess the OVERRIDES method from 9196b820636a9 should be used for gpg
support as well.

Henning

> > 
> > 
> > In general i think that patch should be splittet into two or
> > even more patches. Because it solves at least two problems.
> > 
> > Henning
> >   
> > >  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
> > > "" }" inherit base-apt-helper 
> > >  python () {
> > > -    from urllib.parse import urlparse
> > >      distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
> > > -    wd = d.getVar("WORKDIR", True)
> > > +    aptkeys = []
> > > +
> > >      if distro_apt_keys:
> > > -        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > > ${APTKEYRING}")
> > > -        for key in distro_apt_keys.split():
> > > -            url = urlparse(key)
> > > -            filename = ''.join([wd, url.path])
> > > -            d.appendVar("SRC_URI", " %s" % key)
> > > -            d.appendVar("APTKEYFILES", " %s" % filename)
> > > +        aptkeys += distro_apt_keys.split()
> > > +
> > >      if
> > > bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
> > > own_pub_key = d.getVar("BASE_REPO_KEY", False) if own_pub_key:
> > > -            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > > ${APTKEYRING}")
> > > -            for key in own_pub_key.split():
> > > -                url = urlparse(key)
> > > -                filename = ''.join([wd, url.path])
> > > -                d.appendVar("SRC_URI", " %s" % key)
> > > -                d.appendVar("APTKEYFILES", " %s" % filename)
> > > +            aptkeys += own_pub_keys.split()
> > > +
> > > +    for key in aptkeys:
> > > +        d.appendVar("SRC_URI", " %s" % key)
> > > +        fetcher = bb.fetch2.Fetch([key], d)
> > > +        filename = fetcher.localpath(key)
> > > +        d.appendVar("APTKEYFILES", " %s" % filename)
> > >  }
> > >  
> > >  def aggregate_files(d, file_list, file_out):
> > > @@ -174,13 +169,17 @@ def get_distro_components_argument(d,
> > > is_host): else:
> > >          return ""
> > >  
> > > +APTKEYTMPDIR := "${TMPDIR}/aptkeys"
> > > +
> > > +do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
> > >  do_generate_keyring[dirs] = "${DL_DIR}"
> > >  do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> > >  do_generate_keyring() {
> > >      if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
> > > +        chmod 777 "${APTKEYTMPDIR}"
> > >          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> > > -           gpg --no-default-keyring --keyring "${APTKEYRING}" \
> > > -               --no-tty --homedir "${DL_DIR}"  --import
> > > "$keyfile"
> > > +           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename
> > > "$keyfile")"
> > > +           sudo apt-key add "$keyfile"
> > >          done
> > >      fi
> > >  }
> > > @@ -222,7 +221,6 @@ isar_bootstrap() {
> > >              if [ ${IS_HOST} ]; then
> > >                  ${DEBOOTSTRAP} $debootstrap_args \
> > >                                 ${@get_distro_components_argument(d,
> > > True)} \
> > > -                               ${DEBOOTSTRAP_KEYRING} \
> > >                                 "${@get_distro_suite(d, True)}" \
> > >                                 "${ROOTFSDIR}" \
> > >                                 "${@get_distro_source(d, True)}"
> > > @@ -231,7 +229,6 @@ isar_bootstrap() {
> > >                   "${DEBOOTSTRAP}" $debootstrap_args \
> > >                                    --arch="${DISTRO_ARCH}" \
> > >                                    ${@get_distro_components_argument(d,
> > > False)} \
> > > -                                  ${DEBOOTSTRAP_KEYRING} \
> > >                                    "${@get_distro_suite(d,
> > > False)}" \ "${ROOTFSDIR}" \
> > >                                    "${@get_distro_source(d,
> > > False)}"  
> >   
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-02-27 16:12   ` Henning Schild
  2019-03-01  9:09     ` Andreas Reichel
  2019-03-01  9:16     ` Andreas Reichel
@ 2019-03-01 10:22     ` Henning Schild
  2019-03-06 16:29       ` Andreas Reichel
  2 siblings, 1 reply; 16+ messages in thread
From: Henning Schild @ 2019-03-01 10:22 UTC (permalink / raw)
  To: [ext] Andreas J. Reichel; +Cc: isar-users

Am Wed, 27 Feb 2019 17:12:42 +0100
schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:

> Am Wed, 27 Feb 2019 16:18:54 +0100
> schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:
> 
> > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > 
> > Different fetcher stored keys in different sub dirs, and we can
> > never be sure about where downloaded files go.
> > To avoid this without making any assumptions, we ask the fetcher
> > where the file will be after it is downloaded. This way we also
> > don't need to parse the URL manually.
> > 
> > The code is simplified by removing duplicate code and using apt-key
> > instead of manually calling gpg.
> > 
> > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > ---
> >  meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
> >  .../isar-bootstrap/isar-bootstrap.inc         | 39
> > +++++++++---------- 2 files changed, 29 insertions(+), 21
> > deletions(-)
> > 
> > diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> > b/meta/classes/isar-bootstrap-helper.bbclass index d780b85..b8c41f9
> > 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> > +++ b/meta/classes/isar-bootstrap-helper.bbclass
> > @@ -119,6 +119,16 @@ setup_root_file_system() {
> >      export LANG=C
> >      export LANGUAGE=C
> >      export LC_ALL=C
> > +
> > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > +        for keyfile in ${TMPDIR}/aptkeys/*
> > +        do
> > +            kfn="tmp/$(basename $keyfile)"
> > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add
> > "/$kfn"  
> 
> Would probably be a good idea to work in /tmp/ as well and in a
> subdir. Try naming your key "root" or "usr" ...
> 
> > +            rm "$ROOTFSDIR/$kfn"
> > +        done
> > +    fi
> >      sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
> >          -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
> >          -o Dir::Etc::sourceparts="-" \
> > @@ -128,6 +138,7 @@ setup_root_file_system() {
> >          sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg
> > --add-architecture ${DISTRO_ARCH} sudo -E chroot
> > "$ROOTFSDIR" /usr/bin/apt-get update fi
> > +    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
> >      sudo -E chroot "$ROOTFSDIR" \
> >          /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
> >              ${IMAGE_TRANSIENT_PACKAGES}
> > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > 234d339..2ef3b1e 100644 ---
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -23,35
> > +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources" APTSRCS_INIT =
> > "${WORKDIR}/apt-sources-init" BASEAPTSRCS =
> > "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> > -APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
> > -DEBOOTSTRAP_KEYRING = ""
> >  DEPLOY_ISAR_BOOTSTRAP ?= ""
> > -DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> > +DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2
> > apt-transport-https ca-certificates"  
> 
> That looks more like leftovers from an experiment. Are you sure that
> is needed?
> 
> 
> In general i think that patch should be splittet into two or
> even more patches. Because it solves at least two problems.

One possible split would be:

 - gpg -> apt-key
 - filename.join -> fetcher.localpath
 - adding BASE_PACKAGES depending on GPG needed
 - ...

Henning

> Henning
> 
> >  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
> > "" }" inherit base-apt-helper 
> >  python () {
> > -    from urllib.parse import urlparse
> >      distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
> > -    wd = d.getVar("WORKDIR", True)
> > +    aptkeys = []
> > +
> >      if distro_apt_keys:
> > -        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
> > -        for key in distro_apt_keys.split():
> > -            url = urlparse(key)
> > -            filename = ''.join([wd, url.path])
> > -            d.appendVar("SRC_URI", " %s" % key)
> > -            d.appendVar("APTKEYFILES", " %s" % filename)
> > +        aptkeys += distro_apt_keys.split()
> > +
> >      if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
> >          own_pub_key = d.getVar("BASE_REPO_KEY", False)
> >          if own_pub_key:
> > -            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > ${APTKEYRING}")
> > -            for key in own_pub_key.split():
> > -                url = urlparse(key)
> > -                filename = ''.join([wd, url.path])
> > -                d.appendVar("SRC_URI", " %s" % key)
> > -                d.appendVar("APTKEYFILES", " %s" % filename)
> > +            aptkeys += own_pub_keys.split()
> > +
> > +    for key in aptkeys:
> > +        d.appendVar("SRC_URI", " %s" % key)
> > +        fetcher = bb.fetch2.Fetch([key], d)
> > +        filename = fetcher.localpath(key)
> > +        d.appendVar("APTKEYFILES", " %s" % filename)
> >  }
> >  
> >  def aggregate_files(d, file_list, file_out):
> > @@ -174,13 +169,17 @@ def get_distro_components_argument(d,
> > is_host): else:
> >          return ""
> >  
> > +APTKEYTMPDIR := "${TMPDIR}/aptkeys"
> > +
> > +do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
> >  do_generate_keyring[dirs] = "${DL_DIR}"
> >  do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> >  do_generate_keyring() {
> >      if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
> > +        chmod 777 "${APTKEYTMPDIR}"
> >          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> > -           gpg --no-default-keyring --keyring "${APTKEYRING}" \
> > -               --no-tty --homedir "${DL_DIR}"  --import "$keyfile"
> > +           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
> > +           sudo apt-key add "$keyfile"
> >          done
> >      fi
> >  }
> > @@ -222,7 +221,6 @@ isar_bootstrap() {
> >              if [ ${IS_HOST} ]; then
> >                  ${DEBOOTSTRAP} $debootstrap_args \
> >                                 ${@get_distro_components_argument(d,
> > True)} \
> > -                               ${DEBOOTSTRAP_KEYRING} \
> >                                 "${@get_distro_suite(d, True)}" \
> >                                 "${ROOTFSDIR}" \
> >                                 "${@get_distro_source(d, True)}"
> > @@ -231,7 +229,6 @@ isar_bootstrap() {
> >                   "${DEBOOTSTRAP}" $debootstrap_args \
> >                                    --arch="${DISTRO_ARCH}" \
> >                                    ${@get_distro_components_argument(d,
> > False)} \
> > -                                  ${DEBOOTSTRAP_KEYRING} \
> >                                    "${@get_distro_suite(d, False)}"
> > \ "${ROOTFSDIR}" \
> >                                    "${@get_distro_source(d,
> > False)}"  
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-03-01 10:22     ` Henning Schild
@ 2019-03-06 16:29       ` Andreas Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Andreas Reichel @ 2019-03-06 16:29 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On Fri, Mar 01, 2019 at 11:22:24AM +0100, Henning Schild wrote:
> Am Wed, 27 Feb 2019 17:12:42 +0100
> schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:
> 
> > Am Wed, 27 Feb 2019 16:18:54 +0100
> > schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:
> > 
> > > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > 
> > > Different fetcher stored keys in different sub dirs, and we can
> > > never be sure about where downloaded files go.
> > > To avoid this without making any assumptions, we ask the fetcher
> > > where the file will be after it is downloaded. This way we also
> > > don't need to parse the URL manually.
> > > 
> > > The code is simplified by removing duplicate code and using apt-key
> > > instead of manually calling gpg.
> > > 
> > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > ---
> > >  meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
> > >  .../isar-bootstrap/isar-bootstrap.inc         | 39
> > > +++++++++---------- 2 files changed, 29 insertions(+), 21
> > > deletions(-)
> > > 
> > > diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> > > b/meta/classes/isar-bootstrap-helper.bbclass index d780b85..b8c41f9
> > > 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> > > +++ b/meta/classes/isar-bootstrap-helper.bbclass
> > > @@ -119,6 +119,16 @@ setup_root_file_system() {
> > >      export LANG=C
> > >      export LANGUAGE=C
> > >      export LC_ALL=C
> > > +
> > > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > > +        for keyfile in ${TMPDIR}/aptkeys/*
> > > +        do
> > > +            kfn="tmp/$(basename $keyfile)"
> > > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add
> > > "/$kfn"  
> > 
> > Would probably be a good idea to work in /tmp/ as well and in a
> > subdir. Try naming your key "root" or "usr" ...
> > 
Already worked in tmp :) But factored it out of $kfn in patch v3.

> > > +            rm "$ROOTFSDIR/$kfn"
> > > +        done
> > > +    fi
> > >      sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
> > >          -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
> > >          -o Dir::Etc::sourceparts="-" \
> > > @@ -128,6 +138,7 @@ setup_root_file_system() {
> > >          sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg
> > > --add-architecture ${DISTRO_ARCH} sudo -E chroot
> > > "$ROOTFSDIR" /usr/bin/apt-get update fi
> > > +    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
> > >      sudo -E chroot "$ROOTFSDIR" \
> > >          /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
> > >              ${IMAGE_TRANSIENT_PACKAGES}
> > > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > > 234d339..2ef3b1e 100644 ---
> > > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -23,35
> > > +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources" APTSRCS_INIT =
> > > "${WORKDIR}/apt-sources-init" BASEAPTSRCS =
> > > "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> > > -APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
> > > -DEBOOTSTRAP_KEYRING = ""
> > >  DEPLOY_ISAR_BOOTSTRAP ?= ""
> > > -DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> > > +DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2
> > > apt-transport-https ca-certificates"  
> > 
> > That looks more like leftovers from an experiment. Are you sure that
> > is needed?
> > 
Fixed in v3
> > 
> > In general i think that patch should be splittet into two or
> > even more patches. Because it solves at least two problems.
> 
> One possible split would be:
> 
>  - gpg -> apt-key
>  - filename.join -> fetcher.localpath
>  - adding BASE_PACKAGES depending on GPG needed
>  - ...
> 
Done in v3.

> Henning
> 
> > Henning
> > 
> > >  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
> > > "" }" inherit base-apt-helper 
> > >  python () {
> > > -    from urllib.parse import urlparse
> > >      distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
> > > -    wd = d.getVar("WORKDIR", True)
> > > +    aptkeys = []
> > > +
> > >      if distro_apt_keys:
> > > -        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
> > > -        for key in distro_apt_keys.split():
> > > -            url = urlparse(key)
> > > -            filename = ''.join([wd, url.path])
> > > -            d.appendVar("SRC_URI", " %s" % key)
> > > -            d.appendVar("APTKEYFILES", " %s" % filename)
> > > +        aptkeys += distro_apt_keys.split()
> > > +
> > >      if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
> > >          own_pub_key = d.getVar("BASE_REPO_KEY", False)
> > >          if own_pub_key:
> > > -            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > > ${APTKEYRING}")
> > > -            for key in own_pub_key.split():
> > > -                url = urlparse(key)
> > > -                filename = ''.join([wd, url.path])
> > > -                d.appendVar("SRC_URI", " %s" % key)
> > > -                d.appendVar("APTKEYFILES", " %s" % filename)
> > > +            aptkeys += own_pub_keys.split()
> > > +
> > > +    for key in aptkeys:
> > > +        d.appendVar("SRC_URI", " %s" % key)
> > > +        fetcher = bb.fetch2.Fetch([key], d)
> > > +        filename = fetcher.localpath(key)
> > > +        d.appendVar("APTKEYFILES", " %s" % filename)
> > >  }
> > >  
> > >  def aggregate_files(d, file_list, file_out):
> > > @@ -174,13 +169,17 @@ def get_distro_components_argument(d,
> > > is_host): else:
> > >          return ""
> > >  
> > > +APTKEYTMPDIR := "${TMPDIR}/aptkeys"
> > > +
> > > +do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
> > >  do_generate_keyring[dirs] = "${DL_DIR}"
> > >  do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> > >  do_generate_keyring() {
> > >      if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
> > > +        chmod 777 "${APTKEYTMPDIR}"
> > >          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> > > -           gpg --no-default-keyring --keyring "${APTKEYRING}" \
> > > -               --no-tty --homedir "${DL_DIR}"  --import "$keyfile"
> > > +           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename "$keyfile")"
> > > +           sudo apt-key add "$keyfile"
> > >          done
> > >      fi
> > >  }
> > > @@ -222,7 +221,6 @@ isar_bootstrap() {
> > >              if [ ${IS_HOST} ]; then
> > >                  ${DEBOOTSTRAP} $debootstrap_args \
> > >                                 ${@get_distro_components_argument(d,
> > > True)} \
> > > -                               ${DEBOOTSTRAP_KEYRING} \
> > >                                 "${@get_distro_suite(d, True)}" \
> > >                                 "${ROOTFSDIR}" \
> > >                                 "${@get_distro_source(d, True)}"
> > > @@ -231,7 +229,6 @@ isar_bootstrap() {
> > >                   "${DEBOOTSTRAP}" $debootstrap_args \
> > >                                    --arch="${DISTRO_ARCH}" \
> > >                                    ${@get_distro_components_argument(d,
> > > False)} \
> > > -                                  ${DEBOOTSTRAP_KEYRING} \
> > >                                    "${@get_distro_suite(d, False)}"
> > > \ "${ROOTFSDIR}" \
> > >                                    "${@get_distro_source(d,
> > > False)}"  
> > 
> 

-- 
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas.Reichel@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Fix and simplify apt keyring generation
  2019-03-01 10:18       ` Henning Schild
@ 2019-03-06 16:29         ` Andreas Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Andreas Reichel @ 2019-03-06 16:29 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On Fri, Mar 01, 2019 at 11:18:48AM +0100, Henning Schild wrote:
> Am Fri, 1 Mar 2019 10:09:49 +0100
> schrieb Andreas Reichel <andreas.reichel.ext@siemens.com>:
> 
> > On Wed, Feb 27, 2019 at 05:12:42PM +0100, Henning Schild wrote:
> > > Am Wed, 27 Feb 2019 16:18:54 +0100
> > > schrieb "[ext] Andreas J. Reichel"
> > > <andreas.reichel.ext@siemens.com>: 
> > > > From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > > 
> > > > Different fetcher stored keys in different sub dirs, and we can
> > > > never be sure about where downloaded files go.
> > > > To avoid this without making any assumptions, we ask the fetcher
> > > > where the file will be after it is downloaded. This way we also
> > > > don't need to parse the URL manually.
> > > > 
> > > > The code is simplified by removing duplicate code and using
> > > > apt-key instead of manually calling gpg.
> > > > 
> > > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> > > > ---
> > > >  meta/classes/isar-bootstrap-helper.bbclass    | 11 ++++++
> > > >  .../isar-bootstrap/isar-bootstrap.inc         | 39
> > > > +++++++++---------- 2 files changed, 29 insertions(+), 21
> > > > deletions(-)
> > > > 
> > > > diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> > > > b/meta/classes/isar-bootstrap-helper.bbclass index
> > > > d780b85..b8c41f9 100644 ---
> > > > a/meta/classes/isar-bootstrap-helper.bbclass +++
> > > > b/meta/classes/isar-bootstrap-helper.bbclass @@ -119,6 +119,16 @@
> > > > setup_root_file_system() { export LANG=C
> > > >      export LANGUAGE=C
> > > >      export LC_ALL=C
> > > > +
> > > > +    if [ -d ${TMPDIR}/aptkeys ]; then
> > > > +        for keyfile in ${TMPDIR}/aptkeys/*
> > > > +        do
> > > > +            kfn="tmp/$(basename $keyfile)"
> > > > +            cp $keyfile "$ROOTFSDIR/$kfn"
> > > > +            sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key add
> > > > "/$kfn"  
> > > 
> > > Would probably be a good idea to work in /tmp/ as well and in a
> > > subdir. Try naming your key "root" or "usr" ...
> > >   
> > > > +            rm "$ROOTFSDIR/$kfn"
> > > > +        done
> > > > +    fi
> > > >      sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-get update \
> > > >          -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
> > > >          -o Dir::Etc::sourceparts="-" \
> > > > @@ -128,6 +138,7 @@ setup_root_file_system() {
> > > >          sudo -E chroot "$ROOTFSDIR" /usr/bin/dpkg
> > > > --add-architecture ${DISTRO_ARCH} sudo -E chroot
> > > > "$ROOTFSDIR" /usr/bin/apt-get update fi
> > > > +    sudo -E chroot "$ROOTFSDIR" /usr/bin/apt-key update
> > > >      sudo -E chroot "$ROOTFSDIR" \
> > > >          /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
> > > >              ${IMAGE_TRANSIENT_PACKAGES}
> > > > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > > > 234d339..2ef3b1e 100644 ---
> > > > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -23,35
> > > > +23,30 @@ APTSRCS = "${WORKDIR}/apt-sources" APTSRCS_INIT =
> > > > "${WORKDIR}/apt-sources-init" BASEAPTSRCS =
> > > > "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> > > > -APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
> > > > -DEBOOTSTRAP_KEYRING = ""
> > > >  DEPLOY_ISAR_BOOTSTRAP ?= ""
> > > > -DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> > > > +DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales gnupg2
> > > > apt-transport-https ca-certificates"  
> > > 
> > > That looks more like leftovers from an experiment. Are you sure
> > > that is needed?  
> > 
> > This is needed because debootstrap can switch to https by itself if it
> > is unsure about the sources (apt-transport-https, ca-certificates),
> >  gnupg2 we need for apt-key.
> 
> Adding the packages should probably depend on whether DISTRO_APT_KEYS
> is non-empty. And adding the ones that the https-detector adds should
> be tested, not that dual mention makes debootstrap crap out.
> 
> I guess the OVERRIDES method from 9196b820636a9 should be used for gpg
> support as well.
> 
Done in v3.

> Henning
> 
> > > 
> > > 
> > > In general i think that patch should be splittet into two or
> > > even more patches. Because it solves at least two problems.
> > > 
> > > Henning
> > >   
> > > >  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
> > > > "" }" inherit base-apt-helper 
> > > >  python () {
> > > > -    from urllib.parse import urlparse
> > > >      distro_apt_keys = d.getVar("DISTRO_APT_KEYS", False)
> > > > -    wd = d.getVar("WORKDIR", True)
> > > > +    aptkeys = []
> > > > +
> > > >      if distro_apt_keys:
> > > > -        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > > > ${APTKEYRING}")
> > > > -        for key in distro_apt_keys.split():
> > > > -            url = urlparse(key)
> > > > -            filename = ''.join([wd, url.path])
> > > > -            d.appendVar("SRC_URI", " %s" % key)
> > > > -            d.appendVar("APTKEYFILES", " %s" % filename)
> > > > +        aptkeys += distro_apt_keys.split()
> > > > +
> > > >      if
> > > > bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
> > > > own_pub_key = d.getVar("BASE_REPO_KEY", False) if own_pub_key:
> > > > -            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
> > > > ${APTKEYRING}")
> > > > -            for key in own_pub_key.split():
> > > > -                url = urlparse(key)
> > > > -                filename = ''.join([wd, url.path])
> > > > -                d.appendVar("SRC_URI", " %s" % key)
> > > > -                d.appendVar("APTKEYFILES", " %s" % filename)
> > > > +            aptkeys += own_pub_keys.split()
> > > > +
> > > > +    for key in aptkeys:
> > > > +        d.appendVar("SRC_URI", " %s" % key)
> > > > +        fetcher = bb.fetch2.Fetch([key], d)
> > > > +        filename = fetcher.localpath(key)
> > > > +        d.appendVar("APTKEYFILES", " %s" % filename)
> > > >  }
> > > >  
> > > >  def aggregate_files(d, file_list, file_out):
> > > > @@ -174,13 +169,17 @@ def get_distro_components_argument(d,
> > > > is_host): else:
> > > >          return ""
> > > >  
> > > > +APTKEYTMPDIR := "${TMPDIR}/aptkeys"
> > > > +
> > > > +do_generate_keyring[cleandirs] = "${APTKEYTMPDIR}"
> > > >  do_generate_keyring[dirs] = "${DL_DIR}"
> > > >  do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> > > >  do_generate_keyring() {
> > > >      if [ -n "${@d.getVar("APTKEYFILES", True) or ""}" ]; then
> > > > +        chmod 777 "${APTKEYTMPDIR}"
> > > >          for keyfile in ${@d.getVar("APTKEYFILES", True)}; do
> > > > -           gpg --no-default-keyring --keyring "${APTKEYRING}" \
> > > > -               --no-tty --homedir "${DL_DIR}"  --import
> > > > "$keyfile"
> > > > +           cp "$keyfile" "${APTKEYTMPDIR}"/"$(basename
> > > > "$keyfile")"
> > > > +           sudo apt-key add "$keyfile"
> > > >          done
> > > >      fi
> > > >  }
> > > > @@ -222,7 +221,6 @@ isar_bootstrap() {
> > > >              if [ ${IS_HOST} ]; then
> > > >                  ${DEBOOTSTRAP} $debootstrap_args \
> > > >                                 ${@get_distro_components_argument(d,
> > > > True)} \
> > > > -                               ${DEBOOTSTRAP_KEYRING} \
> > > >                                 "${@get_distro_suite(d, True)}" \
> > > >                                 "${ROOTFSDIR}" \
> > > >                                 "${@get_distro_source(d, True)}"
> > > > @@ -231,7 +229,6 @@ isar_bootstrap() {
> > > >                   "${DEBOOTSTRAP}" $debootstrap_args \
> > > >                                    --arch="${DISTRO_ARCH}" \
> > > >                                    ${@get_distro_components_argument(d,
> > > > False)} \
> > > > -                                  ${DEBOOTSTRAP_KEYRING} \
> > > >                                    "${@get_distro_suite(d,
> > > > False)}" \ "${ROOTFSDIR}" \
> > > >                                    "${@get_distro_source(d,
> > > > False)}"  
> > >   
> > 
> 

-- 
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas.Reichel@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-03-06 16:31 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-27 15:18 [PATCH v2 0/3] Fixes usage of additional apt keys Andreas J. Reichel
2019-02-27 15:18 ` [PATCH v2 1/3] Fix and simplify apt keyring generation Andreas J. Reichel
2019-02-27 16:12   ` Henning Schild
2019-03-01  9:09     ` Andreas Reichel
2019-03-01 10:18       ` Henning Schild
2019-03-06 16:29         ` Andreas Reichel
2019-03-01  9:16     ` Andreas Reichel
2019-03-01 10:11       ` Henning Schild
2019-03-01 10:22     ` Henning Schild
2019-03-06 16:29       ` Andreas Reichel
2019-02-27 15:18 ` [PATCH v2 2/3] Use all source lists in target root apt Andreas J. Reichel
2019-02-27 15:18 ` [PATCH v2 3/3] Separate apt-key entries from default keyring Andreas J. Reichel
2019-02-27 16:14   ` Henning Schild
2019-03-01  9:08     ` Andreas Reichel
2019-02-27 16:15 ` [PATCH v2 0/3] Fixes usage of additional apt keys Henning Schild
2019-03-01  9:17   ` Andreas Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox