[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-08 Thread cron2 (Code Review) 
cron2 has submitted this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..

Move get_tmp_dir to win32-util.c and error out on failure

Currently we only warn in get_tmp_dir fails and set o->tmp_dir to
a null pointer. This will not be caught by check_file_access_chroot
either since that ignores NULL pointers but other parts of OpenVPN
will assume that tmp_dir is set to a non-NULL string.

Also move get_tmp_dir to win32-util.c to use it in unit tests.

Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Signed-off-by: Arne Schwabe 
Acked-by: Frank Lichtenheld 
Message-Id: <20240108171349.15871-1-g...@greenie.muc.de>
URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27964.html
Signed-off-by: Gert Doering 
---
M src/openvpn/options.c
M src/openvpn/win32-util.c
M src/openvpn/win32-util.h
M src/openvpn/win32.c
M src/openvpn/win32.h
5 files changed, 34 insertions(+), 32 deletions(-)




diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 1b28a19..f54f276 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -885,7 +885,15 @@
 #ifdef _WIN32
 /* On Windows, find temp dir via environment variables */
 o->tmp_dir = win_get_tempdir();
-#else
+
+if (!o->tmp_dir)
+{
+/* Error out if we can't find a valid temporary directory, which should
+ * be very unlikely. */
+msg(M_USAGE, "Could not find a suitable temporary directory."
+" (GetTempPath() failed).  Consider using --tmp-dir");
+}
+#else  /* ifdef _WIN32 */
 /* Non-windows platforms use $TMPDIR, and if not set, default to '/tmp' */
 o->tmp_dir = getenv("TMPDIR");
 if (!o->tmp_dir)
diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
index 81e504a..c5e7505 100644
--- a/src/openvpn/win32-util.c
+++ b/src/openvpn/win32-util.c
@@ -147,4 +147,26 @@
 }
 return true;
 }
+
+const char *
+win_get_tempdir(void)
+{
+static char tmpdir[MAX_PATH];
+WCHAR wtmpdir[MAX_PATH];
+
+if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
+{
+return NULL;
+}
+
+if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
+{
+msg(M_WARN, "Could not get temporary directory. Path is too long."
+"  Consider using --tmp-dir");
+return NULL;
+}
+
+WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
+return tmpdir;
+}
 #endif /* _WIN32 */
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
index ac37979..98bf74b 100644
--- a/src/openvpn/win32-util.h
+++ b/src/openvpn/win32-util.h
@@ -40,5 +40,8 @@
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);

+/* Find temporary directory */
+const char *win_get_tempdir(void);
+
 #endif /* OPENVPN_WIN32_UTIL_H */
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index e998d90..6b7ba5e 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1137,34 +1137,6 @@
 set_win_sys_path(buf, es);
 }

-
-const char *
-win_get_tempdir(void)
-{
-static char tmpdir[MAX_PATH];
-WCHAR wtmpdir[MAX_PATH];
-
-if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
-{
-/* Warn if we can't find a valid temporary directory, which should
- * be unlikely.
- */
-msg(M_WARN, "Could not find a suitable temporary directory."
-" (GetTempPath() failed).  Consider using --tmp-dir");
-return NULL;
-}
-
-if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
-{
-msg(M_WARN, "Could not get temporary directory. Path is too long."
-"  Consider using --tmp-dir");
-return NULL;
-}
-
-WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
-return tmpdir;
-}
-
 static bool
 win_block_dns_service(bool add, int index, const HANDLE pipe)
 {
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 3605966..aa8513b 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -286,9 +286,6 @@
 /* call self in a subprocess */
 void fork_to_self(const char *cmdline);

-/* Find temporary directory */
-const char *win_get_tempdir(void);
-
 bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);

 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-08 Thread cron2 (Code Review) 
