#3 Verify downloaded files using gpgv2
Merged 7 years ago by tibbs. Opened 7 years ago by zbyszek.
zbyszek/spectool master  into  master

file modified
+103 -23
@@ -10,15 +10,6 @@ 

  from urllib import request

  

  # Python conversion of spectool.

- # Spectool has two functions:

- # Lists source and patche URLs with macros expanded (currently complete!)

- # Downloads sources and patches from the expanded URLs.

- # XXX Need to do this by shelling out to curl to remain compatible with spectool.

- # XXX Maybe only shell out as an option and handle the rest internally?

- # XXX Can just parse the curl config file for the one option anyone ever puts there.

- 

- # XXX Some enhancements from https://bugzilla.redhat.com/show_bug.cgi?id=1242988

- # XXX A rather complicated idea at https://bugzilla.redhat.com/show_bug.cgi?id=1093712

  

  VERSION = '2.0'

  CURLRC = '/etc/rpmdevrools/curlrc'
@@ -41,12 +32,8 @@ 

  class ProcError(CalledProcessError):

      """A CalledProcessError that also has stderr and stdout, like py3.5's."""

      def __init__(self, args, returncode, **kwargs):

-         self.stderr = None

-         self.stdout = None

-         if 'stderr' in kwargs:

-             self.stderr = kwargs['stderr']

-         if 'stdout' in kwargs:

-             self.stdout = kwargs['stdout']

+         self.stdout = kwargs.get('stdout', None)

+         self.stderr = kwargs.get('stderr', None)

          super().__init__(returncode, args, self.stderr)

  

      def __str__(self):
@@ -207,10 +194,12 @@ 

  

      mode = parser.add_argument_group('Operating mode')

      mode1 = mode.add_mutually_exclusive_group()

