base-apt signing interface could be improved

51 views
Skip to first unread message

Henning Schild

unread,
Jun 6, 2019, 9:46:02 AM6/6/19
to isar-...@googlegroups.com, Vijai Kumar K
Hi,

i just had a quick look at the implementation of the base-apt signing
for the first time. The interface is not ideal and has potential for
the signing key and the checking key not actually belonging together.

As far as i understand the code i read, Isar will start signing
base-apt if BASE_REPO_KEY is set to anything. The private key it will
use to sign the repo is not specified at all, it will be whatever gnupg
defaults to, given its configuration.

I would suggest to switch from "SignWith yes" to "SignWith <keyid>",
and derive the id from BASE_REPO_KEY.

Further improvements would be to actually configure gnupg inside Isar
and not rely on an outside configuration. Relying on the outside config
means that all (multi)configs will have to use the same keypair.
So we would add

BASE_REPO_KEY_PRIVATE and ..._PASSPHRASE

Now we would create a new gpg homedir next to where we store base-apt.
We would import that one key there and potentially unlock it with its
passphrase. If we clean and rebuild we get a working gpghome for sure.

Henning

amy.fo...@gmail.com

unread,
Jun 13, 2019, 12:55:29 PM6/13/19
to isar-users

Hi,

Perhaps something like the following ...

Of course, since BASE_REPO_KEY permits specifying
multiple keys, this raises a question of which keyid?

Amy

From 5ceb4a2ef97bc7fa6c44cd9ce6f73f9a831773f3 Mon Sep 17 00:00:00 2001
From: Amy Fong <Amy_...@mentor.com>
Date: Thu, 13 Jun 2019 12:52:06 -0400
Subject: [PATCH] base-apt: Use BASE_REPO_KEY for signing

Extract keyid from BASE_REPO_KEY for signing

Signed-off-by: Amy Fong <Amy_...@mentor.com>
---
 meta/recipes-devtools/base-apt/base-apt.bb | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
