From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6741460933745311744 Date: Mon, 30 Sep 2019 01:51:31 -0700 (PDT) From: vijai kumar To: isar-users Message-Id: <3f6bf521-82f8-46e3-adfc-afb834e8411b@googlegroups.com> In-Reply-To: <20190930084227.GC10223@lightning> References: <20190927211112.29379-1-Vijaikumar_Kangarajan@mentor.com> <49311e01-52f4-0ae8-ac95-a297e1343a20@siemens.com> <20190930084227.GC10223@lightning> Subject: Re: [PATCH] base-apt: Use gpg keyid instead of yes MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_1098_335876222.1569833492054" X-Google-Token: EJOEx-wFcimSmN4Ctc40 X-Google-IP: 139.181.36.34 X-TUID: pvbEUuoAdNK5 ------=_Part_1098_335876222.1569833492054 Content-Type: multipart/alternative; boundary="----=_Part_1099_396059868.1569833492055" ------=_Part_1099_396059868.1569833492055 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On Monday, September 30, 2019 at 2:12:34 PM UTC+5:30, vijai kumar wrote: > > On Mon, Sep 30, 2019 at 08:17:00AM +0200, Jan Kiszka wrote: > > On 27.09.19 23:11, vijaikumar.kanagarajan@gmail.com wrote: > > > From: Vijai Kumar K > > > > > > When using "SignWith: yes", reprepro uses the default gpg key > > > of the system to sign the repo. The default gpg key might be > > > different from what is specified in BASE_REPO_KEY, resulting > > > in using a wrong key for signing. > > > > > > Derive and use the keyid from the keyfile supplied instead of > > > a generic yes option. > > > > > > Suggested-by: Amy Fong > > > Signed-off-by: Vijai Kumar K > > > --- > > > meta/recipes-devtools/base-apt/base-apt.bb | 22 > +++++++++++++++++++--- > > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > > > diff --git a/meta/recipes-devtools/base-apt/base-apt.bb > b/meta/recipes-devtools/base-apt/base-apt.bb > > > index 74189f1..c74be86 100644 > > > --- a/meta/recipes-devtools/base-apt/base-apt.bb > > > +++ b/meta/recipes-devtools/base-apt/base-apt.bb > > > @@ -4,6 +4,7 @@ > > > SRC_URI = "file://distributions.in" > > > BASE_REPO_KEY ?= "" > > > +KEYFILES ?= "" > > > CACHE_CONF_DIR = "${REPO_BASE_DIR}/${BASE_DISTRO}/conf" > > > do_cache_config[dirs] = "${CACHE_CONF_DIR}" > > > @@ -12,13 +13,18 @@ do_cache_config[lockfiles] = > "${REPO_BASE_DIR}/isar.lock" > > > # Generate reprepro config for current distro if it doesn't exist. > Once it's > > > # generated, this task should do nothing. > > > -do_cache_config() { > > > +repo_config() { > > > if [ ! -e "${CACHE_CONF_DIR}/distributions" ]; then > > > sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \ > > > ${WORKDIR}/distributions.in > > ${CACHE_CONF_DIR}/distributions > > > - if [ "${BASE_REPO_KEY}" ] ; then > > > + if [ -n "${KEYFILES}" ]; then > > > + option="" > > > + for key in ${KEYFILES}; do > > > + keyid=$(cat ${key} | gpg --keyid-format 0xlong > --with-colons - 2>/dev/null |grep "^pub:" |awk -F':' '{print $5;}') > > > > I hope this parsing is stable... > > Having used it for quite sometime I dont see an issue. It would be better > if we error out if the key is not present > in the system. Will add it in v2. > On a second thought that condition check is unnecessary at this point. It could just fail at signing when it is not able to find the key. So no V2. > > > > > > + option="${option}${keyid} " > > > + done > > > # To generate Release.gpg > > > - echo "SignWith: yes" >> ${CACHE_CONF_DIR}/distributions > > > + echo "SignWith: ${option}" >> > ${CACHE_CONF_DIR}/distributions > > > fi > > > fi > > > @@ -35,4 +41,14 @@ do_cache_config() { > > > fi > > > } > > > +python do_cache_config() { > > > + for key in d.getVar('BASE_REPO_KEY').split(): > > > + d.appendVar("SRC_URI", " %s" % key) > > > + fetcher = bb.fetch2.Fetch([key], d) > > > > I wonder if that magically addresses the case that changing key file > content > > should also trigger rebuilds. Similar to > > https://github.com/ilbers/isar/issues/60. > > Not sure about that. May be some testing would reveal it. > > > > > > + filename = fetcher.localpath(key) > > > + d.appendVar("KEYFILES", " %s" % filename) > > > + > > > + bb.build.exec_func('repo_config', d) > > > +} > > > + > > > addtask cache_config after do_build > > > > > > > Looks good - if the keyid extraction if actually robust. > > > > Thanks, > Vijai Kumar K > > > Jan > > > > -- > > Siemens AG, Corporate Technology, CT RDA IOT SES-DE > > Corporate Competence Center Embedded Linux > ------=_Part_1099_396059868.1569833492055 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable


