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

Adding preshared key support #1465

Merged
merged 10 commits into from
Jun 5, 2019
Merged

Adding preshared key support #1465

merged 10 commits into from
Jun 5, 2019

Conversation

elreydetoda
Copy link
Contributor

@elreydetoda elreydetoda commented Jun 2, 2019

Description

So I in this pull request I simply took the existing ansible code and extended it to include preshared key support for wireguard. This is what it says about preshared keys in the man page:

PresharedKey — a base64 preshared key generated by wg genpsk. Optional, and may be omitted. This option adds an additional layer of symmetric-key cryptography to be mixed into the already existing public-key cryptography, for post-quantum resistance.

so since it is as simple as adding a wg genpsk instead of wg genkey and adding it to the configs, and you are supposed to get an extra layer of encryption for pretty much nothing.

Motivation and Context

the project touts that it

It uses the most secure defaults available

and I never see any other tutorials mention about this feature at all. So I figured maybe you didn't know about it and though why not include a little more security for a minimal amount of effort. I just wanted to help out since I saw it didn't include it yet 😄

How Has This Been Tested?

I ran it multiple times from a vagrant box image to a digital ocean vps: https://gist.github.com/elreydetoda/bac472aef59fc8d47145c4f25330187b#file-vagrantfile-elrey

simply ran vagrant up and then ./algo

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
    it is literally copied and pasted of what was already there.

I don't believe it needs documentation because it does this transparently to the user.

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2019

CLA assistant check
All committers have signed the CLA.

@elreydetoda
Copy link
Contributor Author

The travis-ci errors are things that I haven't touched. Just a suggestion for merging (not saying it will, but if it does) I would suggest to squash merge and just keep the same title.

also, thank you so much for making this project. I was going to stand up my own wireguard server but saw how this project allows you to add the dnssec, ad blocking, etc... and I hadn't even thought of doing that! So, thank you so much!
I saw about this project from here (just for reference): https://clo.ng/blog/algo_vpn/

@davidemyers
Copy link
Contributor

This worked for me when deploying to DigitalOcean.

I've also wondered if using the WireGuard PSK would be beneficial. I'm interested to know what others think.

@TC1977
Copy link
Contributor

TC1977 commented Jun 2, 2019

As per Wireguard docs, “WireGuard also supports an optional pre-shared key that is mixed into the public key cryptography. When pre-shared key mode is not in use, the pre-shared key value used below is assumed to be an all-zero string of 32-bytes.”

So Algo still uses the most secure default option available. 😅

On a more serious note, does this really increase security? The client PSKs are still available on the client machine and deploy machine, and the server PSKs are still available on the server, just like the other keys. I think there’s an easier way to attack the server than renting out a quantum computer.

@elreydetoda
Copy link
Contributor Author

elreydetoda commented Jun 2, 2019

Haha, nice find. I had missed that when skimming before 😄 Ya, @TC1977 I am sure there are other cheaper ways, but it doesn't hurt (that I know of) since it is already built in.

@jackivanov
Copy link
Collaborator

@elreydetoda I'm not allowed to push to your branch, so, could you, please, rebase form the master yourself to fix the tests?

@elreydetoda
Copy link
Contributor Author

@elreydetoda I'm not allowed to push to your branch, so, could you, please, rebase form the master yourself to fix the tests?

Thanks for the fix, and looks like we are good 😄

@elreydetoda
Copy link
Contributor Author

Is there anything else that needs to happen with this @jackivanov?

@jackivanov jackivanov merged commit 146cbc7 into trailofbits:master Jun 5, 2019
iBringit added a commit to iBringit/algo that referenced this pull request Nov 4, 2020
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.

5 participants