[Openvpn-devel,2/2] Make most registry values optional

Message ID 1511026858-23281-2-git-send-email-selva.nair@gmail.com
State Accepted
Headers show
Series
  • [Openvpn-devel,1/2] Ensure strings read from registry are null-terminated
Related show

Commit Message

Selva Nair Nov. 18, 2017, 5:40 p.m.
From: Selva Nair <selva.nair@gmail.com>

Not all installations need registry values such as log_dir and
config_dir especially if automatic service is not in use.
This patch provides reasonable defaults for registry values.

- Read the default value of HKLM\Software\PACKAGE_NAME to get the
  install path and construct defaults for exe_path, config_dir,
  log_dir from it. Use "ovpn", "0", NORMAL_PRIORITY as the defaults
  for config file extension, log-append flag and process priority.

The only remaining required registry entry is the root key (usually
HKLM\Software\OpenVPN) whose default value should be set to the
installation path.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpnserv/common.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

Comments

Gert Doering Feb. 20, 2018, 12:23 p.m. | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

"Because it makes sense".  Stared at the code, compile-tested on 16.04.

Same issue as in 1/2 with the "reg_path" change...  but again, only
conflicts in context, not actual code, and context that is not affected 
by the code changes.

Your patch has been applied to the master and release/2.4 branch.

commit db04bca6729e9fe1ea60f0b3bd0329244a6ed611 (master)
commit c07bdace820753ab29def5edc3a6cadbf233cd03 (release/2.4)
Author: Selva Nair
Date:   Sat Nov 18 12:40:58 2017 -0500

     Make most registry values optional

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <1511026858-23281-2-git-send-email-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15892.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering Feb. 20, 2018, 4:14 p.m. | #2
Hi,

well, today is the day that I get to break things, it seems...

On Sat, Nov 18, 2017 at 12:40:58PM -0500, selva.nair@gmail.com wrote:
> +    /* The default value of REG_KEY is the install path */
> +    if (GetRegString(key, NULL, install_path, sizeof(install_path), NULL) != ERROR_SUCCESS)
> +    {
> +        goto out;
> +    }

This seems to break on Win7/64 for me.

Trying to start the latest iservice from Samuli's snapshot builds (master, 
but I assume release/2.4 is also affected) returns a non-useful error 
message.

Looking into the event viewer, it tells me:

	openvpnserv error: Falscher Parameter. (0x57)
	Error querying registry value: HKLM\SOFTWARE\OpenVPN\(null)

("Falscher Parameter" = "invalid argument")


... so maybe this is not the right way to query the default path?

gert
Selva Nair Feb. 20, 2018, 4:19 p.m. | #3
Hi,

On Tue, Feb 20, 2018 at 11:14 AM, Gert Doering <gert@greenie.muc.de> wrote:
> Hi,
>
> well, today is the day that I get to break things, it seems...
>
> On Sat, Nov 18, 2017 at 12:40:58PM -0500, selva.nair@gmail.com wrote:
>> +    /* The default value of REG_KEY is the install path */
>> +    if (GetRegString(key, NULL, install_path, sizeof(install_path), NULL) != ERROR_SUCCESS)
>> +    {
>> +        goto out;
>> +    }
>
> This seems to break on Win7/64 for me.
>
> Trying to start the latest iservice from Samuli's snapshot builds (master,
> but I assume release/2.4 is also affected) returns a non-useful error
> message.
>
> Looking into the event viewer, it tells me:
>
>         openvpnserv error: Falscher Parameter. (0x57)
>         Error querying registry value: HKLM\SOFTWARE\OpenVPN\(null)
>
> ("Falscher Parameter" = "invalid argument")
>
>
> ... so maybe this is not the right way to query the default path?


Hmm.. I thought I had tested the patch.. Looking at it right now -- hold on :)

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index bc99584..759e6a0 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -87,6 +87,8 @@  GetOpenvpnSettings(settings_t *s)
     TCHAR append[2];
     DWORD error;
     HKEY key;
+    TCHAR install_path[MAX_PATH];
+    TCHAR default_value[MAX_PATH];
 
     LONG status = RegOpenKeyEx(HKEY_LOCAL_MACHINE, REG_KEY, 0, KEY_READ, &key);
     if (status != ERROR_SUCCESS)
@@ -95,37 +97,50 @@  GetOpenvpnSettings(settings_t *s)
         return MsgToEventLog(M_SYSERR, TEXT("Could not open Registry key HKLM\\%s not found"), REG_KEY);
     }
 
-    error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s->exe_path), NULL);
+    /* The default value of REG_KEY is the install path */
+    if (GetRegString(key, NULL, install_path, sizeof(install_path), NULL) != ERROR_SUCCESS)
+    {
+        goto out;
+    }
+
+    openvpn_sntprintf(default_value, _countof(default_value), TEXT("%s\\bin\\openvpn.exe"),
+                      install_path);
+    error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s->exe_path), default_value);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("config_dir"), s->config_dir, sizeof(s->config_dir), NULL);
+    openvpn_sntprintf(default_value, _countof(default_value), TEXT("%s\\config"), install_path);
+    error = GetRegString(key, TEXT("config_dir"), s->config_dir, sizeof(s->config_dir),
+                         default_value);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("config_ext"), s->ext_string, sizeof(s->ext_string), NULL);
+    error = GetRegString(key, TEXT("config_ext"), s->ext_string, sizeof(s->ext_string),
+                         TEXT(".ovpn"));
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("log_dir"), s->log_dir, sizeof(s->log_dir), NULL);
+    openvpn_sntprintf(default_value, _countof(default_value), TEXT("%s\\log"), install_path);
+    error = GetRegString(key, TEXT("log_dir"), s->log_dir, sizeof(s->log_dir), default_value);
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("priority"), priority, sizeof(priority), NULL);
+    error = GetRegString(key, TEXT("priority"), priority, sizeof(priority),
+                         TEXT("NORMAL_PRIORITY_CLASS"));
     if (error != ERROR_SUCCESS)
     {
         goto out;
     }
 
-    error = GetRegString(key, TEXT("log_append"), append, sizeof(append), NULL);
+    error = GetRegString(key, TEXT("log_append"), append, sizeof(append), TEXT("0"));
     if (error != ERROR_SUCCESS)
     {
         goto out;