Re: [PATCH 2/2] isar-bootstrap: Do not let gpg-agent to stay running

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

Am Mon, 6 Sep 2021 12:07:09 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 03.09.2021 20:30, Henning Schild wrote:
> > Venkata found that /tmp/gpghomeXXXXXXX to be part of the rootfs. It
> > not being deleted had the same reason the agent was never killed.  
> Even if incorrect check will not fail and gpg-agent will be killed,
> the second part of the original line
>  >chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent && /bin/rm -rf   
> "${MY_GPGHOME}"
> will be executed outside the chroot, so /tmp/gpghome* will not be 
> removed because of wrong path.
> Executing rm outside the chroot with corrected path (`rm -rf 
> "${ROOTFSDIR}${MY_GPGHOME}"`) is more efficient and will fix the
> issue.
> 
> >> +        if [ "${@get_distro_needs_gpg_support(d)}" = "gnupg" -a -d
> >> "${ROOTFSDIR}${MY_GPGHOME}" ]; then  
> > Why that new condition on get_distr_needs_gpg_support? I think this
> > can go away. In fact i think the whole if can go since the rm has a
> > -f.
> >
> > If the condition needs to stay ... i think
> >    
> >    -n "${@get_distro_needs_gpg_support(d)}"
> >
> > would be better than "= gnupg"  
> This one is needed because original `if [ -d
> "${ROOTFSDIR}${MY_GPGHOME}" ]` will be true in case there is no need
> in gpg-agent (MY_GPGHOME empty, but ROOTFSDIR exists). And that's why
> we also can't leave just `rm -rf "${ROOTFSDIR}${MY_GPGHOME}"` there.
> The check `= gnupg` was already used above, but yes I can rebuild 
> previous check also.

In fact we really need a [ -n MY_GPGHOME] before we "rm -rf". Otherwise
we could delete ${ROOTFSDIR}. Because otherwise we assume that when
get_distro_needs_gpg_support is true that variable is filled for sure.

So i would say.

chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent || true
if [ -n "${MY_GPGHOME}" ]; then
  rm -rf "${ROOTFSDIR}"
fi

Henning

> 
> > And in fact we can probably keep using daemon mode and pull that
> > kill out of the if as well
> >
> > chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent || true
> > rm -rf "${ROOTFSDIR}"
> >
> > Could be a little more efficient, but probably not worth too much
> > investigation. Just choose what seems more readable and
> > maintainable.  
> In the original code the number of chroot executions was 
> 2+[keys_number], the number of gpg-agent executions was 1. Now it is 
> just [keys_number] for both. I don't think there will be critical 
> performance drop.
> The first priority of this rebuild was it's stability. gpg-agent
> should not stays running even in case apt-key fails.
> It is also possible to move the loop code inside gpg-agent run
> command, but that will be much less readable.