From 83d8dbca3edc65de072d64b6380fbd49b2f83e2b Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Thu, 18 Aug 2016 08:32:14 -0400 Subject: [PATCH 1/5] fail2ban won't start until the roundcube log file is created fixes #911 --- CHANGELOG.md | 5 +++++ setup/start.sh | 9 ++++++++- setup/system.sh | 5 +++++ setup/webmail.sh | 3 +++ tests/fail2ban.py | 19 ++++++++++--------- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebed0675..dca1d1e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +In Development +-------------- + +* fail2ban won't start if Roundcube had not yet been used - new installations probably do not have fail2ban running. + v0.19 (August 13, 2016) ----------------------- diff --git a/setup/start.sh b/setup/start.sh index 9d19a411..790afe18 100755 --- a/setup/start.sh +++ b/setup/start.sh @@ -111,15 +111,22 @@ source setup/zpush.sh source setup/management.sh source setup/munin.sh -# Ping the management daemon to write the DNS and nginx configuration files. +# Wait for the management daemon to start... until nc -z -w 4 127.0.0.1 10222 do echo Waiting for the Mail-in-a-Box management daemon to start... sleep 2 done + +# ...and then have it write the DNS and nginx configuration files and start those +# services. tools/dns_update tools/web_update +# Give fail2ban another restart. The log files may not all have been present when +# fail2ban was first configured, but they should exist now. +restart_service fail2ban + # If DNS is already working, try to provision TLS certficates from Let's Encrypt. # Suppress extra reasons why domains aren't getting a new certificate. management/ssl_certificates.py -q diff --git a/setup/system.sh b/setup/system.sh index d9f84fda..293ac68d 100755 --- a/setup/system.sh +++ b/setup/system.sh @@ -299,4 +299,9 @@ cat conf/fail2ban/jails.conf \ > /etc/fail2ban/jail.d/mailinabox.conf cp -f conf/fail2ban/filter.d/* /etc/fail2ban/filter.d/ +# On first installation, the log files that the jails look at don't all exist. +# e.g., The roundcube error log isn't normally created until someone logs into +# Roundcube for the first time. This causes fail2ban to fail to start. Later +# scripts will ensure the files exist and then fail2ban is given another +# restart at the very end of setup. restart_service fail2ban diff --git a/setup/webmail.sh b/setup/webmail.sh index 8307149b..cdbc18ae 100755 --- a/setup/webmail.sh +++ b/setup/webmail.sh @@ -133,6 +133,9 @@ EOF mkdir -p /var/log/roundcubemail /tmp/roundcubemail $STORAGE_ROOT/mail/roundcube chown -R www-data.www-data /var/log/roundcubemail /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 + # Password changing plugin settings # The config comes empty by default, so we need the settings # we're not planning to change in config.inc.dist... diff --git a/tests/fail2ban.py b/tests/fail2ban.py index 0f2f1e9f..fb74e706 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -1,20 +1,20 @@ # Test that a box's fail2ban setting are working # correctly by attempting a bunch of failed logins. -# Specify SSH login information the command line - -# we use that to reset fail2ban after each test, -# and we extract the hostname from that to open -# connections to. +# +# Specify a SSH login command (which we use to reset +# fail2ban after each test) and the hostname to +# try to log in to. ###################################################################### import sys, os, time, functools # parse command line -if len(sys.argv) < 2: - print("Usage: tests/fail2ban.py user@hostname") +if len(sys.argv) != 3: + print("Usage: tests/fail2ban.py \"ssh user@hostname\" hostname") sys.exit(1) -ssh_user, hostname = sys.argv[1].split("@", 1) +ssh_command, hostname = sys.argv[1:3] # define some test types @@ -85,7 +85,8 @@ def http_test(url, expected_status, postdata=None, qsargs=None, auth=None): auth=HTTPBasicAuth(*auth) if auth else None, data=postdata, headers={'User-Agent': 'Mail-in-a-Box fail2ban tester'}, - timeout=8) + timeout=8, + verify=False) # don't bother with HTTPS validation, it may not be configured yet except requests.exceptions.ConnectTimeout as e: raise IsBlocked() except requests.exceptions.ConnectionError as e: @@ -106,7 +107,7 @@ def restart_fail2ban_service(final=False): if not final: # Stop recidive jails during testing. command += " && sudo fail2ban-client stop recidive" - os.system("ssh %s@%s \"%s\"" % (ssh_user, hostname, command)) + os.system("%s \"%s\"" % (ssh_command, command)) def testfunc_runner(i, testfunc, *args): print(i+1, end=" ", flush=True) From 7c9f3e0b23506e322220ff586449aa58c420385f Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Thu, 18 Aug 2016 08:36:28 -0400 Subject: [PATCH 2/5] v0.19a --- CHANGELOG.md | 6 ++++-- README.md | 4 ++-- setup/bootstrap.sh | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dca1d1e1..f9d6a19c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,10 @@ CHANGELOG ========= -In Development --------------- +v0.19a (August 18, 2016) +------------------------ + +This update corrects a security issue in v0.19. * fail2ban won't start if Roundcube had not yet been used - new installations probably do not have fail2ban running. diff --git a/README.md b/README.md index 6f6937fa..2078af09 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ by me: $ curl -s https://keybase.io/joshdata/key.asc | gpg --import gpg: key C10BDD81: public key "Joshua Tauberer " imported - $ git verify-tag v0.19 + $ git verify-tag v0.19a gpg: Signature made ..... using RSA key ID C10BDD81 gpg: Good signature from "Joshua Tauberer " gpg: WARNING: This key is not certified with a trusted signature! @@ -72,7 +72,7 @@ and on my [personal homepage](https://razor.occams.info/). (Of course, if this r Checkout the tag corresponding to the most recent release: - $ git checkout v0.19 + $ git checkout v0.19a Begin the installation. diff --git a/setup/bootstrap.sh b/setup/bootstrap.sh index 47fbb6ec..26ca8e94 100644 --- a/setup/bootstrap.sh +++ b/setup/bootstrap.sh @@ -7,7 +7,7 @@ ######################################################### if [ -z "$TAG" ]; then - TAG=v0.19 + TAG=v0.19a fi # Are we running as root? From 35a360ef0b0d8de3d20327ef0e4b6459492efd07 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Fri, 19 Aug 2016 12:42:43 -0400 Subject: [PATCH 3/5] simplify how munin-cgi-graph is called to reduce the attack surface area Seems like if REQUEST_METHOD is set to GET, then we can drop two redundant ways the query string is given. munin-cgi-graph itself reads the environment variables only, but its calls to Perl's CGI::param will look at the command line if REQUEST_METHOD is not used, otherwise it uses environment variables like CGI used to work. Since this is all behind admin auth anyway, there isn't a public vulnerability. #914 was opened without comment which lead me to notice the redundancy and worry about a vulnerability, before I realized this is admin-only anyway. --- management/daemon.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/management/daemon.py b/management/daemon.py index 9bc6429b..3c712303 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -541,10 +541,9 @@ def munin_cgi(filename): headers based on parameters in the requesting URL. All output is written to stdout which munin_cgi splits into response headers and binary response data. - munin-cgi-graph reads environment variables as well as passed input to determine + munin-cgi-graph reads environment variables to determine what it should do. It expects a path to be in the env-var PATH_INFO, and a - querystring to be in the env-var QUERY_STRING as well as passed as input to the - command. + querystring to be in the env-var QUERY_STRING. munin-cgi-graph has several failure modes. Some write HTTP Status headers and others return nonzero exit codes. Situating munin_cgi between the user-agent and munin-cgi-graph enables keeping @@ -552,7 +551,7 @@ def munin_cgi(filename): support infrastructure like spawn-fcgi. """ - COMMAND = 'su - munin --preserve-environment --shell=/bin/bash -c /usr/lib/munin/cgi/munin-cgi-graph "%s"' + COMMAND = 'su - munin --preserve-environment --shell=/bin/bash -c /usr/lib/munin/cgi/munin-cgi-graph' # su changes user, we use the munin user here # --preserve-environment retains the environment, which is where Popen's `env` data is # --shell=/bin/bash ensures the shell used is bash @@ -564,12 +563,10 @@ def munin_cgi(filename): query_str = request.query_string.decode("utf-8", 'ignore') - env = {'PATH_INFO': '/%s/' % filename, 'QUERY_STRING': query_str} - cmd = COMMAND % query_str + env = {'PATH_INFO': '/%s/' % filename, 'REQUEST_METHOD': 'GET', 'QUERY_STRING': query_str} code, binout = utils.shell('check_output', - cmd.split(' ', 5), - # Using a maxsplit of 5 keeps the last 2 arguments together - input=query_str.encode('UTF-8'), + COMMAND.split(" ", 5), + # Using a maxsplit of 5 keeps the last arguments together env=env, return_bytes=True, trap=True) From a14b17794bd9a02e07f9aff2f6213a6e7d26fc2d Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Fri, 19 Aug 2016 12:42:43 -0400 Subject: [PATCH 4/5] simplify how munin-cgi-graph is called to reduce the attack surface area Seems like if REQUEST_METHOD is set to GET, then we can drop two redundant ways the query string is given. munin-cgi-graph itself reads the environment variables only, but its calls to Perl's CGI::param will look at the command line if REQUEST_METHOD is not used, otherwise it uses environment variables like CGI used to work. Since this is all behind admin auth anyway, there isn't a public vulnerability. #914 was opened without comment which lead me to notice the redundancy and worry about a vulnerability, before I realized this is admin-only anyway. The vulnerability was created by 6d6f3ea3919d81abfd1a6eddd8787c98b761e1bb. See #914. This is the v0.19b hotfix commit. --- CHANGELOG.md | 2 ++ management/daemon.py | 15 ++++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9d6a19c..9c81343d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ CHANGELOG ========= +A remote code execution vulnerability is corrected in how the munin system monitoring graphs are generated for the control panel. The vulnerability involves an administrative user visiting a carefully crafted URL. + v0.19a (August 18, 2016) ------------------------ diff --git a/management/daemon.py b/management/daemon.py index 9bc6429b..3c712303 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -541,10 +541,9 @@ def munin_cgi(filename): headers based on parameters in the requesting URL. All output is written to stdout which munin_cgi splits into response headers and binary response data. - munin-cgi-graph reads environment variables as well as passed input to determine + munin-cgi-graph reads environment variables to determine what it should do. It expects a path to be in the env-var PATH_INFO, and a - querystring to be in the env-var QUERY_STRING as well as passed as input to the - command. + querystring to be in the env-var QUERY_STRING. munin-cgi-graph has several failure modes. Some write HTTP Status headers and others return nonzero exit codes. Situating munin_cgi between the user-agent and munin-cgi-graph enables keeping @@ -552,7 +551,7 @@ def munin_cgi(filename): support infrastructure like spawn-fcgi. """ - COMMAND = 'su - munin --preserve-environment --shell=/bin/bash -c /usr/lib/munin/cgi/munin-cgi-graph "%s"' + COMMAND = 'su - munin --preserve-environment --shell=/bin/bash -c /usr/lib/munin/cgi/munin-cgi-graph' # su changes user, we use the munin user here # --preserve-environment retains the environment, which is where Popen's `env` data is # --shell=/bin/bash ensures the shell used is bash @@ -564,12 +563,10 @@ def munin_cgi(filename): query_str = request.query_string.decode("utf-8", 'ignore') - env = {'PATH_INFO': '/%s/' % filename, 'QUERY_STRING': query_str} - cmd = COMMAND % query_str + env = {'PATH_INFO': '/%s/' % filename, 'REQUEST_METHOD': 'GET', 'QUERY_STRING': query_str} code, binout = utils.shell('check_output', - cmd.split(' ', 5), - # Using a maxsplit of 5 keeps the last 2 arguments together - input=query_str.encode('UTF-8'), + COMMAND.split(" ", 5), + # Using a maxsplit of 5 keeps the last arguments together env=env, return_bytes=True, trap=True) From ba75ff7820c652d6ba935ff0619d055cccec3ae6 Mon Sep 17 00:00:00 2001 From: Joshua Tauberer Date: Sat, 20 Aug 2016 11:42:03 -0400 Subject: [PATCH 5/5] v0.19b --- CHANGELOG.md | 5 +++++ README.md | 4 ++-- setup/bootstrap.sh | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c81343d..129090e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +v0.19b (August 20, 2016) +------------------------ + +This update corrects a security issue introduced in v0.18. + A remote code execution vulnerability is corrected in how the munin system monitoring graphs are generated for the control panel. The vulnerability involves an administrative user visiting a carefully crafted URL. v0.19a (August 18, 2016) diff --git a/README.md b/README.md index 2078af09..fa29cb51 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ by me: $ curl -s https://keybase.io/joshdata/key.asc | gpg --import gpg: key C10BDD81: public key "Joshua Tauberer " imported - $ git verify-tag v0.19a + $ git verify-tag v0.19b gpg: Signature made ..... using RSA key ID C10BDD81 gpg: Good signature from "Joshua Tauberer " gpg: WARNING: This key is not certified with a trusted signature! @@ -72,7 +72,7 @@ and on my [personal homepage](https://razor.occams.info/). (Of course, if this r Checkout the tag corresponding to the most recent release: - $ git checkout v0.19a + $ git checkout v0.19b Begin the installation. diff --git a/setup/bootstrap.sh b/setup/bootstrap.sh index 26ca8e94..7d180bfe 100644 --- a/setup/bootstrap.sh +++ b/setup/bootstrap.sh @@ -7,7 +7,7 @@ ######################################################### if [ -z "$TAG" ]; then - TAG=v0.19a + TAG=v0.19b fi # Are we running as root?