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.

diff --git a/src/control.c b/src/control.c
index ad2a725..84098be 100644 (file)
--- 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 e5085ce..2dccd8f 100644 (file)
--- 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 334ea5d..0fedafa 100644 (file)
--- 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 2fe4e79..55e14e5 100644 (file)
--- 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 <your vpn IP address> netmask <netmask of whole VPN>\n");
                fclose(f);
        }

diff --git a/src/tincctl.h b/src/tincctl.h
index aa840ce..920b5a2 100644 (file)
--- 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