cron2 has uploaded a new patch set (#3) to the change originally created by 
plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

The following approvals got outdated and were removed:
Code-Review+1 by cron2, Code-Review+2 by flichtenheld


Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..

Move get_tmp_dir to win32-util.c and error out on failure

Currently we only warn in get_tmp_dir fails and set o->tmp_dir to
a null pointer. This will not be caught by check_file_access_chroot
either since that ignores NULL pointers but other parts of OpenVPN
will assume that tmp_dir is set to a non-NULL string.

Also move get_tmp_dir to win32-util.c to use it in unit tests.

Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Signed-off-by: Arne Schwabe 
Acked-by: Frank Lichtenheld 
Message-Id: <20240108171349.15871-1-g...@greenie.muc.de>
URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27964.html
Signed-off-by: Gert Doering 
---
M src/openvpn/options.c
M src/openvpn/win32-util.c
M src/openvpn/win32-util.h
M src/openvpn/win32.c
M src/openvpn/win32.h
5 files changed, 34 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/81/481/3

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 1b28a19..f54f276 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -885,7 +885,15 @@
 #ifdef _WIN32
 /* On Windows, find temp dir via environment variables */
 o->tmp_dir = win_get_tempdir();
-#else
+
+if (!o->tmp_dir)
+{
+/* Error out if we can't find a valid temporary directory, which should
+ * be very unlikely. */
+msg(M_USAGE, "Could not find a suitable temporary directory."
+" (GetTempPath() failed).  Consider using --tmp-dir");
+}
+#else  /* ifdef _WIN32 */
 /* Non-windows platforms use $TMPDIR, and if not set, default to '/tmp' */
 o->tmp_dir = getenv("TMPDIR");
 if (!o->tmp_dir)
diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
index 81e504a..c5e7505 100644
--- a/src/openvpn/win32-util.c
+++ b/src/openvpn/win32-util.c
@@ -147,4 +147,26 @@
 }
 return true;
 }
+
+const char *
+win_get_tempdir(void)
+{
+static char tmpdir[MAX_PATH];
+WCHAR wtmpdir[MAX_PATH];
+
+if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
+{
+return NULL;
+}
+
+if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
+{
+msg(M_WARN, "Could not get temporary directory. Path is too long."
+"  Consider using --tmp-dir");
+return NULL;
+}
+
+WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
+return tmpdir;
+}
 #endif /* _WIN32 */
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
index ac37979..98bf74b 100644
--- a/src/openvpn/win32-util.h
+++ b/src/openvpn/win32-util.h
@@ -40,5 +40,8 @@
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);

+/* Find temporary directory */
+const char *win_get_tempdir(void);
+
 #endif /* OPENVPN_WIN32_UTIL_H */
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index e998d90..6b7ba5e 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1137,34 +1137,6 @@
 set_win_sys_path(buf, es);
 }

-
-const char *
-win_get_tempdir(void)
-{
-static char tmpdir[MAX_PATH];
-WCHAR wtmpdir[MAX_PATH];
-
-if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
-{
-/* Warn if we can't find a valid temporary directory, which should
- * be unlikely.
- */
-msg(M_WARN, "Could not find a suitable temporary directory."
-" (GetTempPath() failed).  Consider using --tmp-dir");
-return NULL;
-}
-
-if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
-{
-msg(M_WARN, "Could not get temporary directory. Path is too long."
-"  Consider using --tmp-dir");
-return NULL;
-}
-
-WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
-return tmpdir;
-}
-
 static bool
 win_block_dns_service(bool add, int index, const HANDLE pipe)
 {
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 3605966..aa8513b 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -286,9 +286,6 @@
 /* call self in a subprocess */
 void fork_to_self(const char *cmdline);

-/* Find temporary directory */
-const char *win_get_tempdir(void);
-
 bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);

 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-08 Thread flichtenheld (Code Review) 
Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..


Patch Set 2: Code-Review+2

(1 comment)

Patchset:

