From cecda9cec5643d7b89829d1663fcedea346eca23 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Mon, 9 Jun 2014 08:09:45 -0400 Subject: [PATCH] management: shell out external programs in a more secure way --- management/backup.py | 42 ++++++++++++++++++++++++++-------------- management/daemon.py | 20 ++++++++++++------- management/mailconfig.py | 11 ++++++----- management/utils.py | 12 +++++++++++- 4 files changed, 57 insertions(+), 28 deletions(-) diff --git a/management/backup.py b/management/backup.py index c63eb5c0..e1cd3cde 100755 --- a/management/backup.py +++ b/management/backup.py @@ -12,7 +12,7 @@ import os, os.path, subprocess -from utils import exclusive_process, load_environment +from utils import exclusive_process, load_environment, shell env = load_environment() @@ -24,31 +24,43 @@ rdiff_backup_dir = os.path.join(backup_dir, 'rdiff-history') os.makedirs(backup_dir, exist_ok=True) # Stop services. -subprocess.check_call(["service", "dovecot", "stop"]) -subprocess.check_call(["service", "postfix", "stop"]) +shell('check_call', ["/usr/sbin/service", "dovecot", "stop"]) +shell('check_call', ["/usr/sbin/service", "postfix", "stop"]) # Update the backup directory which stores increments. try: - subprocess.check_call([ - "rdiff-backup", + shell('check_call', [ + "/usr/bin/rdiff-backup", "--exclude", backup_dir, env["STORAGE_ROOT"], rdiff_backup_dir]) except subprocess.CalledProcessError: + # Trap the error so we restart services again. pass # Start services. -subprocess.check_call(["service", "dovecot", "start"]) -subprocess.check_call(["service", "postfix", "start"]) +shell('check_call', ["/usr/sbin/service", "dovecot", "start"]) +shell('check_call', ["/usr/sbin/service", "postfix", "start"]) -# Tar the rdiff-backup directory into a single file encrypted using the backup private key. -os.system( - "tar -zcC %s . | openssl enc -aes-256-cbc -a -salt -in /dev/stdin -out %s -pass file:%s" - % - ( rdiff_backup_dir, - os.path.join(backup_dir, "latest.tgz.enc"), - os.path.join(backup_dir, "secret_key.txt"), - )) +# Tar the rdiff-backup directory into a single file. +shell('check_call', [ + "/bin/tar", + "-zc", + "-f", os.path.join(backup_dir, "latest.tgz"), + "-C", rdiff_backup_dir, + "."]) + +# Encrypt the backup using the backup private key. +shell('check_call', [ + "/usr/bin/openssl", + "enc", + "-aes-256-cbc", + "-a", + "-salt", + "-in", os.path.join(backup_dir, "latest.tgz"), + "-out", os.path.join(backup_dir, "latest.tgz.enc"), + "-pass", "file:%s" % os.path.join(backup_dir, "secret_key.txt"), + ]) # The backup can be decrypted with: # openssl enc -d -aes-256-cbc -a -in latest.tgz.enc -out /dev/stdout -pass file:secret_key.txt | tar -z diff --git a/management/daemon.py b/management/daemon.py index 0e79ac33..d0e64a9d 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -1,6 +1,6 @@ #!/usr/bin/python3 -import os, os.path, subprocess +import os, os.path, re from flask import Flask, request, render_template app = Flask(__name__) @@ -59,15 +59,21 @@ def dns_update(): @app.route('/system/updates') def show_updates(): - subprocess.check_call("apt-get -qq update", shell=True) - return subprocess.check_output( - r"""apt-get -qq -s upgrade | grep -v ^Conf | sed "s/^Inst /Updated Package Available: /" | sed "s/\[\(.*\)\] (\(\S*\).*/\(\1 => \2\)/" """, - shell=True) + utils.shell("check_call", ["/usr/bin/apt-get", "-qq", "update"]) + simulated_install = utils.shell("check_output", ["/usr/bin/apt-get", "-qq", "-s", "upgrade"]) + pkgs = [] + for line in simulated_install.split('\n'): + if re.match(r'^Conf .*', line): continue # remove these lines, not informative + line = re.sub(r'^Inst (.*) \[(.*)\] \((\S*).*', r'Updated Package Available: \1 (\3)', line) # make these lines prettier + pkgs.append(line) + return "\n".join(pkgs) @app.route('/system/update-packages', methods=["POST"]) def do_updates(): - subprocess.check_call("apt-get -qq update", shell=True) - return subprocess.check_output("DEBIAN_FRONTEND=noninteractive apt-get -y upgrade", shell=True) + utils.shell("check_call", ["/usr/bin/apt-get", "-qq", "update"]) + return utils.shell("check_output", ["/usr/bin/apt-get", "-y", "upgrade"], env={ + "DEBIAN_FRONTEND": "noninteractive" + }) # APP diff --git a/management/mailconfig.py b/management/mailconfig.py index f56142a2..0974915c 100644 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -1,4 +1,5 @@ import subprocess, shutil, os, sqlite3, re +import utils def validate_email(email, strict): # There are a lot of characters permitted in email addresses, but @@ -52,7 +53,7 @@ def add_mail_user(email, pw, env): conn, c = open_database(env, with_connection=True) # hash the password - pw = subprocess.check_output(["/usr/bin/doveadm", "pw", "-s", "SHA512-CRYPT", "-p", pw]).strip() + pw = utils.shell('check_output', ["/usr/bin/doveadm", "pw", "-s", "SHA512-CRYPT", "-p", pw]).strip() # add the user to the database try: @@ -68,14 +69,14 @@ def add_mail_user(email, pw, env): # Check if the mailboxes exist before creating them. When creating a user that had previously # been deleted, the mailboxes will still exist because they are still on disk. try: - existing_mboxes = subprocess.check_output(["doveadm", "mailbox", "list", "-u", email, "-8"], stderr=subprocess.STDOUT).decode("utf8").split("\n") + existing_mboxes = utils.shell('check_output', ["doveadm", "mailbox", "list", "-u", email, "-8"], capture_stderr=True).split("\n") except subprocess.CalledProcessError as e: c.execute("DELETE FROM users WHERE email=?", (email,)) conn.commit() return ("Failed to initialize the user: " + e.output.decode("utf8"), 400) - if "INBOX" not in existing_mboxes: subprocess.check_call(["doveadm", "mailbox", "create", "-u", email, "-s", "INBOX"]) - if "Spam" not in existing_mboxes: subprocess.check_call(["doveadm", "mailbox", "create", "-u", email, "-s", "Spam"]) + if "INBOX" not in existing_mboxes: utils.shell('check_call', ["doveadm", "mailbox", "create", "-u", email, "-s", "INBOX"]) + if "Spam" not in existing_mboxes: utils.shell('check_call', ["doveadm", "mailbox", "create", "-u", email, "-s", "Spam"]) # Create the user's sieve script to move spam into the Spam folder, and make it owned by mail. maildirstat = os.stat(env["STORAGE_ROOT"] + "/mail/mailboxes") @@ -93,7 +94,7 @@ def add_mail_user(email, pw, env): def set_mail_password(email, pw, env): # hash the password - pw = subprocess.check_output(["/usr/bin/doveadm", "pw", "-s", "SHA512-CRYPT", "-p", pw]).strip() + pw = utils.shell('check_output', ["/usr/bin/doveadm", "pw", "-s", "SHA512-CRYPT", "-p", pw]).strip() # update the database conn, c = open_database(env, with_connection=True) diff --git a/management/utils.py b/management/utils.py index f2ef19b5..657f7b9a 100644 --- a/management/utils.py +++ b/management/utils.py @@ -74,4 +74,14 @@ def is_pid_valid(pid): else: # EINVAL raise else: - return True \ No newline at end of file + return True + +def shell(method, cmd_args, env={}, capture_stderr=False): + # A safe way to execute processes. + # Some processes like apt-get require being given a sane PATH. + import subprocess + env.update({ "PATH": "/sbin:/bin:/usr/sbin:/usr/bin" }) + stderr = None if not capture_stderr else subprocess.STDOUT + ret = getattr(subprocess, method)(cmd_args, env=env, stderr=stderr) + if isinstance(ret, bytes): ret = ret.decode("utf8") + return ret