index 1c0b4c6..81245f7 100644
--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -19,8 +19,15 @@ do_cache_config() {
         sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \
             ${WORKDIR}/distributions.in > ${CACHE_CONF_DIR}/distributions
         if [ "${BASE_REPO_KEY}" ] ; then
+            option="yes"
+            for key in ${BASE_REPO_KEY}; do
+                keyid=$(wget -qO - $key | gpg --keyid-format 0xlong --with-colons - 2>/dev/null |grep "^pub:" |awk -F':' '{print $5;}')
+                if [ -n "$keyid" ]; then
+                    option="$keyid"
+                fi
+            done
             # To generate Release.gpg
-            echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
+            echo "SignWith: $option" >> ${CACHE_CONF_DIR}/distributions
         fi
     fi
 
-- 
2.20.1

 

Henning Schild

unread,
Jun 14, 2019, 4:23:00 AM6/14/19
to amy.fo...@gmail.com, isar-users
Am Thu, 13 Jun 2019 09:55:29 -0700
schrieb "Amy_...@mentor.com" <amy.fo...@gmail.com>:
Oh that is a nice hidden feature, indeed one can specify multiple keys
there. So that variable should be called BASE_REPO_KEYS instead.

And yes reprepro also supports multiple values. So i guess your patch
is correct and it would probably sign the repo with all the keys
specified.

Whether that is what we want is another question, and i am not sure
whether "yes" will also use all keys or just the default one.

> Amy
>
> From 5ceb4a2ef97bc7fa6c44cd9ce6f73f9a831773f3 Mon Sep 17 00:00:00 2001
> From: Amy Fong <Amy_...@mentor.com>
> Date: Thu, 13 Jun 2019 12:52:06 -0400
> Subject: [PATCH] base-apt: Use BASE_REPO_KEY for signing
>
> Extract keyid from BASE_REPO_KEY for signing
>
> Signed-off-by: Amy Fong <Amy_...@mentor.com>
> ---
> meta/recipes-devtools/base-apt/base-apt.bb | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/meta/recipes-devtools/base-apt/base-apt.bb
> b/meta/recipes-devtools/base-apt/base-apt.bb
> index 1c0b4c6..81245f7 100644
> --- a/meta/recipes-devtools/base-apt/base-apt.bb
> +++ b/meta/recipes-devtools/base-apt/base-apt.bb
> @@ -19,8 +19,15 @@ do_cache_config() {
> sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \
> ${WORKDIR}/distributions.in >
> ${CACHE_CONF_DIR}/distributions if [ "${BASE_REPO_KEY}" ] ; then
> + option="yes"

maybe there is a better name for the variable?

Henning

amy.fo...@gmail.com

unread,
Jun 14, 2019, 9:50:58 AM6/14/19
to isar-users


On Friday, 14 June 2019 04:23:00 UTC-4, Henning Schild wrote:
Am Thu, 13 Jun 2019 09:55:29 -0700

How about BASE_REPO_SIGN_KEY?

commit 42ee1139e8383fc27e7d98be522cb4d306fd170c (HEAD -> apt_sign)
Author: Amy Fong <Amy_...@mentor.com>
Date:   Thu Jun 13 12:52:06 2019 -0400

    base-apt: Use BASE_REPO_SIGN_KEY for signing
    
    Extract keyid from BASE_REPO_SIGN_KEY for signing
    
    Signed-off-by: Amy Fong <Amy_...@mentor.com>

diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
index 1c0b4c6..c896add 100644
--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -18,9 +18,14 @@ do_cache_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 [ "${BASE_REPO_SIGN_KEY}" ] ; then
+            option="yes"
+            keyid=$(wget -qO - "${BASE_REPO_SIGN_KEY}" | gpg --keyid-format 0xlong --with-colons - 2>/dev/null |grep "^pub:" |awk -F':' '{print $5;}')
+            if [ -n "$keyid" ]; then
+                option="$keyid"
+            fi

Henning Schild

unread,
Jun 17, 2019, 7:19:58 AM6/17/19
to amy.fo...@gmail.com, isar-users
Am Fri, 14 Jun 2019 06:50:58 -0700
schrieb "Amy_...@mentor.com" <amy.fo...@gmail.com>:

> On Friday, 14 June 2019 04:23:00 UTC-4, Henning Schild wrote:
> >
> > Am Thu, 13 Jun 2019 09:55:29 -0700
> > schrieb "Amy_...@mentor.com <javascript:>" <amy.f...@gmail.com
> > <javascript:>>:
> > > 2001 From: Amy Fong <Amy_...@mentor.com <javascript:>>
> > > Date: Thu, 13 Jun 2019 12:52:06 -0400
> > > Subject: [PATCH] base-apt: Use BASE_REPO_KEY for signing
> > >
> > > Extract keyid from BASE_REPO_KEY for signing
> > >
> > > Signed-off-by: Amy Fong <Amy_...@mentor.com <javascript:>>
I do not understand what you are trying to solve with changing that
name and going back to one-key-only, after you have found that
BASE_REPO_KEY is indeed an array and reprepro also accepts an array.

Now we need to know what "yes", compared to the array.

And any tiny patch like this one, without a proper commit message and
description, is not going to lead anywhere good.

You guys are doing the full story. kas, signed base-apt, multiple keys,
agent-forwarding ...
After you are done you should have a clear picture of what currently
does not work as expected, and how it can be fixes (your initial
implementation).
We can then discuss that implementation and incorporate a full patch
series including docs into kas and Isar.

> commit 42ee1139e8383fc27e7d98be522cb4d306fd170c (HEAD -> apt_sign)
> Author: Amy Fong <Amy_...@mentor.com>
> Date: Thu Jun 13 12:52:06 2019 -0400
>
> base-apt: Use BASE_REPO_SIGN_KEY for signing
>
> Extract keyid from BASE_REPO_SIGN_KEY for signing
>
> Signed-off-by: Amy Fong <Amy_...@mentor.com>
>
> diff --git a/meta/recipes-devtools/base-apt/base-apt.bb
> b/meta/recipes-devtools/base-apt/base-apt.bb
> index 1c0b4c6..c896add 100644
> --- a/meta/recipes-devtools/base-apt/base-apt.bb
> +++ b/meta/recipes-devtools/base-apt/base-apt.bb
> @@ -18,9 +18,14 @@ do_cache_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 [ "${BASE_REPO_SIGN_KEY}" ] ; then
> + option="yes"
> + keyid=$(wget -qO - "${BASE_REPO_SIGN_KEY}" | gpg

Using wget, but that is most likely a "file:///" URI. And whenever you
do networking in a task, you need to take care of proxies.

Henning

Claudius Heine

unread,
Jun 17, 2019, 7:36:29 AM6/17/19
to [ext] Henning Schild, amy.fo...@gmail.com, isar-users
Hi,
Fetching should not be done like this anyway. If something needs to be
fetched then it should be part of the SRC_URI and be fetched by the
do_fetch task. The reasons for this are offline reproducibility among
others.

regards,
Claudius

>
> Henning
>
>> --keyid-format 0xlong --with-colons - 2>/dev/null |grep "^pub:" |awk
>> -F':' '{print $5;}')
>> + if [ -n "$keyid" ]; then
>> + option="$keyid"
>> + fi
>> # To generate Release.gpg
>> - echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
>> + echo "SignWith: $option" >>
>> ${CACHE_CONF_DIR}/distributions fi
>> fi
>>
>>
>

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de

vijaikumar....@gmail.com

unread,
Jun 27, 2019, 1:04:05 PM6/27/19
to isar-users


On Monday, June 17, 2019 at 4:49:58 PM UTC+5:30, Henning Schild wrote:
Am Fri, 14 Jun 2019 06:50:58 -0700

From reprepro manual:

SignWith
When this field is present, a Release.gpg file will be generated. If the value is "yes" or "default", the default key of gpg is used.


So I guess it would not be signed by all the keys instead just with the default key.

Thanks,
Vijai Kumar K

vijaikumar....@gmail.com

unread,
Jun 28, 2019, 2:30:32 AM6/28/19
to isar-users
Hi Claudius,

Just wondering, why cant the BASE_REPO_KEY be a list of key ids instead of the keyfile itself. Since we are using the host gpg agent anyways for signing.

Later if this key needs to be added to apt sources key ring, it can be exported.

The one advantage is that it eliminates the need to maintain the keyfiles in host. It is redundant. Anyway one would need to have the keys as part of gpg keyring to successfully sign the repo.

Thanks,
Vijai Kumar K

On Monday, June 17, 2019 at 5:06:29 PM UTC+5:30, Claudius Heine wrote:
Hi,

On 17/06/2019 13.19, [ext] Henning Schild wrote:
> Am Fri, 14 Jun 2019 06:50:58 -0700

Henning Schild

unread,
Jun 28, 2019, 4:04:40 AM6/28/19
to vijaikumar....@gmail.com, isar-users
Am Thu, 27 Jun 2019 10:04:05 -0700
schrieb <vijaikumar....@gmail.com>:

> On Monday, June 17, 2019 at 4:49:58 PM UTC+5:30, Henning Schild wrote:
> >
> > Am Fri, 14 Jun 2019 06:50:58 -0700
> > schrieb "Amy_...@mentor.com <javascript:>" <amy.f...@gmail.com
> > <javascript:>>:
Thanks for looking this up. That actually means that the current code
is incorrect.
- the variable is an array but the name is singular
- every step, except reprepro deals with that correctly

So the idea of one of your patches is correct, and a fixed version of
it should be merged.

Henning

> Thanks,
> Vijai Kumar K
>
>
> > And any tiny patch like this one, without a proper commit message
> > and description, is not going to lead anywhere good.
> >
> > You guys are doing the full story. kas, signed base-apt, multiple
> > keys, agent-forwarding ...
> > After you are done you should have a clear picture of what
> > currently does not work as expected, and how it can be fixes (your
> > initial implementation).
> > We can then discuss that implementation and incorporate a full
> > patch series including docs into kas and Isar.
> >
> > > commit 42ee1139e8383fc27e7d98be522cb4d306fd170c (HEAD ->
> > > apt_sign) Author: Amy Fong <Amy_...@mentor.com <javascript:>>
> > > Date: Thu Jun 13 12:52:06 2019 -0400
> > >
> > > base-apt: Use BASE_REPO_SIGN_KEY for signing
> > >
> > > Extract keyid from BASE_REPO_SIGN_KEY for signing
> > >
> > > Signed-off-by: Amy Fong <Amy_...@mentor.com <javascript:>>

Henning Schild

unread,
Jun 28, 2019, 4:14:51 AM6/28/19
to vijaikumar....@gmail.com, isar-users
Hi Vijai,

you are assuming kas and the agent implementation that mentor proposed
there.

two things to keep in mind:
- kas is not just for Isar, but also yocto/oe
- Isar can be used without kas
- kas can be used without docker

The current implementation does not work in so many places because it
was never applied on a real use-case. You now find the limitations
because you have that use-case.

Again, please implement the full story, including documentation, and
propose patches that enable such a use-case.

These small intermediate steps just bring isar and kas into the next
intermediate state between early-feature and working. And what might
seem correct today may become a revert and an API breakage tomorrow.

Henning

Am Thu, 27 Jun 2019 23:30:31 -0700
schrieb <vijaikumar....@gmail.com>:

> Hi Claudius,
>
> Just wondering, why cant the BASE_REPO_KEY be a list of key ids
> instead of the keyfile itself. Since we are using the host gpg agent
> anyways for signing.
>
> Later if this key needs to be added to apt sources key ring, it can
> be exported.
>
> The one advantage is that it eliminates the need to maintain the
> keyfiles in host. It is redundant. Anyway one would need to have the
> keys as part of gpg keyring to successfully sign the repo.
>
> Thanks,
> Vijai Kumar K
>
> On Monday, June 17, 2019 at 5:06:29 PM UTC+5:30, Claudius Heine wrote:
> >
> > Hi,
> >
> > On 17/06/2019 13.19, [ext] Henning Schild wrote:
> > > Am Fri, 14 Jun 2019 06:50:58 -0700
> > > schrieb "Amy_...@mentor.com <javascript:>" <amy.f...@gmail.com
> > <javascript:>>:
> > >> apt_sign) Author: Amy Fong <Amy_...@mentor.com <javascript:>>
> > >> Date: Thu Jun 13 12:52:06 2019 -0400
> > >>
> > >> base-apt: Use BASE_REPO_SIGN_KEY for signing
> > >>
> > >> Extract keyid from BASE_REPO_SIGN_KEY for signing
> > >>
> > >> Signed-off-by: Amy Fong <Amy_...@mentor.com <javascript:>>
> > c...@denx.de <javascript:>
> >
>

Vijai Kumar K

unread,
Jul 24, 2019, 4:47:38 AM7/24/19
to Henning Schild, vijaikumar....@gmail.com, isar-users
On Fri, Jun 28, 2019 at 10:14:48AM +0200, Henning Schild wrote:
Hi Henning,

Thanks for info. Sorry for the late reply. Was a little busy testing
some kas changes. I have some observations wrt ISAR and since all
of these is in the package feed creation part I will use this thread
to sum it up.

Hi All,

I have been using ISAR for quite some time and have observed the following
limitations/issues with it, mostly in the package feed creation part.


1. ISAR is not ready for handling password protected keys. Reprepro still
uses password prompt(default pinentry by the system). i.e we cannot automate
the password input to reprepro.

Description:
If we were to generate base-apt with a password protected gpg key then ISAR
build would fail unless the password is saved or entered in the pinentry
prompt.

Possible Solution:
We could possibly have a variable called BASE_REPO_PASSPHRASE which would
store the password for reprepro to use when signing. One issue here is that
reprepro uses the pinentry used by gpg agent. We would need to configure gpg
to loopback pinentry and pass in the password.
BASE_REPO_PASSPHRASE is mostly inspired by the RPM_GPG_PASSPHRASE in oe.
https://git.yoctoproject.org/cgit.cgi/poky/plain/meta/classes/sign_rpm.bbclass


2. Reprepro implementation in ISAR doesnot take into consideration the
GNUPGHOME env.

Description:
Lets say the user has the .gnupg directory in a custom location
(ex: /opt/.gnupg) in his server and has set GNUPGHOME to /opt/.gnupg.
If you trigger a base-apt generation with ISAR in that server, then reprepro
command would fail(if there is no ~/.gnupg). reprepro in bitbake environment
would not know about the GNUPGHOME varible.

Possible solution:
One could add GNUPGHOME to BB_ENV_EXTRAWHITE and if is not empty can export it
to the environment where reprepro is called.
+ if [ ! -z ${GNUPGHOME} ]; then
+ export GNUPGHOME=${GNUPGHOME}
+ fi

3. ISAR repo signing would always use the default GPG-key of the system. One
cannot specify the key to use.

Description:
This is particularly tedious in case of CI. The user might have a server which
has n GPG keys in it and may wanted to sign the repo using the nth key. So one
would probably export the public key and set BASE_REPO_KEY=<path to the key>.
But reprepro would just use the default key in the system which might be a
completely different key.

Possible solution:
We could use the key ids instead of the key file itself. Something like
BASE_REPO_KEYID="6FA1B2F0416E3C24".
And the reprepro distributions file can be as below.
Codename: stretch
Architectures: i386 armhf arm64 amd64 source
Components: main
SignWith: "6FA1B2F0416E3C24"

Please let me know your thoughts on this. Meanwhile I will test some of my
changes for the above issues and post the patchset for review.

Thanks,
Vijai Kumar K
Reply all
Reply to author
Forward
0 new messages