chiark / gitweb /
secnet.git
4 years agohash: Put hash state on the caller's stack
Ian Jackson [Sun, 29 Sep 2019 12:29:09 +0000 (13:29 +0100)]
hash: Put hash state on the caller's stack

This makes the code simpler too!

We rename len to slen, to distinguish hlen and slen (to help avoid
bugs where the wrong amount is allocated).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Break out slog_start
Ian Jackson [Sun, 29 Sep 2019 12:01:46 +0000 (13:01 +0100)]
site: Break out slog_start

This will allow callers in site.c to build up messages bit by bit.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Pass msg into generate_msg
Ian Jackson [Sun, 29 Sep 2019 11:00:33 +0000 (12:00 +0100)]
site: Pass msg into generate_msg

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Pass msg into enter_new_state
Ian Jackson [Sun, 29 Sep 2019 10:46:22 +0000 (11:46 +0100)]
site: Pass msg into enter_new_state

The rules for when this is initialised, in site_incoming, are a
slightly complicated, so document them.

Examination of these rules reveals that the msg argument to
process_msg1 should be const, since process_msg1 (unlike the other
process_msgN functions) receives this, rather than generating it.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Move main `struct msg' into site_incoming
Ian Jackson [Sun, 29 Sep 2019 10:35:30 +0000 (11:35 +0100)]
site: Move main `struct msg' into site_incoming

We are going to want this in more places, and this is going to involve
threading it through site_incoming.  So make this a local variable
there, rather than in each of the process_msgN functions.

We rename the variable `named_msg' to `msg': it was called `named_msg'
because it was only valid after our calls to named_for_us, but now it
is valid after process_msgN too.

No overall functional change, except that stack usage is improved (by
removing a copy of struct msg).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Change `struct msg m' to `struct msg m[1]'
Ian Jackson [Sun, 29 Sep 2019 10:30:53 +0000 (11:30 +0100)]
site: Change `struct msg m' to `struct msg m[1]'

We are going to make this a pointer in a moment.  That implies a lot
of mechanical changes.  This [1] trick lets us do those changes now in
a separate patch, which makes things clearer.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoCOPY_OBJ: we use sizeof(dst) so relax restriction on src
Ian Jackson [Fri, 27 Sep 2019 23:14:47 +0000 (00:14 +0100)]
COPY_OBJ: we use sizeof(dst) so relax restriction on src

No code change, just interface docs.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosig: Move hashing into algorithm
Ian Jackson [Fri, 27 Sep 2019 18:09:22 +0000 (19:09 +0100)]
sig: Move hashing into algorithm

I think it should be up to the pk algorithm to decide on the hash
function, at least in the usual case.  When we have key rollover and
proper enrolment, a public key declaration by a site should specify
precisely the validation algorithm including the hash function.

