Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecureDrop User Offboarding Guide #5012

Merged

Conversation

rocodes
Copy link
Contributor

@rocodes rocodes commented Nov 20, 2019

Resolves #4398
Add off-boarding guide for removing users from SecureDrop. Feedback welcome, thanks to @zenmonkeykstop and others for assistance with these steps in tickets of yore.

Status

Ready for review

Description of Changes

Docs only

Testing

Manual review only

Checklist

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

Copy link
Member

@eloquence eloquence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Looking great. First batch of comments, more TK)

Off-board Administrators and Journalists
========================================

When Journalists and SecureDrop Administrators leave your organization, it is important to off-board them from SecureDrop.
Copy link
Member

@eloquence eloquence Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Journalists->journalists
  • Administrators->administrators (consistency w/ usage in docs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 1def8b9

docs/offboarding.rst Outdated Show resolved Hide resolved
- An *Admin Workstation*. Contact SecureDrop Support or follow our :doc:`guide to rebuilding an Admin Workstation <rebuild_admin>` if you do not have one.
- An Admin account on the Journalist Interface

.. important:: Note that, in particular for SecureDrop Administrators, additional measures may need to be considered if the user's departure is on unfavourable terms, and those measures will vary depending on the circumstances and your own internal incident response procedures. If you are in such a situation, consult the SecureDrop Support team or Digital Security Training team via the SecureDrop contact form, or email [email protected] or a member of our team directly (found on the staff page). Note that, if you are in this situation, you may want to avoid reaching out to us via a ticket in our Support Portal, as that ticket will be visible to all Support Portal users at your organization.
Copy link
Member

@eloquence eloquence Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Administrators->administrators

I suggest we use the standard include includes/getting-support.txt at the end of the document, and link to that section in places where we ask folks to contact us. (Adding the help form to those options is a good idea.) I don't think we need to explicitly reference "the SecureDrop Support team" or "the Digital Security Team" in this context.

We can add that caveat regarding visibility of support tickets just before the include, so that it's visible in all contexts where we link to this document's "contact us" section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - have updated getting-support.txt in 1def8b9 with a condensed version of that disclaimer.

Off-boarding all users
----------------------

- `Inform the SecureDrop Support team`_ that the user's Support Portal account should be deactivated, and indicate if any new staff members should be added to the Support Portal..
Copy link
Member

@eloquence eloquence Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • If you have an account on the support portal, ...
  • Support Portal..->Support Portal. (Note two .. in current text. I wouldn't treat it like a proper noun either, but we're inconsistent in the docs right now.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO they should inform us of any user changes whether they have a support portal account or not, especially if the only support portal user leaves the organization and someone else is reading these docs trying to figure out how to take over admin duties. I have linked to the getting-support section here in 1def8b9 (and fixed the proper noun/typos!)-- what do you think?

----------------------

- `Inform the SecureDrop Support team`_ that the user's Support Portal account should be deactivated, and indicate if any new staff members should be added to the Support Portal..
- De-activate the user's account on the Journalist Interface.
Copy link
Member

@eloquence eloquence Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Journalist Interface -> Journalist Interface
De-activate -> Delete

(Discussion needed. The choice is between deletion vs. passphrase rotation; I'm not aware of negative consequences of deletion in the current SecureDrop experience, but it likely has some side effects in the context of reply attribution which is planned for the client -- i.e. will reply authorship information be lost? At what point do we want to warn about that?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Have changed to "delete" for now, but also welcome discussion re client behaviour. cc @creviera @redshiftzero

docs/offboarding.rst Outdated Show resolved Hide resolved
docs/offboarding.rst Outdated Show resolved Hide resolved
docs/offboarding.rst Outdated Show resolved Hide resolved
Copy link
Member

@eloquence eloquence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've completed a first pass review. This is great work, thanks! Most of my comments are small nits.

docs/offboarding.rst Outdated Show resolved Hide resolved
Rotate the SecureDrop Submission Key
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In some circumstances, you may want to rotate the SecureDrop Submission Key. Since the private key only resides on the airgapped *Secure Viewing Station*, you only need to rotate this key if you believe this key is not under your complete control (for example, if the SecureDrop Submission Key or the entire *SVS* USB could have been copied.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Same here: SecureDrop Submission Key -> Submission Key (twice)
  • "rotate this key if you believe this key" -> "rotate this key if you believe it"

In general, I consider this one of the most important paragraphs in this document. I think we can state the risks a bit more clearly. How about something more like this:

The Submission Private Key resides on the airgapped Secure Viewing Station and should normally not be held by the users of the system. We recommend rotating the Submission Key in the following circumstances:

  • If the separation from this user was not amicable;
  • If the user is still holding on to any Secure Viewing Station USB drive or backup;
  • If you have any other reason to believe the Submission Private Key or the entire Secure Viewing Station USB may have been copied.

docs/offboarding.rst Outdated Show resolved Hide resolved
docs/offboarding.rst Outdated Show resolved Hide resolved
docs/offboarding.rst Outdated Show resolved Hide resolved
- `Inform the SecureDrop Support team`_ that the user's Support Portal account should be deactivated, and indicate if any new staff members should be added to the Support Portal..
- De-activate the user's account on the Journalist Interface.
- Retrieve *Admin Workstation* or *Journalist Workstation* USB drive(s), *Transfer*, *Export*, and *Backup* drive(s), and any other SecureDrop hardware or materials.
- If the user receives email alerts (either OSSEC alerts or daily submission notifications), either directly or as a member of an email alias, remove them from those alerts and :ref:`set up someone new <ossec_guide>` to :ref:`receive those alerts <daily_journalist_alerts>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "either (either)" in this construction is a bit awkward:

If the user receives email alerts (either OSSEC alerts or daily submission notifications), either directly or as a member of an email alias

I would suggest splitting out the alias consideration into its own sentence, e.g., "If you've configured an alias for these email alerts, be sure to remove the user from it." But you could also just get rid of the first "either".


~/Persistent/securedrop/securedrop-admin reset_admin_access

This removes all other SSH keys, except for the new key that you are currently using, from the list of authorized keys on the *Application* and *Monitor Servers*. Note that if you had other folks on different SSH keys who also had remote access, this will also remove their SSH access, and their public keys will have to be re-appended to the authorized_keys file on each server, as in step 3.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I would prefer "users" or "admins" over "folks", just in terms of the tone and voice of the rest of the docs.
  • I would put that warning regarding the impact on other users before the command, and call it out more clearly (with or without an "important" box).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM; fixed in 1def8b9 and called out in important box

docs/offboarding.rst Show resolved Hide resolved
docs/index.rst Outdated
@@ -42,6 +42,7 @@ anonymous sources.
create_admin_account
test_the_installation
onboarding
offboarding
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit uneasy about adding this to the "install" section, because it's not technically part of the installation. I think it's fine for now given how messy the "topic guides" are, but let's revisit the organization soon as it's increasingly clear that we need something like a "maintenance" section for these types of docs.

I do think there is an argument for cross-referencing it from the Admin Guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added to the Topic Guide for now, even though messy, because you're right - it is not part of the installation procedure. Cross-referenced in the Admin Guide in b02ad01


**What you need:**

- An *Admin Workstation*. Contact SecureDrop Support or follow our :doc:`guide to rebuilding an Admin Workstation <rebuild_admin>` if you do not have one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See further below re: how we may want to reference contact options.

@rocodes rocodes force-pushed the 4398-docs-admin-offboarding branch from fec5a3c to 1def8b9 Compare November 28, 2019 20:44
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is great, and covers off the major tasks associated with off-boarding a user that aren't covered in enough detail elsewhere. Consistent formatting and nomenclature would make it even more useful for folks.

One possible large-scale change would be to make the document scenario-based ("what do i do if a journalist leaves? What if an admin leaves? What if an admin leaves under a cloud?" etc) rather than having a single checklist with provisos, but this is pretty much a subjective preference on my part.

docs/offboarding.rst Show resolved Hide resolved
notifications), either directly or as a member of an email alias, remove them
from those alerts and :ref:`set up someone new <ossec_guide>` to
:ref:`receive those alerts <daily_journalist_alerts>`.
- Make sure the user's organizational PGP key is revoked, if applicable, and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems superfluous to the process. It's rare for orgs to centrally manage GPG keys, and if someone has one it may not be SecureDrop-motivated or even involved in the process. (Plus folks may have a personal key with their work email as a uid etc).

docs/offboarding.rst Show resolved Hide resolved
On the Secure Viewing Station
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Boot into the *SVS*, and open the **Passwords and Keys** application. We will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving from "you" to "we" here is a bit inconsistent, as are the references to the Submission key. One alternative approach would be something like "First, change the uid of the current Submission Key to avoid mixing up old and new keys." followed by a numbered list starting with:
" From the Secure Viewing Station Applications menu, choose Utilities > Passwords and Keys."

change the name of the current SecureDrop submission key to avoid mixing
up the old and new keys.

Click on **GnuPG Keys** on the lefthand side of the pane, and select the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, would be good to continue these steps as a numbered list, consistent with your guide to renaming SSH keys above.

./securedrop-admin sdconfig

If the new public key that you placed in ``install_files/ansible_base`` has the
same name the same as the old public key, ``SecureDrop.asc``, the only part of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the sae name the same" seems like a typo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol there's a typo in my complaint about a typo. it's a typoception.


**Do not delete your old submission key!** You'll want to maintain it on the
*SVS* so that you can still decrypt old submissions that were made before you
changed keys. If you like, you can **Revoke** the key by selecting the key in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revoke should not be capitalized or bolded here. Also consider listing the steps as per previous sections, or folding the revocation process into the renaming steps above.

to push the changes to the server.

You may want to immediately create a test submission, then use a Journalist
account to log into the *Journalist Workstation* and download it to a *Transfer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Journalist Interface, not Journalist Workstation. Also It might be better to just ask folks to download and decrypt the submission rather than documenting how here.

docs/offboarding.rst Show resolved Hide resolved
Workstation* USB or that they may have kept a copy of the *Admin Workstation*
SSH key, you should rotate the key in the following manner.

#. **Create a new SSH keypair.**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that the numbered list needs bolded headers.

@rocodes rocodes force-pushed the 4398-docs-admin-offboarding branch 3 times, most recently from abed1b8 to 38c856f Compare December 12, 2019 21:10
…nization.

- Add note about support portal ticket visibility being org-wide.

- Incorporate feedback from review.
- Move offboarding out of install guide and into topic guide (for now).
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another couple of changes outstanding and this should be good to go!

docs/offboarding.rst Show resolved Hide resolved
docs/offboarding.rst Show resolved Hide resolved
Fix screenshot typo in key rotation instructions.
@rocodes rocodes force-pushed the 4398-docs-admin-offboarding branch from 38c856f to 9d1102a Compare December 17, 2019 19:53
@rocodes
Copy link
Contributor Author

rocodes commented Dec 17, 2019

thanks @zenmonkeykstop : fixed those final few changes in 9d1102a

Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with requested updates, approved!

@zenmonkeykstop zenmonkeykstop merged commit aec16ed into freedomofpress:develop Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Add Admin Offboarding Checklist
3 participants