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

Bugfix/spaces in path #427

Merged
merged 7 commits into from Mar 23, 2022
Merged

Conversation

markus-t314
Copy link
Contributor

@markus-t314 markus-t314 commented Jan 22, 2021

This PR addresses a couple of issues when the path to easyrsa or pki contains spaces.
The problem is that in some cases arguments for openssl-calls can contain paths. When those arguments are not quoted and intentionally used for word splitting, paths with spaces are wrongly split and openssl-calls fail.
The fix is to keep paths in a separate variable and to quote it.
Some minor re-formating issues were also addressed to be more consistent with the existing code style.
This PR fixes issues #395 #408 #412
It also supersedes PR #413 as it fixes space-related problems on Linux as well.

For EC algorithm the variable $EASYRSA_ALGO_PARAMS is a path and needs to be quoted.
This also fixes OpenVPN#408
$EASYRSA_PKI is a path that could contain spaces where word splitting must be prevented.
The openssl call relied on word splitting for $crypto_ops
but $crypto_opts consists of a path which could contain spaces.
Now path is stored in $pass_opts which is quoted when using in
openssl call.
while moving them if filename contains spaces.
@TinCanTech TinCanTech added the Priority Acknowledged priority label Mar 15, 2022
@TinCanTech TinCanTech self-assigned this Mar 19, 2022
@TinCanTech
Copy link
Collaborator

TinCanTech commented Mar 19, 2022

I can feel your pain ..

The changes here will not be overlooked - Thanks!

@TinCanTech TinCanTech added this to the v3.0.9 milestone Mar 19, 2022
@TinCanTech TinCanTech added shellcheck shellcheck is correct conflicts Conflicts with current labels Mar 20, 2022
Copy link
Collaborator

@TinCanTech TinCanTech left a comment

Choose a reason for hiding this comment

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

This has been partially addressed by and conflicts with 8e7bac6

@TinCanTech
Copy link
Collaborator

@markus-t314 these changes here are important.

I am happy to work with you to resolve the new conflicts.

@TinCanTech TinCanTech merged commit 14d6e24 into OpenVPN:master Mar 23, 2022
@markus-t314
Copy link
Contributor Author

Oh sorry, I was not available this last week.
But I see you already resolved conflicts and merged this PR, so thanks for that :)

@TinCanTech
Copy link
Collaborator

All done. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Conflicts with current Priority Acknowledged priority shellcheck shellcheck is correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants