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

Update KEY_CONFIG placeholder #8

Closed
wants to merge 1 commit into from
Closed

Update KEY_CONFIG placeholder #8

wants to merge 1 commit into from

Conversation

johnchapin
Copy link

The KEY_CONFIG value is surrounded by backticks, which causes an error when sourcing the file prior to using any of the easy-rsa scripts.

This change removes the backticks and alters the placeholder to suggest the correct value.

The KEY_CONFIG value is surrounded by backticks, which causes an error when sourcing the file prior to using any of the easyrsa scripts.

This change removes the backticks and alters the placeholder to suggest the correct value.
@johnchapin
Copy link
Author

This is a change I have to make before using easy-rsa on OpenBSD 5.3.

I'm not sure if it affects other environments.

@QueuingKoala
Copy link
Contributor

This change is unnecessary (and has the flaw of hardcoding a file that won't exist.) Backticks in POSIX shell code runs the enclosed command in a subshell and returns the output in place of the subshell command. In this case, the "whichopensslcnf" script locates the correct cnf file based on the version string reported by the openssl command. The reality is that running any pre-1.0.x version of openssl is wildly insecure, but the mechanism does support older versions.

Further, this patch won't work since there is no "openssl-X-X-X.cnf" file shipped with the easy-rsa distribution. Hard-coding files that don't exist hardly seems like a service to users; this isn't a file users are expected to edit, so forcing them to locate/choose one seems to me a bad idea. Note that this is easy-rsa specific, so users attempting to use a distro-provided file will be in for a nasty surprise when that breaks with obscure errors citing missing env-vars.

You'll probably have a better time identifying the real cause of this problem by figuring out why the "whichopensslcnf" script is apparently giving you errors on your platform.

@johnchapin
Copy link
Author

Thanks for the detailed explanation - I'll investigate further on my system.

On Oct 28, 2013, at 2:00 PM, Josh Cepek notifications@github.com wrote:

This change is unnecessary (and has the flaw of hardcoding a file that won't exist.) Backticks in POSIX shell code runs the enclosed command in a subshell and returns the output in place of the subshell command. In this case, the "whichopensslcnf" script locates the correct cnf file based on the version string reported by the openssl command. The reality is that running any pre-1.0.x version of openssl is wildly insecure, but the mechanism does support older versions.

Further, this patch won't work since there is no "openssl-X-X-X.cnf" file shipped with the easy-rsa distribution. Hard-coding files that don't exist hardly seems like a service to users; this isn't a file users are expected to edit, so forcing them to locate/choose one seems to me a bad idea. Note that this is easy-rsa specific, so users attempting to use a distro-provided file will be in for a nasty surprise when that breaks with obscure errors citing missing env-vars.

You'll probably have a better time identifying the real cause of this problem by figuring out why the "whichopensslcnf" script is apparently giving you errors on your platform.


Reply to this email directly or view it on GitHub.

@johnchapin
Copy link
Author

@QueuingKoala - The whichopenssl script is missing in the openvpn package on OpenBSD 5.3. I'm going to investigate further, because it actually looks like an intentional decision on the part of the package maintainer.

Apologies for the spurious PR.

@johnchapin johnchapin closed this Oct 29, 2013
@johnchapin johnchapin deleted the patch-1 branch October 29, 2013 13:44
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

2 participants