From c29593b5ef6f1e39b4283d24f272dc49e947492c Mon Sep 17 00:00:00 2001 From: KiekerJan Date: Sun, 15 Jan 2023 14:10:04 +0100 Subject: [PATCH 01/18] explicitly enable fail2ban which didn't start (#2190) --- setup/system.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup/system.sh b/setup/system.sh index d0003f9c..216c2cd8 100755 --- a/setup/system.sh +++ b/setup/system.sh @@ -373,3 +373,5 @@ cp -f conf/fail2ban/filter.d/* /etc/fail2ban/filter.d/ # scripts will ensure the files exist and then fail2ban is given another # restart at the very end of setup. restart_service fail2ban + +systemctl enable fail2ban From 0fc5105da515f6453889d87ab4f0347cbb8b0c58 Mon Sep 17 00:00:00 2001 From: KiekerJan Date: Sun, 15 Jan 2023 14:20:08 +0100 Subject: [PATCH 02/18] Fixes to DNS lookups during status checks when there are timeouts, enforce timeouts better (#2191) * add dns query handling changes * replace exception pass with error message * simplify dns exception catching * Add not set case to blacklist lookup result handling --- management/dns_update.py | 24 +++++++++++++++++------- management/status_checks.py | 13 +++++++++++-- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index 2bfc104f..2fb2baf5 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -992,6 +992,7 @@ def set_custom_dns_record(qname, rtype, value, action, env): def get_secondary_dns(custom_dns, mode=None): resolver = dns.resolver.get_default_resolver() resolver.timeout = 10 + resolver.lifetime = 10 values = [] for qname, rtype, value in custom_dns: @@ -1009,10 +1010,17 @@ def get_secondary_dns(custom_dns, mode=None): # doesn't. if not hostname.startswith("xfr:"): if mode == "xfr": - response = dns.resolver.resolve(hostname+'.', "A", raise_on_no_answer=False) - values.extend(map(str, response)) - response = dns.resolver.resolve(hostname+'.', "AAAA", raise_on_no_answer=False) - values.extend(map(str, response)) + try: + response = resolver.resolve(hostname+'.', "A", raise_on_no_answer=False) + values.extend(map(str, response)) + except dns.exception.DNSException: + pass + + try: + response = resolver.resolve(hostname+'.', "AAAA", raise_on_no_answer=False) + values.extend(map(str, response)) + except dns.exception.DNSException: + pass continue values.append(hostname) @@ -1030,15 +1038,17 @@ def set_secondary_dns(hostnames, env): # Validate that all hostnames are valid and that all zone-xfer IP addresses are valid. resolver = dns.resolver.get_default_resolver() resolver.timeout = 5 + resolver.lifetime = 5 + for item in hostnames: if not item.startswith("xfr:"): # Resolve hostname. try: response = resolver.resolve(item, "A") - except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer): + except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer, dns.resolver.Timeout): try: response = resolver.resolve(item, "AAAA") - except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer): + except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer, dns.resolver.Timeout): raise ValueError("Could not resolve the IP address of %s." % item) else: # Validate IP address. @@ -1071,7 +1081,7 @@ def get_custom_dns_records(custom_dns, qname, rtype): def build_recommended_dns(env): ret = [] for (domain, zonefile, records) in build_zones(env): - # remove records that we don't dislay + # remove records that we don't display records = [r for r in records if r[3] is not False] # put Required at the top, then Recommended, then everythiing else diff --git a/management/status_checks.py b/management/status_checks.py index 0d555441..4b3713ba 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -308,6 +308,8 @@ def run_network_checks(env, output): output.print_ok("IP address is not blacklisted by zen.spamhaus.org.") elif zen == "[timeout]": output.print_warning("Connection to zen.spamhaus.org timed out. We could not determine whether your server's IP address is blacklisted. Please try again later.") + elif zen == "[Not Set]": + output.print_warning("Could not connect to zen.spamhaus.org. We could not determine whether your server's IP address is blacklisted. Please try again later.") else: output.print_error("""The IP address of this machine %s is listed in the Spamhaus Block List (code %s), which may prevent recipients from receiving your email. See http://www.spamhaus.org/query/ip/%s.""" @@ -541,7 +543,7 @@ def check_dns_zone(domain, env, output, dns_zonefiles): for ns in custom_secondary_ns: # We must first resolve the nameserver to an IP address so we can query it. ns_ips = query_dns(ns, "A") - if not ns_ips: + if not ns_ips or ns_ips in {'[Not Set]', '[timeout]'}: output.print_error("Secondary nameserver %s is not valid (it doesn't resolve to an IP address)." % ns) continue # Choose the first IP if nameserver returns multiple @@ -744,6 +746,8 @@ def check_mail_domain(domain, env, output): output.print_ok("Domain is not blacklisted by dbl.spamhaus.org.") elif dbl == "[timeout]": output.print_warning("Connection to dbl.spamhaus.org timed out. We could not determine whether the domain {} is blacklisted. Please try again later.".format(domain)) + elif dbl == "[Not Set]": + output.print_warning("Could not connect to dbl.spamhaus.org. We could not determine whether the domain {} is blacklisted. Please try again later.".format(domain)) else: output.print_error("""This domain is listed in the Spamhaus Domain Block List (code %s), which may prevent recipients from receiving your mail. @@ -788,12 +792,17 @@ def query_dns(qname, rtype, nxdomain='[Not Set]', at=None, as_list=False): # running bind server), or if the 'at' argument is specified, use that host # as the nameserver. resolver = dns.resolver.get_default_resolver() - if at: + + # Make sure at is not a string that cannot be used as a nameserver + if at and at not in {'[Not set]', '[timeout]'}: resolver = dns.resolver.Resolver() resolver.nameservers = [at] # Set a timeout so that a non-responsive server doesn't hold us back. resolver.timeout = 5 + # The number of seconds to spend trying to get an answer to the question. If the + # lifetime expires a dns.exception.Timeout exception will be raised. + resolver.lifetime = 5 # Do the query. try: From 15872487621a66e900d550ba095b8cdd12c5fcc0 Mon Sep 17 00:00:00 2001 From: KiekerJan Date: Sun, 15 Jan 2023 14:22:43 +0100 Subject: [PATCH 03/18] Disable Roundcube password plugin since it was corrupting the user database (#2198) --- setup/webmail.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup/webmail.sh b/setup/webmail.sh index 791bda57..90e97aed 100755 --- a/setup/webmail.sh +++ b/setup/webmail.sh @@ -134,7 +134,7 @@ cat > $RCM_CONFIG < ~256 bits for AES-256, see above -\$config['plugins'] = array('html5_notifier', 'archive', 'zipdownload', 'password', 'managesieve', 'jqueryui', 'persistent_login', 'carddav'); +\$config['plugins'] = array('html5_notifier', 'archive', 'zipdownload', 'managesieve', 'jqueryui', 'persistent_login', 'carddav'); \$config['skin'] = 'elastic'; \$config['login_autocomplete'] = 2; \$config['login_username_filter'] = 'email'; From 57047d96e95c7c1f71da3b80f78d487f1c0acb61 Mon Sep 17 00:00:00 2001 From: Hugh Secker-Walker Date: Sun, 15 Jan 2023 08:25:36 -0500 Subject: [PATCH 04/18] chore(setup): Update obsolete chown group syntax (#2202) Co-authored-by: Hugh Secker-Walker --- setup/mail-dovecot.sh | 4 ++-- setup/munin.sh | 4 ++-- setup/nextcloud.sh | 6 +++--- setup/start.sh | 2 +- setup/webmail.sh | 10 +++++----- tools/owncloud-restore.sh | 4 ++-- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/setup/mail-dovecot.sh b/setup/mail-dovecot.sh index a4bb563b..8d45a50b 100755 --- a/setup/mail-dovecot.sh +++ b/setup/mail-dovecot.sh @@ -202,13 +202,13 @@ chmod -R o-rwx /etc/dovecot # Ensure mailbox files have a directory that exists and are owned by the mail user. mkdir -p $STORAGE_ROOT/mail/mailboxes -chown -R mail.mail $STORAGE_ROOT/mail/mailboxes +chown -R mail:mail $STORAGE_ROOT/mail/mailboxes # Same for the sieve scripts. mkdir -p $STORAGE_ROOT/mail/sieve mkdir -p $STORAGE_ROOT/mail/sieve/global_before mkdir -p $STORAGE_ROOT/mail/sieve/global_after -chown -R mail.mail $STORAGE_ROOT/mail/sieve +chown -R mail:mail $STORAGE_ROOT/mail/sieve # Allow the IMAP/POP ports in the firewall. ufw_allow imaps diff --git a/setup/munin.sh b/setup/munin.sh index 6799cad6..90f93521 100755 --- a/setup/munin.sh +++ b/setup/munin.sh @@ -34,8 +34,8 @@ contact.admin.always_send warning critical EOF # The Debian installer touches these files and chowns them to www-data:adm for use with spawn-fcgi -chown munin. /var/log/munin/munin-cgi-html.log -chown munin. /var/log/munin/munin-cgi-graph.log +chown munin /var/log/munin/munin-cgi-html.log +chown munin /var/log/munin/munin-cgi-graph.log # ensure munin-node knows the name of this machine # and reduce logging level to warning diff --git a/setup/nextcloud.sh b/setup/nextcloud.sh index 13afc6b7..50d1130a 100755 --- a/setup/nextcloud.sh +++ b/setup/nextcloud.sh @@ -110,7 +110,7 @@ InstallNextcloud() { # Make sure permissions are correct or the upgrade step won't run. # $STORAGE_ROOT/owncloud may not yet exist, so use -f to suppress # that error. - chown -f -R www-data.www-data $STORAGE_ROOT/owncloud /usr/local/lib/owncloud || /bin/true + chown -f -R www-data:www-data $STORAGE_ROOT/owncloud /usr/local/lib/owncloud || /bin/true # If this isn't a new installation, immediately run the upgrade script. # Then check for success (0=ok and 3=no upgrade needed, both are success). @@ -259,7 +259,7 @@ EOF EOF # Set permissions - chown -R www-data.www-data $STORAGE_ROOT/owncloud /usr/local/lib/owncloud + chown -R www-data:www-data $STORAGE_ROOT/owncloud /usr/local/lib/owncloud # Execute Nextcloud's setup step, which creates the Nextcloud sqlite database. # It also wipes it if it exists. And it updates config.php with database @@ -311,7 +311,7 @@ var_export(\$CONFIG); echo ";"; ?> EOF -chown www-data.www-data $STORAGE_ROOT/owncloud/config.php +chown www-data:www-data $STORAGE_ROOT/owncloud/config.php # Enable/disable apps. Note that this must be done after the Nextcloud setup. # The firstrunwizard gave Josh all sorts of problems, so disabling that. diff --git a/setup/start.sh b/setup/start.sh index 0626ab01..960ec55d 100755 --- a/setup/start.sh +++ b/setup/start.sh @@ -85,7 +85,7 @@ f=$STORAGE_ROOT while [[ $f != / ]]; do chmod a+rx "$f"; f=$(dirname "$f"); done; if [ ! -f $STORAGE_ROOT/mailinabox.version ]; then setup/migrate.py --current > $STORAGE_ROOT/mailinabox.version - chown $STORAGE_USER.$STORAGE_USER $STORAGE_ROOT/mailinabox.version + chown $STORAGE_USER:$STORAGE_USER $STORAGE_ROOT/mailinabox.version fi # Save the global options in /etc/mailinabox.conf so that standalone diff --git a/setup/webmail.sh b/setup/webmail.sh index 90e97aed..274f7506 100755 --- a/setup/webmail.sh +++ b/setup/webmail.sh @@ -170,7 +170,7 @@ EOF # Create writable directories. mkdir -p /var/log/roundcubemail /var/tmp/roundcubemail $STORAGE_ROOT/mail/roundcube -chown -R www-data.www-data /var/log/roundcubemail /var/tmp/roundcubemail $STORAGE_ROOT/mail/roundcube +chown -R www-data:www-data /var/log/roundcubemail /var/tmp/roundcubemail $STORAGE_ROOT/mail/roundcube # Ensure the log file monitored by fail2ban exists, or else fail2ban can't start. sudo -u www-data touch /var/log/roundcubemail/errors.log @@ -194,14 +194,14 @@ usermod -a -G dovecot www-data # set permissions so that PHP can use users.sqlite # could use dovecot instead of www-data, but not sure it matters -chown root.www-data $STORAGE_ROOT/mail +chown root:www-data $STORAGE_ROOT/mail chmod 775 $STORAGE_ROOT/mail -chown root.www-data $STORAGE_ROOT/mail/users.sqlite +chown root:www-data $STORAGE_ROOT/mail/users.sqlite chmod 664 $STORAGE_ROOT/mail/users.sqlite # Fix Carddav permissions: -chown -f -R root.www-data ${RCM_PLUGIN_DIR}/carddav -# root.www-data need all permissions, others only read +chown -f -R root:www-data ${RCM_PLUGIN_DIR}/carddav +# root:www-data need all permissions, others only read chmod -R 774 ${RCM_PLUGIN_DIR}/carddav # Run Roundcube database migration script (database is created if it does not exist) diff --git a/tools/owncloud-restore.sh b/tools/owncloud-restore.sh index 108c8b77..cdeec4e9 100755 --- a/tools/owncloud-restore.sh +++ b/tools/owncloud-restore.sh @@ -40,8 +40,8 @@ cp "$1/owncloud.db" $STORAGE_ROOT/owncloud/ cp "$1/config.php" $STORAGE_ROOT/owncloud/ ln -sf $STORAGE_ROOT/owncloud/config.php /usr/local/lib/owncloud/config/config.php -chown -f -R www-data.www-data $STORAGE_ROOT/owncloud /usr/local/lib/owncloud -chown www-data.www-data $STORAGE_ROOT/owncloud/config.php +chown -f -R www-data:www-data $STORAGE_ROOT/owncloud /usr/local/lib/owncloud +chown www-data:www-data $STORAGE_ROOT/owncloud/config.php sudo -u www-data php$PHP_VER /usr/local/lib/owncloud/occ maintenance:mode --off From 820a39b8657215101c2fa658c8c7361ba97361f5 Mon Sep 17 00:00:00 2001 From: Hugh Secker-Walker Date: Sun, 15 Jan 2023 08:28:43 -0500 Subject: [PATCH 05/18] chore(python open): Refactor open and gzip.open to use context manager (#2203) Co-authored-by: Hugh Secker-Walker --- management/backup.py | 6 ++++-- management/cli.py | 3 ++- management/dns_update.py | 3 ++- management/mail_log.py | 3 ++- management/ssl_certificates.py | 3 ++- management/status_checks.py | 9 +++++--- management/utils.py | 7 ++++-- management/web_update.py | 24 ++++++++++++--------- tools/editconf.py | 3 ++- tools/parse-nginx-log-bootstrap-accesses.py | 10 +++------ tools/readable_bash.py | 18 +++++++++------- 11 files changed, 52 insertions(+), 37 deletions(-) diff --git a/management/backup.py b/management/backup.py index 8a82c4ad..ad0e267b 100755 --- a/management/backup.py +++ b/management/backup.py @@ -531,7 +531,8 @@ def get_backup_config(env, for_save=False, for_ui=False): # Merge in anything written to custom.yaml. try: - custom_config = rtyaml.load(open(os.path.join(backup_root, 'custom.yaml'))) + with open(os.path.join(backup_root, 'custom.yaml'), 'r') as f: + custom_config = rtyaml.load(f) if not isinstance(custom_config, dict): raise ValueError() # caught below config.update(custom_config) except: @@ -556,7 +557,8 @@ def get_backup_config(env, for_save=False, for_ui=False): config["target"] = "file://" + config["file_target_directory"] ssh_pub_key = os.path.join('/root', '.ssh', 'id_rsa_miab.pub') if os.path.exists(ssh_pub_key): - config["ssh_pub_key"] = open(ssh_pub_key, 'r').read() + with open(ssh_pub_key, 'r') as f: + config["ssh_pub_key"] = f.read() return config diff --git a/management/cli.py b/management/cli.py index 1b91b003..b32089c6 100755 --- a/management/cli.py +++ b/management/cli.py @@ -47,7 +47,8 @@ def read_password(): return first def setup_key_auth(mgmt_uri): - key = open('/var/lib/mailinabox/api.key').read().strip() + with open('/var/lib/mailinabox/api.key', 'r') as f: + key = f.read().strip() auth_handler = urllib.request.HTTPBasicAuthHandler() auth_handler.add_password( diff --git a/management/dns_update.py b/management/dns_update.py index 2fb2baf5..6eaff52f 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -815,7 +815,8 @@ def write_opendkim_tables(domains, env): def get_custom_dns_config(env, only_real_records=False): try: - custom_dns = rtyaml.load(open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml'))) + with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml'), 'r') as f: + custom_dns = rtyaml.load(f) if not isinstance(custom_dns, dict): raise ValueError() # caught below except: return [ ] diff --git a/management/mail_log.py b/management/mail_log.py index bdf757cc..1a963966 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -73,7 +73,8 @@ def scan_files(collector): continue elif fn[-3:] == '.gz': tmp_file = tempfile.NamedTemporaryFile() - shutil.copyfileobj(gzip.open(fn), tmp_file) + with gzip.open(fn, 'rb') as f: + shutil.copyfileobj(f, tmp_file) if VERBOSE: print("Processing file", fn, "...") diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index ab4f2dc8..19f0266a 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -535,7 +535,8 @@ def check_certificate(domain, ssl_certificate, ssl_private_key, warn_if_expiring # Second, check that the certificate matches the private key. if ssl_private_key is not None: try: - priv_key = load_pem(open(ssl_private_key, 'rb').read()) + with open(ssl_private_key, 'rb') as f: + priv_key = load_pem(f.read()) except ValueError as e: return ("The private key file %s is not a private key file: %s" % (ssl_private_key, str(e)), None) diff --git a/management/status_checks.py b/management/status_checks.py index 4b3713ba..033834dd 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -207,7 +207,8 @@ def check_ssh_password(env, output): # the configuration file. if not os.path.exists("/etc/ssh/sshd_config"): return - sshd = open("/etc/ssh/sshd_config").read() + with open("/etc/ssh/sshd_config", "r") as f: + sshd = f.read() if re.search("\nPasswordAuthentication\s+yes", sshd) \ or not re.search("\nPasswordAuthentication\s+no", sshd): output.print_error("""The SSH server on this machine permits password-based login. A more secure @@ -594,7 +595,8 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): # record that we suggest using is for the KSK (and that's how the DS records were generated). # We'll also give the nice name for the key algorithm. dnssec_keys = load_env_vars_from_file(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/%s.conf' % alg_name_map[ds_alg])) - dnsssec_pubkey = open(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/' + dnssec_keys['KSK'] + '.key')).read().split("\t")[3].split(" ")[3] + with open(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/' + dnssec_keys['KSK'] + '.key'), 'r') as f: + dnsssec_pubkey = f.read().split("\t")[3].split(" ")[3] expected_ds_records[ (ds_keytag, ds_alg, ds_digalg, ds_digest) ] = { "record": rr_ds, @@ -956,7 +958,8 @@ def run_and_output_changes(env, pool): # Load previously saved status checks. cache_fn = "/var/cache/mailinabox/status_checks.json" if os.path.exists(cache_fn): - prev = json.load(open(cache_fn)) + with open(cache_fn, 'r') as f: + prev = json.load(f) # Group the serial output into categories by the headings. def group_by_heading(lines): diff --git a/management/utils.py b/management/utils.py index fc04ed4d..b5ca7e59 100644 --- a/management/utils.py +++ b/management/utils.py @@ -14,7 +14,9 @@ def load_env_vars_from_file(fn): # Load settings from a KEY=VALUE file. import collections env = collections.OrderedDict() - for line in open(fn): env.setdefault(*line.strip().split("=", 1)) + with open(fn, 'r') as f: + for line in f: + env.setdefault(*line.strip().split("=", 1)) return env def save_environment(env): @@ -34,7 +36,8 @@ def load_settings(env): import rtyaml fn = os.path.join(env['STORAGE_ROOT'], 'settings.yaml') try: - config = rtyaml.load(open(fn, "r")) + with open(fn, "r") as f: + config = rtyaml.load(f) if not isinstance(config, dict): raise ValueError() # caught below return config except: diff --git a/management/web_update.py b/management/web_update.py index 7230182b..e23bb2d8 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -63,7 +63,8 @@ def get_web_domains_with_root_overrides(env): root_overrides = { } nginx_conf_custom_fn = os.path.join(env["STORAGE_ROOT"], "www/custom.yaml") if os.path.exists(nginx_conf_custom_fn): - custom_settings = rtyaml.load(open(nginx_conf_custom_fn)) + with open(nginx_conf_custom_fn, 'r') as f: + custom_settings = rtyaml.load(f) for domain, settings in custom_settings.items(): for type, value in [('redirect', settings.get('redirects', {}).get('/')), ('proxy', settings.get('proxies', {}).get('/'))]: @@ -75,13 +76,18 @@ def do_web_update(env): # Pre-load what SSL certificates we will use for each domain. ssl_certificates = get_ssl_certificates(env) + # Helper for reading config files and templates + def read_conf(conf_fn): + with open(os.path.join(os.path.dirname(__file__), "../conf", conf_fn), "r") as f: + return f.read() + # Build an nginx configuration file. - nginx_conf = open(os.path.join(os.path.dirname(__file__), "../conf/nginx-top.conf")).read() + nginx_conf = read_conf("nginx-top.conf") # Load the templates. - template0 = open(os.path.join(os.path.dirname(__file__), "../conf/nginx.conf")).read() - template1 = open(os.path.join(os.path.dirname(__file__), "../conf/nginx-alldomains.conf")).read() - template2 = open(os.path.join(os.path.dirname(__file__), "../conf/nginx-primaryonly.conf")).read() + template0 = read_conf("nginx.conf") + template1 = read_conf("nginx-alldomains.conf") + template2 = read_conf("nginx-primaryonly.conf") template3 = "\trewrite ^(.*) https://$REDIRECT_DOMAIN$1 permanent;\n" # Add the PRIMARY_HOST configuration first so it becomes nginx's default server. @@ -141,11 +147,8 @@ def make_domain_config(domain, templates, ssl_certificates, env): def hashfile(filepath): import hashlib sha1 = hashlib.sha1() - f = open(filepath, 'rb') - try: + with open(filepath, 'rb') as f: sha1.update(f.read()) - finally: - f.close() return sha1.hexdigest() nginx_conf_extra += "\t# ssl files sha1: %s / %s\n" % (hashfile(tls_cert["private-key"]), hashfile(tls_cert["certificate"])) @@ -153,7 +156,8 @@ def make_domain_config(domain, templates, ssl_certificates, env): hsts = "yes" nginx_conf_custom_fn = os.path.join(env["STORAGE_ROOT"], "www/custom.yaml") if os.path.exists(nginx_conf_custom_fn): - yaml = rtyaml.load(open(nginx_conf_custom_fn)) + with open(nginx_conf_custom_fn, 'r') as f: + yaml = rtyaml.load(f) if domain in yaml: yaml = yaml[domain] diff --git a/tools/editconf.py b/tools/editconf.py index dc184966..ec112b02 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -76,7 +76,8 @@ for setting in settings: found = set() buf = "" -input_lines = list(open(filename)) +with open(filename, "r") as f: + input_lines = list(f) while len(input_lines) > 0: line = input_lines.pop(0) diff --git a/tools/parse-nginx-log-bootstrap-accesses.py b/tools/parse-nginx-log-bootstrap-accesses.py index b08bc253..c61ed79e 100755 --- a/tools/parse-nginx-log-bootstrap-accesses.py +++ b/tools/parse-nginx-log-bootstrap-accesses.py @@ -17,13 +17,8 @@ accesses = set() # Scan the current and rotated access logs. for fn in glob.glob("/var/log/nginx/access.log*"): # Gunzip if necessary. - if fn.endswith(".gz"): - f = gzip.open(fn) - else: - f = open(fn, "rb") - # Loop through the lines in the access log. - with f: + with (gzip.open if fn.endswith(".gz") else open)(fn, "rb") as f: for line in f: # Find lines that are GETs on the bootstrap script by either curl or wget. # (Note that we purposely skip ...?ping=1 requests which is the admin panel querying us for updates.) @@ -43,7 +38,8 @@ for date, ip in accesses: # Since logs are rotated, store the statistics permanently in a JSON file. # Load in the stats from an existing file. if os.path.exists(outfn): - existing_data = json.load(open(outfn)) + with open(outfn, "r") as f: + existing_data = json.load(f) for date, count in existing_data: if date not in by_date: by_date[date] = count diff --git a/tools/readable_bash.py b/tools/readable_bash.py index 1fcdd5cd..3cb5908c 100644 --- a/tools/readable_bash.py +++ b/tools/readable_bash.py @@ -124,13 +124,14 @@ def generate_documentation(): """) parser = Source.parser() - for line in open("setup/start.sh"): - try: - fn = parser.parse_string(line).filename() - except: - continue - if fn in ("setup/start.sh", "setup/preflight.sh", "setup/questions.sh", "setup/firstuser.sh", "setup/management.sh"): - continue + with open("setup/start.sh", "r") as start_file: + for line in start_file: + try: + fn = parser.parse_string(line).filename() + except: + continue + if fn in ("setup/start.sh", "setup/preflight.sh", "setup/questions.sh", "setup/firstuser.sh", "setup/management.sh"): + continue import sys print(fn, file=sys.stderr) @@ -401,7 +402,8 @@ class BashScript(Grammar): @staticmethod def parse(fn): if fn in ("setup/functions.sh", "/etc/mailinabox.conf"): return "" - string = open(fn).read() + with open(fn, "r") as f: + string = f.read() # tokenize string = re.sub(".* #NODOC\n", "", string) From 02b34ce699d1fd7749a2f7c01b26058b436f7240 Mon Sep 17 00:00:00 2001 From: Hugh Secker-Walker Date: Sun, 15 Jan 2023 10:01:07 -0500 Subject: [PATCH 06/18] fix(backup-display): Fix parsing of rsync target in system-backup.html, fixes #2206 (#2207) --- management/templates/system-backup.html | 38 +++++++++++++++++++++---- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/management/templates/system-backup.html b/management/templates/system-backup.html index 5450b6e5..0ecc87bd 100644 --- a/management/templates/system-backup.html +++ b/management/templates/system-backup.html @@ -259,12 +259,11 @@ function show_custom_backup() { } else if (r.target == "off") { $("#backup-target-type").val("off"); } else if (r.target.substring(0, 8) == "rsync://") { - $("#backup-target-type").val("rsync"); - var path = r.target.substring(8).split('//'); - var host_parts = path.shift().split('@'); - $("#backup-target-rsync-user").val(host_parts[0]); - $("#backup-target-rsync-host").val(host_parts[1]); - $("#backup-target-rsync-path").val('/'+path[0]); + const spec = url_split(r.target); + $("#backup-target-type").val(spec.scheme); + $("#backup-target-rsync-user").val(spec.user); + $("#backup-target-rsync-host").val(spec.host); + $("#backup-target-rsync-path").val(spec.path); } else if (r.target.substring(0, 5) == "s3://") { $("#backup-target-type").val("s3"); var hostpath = r.target.substring(5).split('/'); @@ -344,4 +343,31 @@ function init_inputs(target_type) { set_host($('#backup-target-s3-host-select').val()); } } + +// Return a two-element array of the substring preceding and the substring following +// the first occurence of separator in string. Return [undefined, string] if the +// separator does not appear in string. +const split1_rest = (string, separator) => { + const index = string.indexOf(separator); + return (index >= 0) ? [string.substring(0, index), string.substring(index + separator.length)] : [undefined, string]; +}; + +// Note: The manifest JS URL class does not work in some security-conscious +// settings, e.g. Brave browser, so we roll our own that handles only what we need. +// +// Use greedy separator parsing to get parts of a MIAB backup target url. +// Note: path will not include a leading forward slash '/' +const url_split = url => { + const [ scheme, scheme_rest ] = split1_rest(url, '://'); + const [ user, user_rest ] = split1_rest(scheme_rest, '@'); + const [ host, path ] = split1_rest(user_rest, '/'); + + return { + scheme, + user, + host, + path, + } +}; + From a2565227f24ba5d12db2e62455bef68b62cbdcb4 Mon Sep 17 00:00:00 2001 From: Hugh Secker-Walker Date: Sun, 15 Jan 2023 10:03:05 -0500 Subject: [PATCH 07/18] feat(rsync-port): Add support for non-standard ssh port for rsync backup (#2208) --- management/backup.py | 25 +++++++++++++++++++++---- management/templates/system-backup.html | 4 ++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/management/backup.py b/management/backup.py index ad0e267b..3c4c38d7 100755 --- a/management/backup.py +++ b/management/backup.py @@ -213,9 +213,18 @@ def get_duplicity_additional_args(env): config = get_backup_config(env) if get_target_type(config) == 'rsync': + # Extract a port number for the ssh transport. Duplicity accepts the + # optional port number syntax in the target, but it doesn't appear to act + # on it, so we set the ssh port explicitly via the duplicity options. + from urllib.parse import urlsplit + try: + port = urlsplit(config["target"]).port + except ValueError: + port = 22 + return [ - "--ssh-options= -i /root/.ssh/id_rsa_miab", - "--rsync-options= -e \"/usr/bin/ssh -oStrictHostKeyChecking=no -oBatchMode=yes -p 22 -i /root/.ssh/id_rsa_miab\"", + f"--ssh-options= -i /root/.ssh/id_rsa_miab -p {port}", + f"--rsync-options= -e \"/usr/bin/ssh -oStrictHostKeyChecking=no -oBatchMode=yes -p {port} -i /root/.ssh/id_rsa_miab\"", ] elif get_target_type(config) == 's3': # See note about hostname in get_duplicity_target_url. @@ -408,6 +417,14 @@ def list_target_files(config): rsync_fn_size_re = re.compile(r'.* ([^ ]*) [^ ]* [^ ]* (.*)') rsync_target = '{host}:{path}' + # Strip off any trailing port specifier because it's not valid in rsync's + # DEST syntax. Explicitly set the port number for the ssh transport. + user_host, *_ = target.netloc.rsplit(':', 1) + try: + port = target.port + except ValueError: + port = 22 + target_path = target.path if not target_path.endswith('/'): target_path = target_path + '/' @@ -416,11 +433,11 @@ def list_target_files(config): rsync_command = [ 'rsync', '-e', - '/usr/bin/ssh -i /root/.ssh/id_rsa_miab -oStrictHostKeyChecking=no -oBatchMode=yes', + f'/usr/bin/ssh -i /root/.ssh/id_rsa_miab -oStrictHostKeyChecking=no -oBatchMode=yes -p {port}', '--list-only', '-r', rsync_target.format( - host=target.netloc, + host=user_host, path=target_path) ] diff --git a/management/templates/system-backup.html b/management/templates/system-backup.html index 0ecc87bd..ad534f41 100644 --- a/management/templates/system-backup.html +++ b/management/templates/system-backup.html @@ -45,6 +45,10 @@
+
+ The hostname at your rsync provider, e.g. da2327.rsync.net. Optionally includes a colon + and the provider's non-standard ssh port number, e.g. u215843.your-storagebox.de:23. +
From 7a79153afebca82a3abd35e3ca3185e57468b5e9 Mon Sep 17 00:00:00 2001 From: Steven Conaway Date: Sun, 15 Jan 2023 08:05:13 -0700 Subject: [PATCH 08/18] Remove old darkmode background color (#2218) Removing this old background color solves the problem of the bottom of short pages (like `/admin`'s login page) being white. The background was being set to black, which would be inverted, so it'd appear white. Since the `filter:` css has [~97% support](https://caniuse.com/?search=filter), I think that this change should be made. Tested on latest versions of Chrome (mac and iOS), Firefox, and Safari (mac and iOS). --- management/templates/index.html | 5 ----- 1 file changed, 5 deletions(-) diff --git a/management/templates/index.html b/management/templates/index.html index f9c87f2c..323789ca 100644 --- a/management/templates/index.html +++ b/management/templates/index.html @@ -72,11 +72,6 @@ html { filter: invert(100%) hue-rotate(180deg); } - - /* Set explicit background color (necessary for Firefox) */ - html { - background-color: #111; - } /* Override Boostrap theme here to give more contrast. The black turns to white by the filter. */ .form-control { From 20ec6c20800ef089d6f681a20b95dda6c1c61de6 Mon Sep 17 00:00:00 2001 From: jcm-shove-it Date: Sun, 15 Jan 2023 16:05:36 +0100 Subject: [PATCH 09/18] Updated security.md to reflect the support of ubuntu 22.04 (#2219) --- security.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security.md b/security.md index ac508c93..8782343e 100644 --- a/security.md +++ b/security.md @@ -1,7 +1,7 @@ Mail-in-a-Box Security Guide ============================ -Mail-in-a-Box turns a fresh Ubuntu 18.04 LTS 64-bit machine into a mail server appliance by installing and configuring various components. +Mail-in-a-Box turns a fresh Ubuntu 22.04 LTS 64-bit machine into a mail server appliance by installing and configuring various components. This page documents the security posture of Mail-in-a-Box. The term “box” is used below to mean a configured Mail-in-a-Box. From 26709a3c1dba9cadbd88636b002d95c2f6bd2b14 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Tue, 13 Dec 2022 08:03:39 -0500 Subject: [PATCH 10/18] Improve error messages in the management tools when external command-line tools are run --- management/utils.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/management/utils.py b/management/utils.py index b5ca7e59..f452c16d 100644 --- a/management/utils.py +++ b/management/utils.py @@ -122,13 +122,16 @@ def shell(method, cmd_args, env={}, capture_stderr=False, return_bytes=False, tr if method == "check_output" and input is not None: kwargs['input'] = input - if not trap: + try: ret = getattr(subprocess, method)(cmd_args, **kwargs) - else: - try: - ret = getattr(subprocess, method)(cmd_args, **kwargs) - code = 0 - except subprocess.CalledProcessError as e: + code = 0 + except subprocess.CalledProcessError as e: + if not trap: + # Reformat exception. + msg = "Command failed with exit code {}: {}".format(e.returncode, subprocess.list2cmdline(cmd_args)) + if e.output: msg += "\n\nOutput:\n" + e.output + raise Exception(msg) + else: ret = e.output code = e.returncode if not return_bytes and isinstance(ret, bytes): ret = ret.decode("utf8") From b3743a31e9309920e40e3780fdfb33a3657400f0 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Sun, 15 Jan 2023 08:15:31 -0500 Subject: [PATCH 11/18] Add a status checks check that fail2ban is running using fail2ban-client --- management/status_checks.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/management/status_checks.py b/management/status_checks.py index 033834dd..b31a9818 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -95,6 +95,12 @@ def run_services_checks(env, output, pool): fatal = fatal or fatal2 output2.playback(output) + # Check fail2ban. + code, ret = shell('check_output', ["fail2ban-client", "status"], capture_stderr=True, trap=True) + if code != 0: + output.print_error("fail2ban is not running.") + all_running = False + if all_running: output.print_ok("All system services are running.") From 61d1ea1ea7eba12b2b621895556ed5226c820735 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Sun, 15 Jan 2023 10:16:56 -0500 Subject: [PATCH 12/18] Changelog entries --- CHANGELOG.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4033df1..9f4ae42d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,27 @@ CHANGELOG ========= +In Development +-------------- + +System: + +* fail2ban didn't start after setup. + +Mail: + +* Disable Roundcube password plugin since it was corrupting the user database. + +Control panel: + +* Fix changing existing backup settings when the rsync type is used. +* Allow setting a custom port for rsync backups. +* Fixes to DNS lookups during status checks when there are timeouts, enforce timeouts better. +* A new check is added to ensure fail2ban is running. +* Fixed a color. +* Disable Roundcube password plugin since it was corrupting the user database +* Improve error messages in the management tools when external command-line tools are run. + Version 60.1 (October 30, 2022) ------------------------------- From 5e3e4a2161157237b24a5856f7282bcece2ccff4 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Sat, 21 Jan 2023 08:19:57 -0500 Subject: [PATCH 13/18] v61 --- CHANGELOG.md | 5 ++--- README.md | 2 +- setup/bootstrap.sh | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f4ae42d..11d86ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ CHANGELOG ========= -In Development --------------- +Version 61 (January 21, 2023) +----------------------------- System: @@ -19,7 +19,6 @@ Control panel: * Fixes to DNS lookups during status checks when there are timeouts, enforce timeouts better. * A new check is added to ensure fail2ban is running. * Fixed a color. -* Disable Roundcube password plugin since it was corrupting the user database * Improve error messages in the management tools when external command-line tools are run. Version 60.1 (October 30, 2022) diff --git a/README.md b/README.md index 42792025..1f423011 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Clone this repository and checkout the tag corresponding to the most recent rele $ git clone https://github.com/mail-in-a-box/mailinabox $ cd mailinabox - $ git checkout v60.1 + $ git checkout v61 Begin the installation. diff --git a/setup/bootstrap.sh b/setup/bootstrap.sh index 1c6d475a..4414d7bb 100644 --- a/setup/bootstrap.sh +++ b/setup/bootstrap.sh @@ -23,7 +23,7 @@ if [ -z "$TAG" ]; then if [ "$UBUNTU_VERSION" == "Ubuntu 22.04 LTS" ]; then # This machine is running Ubuntu 22.04, which is supported by # Mail-in-a-Box versions 60 and later. - TAG=v60.1 + TAG=v61 elif [ "$UBUNTU_VERSION" == "Ubuntu 18.04 LTS" ]; then # This machine is running Ubuntu 18.04, which is supported by # Mail-in-a-Box versions 0.40 through 5x. From 4408cb1fba463aa3fdaba518a74468430dd06895 Mon Sep 17 00:00:00 2001 From: Hugh Secker-Walker Date: Sat, 28 Jan 2023 11:04:46 -0500 Subject: [PATCH 14/18] fix(rsync-backup): Provide default port 22 for rsync usage in backup.py (#2226) Co-authored-by: Hugh Secker-Walker --- management/backup.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/management/backup.py b/management/backup.py index 3c4c38d7..99c3d60e 100755 --- a/management/backup.py +++ b/management/backup.py @@ -221,7 +221,9 @@ def get_duplicity_additional_args(env): port = urlsplit(config["target"]).port except ValueError: port = 22 - + if port is None: + port = 22 + return [ f"--ssh-options= -i /root/.ssh/id_rsa_miab -p {port}", f"--rsync-options= -e \"/usr/bin/ssh -oStrictHostKeyChecking=no -oBatchMode=yes -p {port} -i /root/.ssh/id_rsa_miab\"", @@ -424,6 +426,8 @@ def list_target_files(config): port = target.port except ValueError: port = 22 + if port is None: + port = 22 target_path = target.path if not target_path.endswith('/'): From 7af713592a67a712580d11ae5f76d40ce8927b89 Mon Sep 17 00:00:00 2001 From: Hugh Secker-Walker Date: Sat, 28 Jan 2023 11:11:17 -0500 Subject: [PATCH 15/18] feat(status page): Add summary of ok/error/warning counts (#2204) * feat(status page): Add summary of ok/error/warning counts * simplify a bit --------- Co-authored-by: Hugh Secker-Walker Co-authored-by: Joshua Tauberer --- management/templates/system-status.html | 38 ++++++++++++++++++++----- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/management/templates/system-status.html b/management/templates/system-status.html index dc9233a5..12c8aa0b 100644 --- a/management/templates/system-status.html +++ b/management/templates/system-status.html @@ -10,13 +10,13 @@ border-top: none; padding-top: 0; } -#system-checks .status-error td { +#system-checks .status-error td, .summary-error { color: #733; } -#system-checks .status-warning td { +#system-checks .status-warning td, .summary-warning { color: #770; } -#system-checks .status-ok td { +#system-checks .status-ok td, .summary-ok { color: #040; } #system-checks div.extra { @@ -52,6 +52,9 @@
+
+
+ @@ -64,6 +67,9 @@