From 9bde92ce97d5503ff2d31dcc6f0648902580ec14 Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Fri, 8 Aug 2003 22:11:54 +0000 Subject: [PATCH] Simpler checking of permissions on private RSA key and other fixes. --- src/conf.c | 105 +----------------------------------------------- src/conf.h | 4 +- src/meta.c | 10 ++--- src/net_setup.c | 46 ++++++++++++--------- src/process.c | 15 +++---- src/tincd.c | 13 ++++-- 6 files changed, 51 insertions(+), 142 deletions(-) diff --git a/src/conf.c b/src/conf.c index e927abdb..3feb1503 100644 --- a/src/conf.c +++ b/src/conf.c @@ -19,7 +19,7 @@ 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" @@ -424,97 +424,7 @@ bool read_server_config() 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; @@ -573,17 +483,6 @@ FILE *ask_and_safe_open(const char *filename, const char *what, bool safe, const 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; diff --git a/src/conf.h b/src/conf.h index 8960f087..ba235c3d 100644 --- a/src/conf.h +++ b/src/conf.h @@ -17,7 +17,7 @@ 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__ @@ -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 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__ */ diff --git a/src/meta.c b/src/meta.c index 18315ad1..c2fbd7ed 100644 --- a/src/meta.c +++ b/src/meta.c @@ -17,7 +17,7 @@ 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" @@ -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) { - if(!errno || errno == EPIPE) + if(!errno || errno == EPIPE) { 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, @@ -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) { - if(!lenin || !errno) + if(!lenin || !errno) { 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"), diff --git a/src/net_setup.c b/src/net_setup.c index 5bbaa799..c7c12505 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -17,7 +17,7 @@ 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" @@ -149,6 +149,7 @@ bool read_rsa_private_key(void) { FILE *fp; char *fname, *key; + struct stat s; 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(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); - 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); - return false; + return true; } /* diff --git a/src/process.c b/src/process.c index d81fdd69..0ec98802 100644 --- a/src/process.c +++ b/src/process.c @@ -17,7 +17,7 @@ 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" @@ -359,7 +359,6 @@ bool detach(void) bool execute_script(const char *name, char **envp) { #ifdef HAVE_SYSTEM - pid_t pid; 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)) { - 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 */ - 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 */ - logger(LOG_ERR, _("Process %d (%s) terminated abnormally"), pid, - name); + logger(LOG_ERR, _("Script %s terminated abnormally"), name); 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 diff --git a/src/tincd.c b/src/tincd.c index a37a6125..cec0ee5e 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -17,7 +17,7 @@ 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" @@ -300,11 +300,16 @@ static bool keygen(int bits) 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; - + +#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")); @@ -319,7 +324,7 @@ static bool keygen(int bits) 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; -- 2.20.1