PS2:
Looks good to me. I verified that this does not break the Windows client tests.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Comment-Date: Mon, 08 Jan 2024 17:03:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-06 Thread cron2 (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..


Patch Set 2: Code-Review+1

(1 comment)

Patchset:

PS2:
Looks good to me (just browsed through, not actually tested).  Frank?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Sat, 06 Jan 2024 16:26:28 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-02 Thread plaisthos (Code Review) 
Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..


Patch Set 2:

(2 comments)

File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/481/comment/0aedaf28_cfeef61a :
PS1, Line 891: /* Warn if we can't find a valid temporary directory, 
which should
> M_USAGE is not really "Warn"
Done


File src/openvpn/win32-util.h:

http://gerrit.openvpn.net/c/openvpn/+/481/comment/fc97a5eb_3cd9a134 :
PS1, Line 44: const char *win_get_tempdir(void);
> corresponding removal from win32. […]
yes. Fixed.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 02 Jan 2024 16:52:39 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-02 Thread plaisthos (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/481?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld


Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..

Move get_tmp_dir to win32-util.c and error out on failure

Currently we only warn in get_tmp_dir fails and set o->tmp_dir to
a null pointer. This will not be caught by check_file_access_chroot
either since that ignores NULL pointers but other parts of OpenVPN
will assume that tmp_dir is set to a non-NULL string.

Also move get_tmp_dir to ssl-utils.c to use it in unit tests.

Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Signed-off-by: Arne Schwabe 
---
M src/openvpn/options.c
M src/openvpn/win32-util.c
M src/openvpn/win32-util.h
M src/openvpn/win32.c
M src/openvpn/win32.h
5 files changed, 34 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/81/481/2

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..79958db 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -885,7 +885,15 @@
 #ifdef _WIN32
 /* On Windows, find temp dir via environment variables */
 o->tmp_dir = win_get_tempdir();
-#else
+
+if (!o->tmp_dir)
+{
+/* Error out if we can't find a valid temporary directory, which should
+ * be very unlikely. */
+msg(M_USAGE, "Could not find a suitable temporary directory."
+" (GetTempPath() failed).  Consider using --tmp-dir");
+}
+#else  /* ifdef _WIN32 */
 /* Non-windows platforms use $TMPDIR, and if not set, default to '/tmp' */
 o->tmp_dir = getenv("TMPDIR");
 if (!o->tmp_dir)
diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
index 81e504a..c5e7505 100644
--- a/src/openvpn/win32-util.c
+++ b/src/openvpn/win32-util.c
@@ -147,4 +147,26 @@
 }
 return true;
 }
+
+const char *
+win_get_tempdir(void)
+{
+static char tmpdir[MAX_PATH];
+WCHAR wtmpdir[MAX_PATH];
+
+if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
+{
+return NULL;
+}
+
+if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
+{
+msg(M_WARN, "Could not get temporary directory. Path is too long."
+"  Consider using --tmp-dir");
+return NULL;
+}
+
+WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
+return tmpdir;
+}
 #endif /* _WIN32 */
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
index ac37979..98bf74b 100644
--- a/src/openvpn/win32-util.h
+++ b/src/openvpn/win32-util.h
@@ -40,5 +40,8 @@
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);

+/* Find temporary directory */
+const char *win_get_tempdir(void);
+
 #endif /* OPENVPN_WIN32_UTIL_H */
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index e998d90..6b7ba5e 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1137,34 +1137,6 @@
 set_win_sys_path(buf, es);
 }

-
-const char *
-win_get_tempdir(void)
-{
-static char tmpdir[MAX_PATH];
-WCHAR wtmpdir[MAX_PATH];
-
-if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
-{
-/* Warn if we can't find a valid temporary directory, which should
- * be unlikely.
- */
-msg(M_WARN, "Could not find a suitable temporary directory."
-" (GetTempPath() failed).  Consider using --tmp-dir");
-return NULL;
-}
-
-if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
-{
-msg(M_WARN, "Could not get temporary directory. Path is too long."
-"  Consider using --tmp-dir");
-return NULL;
-}
-
-WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
-return tmpdir;
-}
-
 static bool
 win_block_dns_service(bool add, int index, const HANDLE pipe)
 {
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 3605966..aa8513b 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -286,9 +286,6 @@
 /* call self in a subprocess */
 void fork_to_self(const char *cmdline);

-/* Find temporary directory */
-const char *win_get_tempdir(void);
-
 bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);

 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2023-12-13 Thread flichtenheld (Code Review) 
Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..


Patch Set 1: Code-Review-1

(2 comments)

File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/481/comment/466d524c_f6e1e499 :
PS1, Line 891: /* Warn if we can't find a valid temporary directory, 
which should
M_USAGE is not really "Warn"


File src/openvpn/win32-util.h:

http://gerrit.openvpn.net/c/openvpn/+/481/comment/14243ada_3a632f04 :
PS1, Line 44: const char *win_get_tempdir(void);
corresponding removal from win32.h is missing?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Comment-Date: Wed, 13 Dec 2023 14:57:14 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2023-12-13 Thread plaisthos (Code Review) 
Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..


Patch Set 1:

(1 comment)

Patchset:

PS1:
Note that we will actually not be broken when tmp_dir is NULL but you end up 
with tmporary files being created in whatever the current directory is.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Wed, 13 Dec 2023 14:25:28 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2023-12-13 Thread plaisthos (Code Review) 
Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

http://gerrit.openvpn.net/c/openvpn/+/481?usp=email

to review the following change.


Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..

Move get_tmp_dir to win32-util.c and error out on failure

Currently we only warn in get_tmp_dir fails and set o->tmp_dir to
a null pointer. This will not be caught by check_file_access_chroot
either since that ignores NULL pointers but other parts of OpenVPN
will assume that tmp_dir is set to a non-NULL string.

Also move get_tmp_dir to ssl-utils.c to use it in unit tests.

Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Signed-off-by: Arne Schwabe 
---
M src/openvpn/options.c
M src/openvpn/win32-util.c
M src/openvpn/win32-util.h
M src/openvpn/win32.c
4 files changed, 34 insertions(+), 29 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/81/481/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 503e832..9863261 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -885,7 +885,15 @@
 #ifdef _WIN32
 /* On Windows, find temp dir via environment variables */
 o->tmp_dir = win_get_tempdir();
-#else
+
+if (!o->tmp_dir)
+{
+/* Warn if we can't find a valid temporary directory, which should
+ * be unlikely. */
+msg(M_USAGE, "Could not find a suitable temporary directory."
+" (GetTempPath() failed).  Consider using --tmp-dir");
+}
+#else  /* ifdef _WIN32 */
 /* Non-windows platforms use $TMPDIR, and if not set, default to '/tmp' */
 o->tmp_dir = getenv("TMPDIR");
 if (!o->tmp_dir)
diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
index 81e504a..c5e7505 100644
--- a/src/openvpn/win32-util.c
+++ b/src/openvpn/win32-util.c
@@ -147,4 +147,26 @@
 }
 return true;
 }
+
+const char *
+win_get_tempdir(void)
+{
+static char tmpdir[MAX_PATH];
+WCHAR wtmpdir[MAX_PATH];
+
+if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
+{
+return NULL;
+}
+
+if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
+{
+msg(M_WARN, "Could not get temporary directory. Path is too long."
+"  Consider using --tmp-dir");
+return NULL;
+}
+
+WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
+return tmpdir;
+}
 #endif /* _WIN32 */
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
index ac37979..98bf74b 100644
--- a/src/openvpn/win32-util.h
+++ b/src/openvpn/win32-util.h
@@ -40,5 +40,8 @@
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);

+/* Find temporary directory */
+const char *win_get_tempdir(void);
+
 #endif /* OPENVPN_WIN32_UTIL_H */
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index e998d90..6b7ba5e 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1137,34 +1137,6 @@
 set_win_sys_path(buf, es);
 }

-
-const char *
-win_get_tempdir(void)
-{
-static char tmpdir[MAX_PATH];
-WCHAR wtmpdir[MAX_PATH];
-
-if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
-{
-/* Warn if we can't find a valid temporary directory, which should
- * be unlikely.
- */
-msg(M_WARN, "Could not find a suitable temporary directory."
-" (GetTempPath() failed).  Consider using --tmp-dir");
-return NULL;
-}
-
-if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
-{
-msg(M_WARN, "Could not get temporary directory. Path is too long."
-"  Consider using --tmp-dir");
-return NULL;
-}
-
-WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
-return tmpdir;
-}
-
 static bool
 win_block_dns_service(bool add, int index, const HANDLE pipe)
 {

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-MessageType: newchange
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel