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.

Sign up for GitHub

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

Jump to bottom

Additional migration details (SSH, extra admin USBS) #183

Merged
merged 1 commit into from
Mar 25, 2021
Merged

Additional migration details (SSH, extra admin USBS) #183

merged 1 commit into from
Mar 25, 2021

Conversation

rocodes
Copy link
Contributor

@rocodes rocodes commented Mar 17, 2021 •
edited
Loading

Status

Ready for review

Description of Changes

Testing

  • Visual review
  • Step through mon ssh repair

Release

  • n/a

Checklist (Optional)

  • Doc linting (make docs-lint) passed locally
  • Doc link linting (make docs-linkcheck) passed
  • You have previewed (make docs) docs at http://localhost:8000

eloquence
eloquence reviewed Mar 17, 2021
View reviewed changes
docs/backup_and_restore.rst Outdated
find ~/.ssh/ -type f -exec mv {} {}.bak \;

.. note::
You will be generatating fresh SSH credentials for the servers, and any
Copy link
Member

Choose a reason for hiding this comment

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

typo: generatating->generating

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 fc894a2

eloquence
eloquence reviewed Mar 17, 2021
View reviewed changes
docs/backup_and_restore.rst Outdated
- ``~/.ssh/id_rsa``

#. Boot into each additional Admin Workstation, setting an administrative
passphrase during startup. Once logged in, attach the Transfer Device
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 say "set an administration password and unlock the persistent volume on the Tails welcome screen" (that's more consistent with the language we've used elsewhere in the docs).

eloquence
eloquence reviewed Mar 17, 2021
View reviewed changes
docs/backup_and_restore.rst Outdated

#. Once all *Admin Workstations* have been updated, securely wipe the files on
the Transfer Device, by right-clicking them in the file manager and selecting
**Wipe**. Do not skip this step. Then, reformat the Transfer device using the
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 remove "Do not skip this step". If you want the Wipe/Format steps to stand out clearly, we could separate them.

eloquence
eloquence reviewed Mar 17, 2021
View reviewed changes
docs/backup_and_restore.rst Outdated
Device to transfer the public portions of those keys to your primary
*Admin Workstation*, where you will add them to the servers'
``authorized_keys`` files, as described :ref:`here <ssh_add_pubkey>`.
You may also `contact Support`_ for assistance.
Copy link
Member

Choose a reason for hiding this comment

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

  • This feels like it might be better placed in context, or in the preface of this section.
  • In any event, I would suggest making it clearer that this only pertains to the id_rsa/id_rsa.pub portion of the instructions -- they'll still need to copy the service details to each admin stick.

eloquence
eloquence reviewed Mar 17, 2021
View reviewed changes
docs/backup_and_restore.rst Outdated
and unlock it.

#. Ensure that this Admin Workstation is using an up-to-date version of Tails
and is running the latest SecureDrop Application code, |version|.
Copy link
Member

Choose a reason for hiding this comment

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

Application->application

@eloquence
Copy link
Member

Thanks @rocodes! At a high level, the process makes sense to me; my comments are mainly smaller nits. I've not tested the full procedure yet, but I have verified that the find command works as expected. If you have done a full procedural test, I would consider that sufficient to merge.

I'll admit that I personally just did the plug Admin Workstation 2 into Admin Workstation 1 and copy files over thing. There are reasonable security arguments to use an additional device, but also some additional opsec risks. As we do real-world migrations with admins, I would suggest we keep an eye out for these complexity traps, where real world procedures may not end up reflecting what we put in our docs.

@rocodes
Copy link
Contributor Author

rocodes commented Mar 18, 2021

I'll admit that I personally just did the plug Admin Workstation 2 into Admin Workstation 1 and copy files over thing. There are reasonable security arguments to use an additional device, but also some additional opsec risks. As we do real-world migrations with admins, I would suggest we keep an eye out for these complexity traps, where real world procedures may not end up reflecting what we put in our docs.

These are reasonable concerns. I'll defer to others on what the best procedure is, but my reasons for using an intermediate device are:

  • potentially error-prone and/or destructive operation with the only good Admin Workstation; best principle is to propose the nondestructive path
  • people who can understand these instructions and come up with a shortcut are welcome to do so

@rocodes
Copy link
Contributor Author

rocodes commented Mar 23, 2021

Thanks for your comments, addressed in fc894a2

@eloquence
Copy link
Member

Thanks @rocodes! :) Is this ready for final review, or do you want to pull in #175 as well?

@rocodes rocodes force-pushed the migration-docs-fixes branch from fc894a2 to ca986d6 Compare March 23, 2021 16:52
@rocodes
Copy link
Contributor Author

rocodes commented Mar 23, 2021

Ok, updated to also address #175. If we're happy with the 'luks usb to copy credentials to other admin sticks' approach, this is ready for review+merge.

@rocodes rocodes marked this pull request as ready for review March 23, 2021 16:54
@eloquence
Copy link
Member

LGTM; I pushed one small commit for whitespace/formatting tweaks and a typo (encryped->encrypted). We do have a merge conflict with the changes from #181. In case it's helpful for your copy/paste convenience, I've pushed 3782d8b in a separate branch, which squashes the changes in this PR into one commit, and resolves the conflict with main (I didn't want to force-push to this branch to avoid messing with your commit authorship).

@rocodes
Add instructions for repairing additional Admin USBS. Add notes on ro…
16990ae 
…tating SSH key during migration and on connecting directly to firewall during reinstallation.
@rocodes rocodes force-pushed the migration-docs-fixes branch from 73e139e to 16990ae Compare March 25, 2021 18:53
eloquence
eloquence approved these changes Mar 25, 2021
View reviewed changes
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.

Looks great, thanks again for your work on this @rocodes!

@eloquence eloquence merged commit f344df0 into main Mar 25, 2021
@legoktm legoktm deleted the migration-docs-fixes branch May 28, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eloquence eloquence eloquence approved these changes

@zenmonkeykstop zenmonkeykstop Awaiting requested review from zenmonkeykstop

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@rocodes @eloquence