From: Stephen Early <steve@greenend.org.uk>
Date: Fri, 28 Dec 2001 19:31:00 +0000 (+0000)
Subject: Import release 0.1.14
X-Git-Tag: v0.1.14
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?p=secnet.git;a=commitdiff_plain;h=4f5e39ecfaa49376b0a5c3a4c384e91a828c1105;ds=sidebyside

Import release 0.1.14
---

diff --git a/BUGS b/BUGS
index 7145f3a..02a2de3 100644
--- a/BUGS
+++ b/BUGS
@@ -1,2 +1,11 @@
 Known bugs in secnet
 
+(Complaints from Ian:)
+Your init.d script makes it hard to start secnet as non-root, too.
+
+secnet -jv has printed a large routing table full of stuff I wasn't
+interested in.
+
+Make explicit in the documentation that -n causes all log output to go
+to stderr.  Provide an option that is _really_ just "don't fork()" for
+people who want to run secnet from init.
diff --git a/CREDITS b/CREDITS
index 05a92a1..e89f44d 100644
--- a/CREDITS
+++ b/CREDITS
@@ -5,3 +5,5 @@ Steve Reid <sreid@sea-to-sky.net>, James H. Brown <jbrown@burgoyne.com> -
   SHA1 implementation
 Cendio Systems AB - ipaddr.py
 Mark Martinec <mark.martinec@ijs.si> - portable snprintf
+
+Simon Tatham, Jonathan Amery, Ian Jackson - testing and debugging
diff --git a/Makefile.in b/Makefile.in
index f8b37af..0210313 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -18,7 +18,7 @@
 .PHONY:	all clean realclean distclean dist install
 
 PACKAGE:=secnet
-VERSION:=0.1.13
+VERSION:=0.1.14
 
 @SET_MAKE@
 
@@ -118,7 +118,7 @@ conffile.tab.c:	conffile.y
 secnet:	$(OBJECTS)
 
 version.c: Makefile
-	echo "char version[]=\"secnet-$(VERSION)\";" >version.c
+	echo "char version[]=\"secnet $(VERSION)\";" >version.c
 
 install: all
 	$(INSTALL) -d $(prefix)/share/secnet $(sbindir)
diff --git a/NEWS b/NEWS
index 79cb772..ac1bf83 100644
--- a/NEWS
+++ b/NEWS
@@ -18,9 +18,21 @@ etc.
 
 See also file "TODO".
 
-* Planned for version 0.1.14
+* New in version 0.1.14
 
-RFC1812-compliance in netlink.c
+The --help and --version options now send their output to stdout.
+
+Bugfix: TUN flavour "BSD" no longer implies a BSD-style ifconfig and
+route command invocation.  Instead "ioctl"-style is used, which should
+work on both BSD and linux-2.2 systems.
+
+If no "networks" parameter is specified for a netlink device then it
+is assumed to be 0.0.0.0/0 rather than the empty set.  So, by default
+there is a default route from each netlink device to the host machine.
+The "networks" parameter can be used to implement a primitive
+firewall, restricting the destination addresses of packets received
+through tunnels; if a more complex firewall is required then implement
+it on the host.
 
 * New in version 0.1.13
 
diff --git a/NOTES b/NOTES
index dd78b12..84453df 100644
--- a/NOTES
+++ b/NOTES
@@ -60,6 +60,17 @@ explicit option. NB packets may be routed if the source OR the
 destination is marked as allowing routing [otherwise packets couldn't
 get back from eg. chiark to a laptop at greenend]).
 
+[the even newer plan]
+
+secnet sites are configured to grant access to particular IP address
+ranges to the holder of a particular public key.  The key can certify
+other keys, which will then be permitted to use a subrange of the IP
+address range of the certifying key.
+
+This means that secnet won't know in advance (i.e. at configuration
+time) how many tunnels it might be required to support, so we have to
+be able to create them (and routes, and so on) on the fly.
+
 ** VPN-level configuration
 
 At a high level we just want to be able to indicate which groups of
@@ -259,3 +270,9 @@ Keepalives are probably a good idea.
 **** Protocol sub-goal 3: send a packet
 
 9) i?,i?,msg0,(send-packet/msg9,packet)_k
+
+Some messages may take a long time to prepare (software modexp on slow
+machines); this is a "please wait" message to indicate that a message
+is in preparation.
+
+10) i?,i?,msg8,A,B,nA,nB,msg?
diff --git a/TODO b/TODO
index aea076b..8e8800d 100644
--- a/TODO
+++ b/TODO
@@ -1,6 +1,8 @@
 dh.c: change format to binary from decimal string (without introducing
 endianness problems)
 
+log.c: implement a file-descriptor-to-log module
+
 netlink.c: test the 'allow_route' option properly.
 Add fragmentation code.  Check that we comply with RFC1812.
 
@@ -31,9 +33,6 @@ then be combined in the configuration file.  Will allow the user to
 plug in different block ciphers, invent an authenticity-only mode,
 etc. (similar to udptunnel)
 
-tun.c: Solaris support, and configuring the interface and
-creating/deleting routes using ioctl()
-
 udp.c: option for path-MTU discovery (once fragmentation support is
 implemented in netlink)
 
