diff --git a/CREDITS b/CREDITS index 2de7281c..1a37ae97 100644 --- a/CREDITS +++ b/CREDITS @@ -36,6 +36,11 @@ Franck Joncourt the local directory (if it exists) so that it doesn't have to have libfko completely installed in /usr/lib/. This allows the test suite to run FKO tests without installing libfko. + - Contributed a patch to remove unnecessary chmod() call when creating + client rc file and server replay cache file. The permissions are now set + appropriately via open(), and at the same time this patch fixes a + potential race condition since the previous code used fopen() followed by + chmod(). Jonathan Schulz - Submitted patches to change HTTP connection type to 'close' for -R mode diff --git a/ChangeLog b/ChangeLog index b7762d6e..87efa35c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,11 @@ fwknop-2.0.4 (09/20/2012): modes. - [libfko] Restricted usernames embedded in SPA packets to be alpha-numeric along with "-" chars. + - [client+server] Applied patch from Franck Joncourt to remove unnecessary + chmod() call when creating client rc file and server replay cache file. + The permissions are now set appropriately via open(), and at the same + time this patch fixes a potential race condition since the previous code + used fopen() followed by chmod(). - [server] Bug fix to accept SPA packets over ICMP if the fwknop client is executed with '-P icmp' and the user has the required privileges. - [test suite] Applied patch from Franck Joncourt to have the perl FKO diff --git a/client/config_init.c b/client/config_init.c index 5f975602..49503206 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -33,6 +33,8 @@ #include "config_init.h" #include "cmd_opts.h" #include "utils.h" +#include +#include /* Convert a digest_type string to its integer value. */ @@ -132,13 +134,28 @@ parse_time_offset(const char *offset_str) static int create_fwknoprc(const char *rcfile) { - FILE *rc = NULL; + FILE *rc = NULL; + int rcfile_fd = -1; fprintf(stdout, "[*] Creating initial rc file: %s.\n", rcfile); + /* Try to create the initial rcfile with user read/write rights only. + * If the rcfile already exists, an error is returned */ + rcfile_fd = open(rcfile, O_WRONLY|O_CREAT|O_EXCL , S_IRUSR|S_IWUSR); + + // If an error occured ... + if (rcfile_fd == -1) { + fprintf(stderr, "Unable to create initial rc file: %s: %s\n", + rcfile, strerror(errno)); + return(-1); + } + + // Free the rcfile descriptor + close(rcfile_fd); + if ((rc = fopen(rcfile, "w")) == NULL) { - fprintf(stderr, "Unable to create rc file: %s: %s\n", + fprintf(stderr, "Unable to write default setup to rcfile: %s: %s\n", rcfile, strerror(errno)); return(-1); } @@ -218,8 +235,6 @@ create_fwknoprc(const char *rcfile) fclose(rc); - set_file_perms(rcfile); - return(0); } diff --git a/client/fwknop.c b/client/fwknop.c index 1beb66ee..fbe1e4a9 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -33,6 +33,8 @@ #include "spa_comm.h" #include "utils.h" #include "getpasswd.h" +#include +#include /* prototypes */ @@ -660,8 +662,7 @@ save_args(int argc, char **argv) { char args_save_file[MAX_PATH_LEN]; char args_str[MAX_LINE_LEN] = ""; - FILE *args_file_ptr = NULL; - int i = 0, args_str_len = 0; + int i = 0, args_str_len = 0, args_file_fd = -1; #ifdef WIN32 /* Not sure what the right thing is here on Win32, just return @@ -671,26 +672,31 @@ save_args(int argc, char **argv) #endif if (get_save_file(args_save_file)) { - if ((args_file_ptr = fopen(args_save_file, "w")) == NULL) { + args_file_fd = open(args_save_file, O_WRONLY|O_CREAT, S_IRUSR|S_IWUSR); + if (args_file_fd == -1) { fprintf(stderr, "Could not open args file: %s\n", args_save_file); exit(EXIT_FAILURE); } - for (i=0; i < argc; i++) { - args_str_len += strlen(argv[i]); - if (args_str_len >= MAX_PATH_LEN) { - fprintf(stderr, "argument string too long, exiting.\n"); - exit(EXIT_FAILURE); + else { + for (i=0; i < argc; i++) { + args_str_len += strlen(argv[i]); + if (args_str_len >= MAX_PATH_LEN) { + fprintf(stderr, "argument string too long, exiting.\n"); + exit(EXIT_FAILURE); + } + strlcat(args_str, argv[i], MAX_PATH_LEN); + strlcat(args_str, " ", MAX_PATH_LEN); } - strlcat(args_str, argv[i], MAX_PATH_LEN); - strlcat(args_str, " ", MAX_PATH_LEN); + strlcat(args_str, "\n", MAX_PATH_LEN); + if(write(args_file_fd, args_str, strlen(args_str)) + != strlen(args_str)) { + fprintf(stderr, + "warning, did not write expected number of bytes to args save file\n"); + } + close(args_file_fd); } - fprintf(args_file_ptr, "%s\n", args_str); - fclose(args_file_ptr); } - - set_file_perms(args_save_file); - return; } diff --git a/client/utils.c b/client/utils.c index e40115ba..d5b91807 100644 --- a/client/utils.c +++ b/client/utils.c @@ -68,24 +68,6 @@ hex_dump(const unsigned char *data, const int size) } } -int -set_file_perms(const char *file) -{ - int res = 0; - - res = chmod(file, S_IRUSR | S_IWUSR); - - if(res != 0) - { - fprintf(stderr, - "[-] unable to chmod file %s to user read/write (0600, -rw-------): %s\n", - file, - strerror(errno) - ); - } - return res; -} - int verify_file_perms_ownership(const char *file) { diff --git a/client/utils.h b/client/utils.h index 672b8f35..c951314e 100644 --- a/client/utils.h +++ b/client/utils.h @@ -45,7 +45,6 @@ /* Prototypes */ void hex_dump(const unsigned char *data, const int size); -int set_file_perms(const char *file); int verify_file_perms_ownership(const char *file); size_t strlcat(char *dst, const char *src, size_t siz); diff --git a/server/replay_cache.c b/server/replay_cache.c index 28a31f5f..0a141bcb 100644 --- a/server/replay_cache.c +++ b/server/replay_cache.c @@ -37,6 +37,8 @@ #include "fwknopd_errors.h" #include "utils.h" +#include +#include #include #if HAVE_LIBGDBM @@ -230,6 +232,8 @@ replay_file_cache_init(fko_srv_options_t *opts) char src_ip[INET_ADDRSTRLEN+1] = {0}; char dst_ip[INET_ADDRSTRLEN+1] = {0}; long int time_tmp; + int digest_file_fd = -1; + char digest_header[] = "#