[PATCH] base-apt: Use gpg keyid instead of yes

[vijaikumar....@gmail.com]

From: Vijai Kumar K <Vijaikumar_...@mentor.com>

When using "SignWith: yes", reprepro uses the default gpg key
of the system to sign the repo. The default gpg key might be
different from what is specified in BASE_REPO_KEY, resulting
in using a wrong key for signing.

Derive and use the keyid from the keyfile supplied instead of
a generic yes option.

Suggested-by: Amy Fong <Amy_...@mentor.com>
Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com>
---
meta/recipes-devtools/base-apt/base-apt.bb | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
index 74189f1..c74be86 100644
--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -4,6 +4,7 @@
SRC_URI = "file://distributions.in"

BASE_REPO_KEY ?= ""
+KEYFILES ?= ""

CACHE_CONF_DIR = "${REPO_BASE_DIR}/${BASE_DISTRO}/conf"
do_cache_config[dirs] = "${CACHE_CONF_DIR}"
@@ -12,13 +13,18 @@ do_cache_config[lockfiles] = "${REPO_BASE_DIR}/isar.lock"

# Generate reprepro config for current distro if it doesn't exist. Once it's
# generated, this task should do nothing.
-do_cache_config() {
+repo_config() {
if [ ! -e "${CACHE_CONF_DIR}/distributions" ]; then
sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \
${WORKDIR}/distributions.in > ${CACHE_CONF_DIR}/distributions
- if [ "${BASE_REPO_KEY}" ] ; then
+ if [ -n "${KEYFILES}" ]; then
+ option=""
+ for key in ${KEYFILES}; do
+ keyid=$(cat ${key} | gpg --keyid-format 0xlong --with-colons - 2>/dev/null |grep "^pub:" |awk -F':' '{print $5;}')
+ option="${option}${keyid} "
+ done
# To generate Release.gpg
- echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
+ echo "SignWith: ${option}" >> ${CACHE_CONF_DIR}/distributions
fi
fi

@@ -35,4 +41,14 @@ do_cache_config() {
fi
}

+python do_cache_config() {
+ for key in d.getVar('BASE_REPO_KEY').split():
+ d.appendVar("SRC_URI", " %s" % key)
+ fetcher = bb.fetch2.Fetch([key], d)
+ filename = fetcher.localpath(key)
+ d.appendVar("KEYFILES", " %s" % filename)
+
+ bb.build.exec_func('repo_config', d)
+}
+
addtask cache_config after do_build
--
2.17.1

[Jan Kiszka]

I hope this parsing is stable...

> + option="${option}${keyid} "
> + done
> # To generate Release.gpg
> - echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
> + echo "SignWith: ${option}" >> ${CACHE_CONF_DIR}/distributions
> fi
> fi
>
> @@ -35,4 +41,14 @@ do_cache_config() {
> fi
> }
>
> +python do_cache_config() {
> + for key in d.getVar('BASE_REPO_KEY').split():
> + d.appendVar("SRC_URI", " %s" % key)
> + fetcher = bb.fetch2.Fetch([key], d)

I wonder if that magically addresses the case that changing key file content
should also trigger rebuilds. Similar to https://github.com/ilbers/isar/issues/60.

> + filename = fetcher.localpath(key)
> + d.appendVar("KEYFILES", " %s" % filename)
> +
> + bb.build.exec_func('repo_config', d)
> +}
> +
> addtask cache_config after do_build
>

Looks good - if the keyid extraction if actually robust.

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[Vijai Kumar K]

Having used it for quite sometime I dont see an issue. It would be better if we error out if the key is not present
in the system. Will add it in v2.

>
> > + option="${option}${keyid} "
> > + done
> > # To generate Release.gpg
> > - echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
> > + echo "SignWith: ${option}" >> ${CACHE_CONF_DIR}/distributions
> > fi
> > fi
> > @@ -35,4 +41,14 @@ do_cache_config() {
> > fi
> > }
> > +python do_cache_config() {
> > + for key in d.getVar('BASE_REPO_KEY').split():
> > + d.appendVar("SRC_URI", " %s" % key)
> > + fetcher = bb.fetch2.Fetch([key], d)
>
> I wonder if that magically addresses the case that changing key file content
> should also trigger rebuilds. Similar to
> https://github.com/ilbers/isar/issues/60.

Not sure about that. May be some testing would reveal it.

>
> > + filename = fetcher.localpath(key)
> > + d.appendVar("KEYFILES", " %s" % filename)
> > +
> > + bb.build.exec_func('repo_config', d)
> > +}
> > +
> > addtask cache_config after do_build
> >
>
> Looks good - if the keyid extraction if actually robust.
>

Thanks,
Vijai Kumar K

[vijai kumar]

On Monday, September 30, 2019 at 2:12:34 PM UTC+5:30, vijai kumar wrote:

On Mon, Sep 30, 2019 at 08:17:00AM +0200, Jan Kiszka wrote:

> On 27.09.19 23:11, vijaikumar.kanagarajan@gmail.com wrote:
> > From: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>

> >
> > When using "SignWith: yes", reprepro uses the default gpg key
> > of the system to sign the repo. The default gpg key might be
> > different from what is specified in BASE_REPO_KEY, resulting
> > in using a wrong key for signing.
> >
> > Derive and use the keyid from the keyfile supplied instead of
> > a generic yes option.
> >
> > Suggested-by: Amy Fong <Amy_...@mentor.com>

> > Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>

On a second thought that condition check is unnecessary at this point. It could just fail at signing when it is not able to find the key.

So no V2.

[Henning Schild]

Am Mon, 30 Sep 2019 08:17:00 +0200
schrieb Jan Kiszka <jan.k...@siemens.com>:

It looks ok, the format args used are meant for parsing.

But i would like to point out the useless use of cat.
Plus an inconsistent use of spaces around pipes.

Henning

[vijaikumar....@gmail.com]

From: Vijai Kumar K <Vijaikumar_...@mentor.com>

When using "SignWith: yes", reprepro uses the default gpg key
of the system to sign the repo. The default gpg key might be
different from what is specified in BASE_REPO_KEY, resulting
in using a wrong key for signing.

Derive and use the keyid from the keyfile supplied instead of
a generic yes option.

Suggested-by: Amy Fong <Amy_...@mentor.com>
Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com>
---

Changes in v2:
- Address review comments from Henning.

meta/recipes-devtools/base-apt/base-apt.bb | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb

index 6acd6e7..42ff782 100644

--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -4,6 +4,7 @@
SRC_URI = "file://distributions.in"

BASE_REPO_KEY ?= ""
+KEYFILES ?= ""

CACHE_CONF_DIR = "${REPO_BASE_DIR}/${BASE_DISTRO}/conf"
do_cache_config[dirs] = "${CACHE_CONF_DIR}"
@@ -12,13 +13,18 @@ do_cache_config[lockfiles] = "${REPO_BASE_DIR}/isar.lock"

# Generate reprepro config for current distro if it doesn't exist. Once it's
# generated, this task should do nothing.
-do_cache_config() {
+repo_config() {
if [ ! -e "${CACHE_CONF_DIR}/distributions" ]; then
sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \
${WORKDIR}/distributions.in > ${CACHE_CONF_DIR}/distributions
- if [ "${BASE_REPO_KEY}" ] ; then
+ if [ -n "${KEYFILES}" ]; then
+ option=""
+ for key in ${KEYFILES}; do

+ keyid=$(gpg --keyid-format 0xlong --with-colons ${key} 2>/dev/null | grep "^pub:" | awk -F':' '{print $5;}')

+ option="${option}${keyid} "
+ done
# To generate Release.gpg
- echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
+ echo "SignWith: ${option}" >> ${CACHE_CONF_DIR}/distributions
fi
fi

@@ -35,4 +41,14 @@ do_cache_config() {
fi
}

+python do_cache_config() {
+ for key in d.getVar('BASE_REPO_KEY').split():
+ d.appendVar("SRC_URI", " %s" % key)
+ fetcher = bb.fetch2.Fetch([key], d)

+ filename = fetcher.localpath(key)
+ d.appendVar("KEYFILES", " %s" % filename)
+
+ bb.build.exec_func('repo_config', d)
+}
+

addtask cache_config after do_unpack before do_build
--
2.17.1

[vijai kumar]

This v2 is also rebased on top of current next. Forgot to mention that.

Thanks,

Vijai Kumar K

On Tuesday, October 15, 2019 at 2:47:55 PM UTC+5:30, vijai kumar wrote:

From: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>

When using "SignWith: yes", reprepro uses the default gpg key
of the system to sign the repo. The default gpg key might be
different from what is specified in BASE_REPO_KEY, resulting
in using a wrong key for signing.

Derive and use the keyid from the keyfile supplied instead of
a generic yes option.

Suggested-by: Amy Fong <Amy_...@mentor.com>

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>

[Vijai Kumar K]

On Tue, Oct 15, 2019 at 02:19:36AM -0700, vijai kumar wrote:

If there are no more review comments, can this be merged to next?

Thanks,
Vijai Kumar K

> This v2 is also rebased on top of current next. Forgot to mention that.
>
> Thanks,
> Vijai Kumar K
>
> On Tuesday, October 15, 2019 at 2:47:55 PM UTC+5:30, vijai kumar wrote:
> >
> > From: Vijai Kumar K <Vijaikumar_...@mentor.com>
> >
> > When using "SignWith: yes", reprepro uses the default gpg key
> > of the system to sign the repo. The default gpg key might be
> > different from what is specified in BASE_REPO_KEY, resulting
> > in using a wrong key for signing.
> >
> > Derive and use the keyid from the keyfile supplied instead of
> > a generic yes option.
> >
> > Suggested-by: Amy Fong <Amy_...@mentor.com>

> > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com>

> --
> You received this message because you are subscribed to the Google Groups "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/c20121db-766f-49e2-b64b-d25761d4f1bb%40googlegroups.com.

[Baurzhan Ismagulov]

Hello Vijai Kumar,

On Tue, Oct 15, 2019 at 02:47:23PM +0530, vijaikumar....@gmail.com wrote:
> When using "SignWith: yes", reprepro uses the default gpg key
> of the system to sign the repo. The default gpg key might be
> different from what is specified in BASE_REPO_KEY, resulting
> in using a wrong key for signing.
>
> Derive and use the keyid from the keyfile supplied instead of
> a generic yes option.

I'm experiencing the problems below, could you please have a look?

http://ci.isar-build.org:8080/job/isar_ibr_next/lastFailedBuild/consoleFull

bitbake -v -c cache_base_repo multiconfig:qemuarm-stretch:isar-image-base multiconfig:qemuarm64-stretch:isar-image-base multiconfig:qemuamd64-stretch:isar-image-base multiconfig:qemuarm-buster:isar-image-base
...
ERROR: mc:qemuarm-buster:base-apt-1.0-r0 do_cache_config: Function failed: repo_config (log file is located at /workspace/build/isar_ibr_next/12/build/tmp/work/debian-buster-armhf/base-apt/1.0-r0/temp/log.do_cache_config.16781)
...
| + export GNUPGHOME=/tmp/tmp.laEgOqLEko
| + reprepro -b /workspace/build/isar_ibr_next/12/build/downloads/base-apt/debian-buster/apt/debian --dbdir /workspace/build/isar_ibr_next/12/build/downloads/base-apt/debian-buster/db/debian export buster
| Error parsing /workspace/build/isar_ibr_next/12/build/downloads/base-apt/debian-buster/apt/debian/conf/distributions, line 4, column 12: Missing value for SignWith field.

With kind regards,
Baurzhan.

[vijai kumar]

Sure Baurzhan. I'm on travel. I'll look into it first thing Monday morning.

Thanks,

Vijai Kumar K

--
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20191102155725.ecot7nhc4f3pqxck%40yssyq.m.ilbers.de.

[vijaikumar....@gmail.com]

From: Vijai Kumar K <Vijaikumar_...@mentor.com>

CI triggers the build in a chrooted environment with userspec
option set to host user. GPG commands would fail since we donot have
access to the $HOME folder in chroot.

Make GNUPGHOME available to avoid failures due to permission issues.

Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com>
---

Hi Baurzhan,

This patch should fix the issue we are facing in CI.

There is a job running successfully with this patchset at
http://ci.isar-build.org:8080/job/isar_vkk_devel/10/consoleFull


Thanks,
Vijai Kumar K

meta/recipes-devtools/base-apt/base-apt.bb | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
index 42ff782..9a0f7c8 100644
--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -14,6 +14,10 @@ do_cache_config[lockfiles] = "${REPO_BASE_DIR}/isar.lock"

# Generate reprepro config for current distro if it doesn't exist. Once it's
# generated, this task should do nothing.

repo_config() {
+ if [ -n "${GNUPGHOME}" ]; then
+ export GNUPGHOME="${GNUPGHOME}"
+ fi
+

if [ ! -e "${CACHE_CONF_DIR}/distributions" ]; then
sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \
${WORKDIR}/distributions.in > ${CACHE_CONF_DIR}/distributions

@@ -32,9 +36,6 @@ repo_config() {
path_databases="${REPO_BASE_DB_DIR}/${BASE_DISTRO}"

if [ ! -d "${path_databases}" ]; then
- if [ -n "${GNUPGHOME}" ]; then
- export GNUPGHOME="${GNUPGHOME}"
- fi
reprepro -b ${path_cache} \
--dbdir ${path_databases} \
export ${BASE_DISTRO_CODENAME}
--
2.17.1

[vijai kumar]

The build succeeded.

Thanks,

Vijai Kumar K

On Tuesday, November 5, 2019 at 4:03:27 PM UTC+5:30, vijai kumar wrote:

From: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>

CI triggers the build in a chrooted environment with userspec
option set to host user. GPG commands would fail since we donot have
access to the $HOME folder in chroot.

Make GNUPGHOME available to avoid failures due to permission issues.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>

[Baurzhan Ismagulov]

On Tue, Oct 15, 2019 at 02:47:23PM +0530, vijaikumar....@gmail.com wrote:

> When using "SignWith: yes", reprepro uses the default gpg key
> of the system to sign the repo. The default gpg key might be
> different from what is specified in BASE_REPO_KEY, resulting
> in using a wrong key for signing.
>
> Derive and use the keyid from the keyfile supplied instead of
> a generic yes option.

Applied to next, thanks.

With kind regards,
Baurzhan.

[Baurzhan Ismagulov]

On Tue, Nov 05, 2019 at 04:03:14PM +0530, vijaikumar....@gmail.com wrote:
> CI triggers the build in a chrooted environment with userspec
> option set to host user. GPG commands would fail since we donot have
> access to the $HOME folder in chroot.
>
> Make GNUPGHOME available to avoid failures due to permission issues.