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

.ovpn12 files #773

Merged
merged 7 commits into from
Aug 28, 2019
Merged

.ovpn12 files #773

merged 7 commits into from
Aug 28, 2019

Conversation

IcedComputer
Copy link
Contributor

Added new step to create an .ovpn12 file that can be stored on iOS keychain
This step is more secure method and does not require the end-user to keep entering passwords, or storing the client private cert where it can be easily tampered based on documentation located:
https://openvpn.net/faq/how-do-i-use-a-client-certificate-and-private-key-from-the-ios-keychain/

Someone can improve upon this by adding a parameter (possibly -i|--iOS) and then generating the original .ovpn file to not contain the client private certificate.

Added new step to create an .ovpn12 file that can be stored on iOS keychain
This step is more secure method and does not require the end-user to keep entering passwords, or storing the client private cert where it can be easily tampered based on documentation located:
 https://openvpn.net/faq/how-do-i-use-a-client-certificate-and-private-key-from-the-ios-keychain/

Someone can improve upon this by adding a parameter (possibly -i|--iOS) and then generating the original .ovpn file to not contain the client private certificate.
@coolapso
Copy link
Member

Will let other most active users comment on this as I am not at all familiar with IOS devices.

If their feedback is positive then ill gladly merge it.

@orazioedoardo @devZer0 @Giraffe1966 any takes on this?

@orazioedoardo
Copy link
Member

orazioedoardo commented Jun 26, 2019

Yeah, I see that this is more secure because the private parts of the config are not in the .ovpn file, though it's a bit less convenient since the user need to copy two files and perhaps we should instruct the user on how to use them. Also your script doesn't do the whole job:

  • the .ovpn12 file is root owned in the ovpn folder so you should chown and chmod it to allow the user to read it.
  • you need to strip cert and key sections (or just don't add them) in the .ovpn otherwise you defeat initial purpose.
  • probably you should use expect to type the export password and the private key password non-interactively.

By the way I wouldn't make this the default, I would add a one time dialog after the first client creation with some text like "TIP: If you have an iOS device you can pass -i|--iOS to [...]"

@Giraffe1966
Copy link
Contributor

Giraffe1966 commented Jun 26, 2019

No need to use expect for the password, you can use the -passin env:PASSWD and -passout env:PASSWD options to openssl to do this non-interactively. Also, don't use sudo for openssl as this script is run as root and this would break on systems without sudo installed.

added changes related to chown and chmod of .ovpn12 file.  Also removed sudo.
Incorporated feedback on how to properly implement .ovpn12 files.
the makeOVPN.sh now generates .ovpn12 files in the /home/${INSTALL_USER}/ovpns/ directory.
The remove script was updated to remove both the .ovpn and .ovpn12 files
@IcedComputer
Copy link
Contributor Author

Updated.

@coolapso
Copy link
Member

coolapso commented Aug 8, 2019

@orazioedoardo , @Giraffe1966 what is your take on this? is it good to be merged?

thanks a lot for keeping the boat moving! =)

Copy link
Contributor

@Giraffe1966 Giraffe1966 left a comment

Choose a reason for hiding this comment

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

Looks pretty good overall!

echo ":::"
echo "::: Commands:"
echo "::: [none] Interactive mode"
echo "::: nopass Create a client without a password"
echo "::: -d,--days Expire the certificate after specified number of days (default: 1080)"
echo "::: -n,--name Name for the Client (default: '"$(hostname)"')"
echo "::: -p,--password Password for the Client (no default)"
echo "::: -i,--iOS Generate a certificate that leverages iOS keychain"
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps include a link to https://openvpn.net/faq/how-do-i-use-a-client-certificate-and-private-key-from-the-ios-keychain/ so users know how to use the .ovpn12 file, and why this option increases security.

Copy link
Member

Choose a reason for hiding this comment

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

wouldn't a link be too much of information there?
Maybe we should put the link in the FAQ or wiki or whatsoever and get a simple reference to it?

scripts/makeOVPN.sh Outdated Show resolved Hide resolved
scripts/makeOVPN.sh Outdated Show resolved Hide resolved
@coolapso
Copy link
Member

coolapso commented Aug 9, 2019

@IcedComputer please apply the changes and suggestions mentioned by @Giraffe1966 and ill merge your PR

IcedComputer and others added 3 commits August 27, 2019 12:42
Co-Authored-By: Giraffe1966 <[email protected]>
Removed typo and changed -passin pass:$PASSWD to -passin env:$PASSWD
@IcedComputer
Copy link
Contributor Author

@4s3ti - I believe this is ready to go.

@coolapso
Copy link
Member

Thanks @IcedComputer , Merging.

@coolapso coolapso merged commit 40dff85 into pivpn:test Aug 28, 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.

None yet

4 participants