For `rsa' we can't do that because in theory people might have bound
the `hash' config key to something unusual.  So provide a way for that
to work.  The approach is to have site.c (the only caller of the sig
closures) find out whether to do the `hash' config key lookup by
seeing whether the pk algorithm wants it.

Then we can move all the hash-related machinations into rsa.c.  (A
future pk algorithm can do this a lot more simply by calling the
appropriate hash functions directly.)

An effect is to move the allocation of the hash result buffer from
per-packet to initialisation (!)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosig: Move unmarshalling responsibility into algorithm
Ian Jackson [Fri, 27 Sep 2019 17:40:42 +0000 (18:40 +0100)]
sig: Move unmarshalling responsibility into algorithm

Because site wants to first unpick the packet, and only later actually
check the signature, we provide two entrypoints.  The first, `unpick',
basically just computes the length.  So the result of `unpick' is
simply a note of the part of the buffer which contains the signature.

The alternative would be to have site.c handle the length, so there
would be one entrypoint `check' which would get a byte block.  This
would move complexity from the `unpick'/`check' interface to the
`sign' interface (which would have to negotiate about space).  It
would mean that for algorithms where signatures are of fixed size, we
couldn't omit the length field.

rsa.c needs to do some shenanigans: because it wants to use
mpz_set_str (for historical reasons), it needs the buffer to be
nul-terminated.  So `unpick' checks that there will be a spare byte
afterwards into which we can write the nul.  `check' writes the nul -
and puts the previous character back, so that we don't have to write
weird stuff in the algorithm api.  Doing better than this would be
turd-polishing since this algorithm is obsolete.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosig: Move marshalling responsibility into sign function
Ian Jackson [Fri, 27 Sep 2019 17:37:40 +0000 (18:37 +0100)]
sig: Move marshalling responsibility into sign function

This is the first part of making the pk algorithm responsible for
understanding its signature format.

The sign function is expected to produce some bytes which (in a
moment) its companion functions will be able to parse.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosig: Make closure interface not contain sig alg name "rsa"
Ian Jackson [Thu, 26 Sep 2019 21:29:02 +0000 (22:29 +0100)]
sig: Make closure interface not contain sig alg name "rsa"

We intend to be able to support other signature algorithms.  This will
be done with this closure, but it ought to have a generic name.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Prepare for adding more MSG3 variants
Mark Wooding [Sun, 30 Apr 2017 23:11:25 +0000 (00:11 +0100)]
site: Prepare for adding more MSG3 variants

  * Introduce a macro listing the known MSG3 variants.  Use this in
    `type_is_msg34' and `site_incoming', and in the `process_msg3'
    molly-guard.

  * Break out MSG3-ish label minor numbers and analyse them using the
    sensible ordering, in `generate_msg' and `unpick_msg'.

  * Have `check_msg' fall back to trusting `process_msg3' for all
    MSG3-ish messages.  (It already has a more vicious molly-guard
    anyway.)

  * Reformat the decision tree in `generate_msg3' so that adding more
    branches is cleaner.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosite: Replace remote's caps after verifying MSG3
Mark Wooding [Sat, 28 Sep 2019 15:13:45 +0000 (16:13 +0100)]
site: Replace remote's caps after verifying MSG3

Previously we'd just `or' the new capability bits into
`st->remote_capabilities' prior to verification, with the rather
unfortunate result that an adversary could convince us that the remote
site has features which it doesn't, in fact, implement.  At present, the
worst effect here is preventing key-exchange from completion, but it's
imaginable that future capability bits have worse effects.

Instead, (a) simply replace our idea of the remote site's capabilities
rather than accumulating a union of all mentioned capabilities, and (b)
do this /after/ verifying the signature on the message.  (This is safe
because there's no mention of `st->remote_capabilities' in the
intervening code in `process_msg3', or in the common message-
verification code in `process_msg3_msg4'.)

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agogitignore: add msgcode-test files
Ian Jackson [Sat, 28 Sep 2019 11:08:16 +0000 (12:08 +0100)]
gitignore: add msgcode-test files

This was erroneously omitted in 7b2ef2245c06
 "magic.h: Present message labels as an encoding of major and minor numbers."

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agostyle: util.[ch]: Introduce hex_encode_alloc name
Ian Jackson [Sat, 28 Sep 2019 10:55:17 +0000 (11:55 +0100)]
style: util.[ch]: Introduce hex_encode_alloc name

Prompted by review of 7be31e47b2a8
  "util.[ch]: Factor out hex encoding and decoding utilities."
which says
  The interface is a bit odd, but it will fit with the uses
  I have in mind.

Not sure if it's the encode or decode interface which is referred to.
Certainly there should be a non-allocating variant.  I decided to
rename the allocating one.

The two separate buffer arguments to hex_decode are indeed a bit odd
but IMO tolerable.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agostyle: util.[ch]: Move doc comments into header file
Ian Jackson [Sat, 28 Sep 2019 10:51:01 +0000 (11:51 +0100)]
style: util.[ch]: Move doc comments into header file

I think doc comments belong (only) in the header.

Spotted this anomaly during review of 7be31e47b2a8
  "util.[ch]: Factor out hex encoding and decoding utilities."
