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 <hji@dyntopia.com>
This commit is contained in:
parent
2dadbcfdcb
commit
e5e006d933
@ -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()
|
||||
|
Loading…
Reference in New Issue
Block a user