daily email alert to journalists about submissions received in the past 24h #2803

ghost · 2018-01-07T13:46:29Z

Status

Ready for Review

Description of Changes

A cron job runs daily on the app server and updates the

/var/lib/securedrop/submissions_today.txt

file which contains the number of submissions sent in the past 24h, as
created by the manage.py how_many_submissions_today command.

The OSSEC agent on the app server runs a command daily, displaying
the content of /var/lib/securedrop/submissions_today.txt. The output
of the command is sent to the OSSEC server.

A new rule is defined on the OSSEC server to send a mail to when the
output is received from the OSSEC agent running on the app server.

A new procmail rule is definied on the OSSEC server to catch mails
encrypt mails containing the /var/lib/securedrop/submissions_today.txt
string and send them to the email defined by the
journalist_alert_email ansible variable.

A new set of (optional) ansible variables, similar to
ossec_alert_gpg_public_key, ossec_gpg_fpr, ossec_alert_email are
defined: journalist_alert_gpg_public_key, journalist_gpg_fpr,
journalist_alert_email. They are used to upload a journalist public
key to the OSSEC server and inserted into the send_encrypted_alarm.sh
script which handles mails received by procmail.

The modified send_encrypted_alarm.sh script takes one
argument (journalist or ossec) and dispatches the mail read from
stdin to the corresponding recipient.

Integration tests are implemented to verify the following:

manage.py how_many_submissions_today
the app OSSEC agent sends a mail to the journalist address
cover all branches of send_encrypted_alarm.sh

Testing

make build-debs
vagrant up /staging/
./testinfra/test.py staging

Running Testinfra suite against 'staging'...
Target roles:
    - testinfra/ossec
==================================================================== test session starts =====================================================================
platform linux2 -- Python 2.7.13, pytest-3.3.1, py-1.5.2, pluggy-0.6.0 -- /home/loic/software/securedrop/virtualenv/bin/python2
cachedir: .cache
rootdir: /home/loic/software/securedrop/securedrop, inifile: setup.cfg
plugins: testinfra-1.7.1, xdist-1.21.0, forked-0.2
collected 10 items                                                                                                                                           

testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_procmail[ansible://app-staging] SKIPPED                                              [ 10%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_send_encrypted_alert[ansible://app-staging] SKIPPED                                  [ 20%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_missing_journalist_alert[ansible://app-staging] SKIPPED                              [ 30%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_ossec_rule_journalist[ansible://app-staging] SKIPPED                                 [ 40%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_journalist_mail_notification[ansible://app-staging] SKIPPED                          [ 50%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_procmail[ansible://mon-staging] PASSED                                               [ 60%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_send_encrypted_alert[ansible://mon-staging] PASSED                                   [ 70%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_missing_journalist_alert[ansible://mon-staging] PASSED                               [ 80%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_ossec_rule_journalist[ansible://mon-staging] PASSED                                  [ 90%]
testinfra/ossec/test_journalist_mail.py::TestJournalistMail::test_journalist_mail_notification[ansible://mon-staging] PASSED                           [100%]

====================================================================== warnings summary ======================================================================
None
  Module already imported so cannot be rewritten: testinfra

-- Docs: http://doc.pytest.org/en/latest/warnings.html
===================================================== 5 passed, 5 skipped, 1 warnings in 106.35 seconds ======================================================

Comments

See the forum discussion for the rationale behind the testinfra: remove some XXX tests duplicating Ansible commits.

Deployment

The journalist emails is not set for existing installations, nothing will happen
During sdconfig the admin will be prompted for the email of the journalist and the encryption key. If set, the daily notification mails will be sent.

Checklist

If you made changes to the app code:

Unit and functional tests pass on the development VM

If you made changes to the system configuration:

Configuration tests pass

If you made changes to documentation:

Doc linting passed locally

ghost · 2018-01-27T21:51:33Z

@emkll this is the reason why sdconfig was refactored :-) This is the implementatoin we discussed a few weeks ago. It is based on OSSEC, reason why I invite you for a first review. It is mostly complete but still missing a few bits (documentation, sdconfig etc.).

ghost · 2018-01-28T00:04:27Z

Not sure this error comes from this PR, restarting https://circleci.com/gh/freedomofpress/securedrop/7409?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

 RUNNING HANDLER [tor-hidden-services : restart tor (simple)] *******************
    Sunday 28 January 2018  00:00:10 +0000 (0:00:00.071)       0:05:17.882 ********
    changed: [mon-staging]
    fatal: [app-staging]: FAILED! => {"changed": false, "msg": " * Stopping tor daemon...\n   ...done.\n * Starting tor daemon...\n   ...fail!\n"}

codecov-io · 2018-01-28T08:24:53Z

Codecov Report

Merging #2803 into develop will decrease coverage by 1.81%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2803      +/-   ##
===========================================
- Coverage    86.38%   84.57%   -1.82%     
===========================================
  Files           34       34              
  Lines         2122     2048      -74     
  Branches       233      222      -11     
===========================================
- Hits          1833     1732     -101     
- Misses         234      261      +27     
  Partials        55       55

Impacted Files	Coverage Δ
i18n_tool.py	`67.03% <0%> (-29.13%)`	⬇️
manage.py	`77.22% <0%> (-0.07%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d272e85...0713b76. Read the comment docs.

…ired

ghost · 2018-04-03T11:11:15Z

repushed with a clarification in the documentation, as suggested by @redshiftzero, thanks to @conorsch reminder!

conorsch · 2018-04-05T00:23:58Z

Update: haven't received any messages yet. OSSEC admin alerts are flowing fine, but nothing on the journalist address. Will leave the VMs running and see if I can't catch one. If not, I'll log in and poke and try to identify further manual action that was required to make the setup work as expected.

ghost · 2018-04-05T07:44:24Z

Update: haven't received any messages yet.

Could you please try to submit one document today? And tomorrow morning, if you did not receive any e-mail, could you, on the app server:

ls -l /var/lib/securedrop/submissions_today.txt
cat /var/lib/securedrop/submissions_today.txt

If that file exists and contains a number different from zero, it will help me figure out what's not working as expected.

If you receive an email tomorrow, it probably means there is a corner case when bootstraping the system.

I assume you're running this test on prod vms started with vagrant up /prod/?

Thanks a lot for your patience on this one: it's really good we get a chance to diagnose the problem now :-)

conorsch · 2018-04-05T16:36:39Z

@dachary The problem was indeed that /var/lib/securedrop/submissions_today.txt didn't exist yet. I've submitted another doc to the test instance and am currently waiting again. If the @daily marker doesn't trigger the cron job within the next few hours, I'll kick it off manually to expedite review.

It occurs to me that perhaps a slightly more frequent cron schedule would help to keep the "submissions today" logic accurate. That's a nit, though. This is definitely a huge step up in terms of actionable information for journalists using the system.

ghost · 2018-04-05T17:52:45Z

If the @daily marker doesn't trigger the cron job within the next few hours, I'll kick it off manually to expedite review.

We can wait another 24h, it really is valuable to be in a realistic scenario, thanks again for your patience doing this.

conorsch · 2018-04-06T21:51:52Z

My previous attempt at testing was misguided: I used prod VMs, mostly to validate the Tails workflow on setting up the new vars, but failed to install the locally built debs. Retried in staging with overridden vars (similar to what @redshiftzero described in #2803 (comment)) and can confirm working messages:

There has been submission activity in the past 24 hours.   
You should login and check SecureDrop.

Overall the Admin-facing workflow is impressively smooth. Because the new functionality is disabled by default, I'm comfortable with this going out as-is. Noticed some extremely small typos that could be cleaned up, will comment in-line.

conorsch

Requesting minor typographical changes to the docs and comments throughout. Functionally this works as advertised, and is surprisingly convenient for an Admin to configure (if their journalists are amenable to GPG usage).

Happy to address the docs nits myself, and push up here, @dachary. Thanks for your patience during the extremely long review cycle.

@redshiftzero I believe your comments have been addressed here, but the compression algo question stand out to me as warranting a 👍 for you that it's not a problem anymore. I didn't encounter any issues with it.

conorsch · 2018-04-06T21:52:59Z

install_files/ansible-base/roles/ossec/templates/send_encrypted_alarm.sh

+            echo "journalist alert email unset, no notification sent"
+            return
+        fi
+        compression="--compress-algo none"


I didn't encounter this problem; @redshiftzero do you still recommend the change here?

conorsch · 2018-04-06T21:53:32Z

securedrop/manage.py

+    parser = subps.add_parser(
+        'were-there-submissions-today',
+        help=('Update the file indicating '
+              'iff submissions were received in the past 24h'))


Nit: s/iff/whether/

conorsch · 2018-04-06T21:56:54Z

docs/install.rst

@@ -60,6 +60,21 @@ continuing:
   can add more later)
 -  The username of the system admin

+You can also, optionally, configure a :doc:`daily notifications
+<journalist>` about whether or not submission activity occurred in the


Nit: remove "a " in "a daily notifications".

redshiftzero

👍 from me (should also get @conorsch's 👍 prior to merge)

@dachary

Caught during review, didn't want to trouble @dachary by requesting changes.

redshiftzero · 2018-04-11T21:59:56Z

The lint CI job is consistently failing due to taking too long (build here), this is a new one...

redshiftzero · 2018-04-11T23:13:45Z

Ok waited a while, restarted and now the lint job is passing 🙃. Going to chalk this one up to fluctuations in the Circle CI force

redshiftzero · 2018-04-12T18:59:07Z

TIL about CircleCI workflows: to restart failed jobs that are dependencies for other jobs: one must do this or the subsequent jobs will not run

redshiftzero · 2018-04-12T19:31:32Z

Merging, thanks @dachary for your patience on this one and thanks to @emkll and @conorsch for the reviews and testing 😃

ghost added feature goals: journalist experience labels Jan 7, 2018

ghost requested review from conorsch, heartsucker, msheiny and redshiftzero as code owners January 7, 2018 13:46

ghost force-pushed the wip-dachary-1195-journalist-notification branch 3 times, most recently from 7b9ca5a to 9d32171 Compare January 14, 2018 13:52

ghost force-pushed the wip-dachary-1195-journalist-notification branch 7 times, most recently from c3a8a82 to 9e948cf Compare January 27, 2018 21:23

ghost added the OSSEC label Jan 27, 2018

ghost force-pushed the wip-dachary-1195-journalist-notification branch from 9e948cf to f6f5a50 Compare January 27, 2018 21:48

ghost force-pushed the wip-dachary-1195-journalist-notification branch 4 times, most recently from 09b3a34 to 1565f63 Compare January 27, 2018 23:34

ghost force-pushed the wip-dachary-1195-journalist-notification branch 2 times, most recently from 2eb3e36 to 081fc84 Compare January 28, 2018 07:40

ghost force-pushed the wip-dachary-1195-journalist-notification branch 3 times, most recently from 66f6791 to 330820f Compare January 29, 2018 14:32

Loic Dachary added 3 commits April 3, 2018 13:09

securedrop-admin: add sdconfig journalist_alert_email input

8b841bd

securedrop-admin: when present the journalist alert key must be verified

a30499b

securedrop-admin: when a journalist key is present, the email is requ…

0713b76

…ired

ghost force-pushed the wip-dachary-1195-journalist-notification branch from 4020536 to 0713b76 Compare April 3, 2018 11:09

eloquence mentioned this pull request Apr 5, 2018

Periodically send email notifications to journalists #544

Closed

conorsch suggested changes Apr 6, 2018

View reviewed changes

redshiftzero previously approved these changes Apr 9, 2018

View reviewed changes

Small typo fixes in journalist notification docs

7bec4ed

Caught during review, didn't want to trouble @dachary by requesting changes.

conorsch dismissed redshiftzero’s stale review via 7bec4ed April 11, 2018 20:35

conorsch approved these changes Apr 11, 2018

View reviewed changes

redshiftzero merged commit bca891b into develop Apr 12, 2018

redshiftzero deleted the wip-dachary-1195-journalist-notification branch April 12, 2018 19:31

redshiftzero added this to the 0.7 milestone Apr 24, 2018

emkll mentioned this pull request Apr 25, 2018

Release SecureDrop 0.7.0 #3307

Closed

21 tasks

This was referenced May 4, 2018

[0.7.0] Journalist and admin notifications can group and cause "SecureDrop Submissions Error" #3368

Closed

[0.7.0] Journalist notification sends email every time the app server is manually restarted #3367

Closed

conorsch mentioned this pull request May 9, 2018

ossec: resolve journalist notification racing with reboots #3374

Merged

1 task

redshiftzero mentioned this pull request Jun 20, 2018

Disable do-release-upgrade OSSEC notification #3546

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

daily email alert to journalists about submissions received in the past 24h #2803

daily email alert to journalists about submissions received in the past 24h #2803

ghost commented Jan 7, 2018 •

edited by ghost

Loading

ghost commented Jan 27, 2018 •

edited by ghost

Loading

ghost commented Jan 28, 2018

codecov-io commented Jan 28, 2018 •

edited

Loading

ghost commented Apr 3, 2018

conorsch commented Apr 5, 2018

ghost commented Apr 5, 2018 •

edited by ghost

Loading

conorsch commented Apr 5, 2018

ghost commented Apr 5, 2018

conorsch commented Apr 6, 2018

conorsch left a comment

conorsch Apr 6, 2018

conorsch Apr 6, 2018

conorsch Apr 6, 2018

redshiftzero left a comment

redshiftzero commented Apr 11, 2018

redshiftzero commented Apr 11, 2018

redshiftzero commented Apr 12, 2018 •

edited

Loading

redshiftzero commented Apr 12, 2018

daily email alert to journalists about submissions received in the past 24h #2803

daily email alert to journalists about submissions received in the past 24h #2803

Conversation

ghost commented Jan 7, 2018 • edited by ghost Loading

Status

Description of Changes

Testing

Comments

Deployment

Checklist

If you made changes to the app code:

If you made changes to the system configuration:

If you made changes to documentation:

ghost commented Jan 27, 2018 • edited by ghost Loading

ghost commented Jan 28, 2018

codecov-io commented Jan 28, 2018 • edited Loading

Codecov Report

ghost commented Apr 3, 2018

conorsch commented Apr 5, 2018

ghost commented Apr 5, 2018 • edited by ghost Loading

conorsch commented Apr 5, 2018

ghost commented Apr 5, 2018

conorsch commented Apr 6, 2018

conorsch left a comment

Choose a reason for hiding this comment

conorsch Apr 6, 2018

Choose a reason for hiding this comment

conorsch Apr 6, 2018

Choose a reason for hiding this comment

conorsch Apr 6, 2018

Choose a reason for hiding this comment

redshiftzero left a comment

Choose a reason for hiding this comment

redshiftzero commented Apr 11, 2018

redshiftzero commented Apr 11, 2018

redshiftzero commented Apr 12, 2018 • edited Loading

redshiftzero commented Apr 12, 2018

ghost commented Jan 7, 2018 •

edited by ghost

Loading

ghost commented Jan 27, 2018 •

edited by ghost

Loading

codecov-io commented Jan 28, 2018 •

edited

Loading

ghost commented Apr 5, 2018 •

edited by ghost

Loading

redshiftzero commented Apr 12, 2018 •

edited

Loading