(although it wasn't introduced there).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agostyle: util.h: Adjust comment style twice
Ian Jackson [Sat, 28 Sep 2019 10:44:15 +0000 (11:44 +0100)]
style: util.h: Adjust comment style twice

Prompted by review of 7be31e47b2a8
  "util.[ch]: Factor out hex encoding and decoding utilities."

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoconffile.fl: Use %option noyywrap rather than providing yywrap
Ian Jackson [Sat, 28 Sep 2019 10:39:04 +0000 (11:39 +0100)]
conffile.fl: Use %option noyywrap rather than providing yywrap

Prompted by review of fe0c91cce702
  "configure.in, conffile.fl: Remove dependency on `libfl.a'."

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite.c, magic.h, NOTES: Make early capabilities be dynamic.
Mark Wooding [Sun, 30 Apr 2017 23:18:39 +0000 (00:18 +0100)]
site.c, magic.h, NOTES: Make early capabilities be dynamic.

Replace the `CAPAB_EARLY' macro by a site member variable
`st->early_capabilities'.  The variable is always zero for now, like the
old macro, so there's no functional change.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosecnet.8: Describe capability negotiation in its own section.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
secnet.8: Describe capability negotiation in its own section.

The notion is a little complicated, and we can give it the space it
deserves.  Also, this saves on a lot of repeated text, especially if we
add more things which require assignment of capability bits.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosite.c: Abstract out the various parts of capability handling.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
site.c: Abstract out the various parts of capability handling.

Introduce macros for: setting the local capability flags from
crypto-algorithm closures; selecting a crypto algorithm based on the
capabilities reported by a peer site; and finding the local closure
based on the peer's algorithm decision.

This will make introducing new kinds of negotiation much less painful.
No functional change.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agomagic.h: Present message labels as an encoding of major and minor numbers.
Mark Wooding [Sun, 30 Apr 2017 23:03:08 +0000 (00:03 +0100)]
magic.h: Present message labels as an encoding of major and minor numbers.

The encoding is strange for historical reasons, but represents all pairs
of 16-bit major and minor codes.

I've exhaustively verified that the encoding is invertable, and that it
reproduces the old manually assigned labels; this program is
`msgcode-test.c', which I've added to the standard test run, though it's
rather slow to run.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agomagic.h: Put the CAPAB_... definitions together, under the big comment.
Mark Wooding [Sun, 30 Apr 2017 22:59:03 +0000 (23:59 +0100)]
magic.h: Put the CAPAB_... definitions together, under the big comment.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosite.c: Rename `remote_transforms' in `process_msg2'.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
site.c: Rename `remote_transforms' in `process_msg2'.

As part of the drive to eliminate the idea of specific `transform
capabilities'.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosecnet.8, magic.h: Rephrase documentation of `capab-num' settings.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
secnet.8, magic.h: Rephrase documentation of `capab-num' settings.

In particular, I've abolished the idea of a specific class of `transform
capabilities'.  They're all just capabilities, and they need to mean the
same thing at both ends.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agomagic.h, etc.: Rename the transform capability bits.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
magic.h, etc.: Rename the transform capability bits.

Mostly mechanical, with the following rune:

git grep -zil TRANSFORMNUM | xargs -0r sed -i '
s/TRANSFORMNUM/BIT/g
s/transformnum/bit/g
s/BIT_ANCIENT/&TRANSFORM/g'

But I renamed the `capab_transformnum' member of `struct msg' back by
hand, because it's referring specifically to a selected
transform.  (This will make sense later.)

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosecnet-wireshark.lua: Add a Wireshark dissector.
Mark Wooding [Thu, 13 Jul 2017 11:30:57 +0000 (12:30 +0100)]
secnet-wireshark.lua: Add a Wireshark dissector.

(Some parts are a little strange, because it's been sent from the
future: it's structured to cope with protocol changes which haven't
happened yet.)

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoutil.[ch]: Factor out hex encoding and decoding utilities.
Mark Wooding [Wed, 26 Apr 2017 10:53:05 +0000 (11:53 +0100)]
util.[ch]: Factor out hex encoding and decoding utilities.

Also improve the decoder's error handling.  The interface is a bit odd,
but it will fit with the uses I have in mind.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agopolypath.c: Fix missing include of <limits.h>.
Mark Wooding [Fri, 28 Apr 2017 18:41:30 +0000 (19:41 +0100)]
polypath.c: Fix missing include of <limits.h>.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoconfigure.in, conffile.fl: Remove dependency on `libfl.a'.
Mark Wooding [Sat, 21 Sep 2019 13:35:44 +0000 (14:35 +0100)]
configure.in, conffile.fl: Remove dependency on `libfl.a'.

The `libfl' library contains two functions:

  * `main', which basically just calls `yylex' a lot, as an easy way to
    write simple programs in lex(1); and

  * `yywap', which lets a lex(1)-generated lexer know what to do when it
    encounters end-of-file.  Specifically, it can return nonzero to say
    `that's it, we're done', or zero to say `there's more: I've set up
    ``yyin'' so that you can read more stuff'.

The library doesn't do anything very sensible for `yywrap': it just
always returns 1.  (If you wanted to do something more complicated, you
should just write `yywrap' yourself.)

Secnet has its own `main' function which is fine.  It wants `yywrap',
though.  This causes trouble with upstream `flex', which nowadays builds
a shared `libfl.so' library.  This contains /both/ `yywrap' /and/
`main', which breaks the `configure' test: what happens is that the test
program requires `yywrap', which brings in `libfl.so', which brings in
its `main', which refers to an undefined symbol `yylex' that's not
defined in the test program.  This doesn't go wrong in Debian, because
Debian replaces the shared-library `libfl.so' with a linker script which
says `oh, no, you don't want this: you want that ``libfl_pic.a'' over
there'.  The latter is a traditional archive, and ld(1) can pick
`yywrap' out of it without pulling in the bogus `main' and its
dependency on `yylex'.

Anyway, this is all more trouble than it's worth.  Define our own
`yywrap' in `conffile.fl', and delete the `configure' machinery.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoNOTES: Fix text, now than an early bit exists.
Mark Wooding [Sat, 21 Sep 2019 13:00:30 +0000 (14:00 +0100)]
NOTES: Fix text, now than an early bit exists.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoMakefile.in: Drop dist target
Ian Jackson [Sat, 21 Sep 2019 11:42:29 +0000 (12:42 +0100)]
Makefile.in: Drop dist target

It is much easier to do this with dgit sbuild, like the release
checklist now suggests.  People who don't want to use dgit or sbuild
could use git-archive.  dist targets are IMO obsolete.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoMakefile.in: Completely overhaul release checklist
Ian Jackson [Sat, 21 Sep 2019 11:43:35 +0000 (12:43 +0100)]
Makefile.in: Completely overhaul release checklist

This is roughly what I did for 0.4.4 and exactly what I did for 0.4.5.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoMakefile.in: VERSION: Use =, not :=
Ian Jackson [Sat, 21 Sep 2019 11:42:59 +0000 (12:42 +0100)]
Makefile.in: VERSION: Use =, not :=

This has no effect on make since this variable doesn't contain other
variable references.  But it makes the line directly c&p-able into a
shell.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agochangelog: start 0.4.6
Ian Jackson [Sat, 21 Sep 2019 11:36:20 +0000 (12:36 +0100)]
changelog: start 0.4.6

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoFinalise 0.4.5 v0.4.5
Ian Jackson [Sat, 21 Sep 2019 11:04:53 +0000 (12:04 +0100)]
Finalise 0.4.5

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agochangelog: Changes since 0.4.4
Ian Jackson [Sat, 21 Sep 2019 11:02:47 +0000 (12:02 +0100)]
changelog: Changes since 0.4.4

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoINSTALL: Mention that rsa key generation might need ssh-keygen1
Ian Jackson [Tue, 26 Apr 2016 13:23:07 +0000 (14:23 +0100)]
INSTALL: Mention that rsa key generation might need ssh-keygen1

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoudp.c: Add a comment about the salen cast
Ian Jackson [Sat, 21 Sep 2019 10:05:21 +0000 (11:05 +0100)]
udp.c: Add a comment about the salen cast

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoudp.c: Add explicit cast to muffle bogus Clang warning.
Mark Wooding [Thu, 19 Sep 2019 20:01:24 +0000 (20:01 +0000)]
udp.c: Add explicit cast to muffle bogus Clang warning.

Clang is complaining (`-Wsign-compare') about the comparison between
`salen' (`socklen_t', i.e., an `int' with a false moustache) and
`size_t' (`unsigned int' in this case).  I can see that some warnings of
this kind are useful, but not this one.  The usual arithmetic
conversions apply, so `salen' is converted to `size_t'.  If it was
negative before, it's now very positive, which will trip the the
comparison and call `FAIL' -- which seems like a plausible outcome.

Muffle the warning by adding an explicit cast.  This is ugly and
pointless, though: other suggestions are welcome.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agolog.c: Spray extra `FORMAT(...)' attributes to muffle Clang warnings.
Mark Wooding [Thu, 19 Sep 2019 20:01:22 +0000 (20:01 +0000)]
log.c: Spray extra `FORMAT(...)' attributes to muffle Clang warnings.

Clang gets really upset about non-literal format strings, unless it can
check from the function attributes that you're playing by the rules.

I've taken the liberty of simplifying the annotations on static
functions: rather than duplicating the entire argument list, it suffices
to attach the attribute to the start of the function definition.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite.c (we_have_priority): Fix unintended `&&'.
Mark Wooding [Thu, 19 Sep 2019 20:01:23 +0000 (20:01 +0000)]
site.c (we_have_priority): Fix unintended `&&'.

`CAPAB_PRIORITY_MOBILE' is 0x80000000, which is nonzero, so that doesn't
change the outcome.  So the code is only checking whether the local and
remote capabilities overlap at all, which seems unhelpful.

Instead, check that both advertise `CAPAB_PRIORITY_MOBILE' here.

Spotted by Clang.

The effect is that a new secnet would always think the peer had
advertised CAPAB_PRIORITY_MOBILE.  This might (with roughly 50%
probability) mess up resolution of crossed key setup attempts
involving a mobile end and mixed secnet versions.

The consequences would be mitigated by 19074a85692b
  site: Randomise key setup retry time
so the key setup would very likely eventually succeed.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoFix bizarre `if (!consttime_memeq(X, Y, N)!=0)' idioms.
Mark Wooding [Thu, 19 Sep 2019 20:01:22 +0000 (20:01 +0000)]
Fix bizarre `if (!consttime_memeq(X, Y, N)!=0)' idioms.

Clang thinks the `!' is in the wrong place.  I think the `!=0' isn't
doing any work, so I've deleted it.

This stems from 5ad34db2ccbb "memcmp: Introduce and use
consttime_memeq",

  -    if (memcmp(m->nR,st->remoteN,NONCELEN)!=0) {
  +    if (!consttime_memeq(m->nR,st->remoteN,NONCELEN)!=0) {

at which time the !=0 was already redundant and became more confusing.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoMakefile.in: Support installation from a `VPATH' build.
Mark Wooding [Thu, 19 Sep 2019 20:01:24 +0000 (20:01 +0000)]
Makefile.in: Support installation from a `VPATH' build.

I was pleasantly surprised that Secnet cross-compiles without
significant trouble, but then tripped over this at the very end.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agochangelog: start 0.4.5~
Ian Jackson [Sun, 8 Sep 2019 22:06:06 +0000 (23:06 +0100)]
changelog: start 0.4.5~

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agofinalise 0.4.4 v0.4.4
Ian Jackson [Sun, 8 Sep 2019 21:53:19 +0000 (22:53 +0100)]
finalise 0.4.4

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agochangelog: Document protocol incompatibility
Ian Jackson [Sun, 8 Sep 2019 21:51:14 +0000 (22:51 +0100)]
changelog: Document protocol incompatibility

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agochangelog: Document changes
Ian Jackson [Sun, 8 Sep 2019 21:43:36 +0000 (22:43 +0100)]
changelog: Document changes

... since "Administrivia: Fix erroneous GPL3+ licence notices".

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: in "entering state RUN", say whether key is set up
Ian Jackson [Sat, 18 May 2019 01:25:34 +0000 (02:25 +0100)]
site: in "entering state RUN", say whether key is set up

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agosite: Randomise key setup retry time
Ian Jackson [Sat, 18 May 2019 00:49:14 +0000 (01:49 +0100)]
site: Randomise key setup retry time

This reduces the chance that retries (at both ends of a link, or
within a single secnet) end up synchronised.  Such synchronisation is
not supposed to matter but in practice there have been some bugs where
it does, and it is undesirable anyway.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agorandom: Admit that we will never add error checking everywhere
Ian Jackson [Sat, 18 May 2019 00:42:55 +0000 (01:42 +0100)]
random: Admit that we will never add error checking everywhere

Literally no-one checks this return value.  Abolish it.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agosite: Replace wait_timeout variable with function
Ian Jackson [Sat, 18 May 2019 00:32:45 +0000 (01:32 +0100)]
site: Replace wait_timeout variable with function

We are going to wait for stochastic amounts.  For now, rname the
variable and change all the users to call this function instead.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agopolypath asymmetric routing: Handle MSG2-4 late dupes
Ian Jackson [Wed, 15 May 2019 20:59:18 +0000 (21:59 +0100)]
polypath asymmetric routing: Handle MSG2-4 late dupes

If we get a MSG2-4 and it seems too late for the protocol stage, and
it had the same nonces, it was probably an old packet via a different
route.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agopolypath asymmetric routing: Handle MSG1 late dupes
Ian Jackson [Wed, 15 May 2019 20:55:55 +0000 (21:55 +0100)]
polypath asymmetric routing: Handle MSG1 late dupes

If we get a MSG1 and it seems too late for the protocol stage, and it
had the same peer nonce, it was probably an old packet via a different
route.

In theory it might seem like we should do this in SENTMSG3 and 5 too
because we might have had concurrent setup, and higher priority, so the
peer also sent us MSG2 or 4.  But mobile peers have priority.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agopolypath asymmetric routing: Handle data packet dupes
Ian Jackson [Wed, 15 May 2019 20:40:40 +0000 (21:40 +0100)]
polypath asymmetric routing: Handle data packet dupes

If polypath has asymmetric routing, where path A upload is faster than
path B, but path A download is completely broken, then we need to
retain both paths A and B in our list of transport peers.

Stepping back, we need to treat dupes of recent packets as evidence
that the peer is at that address, even though we do not pass the
packets to the netlink.

We don't want to do this for arbitrarily old data packets.  The
heuristic we use here is rather crude: packets which are more than
32 (by default) out of order are treated as too old.  This will be too
short if path A is a high-bandwidth link and path B is quite slow, and
it will be too long if the link is very idle.  Hopefully this will not
matter in practice.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoIntroduce transform_apply_seqdupe
Ian Jackson [Tue, 14 May 2019 23:42:16 +0000 (00:42 +0100)]
Introduce transform_apply_seqdupe

This distinguishes the two cases.  Right now they are still handled
the same everywhere.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoIntroduce transform_apply_return_badseq
Ian Jackson [Tue, 14 May 2019 23:39:42 +0000 (00:39 +0100)]
Introduce transform_apply_return_badseq

This abstraction will allow us to distinguish two problems in a
moment.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Change return value of decrypt_msg0
Ian Jackson [Tue, 14 May 2019 23:29:24 +0000 (00:29 +0100)]
site: Change return value of decrypt_msg0

The caller is going to want to do something more subtle.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Make return value of transforms be an enum
Ian Jackson [Tue, 14 May 2019 23:23:47 +0000 (00:23 +0100)]
site: Make return value of transforms be an enum

We are going to need to distinguish more cases.  It was always bad to
have these hardcoded values.

transform_apply_seqrange is, right now, returned even when the problem
is that the packet is recent but is a duplicate.  This is wrong.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agopolypath asymmetric routing: Priority to mobile sites
Ian Jackson [Wed, 15 May 2019 21:42:10 +0000 (22:42 +0100)]
polypath asymmetric routing: Priority to mobile sites

It is better for the mobile peer to win the key setup priority
battle.  That makes handling the transport address implications,
particularly those of the MSG1, easier.

Since both ends must agree on who has priority, this must be
negotiated.  We use a capability bit for this.  Since the decision is
taken when we have only seen each other's MSG1, it must be an early
capability.  For compatibility with ancient (and security-buggy)
secnets, we can avoid advertising it if neither end is mobile.

In practice, in my tests, this change avoids a spurious key setup
failure when my laptop's secnet is restarted: the new secnet gets a
new NATted address, but the server has priority and insists on talking
to the old address.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: Document in NOTES following rebase over
    "NOTES: Describe the current allocation of capability bits."

4 years agosite: Break out we_have_priority
Ian Jackson [Wed, 15 May 2019 21:26:54 +0000 (22:26 +0100)]
site: Break out we_have_priority

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite: Rename setup_priority to our_name_later
Ian Jackson [Wed, 15 May 2019 21:23:49 +0000 (22:23 +0100)]
site: Rename setup_priority to our_name_later

This is also used for setting the transform direction - ie,
distinguishing us and them.  We are going to make the priority system
more subtle, and don't want to mess with that bit while changing the
priority rules.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: Fix conflict due to rebase on top of bugfix
     "site.c: Cope with failure of transform `setkey' method."

4 years agosite: Log about crossed MSG1 with a higher priority
Ian Jackson [Fri, 17 May 2019 22:38:47 +0000 (23:38 +0100)]
site: Log about crossed MSG1 with a higher priority

This means that we normally get these messages.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agosite: Log about crossed MSG1 ignored only once
Ian Jackson [Sat, 18 May 2019 00:28:02 +0000 (01:28 +0100)]
site: Log about crossed MSG1 ignored only once

If for some reason our peer isn't getting our MSG1s, they will
retransmit and we will ignore each retransmission.  Log this only
once.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agosite: transport peers update: avoid nearly-trivial debug
Ian Jackson [Fri, 17 May 2019 22:10:03 +0000 (23:10 +0100)]
site: transport peers update: avoid nearly-trivial debug

When the order of peers changes, but not the total set, we probably
don't want to log it.  Actually comparing the before and after lists
setwise is rather too hard.  But we can do it fairly easily when
there's only one peer being recorded.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
v2: New patch

4 years agoconfigure: rerun autogen.sh with autoconf 2.69-10
Ian Jackson [Tue, 14 May 2019 23:32:08 +0000 (00:32 +0100)]
configure: rerun autogen.sh with autoconf 2.69-10

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agoNOTES: tiny fix
Ian Jackson [Wed, 15 May 2019 21:02:10 +0000 (22:02 +0100)]
NOTES: tiny fix

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
4 years agosite.c: Cope with failure of transform `setkey' method.
Mark Wooding [Wed, 26 Apr 2017 10:53:05 +0000 (11:53 +0100)]
site.c: Cope with failure of transform `setkey' method.

The `setkey' method can fail, and indicates this by returning False.
Indeed, the `serpent-cbc256' transform will fail if the shared secret
it's given is too short.

Change `set_new_transform' and its callers to propagate failures
properly.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoREADME: Note that I've hacked on the code.
Mark Wooding [Wed, 26 Apr 2017 10:53:05 +0000 (11:53 +0100)]
README: Note that I've hacked on the code.

I shall be hacking on it further.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosite.c: Don't overwrite `st->sharedsecret' if it's null.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
site.c: Don't overwrite `st->sharedsecret' if it's null.

In this case, `st->sharesecretlen' is zero, but this is still undefined
behaviour.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosite.c: Make sure there's enough buffer space for the signature terminator.
Mark Wooding [Fri, 28 Apr 2017 21:51:36 +0000 (22:51 +0100)]
site.c: Make sure there's enough buffer space for the signature terminator.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agorsa.c transform-cbcmac.c: Fix configuration error messages.
Mark Wooding [Fri, 28 Apr 2017 21:51:20 +0000 (22:51 +0100)]
rsa.c transform-cbcmac.c: Fix configuration error messages.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoNOTES: Describe the current allocation of capability bits.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
NOTES: Describe the current allocation of capability bits.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agomake-secnet-sites: Don't allow setting new VPN-level props when restricted.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
make-secnet-sites: Don't allow setting new VPN-level props when restricted.

Currently, one can say something like

vpn thing
renegotiate-time 1

location evil
## ...

and if the VPN admin failed to set a value for `renegotiate-time' then
everyone will spin their CPUs doing key exchange.

Fix this lacuna.  Now user input can only modify location and site
properties.  If the administrator didn't set a location-level
`restrict-nets', then a user can do this, but obviously that can't make
anything worse.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agomake-secnet-sites: Remove duplicate `address' entry in sitelevel.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
make-secnet-sites: Remove duplicate `address' entry in sitelevel.

It was already there, with a functionally equivalent presentation
function.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agosecnet.8: Fix wrong information.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
secnet.8: Fix wrong information.

No, sites don't all have to use the same DH group.  It's true that sites
have to agree pairwise to use the same group when talking to each other.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoREADME.make-secnet-sites: Provide some documentation for this tool.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
README.make-secnet-sites: Provide some documentation for this tool.

Constructed by reverse-engineering.  I may well have misunderstood
things.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years ago.dir-locals.el: Settings for Python code.
Mark Wooding [Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)]
.dir-locals.el: Settings for Python code.

This project has very weird Python style.  Even by my standards.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
4 years agoutil.c: Don't byte-swap IPv4 addresses, even if we don't have IPv6.
Mark Wooding [Fri, 28 Apr 2017 21:51:36 +0000 (22:51 +0100)]
util.c: Don't byte-swap IPv4 addresses, even if we don't have IPv6.

The `string_item_to_ipaddr' function returns addresses as a single
integer in host byte order.  But this isn't what's wanted for setting up
`struct sockaddr_in', for example.  The function `adns_text2addr' does
the right thing.

I think this has always been wrong for setting up UDP sockets: before
the introduction of `string_item_to_iaddr', `udp_apply' would call
`string_item_to_ipaddr' directly, and neglected to swap the bytes.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
6 years agoAdministrivia: Fix erroneous GPL3+ licence notices "version d or later" (!)
Ian Jackson [Sat, 25 Nov 2017 16:14:00 +0000 (16:14 +0000)]
Administrivia: Fix erroneous GPL3+ licence notices "version d or later" (!)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agochangelog: start 0.4.4~
Ian Jackson [Sat, 25 Nov 2017 16:12:32 +0000 (16:12 +0000)]
changelog: start 0.4.4~

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agofinalise 0.4.3 v0.4.3
Ian Jackson [Sat, 25 Nov 2017 14:31:56 +0000 (14:31 +0000)]
finalise 0.4.3

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agochangelog, Makefile.in: burn version numbers 0.4.1 and 0.4.2
Ian Jackson [Sat, 25 Nov 2017 14:30:19 +0000 (14:30 +0000)]
changelog, Makefile.in: burn version numbers 0.4.1 and 0.4.2

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agochangelog: Retrospectively some items missing from 0.4.1 v0.4.2
Ian Jackson [Sat, 25 Nov 2017 14:23:34 +0000 (14:23 +0000)]
changelog: Retrospectively some items missing from 0.4.1

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild: Release checklist fixes.
Ian Jackson [Sat, 25 Nov 2017 14:17:07 +0000 (14:17 +0000)]
build: Release checklist fixes.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild: #include <limits.h>
Ian Jackson [Sat, 25 Nov 2017 14:16:32 +0000 (14:16 +0000)]
build: #include <limits.h>

Fixes the build on jessie.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agobuild: Tolerate building from a git checkout, but with git not installed.
Ian Jackson [Sat, 25 Nov 2017 14:07:31 +0000 (14:07 +0000)]
build: Tolerate building from a git checkout, but with git not installed.

This can happen in chroots.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agochangelog: start 0.4.2~
Ian Jackson [Sat, 25 Nov 2017 14:07:16 +0000 (14:07 +0000)]
changelog: start 0.4.2~

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
6 years agofinalise 0.4.1 v0.4.1
Ian Jackson [Sat, 25 Nov 2017 13:41:47 +0000 (13:41 +0000)]
finalise 0.4.1

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agoMerge remote-tracking branch 'mdw/mdw/powm-sec'
Ian Jackson [Tue, 25 Apr 2017 12:05:53 +0000 (13:05 +0100)]
Merge remote-tracking branch 'mdw/mdw/powm-sec'

7 years agoWhen turning on debug, turn on verbose too.
Ian Jackson [Sun, 23 Apr 2017 19:59:18 +0000 (20:59 +0100)]
When turning on debug, turn on verbose too.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agoWhen printing messages about dropping IPv6, do not print anything about ihl.
Ian Jackson [Sun, 23 Apr 2017 19:58:22 +0000 (20:58 +0100)]
When printing messages about dropping IPv6, do not print anything about ihl.

Check the IP version field first !

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agofixup! polypath: Introduce comm-info/dedicated
Ian Jackson [Sun, 23 Apr 2017 17:14:11 +0000 (18:14 +0100)]
fixup! polypath: Introduce comm-info/dedicated

7 years agofixup! polypath: Plumb ifname_wanted
Ian Jackson [Sun, 23 Apr 2017 17:13:55 +0000 (18:13 +0100)]
fixup! polypath: Plumb ifname_wanted

7 years agofixup! polypath: change return type of ifname_wanted
Ian Jackson [Sun, 23 Apr 2017 17:12:20 +0000 (18:12 +0100)]
fixup! polypath: change return type of ifname_wanted

7 years agochangelog: mention hippotat
Ian Jackson [Sun, 23 Apr 2017 16:25:15 +0000 (17:25 +0100)]
changelog: mention hippotat

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agopolypath: Introduce comm-info/dedicated-interface-addr
Ian Jackson [Sun, 23 Apr 2017 11:36:09 +0000 (12:36 +0100)]
polypath: Introduce comm-info/dedicated-interface-addr

Also, rename `interfs' to `interfs_general' in struct polypath, to
ensure we found everywhere this list is processed.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agopolypath: Break out polypath_sendmsg_interf
Ian Jackson [Sun, 23 Apr 2017 13:25:58 +0000 (14:25 +0100)]
polypath: Break out polypath_sendmsg_interf

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agopolypath: Plumb ifname_wanted `want' through privsep etc.
Ian Jackson [Sun, 23 Apr 2017 13:07:57 +0000 (14:07 +0100)]
polypath: Plumb ifname_wanted `want' through privsep etc.

Prepare for there being multiple interface lists, and for a new kind
of `want'.  Specifically:

* Arrange to declare the type `struct interf_list'.
* Pass the want via the privsep protocol, as a character.
* Pass the want in lots of command line arguments.
* Move assert for rogue values to the use site, which is now in
  a different process so it can't be an assert.
* Introduce a variable `interfs' in polypath_record_ifaddr to allow
  it to manipulate a different list.
* Introduce a variable `max_interfs' in polypath_record_ifaddr to
  allow a different check.
* Print the relevant want in debugging output.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
7 years agopolypath: change return type of ifname_wanted, to char
Ian Jackson [Sun, 23 Apr 2017 12:46:05 +0000 (13:46 +0100)]
polypath: change return type of ifname_wanted, to char

We are going to want to provide other answers besides just
yes (True, '+') and no (False, '!').  Prepare for this.

No functional change just yet.

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