Simpler checking of permissions on private RSA key and other fixes.
authorGuus Sliepen <guus@tinc-vpn.org>
Fri, 8 Aug 2003 22:11:54 +0000 (22:11 +0000)
committerGuus Sliepen <guus@tinc-vpn.org>
Fri, 8 Aug 2003 22:11:54 +0000 (22:11 +0000)
src/conf.c
src/conf.h
src/meta.c
src/net_setup.c
src/process.c
src/tincd.c

index e927abd..3feb150 100644 (file)
@@ -19,7 +19,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-    $Id: conf.c,v 1.9.4.74 2003/08/08 14:59:27 guus Exp $
+    $Id: conf.c,v 1.9.4.75 2003/08/08 22:11:54 guus Exp $
 */
 
 #include "system.h"
 */
 
 #include "system.h"
@@ -424,97 +424,7 @@ bool read_server_config()
        return x == 0;
 }
 
        return x == 0;
 }
 
-bool is_safe_path(const char *file)
-{
-#if !(defined(HAVE_CYGWIN) || defined(HAVE_MINGW))
-       char *p;
-       const char *f;
-       char x;
-       struct stat s;
-       char l[MAXBUFSIZE];
-
-       if(*file != '/') {
-               logger(LOG_ERR, _("`%s' is not an absolute path"), file);
-               return false;
-       }
-
-       p = strrchr(file, '/');
-
-       if(p == file)                           /* It's in the root */
-               p++;
-
-       x = *p;
-       *p = '\0';
-
-       f = file;
-
-check1:
-       if(lstat(f, &s) < 0) {
-               logger(LOG_ERR, _("Couldn't stat `%s': %s"), f, strerror(errno));
-               return false;
-       }
-
-       if(s.st_uid != geteuid()) {
-               logger(LOG_ERR, _("`%s' is owned by UID %d instead of %d"),
-                          f, s.st_uid, geteuid());
-               return false;
-       }
-
-       if(S_ISLNK(s.st_mode)) {
-               logger(LOG_WARNING, _("Warning: `%s' is a symlink"), f);
-
-               if(readlink(f, l, MAXBUFSIZE) < 0) {
-                       logger(LOG_ERR, _("Unable to read symbolic link `%s': %s"), f,
-                                  strerror(errno));
-                       return false;
-               }
-
-               f = l;
-               goto check1;
-       }
-
-       *p = x;
-       f = file;
-
-check2:
-       if(lstat(f, &s) < 0 && errno != ENOENT) {
-               logger(LOG_ERR, _("Couldn't stat `%s': %s"), f, strerror(errno));
-               return false;
-       }
-
-       if(errno == ENOENT)
-               return true;
-
-       if(s.st_uid != geteuid()) {
-               logger(LOG_ERR, _("`%s' is owned by UID %d instead of %d"),
-                          f, s.st_uid, geteuid());
-               return false;
-       }
-
-       if(S_ISLNK(s.st_mode)) {
-               logger(LOG_WARNING, _("Warning: `%s' is a symlink"), f);
-
-               if(readlink(f, l, MAXBUFSIZE) < 0) {
-                       logger(LOG_ERR, _("Unable to read symbolic link `%s': %s"), f,
-                                  strerror(errno));
-                       return false;
-               }
-
-               f = l;
-               goto check2;
-       }
-
-       if(s.st_mode & 0007) {
-               /* Accessible by others */
-               logger(LOG_ERR, _("`%s' has unsecure permissions"), f);
-               return false;
-       }
-#endif
-
-       return true;
-}
-
-FILE *ask_and_safe_open(const char *filename, const char *what, bool safe, const char *mode)
+FILE *ask_and_open(const char *filename, const char *what, const char *mode)
 {
        FILE *r;
        char *directory;
 {
        FILE *r;
        char *directory;
@@ -573,17 +483,6 @@ FILE *ask_and_safe_open(const char *filename, const char *what, bool safe, const
                return NULL;
        }
 
                return NULL;
        }
 
-       /* Then check the file for nasty attacks */
-       if(safe) {
-               if(!is_safe_path(fn)) {         /* Do not permit any directories that are readable or writeable by other users. */
-                       fprintf(stderr, _("The file `%s' (or any of the leading directories) has unsafe permissions.\n"
-                                        "I will not create or overwrite this file.\n"), fn);
-                       fclose(r);
-                       free(fn);
-                       return NULL;
-               }
-       }
-
        free(fn);
 
        return r;
        free(fn);
 
        return r;
index 8960f08..ba235c3 100644 (file)
@@ -17,7 +17,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-    $Id: conf.h,v 1.6.4.42 2003/07/30 21:52:41 guus Exp $
+    $Id: conf.h,v 1.6.4.43 2003/08/08 22:11:54 guus Exp $
 */
 
 #ifndef __TINC_CONF_H__
 */
 
 #ifndef __TINC_CONF_H__
@@ -57,7 +57,7 @@ extern bool get_config_subnet(const config_t *, struct subnet_t **);
 
 extern int read_config_file(avl_tree_t *, const char *);
 extern bool read_server_config(void);
 
 extern int read_config_file(avl_tree_t *, const char *);
 extern bool read_server_config(void);
-extern FILE *ask_and_safe_open(const char *, const char *, bool, const char *);
+extern FILE *ask_and_open(const char *, const char *, const char *);
 extern bool is_safe_path(const char *);
 
 #endif                                                 /* __TINC_CONF_H__ */
 extern bool is_safe_path(const char *);
 
 #endif                                                 /* __TINC_CONF_H__ */
index 18315ad..c2fbd7e 100644 (file)
@@ -17,7 +17,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-    $Id: meta.c,v 1.1.2.39 2003/08/08 14:24:09 guus Exp $
+    $Id: meta.c,v 1.1.2.40 2003/08/08 22:11:54 guus Exp $
 */
 
 #include "system.h"
 */
 
 #include "system.h"
@@ -54,10 +54,10 @@ bool send_meta(connection_t *c, char *buffer, int length)
        while(length) {
                result = send(c->socket, bufp, length, 0);
                if(result <= 0) {
        while(length) {
                result = send(c->socket, bufp, length, 0);
                if(result <= 0) {
-                       if(!errno || errno == EPIPE)
+                       if(!errno || errno == EPIPE) {
                                ifdebug(CONNECTIONS) logger(LOG_NOTICE, _("Connection closed by %s (%s)"),
                                                   c->name, c->hostname);
                                ifdebug(CONNECTIONS) logger(LOG_NOTICE, _("Connection closed by %s (%s)"),
                                                   c->name, c->hostname);
-                       else if(errno == EINTR)
+                       else if(errno == EINTR)
                                continue;
                        else
                                logger(LOG_ERR, _("Sending meta data to %s (%s) failed: %s"), c->name,
                                continue;
                        else
                                logger(LOG_ERR, _("Sending meta data to %s (%s) failed: %s"), c->name,
@@ -121,10 +121,10 @@ bool receive_meta(connection_t *c)
        lenin = recv(c->socket, c->buffer + c->buflen, MAXBUFSIZE - c->buflen, 0);
 
        if(lenin <= 0) {
        lenin = recv(c->socket, c->buffer + c->buflen, MAXBUFSIZE - c->buflen, 0);
 
        if(lenin <= 0) {
-               if(!lenin || !errno)
+               if(!lenin || !errno) {
                        ifdebug(CONNECTIONS) logger(LOG_NOTICE, _("Connection closed by %s (%s)"),
                                           c->name, c->hostname);
                        ifdebug(CONNECTIONS) logger(LOG_NOTICE, _("Connection closed by %s (%s)"),
                                           c->name, c->hostname);
-               else if(errno == EINTR)
+               else if(errno == EINTR)
                        return true;
                else
                        logger(LOG_ERR, _("Metadata socket read error for %s (%s): %s"),
                        return true;
                else
                        logger(LOG_ERR, _("Metadata socket read error for %s (%s): %s"),
index 5bbaa79..c7c1250 100644 (file)
@@ -17,7 +17,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-    $Id: net_setup.c,v 1.1.2.41 2003/07/30 11:50:45 guus Exp $
+    $Id: net_setup.c,v 1.1.2.42 2003/08/08 22:11:54 guus Exp $
 */
 
 #include "system.h"
 */
 
 #include "system.h"
@@ -149,6 +149,7 @@ bool read_rsa_private_key(void)
 {
        FILE *fp;
        char *fname, *key;
 {
        FILE *fp;
        char *fname, *key;
+       struct stat s;
 
        cp();
 
 
        cp();
 
@@ -164,32 +165,39 @@ bool read_rsa_private_key(void)
        if(!get_config_string(lookup_config(config_tree, "PrivateKeyFile"), &fname))
                asprintf(&fname, "%s/rsa_key.priv", confbase);
 
        if(!get_config_string(lookup_config(config_tree, "PrivateKeyFile"), &fname))
                asprintf(&fname, "%s/rsa_key.priv", confbase);
 
-       if(is_safe_path(fname)) {
-               fp = fopen(fname, "r");
+       fp = fopen(fname, "r");
 
 
-               if(!fp) {
-                       logger(LOG_ERR, _("Error reading RSA private key file `%s': %s"),
-                                  fname, strerror(errno));
-                       free(fname);
-                       return false;
-               }
+       if(!fp) {
+               logger(LOG_ERR, _("Error reading RSA private key file `%s': %s"),
+                          fname, strerror(errno));
+               free(fname);
+               return false;
+       }
 
 
+#if !defined(HAVE_MINGW) && !defined(HAVE_CYGWIN)
+       if(fstat(fileno(fp), &s)) {
+               logger(LOG_ERR, _("Could not stat RSA private key file `%s': %s'"),
+                               fname, strerror(errno));
                free(fname);
                free(fname);
-               myself->connection->rsa_key =
-                       PEM_read_RSAPrivateKey(fp, NULL, NULL, NULL);
-               fclose(fp);
+               return false;
+       }
 
 
-               if(!myself->connection->rsa_key) {
-                       logger(LOG_ERR, _("Reading RSA private key file `%s' failed: %s"),
-                                  fname, strerror(errno));
-                       return false;
-               }
+       if(s.st_mode & ~0700)
+               logger(LOG_WARNING, _("Warning: insecure file permissions for RSA private key file `%s'!"), fname);
+#endif
 
 
-               return true;
+       myself->connection->rsa_key = PEM_read_RSAPrivateKey(fp, NULL, NULL, NULL);
+       fclose(fp);
+
+       if(!myself->connection->rsa_key) {
+               logger(LOG_ERR, _("Reading RSA private key file `%s' failed: %s"),
+                          fname, strerror(errno));
+               free(fname);
+               return false;
        }
 
        free(fname);
        }
 
        free(fname);
-       return false;
+       return true;
 }
 
 /*
 }
 
 /*
index d81fdd6..0ec9880 100644 (file)
@@ -17,7 +17,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-    $Id: process.c,v 1.1.2.68 2003/08/08 19:56:11 guus Exp $
+    $Id: process.c,v 1.1.2.69 2003/08/08 22:11:54 guus Exp $
 */
 
 #include "system.h"
 */
 
 #include "system.h"
@@ -359,7 +359,6 @@ bool detach(void)
 bool execute_script(const char *name, char **envp)
 {
 #ifdef HAVE_SYSTEM
 bool execute_script(const char *name, char **envp)
 {
 #ifdef HAVE_SYSTEM
-       pid_t pid;
        int status;
        struct stat s;
        char *scriptname;
        int status;
        struct stat s;
        char *scriptname;
@@ -394,22 +393,20 @@ bool execute_script(const char *name, char **envp)
        if(status != -1) {
                if(WIFEXITED(status)) { /* Child exited by itself */
                        if(WEXITSTATUS(status)) {
        if(status != -1) {
                if(WIFEXITED(status)) { /* Child exited by itself */
                        if(WEXITSTATUS(status)) {
-                               logger(LOG_ERR, _("Process %d (%s) exited with non-zero status %d"),
-                                          pid, name, WEXITSTATUS(status));
+                               logger(LOG_ERR, _("Script %s exited with non-zero status %d"),
+                                          name, WEXITSTATUS(status));
                                return false;
                        }
                } else if(WIFSIGNALED(status)) {        /* Child was killed by a signal */
                                return false;
                        }
                } else if(WIFSIGNALED(status)) {        /* Child was killed by a signal */
-                       logger(LOG_ERR, _("Process %d (%s) was killed by signal %d (%s)"), pid,
+                       logger(LOG_ERR, _("Script %s was killed by signal %d (%s)"),
                                   name, WTERMSIG(status), strsignal(WTERMSIG(status)));
                        return false;
                } else {                        /* Something strange happened */
                                   name, WTERMSIG(status), strsignal(WTERMSIG(status)));
                        return false;
                } else {                        /* Something strange happened */
-                       logger(LOG_ERR, _("Process %d (%s) terminated abnormally"), pid,
-                                  name);
+                       logger(LOG_ERR, _("Script %s terminated abnormally"), name);
                        return false;
                }
        } else {
                        return false;
                }
        } else {
-               logger(LOG_ERR, _("System call `%s' failed: %s"), "system",
-                          strerror(errno));
+               logger(LOG_ERR, _("System call `%s' failed: %s"), "system", strerror(errno));
                return false;
        }
 #endif
                return false;
        }
 #endif
index a37a612..cec0ee5 100644 (file)
@@ -17,7 +17,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-    $Id: tincd.c,v 1.10.4.84 2003/08/08 19:45:21 guus Exp $
+    $Id: tincd.c,v 1.10.4.85 2003/08/08 22:11:54 guus Exp $
 */
 
 #include "system.h"
 */
 
 #include "system.h"
@@ -300,11 +300,16 @@ static bool keygen(int bits)
                fprintf(stderr, _("Done.\n"));
 
        asprintf(&filename, "%s/rsa_key.priv", confbase);
                fprintf(stderr, _("Done.\n"));
 
        asprintf(&filename, "%s/rsa_key.priv", confbase);
-       f = ask_and_safe_open(filename, _("private RSA key"), true, "a");
+       f = ask_and_open(filename, _("private RSA key"), "a");
 
        if(!f)
                return false;
 
        if(!f)
                return false;
-
+  
+#ifdef HAVE_FCHMOD
+       /* Make it unreadable for others. */
+       fchmod(fileno(f), 0600);
+#endif
+               
        if(ftell(f))
                fprintf(stderr, _("Appending key to existing contents.\nMake sure only one key is stored in the file.\n"));
 
        if(ftell(f))
                fprintf(stderr, _("Appending key to existing contents.\nMake sure only one key is stored in the file.\n"));
 
@@ -319,7 +324,7 @@ static bool keygen(int bits)
        else
                asprintf(&filename, "%s/rsa_key.pub", confbase);
 
        else
                asprintf(&filename, "%s/rsa_key.pub", confbase);
 
-       f = ask_and_safe_open(filename, _("public RSA key"), false, "a");
+       f = ask_and_open(filename, _("public RSA key"), "a");
 
        if(!f)
                return false;
 
        if(!f)
                return false;