diff --git a/conffile.c b/conffile.c
index 9c373c2..8f2d0fd 100644
--- a/conffile.c
+++ b/conffile.c
@@ -73,7 +73,7 @@ static void dict_iadd(dict_t *dict, atom_t key, list_t *val)
 {
     struct entry *e;
     if (dict_ilookup_primitive(dict, key)) {
-	fatal("duplicate key \"%s\" in dictionary\n",key);
+	fatal("duplicate key \"%s\" in dictionary",key);
     }
     e=safe_malloc(sizeof(*e),"dict_add");
     e->next=dict->entries;
@@ -128,25 +128,25 @@ static void ptree_mangle(struct p_node *t)
     ptree_mangle(t->r);
     switch (t->type) {
     case T_DICT:
-	/* ASSERT !t->l || t->l->type==T_ALIST */
-	/* ASSERT !t->r || t->r->type==T_LISTITEM */
+	ASSERT(!t->l || t->l->type==T_ALIST);
+	ASSERT(!t->r || t->r->type==T_LISTITEM);
 	t->l=list_reverse(t->l);
 	t->r=list_reverse(t->r);
 	break;
     case T_ASSIGNMENT:
-	/* ASSERT t->l->type==T_KEY */
-	/* ASSERT t->r->type==T_LISTITEM */
+	ASSERT(t->l->type==T_KEY);
+	ASSERT(t->r->type==T_LISTITEM);
 	t->r=list_reverse(t->r);
 	break;
     case T_ABSPATH:
     case T_RELPATH:
-	/* ASSERT t->l==NULL */
-	/* ASSERT t->r->type==T_PATHELEM */
+	ASSERT(t->l==NULL);
+	ASSERT(t->r->type==T_PATHELEM);
 	t->r=list_reverse(t->r);
 	break;
     case T_EXEC:
-	/* ASSERT t->l */
-	/* ASSERT t->r->type==T_LISTITEM */
+	ASSERT(t->l);
+	ASSERT(t->r==NULL || t->r->type==T_LISTITEM);
 	t->r=list_reverse(t->r);
 	break;
     }
@@ -220,8 +220,8 @@ static list_t *dict_lookup_path(dict_t *context, struct p_node *p)
     dict_t *i;
     list_t *l;
 
-    /* ASSERT p->type==T_PATHELEM */
-    /* ASSERT p->l->type==T_KEY */
+    ASSERT(p->type==T_PATHELEM);
+    ASSERT(p->l->type==T_KEY);
     l=dict_ilookup(context, p->l->data.key);
     if (!l) {
 	cfgfatal(p->loc,"conffile","can't find key %s\n",
@@ -288,7 +288,7 @@ static list_t *process_item(dict_t *context, struct p_node *i)
     default:
 #ifdef DUMP_PARSE_TREE
 	ptree_dump(i,0);
-	fatal("process_item: invalid node type for a list item (%s)\n",
+	fatal("process_item: invalid node type for a list item (%s)",
 	      ntype(i->type));
 #else
 	fatal("process_item: list item has invalid node type %d - recompile "
@@ -305,7 +305,7 @@ static list_t *process_ilist(dict_t *context, struct p_node *l)
     struct p_node *i;
     list_t *r;
 
-    /* ASSERT l->type==T_LISTITEM */
+    ASSERT(!l || l->type==T_LISTITEM);
 
     r=list_new();
 
@@ -321,9 +321,8 @@ static list_t *process_invocation(dict_t *context, struct p_node *i)
     item_t *cl;
     list_t *args;
 
-    /* ASSERT i->type==T_EXEC */
-    /* ASSERT i->r->type==T_LISTITEM */
-    /* XXX it might be null too */
+    ASSERT(i->type==T_EXEC);
+    ASSERT(i->r==NULL || i->r->type==T_LISTITEM);
     cll=process_item(context,i->l);
     cl=cll->item;
     if (cl->type != t_closure) {
@@ -344,15 +343,15 @@ static void process_alist(dict_t *context, struct p_node *c)
 
     if (!c) return; /* NULL assignment lists are valid (empty dictionary) */
 
-    /* ASSERT c->type==T_ALIST */
+    ASSERT(c->type==T_ALIST);
     if (c->type!=T_ALIST) {
-	fatal("invalid node type in assignment list\n");
+	fatal("invalid node type in assignment list");
     }
 
     for (i=c; i; i=i->r) {
-	/* ASSERT i->l && i->l->type==T_ASSIGNMENT */
-	/* ASSERT i->l->l->type==T_KEY */
-	/* ASSERT i->l->r->type==T_LISTITEM */
+	ASSERT(i->l && i->l->type==T_ASSIGNMENT);
+	ASSERT(i->l->l->type==T_KEY);
+	ASSERT(i->l->r->type==T_LISTITEM);
 	k=i->l->l->data.key;
 	l=process_ilist(context, i->l->r);
 	dict_iadd(context, k, l);
@@ -444,8 +443,9 @@ static list_t *readfile(closure_t *self, struct cloc loc,
     r=new_item(t_string,loc);
     r->data.string=safe_malloc(length+1,"readfile");
     if (fread(r->data.string,length,1,f)!=1) {
-	fatal("readfile (%s:%d): fread: could not read all of file\n",
-	      loc.file,loc.line);
+	(ferror(f) ? fatal_perror : fatal)
+	    ("readfile (%s:%d): fread: could not read all of file",
+	     loc.file,loc.line);
     }
     r->data.string[length]=0;
     if (fclose(f)!=0) {
diff --git a/conffile.fl b/conffile.fl
index c1b4b04..66caa1f 100644
--- a/conffile.fl
+++ b/conffile.fl
@@ -12,6 +12,14 @@
 
 #define YY_NO_UNPUT
 
+#define YY_INPUT(buf,result,max_size)					\
+do{									\
+	(result)= fread((buf),1,(max_size),yyin);			\
+	if (ferror(yyin))						\
+		fatal_perror("Error reading configuration file (%s)",	\
+			     config_file);				\
+}while(0)
+
 #define MAX_INCLUDE_DEPTH 10
 struct include_stack_item {
 	YY_BUFFER_STATE bst;
diff --git a/config.h.bot b/config.h.bot
index cf288bd..bb24eb1 100644
--- a/config.h.bot
+++ b/config.h.bot
@@ -55,4 +55,12 @@ typedef unsigned char uint8_t;
 #include "snprintf.h"
 #endif
 
+#ifdef __GNUC__
+#define NORETURN(_x) void _x __attribute__ ((noreturn))
+#define FORMAT(_a,_b,_c) __attribute__ ((format (_a,_b,_c)))
+#else
+#define NORETURN(_x) _x
+#define FORMAT(_a,_b,_c)
+#endif
+
 #endif /* _CONFIG_H */
diff --git a/config.h.in b/config.h.in
index 508a8a8..8492df2 100644
--- a/config.h.in
+++ b/config.h.in
@@ -160,4 +160,12 @@ typedef unsigned char uint8_t;
 #include "snprintf.h"
 #endif
 
+#ifdef __GNUC__
+#define NORETURN(_x) void _x __attribute__ ((noreturn))
+#define FORMAT(_a,_b,_c) __attribute__ ((format (_a,_b,_c)))
+#else
+#define NORETURN(_x) _x
+#define FORMAT(_a,_b,_c)
+#endif
+
 #endif /* _CONFIG_H */
diff --git a/debian/changelog b/debian/changelog
index 1327d12..2462c6f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,5 +1,5 @@
-secnet (0.1.13-1) unstable; urgency=low
+secnet (0.1.14-1) unstable; urgency=low
 
   * New upstream version.
 
- -- Stephen Early <steve@greenend.org.uk>  Sat, 27 Oct 2001 15:00:00 +0100
+ -- Stephen Early <steve@greenend.org.uk>  Sat, 29 Dec 2001 15:00:00 +0100
diff --git a/debian/conffiles b/debian/conffiles
new file mode 100644
index 0000000..941529d
--- /dev/null
+++ b/debian/conffiles
@@ -0,0 +1 @@
+/etc/init.d/secnet
diff --git a/debian/init b/debian/init
index c22b55f..75c21af 100644
--- a/debian/init
+++ b/debian/init
@@ -32,8 +32,8 @@ case "$1" in
 	;;
   stop)
 	echo -n "Stopping $DESC: "
-	start-stop-daemon --stop --quiet --pidfile /var/run/$NAME.pid \
-		--exec $DAEMON
+	start-stop-daemon --stop --quiet --oknodo --pidfile \
+	    /var/run/$NAME.pid --exec $DAEMON
 	echo "$NAME."
 	;;
   #reload)
diff --git a/example.conf b/example.conf
index b6d5dc2..634467c 100644
--- a/example.conf
+++ b/example.conf
@@ -65,22 +65,18 @@ setup-retries 10;
 setup-timeout 2000;
 
 # Use the universal TUN/TAP driver to get packets to and from the kernel
-#  (use tun-old if you are not on Linux-2.4)
 netlink tun {
 	name "netlink-tun"; # Printed in log messages from this netlink
 #	interface "tun0"; # You may set your own interface name if you wish;
 		# if you don't one will be chosen for you.
 #	device "/dev/net/tun";
 
-	# local networks served by this netlink device
-	# incoming tunneled packets for other networks will be discarded
-	networks "192.168.x.x/24", "192.168.x.x/24", "172.x.x.x/24";
 	local-address "192.168.x.x"; # IP address of host's tunnel interface
 	secnet-address "192.168.x.x"; # IP address of this secnet
 
 	# Tunnels are only allowed to use these networks; attempts to
 	# claim IP addresses in any other ranges is a configuration error
-	remote-networks "192.168.0.0/24", "172.16.0.0/12", "10.0.0.0/8";
+	remote-networks "192.168.0.0/16", "172.16.0.0/12", "10.0.0.0/8";
 
 	# MTU of the tunnel interface. Should be kept under the path-MTU
 	# (by at least 60 bytes) between this secnet and its peers for
@@ -152,10 +148,10 @@ include /etc/secnet/sites.conf
 # a newer version. MAKE SURE YOU GET AN AUTHENTIC COPY OF THE FILE - it
 # contains public keys for all sites.
 
-sites
-	site(vpn-data/example/location1/site1),
-	site(vpn-data/example/location2/site1),
-	site(vpn-data/example/location2/site2);
+sites map(site,
+	vpn-data/example/location1/site1,
+	vpn-data/example/location2/site1,
+	vpn-data/example/location2/site2);
 
 # If you want to communicate with all the VPN sites, you can use something
 # like the following instead:
diff --git a/log.c b/log.c
index 32f0901..e43c3cf 100644
--- a/log.c
+++ b/log.c
@@ -5,6 +5,7 @@
 #include <time.h>
 #include <errno.h>
 #include <syslog.h>
+#include <assert.h>
 #include "process.h"
 
 bool_t secnet_is_daemon=False;
@@ -49,6 +50,9 @@ void Message(uint32_t class, char *message, ...)
     va_end(ap);
 }
 
+static NORETURN(vfatal(int status, bool_t perror, char *message,
+		       va_list args));
+
 static void vfatal(int status, bool_t perror, char *message, va_list args)
 {
     int err;
@@ -56,15 +60,12 @@ static void vfatal(int status, bool_t perror, char *message, va_list args)
     err=errno;
 
     enter_phase(PHASE_SHUTDOWN);
-    if (perror) {
-	Message(M_FATAL, "secnet fatal error: ");
-	vMessage(M_FATAL, message, args);
+    Message(M_FATAL, "secnet fatal error: ");
+    vMessage(M_FATAL, message, args);
+    if (perror)
 	Message(M_FATAL, ": %s\n",strerror(err));
-    }
-    else {
-	Message(M_FATAL, "secnet fatal error: ");
-	vMessage(M_FATAL,message,args);
-    }
+    else
+	Message(M_FATAL, "\n");
     exit(status);
 }
 
@@ -100,15 +101,20 @@ void fatal_perror_status(int status, char *message, ...)
     va_end(args);
 }
 
-void cfgfatal(struct cloc loc, string_t facility, char *message, ...)
+void vcfgfatal_maybefile(FILE *maybe_f /* or 0 */, struct cloc loc,
+			 string_t facility, char *message, va_list args)
 {
-    va_list args;
-
-    va_start(args,message);
-
     enter_phase(PHASE_SHUTDOWN);
 
-    if (loc.file && loc.line) {
+    if (maybe_f && ferror(maybe_f)) {
+	assert(loc.file);
+	Message(M_FATAL, "error reading config file (%s, %s): %s",
+		facility, loc.file, strerror(errno));
+    } else if (maybe_f && feof(maybe_f)) {
+	assert(loc.file);
+	Message(M_FATAL, "unexpected end of config file (%s, %s)",
+		facility, loc.file);
+    } else if (loc.file && loc.line) {
 	Message(M_FATAL, "config error (%s, %s:%d): ",facility,loc.file,
 		loc.line);
     } else if (!loc.file && loc.line) {
@@ -118,10 +124,41 @@ void cfgfatal(struct cloc loc, string_t facility, char *message, ...)
     }
     
     vMessage(M_FATAL,message,args);
-    va_end(args);
     exit(current_phase);
 }
 
+void cfgfatal_maybefile(FILE *maybe_f, struct cloc loc, string_t facility,
+			char *message, ...)
+{
+    va_list args;
+
+    va_start(args,message);
+    vcfgfatal_maybefile(maybe_f,loc,facility,message,args);
+    va_end(args);
+}    
+
+void cfgfatal(struct cloc loc, string_t facility, char *message, ...)
+{
+    va_list args;
+
+    va_start(args,message);
+    vcfgfatal_maybefile(0,loc,facility,message,args);
+    va_end(args);
+}
+
+void cfgfile_postreadcheck(struct cloc loc, FILE *f)
+{
+    assert(loc.file);
+    if (ferror(f)) {
+	Message(M_FATAL, "error reading config file (%s): %s",
+		loc.file, strerror(errno));
+	exit(current_phase);
+    } else if (feof(f)) {
+	Message(M_FATAL, "unexpected end of config file (%s)", loc.file);
+	exit(current_phase);
+    }
+}
+
 /* Take a list of log closures and merge them */
 struct loglist {
     struct log_if *l;
diff --git a/md5.c b/md5.c
index 8c962d9..043c9c1 100644
--- a/md5.c
+++ b/md5.c
@@ -296,7 +296,7 @@ void md5_module(dict_t *dict)
     md5_final(ctx,digest);
     for (i=0; i<16; i++) {
 	if (digest[i]!=expected[i]) {
-	    fatal("md5 module failed self-test\n");
+	    fatal("md5 module failed self-test");
 	}
     }
 }
diff --git a/netlink.c b/netlink.c
index 704dcf6..8e843bc 100644
--- a/netlink.c
+++ b/netlink.c
@@ -414,7 +414,7 @@ static void netlink_packet_deliver(struct netlink *st,
     BUF_ASSERT_USED(buf);
 
     if (dest==st->secnet_address) {
-	Message(M_ERR,"%s: trying to deliver a packet to myself!\n");
+	Message(M_ERR,"%s: trying to deliver a packet to myself!\n",st->name);
 	BUF_FREE(buf);
 	return;
     }
@@ -961,9 +961,10 @@ netlink_deliver_fn *netlink_init(struct netlink *st,
     if (l) 
 	st->networks=string_list_to_ipset(l,loc,st->name,"networks");
     else {
-	Message(M_WARNING,"%s: no local networks (parameter \"networks\") "
-		"defined\n",st->name);
-	st->networks=ipset_new();
+	struct ipset *empty;
+	empty=ipset_new();
+	st->networks=ipset_complement(empty);
+	ipset_free(empty);
     }
     l=dict_lookup(dict,"remote-networks");
     if (l) {
diff --git a/process.c b/process.c
index 0c6d443..f2a2e42 100644
--- a/process.c
+++ b/process.c
@@ -68,7 +68,7 @@ pid_t makesubproc(process_entry_fn *entry, process_callback_fn *cb,
     c->cst=cst;
 
     if (!signal_handling) {
-	fatal("makesubproc called before signal handling started\n");
+	fatal("makesubproc called before signal handling started");
     }
     p=fork();
     if (p==0) {
diff --git a/random.c b/random.c
index 3fc2314..111f16e 100644
--- a/random.c
+++ b/random.c
@@ -4,6 +4,7 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <assert.h>
 
 struct rgen_data {
     closure_t cl;
@@ -16,9 +17,29 @@ static random_fn random_generate;
 static bool_t random_generate(void *data, uint32_t bytes, uint8_t *buff)
 {
     struct rgen_data *st=data;
+    int r;
 
-    /* XXX XXX error checking */
-    read(st->fd,buff,bytes);
+    r= read(st->fd,buff,bytes);
+
+    assert(r == bytes);
+    /* This is totally crap error checking, but AFAICT many callers of
+     * this function do not check the return value.  This is a minimal
+     * change to make the code not fail silently-but-insecurely.
+     *
+     * A proper fix requires either:
+     *  - Declare all random number generation failures as fatal
+     *    errors, and make this return void, and fix all callers,
+     *    and make this call some appropriate function if it fails.
+     *  - Make this have proper error checking (and reporting!)
+     *    and make all callers check the error (and report!);
+     *    this will be tricky, I think, because you have to report
+     *    the errno somewhere.
+     *
+     * There's also the issue that this is only one possible
+     * implementation of a random number source; others may not rely
+     * on reading from a file descriptor, and may not produce
+     * appropriate settings of errno.
+     */
 
     return True;
 }
@@ -45,7 +66,7 @@ static list_t *random_apply(closure_t *self, struct cloc loc,
     arg2=list_elem(args,1);
 
     if (!arg1) {
-	fatal("randomsource: requires a filename\n");
+	cfgfatal(loc,"randomsource","requires a filename\n");
     }
     if (arg1->type != t_string) {
 	cfgfatal(arg1->loc,"randomsource",
@@ -62,7 +83,7 @@ static list_t *random_apply(closure_t *self, struct cloc loc,
     }
 
     if (!filename) {
-	fatal("randomsource requires a filename");
+	cfgfatal(loc,"randomsource","requires a filename\n");
     }
     st->fd=open(filename,O_RDONLY);
     if (st->fd<0) {
diff --git a/resolver.c b/resolver.c
index 282db80..8ffdc28 100644
--- a/resolver.c
+++ b/resolver.c
@@ -74,7 +74,7 @@ static void resolver_afterpoll(void *sst, struct pollfd *fds, int nfds,
 	} else if (rv==EAGAIN || rv==ESRCH) {
 	    break;
 	} else {
-	    fatal("resolver_afterpoll: adns_check() returned %d\n",rv);
+	    fatal("resolver_afterpoll: adns_check() returned %d",rv);
 	}
     }
 
diff --git a/rsa.c b/rsa.c
index 2b4c729..a73338b 100644
--- a/rsa.c
+++ b/rsa.c
@@ -37,7 +37,7 @@ static string_t rsa_sign(void *sst, uint8_t *data, uint32_t datalen)
     msize=mpz_sizeinbase(&st->n, 16);
 
     if (datalen*2+4>=msize) {
-	fatal("rsa_sign: message too big\n");
+	fatal("rsa_sign: message too big");
     }
 
     strcpy(buff,"0001");
@@ -153,21 +153,23 @@ static list_t *rsapub_apply(closure_t *self, struct cloc loc, dict_t *context,
     return new_closure(&st->cl);
 }
 
-static uint32_t keyfile_get_int(FILE *f)
+static uint32_t keyfile_get_int(struct cloc loc, FILE *f)
 {
     uint32_t r;
     r=fgetc(f)<<24;
     r|=fgetc(f)<<16;
     r|=fgetc(f)<<8;
     r|=fgetc(f);
+    cfgfile_postreadcheck(loc,f);
     return r;
 }
 
-static uint16_t keyfile_get_short(FILE *f)
+static uint16_t keyfile_get_short(struct cloc loc, FILE *f)
 {
     uint16_t r;
     r=fgetc(f)<<8;
     r|=fgetc(f);
+    cfgfile_postreadcheck(loc,f);
     return r;
 }
 
@@ -221,69 +223,72 @@ static list_t *rsapriv_apply(closure_t *self, struct cloc loc, dict_t *context,
     length=strlen(AUTHFILE_ID_STRING)+1;
     b=safe_malloc(length,"rsapriv_apply");
     if (fread(b,length,1,f)!=1 || memcmp(b,AUTHFILE_ID_STRING,length)!=0) {
-	cfgfatal(loc,"rsa-private","file \"%s\" is not a "
-		 "SSH1 private keyfile\n",filename);
+	cfgfatal_maybefile(f,loc,"rsa-private","failed to read magic ID"
+			   " string from SSH1 private keyfile \"%s\"\n",
+			   filename);
     }
     free(b);
 
     cipher_type=fgetc(f);
-    keyfile_get_int(f); /* "Reserved data" */
+    keyfile_get_int(loc,f); /* "Reserved data" */
     if (cipher_type != 0) {
 	cfgfatal(loc,"rsa-private","we don't support encrypted keyfiles\n");
     }
 
     /* Read the public key */
-    keyfile_get_int(f); /* Not sure what this is */
-    length=(keyfile_get_short(f)+7)/8;
+    keyfile_get_int(loc,f); /* Not sure what this is */
+    length=(keyfile_get_short(loc,f)+7)/8;
     if (length>1024) {
 	cfgfatal(loc,"rsa-private","implausible length %ld for modulus\n",
 		 length);
     }
     b=safe_malloc(length,"rsapriv_apply");
     if (fread(b,length,1,f) != 1) {
-	cfgfatal(loc,"rsa-private","error reading modulus\n");
+	cfgfatal_maybefile(f,loc,"rsa-private","error reading modulus");
     }
     mpz_init(&st->n);
     read_mpbin(&st->n,b,length);
     free(b);
-    length=(keyfile_get_short(f)+7)/8;
+    length=(keyfile_get_short(loc,f)+7)/8;
     if (length>1024) {
 	cfgfatal(loc,"rsa-private","implausible length %ld for e\n",length);
     }
     b=safe_malloc(length,"rsapriv_apply");
     if (fread(b,length,1,f)!=1) {
-	cfgfatal(loc,"rsa-private","error reading e\n");
+	cfgfatal_maybefile(f,loc,"rsa-private","error reading e\n");
     }
     mpz_init(&e);
     read_mpbin(&e,b,length);
     free(b);
     
-    length=keyfile_get_int(f);
+    length=keyfile_get_int(loc,f);
     if (length>1024) {
 	cfgfatal(loc,"rsa-private","implausibly long (%ld) key comment\n",
 		 length);
     }
     c=safe_malloc(length+1,"rsapriv_apply");
     if (fread(c,length,1,f)!=1) {
-	cfgfatal(loc,"rsa-private","error reading key comment\n");
+	cfgfatal_maybefile(f,loc,"rsa-private","error reading key comment\n");
     }
     c[length]=0;
 
     /* Check that the next two pairs of characters are identical - the
        keyfile is not encrypted, so they should be */
-    if (keyfile_get_short(f) != keyfile_get_short(f)) {
+
+    if (keyfile_get_short(loc,f) != keyfile_get_short(loc,f)) {
 	cfgfatal(loc,"rsa-private","corrupt keyfile\n");
     }
 
     /* Read d */
-    length=(keyfile_get_short(f)+7)/8;
+    length=(keyfile_get_short(loc,f)+7)/8;
     if (length>1024) {
 	cfgfatal(loc,"rsa-private","implausibly long (%ld) decryption key\n",
 		 length);
     }
     b=safe_malloc(length,"rsapriv_apply");
     if (fread(b,length,1,f)!=1) {
-	cfgfatal(loc,"rsa-private","error reading decryption key\n");
+	cfgfatal_maybefile(f,loc,"rsa-private",
+			   "error reading decryption key\n");
     }
     mpz_init(&st->d);
     read_mpbin(&st->d,b,length);
diff --git a/secnet.c b/secnet.c
index 8987af3..a21e52b 100644
--- a/secnet.c
+++ b/secnet.c
@@ -80,28 +80,27 @@ static void parse_options(int argc, char **argv)
 	switch(c) {
 	case 2:
 	    /* Help */
-	    fprintf(stderr,
-		    "Usage: secnet [OPTION]...\n\n"
-		    "  -f, --silent, --quiet   suppress error messages\n"
-		    "  -w, --nowarnings        suppress warnings\n"
-		    "  -v, --verbose           output extra diagnostics\n"
-		    "  -c, --config=filename   specify a configuration file\n"
-		    "  -j, --just-check-config stop after reading "
-		    "configuration file\n"
-		    "  -s, --sites-key=name    configuration key that "
-		    "specifies active sites\n"
-		    "  -n, --nodetach          do not run in background\n"
-		    "  -d, --debug=item,...    set debug options\n"
-		    "      --help              display this help and exit\n"
-		    "      --version           output version information "
-		    "and exit\n"
+	    printf("Usage: secnet [OPTION]...\n\n"
+		   "  -f, --silent, --quiet   suppress error messages\n"
+		   "  -w, --nowarnings        suppress warnings\n"
+		   "  -v, --verbose           output extra diagnostics\n"
+		   "  -c, --config=filename   specify a configuration file\n"
+		   "  -j, --just-check-config stop after reading "
+		   "configuration file\n"
+		   "  -s, --sites-key=name    configuration key that "
+		   "specifies active sites\n"
+		   "  -n, --nodetach          do not run in background\n"
+		   "  -d, --debug=item,...    set debug options\n"
+		   "      --help              display this help and exit\n"
+		   "      --version           output version information "
+		   "and exit\n"
 		);
 	    exit(0);
 	    break;
       
 	case 1:
 	    /* Version */
-	    fprintf(stderr,"%s\n",version);
+	    printf("%s\n",version);
 	    exit(0);
 	    break;
 
@@ -170,7 +169,7 @@ static void setup(dict_t *config)
     l=dict_lookup(config,"system");
 
     if (!l || list_elem(l,0)->type!=t_dict) {
-	fatal("configuration does not include a \"system\" dictionary\n");
+	fatal("configuration does not include a \"system\" dictionary");
     }
     system=list_elem(l,0)->data.dict;
     loc=list_elem(l,0)->loc;
@@ -178,7 +177,7 @@ static void setup(dict_t *config)
     /* Arrange systemwide log facility */
     l=dict_lookup(system,"log");
     if (!l) {
-	fatal("configuration does not include a system/log facility\n");
+	fatal("configuration does not include a system/log facility");
     }
     system_log=init_log(l);
 
@@ -194,7 +193,7 @@ static void setup(dict_t *config)
 	} while(pw);
 	endpwent();
 	if (uid==0) {
-	    fatal("userid \"%s\" not found\n",userid);
+	    fatal("userid \"%s\" not found",userid);
 	}
     }
 
@@ -203,8 +202,8 @@ static void setup(dict_t *config)
 
     /* Check whether we need root privileges */
     if (require_root_privileges && uid!=0) {
-	fatal("the following configured feature (\"%s\") requires "
-	      "that secnet retain root privileges while running.\n",
+	fatal("the configured feature \"%s\" requires "
+	      "that secnet retain root privileges while running.",
 	      require_root_privileges_explanation);
     }
 
@@ -266,7 +265,7 @@ static void run(void)
 
     fds=alloca(sizeof(*fds)*total_nfds);
     if (!fds) {
-	fatal("run: couldn't alloca\n");
+	fatal("run: couldn't alloca");
     }
 
     Message(M_NOTICE,"%s [%d]: starting\n",version,secnet_pid);
@@ -291,10 +290,10 @@ static void run(void)
 	    if (rv!=0) {
 		/* XXX we need to handle this properly: increase the
 		   nfds available */
-		fatal("run: beforepoll_fn (%s) returns %d\n",i->desc,rv);
+		fatal("run: beforepoll_fn (%s) returns %d",i->desc,rv);
 	    }
 	    if (timeout<-1) {
-		fatal("run: beforepoll_fn (%s) set timeout to %d\n",timeout);
+		fatal("run: beforepoll_fn (%s) set timeout to %d",timeout);
 	    }
 	    idx+=nfds;
 	    remain-=nfds;
diff --git a/secnet.h b/secnet.h
index 7eed367..f8062e6 100644
--- a/secnet.h
+++ b/secnet.h
@@ -6,6 +6,7 @@
 #include "config.h"
 #include <stdlib.h>
 #include <stdarg.h>
+#include <stdio.h>
 #include <sys/poll.h>
 #include <sys/types.h>
 #include <sys/time.h>
@@ -15,7 +16,7 @@ typedef char *string_t;
 typedef enum {False,True} bool_t;
 
 #define ASSERT(x) do { if (!(x)) { fatal("assertion failed line %d file " \
-					 __FILE__ "\n",__LINE__); } } while(0)
+					 __FILE__,__LINE__); } } while(0)
 
 /***** CONFIGURATION support *****/
 
@@ -109,28 +110,6 @@ extern uint32_t string_list_to_word(list_t *l, struct flagstr *f,
 
 /***** END of configuration support *****/
 
-/***** LOG functions *****/
-
-#define M_DEBUG_CONFIG 0x001
-#define M_DEBUG_PHASE  0x002
-#define M_DEBUG        0x004
-#define M_INFO	       0x008
-#define M_NOTICE       0x010
-#define M_WARNING      0x020
-#define M_ERR	       0x040
-#define M_SECURITY     0x080
-#define M_FATAL	       0x100
-
-extern void fatal(char *message, ...);
-extern void fatal_perror(char *message, ...);
-extern void fatal_status(int status, char *message, ...);
-extern void fatal_perror_status(int status, char *message, ...);
-extern void cfgfatal(struct cloc loc, string_t facility, char *message, ...);
-
-extern void Message(uint32_t class, char *message, ...);
-
-/***** END of log functions *****/
-
 /***** UTILITY functions *****/
 
 extern char *safe_strdup(char *string, char *message);
@@ -299,7 +278,8 @@ struct log_if {
     log_vmsg_fn *vlog;
 };
 /* (convenience function, defined in util.c) */
-extern void log(struct log_if *lf, int class, char *message, ...);
+extern void log(struct log_if *lf, int class, char *message, ...)
+FORMAT(printf,3,4);
 
 /* SITE interface */
 
@@ -423,4 +403,36 @@ struct buffer_if {
     uint32_t len; /* Total length allocated at base */
 };
 
+/***** LOG functions *****/
+
+#define M_DEBUG_CONFIG 0x001
+#define M_DEBUG_PHASE  0x002
+#define M_DEBUG        0x004
+#define M_INFO	       0x008
+#define M_NOTICE       0x010
+#define M_WARNING      0x020
+#define M_ERR	       0x040
+#define M_SECURITY     0x080
+#define M_FATAL	       0x100
+
+/* The fatal() family of functions require messages that do not end in '\n' */
+extern NORETURN(fatal(char *message, ...));
+extern NORETURN(fatal_perror(char *message, ...));
+extern NORETURN(fatal_status(int status, char *message, ...));
+extern NORETURN(fatal_perror_status(int status, char *message, ...));
+
+/* The cfgfatal() family of functions require messages that end in '\n' */
+extern NORETURN(cfgfatal(struct cloc loc, string_t facility, char *message,
+			 ...));
+extern void cfgfile_postreadcheck(struct cloc loc, FILE *f);
+extern NORETURN(vcfgfatal_maybefile(FILE *maybe_f, struct cloc loc,
+				    string_t facility, char *message,
+				    va_list));
+extern NORETURN(cfgfatal_maybefile(FILE *maybe_f, struct cloc loc,
+				   string_t facility, char *message, ...));
+
+extern void Message(uint32_t class, char *message, ...) FORMAT(printf,2,3);
+
+/***** END of log functions *****/
+
 #endif /* secnet_h */
diff --git a/sha1.c b/sha1.c
index d85e0a8..87012f3 100644
--- a/sha1.c
+++ b/sha1.c
@@ -347,7 +347,7 @@ void sha1_module(dict_t *dict)
     sha1_final(ctx,digest);
     for (i=0; i<20; i++) {
 	if (digest[i]!=expected[i]) {
-	    fatal("sha1 module failed self-test\n");
+	    fatal("sha1 module failed self-test");
 	}
     }
 }
diff --git a/site.c b/site.c
index 2b51a5d..55efa4d 100644
--- a/site.c
+++ b/site.c
@@ -28,7 +28,7 @@
 #define DEFAULT_KEY_LIFETIME 3600000 /* One hour */
 #define DEFAULT_KEY_RENEGOTIATE_GAP 300000 /* Five minutes */
 #define DEFAULT_SETUP_RETRIES 5
-#define DEFAULT_SETUP_TIMEOUT 1000
+#define DEFAULT_SETUP_TIMEOUT 2000
 #define DEFAULT_WAIT_TIME 20000
 
 /* Each site can be in one of several possible states. */
@@ -763,7 +763,7 @@ static void delete_key(struct site *st, string_t reason, uint32_t loglevel)
 
 static void state_assert(struct site *st, bool_t ok)
 {
-    if (!ok) fatal("state_assert\n");
+    if (!ok) fatal("site:state_assert");
 }
 
 static void enter_state_stop(struct site *st)
@@ -853,7 +853,7 @@ static bool_t enter_new_state(struct site *st, uint32_t next)
 	break;
     default:
 	gen=NULL;
-	fatal("enter_new_state(%s): invalid new state\n",state_name(next));
+	fatal("enter_new_state(%s): invalid new state",state_name(next));
 	break;
     }
 
diff --git a/slip.c b/slip.c
index fe6cb15..369651f 100644
--- a/slip.c
+++ b/slip.c
@@ -83,7 +83,7 @@ static void slip_unstuff(struct slip *st, uint8_t *buf, uint32_t l)
 		*(uint8_t *)buf_append(st->buff,1)=SLIP_ESC;
 		break;
 	    default:
-		fatal("userv_afterpoll: bad SLIP escape character\n");
+		fatal("userv_afterpoll: bad SLIP escape character");
 	    }
 	} else {
 	    switch (buf[i]) {
@@ -168,7 +168,7 @@ static void userv_afterpoll(void *sst, struct pollfd *fds, int nfds,
 		fatal_perror("%s: userv_afterpoll: read(rxfd)",
 			     st->slip.nl.name);
 	} else if (l==0) {
-	    fatal("%s: userv_afterpoll: read(rxfd)=0; userv gone away?\n",
+	    fatal("%s: userv_afterpoll: read(rxfd)=0; userv gone away?",
 		  st->slip.nl.name);
 	} else slip_unstuff(&st->slip,rxbuf,l);
     }
@@ -193,13 +193,13 @@ static void userv_userv_callback(void *sst, pid_t pid, int status)
     }
     if (!st->expecting_userv_exit) {
 	if (WIFEXITED(status)) {
-	    fatal("%s: userv exited unexpectedly with status %d\n",
+	    fatal("%s: userv exited unexpectedly with status %d",
 		  st->slip.nl.name,WEXITSTATUS(status));
 	} else if (WIFSIGNALED(status)) {
-	    fatal("%s: userv exited unexpectedly: uncaught signal %d\n",
+	    fatal("%s: userv exited unexpectedly: uncaught signal %d",
 		  st->slip.nl.name,WTERMSIG(status));
 	} else {
-	    fatal("%s: userv stopped unexpectedly\n");
+	    fatal("%s: userv stopped unexpectedly");
 	}
     }
     Message(M_WARNING,"%s: userv subprocess died with status %d\n",
@@ -240,11 +240,11 @@ static void userv_invoke_userv(struct userv *st)
     struct netlink_client *r;
     struct ipset *allnets;
     struct subnet_list *snets;
-    int i;
+    int i, nread;
     uint8_t confirm;
 
     if (st->pid) {
-	fatal("userv_invoke_userv: already running\n");
+	fatal("userv_invoke_userv: already running");
     }
 
     /* This is where we actually invoke userv - all the networks we'll
@@ -323,23 +323,25 @@ static void userv_invoke_userv(struct userv *st)
     Message(M_INFO,"%s: userv-ipif pid is %d\n",st->slip.nl.name,st->pid);
     /* Read a single character from the pipe to confirm userv-ipif is
        running. If we get a SIGCHLD at this point then we'll get EINTR. */
-    if (read(st->rxfd,&confirm,1)!=1) {
+    if ((nread=read(st->rxfd,&confirm,1))!=1) {
 	if (errno==EINTR) {
 	    Message(M_WARNING,"%s: read of confirmation byte was "
 		    "interrupted\n",st->slip.nl.name);
 	} else {
-	    fatal_perror("%s: read() of confirmation byte",st->slip.nl.name);
+	    if (nread<0) {
+		fatal_perror("%s: error reading confirmation byte",
+			     st->slip.nl.name);
+	    } else {
+		fatal("%s: unexpected EOF instead of confirmation byte"
+		      " - userv ipif failed?", st->slip.nl.name);
+	    }
 	}
     } else {
 	if (confirm!=SLIP_END) {
-	    fatal("%s: bad confirmation byte %d from userv-ipif\n",
+	    fatal("%s: bad confirmation byte %d from userv-ipif",
 		  st->slip.nl.name,confirm);
 	}
     }
-    /* Mark rxfd non-blocking */
-    if (fcntl(st->rxfd, F_SETFL, fcntl(st->rxfd, F_GETFL)|O_NONBLOCK)==-1) {
-	fatal_perror("%s: fcntl(O_NONBLOCK)",st->slip.nl.name);
-    }
 }
 
 static void userv_kill_userv(struct userv *st)
diff --git a/transform.c b/transform.c
index dba0ef8..c4482db 100644
--- a/transform.c
+++ b/transform.c
@@ -405,14 +405,14 @@ void transform_module(dict_t *dict)
 	ciphertext[2]!=0x83C31E69 ||
 	ciphertext[1]!=0xec52bd82 ||
 	ciphertext[0]!=0x27a46120) {
-	fatal("transform_module: serpent failed self-test (encrypt)\n");
+	fatal("transform_module: serpent failed self-test (encrypt)");
     }
     serpent_decrypt(&k,ciphertext,plaintext);
     if (plaintext[0]!=0 ||
 	plaintext[1]!=1 ||
 	plaintext[2]!=2 ||
 	plaintext[3]!=3) {
-	fatal("transform_module: serpent failed self-test (decrypt)\n");
+	fatal("transform_module: serpent failed self-test (decrypt)");
     }
 
     add_closure(dict,"serpent256-cbc",transform_apply);
diff --git a/tun.c b/tun.c
index 63aa1f1..35d6418 100644
--- a/tun.c
+++ b/tun.c
@@ -123,7 +123,7 @@ static void tun_afterpoll(void *sst, struct pollfd *fds, int nfds,
 	    fatal_perror("tun_afterpoll: read()");
 	}
 	if (l==0) {
-	    fatal("tun_afterpoll: read()=0; device gone away?\n");
+	    fatal("tun_afterpoll: read()=0; device gone away?");
 	}
 	if (l>0) {
 	    st->buff->size=l;
@@ -211,12 +211,12 @@ static bool_t tun_set_route(void *sst, struct netlink_client *routes)
 		fatal_perror("tun_set_route: ioctl()");
 	    }
 #else
-	    fatal("tun_set_route: ioctl method not supported\n");
+	    fatal("tun_set_route: ioctl method not supported");
 #endif
 	}
 	break;
 	default:
-	    fatal("tun_set_route: unsupported route command type\n");
+	    fatal("tun_set_route: unsupported route command type");
 	    break;
 	}
 	free(network); free(mask);
@@ -255,7 +255,7 @@ static void tun_phase_hook(void *sst, uint32_t newphase)
 		}
 	    }
 	    if (st->fd==-1) {
-		fatal("%s: unable to open any TUN device (%s...)\n",
+		fatal("%s: unable to open any TUN device (%s...)",
 		      st->nl.name,st->device_path);
 	    }
 	} else {
@@ -291,7 +291,7 @@ static void tun_phase_hook(void *sst, uint32_t newphase)
 		    st->interface_name);
 	}
 #else
-	fatal("tun_phase_hook: TUN_FLAVOUR_LINUX unexpected\n");
+	fatal("tun_phase_hook: TUN_FLAVOUR_LINUX unexpected");
 #endif /* LINUX_TUN_SUPPORTED */
     } else if (st->tun_flavour==TUN_FLAVOUR_STREAMS) {
 #ifdef HAVE_TUN_STREAMS
@@ -322,10 +322,10 @@ static void tun_phase_hook(void *sst, uint32_t newphase)
 	sprintf(st->interface_name,"tun%d",ppa);
 	st->fd=tun_fd;
 #else
-	fatal("tun_phase_hook: TUN_FLAVOUR_STREAMS unexpected\n");
+	fatal("tun_phase_hook: TUN_FLAVOUR_STREAMS unexpected");
 #endif /* HAVE_TUN_STREAMS */
     } else {
-	fatal("tun_phase_hook: unknown flavour of TUN\n");
+	fatal("tun_phase_hook: unknown flavour of TUN");
     }
     /* All the networks we'll be using have been registered. Invoke ifconfig
        to set the TUN device's address, and route to add routes to all
@@ -405,11 +405,11 @@ static void tun_phase_hook(void *sst, uint32_t newphase)
 	close(fd);
     }
 #else
-    fatal("tun_apply: ifconfig by ioctl() not supported\n");
+    fatal("tun_apply: ifconfig by ioctl() not supported");
 #endif /* HAVE_NET_IF_H */
     break;
     default:
-	fatal("tun_apply: unsupported ifconfig method\n");
+	fatal("tun_apply: unsupported ifconfig method");
 	break;
     }
 	
@@ -501,7 +501,9 @@ static list_t *tun_create(closure_t *self, struct cloc loc, dict_t *context,
 	    st->ifconfig_type=TUN_CONFIG_IOCTL;
 	    break;
 	case TUN_FLAVOUR_BSD:
-	    st->ifconfig_type=TUN_CONFIG_BSD;
+	    /* XXX on Linux we still want TUN_CONFIG_IOCTL.  Perhaps we can
+	       use this on BSD too. */
+	    st->ifconfig_type=TUN_CONFIG_IOCTL;
 	    break;
 	case TUN_FLAVOUR_STREAMS:
 	    st->ifconfig_type=TUN_CONFIG_BSD;
diff --git a/util.c b/util.c
index 48814c1..883edf5 100644
--- a/util.c
+++ b/util.c
@@ -193,7 +193,7 @@ bool_t add_hook(uint32_t phase, hook_fn *fn, void *state)
 
 bool_t remove_hook(uint32_t phase, hook_fn *fn, void *state)
 {
-    fatal("remove_hook: not implemented\n");
+    fatal("remove_hook: not implemented");
 
     return False;
 }
@@ -215,7 +215,7 @@ struct buffer {
 void buffer_assert_free(struct buffer_if *buffer, string_t file, uint32_t line)
 {
     if (!buffer->free) {
-	fatal("BUF_ASSERT_FREE, %s line %d, owned by %s\n",
+	fatal("BUF_ASSERT_FREE, %s line %d, owned by %s",
 	      file,line,buffer->owner);
     }
 }
@@ -223,7 +223,7 @@ void buffer_assert_free(struct buffer_if *buffer, string_t file, uint32_t line)
 void buffer_assert_used(struct buffer_if *buffer, string_t file, uint32_t line)
 {
     if (buffer->free) {
-	fatal("BUF_ASSERT_USED, %s line %d, last owned by %s\n",
+	fatal("BUF_ASSERT_USED, %s line %d, last owned by %s",
 	      file,line,buffer->owner);
     }
 }