From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6699413522129879040 Date: Thu, 27 Jun 2019 10:04:05 -0700 (PDT) From: vijaikumar.kanagarajan@gmail.com To: isar-users Message-Id: <6cd52b1a-5d97-468f-b262-61e8bc461f57@googlegroups.com> In-Reply-To: <20190617131937.2852d692@md1za8fc.ad001.siemens.net> References: <20190606154558.7eea07bd@md1za8fc.ad001.siemens.net> <20190614102255.0c782b51@md1za8fc.ad001.siemens.net> <20190617131937.2852d692@md1za8fc.ad001.siemens.net> Subject: Re: base-apt signing interface could be improved MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_436_492020416.1561655045275" X-Google-Token: EIXu0-gFuYZe0Aa5xps0 X-Google-IP: 192.94.34.34 X-TUID: 5o8nnZKA/VSX ------=_Part_436_492020416.1561655045275 Content-Type: multipart/alternative; boundary="----=_Part_437_479907406.1561655045275" ------=_Part_437_479907406.1561655045275 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 " >: > > > 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 " > > >: > > > > > > > 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 > > > > > ", 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 > > > > > 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 > > > > > --- > > > > 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 > > > 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 > > > > > 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 > > > > > > ------=_Part_437_479907406.1561655045275 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable


On Monday, June 17, 2019 at 4:49:58 PM UTC+5:30, H= enning Schild wrote:
Am Fri, 14= Jun 2019 06:50:58 -0700
schrieb "Amy_...@mentor.com" <amy.f...@gmail.com>:

