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

[vijai kumar <vijaikumar.kanagarajan@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3908 bytes --]



On Monday, September 30, 2019 at 2:12:34 PM UTC+5:30, vijai kumar wrote:
>
> On Mon, Sep 30, 2019 at 08:17:00AM +0200, Jan Kiszka wrote: 
> > On 27.09.19 23:11, vijaikumar.kanagarajan@gmail.com wrote: 
> > > From: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com> 
> > > 
> > > When using "SignWith: yes", reprepro uses the default gpg key 
> > > of the system to sign the repo. The default gpg key might be 
> > > different from what is specified in BASE_REPO_KEY, resulting 
> > > in using a wrong key for signing. 
> > > 
> > > Derive and use the keyid from the keyfile supplied instead of 
> > > a generic yes option. 
> > > 
> > > Suggested-by: Amy Fong <Amy_Fong@mentor.com> 
> > > Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com> 
> > > --- 
> > >   meta/recipes-devtools/base-apt/base-apt.bb | 22 
> +++++++++++++++++++--- 
> > >   1 file changed, 19 insertions(+), 3 deletions(-) 
> > > 
> > > diff --git a/meta/recipes-devtools/base-apt/base-apt.bb 
> b/meta/recipes-devtools/base-apt/base-apt.bb 
> > > index 74189f1..c74be86 100644 
> > > --- a/meta/recipes-devtools/base-apt/base-apt.bb 
> > > +++ b/meta/recipes-devtools/base-apt/base-apt.bb 
> > > @@ -4,6 +4,7 @@ 
> > >   SRC_URI = "file://distributions.in" 
> > >   BASE_REPO_KEY ?= "" 
> > > +KEYFILES ?= "" 
> > >   CACHE_CONF_DIR = "${REPO_BASE_DIR}/${BASE_DISTRO}/conf" 
> > >   do_cache_config[dirs] = "${CACHE_CONF_DIR}" 
> > > @@ -12,13 +13,18 @@ do_cache_config[lockfiles] = 
> "${REPO_BASE_DIR}/isar.lock" 
> > >   # Generate reprepro config for current distro if it doesn't exist. 
> Once it's 
> > >   # generated, this task should do nothing. 
> > > -do_cache_config() { 
> > > +repo_config() { 
> > >       if [ ! -e "${CACHE_CONF_DIR}/distributions" ]; then 
> > >           sed -e "s#{CODENAME}#"${BASE_DISTRO_CODENAME}"#g" \ 
> > >               ${WORKDIR}/distributions.in > 
> ${CACHE_CONF_DIR}/distributions 
> > > -        if [ "${BASE_REPO_KEY}" ] ; then 
> > > +        if [ -n "${KEYFILES}" ]; then 
> > > +            option="" 
> > > +            for key in ${KEYFILES}; do 
> > > +                keyid=$(cat ${key} | gpg --keyid-format 0xlong 
> --with-colons - 2>/dev/null |grep "^pub:" |awk -F':' '{print $5;}') 
> > 
> > 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 
>

[-- Attachment #1.2: Type: text/html, Size: 9649 bytes --]