-     mode1.add_argument('-l', '--lf', '--list-files', action='store_true', dest='listfiles',

+     mode1.add_argument('-l', '--list-files', '--lf', action='store_true',

              help='lists the expanded sources/patches (default)')

-     mode1.add_argument('-g', '--gf', '--get-files', action='store_true', dest='getfiles',

+     mode1.add_argument('-g', '--get-files', '--gf', action='store_true',

              help='gets the sources/patches that are listed with a URL')

+     mode1.add_argument('--verify', action='store_true', default=None,

+             help='verify the signatures on files')

      mode.add_argument('-h', '--help', action='help',

              help="display this help screen")

  
@@ -243,6 +232,11 @@ 

      misc.add_argument('-f', '--force', action='store_true',

              help="try to unlink and download if target files exist")

  

+     misc.add_argument('--keyring',

+             help="path to file or Source number for the keyring with trusted key")

+     misc.add_argument('--no-verify', action='store_false', dest='verify',

+             help='skip signatures verification')

+ 

      misc.add_argument('-D', '--debug', action='store_true',

              help="output debug info, don't clean up when done")

  
@@ -333,7 +327,7 @@ 

          else:

              yield 'Error', patch, 'No patch item {}'.format(patch)

  

- def listfiles(spec, opts, selected):

+ def list_files(spec, opts, selected):

      for typ, num, asset in generate_asset_list(spec, opts, selected):

          if typ == 'Error':

              print(asset)
@@ -345,6 +339,18 @@ 

      """Check that string is a valid URL of a protocol which we can handle."""

      return url.split('://')[0] in {'http', 'https', 'ftp'}

  

+ def is_signature(url):

+     """Check that path looks like a signature."""

+     return url.endswith(".gpg") or url.endswith(".sig")

+ 

+ def is_keyring(url):

+     """Check that path looks like a keyring.

+ 

+     Sometimes .gpg extension is also used for keyrings, so if we don't

+     find a .kbx file, we should look for an extraneous .gpg file.

+     """

+     return url.endswith(".kbx")

+ 

  def path_download_name(url):

      return url.split('/')[-1]

  
@@ -373,6 +379,80 @@ 

                  if not opts.dryrun:

                      download_file(asset, dest)

  

+ def verify_signature(path, signature, keyring):

+     if not os.path.exists(path):

+         print('{} not downloaded yet, not checking'.format(path))

+         return

+ 

+     cmdline = ['gpgv2', '--quiet', '--keyring', keyring, signature, path]

+     try:

+         proc = run(cmdline)

+     except ProcError as e:

+         print(e.stdout)

+         error('Error: signature verification failed for {}!'.format(path), e)

+     print('{} has a good signature'.format(path))

+ 

+ def verify_file(path, signatures, keyring):

+     for ext in ('.sig', '.gpg'):

+         if path + ext in signatures:

+             return verify_signature(path, path + ext, keyring)

+     else:

+         print('No signature for {}'.format(path))

+ 

+ def verify(spec, opts, selected):

+     """

+     Verify signatures on files.

+     """

+     dir = get_download_location(spec, opts)

+ 

+     if opts.keyring:

+         try:

+             num = int(opts.keyring)

+         except ValueError:

+             keyring = opts.keyring

+             if '/' not in keyring:

+                 # gpgv2 will look in ~/.gnupg for the keyring if it not a path

+                 keyring = os.path.join('.', keyring)

+         else:

+             if num not in spec.sourcenums:

+                 error("No source item {} (for the keyring)")

+             keyring = os.path.join(dir, path_download_name(spec.sources[num]))

+     else:

+         keyring = None

+ 

+     sigs = set()

+     files = set()

+     keyrings = set()

+     for typ, num, asset in generate_asset_list(spec, opts, selected):

+         if typ == 'Error':

+             raise IndexError(asset)

+         path = os.path.join(dir, path_download_name(asset))

+         if is_signature(path):

+             sigs.add(path)

+         else:

+             files.add(path)

+ 

+         if is_keyring(path):

+             keyrings.add(path)

+ 

+     if keyring is None:

+         # try to guess

+         if len(keyrings) == 1:

+             keyring == keyrings.pop()

+         elif len(keyrings) >= 2:

+             error('Too many candidate keyrings!')

+         else:

+             # look in the signatures list

+             for sig in sigs:

+                 if sig[:-4] not in files:

+                     keyring = sig

+                     break

+             else:

+                 error('Please specify the keyring using --keyring option.')

+ 

+     for path in files:

+         verify_file(path, sigs, keyring)

+ 

  def show_parsed_data(spec, opts):

      print("Parsed these tags:")

      print("-> Name:    {}".format(spec.name))
@@ -393,12 +473,12 @@ 

          show_parsed_data(spec, opts)

  

      selected = Selections(spec, opts)

-     if not opts.getfiles or opts.listfiles:

-         listfiles(spec, opts, selected)

- 

-     if opts.getfiles:

+     if opts.list_files or not (opts.get_files or opts.verify):

+         list_files(spec, opts, selected)

+     if opts.get_files:

          download_files(spec, opts, selected)

- 

+     if opts.verify or (opts.get_files and opts.verify is not False):

+         verify(spec, opts, selected)

  

  if __name__ == '__main__':

      main()

no initial comment

This tries to DTRT by default: if signatures are listed in sources, it'll try to verify files after downloading or when explicitly requested. It makes an educated guess at which files are signatures and which is the keyring.

Probably various corner cases are not covered. It should be easy enough to add support for more signature conventions later.

$ ~/python/spectool/spectool -g
Source0: https://yt-dl.org/downloads/2016.03.06/youtube-dl-2016.03.06.tar.gz → ./youtube-dl-2016.03.06.tar.gz
Source1: ./youtube-dl-2016.03.06.tar.gz.sig already exists
Source2: gpgkey-7D33D762FD6C35130481347FDB4B54CBA4826A18.gpg cannot be downloaded
Source3: youtube-dl.conf cannot be downloaded
No signature for ./youtube-dl.conf
./youtube-dl-2016.03.06.tar.gz has a good signature

$ echo "blah" >> ./youtube-dl-2016.03.06.tar.gz
$ ~/python/spectool/spectool -g
Source0: ./youtube-dl-2016.03.06.tar.gz already exists
Source1: ./youtube-dl-2016.03.06.tar.gz.sig already exists
Source2: gpgkey-7D33D762FD6C35130481347FDB4B54CBA4826A18.gpg cannot be downloaded
Source3: youtube-dl.conf cannot be downloaded
gpgv: Signature made Sun 06 Mar 2016 04:11:54 AM EST using RSA key ID A4826A18
gpgv: BAD signature from "Philipp Hagemeister phihag@phihag.de"

Error: signature verification failed for ./youtube-dl-2016.03.06.tar.gz!
Return code 1

This makes sense to me, and I'll merge it shortly. Since you're doing far more work on this than I am at this point, I went ahead and added you to the repo proper.

If you could, write a bit of documentation for the heuristics you use for finding keys and signatures, because I would like to implement the same thing in Lua code for use in RPM macros (since there have been requests for that kind of thing).

checking signatures in a specfile macro, so it would be good if they directly in rpm macros

Pull-Request has been merged by tibbs

7 years ago

Thanks.

Maybe the docs should be inline in the code and converted into some html format using sphinx? I think this would be relatively little work with a high chance of staying up-to-date over time.

I sent an email reply via email but it doesn't appear to have made it.

I know essentially zero about generating docs from python source in the
manner you describe, but it seems like a good idea.

I sent an email reply via email but it doesn't appear to have made it.
Yeah, I didn't get anything afaict.

I know essentially zero about generating docs from python source in the
manner you describe, but it seems like a good idea.

I started adding sphinx docs, but it seems overkill in this case. I extended the --help output and README.rst. If we create a rpm file, help2man can be used to generate a decent man page.

I pushed the changes to the main repo.

BTW, I don't really understand this comment in is_keyring:

Sometimes .gpg extension is also used for keyrings, so if we don't
find a .kbx file, we should look for an extraneous .gpg file.

Is that just a TODO item? I understood .gpg files to be more common, and the the discussion around verifying upstream signatures that I've seen hasn't really mentioned .kbx files. Does it suffice to just check for both, or do you call gpg differently depending on what you found?

No, that describes the implementation. I was trying to be smart: if .kbx is found, then that is the keyring. Otherwise, look at the .gpg files, if one of them does not look like a signature for another file (based on name matching), that that is the keyring. Otherwise bail.

Later I learned that the kernel signes the uncompressed tarball. I'll implement that too, it should be fairly easy.

Hmm, OK, it just seems weird to see that comment there while the actual check for .gpg is in is_signature.

Also, in the examples I've seen, both the key and the signature are .asc files, which makes for additional fun. I guess I really need to see more examples.

If you find anything interesting, just paste the name of the package here.

Metadata