From a38e0d621397d6d69c939ccc287d5a803b668195 Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Fri, 2 Aug 2013 19:27:06 +0200 Subject: [PATCH] Use umask() to set file and UNIX socket permissions without race conditions. As mentioned by Erik Tews, calling fchmod() after fopen() leaves a small window for exploits. As long as tinc is single-threaded, we can use umask() instead to reduce file permissions. This also works when creating the AF_UNIX control socket. The umask of the user running tinc(d) is used for most files, except for the private keys, invitation files, PID file and control socket. --- src/control.c | 9 +++---- src/invitation.c | 25 ++++++++---------- src/net_setup.c | 7 ++++- src/tincctl.c | 66 +++++++++++++++++++++++------------------------- src/tincctl.h | 1 + 5 files changed, 52 insertions(+), 56 deletions(-) diff --git a/src/control.c b/src/control.c index ad2a7256..84098be5 100644 --- a/src/control.c +++ b/src/control.c @@ -137,17 +137,16 @@ bool init_control(void) { randomize(controlcookie, sizeof controlcookie / 2); bin2hex(controlcookie, controlcookie, sizeof controlcookie / 2); + mode_t mask = umask(0); + umask(mask | 077); FILE *f = fopen(pidfilename, "w"); + umask(mask); + if(!f) { logger(DEBUG_ALWAYS, LOG_ERR, "Cannot write control socket cookie file %s: %s", pidfilename, strerror(errno)); return false; } -#ifdef HAVE_FCHMOD - fchmod(fileno(f), 0600); -#else - chmod(pidfilename, 0600); -#endif // Get the address and port of the first listening socket char *localhost = NULL; diff --git a/src/invitation.c b/src/invitation.c index e5085cea..2dccd8fb 100644 --- a/src/invitation.c +++ b/src/invitation.c @@ -519,12 +519,12 @@ make_names: goto make_names; } - if(mkdir(confbase, 0755) && errno != EEXIST) { + if(mkdir(confbase, 0777) && errno != EEXIST) { fprintf(stderr, "Could not create directory %s: %s\n", confbase, strerror(errno)); return false; } - if(mkdir(hosts_dir, 0755) && errno != EEXIST) { + if(mkdir(hosts_dir, 0777) && errno != EEXIST) { fprintf(stderr, "Could not create directory %s: %s\n", hosts_dir, strerror(errno)); return false; } @@ -652,12 +652,7 @@ make_names: return false; xasprintf(&filename, "%s" SLASH "ecdsa_key.priv", confbase); - f = fopen(filename, "w"); - -#ifdef HAVE_FCHMOD - /* Make it unreadable for others. */ - fchmod(fileno(f), 0600); -#endif + f = fopenmask(filename, "w", 0600); if(!ecdsa_write_pem_private_key(key, f)) { fprintf(stderr, "Error writing private key!\n"); @@ -676,12 +671,7 @@ make_names: rsa_t *rsa = rsa_generate(2048, 0x1001); xasprintf(&filename, "%s" SLASH "rsa_key.priv", confbase); - f = fopen(filename, "w"); - -#ifdef HAVE_FCHMOD - /* Make it unreadable for others. */ - fchmod(fileno(f), 0600); -#endif + f = fopenmask(filename, "w", 0600); rsa_write_pem_private_key(rsa, f); fclose(f); @@ -772,7 +762,12 @@ int cmd_join(int argc, char *argv[]) { } // Make sure confbase exists and is accessible. - if(mkdir(confbase, 0755) && errno != EEXIST) { + if(strcmp(confdir, confbase) && mkdir(confdir, 0755) && errno != EEXIST) { + fprintf(stderr, "Could not create directory %s: %s\n", confdir, strerror(errno)); + return 1; + } + + if(mkdir(confbase, 0777) && errno != EEXIST) { fprintf(stderr, "Could not create directory %s: %s\n", confbase, strerror(errno)); return 1; } diff --git a/src/net_setup.c b/src/net_setup.c index 334ea5d1..0fedafa8 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -868,7 +868,12 @@ static bool setup_myself(void) { unlink(unixsocketname); - if(bind(unix_fd, (struct sockaddr *)&sa, sizeof sa) < 0) { + mode_t mask = umask(0); + umask(mask | 077); + int result = bind(unix_fd, (struct sockaddr *)&sa, sizeof sa); + umask(mask); + + if(result < 0) { logger(DEBUG_ALWAYS, LOG_ERR, "Could not bind UNIX socket to %s: %s", unixsocketname, sockstrerror(errno)); return false; } diff --git a/src/tincctl.c b/src/tincctl.c index 2fe4e790..55e14e53 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -211,6 +211,23 @@ static bool parse_options(int argc, char **argv) { return true; } +/* Open a file with the desired permissions, minus the umask. + Also, if we want to create an executable file, we call fchmod() + to set the executable bits. */ + +FILE *fopenmask(const char *filename, const char *mode, mode_t perms) { + mode_t mask = umask(0); + perms &= ~mask; + umask(~perms); + FILE *f = fopen(filename, mode); +#ifdef HAVE_FCHMOD + if((perms & 0444) && f) + fchmod(fileno(f), perms); +#endif + umask(mask); + return f; +} + static void disable_old_keys(const char *filename, const char *what) { char tmpfile[PATH_MAX] = ""; char buf[1024]; @@ -225,17 +242,9 @@ static void disable_old_keys(const char *filename, const char *what) { snprintf(tmpfile, sizeof tmpfile, "%s.tmp", filename); - w = fopen(tmpfile, "w"); - -#ifdef HAVE_FCHMOD - /* Let the temporary file have the same permissions as the original. */ - - if(w) { - struct stat st = {.st_mode = 0600}; - fstat(fileno(r), &st); - fchmod(fileno(w), st.st_mode); - } -#endif + struct stat st = {.st_mode = 0600}; + fstat(fileno(r), &st); + w = fopenmask(tmpfile, "w", st.st_mode); while(fgets(buf, sizeof buf, r)) { if(!block && !strncmp(buf, "-----BEGIN ", 11)) { @@ -298,7 +307,7 @@ static void disable_old_keys(const char *filename, const char *what) { unlink(tmpfile); } -static FILE *ask_and_open(const char *filename, const char *what, const char *mode, bool ask) { +static FILE *ask_and_open(const char *filename, const char *what, const char *mode, bool ask, mode_t perms) { FILE *r; char *directory; char buf[PATH_MAX]; @@ -338,7 +347,7 @@ static FILE *ask_and_open(const char *filename, const char *what, const char *mo /* Open it first to keep the inode busy */ - r = fopen(filename, mode); + r = fopenmask(filename, mode, perms); if(!r) { fprintf(stderr, "Error opening file `%s': %s\n", filename, strerror(errno)); @@ -366,17 +375,12 @@ static bool ecdsa_keygen(bool ask) { fprintf(stderr, "Done.\n"); xasprintf(&privname, "%s" SLASH "ecdsa_key.priv", confbase); - f = ask_and_open(privname, "private ECDSA key", "a", ask); + f = ask_and_open(privname, "private ECDSA key", "a", ask, 0600); free(privname); if(!f) return false; -#ifdef HAVE_FCHMOD - /* Make it unreadable for others. */ - fchmod(fileno(f), 0600); -#endif - if(!ecdsa_write_pem_private_key(key, f)) { fprintf(stderr, "Error writing private key!\n"); ecdsa_free(key); @@ -391,7 +395,7 @@ static bool ecdsa_keygen(bool ask) { else xasprintf(&pubname, "%s" SLASH "ecdsa_key.pub", confbase); - f = ask_and_open(pubname, "public ECDSA key", "a", ask); + f = ask_and_open(pubname, "public ECDSA key", "a", ask, 0666); free(pubname); if(!f) @@ -425,17 +429,12 @@ static bool rsa_keygen(int bits, bool ask) { fprintf(stderr, "Done.\n"); xasprintf(&privname, "%s" SLASH "rsa_key.priv", confbase); - f = ask_and_open(privname, "private RSA key", "a", ask); + f = ask_and_open(privname, "private RSA key", "a", ask, 0600); free(privname); if(!f) return false; -#ifdef HAVE_FCHMOD - /* Make it unreadable for others. */ - fchmod(fileno(f), 0600); -#endif - if(!rsa_write_pem_private_key(key, f)) { fprintf(stderr, "Error writing private key!\n"); fclose(f); @@ -450,7 +449,7 @@ static bool rsa_keygen(int bits, bool ask) { else xasprintf(&pubname, "%s" SLASH "rsa_key.pub", confbase); - f = ask_and_open(pubname, "public RSA key", "a", ask); + f = ask_and_open(pubname, "public RSA key", "a", ask, 0666); free(pubname); if(!f) @@ -1756,17 +1755,17 @@ static int cmd_init(int argc, char *argv[]) { return 1; } - if(mkdir(confdir, 0755) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", CONFDIR, strerror(errno)); + if(strcmp(confdir, confbase) && mkdir(confdir, 0755) && errno != EEXIST) { + fprintf(stderr, "Could not create directory %s: %s\n", confdir, strerror(errno)); return 1; } - if(mkdir(confbase, 0755) && errno != EEXIST) { + if(mkdir(confbase, 0777) && errno != EEXIST) { fprintf(stderr, "Could not create directory %s: %s\n", confbase, strerror(errno)); return 1; } - if(mkdir(hosts_dir, 0755) && errno != EEXIST) { + if(mkdir(hosts_dir, 0777) && errno != EEXIST) { fprintf(stderr, "Could not create directory %s: %s\n", hosts_dir, strerror(errno)); return 1; } @@ -1789,14 +1788,11 @@ static int cmd_init(int argc, char *argv[]) { char *filename; xasprintf(&filename, "%s" SLASH "tinc-up", confbase); if(access(filename, F_OK)) { - FILE *f = fopen(filename, "w"); + FILE *f = fopenmask(filename, "w", 0777); if(!f) { fprintf(stderr, "Could not create file %s: %s\n", filename, strerror(errno)); return 1; } - mode_t mask = umask(0); - umask(mask); - fchmod(fileno(f), 0755 & ~mask); fprintf(f, "#!/bin/sh\n\necho 'Unconfigured tinc-up script, please edit!'\n\n#ifconfig $INTERFACE netmask \n"); fclose(f); } diff --git a/src/tincctl.h b/src/tincctl.h index aa840cec..920b5a20 100644 --- a/src/tincctl.h +++ b/src/tincctl.h @@ -48,6 +48,7 @@ extern bool connect_tincd(bool verbose); extern bool sendline(int fd, char *format, ...); extern bool recvline(int fd, char *line, size_t len); extern int check_port(char *name); +extern FILE *fopenmask(const char *filename, const char *mode, mode_t perms); #endif -- 2.20.1