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

Documentation updates #1607

Merged
merged 24 commits into from
Dec 10, 2019
Merged

Documentation updates #1607

merged 24 commits into from
Dec 10, 2019

Conversation

TC1977
Copy link
Contributor

@TC1977 TC1977 commented Oct 9, 2019

Description

  • Fixes the new variable name (store_cakey -> store_pki) in the deploy from Ansible docs
  • Updates "mission statement" paragraph to reflect the primary use of Wireguard (and its client apps)
  • Clarifies the FAQ about clients connecting to each other.
  • Clarifies the prompts about VPN On Demand apply only to Apple IPsec clients.
  • Encourages people to set their config options and review config.cfg before deployment.
  • Clarifies that servers (mostly) can't be upgraded to take advantage of "new Algo features".
  • Adds instructions to the cloud-init docs about updating user list, and adds the correct locations of install log and configs.
  • Adds a FAQ about monitoring user activity.
  • Documents Python version incompatibilities described in Error using Python 3.8 on MacOS 10.14 #1622
  • Adds Road warrior instructions to the "deploy to Ubuntu" instructions.
  • Reorganizes config.cfg and (again) encourages setting options before deployment.

Motivation and Context

Fixes #1606.
Fixes question asked in #624 (and in the Gitter chat, numerous times)
Attempts to address #1538 (again)
Closes #1596 .
Nice callback to #1291 .
Addresses #1283, #1216, #1555, closes #1600 , and again numerous questions in Gitter chat
Through a time warp, closes #1610 before it was raised.
Closes #1622 .
Closes #1609 .
Documents the "BetweenClients_DROP" option brought up in #1516, #821, #1171 among others.

How Has This Been Tested?

Only way to test if documentation works is to see if people still have questions.

Checklist:

  • I have updated the documentation accordingly.

README.md Show resolved Hide resolved
@davidemyers
Copy link
Contributor

Encouraging people to review config.cfg before deployment is a great idea, but it still says ### Advanced users only below this line ### immediately after the users section. We should consider re-ordering config.cfg so that only the truly advanced stuff like subnet numbering and cloud provider configuration are at the bottom of the file below this warning. Probably best done as a separate PR.

@TC1977
Copy link
Contributor Author

TC1977 commented Oct 10, 2019

Great feedback. I'm on the road and will try to address this after the weekend.

@TC1977
Copy link
Contributor Author

TC1977 commented Oct 27, 2019

Encouraging people to review config.cfg before deployment is a great idea, but it still says ### Advanced users only below this line ### immediately after the users section. We should consider re-ordering config.cfg so that only the truly advanced stuff like subnet numbering and cloud provider configuration are at the bottom of the file below this warning. Probably best done as a separate PR.

@davidemyers Done! See what you think.

@davidemyers
Copy link
Contributor

I like it. Just a few suggestions:

I think unattended_reboot should be "above the line" so users are encouraged to review it, but it still needs to be disabled by default.

I suggest the Advanced section start with dnscrypt_servers and dns_servers, which are probably the first things an advanced user will look for.

I also suggest burying network definitions like strongswan_network, wireguard_network, and local_service_ip at the bottom of the file since they'll almost never be changed and this might encourage even advanced users to leave them alone.

Thanks!

As per feedback, also better explanation of keys_clean_all
@TC1977
Copy link
Contributor Author

TC1977 commented Oct 27, 2019

I like it. Just a few suggestions:

I think unattended_reboot should be "above the line" so users are encouraged to review it, but it still needs to be disabled by default.

I suggest the Advanced section start with dnscrypt_servers and dns_servers, which are probably the first things an advanced user will look for.

I also suggest burying network definitions like strongswan_network, wireguard_network, and local_service_ip at the bottom of the file since they'll almost never be changed and this might encourage even advanced users to leave them alone.

Thanks!

Ok, done! I also changed False to false for the keys_clean_all option, and rewrote the comment.

@TC1977
Copy link
Contributor Author

TC1977 commented Nov 14, 2019

Any other feedback, or anything else I should do to get this merged? Worried about these doc changes getting stale and requiring further updates.

@TC1977
Copy link
Contributor Author

TC1977 commented Dec 5, 2019

Given how many times the BetweenClients_DROP flag has come up, including twice in the last week on the Gitter chat, should that be promoted to a FAQ rather than buried in the deploy to Ubuntu instructions?

@TC1977
Copy link
Contributor Author

TC1977 commented Dec 9, 2019

Trying to keep these changes in sync with the latest commits. Is there a part that I can drop to get the rest of the doc updates in?

@jackivanov
Copy link
Collaborator

I think we're good

@jackivanov jackivanov merged commit 45aa006 into trailofbits:master Dec 10, 2019
@TC1977 TC1977 deleted the update-docs branch December 10, 2019 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants