From e5e006d933b3f45c9bcee6cd891ddc5dd3178816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 9 Oct 2019 04:49:01 +0200 Subject: [PATCH] Fix various issues with qubes-dom0-update - fix unescaped dot in package_regex - if any package fails verification, remove the whole directory, not only that single package - abort (and remove the whole directory) on any exception - don't include file name in the error message, if it failed verification This, among other things, fix handling symlinks and directories sent by potentially malicious UpdateVM. os.remove() can't remove non-empty directories, so it would fail. Fortunately metadata is created only after successful verification, so dnf/yum wouldn't touch packages that failed verification and also weren't removed. But make the code better handle such situations. Reported-by: Hans Jerry Illikainen --- dom0-updates/qubes-receive-updates | 48 ++++++++++++++---------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/dom0-updates/qubes-receive-updates b/dom0-updates/qubes-receive-updates index bd33549..b1722a4 100755 --- a/dom0-updates/qubes-receive-updates +++ b/dom0-updates/qubes-receive-updates @@ -31,13 +31,12 @@ updates_dir = "/var/lib/qubes/updates" updates_rpm_dir = updates_dir + "/rpm" updates_repodata_dir = updates_dir + "/repodata" updates_error_file = updates_dir + "/errors" -updates_error_file_handle = None comps_file = None if os.path.exists('/usr/share/qubes/Qubes-comps.xml'): comps_file = '/usr/share/qubes/Qubes-comps.xml' -package_regex = re.compile(r"^[A-Za-z0-9._+-]{1,128}.rpm$") +package_regex = re.compile(r"^[A-Za-z0-9._+-]{1,128}\.rpm$") # example valid outputs: # .....rpm: rsa sha1 (md5) pgp md5 OK # .....rpm: (sha1) dsa sha1 md5 gpg OK @@ -49,18 +48,15 @@ package_regex = re.compile(r"^[A-Za-z0-9._+-]{1,128}.rpm$") gpg_ok_regex = re.compile(r": [a-z0-9() ]* (pgp|gpg|signatures) [a-z0-9 ]*OK$") -def dom0updates_fatal(pkg, msg): - global updates_error_file_handle +def dom0updates_fatal(msg): print(msg, file=sys.stderr) - if updates_error_file_handle is None: - updates_error_file_handle = open(updates_error_file, "a") - updates_error_file_handle.write(msg + "\n") - os.remove(pkg) + with open(updates_error_file, "a") as updates_error_file_handle: + updates_error_file_handle.write(msg + "\n") + shutil.rmtree(updates_rpm_dir) + exit(1) def handle_dom0updates(updatevm): - global updates_error_file_handle - source = os.getenv("QREXEC_REMOTE_DOMAIN") if source != updatevm.name: print('Domain ' + str(source) + ' not allowed to send dom0 updates', @@ -79,14 +75,14 @@ def handle_dom0updates(updatevm): os.mkdir(updates_rpm_dir) os.chown(updates_rpm_dir, -1, qubes_gid) os.chmod(updates_rpm_dir, 0o0775) - subprocess.check_call(["/usr/libexec/qubes/qfile-dom0-unpacker", - str(os.getuid()), updates_rpm_dir]) - # Verify received files - for untrusted_f in os.listdir(updates_rpm_dir): - if not package_regex.match(untrusted_f): - dom0updates_fatal(updates_rpm_dir + '/' + untrusted_f, - 'Domain ' + source + ' sent unexpected file: ' + untrusted_f) - else: + try: + subprocess.check_call(["/usr/libexec/qubes/qfile-dom0-unpacker", + str(os.getuid()), updates_rpm_dir]) + # Verify received files + for untrusted_f in os.listdir(updates_rpm_dir): + if not package_regex.match(untrusted_f): + raise Exception( + 'Domain ' + source + ' sent unexpected file') f = untrusted_f assert '/' not in f assert '\0' not in f @@ -94,19 +90,19 @@ def handle_dom0updates(updatevm): full_path = updates_rpm_dir + "/" + f if os.path.islink(full_path) or not os.path.isfile(full_path): - dom0updates_fatal( - full_path, 'Domain ' + source + ' sent not regular file') + raise Exception( + 'Domain ' + source + ' sent not regular file') p = subprocess.Popen(["/bin/rpm", "-K", full_path], stdout=subprocess.PIPE) output = p.communicate()[0].decode('ascii') if p.returncode != 0: - dom0updates_fatal(full_path, + raise Exception( 'Error while verifing %s signature: %s' % (f, output)) if not gpg_ok_regex.search(output.strip()): - dom0updates_fatal(full_path, + raise Exception( 'Domain ' + source + ' sent not signed rpm: ' + f) - if updates_error_file_handle is not None: - updates_error_file_handle.close() + except Exception as e: + dom0updates_fatal(str(e)) # After updates received - create repo metadata createrepo_cmd = ["/usr/bin/createrepo_c"] if comps_file: @@ -130,4 +126,6 @@ def main(): exit(1) handle_dom0updates(updatevm) -main() + +if __name__ == '__main__': + main()