From f40361971cb1261d5da57bf46afc0a2030f61ef7 Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Sat, 5 May 2018 16:13:16 +0200 Subject: [PATCH 1/4] ossec: resolve journalist notification racing with reboots The app server is rebooted every 24h and will send a notification at boot time. The ossec server is also rebooted and will immediately send the email to the journalist, regardless of when the previous mail was sent (mail frequency is not a feature of ossec-maild). Always running the localfile command at boot time is an undocumented OSSEC behavior https://github.com/ossec/ossec-hids/issues/1415 in 2.8.2 as well as 2.9.3. This guarantees exactly one mail will be sent daily. Setting the 25 hours frequency element is a safeguard: * against the following race a) command runs because the 24h period expires, b) the server reboots shortly after because it reboots every 24h, c) command runs again after the server is rebooted, causing two notifications to be sent in a row * in case the server does not reboot for some reason, the notification will still be sent every 25h Fixes: https://github.com/freedomofpress/securedrop/issues/3367 (cherry picked from commit 91552ebc7fb0f3336f624f84cb89887812e28ea8) --- docs/install.rst | 3 +++ install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/install.rst b/docs/install.rst index 9ee736701d..636da72c97 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -75,6 +75,9 @@ worth checking the *Journalist Interface*. For this you will need: the GPG private key, it is not possible to specify multiple GPG keys. +.. note:: The journalist notification is sent after the daily reboot + of the *Application Server*. + You will have to copy the following required files to ``install_files/ansible-base``: diff --git a/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf b/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf index a5061e48f8..25bac9df98 100644 --- a/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf +++ b/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf @@ -108,7 +108,7 @@ full_command head -1 /var/lib/securedrop/submissions_today.txt | grep '^[0-9]*$' - 86400 + 90000 From ec1fdcd32dcb4837970f7f5ef50e89ed937a4b2d Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Fri, 4 May 2018 23:59:14 +0200 Subject: [PATCH 2/4] ossec: never send more than one journalist notification per 24h Relates to: https://github.com/freedomofpress/securedrop/issues/3368 (cherry picked from commit 3df640962ec4fca4ac49c28b0080639ba498f309) --- .../ossec/files/process_submissions_today.sh | 46 +++++++++++++++++-- testinfra/ossec/test_journalist_mail.py | 29 +++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh b/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh index 899d83df86..936c83e879 100755 --- a/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh +++ b/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh @@ -1,10 +1,28 @@ #!/bin/bash +SENT_STAMP=/var/ossec/logs/journalist_notification_sent.stamp + function send_encrypted_alarm() { /var/ossec/send_encrypted_alarm.sh "$1" } function main() { + if modified_in_the_past_24h "${SENT_STAMP}" ; then + logger "$0 journalist notification suppressed" + else + handle_notification "$@" + fi +} + +function modified_in_the_past_24h() { + local stamp + stamp="$1" + test -f "${stamp}" && \ + find "${stamp}" -mtime -1 | \ + grep --quiet "${stamp}" +} + +function handle_notification() { local sender local stdin sender=${1:-send_encrypted_alarm} @@ -24,6 +42,7 @@ function main() { echo "There has been no submission activity in the past 24 hours." echo "You do not need to login to SecureDrop." fi | $sender journalist + touch "${SENT_STAMP}" else export SUBJECT="SecureDrop Submissions Error" ( @@ -34,33 +53,50 @@ function main() { fi } +function forget() { + rm -f "${1:-$SENT_STAMP}" +} + +function test_modified_in_the_past_24h() { + local stamp + stamp=$(mktemp) + + modified_in_the_past_24h "${stamp}" || exit 1 + + touch --date '-2 days' "${stamp}" + ! modified_in_the_past_24h "${stamp}" || exit 1 + + forget "${stamp}" + ! modified_in_the_past_24h "${stamp}" || exit 1 +} + function test_send_encrypted_alarm() { echo "$1" cat } -function test_main() { +function test_handle_notification() { shopt -s -o xtrace PS4='${BASH_SOURCE[0]}:$LINENO: ${FUNCNAME[0]}: ' - echo BUGOUS | main test_send_encrypted_alarm | \ + echo BUGOUS | handle_notification test_send_encrypted_alarm | \ tee /dev/stderr | \ grep -q 'failed to find 0/1 submissions boolean' || exit 1 ( echo 'ossec: output' echo 'NOTANUMBER' - ) | main test_send_encrypted_alarm | tee /dev/stderr | grep -q 'failed to find 0/1 submissions boolean' || exit 1 + ) | handle_notification test_send_encrypted_alarm | tee /dev/stderr | grep -q 'failed to find 0/1 submissions boolean' || exit 1 ( echo 'ossec: output' echo '1' - ) | main test_send_encrypted_alarm | tee /tmp/submission-yes.txt | grep -q 'There has been submission activity' || exit 1 + ) | handle_notification test_send_encrypted_alarm | tee /tmp/submission-yes.txt | grep -q 'There has been submission activity' || exit 1 ( echo 'ossec: output' echo '0' - ) | main test_send_encrypted_alarm | tee /tmp/submission-no.txt | grep -q 'There has been no submission activity' || exit 1 + ) | handle_notification test_send_encrypted_alarm | tee /tmp/submission-no.txt | grep -q 'There has been no submission activity' || exit 1 if test "$(stat --format=%s /tmp/submission-no.txt)" != "$(stat --format=%s /tmp/submission-yes.txt)" ; then echo both files are expected to have exactly the same size, padding must be missing diff --git a/testinfra/ossec/test_journalist_mail.py b/testinfra/ossec/test_journalist_mail.py index 69dd319906..b98f169371 100644 --- a/testinfra/ossec/test_journalist_mail.py +++ b/testinfra/ossec/test_journalist_mail.py @@ -70,6 +70,8 @@ def test_procmail(self, host): for (destination, payload) in ( ('journalist', today_payload), ('ossec', 'MYGREATPAYLOAD')): + assert self.run(host, + "/var/ossec/process_submissions_today.sh forget") assert self.run(host, "postsuper -d ALL") assert self.run( host, @@ -82,7 +84,12 @@ def test_procmail(self, host): self.service_stopped(host, "postfix") def test_process_submissions_today(self, host): - self.run(host, "/var/ossec/process_submissions_today.sh test_main") + assert self.run(host, + "/var/ossec/process_submissions_today.sh " + "test_handle_notification") + assert self.run(host, + "/var/ossec/process_submissions_today.sh " + "test_modified_in_the_past_24h") def test_send_encrypted_alert(self, host): self.service_started(host, "postfix") @@ -185,10 +192,15 @@ def test_journalist_mail_notification(self, host): # empty the mailq on mon in case there were leftovers # assert self.run(mon, "postsuper -d ALL") + # + # forget about past notifications in case there were leftovers + # + assert self.run(mon, "/var/ossec/process_submissions_today.sh forget") # # the command fires every time ossec starts, # regardless of the frequency + # https://github.com/ossec/ossec-hids/issues/1415 # with app.sudo(): self.service_restarted(app, "ossec") @@ -203,6 +215,21 @@ def test_journalist_mail_notification(self, host): mon, "test 1 = $(mailq | grep journalist@ossec.test | wc -l)") + assert self.run( + mon, + "grep --count 'notification suppressed' /var/log/syslog " + "> /tmp/before") + + # + # The second notification within less than 24h is suppressed + # + with app.sudo(): + self.service_restarted(app, "ossec") + assert self.wait_for_command(mon, """ + grep --count 'notification suppressed' /var/log/syslog > /tmp/after + test $(cat /tmp/before) -lt $(cat /tmp/after) + """) + # # teardown the ossec and postfix on mon and app # From 51d6e23a4e8f705178860c9b51cea8fc1df2c3f5 Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Tue, 8 May 2018 17:02:24 +0200 Subject: [PATCH 3/4] testinfra: fake payload are ok to verify journalist encryption (cherry picked from commit 81e96878e4368bc2c8e2fed4b4b436b15ffff95d) --- testinfra/ossec/test_journalist_mail.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testinfra/ossec/test_journalist_mail.py b/testinfra/ossec/test_journalist_mail.py index b98f169371..db8dfe1a89 100644 --- a/testinfra/ossec/test_journalist_mail.py +++ b/testinfra/ossec/test_journalist_mail.py @@ -116,8 +116,8 @@ def trigger(who, payload): # encrypted mail to journalist or ossec contact # for (who, payload, expected) in ( - ('journalist', 'ossec: output\n1', '1'), - ('ossec', 'MYGREATPAYLOAD', 'MYGREATPAYLOAD')): + ('journalist', 'JOURNALISTPAYLOAD', 'JOURNALISTPAYLOAD'), + ('ossec', 'OSSECPAYLOAD', 'OSSECPAYLOAD')): assert self.run(host, "postsuper -d ALL") trigger(who, payload) assert self.run( From a12c65183f0f91511115d53a27661fb964540ee6 Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Tue, 8 May 2018 17:03:31 +0200 Subject: [PATCH 4/4] make journalist notifications resilient to double ossec alerts Under some circumstances daily journalist notifications may be grouped with other ossec alerts. In all cases where this transient error was observed, a well formed journalist notification alert was also included in the payload. By changing the regular expression we make the script resilient to payloads that contain unrelated content. Mitigates https://github.com/freedomofpress/securedrop/issues/3368 (cherry picked from commit 0e5694b4631702e0e56c37ec8a8483ef191946fc) --- .../ossec/files/process_submissions_today.sh | 2 +- testinfra/ossec/alert-journalist-one.txt | 14 +++++++ testinfra/ossec/alert-journalist-two.txt | 39 +++++++++++++++++++ testinfra/ossec/alert-ossec.txt | 22 +++++++++++ testinfra/ossec/test_journalist_mail.py | 15 ++++--- 5 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 testinfra/ossec/alert-journalist-one.txt create mode 100644 testinfra/ossec/alert-journalist-two.txt create mode 100644 testinfra/ossec/alert-ossec.txt diff --git a/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh b/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh index 936c83e879..b55fb146fe 100755 --- a/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh +++ b/install_files/ansible-base/roles/ossec/files/process_submissions_today.sh @@ -29,7 +29,7 @@ function handle_notification() { stdin="$(< /dev/stdin)" local count - count=$(echo "$stdin" | perl -ne 'print scalar(<>) and exit if(/ossec: output/);') + count=$(echo "$stdin" | perl -ne "print scalar(<>) and exit if(m|ossec: output: 'head -1 /var/lib/securedrop/submissions_today.txt|);") if [[ "$count" =~ ^[0-9]+$ ]] ; then export SUBJECT="Submissions in the past 24h" # diff --git a/testinfra/ossec/alert-journalist-one.txt b/testinfra/ossec/alert-journalist-one.txt new file mode 100644 index 0000000000..2020c83ef0 --- /dev/null +++ b/testinfra/ossec/alert-journalist-one.txt @@ -0,0 +1,14 @@ +OSSEC HIDS Notification. +2018 May 06 20:18:24 + +Received From: (app) 10.0.1.4->head -1 /var/lib/securedrop/submissions_today.txt | grep '^[0-9]*$' +Rule: 400600 fired (level 1) -> "Boolean value indicating if there were submissions in the past 24h." +Portion of the log(s): + +ossec: output: 'head -1 /var/lib/securedrop/submissions_today.txt | grep '^[0-9]*$'': +0 + + + + --END OF NOTIFICATION + diff --git a/testinfra/ossec/alert-journalist-two.txt b/testinfra/ossec/alert-journalist-two.txt new file mode 100644 index 0000000000..4c3e2f8cab --- /dev/null +++ b/testinfra/ossec/alert-journalist-two.txt @@ -0,0 +1,39 @@ +OSSEC HIDS Notification. +2018 May 06 20:18:24 + +Received From: (app) 10.0.1.4->netstat -tan |grep LISTEN |grep -v 127.0.0.1 | sort +Rule: 533 fired (level 7) -> "Listened ports status (netstat) changed (new port opened or closed)." +Portion of the log(s): + +ossec: output: 'netstat -tan |grep LISTEN |grep -v 127.0.0.1 | sort': +tcp 0 0 0.0.0.0:111 0.0.0.0:* LISTEN +tcp 0 0 0.0.0.0:37294 0.0.0.0:* LISTEN +tcp6 0 0 :::111 :::* LISTEN +tcp6 0 0 :::44352 :::* LISTEN +Previous output: +ossec: output: 'netstat -tan |grep LISTEN |grep -v 127.0.0.1 | sort': +tcp 0 0 0.0.0.0:111 0.0.0.0:* LISTEN +tcp 0 0 0.0.0.0:48638 0.0.0.0:* LISTEN +tcp6 0 0 :::111 :::* LISTEN +tcp6 0 0 :::37312 :::* LISTEN + + + + --END OF NOTIFICATION + + + +OSSEC HIDS Notification. +2018 May 06 20:18:24 + +Received From: (app) 10.0.1.4->head -1 /var/lib/securedrop/submissions_today.txt | grep '^[0-9]*$' +Rule: 400600 fired (level 1) -> "Boolean value indicating if there were submissions in the past 24h." +Portion of the log(s): + +ossec: output: 'head -1 /var/lib/securedrop/submissions_today.txt | grep '^[0-9]*$'': +0 + + + + --END OF NOTIFICATION + diff --git a/testinfra/ossec/alert-ossec.txt b/testinfra/ossec/alert-ossec.txt new file mode 100644 index 0000000000..4203a88c84 --- /dev/null +++ b/testinfra/ossec/alert-ossec.txt @@ -0,0 +1,22 @@ +OSSEC HIDS Notification. +2018 May 06 20:18:24 + +Received From: (app) 10.0.1.4->netstat -tan |grep LISTEN |grep -v 127.0.0.1 | sort +Rule: 533 fired (level 7) -> "Listened ports status (netstat) changed (new port opened or closed)." +Portion of the log(s): + +ossec: output: 'netstat -tan |grep LISTEN |grep -v 127.0.0.1 | sort': +tcp 0 0 0.0.0.0:111 0.0.0.0:* LISTEN +tcp 0 0 0.0.0.0:37294 0.0.0.0:* LISTEN +tcp6 0 0 :::111 :::* LISTEN +tcp6 0 0 :::44352 :::* LISTEN +Previous output: +ossec: output: 'netstat -tan |grep LISTEN |grep -v 127.0.0.1 | sort': +tcp 0 0 0.0.0.0:111 0.0.0.0:* LISTEN +tcp 0 0 0.0.0.0:48638 0.0.0.0:* LISTEN +tcp6 0 0 :::111 :::* LISTEN +tcp6 0 0 :::37312 :::* LISTEN + + + + --END OF NOTIFICATION diff --git a/testinfra/ossec/test_journalist_mail.py b/testinfra/ossec/test_journalist_mail.py index db8dfe1a89..702007f6ea 100644 --- a/testinfra/ossec/test_journalist_mail.py +++ b/testinfra/ossec/test_journalist_mail.py @@ -64,19 +64,18 @@ class TestJournalistMail(TestBase): def test_procmail(self, host): self.service_started(host, "postfix") - today_payload = ( - 'ossec: output: head -1 /var/lib/securedrop/submissions_today.txt' - '\n1234') - for (destination, payload) in ( - ('journalist', today_payload), - ('ossec', 'MYGREATPAYLOAD')): + for (destination, f) in ( + ('journalist', 'alert-journalist-one.txt'), + ('journalist', 'alert-journalist-two.txt'), + ('ossec', 'alert-ossec.txt')): + self.ansible(host, "copy", + "dest=/tmp/{f} src=testinfra/ossec/{f}".format(f=f)) assert self.run(host, "/var/ossec/process_submissions_today.sh forget") assert self.run(host, "postsuper -d ALL") assert self.run( host, - "echo -e '{payload}' | " - "mail -s 'abc' root@localhost".format(payload=payload)) + "cat /tmp/{f} | mail -s 'abc' root@localhost".format(f=f)) assert self.wait_for_command( host, "mailq | grep -q {destination}@ossec.test".format(