diff --git a/CHANGELOG.md b/CHANGELOG.md index c4033df1..dddbb546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,33 @@ CHANGELOG ========= +Version 61.1 (January 28, 2022) +------------------------------- + +* Fixed rsync backups not working with the default port. +* Reverted "Improve error messages in the management tools when external command-line tools are run." because of the possibility of user secrets being included in error messages. +* Fix for TLS certificate SHA fingerprint not being displayed during setup. + +Version 61 (January 21, 2023) +----------------------------- + +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. +* 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..435530ce 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.1 Begin the installation. diff --git a/management/backup.py b/management/backup.py index 8a82c4ad..99c3d60e 100755 --- a/management/backup.py +++ b/management/backup.py @@ -213,9 +213,20 @@ 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 + if port is None: + 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 +419,16 @@ 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 + if port is None: + port = 22 + target_path = target.path if not target_path.endswith('/'): target_path = target_path + '/' @@ -416,11 +437,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) ] @@ -531,7 +552,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 +578,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 old mode 100755 new mode 100644 index 5fb2d339..9e8786dd --- 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 f7ab680c..ce65f6ac 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.") @@ -207,7 +213,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 +601,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, @@ -966,7 +974,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/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 { diff --git a/management/templates/system-backup.html b/management/templates/system-backup.html index 5450b6e5..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. +
@@ -259,12 +263,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 +347,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, + } +}; + 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 @@