> On Friday, 14 June 2019 04:23:00 UTC-4, Henning Schild wrote:
> >
> > Am Thu, 13 Jun 2019 09:55:29 -0700=20
> > schrieb "Amy_...@mentor.com <javascript:>&q= uot; <amy.f...@gmail.com=20
> > <javascript:>>:=20
> > =C2=A0
> > > On Thursday, 6 June 2019 09:46:02 UTC-4, Henning Schild = wrote: =C2=A0=20
> > > >=20
> > > > Hi,=20
> > > >=20
> > > > i just had a quick look at the implementation of th= e base-apt=20
> > > > signing for the first time. The interface is not id= eal and has=20
> > > > potential for the signing key and the checking key = not actually=20
> > > > belonging together.=20
> > > >=20
> > > > As far as i understand the code i read, Isar will s= tart signing=20
> > > > base-apt if BASE_REPO_KEY is set to anything. The p= rivate key
> > > > it will use to sign the repo is not specified at al= l, it will
> > > > be whatever gnupg defaults to, given its configurat= ion.=20
> > > >=20
> > > > I would suggest to switch from "SignWith yes&q= uot; to "SignWith=20
> > > > <keyid>", and derive the id from BASE_RE= PO_KEY.=20
> > > >=20
> > > > Further improvements would be to actually configure= gnupg
> > > > inside Isar and not rely on an outside configuratio= n. Relying
> > > > on the outside config means that all (multi)configs= will have
> > > > to use the same keypair. So we would add=20
> > > >=20
> > > > BASE_REPO_KEY_PRIVATE and ..._PASSPHRASE=20
> > > >=20
> > > > Now we would create a new gpg homedir next to where= we store=20
> > > > base-apt. We would import that one key there and po= tentially
> > > > unlock it with its passphrase. If we clean and rebu= ild we get a
> > > > working gpghome for sure.=20
> > > >=20
> > > > Henning=20
> > > > =C2=A0 =C2=A0=20
> > >=20
> > > Hi,=20
> > >=20
> > > Perhaps something like the following ...=20
> > >=20
> > > Of course, since BASE_REPO_KEY permits specifying=20
> > > multiple keys, this raises a question of which keyid? = =C2=A0=20
> >
> > Oh that is a nice hidden feature, indeed one can specify mult= iple
> > keys there. So that variable should be called BASE_REPO_KEYS
> > instead.=20
> >
> > And yes reprepro also supports multiple values. So i guess yo= ur
> > patch is correct and it would probably sign the repo with all= the
> > keys specified.=20
> >
> > Whether that is what we want is another question, and i am no= t sure=20
> > whether "yes" will also use all keys or just the de= fault one.=20
> > =C2=A0
> > > Amy=20
> > >=20
> > > From 5ceb4a2ef97bc7fa6c44cd9ce6f73f9a831773f3 Mon S= ep 17 00:00:00
> > > 2001 From: Amy Fong <Amy_...@mentor.com <ja= vascript:>>=20
> > > Date: Thu, 13 Jun 2019 12:52:06 -0400=20
> > > Subject: [PATCH] base-apt: Use BASE_REPO_KEY for signing= =20
> > >=20
> > > Extract keyid from BASE_REPO_KEY for signing=20
> > >=20
> > > Signed-off-by: Amy Fong <Amy_...@mentor.com &l= t;javascript:>>=20
> > > ---=20
> > > =C2=A0meta/recipes-devtools/base-apt/base-apt.bb | 9 ++++++++-=20
> > > =C2=A01 file changed, 8 insertions(+), 1 deletion(-)=20
> > >=20
> > > diff --git a/meta/recipes-devtools/base-apt/base-apt.bb=20
> > > b/meta/recipes-devtools/base-apt/base-apt.bb=20
> > > index 1c0b4c6..81245f7 100644=20
> > > --- a/meta/recipes-devtools/base-apt/base-apt.bb=20
> > > +++ b/meta/recipes-devtools/base-apt/base-apt.bb=20
> > > @@ -19,8 +19,15 @@ do_cache_config() {=20
> > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sed -e "s#{CODENA= ME}#"${BASE_DISTRO_CODENAME}"#g" \=20
> > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0${WORKDI= R}/distributions.in >=20
> > > ${CACHE_CONF_DIR}/distributions if [ "${BASE_R= EPO_KEY}" ] ; then=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0option=3D&quo= t;yes" =C2=A0=20
> >
> > maybe there is a better name for the variable?=20
> >
> > Henning=20
> > =C2=A0
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for key in ${= BASE_REPO_KEY}; do=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0keyid=3D$(wget -qO - $key | gpg --keyid-format
> > > 0xlong --with-colons - 2>/dev/null |grep "^pub:&= quot; |awk -F':'
> > > '{print $5;}')=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0if [ -n "$keyid" ]; then=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0option=3D"$keyid"=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0fi=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0done=20
> > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# To gen= erate Release.gpg=20
> > > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "Si= gnWith: yes" >>
> > > ${CACHE_CONF_DIR}/distributions=20
> > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "Si= gnWith: $option" >>=20
> > > ${CACHE_CONF_DIR}/distributions fi=20
> > > =C2=A0 =C2=A0 =C2=A0fi=20
> > > =C2=A0 =C2=A0=20
> > =C2=A0
>=20
> 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.


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





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 -> a= pt_sign)
> Author: Amy Fong <Amy_...@mentor.com>
> Date: =C2=A0 Thu Jun 13 12:52:06 2019 -0400
>=20
> =C2=A0 =C2=A0 base-apt: Use BASE_REPO_SIGN_KEY for signing
> =C2=A0 =C2=A0=20
> =C2=A0 =C2=A0 Extract keyid from BASE_REPO_SIGN_KEY for signing
> =C2=A0 =C2=A0=20
> =C2=A0 =C2=A0 Signed-off-by: Amy Fong <Amy_...@mentor.com>
>=20
> diff --git a/meta/recipes-devtools/base-apt/base-apt.bb=20
> 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() {
> =C2=A0 =C2=A0 =C2=A0if [ ! -e "${CACHE_CONF_DIR}/distrib= utions" ]; then
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sed -e "s#{CODENAME}#"= ${BASE_DISTRO_CODENAME}"#g" \
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0${WORKDIR}/distributions.in >
> ${CACHE_CONF_DIR}/distributions
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0if [ "${BASE_REPO_KEY}" ] ;= then
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if [ "${BASE_REPO_SIGN_KEY}"= ; ] ; then
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0option=3D"yes"= ;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0keyid=3D$(wget -qO - &q= uot;${BASE_REPO_SIGN_KEY}" | gpg=20

Using wget, but that is most likely a "file:///" URI. And whe= never 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;}')
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if [ -n "$keyid&qu= ot; ]; then
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0option=3D= "$keyid"
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fi
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# To generate Rele= ase.gpg
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "SignWith: ye= s" >> ${CACHE_CONF_DIR}/distributions
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "SignWith: $o= ption" >>
> ${CACHE_CONF_DIR}/distributions fi
> =C2=A0 =C2=A0 =C2=A0fi
> =C2=A0
>=20

------=_Part_437_479907406.1561655045275-- ------=_Part_436_492020416.1561655045275--