forced invalid path in windows

[jktseug]

In the code openvpn forces the PATH environment to the C:\ drive.
This is not correct and the OS can be installed on any drive letter.

src/openvpn/win32.c

env_block (const struct env_set *es)
{

char * force_path = "PATH=C:
Windows
System32;C:
WINDOWS;C:
WINDOWS
System32
Wbem";

[jktseug]

This is a problem at the very least in the up script, could be in the down script as well.

[Gert Döring]

Patch welcome to figure out what would be "correct".

Not being a windows programmer, I did what I had to do to make the netsh.exe calls for IPv6 work (having ...\wbem in the active path) - and since I don't know anyone who installs windows anywhere but c:\windows, it never caused problems.

"Version 2.2.2" is definitely not right here, as this code didn't appear before 2.3.0

[jktseug]

well, you could just append to the current path
PATH=%PATH%;%systemroot%\system32\wbem
should suffice to get \wbem in the path list.

at least on windows 7, wbem is in the path variable already.

You can also pull the registry
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet?\Control\Session
Manager\Environment

this would also effect if windows is installed to something other than c:\windows
ie c:\win
c:\windowsold

This effects winpe environments where the system root is actually x:\

[Samuli Seppänen]

Appending to the current PATH sounds reasonable. Managing the registry would probably be more hairy, as OpenVPN does not (afaik) contain any registry-managing code.

[Gert Döring]

I'm not sure if it will work if I use %systemroot% inside C - that's .bat syntax to evaluate variables. Need to test.

[jktseug]

Replying to cron2:

I'm not sure if it will work if I use %systemroot% inside C - that's .bat syntax to evaluate variables. Need to test.

It likely won't.

There are two things going on here.
adding environment variables, anytime you do that it clears out the current environment variables set up by the calling process.

After that the block needs the PATH variable to find certain commands.

If there is a reason to clear out the environmental variables (maybe some security reason, or similar) then it will have to be recreated. If not, then it should be trivial to just grab the entire environment and add the variables needed to it.

I can look a little more, if I get some time maybe I can propose a patch.

[jktseug]

looks like std::getenv("PATH"); should work.

make sure to include the cstdlib
then

const char * force_path= std::getenv("PATH");

You will need to add PATH= to the beginning of this as it should just return the value.

[Gert Döring]

Uh, std::getenv() looks fairly C++'y - OpenVPN is pure C. Will that work in a C program?

[jktseug]

​http://www.tutorialspoint.com/c_standard_library/c_function_getenv.htm

I am pretty sure this is in the C standard

in fact, it should be available on every OS.

you may have to use just getenv() instead of adding the namespace to use it in C.

[Gert Döring]

Fortunately someone came to my rescue and just sent a patch to the openvpn-devel list - thanks :-)

commit 7546cba4761b24f2195034dcd3407aecd43fd3be (master)
commit 9f87007d8997d8127b9ea5fb0cb6b6cb9c88a4f3 (release/2.3)

Author: Selva Nair
Date: Thu Nov 12 21:41:27 2015 -0500

Do not hard-code windows systemroot in env_block

merged and pushed, will show up in 2.3.9 "in the next few weeks".

Closing the ticket as I'm convinced the fix is the right thing to do (especially as inside OpenVPN, "systemroot" is actually already known - which I didn't know). If it does not work, please reopen.