From 9cd818a2bf30e4a808593b540bc68bee46f066b1 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Apr 03 2016 18:58:23 +0000 Subject: [PATCH 1/5] Some simplifications --- diff --git a/spectool b/spectool index e05c807..9772451 100755 --- a/spectool +++ b/spectool @@ -41,12 +41,8 @@ class CompletedProcess(object): 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,9 +203,9 @@ def parseopts(): 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') mode.add_argument('-h', '--help', action='help', help="display this help screen") @@ -333,7 +329,7 @@ def generate_asset_list(spec, opts, selected): 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) @@ -393,10 +389,9 @@ def main(): 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: + list_files(spec, opts, selected) + if opts.get_files: download_files(spec, opts, selected) From d2b0261bb95c6fbfa9b8cce5d9b649a32c549303 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Apr 03 2016 19:16:17 +0000 Subject: [PATCH 2/5] Add verification of signatures using gpgv2 I looked into using pygpgme, but it seems that would be much more complicated. In particular setting the keyring is non-obvious, and the results have to verified manually, etc. Just doesn't seem worth the trouble. --- diff --git a/spectool b/spectool index 9772451..11aa626 100755 --- a/spectool +++ b/spectool @@ -207,6 +207,8 @@ def parseopts(): help='lists the expanded sources/patches (default)') 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', + help='verify the signatures on files') mode.add_argument('-h', '--help', action='help', help="display this help screen") @@ -239,6 +241,9 @@ def parseopts(): 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('-D', '--debug', action='store_true', help="output debug info, don't clean up when done") @@ -341,6 +346,10 @@ def is_downloadable(url): """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 path_download_name(url): return url.split('/')[-1] @@ -369,6 +378,61 @@ def download_files(spec, opts, selected): 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: + error("not implemented") + + sigs = set() + files = set() + for typ, num, asset in generate_asset_list(spec, opts, selected): + if typ == 'Error': + raise IndexError(asset) + dest = os.path.join(dir, path_download_name(asset)) + if is_signature(dest): + sigs.add(dest) + else: + files.add(dest) + + for path in files: + verify_file(path, sigs, keyring) + def show_parsed_data(spec, opts): print("Parsed these tags:") print("-> Name: {}".format(spec.name)) @@ -389,11 +453,12 @@ def main(): show_parsed_data(spec, opts) selected = Selections(spec, opts) - if opts.list_files or not opts.get_files: + 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: + verify(spec, opts, selected) if __name__ == '__main__': main() From ad2378d2a153419057dc824139c4712fe7047a05 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Apr 03 2016 19:16:54 +0000 Subject: [PATCH 3/5] Implement keyring guessing --- diff --git a/spectool b/spectool index 11aa626..93d2843 100755 --- a/spectool +++ b/spectool @@ -350,6 +350,14 @@ 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] @@ -417,18 +425,37 @@ def verify(spec, opts, selected): error("No source item {} (for the keyring)") keyring = os.path.join(dir, path_download_name(spec.sources[num])) else: - error("not implemented") + 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) - dest = os.path.join(dir, path_download_name(asset)) - if is_signature(dest): - sigs.add(dest) + path = os.path.join(dir, path_download_name(asset)) + if is_signature(path): + sigs.add(path) else: - files.add(dest) + 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) From 03623857867f094b447250a3bcb348dab1727774 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Apr 03 2016 19:17:30 +0000 Subject: [PATCH 4/5] Remove comment which is now obsolete --- diff --git a/spectool b/spectool index 93d2843..d14782b 100755 --- a/spectool +++ b/spectool @@ -10,15 +10,6 @@ from subprocess import CalledProcessError, PIPE, Popen, TimeoutExpired 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' From b451374699b9e7026530bb35fa513cedfe012c66 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Apr 03 2016 19:33:44 +0000 Subject: [PATCH 5/5] Enable verification by default Verification is now performed by default when downloading files, and when requested explicitly with --verify. Can be disabled with --no-verify. Tested with youtube-dl.spec. --- diff --git a/spectool b/spectool index d14782b..1bc4d03 100755 --- a/spectool +++ b/spectool @@ -198,7 +198,7 @@ def parseopts(): help='lists the expanded sources/patches (default)') 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', + 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") @@ -234,6 +234,8 @@ def parseopts(): 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") @@ -475,7 +477,7 @@ def main(): list_files(spec, opts, selected) if opts.get_files: download_files(spec, opts, selected) - if opts.verify: + if opts.verify or (opts.get_files and opts.verify is not False): verify(spec, opts, selected) if __name__ == '__main__':