Re: base-apt signing interface could be improved

[Henning Schild <henning.schild@siemens.com>]

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:> 
> >  
>