* base-apt signing interface could be improved
@ 2019-06-06 13:45 Henning Schild
2019-06-13 16:55 ` Amy_Fong@mentor.com
0 siblings, 1 reply; 11+ messages in thread
From: Henning Schild @ 2019-06-06 13:45 UTC (permalink / raw)
To: isar-users; +Cc: 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-06 13:45 base-apt signing interface could be improved Henning Schild
@ 2019-06-13 16:55 ` Amy_Fong@mentor.com
2019-06-14 8:22 ` Henning Schild
0 siblings, 1 reply; 11+ messages in thread
From: Amy_Fong@mentor.com @ 2019-06-13 16:55 UTC (permalink / raw)
To: isar-users
[-- Attachment #1.1: Type: text/plain, Size: 2745 bytes --]
On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
>
> 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
>
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_Fong@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_Fong@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
[-- Attachment #1.2: Type: text/html, Size: 3671 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-13 16:55 ` Amy_Fong@mentor.com
@ 2019-06-14 8:22 ` Henning Schild
2019-06-14 13:50 ` Amy_Fong@mentor.com
0 siblings, 1 reply; 11+ messages in thread
From: Henning Schild @ 2019-06-14 8:22 UTC (permalink / raw)
To: Amy_Fong@mentor.com; +Cc: isar-users
Am Thu, 13 Jun 2019 09:55:29 -0700
schrieb "Amy_Fong@mentor.com" <amy.fong.3142@gmail.com>:
> On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
> >
> > 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
> >
>
> Hi,
>
> Perhaps something like the following ...
>
> Of course, since BASE_REPO_KEY permits specifying
> multiple keys, this raises a question of which keyid?
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_Fong@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_Fong@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
> + 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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-14 8:22 ` Henning Schild
@ 2019-06-14 13:50 ` Amy_Fong@mentor.com
2019-06-17 11:19 ` Henning Schild
0 siblings, 1 reply; 11+ messages in thread
From: Amy_Fong@mentor.com @ 2019-06-14 13:50 UTC (permalink / raw)
To: isar-users
[-- Attachment #1.1: Type: text/plain, Size: 5152 bytes --]
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:>>:
>
> > On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
> > >
> > > 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
> > >
> >
> > Hi,
> >
> > Perhaps something like the following ...
> >
> > Of course, since BASE_REPO_KEY permits specifying
> > multiple keys, this raises a question of which keyid?
>
> 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 <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:>>
> > ---
> > 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
>
> > + 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
> >
>
How about BASE_REPO_SIGN_KEY?
commit 42ee1139e8383fc27e7d98be522cb4d306fd170c (HEAD -> apt_sign)
Author: Amy Fong <Amy_Fong@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_Fong@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
# To generate Release.gpg
- echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions
+ echo "SignWith: $option" >> ${CACHE_CONF_DIR}/distributions
fi
fi
[-- Attachment #1.2: Type: text/html, Size: 10015 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-14 13:50 ` Amy_Fong@mentor.com
@ 2019-06-17 11:19 ` Henning Schild
2019-06-17 11:36 ` Claudius Heine
2019-06-27 17:04 ` vijaikumar.kanagarajan
0 siblings, 2 replies; 11+ messages in thread
From: Henning Schild @ 2019-06-17 11:19 UTC (permalink / raw)
To: Amy_Fong@mentor.com; +Cc: isar-users
Am Fri, 14 Jun 2019 06:50:58 -0700
schrieb "Amy_Fong@mentor.com" <amy.fong.3142@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:>>:
> >
> > > On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
> > > >
> > > > 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
> > > >
> > >
> > > Hi,
> > >
> > > Perhaps something like the following ...
> > >
> > > Of course, since BASE_REPO_KEY permits specifying
> > > multiple keys, this raises a question of which keyid?
> >
> > 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 <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:>>
> > > ---
> > > 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
> >
> > > + 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
> > >
> >
>
> How about BASE_REPO_SIGN_KEY?
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_Fong@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_Fong@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
> --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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-17 11:19 ` Henning Schild
@ 2019-06-17 11:36 ` Claudius Heine
2019-06-28 6:30 ` vijaikumar.kanagarajan
2019-06-27 17:04 ` vijaikumar.kanagarajan
1 sibling, 1 reply; 11+ messages in thread
From: Claudius Heine @ 2019-06-17 11:36 UTC (permalink / raw)
To: [ext] Henning Schild, Amy_Fong@mentor.com; +Cc: isar-users
Hi,
On 17/06/2019 13.19, [ext] Henning Schild wrote:
> Am Fri, 14 Jun 2019 06:50:58 -0700
> schrieb "Amy_Fong@mentor.com" <amy.fong.3142@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:>>:
>>>
>>>> On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
>>>>>
>>>>> 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
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> Perhaps something like the following ...
>>>>
>>>> Of course, since BASE_REPO_KEY permits specifying
>>>> multiple keys, this raises a question of which keyid?
>>>
>>> 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 <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:>>
>>>> ---
>>>> 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
>>>
>>>> + 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
>>>>
>>>
>>
>> How about BASE_REPO_SIGN_KEY?
>
> 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_Fong@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_Fong@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.
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: ch@denx.de
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-17 11:19 ` Henning Schild
2019-06-17 11:36 ` Claudius Heine
@ 2019-06-27 17:04 ` vijaikumar.kanagarajan
2019-06-28 8:04 ` Henning Schild
1 sibling, 1 reply; 11+ messages in thread
From: vijaikumar.kanagarajan @ 2019-06-27 17:04 UTC (permalink / raw)
To: isar-users
[-- Attachment #1.1: Type: text/plain, Size: 7284 bytes --]
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:>>:
>
> > 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:>>:
> > >
> > > > On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
> > > > >
> > > > > 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
> > > > >
> > > >
> > > > Hi,
> > > >
> > > > Perhaps something like the following ...
> > > >
> > > > Of course, since BASE_REPO_KEY permits specifying
> > > > multiple keys, this raises a question of which keyid?
> > >
> > > 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 <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:>>
> > > > ---
> > > > 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
> > >
> > > > + 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
> > > >
> > >
> >
> > How about BASE_REPO_SIGN_KEY?
>
> 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.
>
>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.
Reference: https://manpages.debian.org/stretch/reprepro/reprepro.1.en.html
So I guess it would not be signed by all the keys instead just with the
default key.
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:>>
> >
> > 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
>
> > --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
> >
> >
>
>
[-- Attachment #1.2: Type: text/html, Size: 15281 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-17 11:36 ` Claudius Heine
@ 2019-06-28 6:30 ` vijaikumar.kanagarajan
2019-06-28 8:14 ` Henning Schild
0 siblings, 1 reply; 11+ messages in thread
From: vijaikumar.kanagarajan @ 2019-06-28 6:30 UTC (permalink / raw)
To: isar-users
[-- Attachment #1.1: Type: text/plain, Size: 7979 bytes --]
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:>>:
> >
> >> 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:>>:
> >>>
> >>>> On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild wrote:
> >>>>>
> >>>>> 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
> >>>>>
> >>>>
> >>>> Hi,
> >>>>
> >>>> Perhaps something like the following ...
> >>>>
> >>>> Of course, since BASE_REPO_KEY permits specifying
> >>>> multiple keys, this raises a question of which keyid?
> >>>
> >>> 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 <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:>>
> >>>> ---
> >>>> 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
> >>>
> >>>> + 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
> >>>>
> >>>
> >>
> >> How about BASE_REPO_SIGN_KEY?
> >
> > 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 <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:>>
> >>
> >> 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.
>
> 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
> <javascript:>
>
[-- Attachment #1.2: Type: text/html, Size: 16357 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-27 17:04 ` vijaikumar.kanagarajan
@ 2019-06-28 8:04 ` Henning Schild
0 siblings, 0 replies; 11+ messages in thread
From: Henning Schild @ 2019-06-28 8:04 UTC (permalink / raw)
To: vijaikumar.kanagarajan; +Cc: isar-users
Am Thu, 27 Jun 2019 10:04:05 -0700
schrieb <vijaikumar.kanagarajan@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:>>:
> >
> > > 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:>>:
> > > >
> > > > > On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild
> > > > > wrote:
> > > > > >
> > > > > > 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
> > > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > Perhaps something like the following ...
> > > > >
> > > > > Of course, since BASE_REPO_KEY permits specifying
> > > > > multiple keys, this raises a question of which keyid?
> > > >
> > > > 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
> > > > > <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:>>
> > > > > ---
> > > > > 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
> > > >
> > > > > + 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
> > > > >
> > > >
> > >
> > > How about BASE_REPO_SIGN_KEY?
> >
> > 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.
> >
>
> 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.
>
> Reference:
> https://manpages.debian.org/stretch/reprepro/reprepro.1.en.html
>
> So I guess it would not be signed by all the keys instead just with
> the default key.
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:>>
> > >
> > > 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
> >
> > > --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
> > >
> > >
> >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-28 6:30 ` vijaikumar.kanagarajan
@ 2019-06-28 8:14 ` Henning Schild
2019-07-24 8:47 ` Vijai Kumar K
0 siblings, 1 reply; 11+ messages in thread
From: Henning Schild @ 2019-06-28 8:14 UTC (permalink / raw)
To: vijaikumar.kanagarajan; +Cc: 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.kanagarajan@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:>>:
> > >
> > >> 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:>>:
> > >>>
> > >>>> On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild
> > >>>> wrote:
> > >>>>>
> > >>>>> 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
> > >>>>>
> > >>>>
> > >>>> Hi,
> > >>>>
> > >>>> Perhaps something like the following ...
> > >>>>
> > >>>> Of course, since BASE_REPO_KEY permits specifying
> > >>>> multiple keys, this raises a question of which keyid?
> > >>>
> > >>> 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
> > >>>> <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:>>
> > >>>> ---
> > >>>> 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
> > >>>
> > >>>> + 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
> > >>>>
> > >>>
> > >>
> > >> How about BASE_REPO_SIGN_KEY?
> > >
> > > 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 <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:>>
> > >>
> > >> 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.
> >
> > 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 <javascript:>
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: base-apt signing interface could be improved
2019-06-28 8:14 ` Henning Schild
@ 2019-07-24 8:47 ` Vijai Kumar K
0 siblings, 0 replies; 11+ messages in thread
From: Vijai Kumar K @ 2019-07-24 8:47 UTC (permalink / raw)
To: Henning Schild; +Cc: vijaikumar.kanagarajan, 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
> 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.kanagarajan@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:>>:
> > > >
> > > >> 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:>>:
> > > >>>
> > > >>>> On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild
> > > >>>> wrote:
> > > >>>>>
> > > >>>>> 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
> > > >>>>>
> > > >>>>
> > > >>>> Hi,
> > > >>>>
> > > >>>> Perhaps something like the following ...
> > > >>>>
> > > >>>> Of course, since BASE_REPO_KEY permits specifying
> > > >>>> multiple keys, this raises a question of which keyid?
> > > >>>
> > > >>> 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
> > > >>>> <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:>>
> > > >>>> ---
> > > >>>> 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
> > > >>>
> > > >>>> + 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
> > > >>>>
> > > >>>
> > > >>
> > > >> How about BASE_REPO_SIGN_KEY?
> > > >
> > > > 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 <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:>>
> > > >>
> > > >> 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.
> > >
> > > 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 <javascript:>
> > >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-07-24 8:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 13:45 base-apt signing interface could be improved Henning Schild
2019-06-13 16:55 ` Amy_Fong@mentor.com
2019-06-14 8:22 ` Henning Schild
2019-06-14 13:50 ` Amy_Fong@mentor.com
2019-06-17 11:19 ` Henning Schild
2019-06-17 11:36 ` Claudius Heine
2019-06-28 6:30 ` vijaikumar.kanagarajan
2019-06-28 8:14 ` Henning Schild
2019-07-24 8:47 ` Vijai Kumar K
2019-06-27 17:04 ` vijaikumar.kanagarajan
2019-06-28 8:04 ` Henning Schild
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox