chiark / gitweb /
secnet.git
6 years agoWIP dns transport packets etc. wip.ip-over-dns
Ian Jackson [Wed, 17 Aug 2011 20:51:40 +0000 (21:51 +0100)]
WIP dns transport packets etc.

6 years agoWIP dns transport packets etc.
Ian Jackson [Sun, 14 Aug 2011 10:28:44 +0000 (11:28 +0100)]
WIP dns transport packets etc.

6 years agoWIP dns transport notes
Ian Jackson [Sat, 6 Aug 2011 13:33:36 +0000 (14:33 +0100)]
WIP dns transport notes

6 years agoWIP DNS notes
Ian Jackson [Wed, 3 Aug 2011 08:44:38 +0000 (09:44 +0100)]
WIP DNS notes

6 years agoWIP DNS turn bitenc debugging off by default
Ian Jackson [Mon, 1 Aug 2011 14:23:53 +0000 (15:23 +0100)]
WIP DNS turn bitenc debugging off by default

6 years agoWIP DNS bugfixes and debugging - dns-bitenc-test seems to work
Ian Jackson [Mon, 1 Aug 2011 14:19:02 +0000 (15:19 +0100)]
WIP DNS bugfixes and debugging - dns-bitenc-test seems to work

6 years agoWIP DNS bugfixes and debugging
Ian Jackson [Mon, 1 Aug 2011 13:44:59 +0000 (14:44 +0100)]
WIP DNS bugfixes and debugging

6 years agoWIP DNS bugfixes and debugging
Ian Jackson [Mon, 1 Aug 2011 13:37:27 +0000 (14:37 +0100)]
WIP DNS bugfixes and debugging

6 years agoWIP DNS bugfixes
Ian Jackson [Mon, 1 Aug 2011 11:59:53 +0000 (12:59 +0100)]
WIP DNS bugfixes

6 years agoWIP DNS bitenc test compiles
Ian Jackson [Mon, 1 Aug 2011 11:33:09 +0000 (12:33 +0100)]
WIP DNS bitenc test compiles

6 years agoWIP dns transport
Ian Jackson [Sat, 30 Jul 2011 23:20:13 +0000 (00:20 +0100)]
WIP dns transport

6 years agoWIP dns packet en/decoding test program
Ian Jackson [Thu, 21 Jul 2011 19:02:39 +0000 (20:02 +0100)]
WIP dns packet en/decoding test program

6 years agoWIP dns packet en/decoding
Ian Jackson [Thu, 21 Jul 2011 18:41:29 +0000 (19:41 +0100)]
WIP dns packet en/decoding

6 years agoWIP NEW COMM
Ian Jackson [Thu, 21 Jul 2011 15:31:32 +0000 (16:31 +0100)]
WIP NEW COMM

6 years agocomm,site: Make peer address configuration the responsibility of comm (udp) wip.comm-does-lookup
Ian Jackson [Thu, 21 Jul 2011 16:21:40 +0000 (17:21 +0100)]
comm,site: Make peer address configuration the responsibility of comm (udp)

We want to be able to introduce new kinds of comm.  These may need to
be configured differently to the current udp comm, which expects to be
given a sockaddr_in.

So move the knowledge of how to configure a peer address (ie, which
keys to look up in the site dictionary) to udp.  udp gets passed
site's dictionary during configuration setup, and extracts the
relevant information.  site can then later ask udp to (try to) find
the peer address.

As a side-effect, the resolver key now belongs to comm (ie, udp), not
to the site.  This is a backwards-incompatible change because it means
that (in the usual kind of situation) the resolver must be defined
before the udp comm in the config file.  I don't know what proportion
of existing config files need adjusting but certainly test-example
did.

We could avoid the backward-compatibility problem by having udp look
up the "resolver" key in the supplied site dictionary, and stash the
result away, but this would result in a more-convoluted configuration
setup.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocomm: Move construction of comm_addr behind comm interface.
Ian Jackson [Thu, 21 Jul 2011 13:24:11 +0000 (14:24 +0100)]
comm: Move construction of comm_addr behind comm interface.

Now only the specific comm is allowed to construct a comm_addr; site
no longer does so.  It passes the sockaddr_in containing the address
and port number from the configuration.  This paves the way for more
exciting kinds of comm.

No international functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoWIP, NOTES
Ian Jackson [Thu, 21 Jul 2011 13:01:13 +0000 (14:01 +0100)]
WIP, NOTES

6 years agoMobile sites: Update test example wip.mobile
Ian Jackson [Wed, 20 Jul 2011 18:54:22 +0000 (19:54 +0100)]
Mobile sites: Update test example

Make the "inside" site in test-example be mobile.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoMultiple udp ports: Add to test
Ian Jackson [Wed, 20 Jul 2011 18:03:57 +0000 (19:03 +0100)]
Multiple udp ports: Add to test

For testing, add an extra port to one of the sites in test-example.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoMultiple udp ports for the same site (multiple "comm"s)
Ian Jackson [Sun, 19 Jun 2011 11:02:24 +0000 (12:02 +0100)]
Multiple udp ports for the same site (multiple "comm"s)

Now you can meaningfully specify more than one comm closure (ie, more
than one udp port) for a site.  secnet will respond to incoming key
exchange and data packets on any of the ports.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoMobile sites: Support in make-secnet-sites
Ian Jackson [Wed, 20 Jul 2011 19:03:11 +0000 (20:03 +0100)]
Mobile sites: Support in make-secnet-sites

This allows a sites file to specify that a site is mobile; the mobile
flag gets propagated into the generated sites.conf.

Includes new support for boolean values.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoMobile sites: Use different default tuning parameters
Ian Jackson [Wed, 20 Jul 2011 16:46:37 +0000 (17:46 +0100)]
Mobile sites: Use different default tuning parameters

Links involving mobile peers are best served by somewhat different
tuning parameters.  So make the defaults vary accordingly.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoMobile sites: Require specification of whether we think we are mobile
Ian Jackson [Wed, 20 Jul 2011 16:36:30 +0000 (17:36 +0100)]
Mobile sites: Require specification of whether we think we are mobile

We introduce a new config option "local-mobile" which must match our
peers' idea of whether we are mobile.  This is cross-checked with the
"site" entry for our own site, if there is one, to detect mistakes.

We do not transfer this data in the protocol because we don't want to
break compatibility with older secnets which do not understand mobile
peers at all.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoMobile sites: Maintain multiple addresses for some peers (new feature)
Ian Jackson [Thu, 14 Jul 2011 00:02:55 +0000 (01:02 +0100)]
Mobile sites: Maintain multiple addresses for some peers (new feature)

Provides the following new config option, and some satellite options:

  mobile (bool): if True then peer is "mobile" ie we assume it may change
    its apparent IP address and port number without either it or us
    being aware of the change; so, we remember the last several port/addr
    pairs we've seen and send packets to all of them (subject to a timeout).
    We maintain one set of addresses for key setup exchanges, and another for
    data traffic. [false]

In the code this involves replacing the comm_addrs for setup_peer and
peer with a new struct transport_peers which contains zero or more
comm_addrs.  This subsumes peer_valid, too.

Additionally, we are slightly cleaner about the use of setup_peer: we
ensure that we clean it out appropriately when we go into states where
it won't (shouldn't) be used.

The transport_peers structure is opaque to most of site.c and is
manipulated by a new set of functions which implement the detailed
semantics described in site.c.

The main code in site.c is no longer supposed to call dump_packet and
st->comm->sendmsg directly; it should use transport_xmit (which will
do dump_packet as well as appropriate sendmsgs).

No intentional functional change if mobile=false (the default) and the
new debug log feature ("peer-addrs") is not enabled.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoProtocol change: Initiate key setup on incoming packets, not outgoing ones
Ian Jackson [Wed, 20 Jul 2011 16:02:06 +0000 (17:02 +0100)]
Protocol change: Initiate key setup on incoming packets, not outgoing ones

If data is exchanged after the "renegotiation time", we need to
refresh the data transfer key - ie, to initiate a key setup.
Previously this was done by the peer which wanted to transmit data
using an existing but in-need-of-refreshing key.

However, mobile peers may often be disconnected.  There is no point
trying to do a key renegotiation while they are disconnected.  Thus it
is better to have the renegotiation initiated by a peer which receives
a data packet.  That means that if there is a network outage,
renegotiation will be deferred until the network is restored.

In particular, it means that a mobile node which has no underlying
network but which has applications trying to send data will not waste
effort attempting key renegotiation until it once more has
connectivity.

This minor functional change should be harmless or even beneficial for
non-mobile sites too.  It simply means that the other peer will play
the role of initiator during renegotiation, but since which peer
played this role is arbitrary for non-mobile sites this should make no
difference.

Compatibility: In the case of an old version of secnet talking to a
new version, only data packets in one direction will cause
renegotiation.  This should not be a problem since all real-world
IP protocols involve data in both directions.

So we make the new behaviour universal rather than making it depend on
the forthcoming "mobile-peer" site config option.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocomm, udp: Provide an addr_to_string method
Ian Jackson [Sun, 17 Jul 2011 23:59:50 +0000 (00:59 +0100)]
comm, udp: Provide an addr_to_string method

This method writes (into a single static buffer) a string describing a
comm_addr.  It describes both the comm instance and the peer address.

No callers yet, but one is about to be introduced.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoSite tuning defaults: Improve documentation; internal improvements
Ian Jackson [Wed, 20 Jul 2011 15:09:42 +0000 (16:09 +0100)]
Site tuning defaults: Improve documentation; internal improvements

Two documentation fixes:

* Quote the correct default value for setup-timeout
  (setup_retry_interval) in the README.
* Improve the documentation of the default value for renegotiate-time.

Three simple internal improvements:

* Change definitions of DEFAULT_* to forms which make the semantics
  of the values clearer, thus obviating the need for the comments
  giving human-readable values.
* Add comments to DEFAULT_* time values giving the units (currently,
  they are all in millisecons).
* Rename (in the code) SETUP_TIMEOUT and setup_timeout to
  SETUP_RETRY_INTERVAL, which is more accurate.  We leave the config
  dictionary entry with the misleading name (particularly, since we
  have no facility for spotting obsolete or misspelled config keys).

Also two changes which are preparatory to the mobile peers support:

* New macro DEFAULT(D) which currently simply expands to DEFAULT_*.
* New convenience macro CFG_NUMBER for a common pattern of use for
  dict_read_number, which uses DEFAULT().

No intentional functional changes.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocomm, site: pass a new "struct comm_addr" rather than sockaddr_in
Ian Jackson [Sun, 19 Jun 2011 11:01:08 +0000 (12:01 +0100)]
comm, site: pass a new "struct comm_addr" rather than sockaddr_in

This abstracts away the fact that the peer address is a sockaddr_in;
instead, most of the code in site.c now only handles the peer address
as an opaque structure.

We put the  struct comm_if*  in the address structure too, as this will
be useful later.  Amongst other things, doing this arranges that the
comm client knows which comm is notifying about an incoming packet.
Previously the client was expected to "just know" because the only
actual client in secnet is site which currently only deals with one
comm.

Also make the relevant arguments const-correct.

Contains consequential changes the signatures of many functions but no
intentional functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agosite: When shutting down, if debug enabled, do dump the MSG7
Ian Jackson [Thu, 14 Jul 2011 00:00:45 +0000 (01:00 +0100)]
site: When shutting down, if debug enabled, do dump the MSG7

Insert a call to dump_packet in send_msg7.  The packet is mostly
ciphertext and may not make a great deal of sense but turning on
debugging should not show all management packets; only data packets
should not be shown.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agosite: Remove some spurious "break"s
Ian Jackson [Wed, 20 Jul 2011 15:43:21 +0000 (16:43 +0100)]
site: Remove some spurious "break"s

Remove two occurrences of "break" which immediately follow
unconditional "return".  No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoKeepalives: Document that they're unimplemented; remove vestigial code
Ian Jackson [Wed, 13 Jul 2011 23:52:47 +0000 (00:52 +0100)]
Keepalives: Document that they're unimplemented; remove vestigial code

Keepalives are not actually implemented; the keepalive option does
nothing.  Mention this in the README.  Remove the option's parsing
and recording from site.c.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: Style improvement to python literals in make-secnet-sites
Ian Jackson [Wed, 20 Jul 2011 19:01:18 +0000 (20:01 +0100)]
cleanup: Style improvement to python literals in make-secnet-sites

Python literals can have a trailing comma after the last element.
Doing this is a good idea as it means that adding new elements at the
end doesn't necessitate modifying the previous line.

No intentional functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: provide helpful FILLZERO macro (for certain memset calls)
Ian Jackson [Wed, 13 Jul 2011 22:59:17 +0000 (23:59 +0100)]
cleanup: provide helpful FILLZERO macro (for certain memset calls)

This macro replaces these idioms:
  memset(&foo,0,sizeof(foo));   =>   FILLZERO(foo);
  memset(foo,0,sizeof(*foo));   =>   FILLZERO(*foo);

This makes it impossible to accidentally get the wrong size.

Use this macro in all such patterns in secnet, apart from two in
site.c which are going to be removed soon anyway.

No intentional functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agosecnet.h: provide helpful STRING macro (for preprocessor stringification)
Ian Jackson [Wed, 13 Jul 2011 22:53:55 +0000 (23:53 +0100)]
secnet.h: provide helpful STRING macro (for preprocessor stringification)

Uses preprocessor stringification; can be helpful for rendering
compile-time constants (eg compile time limit macros) for messages,
etc.

Not used yet; will be used later in this series.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoTest example: improve logging, choice of ports
Ian Jackson [Wed, 20 Jul 2011 18:42:58 +0000 (19:42 +0100)]
Test example: improve logging, choice of ports

* Turn up the logging in test-example/common.conf, to "all".

* Change the udp port numbers we use to 16900 and 16910, which
  conveniently have a "0" for "outside" and a "1" for "inside".

* Swap the order of the inside and outside sites in the sites file, so
  that the consistentliy-lower-numbered "outside" comes first.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years ago.gitignore build-stamp.
Richard Kettlewell [Sun, 24 Jul 2011 16:44:08 +0000 (17:44 +0100)]
.gitignore build-stamp.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoIntroduce an installdirs target.
Richard Kettlewell [Sun, 24 Jul 2011 16:03:26 +0000 (17:03 +0100)]
Introduce an installdirs target.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoCorrect a couple of errors in the README.
Richard Kettlewell [Sun, 24 Jul 2011 16:02:34 +0000 (17:02 +0100)]
Correct a couple of errors in the README.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoProhibit lists where single values expected.
Richard Kettlewell [Sun, 24 Jul 2011 16:00:02 +0000 (17:00 +0100)]
Prohibit lists where single values expected.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoDocument XCode 3.2 import.
Richard Kettlewell [Sun, 24 Jul 2011 15:59:53 +0000 (16:59 +0100)]
Document XCode 3.2 import.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoFix version number in a couple of places.
Richard Kettlewell [Sun, 24 Jul 2011 16:41:44 +0000 (17:41 +0100)]
Fix version number in a couple of places.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoSecurity: Reduce impact of bogus key setup packet DoS
Ian Jackson [Wed, 6 Jul 2011 23:22:58 +0000 (00:22 +0100)]
Security: Reduce impact of bogus key setup packet DoS

If a MSG1 (key setup initiation packet) is received containing
expected local and remote site names, the receiving secnet will start
a key setup attempt with details from that packet.

MSG1 packets are (almost necessarily) unauthenticated, so anyone on
the Internet can cause this to happen.  secnet is only willing to have
one key exchange attempt ongoing at once, and will ignore subsequent
incoming MSG1s until it has dealt with the first key exchange attempt.

So this means that an attacker who can send packets to any secnet
instance can DoS secnet at session setup (or key renewal) time.  All
the attacker needs to know is the secnet site names, and the IP
address and port number of one of the secnets.  The attacker does not
need to spoof their IP address or know any secret keys.

If the attacker sends a contant stream of bogus packets they can
probably prevent the link coming up at all.

This is difficult to fix without changing the protocol.

However, there is worse: when the key setup with the bogus peer
eventually fails, as it must, secnet invalidates the current session
key and its note of where to send actual data packets.  It will then
refuse to attempt a new key exchange for a timeout period.  During
this period, data packets will not flow.

This means that sending one fairly easy to construct udp packet can
cause a 20s outage.  Worse, after this one packet has had its effect,
the attacker can prevent the connection being reestablished, as
described above.

In this patch we fix the latter problem.  It is simply a bug that the
session key and data transport peer address (resulting from a previous
successful key exchange) are discarded when a key setup fails.

We also provide a test program "test-example/bogus-setutp-request.c"
which can be used to reproduce the problem.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoTest example: instructions for running under valgrind memcheck.
Ian Jackson [Sat, 2 Jul 2011 17:44:28 +0000 (18:44 +0100)]
Test example: instructions for running under valgrind memcheck.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoTest example: Files for a simple testing configuration now in test-example/
Ian Jackson [Sat, 2 Jul 2011 17:17:37 +0000 (18:17 +0100)]
Test example: Files for a simple testing configuration now in test-example/

Including a set of dummy keys, and dummy IP addresses in 172.18.232.0/28.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild system: add *.pyc to .gitignore
Ian Jackson [Sat, 2 Jul 2011 17:14:27 +0000 (18:14 +0100)]
build system: add *.pyc to .gitignore

Python tends to leave this dropping.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoresolver: support IPv4 address literals
Ian Jackson [Fri, 1 Jul 2011 23:01:31 +0000 (00:01 +0100)]
resolver: support IPv4 address literals

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
IP ADDR LIT size_t strlen

6 years agoBuild system: make several patterns in .gitignore absolute paths
Ian Jackson [Sun, 10 Jul 2011 22:06:35 +0000 (23:06 +0100)]
Build system: make several patterns in .gitignore absolute paths

Specifically, version.c, and the files which are generated by
autoconf/automake but whose names do not unambiguously say so.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoLog write failures on tun device.
Richard Kettlewell [Mon, 11 Jul 2011 18:44:18 +0000 (19:44 +0100)]
Log write failures on tun device.

This is useful when debugging.  Short writes count as failures The
error reporting is rate limited to ensure that the logs are not
flooded.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agosys_cmd error handling improved.
Richard Kettlewell [Mon, 11 Jul 2011 18:44:18 +0000 (19:44 +0100)]
sys_cmd error handling improved.

(1) If the subprocess exits nonzero then the exit status
    is unpicked and logged.
(2) If the exec in the child fails, the command and
    errno string are written to stderr (which should
    end up in secnet's usual log output).
(3) _exit() is used instead of exit(), to avoid
    any possibility of craziness with stdio/atexit/etc.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years ago.gitignore XCode-specific files.
Richard Kettlewell [Sat, 9 Jul 2011 14:19:20 +0000 (15:19 +0100)]
.gitignore XCode-specific files.

Checking those files in doesn't make much sense as they change at the
drop of a hat.

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoUpdate Mac-specific notes.
Richard Kettlewell [Sat, 9 Jul 2011 12:46:28 +0000 (13:46 +0100)]
Update Mac-specific notes.

- how (and why) to enable IP forwarding
- correct 'launchctl unload' invocation

Signed-off-by: Richard Kettlewell <rjk@terraraq.org.uk>
6 years agoExit nonzero if any unknown options are provided on the command line. origin/master
Richard Kettlewell [Sat, 9 Jul 2011 12:17:25 +0000 (13:17 +0100)]
Exit nonzero if any unknown options are provided on the command line.

Signed-off-by: Richard Kettlewell <rjk@greenend.org.uk>
6 years agoMake -d option consistent.
Richard Kettlewell [Sat, 9 Jul 2011 12:17:25 +0000 (13:17 +0100)]
Make -d option consistent.

Previously --debug required an argument and -d did not.  The argument
was in any case ignored.

Signed-off-by: Richard Kettlewell <rjk@greenend.org.uk>
6 years agotun: Drop support for Linux 2.2.
Ian Jackson [Fri, 1 Jul 2011 22:57:15 +0000 (23:57 +0100)]
tun: Drop support for Linux 2.2.

tun.c attempted to guess at runtime what kind of tun interface to use
by looking at uname.  Specifically, for Linux it would do
  TUN_FLAVOUR_BSD if u.release starts 2.2
  TUN_FLAVOUR_LINUX if u.release starts 2.4
and require the user to specify, otherwise.

Linux 2.6 and all later versions use TUN_FLAVOUR_LINUX.  The guessing
code is pretty nasty and Linux 2.2 is very very obsolete, so drop it.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoPoll loop: fix read of unused fds[]
Ian Jackson [Fri, 1 Jul 2011 22:47:10 +0000 (23:47 +0100)]
Poll loop: fix read of unused fds[]

Commit 29672515
 portability: Work around Apple's bizarrely deficient poll() implementation
introduced a read of unitialised data in fds[], because it failed to
honour i->nfds which is 0 on the first iteration.

The symptom is that secnet may, on the first iteration, die due to
mistakenly seeing POLLNVAL.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoEvict built-in getopt() implementation.
Richard Kettlewell [Sun, 19 Jun 2011 16:58:45 +0000 (17:58 +0100)]
Evict built-in getopt() implementation.
It intercepts #include commands that should get the system's version.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoWork around Bison's crazy redeclaration of malloc/free.
Richard Kettlewell [Sun, 19 Jun 2011 16:57:28 +0000 (17:57 +0100)]
Work around Bison's crazy redeclaration of malloc/free.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
6 years agoportability: Scripts and documentation for Mac OS X support.
Richard Kettlewell [Sun, 19 Jun 2011 08:08:17 +0000 (09:08 +0100)]
portability: Scripts and documentation for Mac OS X support.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoportability: Work around Apple's bizarrely deficient poll() implementation.
Richard Kettlewell [Sun, 19 Jun 2011 08:08:17 +0000 (09:08 +0100)]
portability: Work around Apple's bizarrely deficient poll() implementation.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobugfix: generate 32-bit site index in packets in a sane way
Richard Kettlewell [Sun, 19 Jun 2011 08:08:18 +0000 (09:08 +0100)]
bugfix: generate 32-bit site index in packets in a sane way

Use a sequential nonzero integer as site index instead of the address
of the site structure(!).  This value is not secret but does need to
be unique, and truncating the address to 32 bits doesn't guarantee
that.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: Remove pointless double assignment
Richard Kettlewell [Sun, 19 Jun 2011 08:08:17 +0000 (09:08 +0100)]
cleanup: Remove pointless double assignment

-Wsequence-point doesn't like it.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: correct type of key exchange strings from uint8_t* to char*
Richard Kettlewell [Sun, 19 Jun 2011 08:08:17 +0000 (09:08 +0100)]
cleanup: correct type of key exchange strings from uint8_t* to char*

Change type of kx signature and pubkey fields to reflect that they are
strings, squashing a GCC warning.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: buffer size for snprintf should come from sizeof
Ian Jackson [Sun, 19 Jun 2011 12:52:13 +0000 (13:52 +0100)]
cleanup: buffer size for snprintf should come from sizeof

Previously, hardcoded copies of the buffer size literals were used.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoportability: use socklen_t for argument to recvfrom
Ian Jackson [Sun, 19 Jun 2011 12:50:51 +0000 (13:50 +0100)]
portability: use socklen_t for argument to recvfrom

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: fix up the types of input to hashes to const void* from const uint8_t*
Ian Jackson [Sun, 19 Jun 2011 12:48:34 +0000 (13:48 +0100)]
cleanup: fix up the types of input to hashes to const void* from const uint8_t*

It is permissible to pass any object to these functions and expect
them to hash it.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agopackaging: Add LSB headers to init script.
Richard Kettlewell [Sun, 19 Jun 2011 08:08:16 +0000 (09:08 +0100)]
packaging: Add LSB headers to init script.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
6 years agobuild system: Builds OK under dpkg-buildpackage.
Richard Kettlewell [Sun, 19 Jun 2011 08:07:08 +0000 (09:07 +0100)]
build system: Builds OK under dpkg-buildpackage.

  - added a Build-Depends line
  - made 'debian/rules clean' work even if there has been no build.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
6 years agobuild system: provide configure option --enable-hacky-parallel
Ian Jackson [Sun, 12 Jun 2011 19:20:20 +0000 (20:20 +0100)]
build system: provide configure option --enable-hacky-parallel

This allows -DHACKY_PARALLEL to be turned on with an option to configure.
The corresponding comments in the Makefile.in, suggesting to edit it by
hand there, are therefore removed.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agosite setup: actually use calculated default for st->key_renegotiate_time
Ian Jackson [Sun, 12 Jun 2011 18:13:38 +0000 (19:13 +0100)]
site setup: actually use calculated default for st->key_renegotiate_time

We go to some trouble to calculate an appropriate default value for
st->key_renegotiate_time.  However, when we actually do the config
file lookup we overwrote the result and used st->key_lifetime as the
default instead, which is wrong.

The upshot is that prior to this patch, DEFAULT_KEY_RENEGOTIATE_GAP
and the associated logic was unused, and keys were only renegotiated
at the point where they expired, which would produce a small gap in
connectivity.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agosite setup: move st->key_renegotiate_time default calculation nearer use
Ian Jackson [Sun, 12 Jun 2011 18:11:45 +0000 (19:11 +0100)]
site setup: move st->key_renegotiate_time default calculation nearer use

st->key_renegotiate_time is first calculated as a default and then
that value ought to be used as a default for the config lookup.  So
move the default calculation near where the config lookup takes place
to keep the related code together.

Pure code motion; no functional change.  Also, the value computed is
not currently used due to a bug which will be fixed in the next patch.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agosite setup: Correct logic for DEFAULT_KEY_RENEGOTIATE_GAP
Ian Jackson [Sat, 11 Jun 2011 14:10:08 +0000 (15:10 +0100)]
site setup: Correct logic for DEFAULT_KEY_RENEGOTIATE_GAP

Previously, key_renegotiate_time would be set to
   key_lifetime - DEFAULT_KEY_RENEGOTIATE_GAP
unless that was negative, in which case it would be set to
   key_lifetime / 2
This is illogical as it is not monotonic in key_lifetime.
Instead we now set it to the larger of those two values.

(This bug has had no effect as the buggy calculation was ignored due
to another bug, which will be fixed in a later patch.)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoevent loop: remove now parameter from site_settimeout
Ian Jackson [Sat, 11 Jun 2011 12:26:49 +0000 (13:26 +0100)]
event loop: remove now parameter from site_settimeout

This is now available as a global variable.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoevent loop: remove now and tv_now from before/afterpoll API
Ian Jackson [Sat, 11 Jun 2011 12:26:14 +0000 (13:26 +0100)]
event loop: remove now and tv_now from before/afterpoll API

beforepoll/afterpoll routines now use the global variables provided.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoevent loop: make tv_now and now into globals
Ian Jackson [Mon, 16 May 2011 10:09:13 +0000 (11:09 +0100)]
event loop: make tv_now and now into globals

This makes these values available to all functions in the rest of the
program, even if the particular event before/after function doesn't
pass them on.

We give variables new names so that we can introduce convenience
pointer aliases; that way, all the other references in the whole
program can remain unchanged when we remove now and tv_now from the
beforepoll/afterpoll parameter lists.

The pointer aliases are "static const *const" which is somewhat ugly.
We do this in the hope that the compiler will optimise out the actual
variable, which it couldn't do if the pointer alias had external
linkage.  The alternative would be a macro but we don't want to
local scopes (and not just in the current before/afterpoll api).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: Turn on -Werror
Ian Jackson [Sat, 11 Jun 2011 11:16:33 +0000 (12:16 +0100)]
cleanup: Turn on -Werror

Having cleaned up the code to be warning-free we can now turn on -Werror.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobugfix: -fno-strict-aliasing, because tun.c breaks the rules.
Richard Kettlewell [Sun, 19 Jun 2011 08:08:16 +0000 (09:08 +0100)]
bugfix: -fno-strict-aliasing, because tun.c breaks the rules.

(What it does is: fills in 'struct sockaddr' fields of 'struct
rtentry' by casting to 'struct sockaddr_in' pointers and accessing
through those.  A more intrusive change would be to construct the
sockaddr_in in another object and then memcpy it into place, which
would achieve the same effect without (AFAIK) breaching C's aliasing
rules.)

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: remove unused "line" member in struct transform
Ian Jackson [Sun, 19 Jun 2011 14:47:03 +0000 (15:47 +0100)]
cleanup: remove unused "line" member in struct transform

This member is never set or used.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agoportability: warn if provided snprintf.c is used
Ian Jackson [Sun, 19 Jun 2011 14:46:01 +0000 (15:46 +0100)]
portability: warn if provided snprintf.c is used

We don't have any visibility of changes to fix bugs in the snprintf
which is supplied with secnet.  Therefore at the very least we should
print a warning if we turn out to use it, rather than perhaps silently
using an ancient and perhaps-buggy implementation.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agointeger arithmetic types: do not use unsigned for site timeouts etc.
Ian Jackson [Sun, 19 Jun 2011 15:25:26 +0000 (16:25 +0100)]
integer arithmetic types: do not use unsigned for site timeouts etc.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agointeger arithmetic types: do not use unsigned for lengths
Ian Jackson [Sun, 12 Jun 2011 21:34:09 +0000 (22:34 +0100)]
integer arithmetic types: do not use unsigned for lengths

In C it is not normally a good idea to use an unsigned integer type
for integer values, even if they are known not ever to be zero (for
example, because they are lengths).  This is because C unsigned
arithmetic has unhelpful behaviour when the values would become
negative.

In particular, comparing signed and unsigned integers, and doing
arithmetic (especially subtraction) when unsigned integers are
present, can be dangerous and lead to unexpected results.

So fix the resulting warnings (which are due to -Wsign-compare which
comes from -W) by making all lengths, counts (and iterators over them)
and return values from scanf be of signed types, usually int32_t
instead of uint32_t (but occasionally int).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agointeger arithmetic types: make get_uint32, get_uint16 return the correct type
Ian Jackson [Sun, 12 Jun 2011 21:37:24 +0000 (22:37 +0100)]
integer arithmetic types: make get_uint32, get_uint16 return the correct type

Previously get_uint32 and get_uint16 would return whatever the usual
arithmetic conversions produced.  (The previous code was not in fact
even guaranteed to work properly on a machine with 16-bit ints.)

Now we cast the individual bytes before shifting.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agointeger arithmetic types: correct perhaps-possible negative timeout situation
Ian Jackson [Sun, 12 Jun 2011 19:35:47 +0000 (20:35 +0100)]
integer arithmetic types: correct perhaps-possible negative timeout situation

site_settimeout assumes that its timeout parameter is not before now.
Following the logic of the code this would appear to be currently
true, although I'm not absolutely certain.

Nevertheless it would be better to avoid this assumption.  Instead,
use a signed variable for the time until the timeout, and explicitly
turn negative values into zero.

The use of an int64_t will not cause an arithmetic overflow provided
that no timeouts are more than 2^64 milliseconds (around 580x10^6 yr)
in the past or the future.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agointeger and buffer overflows: introduce safe_malloc_ary
Ian Jackson [Sun, 12 Jun 2011 21:28:33 +0000 (22:28 +0100)]
integer and buffer overflows: introduce safe_malloc_ary

When allocating an array, it is necessary to check that the
multiplication (to compute the size in bytes) does not overflow.

Do this in a new function safe_malloc_ary, which we call in both the
places where safe_malloc was previously used with an unchecked
multiplication.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agointeger and buffer overflows: introduce a number of asserts
Ian Jackson [Sun, 12 Jun 2011 21:23:15 +0000 (22:23 +0100)]
integer and buffer overflows: introduce a number of asserts

In various places we add and increment integers, hoping that they
don't overflow.  We also prepend and append things to our internal
buffer, which is of fixed size, without checking that they will fit.

This means that malicious configuration (for example, long site names)
might be able to take over the secnet program.

So, add a whole lot of checking.  Many of these places don't have a
sensible way to return an error; in those cases we assert.  Some of
the checks are off-by-one in the sense that they say "assert(x<...)"
when "<=" would be OK too.  This is done to avoid having to think too
hard about fenceposts, as it's a simple way to avoid introducing bugs.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
6 years agopossible security fix: do not call slilog with intended message as format string
Ian Jackson [Sun, 12 Jun 2011 19:00:10 +0000 (20:00 +0100)]
possible security fix: do not call slilog with intended message as format string

vMessage would call slilog with part of the intended log message as
the format string.  This is a potential format string vulnerability,
detected by -Wformat-security.

I have not analysed the code in detail to determine in exactly which
circumstances a secnet installation will be vulnerable, but in general
a vulnerability (at least for DOS) will exist in any situation where
an attacker can cause a log message to contain things which look like
printf directives.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agologging: provide vslilog as well as slilog
Ian Jackson [Sun, 12 Jun 2011 19:03:33 +0000 (20:03 +0100)]
logging: provide vslilog as well as slilog

In general it is a mistake to provide a varargs function which takes a
format string, but not the corresponding function taking a va_list.

We implement slilog in terms of vslilog in the obvious way.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: specify never-interactive option for flex scanner
Ian Jackson [Sat, 11 Jun 2011 11:11:15 +0000 (12:11 +0100)]
cleanup: specify never-interactive option for flex scanner

We never parse configuration interactively.  That's just as well,
because without "%option never-interactive" flex generates a redundant
declaration of isatty which upsets -Wredundant-decls.

This is a bug in flex IMO but the workaround is fine for secnet.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: use flex-generated declarations for scanner interface
Ian Jackson [Sat, 11 Jun 2011 11:07:48 +0000 (12:07 +0100)]
cleanup: use flex-generated declarations for scanner interface

We now pass the --header=... option to flex.  This causes it to
generate a header file describing the flex interface.  We #include
this almost everywhere we #include conffile_internal.h, and that makes
the declarations of yyin and yylex in conffile_internal.h redundant so
remove them.

Note that flex generates a conffile.yy.c which also contains many of
the same declarations as in conffile.yy.h.  So, unfortunately, we
should not #include conffile.yy.h in conffile.hh.c and therefore not
in conffile_internal.h either (if we want -Wredundant-decls, which we do).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: move declaration of version[] into secnet.h
Ian Jackson [Sun, 5 Jun 2011 12:58:06 +0000 (13:58 +0100)]
cleanup: move declaration of version[] into secnet.h

Also, #include secnet.h in the autogenerated version.c.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: fix up the type of string buffers
Ian Jackson [Wed, 25 May 2011 20:00:20 +0000 (21:00 +0100)]
cleanup: fix up the type of string buffers

vsnprintf expects a char*, and the format string passed to ->log does
too, so make buffers be char[].

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
6 years agocleanup: turn off some unused flex options
Ian Jackson [Wed, 25 May 2011 20:00:06 +0000 (21:00 +0100)]
cleanup: turn off some unused flex options

We do not use yyunput or yyinput.  Turning them off slightly improves
the scanner performance (not that that's important) but also prevents
"defined but not used" compiler warnings.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: remove other redundant declarations
Ian Jackson [Sun, 5 Jun 2011 12:59:01 +0000 (13:59 +0100)]
cleanup: remove other redundant declarations

These declarations are now in secnet.h.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: remove redundant "init_module" declarations
Ian Jackson [Sat, 11 Jun 2011 10:18:21 +0000 (11:18 +0100)]
cleanup: remove redundant "init_module" declarations

These declarations are now provided in secnet.h and should not appear
in individual .c files.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: move declarations of external objects into header files
Ian Jackson [Mon, 16 May 2011 10:13:59 +0000 (11:13 +0100)]
cleanup: move declarations of external objects into header files

It is not a good idea to declare external objects in .c files.  Every
external object (ie, object with external linkage) should be declared
exactly once in a .h file, and every .c file that refers to it or
defines it should #include that header.

When combined with appropriate compiler warnings, this ensures that
every file sees the same signature for every such object.  (At least
for functions.)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: add many compiler warning options
Ian Jackson [Wed, 25 May 2011 20:00:36 +0000 (21:00 +0100)]
cleanup: add many compiler warning options

This is a set of warning options which are useful and IMO should be
enabled in secnet.  Currently the code is not warning-clean to the
required standard; subsequent changes will fix the warnings.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild system: commit configure
Ian Jackson [Sun, 12 Jun 2011 21:59:39 +0000 (22:59 +0100)]
build system: commit configure

Commit configure (as generated by autoconf 2.67-2 Debian i386) into
the repo and remove it from .gitignore.

This now means that, provided you don't want to modify configure.in,
you can build secnet from a git clone without autoconf installed and
without any worries about the autoconf version.

If you _do_ modify configure.in, the resulting changes to configure
will end up in the repo, just as previously changes to config.h.in
were committed too.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild system: rerun autoheader
Ian Jackson [Sun, 12 Jun 2011 21:58:57 +0000 (22:58 +0100)]
build system: rerun autoheader

Run a recent autoheader (autoconf 2.67-2 from Debian i386) and commit
the resulting config.h.in.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild system: remove *~ files in clean target
Ian Jackson [Sun, 5 Jun 2011 12:43:54 +0000 (13:43 +0100)]
build system: remove *~ files in clean target

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agocleanup: add a .dir-locals.el
Richard Kettlewell [Sun, 19 Jun 2011 13:20:54 +0000 (14:20 +0100)]
cleanup: add a .dir-locals.el

Ensures indent level is right for Emacs users.

Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild system: add a .gitignore
Ian Jackson [Sun, 5 Jun 2011 12:43:16 +0000 (13:43 +0100)]
build system: add a .gitignore

Ignore all files generated by running autoconf, ./configure, and make,
and the debian build, and editor backup files.

(The files generated by running autoheader are committed and therefore
we don't add them to .gitignore.  There is probably a mistake here,
which will be fixed later in this series.)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Richard Kettlewell <richard@greenend.org.uk>