On Monday, September 30, 2019 at 2:12:34 PM UTC+5:= 30, vijai kumar wrote:
On Mon, = Sep 30, 2019 at 08:17:00AM +0200, Jan Kiszka wrote:
> On 27.09.19 23:11, vijaiku= mar.kanagarajan@gmail.com wrote:
> > From: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> >=20
> > When using "SignWith: yes", reprepro uses the defau= lt gpg key
> > of the system to sign the repo. The default gpg key might be
> > different from what is specified in BASE_REPO_KEY, resulting
> > in using a wrong key for signing.
> >=20
> > Derive and use the keyid from the keyfile supplied instead of
> > a generic yes option.
> >=20
> > Suggested-by: Amy Fong <Amy_Fong@mentor.com>
> > Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> > ---
> > =C2=A0 meta/recipes-devtools/base-apt/base-apt.bb | 22 +++++++++++++++++++---
> > =C2=A0 1 file changed, 19 insertions(+), 3 deletions(-)
> >=20
> > diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
> > index 74189f1..c74be86 100644
> > --- a/meta/recipes-devtools/base-apt/base-apt.bb
> > +++ b/meta/recipes-devtools/base-apt/base-apt.bb
> > @@ -4,6 +4,7 @@
> > =C2=A0 SRC_URI =3D "file://distributions.in"
> > =C2=A0 BASE_REPO_KEY ?=3D ""
> > +KEYFILES ?=3D ""
> > =C2=A0 CACHE_CONF_DIR =3D "${REPO_BASE_DIR}/${BASE_= DISTRO}/conf"
> > =C2=A0 do_cache_config[dirs] =3D "${CACHE_CONF_DIR}"= ;
> > @@ -12,13 +13,18 @@ do_cache_config[lockfiles] =3D "${RE= PO_BASE_DIR}/isar.lock"
> > =C2=A0 # Generate reprepro config for current distro if it do= esn't exist. Once it's
> > =C2=A0 # generated, this task should do nothing.
> > -do_cache_config() {
> > +repo_config() {
> > =C2=A0 =C2=A0 =C2=A0 if [ ! -e "${CACHE_CONF_DIR}/d= istributions" ]; then
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sed -e "s#{CODENAME}#= "${BASE_DISTRO_CODENAME}"#g" \
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ${WORKDIR}/<= a href=3D"http://distributions.in" target=3D"_blank" rel=3D"nofollow" onmou= sedown=3D"this.href=3D'http://www.google.com/url?q\x3dhttp%3A%2F%2Fdist= ributions.in\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGd2cwX67UcRJVEC437oBd3= 4zLqcw';return true;" onclick=3D"this.href=3D'http://www.google.com= /url?q\x3dhttp%3A%2F%2Fdistributions.in\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dA= FQjCNGd2cwX67UcRJVEC437oBd34zLqcw';return true;">distributions.in &= gt; ${CACHE_CONF_DIR}/distributions
> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0if [ "${BASE_REPO_KEY}"= ; ] ; then
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if [ -n "${KEYFILES}" = ]; then
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0option=3D"&qu= ot;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for key in ${KEYFI= LES}; do
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0keyi= d=3D$(cat ${key} | gpg --keyid-format 0xlong --with-colons - 2>/dev/null= |grep "^pub:" |awk -F':' '{print $5;}')
>=20
> I hope this parsing is stable...

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

On a second thought that condition che= ck is unnecessary at this point. It could just fail at signing when it is n= ot able to find the key.
So no V2.
=C2=A0

>=20
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opti= on=3D"${option}${keyid} "
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0done
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # To generat= e Release.gpg
> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "SignWit= h: yes" >> ${CACHE_CONF_DIR}/distributions
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "SignWit= h: ${option}" >> ${CACHE_CONF_DIR}/distributions
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fi
> > =C2=A0 =C2=A0 =C2=A0 fi
> > @@ -35,4 +41,14 @@ do_cache_config() {
> > =C2=A0 =C2=A0 =C2=A0 fi
> > =C2=A0 }
> > +python do_cache_config() {
> > + =C2=A0 =C2=A0for key in d.getVar('BASE_REPO_KEY').<= wbr>split():
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0d.appendVar("SRC_URI",= " %s" % key)
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0fetcher =3D bb.fetch2.Fetch([key= ], d)
>=20
> I wonder if that magically addresses the case that changing key fi= le content
> should also trigger rebuilds. Similar to
> https://github.com= /ilbers/isar/issues/60.

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

>=20
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0filename =3D fetcher.localpath(k= ey)
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0d.appendVar("KEYFILES"= , " %s" % filename)
> > +
> > + =C2=A0 =C2=A0bb.build.exec_func('repo_config',= d)
> > +}
> > +
> > =C2=A0 addtask cache_config after do_build
> >=20
>=20
> Looks good - if the keyid extraction if actually robust.
>=20

Thanks,
Vijai Kumar K

> Jan
>=20
> --=20
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
------=_Part_1099_396059868.1569833492055-- ------=_Part_1098_335876222.1569833492054--