Improve error reporting on file access to --client-config-dir and --ccd-exclusive
PATCH 1/1 Improve error reporting on file access to --client-config-dir and --ccd-exclusive
From: David Sommerseth <[email protected]...>
OpenVPN will do some simple sanity checking at startup to ensure the expected files and directories is in place. However, with --client-config-dir and --ccd-exclusive, things are slightly different. In both cases it is perfectly fine that files does not exists, and we cannot know any file names beforehand due to these filenames being based upon the certificate's CN field.
The problem arises when OpenVPN cannot open files inside a directory because the directory permissions are too restrictive, have wrong ownership (triggered by the usage of --user/--group) or other security mechanisms the OS uses.
When a client connects, the test_file() function is used to check if a client config file has been prepared. And if not, it continues without trying to read it. So, if the privileges of the running OpenVPN process is not allowed to open and read an existing file, OpenVPN will treat this as a non-existing file without saying anything. This is clearly wrong. So this patch adds an warning message in the OpenVPN log if it could not open the file due to lack of permissions.
This will work fine on all *nix based OSes. Windows however reports 'no such file or directory' (errno=2/-ENOENT) even on privilege access errors when the directory this file resides is too restrictive. But there is no easy way to work around that. However, I believe that the initial sanity checking at startup will catch that one, as it will check if the directories it needs exists.
This patch has not been thoroughly tested, so please test!
Trac: #277
Trac-URL: https://community.openvpn.net/openvpn/ticket/277
--- src/openvpn/misc.c | 6 ++++++ 1 file changed, 6 insertions(+)
src/openvpn/misc.c +6 -0
@@ -861,6 +861,12 @@ test_file (const char *filename)fclose (fp); ret = true; } +else + { +if ( errno ==EACCES ) { +msg (M_WARN |M_ERRNO ,"Could not access file '%s'" , filename); + } + } } dmsg (D_TEST_FILE, "TEST FILE '%s' [%d]",
Signature · 1 line
1.8.3.1
Hi,
Quoted 7 lines · On 16-04-14 17:43, [email protected]... wrote:
When a client connects, the
test_file()function is used to check if a client config file has been prepared. And if not, it continues without trying to read it. So, if the privileges of the running OpenVPN process is not allowed to open and read an existing file, OpenVPN will treat this as a non-existing file without saying anything. This is clearly wrong. So this patch adds an warning message in the OpenVPN log if it could not open the file due to lack of permissions.
Makes sense. Feature ACK.
Quoted 6 lines
This will work fine on all *nix based OSes. Windows however reports 'no such file or directory' (errno=2/-ENOENT) even on privilege access errors when the directory this file resides is too restrictive. But there is no easy way to work around that. However, I believe that the initial sanity checking at startup will catch that one, as it will check if the directories it needs exists.
I see you're using errno for Windows builds. OpenVPN also has openvpn_errno(), which expands to errno on *nix OSes, but to GetLastError() for Windows. I'm not familiar with the details, but maybe using openvpn_errno() fixes the Windows ENOENT?
-Steffan
PATCH v2 1/1 Improve error reporting on file access to --client-config-dir and --ccd-exclusive
In reply to [email protected]...OpenVPN will do some simple sanity checking at startup to ensure the expected files and directories is in place. However, with --client-config-dir and --ccd-exclusive, things are slightly different. In both cases it is perfectly fine that files does not exists, and we cannot know any file names beforehand due to these filenames being based upon the certificate's CN field.
The problem arises when OpenVPN cannot open files inside a directory because the directory permissions are too restrictive, have wrong ownership (triggered by the usage of --user/--group) or other security mechanisms the OS uses.
When a client connects, the test_file() function is used to check if a client config file has been prepared. And if not, it continues without trying to read it. So, if the privileges of the running OpenVPN process is not allowed to open and read an existing file, OpenVPN will treat this as a non-existing file without saying anything. This is clearly wrong. So this patch adds an warning message in the OpenVPN log if it could not open the file due to lack of permissions.
This will work fine on all *nix based OSes. Windows however reports 'no such file or directory' (errno=2/-ENOENT) even on privilege access errors when the directory this file resides is too restrictive. But there is no easy way to work around that. However, I believe that the initial sanity checking at startup will catch that one, as it will check if the directories it needs exists.
This patch has only gone through simple basic testing, with both too few privileges and with proper privileges to the CCD directory. With wrong privileges, the following error can be found if CN=Test client
Fri May 2 00:00:10 2014 us=281993 127.0.0.1:41017 Could not access file '/etc/clients/Test client': Permission denied (errno=13)
[v2 - use openvpn_errno() instead of errno, for better platform support]
Trac: #277
Trac-URL: https://community.openvpn.net/openvpn/ticket/277
--- src/openvpn/misc.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
src/openvpn/misc.c +6 -0
@@ -861,6 +861,12 @@ test_file (const char *filename)fclose (fp); ret = true; } +else + { +if (openvpn_errno () ==EACCES ) { +msg (M_WARN |M_ERRNO ,"Could not access file '%s'" , filename); + } + } } dmsg (D_TEST_FILE, "TEST FILE '%s' [%d]",
Signature · 1 line
1.7.1
ACK
Sorry it took so long. Patch looks good, and works as expected for me.
-Steffan
Quoted 58 lines · David Sommerseth
OpenVPN will do some simple sanity checking at startup to ensure the expected files and directories is in place. However, with --client-config-dir and --ccd-exclusive, things are slightly different. In both cases it is perfectly fine that files does not exists, and we cannot know any file names beforehand due to these filenames being based upon the certificate's CN field.
The problem arises when OpenVPN cannot open files inside a directory because the directory permissions are too restrictive, have wrong ownership (triggered by the usage of --user/--group) or other security mechanisms the OS uses.
When a client connects, the
test_file()function is used to check if a client config file has been prepared. And if not, it continues without trying to read it. So, if the privileges of the running OpenVPN process is not allowed to open and read an existing file, OpenVPN will treat this as a non-existing file without saying anything. This is clearly wrong. So this patch adds an warning message in the OpenVPN log if it could not open the file due to lack of permissions.This will work fine on all *nix based OSes. Windows however reports 'no such file or directory' (errno=2/-ENOENT) even on privilege access errors when the directory this file resides is too restrictive. But there is no easy way to work around that. However, I believe that the initial sanity checking at startup will catch that one, as it will check if the directories it needs exists.
This patch has only gone through simple basic testing, with both too few privileges and with proper privileges to the CCD directory. With wrong privileges, the following error can be found if CN=Test client
Fri May 2 00:00:10 2014 us=281993 127.0.0.1:41017 Could not access file '
/etc/clients/Testclient': Permission denied (errno=13)[v2 - use
openvpn_errno()instead of errno, for better platform support]Trac: #277
Trac-URL: https://community.openvpn.net/openvpn/ticket/277Signed-off-by David Sommerseth <[email protected]...>--- src/openvpn/misc.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)src/openvpn/misc.c +6
-0@@ -861,6 +861,12 @@ test_file (const char *filename)fclose (fp); ret = true; } +else + { +if (openvpn_errno () ==EACCES ) { +msg (M_WARN |M_ERRNO ,"Could not access file '%s'" , filename); + } + } } dmsg (D_TEST_FILE, "TEST FILE '%s' [%d]",
Your patch has been applied to the master and release/2.3 branches.
commit 4978dadaed4ecf1b9dd256f57c6a5c895691580b (master) commit 488994b3f65242ec31b015f53e21d935f75b8bee (release/2.3)
Author: David Sommerseth Date: Fri May 2 02:28:24 2014 +0200
Improve error reporting on file access to --client-config-dir and --ccd-exclusive
Trac-URL: https://community.openvpn.net/openvpn/ticket/277
Signed-off-by: David Sommerseth <[email protected]...>
Acked-by: Steffan Karger <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8688
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering