Skip to contentSkip to conversation

Make easy-rsa/2.0/vars handle paths with whitespaces

PATCH In review Tanel Rebane 5 participants 11 messages Aug 13, 2011 – Aug 31, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Make easy-rsa/2.0/vars handle paths with whitespaces

Hello,

Line 29 of vars, export KEY_CONFIG=$EASY_RSA/whichopensslcnf $EASY_RSA, cannot handle whitespaces contained in $EASY_RSA. This line could be changed - depending on preference - to either of:
a) export KEY_CONFIG=""${EASY_RSA}"/whichopensslcnf "${EASY_RSA}""
b) export KEY_CONFIG=$("${EASY_RSA}"/whichopensslcnf "${EASY_RSA}")

Regards,
Tanel Rebane

#1 lore raw
Quoted 12 lines

Hello,

Line 29 of vars, export KEY_CONFIG=$EASY_RSA/whichopensslcnf $EASY_RSA, cannot handle whitespaces contained in $EASY_RSA. This line could be changed - depending on preference - to either of:
a) export KEY_CONFIG=""${EASY_RSA}"/whichopensslcnf "${EASY_RSA}""
b) export KEY_CONFIG=$("${EASY_RSA}"/whichopensslcnf "${EASY_RSA}")

Regards,
Tanel Rebane

Quoted 10 lines

------------------------------------------------------------------------------
FREE DOWNLOAD - uberSVN with Social Coding for Subversion. Subversion made easy with a complete admin console. Easy to use, easy to manage, easy to install, easy to extend. Get a Free download of the new open ALM Subversion platform now.
http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Hi Tanel,

Both approaches seem POSIX-compliant. I'll do some tests and send in a proper patch.

Btw. I noted that some of the easy-rsa scripts lack the customary interpreter line (e.g. #!/bin/sh). We should probably fix that, too.

Signature · 5 lines
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock
#2 lore raw

PATCH 1/1 Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

From: Samuli Seppänen <[email protected]...>

Original $KEY_CONFIG variable generation in broke if $EASY_RSA had whitespace in it. This patch fixes the issue in a POSIX-compliant way.

---
 easy-rsa/2.0/vars |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
easy-rsa/2.0/vars +1 -1
index 2ea1ced..780ecf6 100755
@@ -26,7 +26,7 @@ export GREP="grep"
 # This variable should point to
 # the openssl.cnf file included
 # with easy-rsa.
-export KEY_CONFIG=`$EASY_RSA/whichopensslcnf $EASY_RSA`
+export KEY_CONFIG=$("$EASY_RSA"/whichopensslcnf "$EASY_RSA")
 
 # Edit this variable to point to
 # your soon-to-be-created key
Signature · 1 line
1.7.4.1
#3 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 17 lines · On 18/08/11 09:25, [email protected]... wrote:

From: Samuli Seppänen <[email protected]...>

Original $KEY_CONFIG variable generation in broke if $EASY_RSA had whitespace in it. This patch fixes the issue in a POSIX-compliant way.

---
 easy-rsa/2.0/vars |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
easy-rsa/2.0/vars +0 -0
index 2ea1ced..780ecf6 100755

easy-rsa/2.0/vars export GREP="grep" · line 29

Quoted 5 lines
easy-rsa/2.0/vars +1 -1
@@ -26,7 +26,7 @@ export GREP="grep"
 # This variable should point to
 # the openssl.cnf file included
 # with easy-rsa.
-export KEY_CONFIG=`$EASY_RSA/whichopensslcnf $EASY_RSA`
+export KEY_CONFIG=$("$EASY_RSA"/whichopensslcnf "$EASY_RSA")

Even though I personally like this syntax better, isn't it better to use the `` syntax instead of $() due to portability? Or are we sure that $() functional everywhere nowadays?

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk5XUmoACgkQDC186MBRfrq1CwCfbN5YVMx2ZMsChWO3kik3dA5/ tdEAoKN+9lvvUoLU4zwsgaM1IEimnUas
=l3K8
-----END PGP SIGNATURE-----

#4 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

Quoted 29 lines · On 18/08/11 09:25, [email protected]... wrote:
Quoted 3 lines

From: Samuli Seppänen <[email protected]...>

Original $KEY_CONFIG variable generation in broke if $EASY_RSA had

whitespace in

Quoted 14 lines

it. This patch fixes the issue in a POSIX-compliant way.

---
 easy-rsa/2.0/vars |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
easy-rsa/2.0/vars +0 -0
index 2ea1ced..780ecf6 100755

easy-rsa/2.0/vars export GREP="grep" · line 29

Quoted 5 lines
easy-rsa/2.0/vars +1 -1
@@ -26,7 +26,7 @@ export GREP="grep"
 # This variable should point to
 # the openssl.cnf file included
 # with easy-rsa.
-export KEY_CONFIG=`$EASY_RSA/whichopensslcnf $EASY_RSA`
+export KEY_CONFIG=$("$EASY_RSA"/whichopensslcnf "$EASY_RSA")

Even though I personally like this syntax better, isn't it better to use the `` syntax instead of $() due to portability? Or are we sure that $() functional everywhere nowadays?

Quoted 3 lines

kind regards,

David Sommerseth

There shouldn't be a difference, I did quite a bit of Googling just to be sure. For example, the Dash shell seems to be used often as POSIX-compliance reference implementation. From
<http://linux.die.net/man/1/dash>:

"Only features designated by POSIX, plus a few Berkeley extensions, are being incorporated into this shell. We expect POSIX conformance by the time 4.4 BSD is released."

Dash supports both $() and `` command substitution syntaces. Of course, there might be some other shells which don't support the $() syntax. If somebody knows any, please raise your voice :).

Samuli

#5 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

Quoted 3 lines

There shouldn't be a difference, I did quite a bit of Googling just to be sure. For example, the Dash shell seems to be used often as POSIX-compliance reference implementation. From

POSIX mandates $() so any posix shell will support it. The issue is with non-POSIX shells, if you do want to support them (e.g. Solaris's /bin/sh).

Stefan

#6 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

Hi,

Quoted 3 lines · Stefan Monnier

POSIX mandates $() so any posix shell will support it. The issue is with non-POSIX shells, if you do want to support them (e.g. Solaris's /bin/sh).

I'd rather not go down that rat-hole. Even Solaris has POSIX-compliant shells today (ISTR that there is /bin/ksh).

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#7 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

Quoted 11 lines

Hi,

Quoted 3 lines · Stefan Monnier

POSIX mandates $() so any posix shell will support it. The issue is with non-POSIX shells, if you do want to support them (e.g. Solaris's /bin/sh).

I'd rather not go down that rat-hole. Even Solaris has POSIX-compliant shells today (ISTR that there is /bin/ksh).

gert

Perhaps this is simply a documentation issue:

"Please use a POSIX-compliant shell (e.g. dash) with easy-rsa or you may get unexpected results. For example, /bin/sh on Solaris is not compliant, but you can use /bin/ksh instead."

What do you think?

Signature · 5 lines
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock
#8 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 10 lines · Samuli Seppänen
Quoted 10 lines

Hi,

Quoted 3 lines · Stefan Monnier

POSIX mandates $() so any posix shell will support it. The issue is with non-POSIX shells, if you do want to support them (e.g. Solaris's /bin/sh).

I'd rather not go down that rat-hole. Even Solaris has POSIX-compliant shells today (ISTR that there is /bin/ksh).

Quoted 8 lines
Quoted 1 line

gert

Perhaps this is simply a documentation issue:

"Please use a POSIX-compliant shell (e.g. dash) with easy-rsa or you may get unexpected results. For example, /bin/sh on Solaris is not compliant, but you can use /bin/ksh instead."

What do you think?

ACK! That makes sense to me. But it needs to be clearly visible in some kind of requirements list. If we don't have that, that's probably something we should have.

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk5d/4YACgkQDC186MBRfrohGACgoQFXyVf3n653bcAfNgPKssg0 K4MAnjYSnGkMxY1Fa64sNqR1MSU6B7L0
=tpDb
-----END PGP SIGNATURE-----

#9 lore raw

Re: [Openvpn-devel] [PATCH] Added whitespace handling for $KEY_CONFIG variable in easy-rsa/2.0/vars

Quoted 3 lines

ACK! That makes sense to me. But it needs to be clearly visible in some kind of requirements list. If we don't have that, that's probably something we should have.

You could even start your shell scripts with something like:

test "x$(pwd)" = "xpwd" || (echo "This shell is not good enough"; exit 1)

Stefan

#10 lore raw
In reply to Samuli Seppänen

As I've understood, the second approach is safer, but the first one is more portable. Then again, don't hold me to it as I have no sources available (other than myself) confirming that.

I also noticed that easy-rsa/2.0/build-dh has issues with whitespaces. This can easily be fixed with some double-quotes on line 6 and 7.

The current...
if [ -d $KEY_DIR ] && [ $KEY_SIZE ]; then

    $OPENSSL dhparam -out ${KEY_DIR}/dh${KEY_SIZE}.pem ${KEY_SIZE}

...becomes:
if [ -d "$KEY_DIR" ] && [ "$KEY_SIZE" ]; then

    "$OPENSSL" dhparam -out "$KEY_DIR"/dh"$KEY_SIZE".pem "$KEY_SIZE"

Also, I don't see the point of those curly brackets, might as well keep it consistent.

I general I think it would be a good idea to run a "grep '$' easy-rsa/2.0/*" and make sure that variables are properly quoted.

Regards,
Tanel Rebane

2011/8/17 Samuli Seppänen <[email protected]...>:

Quoted 41 lines
Quoted 12 lines

Hello,

Line 29 of vars, export KEY_CONFIG=$EASY_RSA/whichopensslcnf $EASY_RSA, cannot handle whitespaces contained in $EASY_RSA. This line could be changed - depending on preference - to either of:
a) export KEY_CONFIG=""${EASY_RSA}"/whichopensslcnf "${EASY_RSA}""
b) export KEY_CONFIG=$("${EASY_RSA}"/whichopensslcnf "${EASY_RSA}")

Regards,
Tanel Rebane

Quoted 10 lines

------------------------------------------------------------------------------
FREE DOWNLOAD - uberSVN with Social Coding for Subversion. Subversion made easy with a complete admin console. Easy to use, easy to manage, easy to install, easy to extend. Get a Free download of the new open ALM Subversion platform now.
http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Hi Tanel,

Both approaches seem POSIX-compliant. I'll do some tests and send in a proper patch.

Btw. I noted that some of the easy-rsa scripts lack the customary interpreter line (e.g. #!/bin/sh). We should probably fix that, too.

--

Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock