Re: [OE-core] [PATCH] lib/oe/gpg_sign.py: Avoid race when creating .sig files in detach_sign

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Thu, 2023-03-23 at 11:08 +0100, Tobias Hagelborn wrote:
> Move the signature file into place only after it is successfully signed.
> This to avoid race and corrupted .sig files in cases multiple onging
> builds write to a shared sstate-cache dir.
> 
> Signed-off-by: Tobias Hagelborn <tobiasha@axis.com>
> ---
>  meta/lib/oe/gpg_sign.py | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/meta/lib/oe/gpg_sign.py b/meta/lib/oe/gpg_sign.py
> index 613dab8561..868846cdc5 100644
> --- a/meta/lib/oe/gpg_sign.py
> +++ b/meta/lib/oe/gpg_sign.py
> @@ -5,11 +5,12 @@
>  #
>  
>  """Helper module for GPG signing"""
> -import os
>  
>  import bb
> -import subprocess
> +import os
>  import shlex
> +import subprocess
> +import tempfile
>  
>  class LocalSigner(object):
>      """Class for handling local (on the build host) signing"""
> @@ -73,8 +74,6 @@ class LocalSigner(object):
>              cmd += ['--homedir', self.gpg_path]
>          if armor:
>              cmd += ['--armor']
> -        if output_suffix:
> -            cmd += ['-o', input_file + "." + output_suffix]
>          if use_sha256:
>              cmd += ['--digest-algo', "SHA256"]
>  
> @@ -83,19 +82,25 @@ class LocalSigner(object):
>          if self.gpg_version > (2,1,):
>              cmd += ['--pinentry-mode', 'loopback']
>  
> -        cmd += [input_file]
> -
>          try:
>              if passphrase_file:
>                  with open(passphrase_file) as fobj:
>                      passphrase = fobj.readline();
>  
> -            job = subprocess.Popen(cmd, stdin=subprocess.PIPE, stderr=subprocess.PIPE)
> -            (_, stderr) = job.communicate(passphrase.encode("utf-8"))
> +            output_file = input_file + "." + (output_suffix or 'sig')

This doesn't match the behaviour of output_suffix as used above where
it defaults to None. This forces it to a default of "sig" instead?

If that intentional that should be in the commit message.


> +            with tempfile.TemporaryDirectory(dir=os.path.dirname(output_file)) as tmp_dir:
> +                tmp_file = os.path.join(tmp_dir, os.path.basename(output_file))
> +                cmd += ['-o', tmp_file]
> +
> +                cmd += [input_file]
> +
> +                job = subprocess.Popen(cmd, stdin=subprocess.PIPE, stderr=subprocess.PIPE)
> +                (_, stderr) = job.communicate(passphrase.encode("utf-8"))
>  
> -            if job.returncode:
> -                bb.fatal("GPG exited with code %d: %s" % (job.returncode, stderr.decode("utf-8")))
> +                if job.returncode:
> +                    bb.fatal("GPG exited with code %d: %s" % (job.returncode, stderr.decode("utf-8")))
>  
> +                os.rename(tmp_file, output_file)
>          except IOError as e:
>              bb.error("IO error (%s): %s" % (e.errno, e.strerror))
>              raise Exception("Failed to sign '%s'" % input_file)

Cheers,

Richard