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

Don't put the cipher key in if no cipher is specified. #117

Closed
wants to merge 1 commit into from

Conversation

typhonius
Copy link
Contributor

If no cipher is specified in puppet, the client configuration files get written with a cipher key but no value. This causes the android openvpn app to complain.

I envisage 3 possible solutions, of which this PR implements the most simple:

  • Copy server.erb and only write the cipher if it not empty
  • Default the cipher to a strong cipher ie AES-256-CBC
  • Default the cipher to undef so it does not get written with the template file in its current state.

It's also worth noting that server.erb and client.erb should probably implement similar variable checking and standardising on one or other solution.

@luxflux
Copy link
Contributor

luxflux commented Jan 10, 2015

Thanks for your fix.
eah, you are right, the variable checking should be the same. I just didn't have the time refactor the module to have to handle this at one place 😞

Could you merge master (which fixes the travis build) and add a test for this?

@typhonius
Copy link
Contributor Author

@luxflux thanks for your feedback. I've rebased on master and added a test.

@luxflux
Copy link
Contributor

luxflux commented Jan 11, 2015

After merging your other feature this one has merge conflicts... Could you rebase once more? Thanks!

@luxflux
Copy link
Contributor

luxflux commented Feb 2, 2015

Thanks for your work! I am going to merge this pull request with some other changes via #129.

@luxflux luxflux closed this Feb 2, 2015
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.

2 participants