[OE-core] [PATCH 2/2] gpg_sign/selftest: Fix secmem parameter handling

[akuster808]

On 6/6/19 3:17 PM, Richard Purdie wrote:
> We keep seeing "cannot allocate memory" errors from rpm when signing packages
> on the autobuilder. The following were tried:
>
> * checking locked memory use (isn't hitting limits)
> * Restricting RPM_GPG_SIGN_CHUNK to 1
> * Limiting to 10 parallel do_package_write_rpm tasks
> * Allowing unlimied memory overcommit
> * Disabling rpm parallel compression

Thanks for fixing this.

I thought I broke the stable branches while testing backports.

- Armin
>
> and the test still failed. Further invetigation showed that the --auto-expand-secmem
> wasn't being passed to gpg-agent which meant the secmem couldn't be expanded hence the
> errors when there was pressure on the agent.
>
> The reason this happens is that some of the early gpg commands can start the agent
> without the option and it sticks around in memory so a version with the correct
> option may or may not get started.
>
> We therefore add the option to all the key gpg calls.
>
> Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> ---
>  meta/lib/oe/gpg_sign.py                 | 39 ++++++++++++++-----------
>  meta/lib/oeqa/selftest/cases/signing.py |  3 +-
>  2 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/meta/lib/oe/gpg_sign.py b/meta/lib/oe/gpg_sign.py
> index a95d2ba34c6..2fd8c3b1ac3 100644
> --- a/meta/lib/oe/gpg_sign.py
> +++ b/meta/lib/oe/gpg_sign.py
> @@ -15,21 +15,27 @@ class LocalSigner(object):
>      def __init__(self, d):
>          self.gpg_bin = d.getVar('GPG_BIN') or \
>                    bb.utils.which(os.getenv('PATH'), 'gpg')
> +        self.gpg_cmd = [self.gpg_bin]
> +        self.gpg_agent_bin = bb.utils.which(os.getenv('PATH'), "gpg-agent")
> +        # Without this we see "Cannot allocate memory" errors when running processes in parallel
> +        # It needs to be set for any gpg command since any agent launched can stick around in memory
> +        # and this parameter must be set.
> +        if self.gpg_agent_bin:
> +            self.gpg_cmd += ["--agent-program=%s|--auto-expand-secmem" % (self.gpg_agent_bin)]
>          self.gpg_path = d.getVar('GPG_PATH')
> -        self.gpg_version = self.get_gpg_version()
>          self.rpm_bin = bb.utils.which(os.getenv('PATH'), "rpmsign")
> -        self.gpg_agent_bin = bb.utils.which(os.getenv('PATH'), "gpg-agent")
> +        self.gpg_version = self.get_gpg_version()
> +
>  
>      def export_pubkey(self, output_file, keyid, armor=True):
>          """Export GPG public key to a file"""
> -        cmd = '%s --no-permission-warning --batch --yes --export -o %s ' % \
> -                (self.gpg_bin, output_file)
> +        cmd = self.gpg_cmd + ["--no-permission-warning", "--batch", "--yes", "--export", "-o", output_file]
>          if self.gpg_path:
> -            cmd += "--homedir %s " % self.gpg_path
> +            cmd += ["--homedir", self.gpg_path]
>          if armor:
> -            cmd += "--armor "
> -        cmd += keyid
> -        subprocess.check_output(shlex.split(cmd), stderr=subprocess.STDOUT)
> +            cmd += ["--armor"]
> +        cmd += [keyid]
> +        subprocess.check_output(cmd, stderr=subprocess.STDOUT)
>  
>      def sign_rpms(self, files, keyid, passphrase, digest, sign_chunk, fsk=None, fsk_password=None):
>          """Sign RPM files"""
> @@ -59,7 +65,7 @@ class LocalSigner(object):
>          if passphrase_file and passphrase:
>              raise Exception("You should use either passphrase_file of passphrase, not both")
>  
> -        cmd = [self.gpg_bin, '--detach-sign', '--no-permission-warning', '--batch',
> +        cmd = self.gpg_cmd + ['--detach-sign', '--no-permission-warning', '--batch',
>                 '--no-tty', '--yes', '--passphrase-fd', '0', '-u', keyid]
>  
>          if self.gpg_path:
> @@ -72,9 +78,6 @@ class LocalSigner(object):
>          if self.gpg_version > (2,1,):
>              cmd += ['--pinentry-mode', 'loopback']
>  
> -        if self.gpg_agent_bin:
> -            cmd += ["--agent-program=%s|--auto-expand-secmem" % (self.gpg_agent_bin)]
> -
>          cmd += [input_file]
>  
>          try:
> @@ -101,7 +104,8 @@ class LocalSigner(object):
>      def get_gpg_version(self):
>          """Return the gpg version as a tuple of ints"""
>          try:
> -            ver_str = subprocess.check_output((self.gpg_bin, "--version", "--no-permission-warning")).split()[2].decode("utf-8")
> +            cmd = self.gpg_cmd + ["--version", "--no-permission-warning"]
> +            ver_str = subprocess.check_output(cmd).split()[2].decode("utf-8")
>              return tuple([int(i) for i in ver_str.split("-")[0].split('.')])
>          except subprocess.CalledProcessError as e:
>              raise bb.build.FuncFailed("Could not get gpg version: %s" % e)
> @@ -109,11 +113,12 @@ class LocalSigner(object):
>  
>      def verify(self, sig_file):
>          """Verify signature"""
> -        cmd = self.gpg_bin + " --verify --no-permission-warning "
> +        cmd = self.gpg_cmd + [" --verify", "--no-permission-warning"]
>          if self.gpg_path:
> -            cmd += "--homedir %s " % self.gpg_path
> -        cmd += sig_file
> -        status = subprocess.call(shlex.split(cmd))
> +            cmd += ["--homedir", self.gpg_path]
> +
> +        cmd += [sig_file]
> +        status = subprocess.call(cmd)
>          ret = False if status else True
>          return ret
>  
> diff --git a/meta/lib/oeqa/selftest/cases/signing.py b/meta/lib/oeqa/selftest/cases/signing.py
> index 4ef2f70075e..404d68b899b 100644
> --- a/meta/lib/oeqa/selftest/cases/signing.py
> +++ b/meta/lib/oeqa/selftest/cases/signing.py
> @@ -30,7 +30,8 @@ class Signing(OESelftestTestCase):
>          self.secret_key_path = os.path.join(self.testlayer_path, 'files', 'signing', "key.secret")
>  
>          nsysroot = get_bb_var("RECIPE_SYSROOT_NATIVE", "gnupg-native")
> -        runCmd('gpg --batch --homedir %s --import %s %s' % (self.gpg_dir, self.pub_key_path, self.secret_key_path), native_sysroot=nsysroot)
> +
> +        runCmd('gpg --agent-program=`which gpg-agent`\|--auto-expand-secmem --batch --homedir %s --import %s %s' % (self.gpg_dir, self.pub_key_path, self.secret_key_path), native_sysroot=nsysroot)
>          return nsysroot + get_bb_var("bindir_native")
>  
>