From e3a78a175c664ee51de1fb8086deb96a1d017ac3 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sat, 1 Sep 2012 21:55:52 -0400 Subject: [PATCH] verify_file_perms_ownership() to just return if the file doesn't exist --- client/config_init.c | 2 +- client/fwknop.c | 4 ++-- client/utils.c | 23 ++++++++++++++++------- server/fwknopd.c | 3 ++- server/utils.c | 25 +++++++++++++++++-------- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/client/config_init.c b/client/config_init.c index 5c001fc..e3c7e31 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -448,7 +448,7 @@ process_rc(fko_cli_options_t *options) strlcat(rcfile, ".fwknoprc", MAX_PATH_LEN); /* Check rc file permissions - if anything other than user read/write, - * then don't process it. This change was made to help ensure that the + * then throw a warning. This change was made to help ensure that the * client consumes a proper rc file with strict permissions set (thanks * to Fernando Arnaboldi from IOActive for pointing this out). */ diff --git a/client/fwknop.c b/client/fwknop.c index d3096ad..1beb66e 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -559,12 +559,12 @@ show_last_command(void) #endif if (get_save_file(args_save_file)) { + verify_file_perms_ownership(args_save_file); if ((args_file_ptr = fopen(args_save_file, "r")) == NULL) { fprintf(stderr, "Could not open args file: %s\n", args_save_file); exit(EXIT_FAILURE); } - verify_file_perms_ownership(args_save_file); if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL) { printf("Last fwknop client command line: %s", args_str); } else { @@ -602,13 +602,13 @@ run_last_args(fko_cli_options_t *options) if (get_save_file(args_save_file)) { + verify_file_perms_ownership(args_save_file); if ((args_file_ptr = fopen(args_save_file, "r")) == NULL) { fprintf(stderr, "Could not open args file: %s\n", args_save_file); exit(EXIT_FAILURE); } - verify_file_perms_ownership(args_save_file); if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL) { args_str[MAX_LINE_LEN-1] = '\0'; diff --git a/client/utils.c b/client/utils.c index 02718ce..e40115b 100644 --- a/client/utils.c +++ b/client/utils.c @@ -89,6 +89,7 @@ set_file_perms(const char *file) int verify_file_perms_ownership(const char *file) { + int res = 1; #if HAVE_STAT struct stat st; @@ -97,9 +98,17 @@ verify_file_perms_ownership(const char *file) */ if((stat(file, &st)) != 0) { - fprintf(stderr, "[-] unable to run stat() against file: %s: %s\n", - file, strerror(errno)); - exit(EXIT_FAILURE); + /* if the path doesn't exist, just return, but otherwise something + * went wrong + */ + if(errno == ENOENT) + { + return 0; + } else { + fprintf(stderr, "[-] stat() against file: %s returned: %s\n", + file, strerror(errno)); + exit(EXIT_FAILURE); + } } /* Make sure it is a regular file or symbolic link @@ -110,7 +119,7 @@ verify_file_perms_ownership(const char *file) "[-] file: %s is not a regular file or symbolic link.\n", file ); - return 0; + res = 0; } if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR)) @@ -119,18 +128,18 @@ verify_file_perms_ownership(const char *file) "[-] file: %s permissions should only be user read/write (0600, -rw-------)\n", file ); - return 0; + res = 0; } if(st.st_uid != getuid()) { fprintf(stderr, "[-] file: %s not owned by current effective user id.\n", file); - return 0; + res = 0; } #endif - return 1; + return res; } /***EOF***/ diff --git a/server/fwknopd.c b/server/fwknopd.c index f66ef81..eabd5e7 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -677,11 +677,12 @@ get_running_pid(const fko_srv_options_t *opts) pid_t rpid = 0; + verify_file_perms_ownership(opts->config[CONF_FWKNOP_PID_FILE]); + op_fd = open(opts->config[CONF_FWKNOP_PID_FILE], O_RDONLY); if(op_fd > 0) { - verify_file_perms_ownership(opts->config[CONF_FWKNOP_PID_FILE]); if (read(op_fd, buf, PID_BUFLEN) > 0) { buf[PID_BUFLEN-1] = '\0'; diff --git a/server/utils.c b/server/utils.c index 4463607..aa24940 100644 --- a/server/utils.c +++ b/server/utils.c @@ -184,17 +184,26 @@ set_file_perms(const char *file) int verify_file_perms_ownership(const char *file) { + int res = 1; #if HAVE_STAT struct stat st; - /* Every file that the fwknop client deals with should be owned + /* Every file that fwknopd deals with should be owned * by the user and permissions set to 600 (user read/write) */ if((stat(file, &st)) != 0) { - fprintf(stderr, "[-] unable to stat() file: %s: %s\n", - file, strerror(errno)); - exit(EXIT_FAILURE); + /* if the path doesn't exist, just return, but otherwise something + * went wrong + */ + if(errno == ENOENT) + { + return 0; + } else { + fprintf(stderr, "[-] stat() against file: %s returned: %s\n", + file, strerror(errno)); + exit(EXIT_FAILURE); + } } /* Make sure it is a regular file @@ -205,7 +214,7 @@ verify_file_perms_ownership(const char *file) "[-] file: %s is not a regular file or symbolic link.\n", file ); - return 0; + res = 0; } if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR)) @@ -214,18 +223,18 @@ verify_file_perms_ownership(const char *file) "[-] file: %s permissions should only be user read/write (0600, -rw-------)\n", file ); - return 0; + res = 0; } if(st.st_uid != getuid()) { fprintf(stderr, "[-] file: %s not owned by current effective user id\n", file); - return 0; + res = 0; } #endif - return 1; + return res; } /* Determine if a buffer contains only characters from the base64 -- 1.7.5.4