diff mbox series

[1/2] Create debug log file with more sane file permissions

Message ID 20181011202010.29226-1-karol@babioch.de
State Superseded
Headers show
Series [1/2] Create debug log file with more sane file permissions | expand

Commit Message

Karol Babioch Oct. 11, 2018, 8:20 p.m. UTC
Previously the file permissions for the debug log file were not explicitly set.
Instead it was implicitly relying on a secure umask, which in most cases would
result in a file that is world-readable. This is a violation of good
practices, since not very user of a file should have access to sensitive
information that might be contained in the debug log file.

This commit will explicitly set sane default file permissions in case
the file is newly created.

Unfortunately the fopen(3) function does not provide such a facility, so the
approach needs to be changed in the following way:

1.) The file descriptor needs to be created manually using the open(3)
function with the correct flags and the desired mode set.

2.) fdopen(3) can then be used on the file descriptor to associate a
file stream with it.

Note: This modification will not change the file permissions of any already
existing debug log files, and only applies to newly created ones.

Signed-off-by: Karol Babioch <karol@babioch.de>
---
 src/utils/wpa_debug.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Jan Ceuleers Oct. 12, 2018, 7:02 a.m. UTC | #1
On 11/10/18 22:20, Karol Babioch wrote:
>  src/utils/wpa_debug.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/utils/wpa_debug.c b/src/utils/wpa_debug.c
> index 62758d864..b412f88e3 100644
> --- a/src/utils/wpa_debug.c
> +++ b/src/utils/wpa_debug.c
> @@ -58,6 +58,10 @@ static int wpa_to_android_level(int level)
>  #ifndef CONFIG_NO_STDOUT_DEBUG
>  
>  #ifdef CONFIG_DEBUG_FILE
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +static int out_fd = -1;

Does out_fd have to be a global variable given that it is only used in
wpa_debug_open?
Karol Babioch Oct. 12, 2018, 8:31 a.m. UTC | #2
Hi Jan,

Am 12.10.18 um 09:02 schrieb Jan Ceuleers:
> Does out_fd have to be a global variable given that it is only used in
> wpa_debug_open?

Yup, you are right. For now this does not need to be exposed beyond the
scope of the function, so I will resend this patch with a local-scoped
variable.

Thanks for the input.

Best regards,
Karol Babioch
diff mbox series

Patch

diff --git a/src/utils/wpa_debug.c b/src/utils/wpa_debug.c
index 62758d864..b412f88e3 100644
--- a/src/utils/wpa_debug.c
+++ b/src/utils/wpa_debug.c
@@ -58,6 +58,10 @@  static int wpa_to_android_level(int level)
 #ifndef CONFIG_NO_STDOUT_DEBUG
 
 #ifdef CONFIG_DEBUG_FILE
+#include <sys/types.h>
+#include <sys/stat.h>
+
+static int out_fd = -1;
 static FILE *out_file = NULL;
 #endif /* CONFIG_DEBUG_FILE */
 
@@ -548,12 +552,21 @@  int wpa_debug_open_file(const char *path)
 		last_path = os_strdup(path);
 	}
 
-	out_file = fopen(path, "a");
+	out_fd = open(path, O_CREAT | O_APPEND, S_IRUSR | S_IWUSR | S_IRGRP);
+	if (out_fd < 0) {
+		wpa_printf(MSG_ERROR, "wpa_debug_open_file: Failed to open "
+			    "output file descriptor, using standard output");
+		return -1;
+	}
+
+	out_file = fdopen(out_fd, "a");
 	if (out_file == NULL) {
 		wpa_printf(MSG_ERROR, "wpa_debug_open_file: Failed to open "
 			   "output file, using standard output");
+		close(out_fd);
 		return -1;
 	}
+
 #ifndef _WIN32
 	setvbuf(out_file, NULL, _IOLBF, 0);
 #endif /* _WIN32 */