[PATCH 23/25] site: support multiple transforms
Ian Jackson
ijackson at chiark.greenend.org.uk
Sat Jul 20 00:39:07 BST 2013
The "transform" key in site's dictionary argument can now be a list,
as well as just a single transform.
We use 16 bits of the capability mechanism to advertise the transforms
we support; the config is supposed to nominate a transform capability
number (from 0 to 15) for each transform closure - although the
default numbers are sufficient if you don't need to do parameter
rollover.
The receiver of MSG2 intersects the two bitmaps and chooses the best
transform, and states its choice in MSG3.
A protocol downgrade attack is prevented by the fact that the
capability bitmaps are advertised in the signed parts of MSG3 and
MSG4. (If the one in MSG4 doesn't match what was in MSG2, the MSG4 is
rejected and presumably the key exchange fails.)
Signed-off-by: Ian Jackson <ijackson at chiark.greenend.org.uk>
---
NOTES | 2 +-
example.conf | 4 +-
magic.h | 28 +++++++-
secnet.8 | 27 +++++++-
secnet.h | 1 +
site.c | 173 ++++++++++++++++++++++++++++++++++-----------
test-example/common.conf | 4 +-
transform-cbcmac.c | 2 +
transform-common.h | 10 +++
transform-eax.c | 2 +
10 files changed, 199 insertions(+), 54 deletions(-)
diff --git a/NOTES b/NOTES
index 8619ee5..7ead923 100644
--- a/NOTES
+++ b/NOTES
@@ -232,7 +232,7 @@ zero as its "index" for another site.)
(The order of B and A reverses in alternate messages so that the same
code can be used to construct them...)
-3) A->B: {iB,iA,msg3,A+,B+,nA,nB,g^x mod m}_PK_A^-1
+3) A->B: {iB,iA,msg3,A+,B+,[chosen-transform],nA,nB,g^x mod m}_PK_A^-1
If message 1 was a replay then A will not generate message 3, because
it doesn't recognise nA.
diff --git a/example.conf b/example.conf
index d6908ff..ab40d05 100644
--- a/example.conf
+++ b/example.conf
@@ -148,9 +148,7 @@ local-name "your-site-name";
local-key rsa-private("/etc/secnet/key");
# On dodgy links you may want to specify a higher maximum sequence number skew
-transform serpent256-cbc {
- max-sequence-skew 10;
-};
+transform eax-serpent, serpent256-cbc;
include /etc/secnet/sites.conf
diff --git a/magic.h b/magic.h
index 3ae7af4..f2ba3a0 100644
--- a/magic.h
+++ b/magic.h
@@ -16,7 +16,31 @@
#define LABEL_MSG9 0x09090909
/* uses of the 32-bit capability bitmap */
-/* no flags currently defined */
-#define CAPAB_EARLY 0x00000000 /* no Early flags defined (see NOTES) */
+#define CAPAB_EARLY 0x00000000 /* no Early flags yet (see NOTES) */
+#define CAPAB_TRANSFORM_MASK 0x0000ffff
+/* remaining 16 bits are unused */
+
+/*
+ * The transform capability mask is a set of bits, one for each
+ * transform supported. The transform capability numbers are set in
+ * the configuration (and should correspond between the two sites),
+ * although there are sensible defaults.
+ *
+ * Iff both ends' transform capability masks are nonempty, MSG3
+ * contains an additional byte specifying the transform capability
+ * number actually chosen by the MSG3 sender.
+ *
+ * Aside from that, an empty bitmask is treated the same as
+ * 1u<<CAPAB_TRANSFORMNUM_ANCIENT
+ */
+
+/* bit indices, 0 is ls bit */
+#define CAPAB_TRANSFORMNUM_USER_MIN 0
+#define CAPAB_TRANSFORMNUM_USER_MAX 7
+#define CAPAB_TRANSFORMNUM_SERPENT256CBC 8
+#define CAPAB_TRANSFORMNUM_EAXSERPENT 9
+#define CAPAB_TRANSFORMNUM_MAX 15
+
+#define CAPAB_TRANSFORMNUM_ANCIENT CAPAB_TRANSFORMNUM_SERPENT256CBC
#endif /* magic_h */
diff --git a/secnet.8 b/secnet.8
index e553321..e69a187 100644
--- a/secnet.8
+++ b/secnet.8
@@ -434,6 +434,17 @@ blocksize, 16. Must be have the same value at both ends.
.B padding-rounding
Messages are padded to a multiple of this many bytes. This
serves to obscure the exact length of messages. The default is 16,
+.TP
+.B capab-num
+The transform capability number to use when advertising this
+transform. Both ends must have the same meaning (or, at least, a
+compatible transform) for each transform capability number they have
+in common. The default for serpent-eax is 9.
+.IP
+Transform capability numbers in the range 8..15 are intended for
+allocation by the implementation, and may be assigned as the default
+for new transforms in the future. Transform capability numbers in the
+range 0..7 are reserved for definition by the user.
.PP
A \fItransform closure\fR is a reversible means of transforming
messages for transmission over a (presumably) insecure network.
@@ -442,8 +453,15 @@ It is responsible for both confidentiality and integrity.
.SS serpent256-cbc
\fBserpent256-cbc(\fIDICT\fB)\fR => \fItransform closure\fR
.PP
+This transform
+is deprecated as its security properties are poor; it should be
+specified only alongside a better transform such as eax-serpent.
+.PP
Valid keys in the \fIDICT\fR argument are:
.TP
+.B capab-num
+As above. The default for serpent256-cbc is 8.
+.TP
.B max-sequence-skew
As above.
@@ -518,8 +536,13 @@ An \fIrsapubkey closure\fR.
The key used to verify the peer's identity.
.TP
.B transform
-A \fItransform closure\fR.
-Used to protect packets exchanged with the peer.
+One or more \fItransform closures\fR.
+Used to protect packets exchanged with the peer. These should
+all have distinct \fBcapab-num\fR values, and the same \fBcapab-num\fR
+value should refer to the same (or a compatible) transform at both
+ends. The list should be in order of preference, most preferred
+first. (The end which sends MSG1,MSG3 ends up choosing; the ordering
+at the other end is irrelevant.)
.TP
.B dh
A \fIdh closure\fR.
diff --git a/secnet.h b/secnet.h
index 3db0f6c..4dd5c03 100644
--- a/secnet.h
+++ b/secnet.h
@@ -418,6 +418,7 @@ struct transform_if {
void *st;
int32_t max_start_pad; /* these two are both <<< INT_MAX */
int32_t keylen; /* 0 means give the transform exactly as much as there is */
+ int capab_transformnum;
transform_createinstance_fn *create;
};
diff --git a/site.c b/site.c
index 35c2814..df45fd8 100644
--- a/site.c
+++ b/site.c
@@ -239,7 +239,8 @@ struct site {
struct random_if *random;
struct rsaprivkey_if *privkey;
struct rsapubkey_if *pubkey;
- struct transform_if *transform;
+ struct transform_if **transforms;
+ int ntransforms;
struct dh_if *dh;
struct hash_if *hash;
@@ -277,6 +278,7 @@ struct site {
timeout before we can listen for another setup packet); perhaps
we should keep a list of 'bad' sources for setup packets. */
uint32_t remote_capabilities;
+ struct transform_if *chosen_transform;
uint32_t setup_session_id;
transport_peers setup_peers;
uint8_t localN[NONCELEN]; /* Nonces for key exchange */
@@ -287,7 +289,7 @@ struct site {
uint64_t timeout; /* Timeout for current state */
uint8_t *dhsecret;
uint8_t *sharedsecret;
- uint32_t sharedsecretlen;
+ uint32_t sharedsecretlen, sharedsecretallocd;
struct transform_inst_if *new_transform; /* For key setup/verify */
};
@@ -360,6 +362,7 @@ struct msg {
struct parsedname remote;
struct parsedname local;
uint32_t remote_capabilities;
+ int capab_transformnum;
uint8_t *nR;
uint8_t *nL;
int32_t pklen;
@@ -369,9 +372,23 @@ struct msg {
char *sig;
};
-static void set_new_transform(struct site *st)
+static void set_new_transform(struct site *st, char *pk)
{
- struct transform_if *generator=st->transform;
+ /* Make room for the shared key */
+ st->sharedsecretlen=st->chosen_transform->keylen?:st->dh->ceil_len;
+ assert(st->sharedsecretlen);
+ if (st->sharedsecretlen > st->sharedsecretallocd) {
+ st->sharedsecretallocd=st->sharedsecretlen;
+ st->sharedsecret=realloc(st->sharedsecret,st->sharedsecretallocd);
+ }
+ if (!st->sharedsecret) fatal_perror("site:sharedsecret");
+
+ /* Generate the shared key */
+ st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,pk,
+ st->sharedsecret,st->sharedsecretlen);
+
+ /* Set up the transform */
+ struct transform_if *generator=st->chosen_transform;
struct transform_inst_if *new_transform=generator->create(generator->st);
new_transform->setkey(new_transform->st,st->sharedsecret,
st->sharedsecretlen,st->setup_priority);
@@ -380,6 +397,19 @@ static void set_new_transform(struct site *st)
st->new_transform->destroy(st->new_transform->st);
}
st->new_transform=new_transform;
+
+ slog(st,LOG_SETUP_INIT,"key exchange negotiated transform"
+ " capab-num=%d (capabilities ours=%#"PRIx32" theirs=%#"PRIx32")",
+ st->chosen_transform->capab_transformnum,
+ st->local_capabilities, st->remote_capabilities);
+}
+
+static bool_t msg_specifies_transform(struct site *st, uint32_t msgtype)
+{
+ return
+ (msgtype==LABEL_MSG3) &&
+ (st->local_capabilities & CAPAB_TRANSFORM_MASK) &&
+ (st->remote_capabilities & CAPAB_TRANSFORM_MASK);
}
struct xinfoadd {
@@ -441,6 +471,10 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what)
if (hacky_par_mid_failnow()) return False;
+ if (msg_specifies_transform(st,type))
+ *(uint8_t*)buf_append(&st->buffer,1)=
+ st->chosen_transform->capab_transformnum;
+
dhpub=st->dh->makepublic(st->dh->st,st->dhsecret,st->dh->len);
buf_append_string(&st->buffer,dhpub);
free(dhpub);
@@ -475,6 +509,7 @@ static bool_t unpick_name(struct buffer_if *msg, struct parsedname *nm)
static bool_t unpick_msg(struct site *st, uint32_t type,
struct buffer_if *msg, struct msg *m)
{
+ m->capab_transformnum=-1;
m->hashstart=msg->start;
CHECK_AVAIL(msg,4);
m->dest=buf_unprepend_uint32(msg);
@@ -500,6 +535,12 @@ static bool_t unpick_msg(struct site *st, uint32_t type,
CHECK_EMPTY(msg);
return True;
}
+ if (msg_specifies_transform(st,type)) {
+ CHECK_AVAIL(msg,1);
+ m->capab_transformnum = *(uint8_t*)buf_unprepend(msg,1);
+ } else {
+ m->capab_transformnum = CAPAB_TRANSFORMNUM_ANCIENT;
+ }
CHECK_AVAIL(msg,2);
m->pklen=buf_unprepend_uint16(msg);
CHECK_AVAIL(msg,m->pklen);
@@ -596,6 +637,29 @@ static bool_t process_msg2(struct site *st, struct buffer_if *msg2,
}
st->setup_session_id=m.source;
st->remote_capabilities=m.remote_capabilities;
+
+ /* Select the transform to use */
+
+ uint32_t remote_transforms = st->remote_capabilities & CAPAB_TRANSFORM_MASK;
+ if (!remote_transforms)
+ /* old secnets only had this one transform */
+ remote_transforms = 1UL << CAPAB_TRANSFORMNUM_ANCIENT;
+
+ struct transform_if *ti;
+ int i;
+ for (i=0; i<st->ntransforms; i++) {
+ ti=st->transforms[i];
+ if ((1UL << ti->capab_transformnum) & remote_transforms)
+ goto transform_found;
+ }
+ slog(st,LOG_ERROR,"no transforms in common"
+ " (us %#"PRIx32"; them: %#"PRIx32")",
+ st->local_capabilities & CAPAB_TRANSFORM_MASK,
+ remote_transforms);
+ return False;
+ transform_found:
+ st->chosen_transform=ti;
+
memcpy(st->remoteN,m.nR,NONCELEN);
return True;
}
@@ -631,6 +695,19 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3,
}
st->remote_capabilities|=m.remote_capabilities;
+ struct transform_if *ti;
+ int i;
+ for (i=0; i<st->ntransforms; i++) {
+ ti=st->transforms[i];
+ if (ti->capab_transformnum == m.capab_transformnum)
+ goto transform_found;
+ }
+ slog(st,LOG_SEC,"peer chose unknown-to-us transform %d!",
+ m.capab_transformnum);
+ return False;
+ transform_found:
+ st->chosen_transform=ti;
+
/* Check signature and store g^x mod m */
hash=safe_malloc(st->hash->len, "process_msg3");
hst=st->hash->init();
@@ -650,12 +727,8 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3,
/* Invent our DH secret key */
st->random->generate(st->random->st,st->dh->len,st->dhsecret);
- /* Generate the shared key */
- st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,m.pk,
- st->sharedsecret,st->sharedsecretlen);
-
- /* Set up the transform */
- set_new_transform(st);
+ /* Generate the shared key and set up the transform */
+ set_new_transform(st,m.pk);
return True;
}
@@ -697,11 +770,9 @@ static bool_t process_msg4(struct site *st, struct buffer_if *msg4,
/* Terminate their DH public key with a '0' */
m.pk[m.pklen]=0;
- /* Generate the shared key */
- st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,m.pk,
- st->sharedsecret,st->sharedsecretlen);
- /* Set up the transform */
- set_new_transform(st);
+
+ /* Generate the shared key and set up the transform */
+ set_new_transform(st,m.pk);
return True;
}
@@ -1558,19 +1629,25 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
st->local_capabilities = 0;
st->netlink=find_cl_if(dict,"link",CL_NETLINK,True,"site",loc);
- list_t *comms_cfg=dict_lookup(dict,"comm");
- if (!comms_cfg) cfgfatal(loc,"site","closure list \"comm\" not found\n");
- st->ncomms=list_length(comms_cfg);
- st->comms=safe_malloc_ary(sizeof(*st->comms),st->ncomms,"comms");
- assert(st->ncomms);
- for (i=0; i<st->ncomms; i++) {
- item_t *item=list_elem(comms_cfg,i);
- if (item->type!=t_closure)
- cfgfatal(loc,"site","comm is not a closure\n");
- closure_t *cl=item->data.closure;
- if (cl->type!=CL_COMM) cfgfatal(loc,"site","comm closure wrong type\n");
- st->comms[i]=cl->interface;
- }
+#define GET_CLOSURE_LIST(dictkey,things,nthings,CL_TYPE) do{ \
+ list_t *closures_cfg=dict_lookup(dict,dictkey); \
+ if (!closures_cfg) \
+ cfgfatal(loc,"site","closure list \"%s\" not found\n",dictkey); \
+ st->nthings=list_length(closures_cfg); \
+ st->things=safe_malloc_ary(sizeof(*st->things),st->nthings,dictkey "s"); \
+ assert(st->nthings); \
+ for (i=0; i<st->nthings; i++) { \
+ item_t *item=list_elem(closures_cfg,i); \
+ if (item->type!=t_closure) \
+ cfgfatal(loc,"site","%s is not a closure\n",dictkey); \
+ closure_t *cl=item->data.closure; \
+ if (cl->type!=CL_TYPE) \
+ cfgfatal(loc,"site","%s closure wrong type\n",dictkey); \
+ st->things[i]=cl->interface; \
+ } \
+}while(0)
+
+ GET_CLOSURE_LIST("comm",comms,ncomms,CL_COMM);
st->resolver=find_cl_if(dict,"resolver",CL_RESOLVER,True,"site",loc);
st->log=find_cl_if(dict,"log",CL_LOG,True,"site",loc);
@@ -1583,8 +1660,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
else st->remoteport=0;
st->pubkey=find_cl_if(dict,"key",CL_RSAPUBKEY,True,"site",loc);
- st->transform=
- find_cl_if(dict,"transform",CL_TRANSFORM,True,"site",loc);
+ GET_CLOSURE_LIST("transform",transforms,ntransforms,CL_TRANSFORM);
st->dh=find_cl_if(dict,"dh",CL_DH,True,"site",loc);
st->hash=find_cl_if(dict,"hash",CL_HASH,True,"site",loc);
@@ -1643,29 +1719,40 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
st->timeout=0;
st->remote_capabilities=0;
+ st->chosen_transform=0;
st->current.key_timeout=0;
st->auxiliary_key.key_timeout=0;
transport_peers_clear(st,&st->peers);
transport_peers_clear(st,&st->setup_peers);
/* XXX mlock these */
st->dhsecret=safe_malloc(st->dh->len,"site:dhsecret");
- st->sharedsecretlen=st->transform->keylen?:st->dh->ceil_len;
- st->sharedsecret=safe_malloc(st->sharedsecretlen,"site:sharedsecret");
-
- /* We need to compute some properties of our comms */
-#define COMPUTE_WORST(pad) \
- int worst_##pad=0; \
- for (i=0; i<st->ncomms; i++) { \
- int thispad=st->comms[i]->pad; \
- if (thispad > worst_##pad) \
- worst_##pad=thispad; \
+ st->sharedsecretlen=st->sharedsecretallocd=0;
+ st->sharedsecret=0;
+
+ /* We need to compute some properties of our comms and transports */
+#define COMPUTE_WORST(things,pad) \
+ int things##_worst_##pad=0; \
+ for (i=0; i<st->n##things; i++) { \
+ int thispad=st->things[i]->pad; \
+ if (thispad > things##_worst_##pad) \
+ things##_worst_##pad=thispad; \
+ }
+ COMPUTE_WORST(comms,min_start_pad)
+ COMPUTE_WORST(transforms,max_start_pad)
+
+ for (i=0; i<st->ntransforms; i++) {
+ struct transform_if *ti=st->transforms[i];
+ uint32_t capbit = 1UL << ti->capab_transformnum;
+ if (st->local_capabilities & capbit)
+ slog(st,LOG_ERROR,"transformnum capability bit"
+ " %d (%#"PRIx32") reused", ti->capab_transformnum, capbit);
+ st->local_capabilities |= capbit;
}
- COMPUTE_WORST(min_start_pad)
/* We need to register the remote networks with the netlink device */
st->netlink->reg(st->netlink->st, site_outgoing, st,
- st->transform->max_start_pad+(4*4)+
- worst_min_start_pad);
+ transforms_worst_max_start_pad+(4*4)+
+ comms_worst_min_start_pad);
for (i=0; i<st->ncomms; i++)
st->comms[i]->request_notify(st->comms[i]->st, st, site_incoming);
diff --git a/test-example/common.conf b/test-example/common.conf
index 64bfc81..f41eab7 100644
--- a/test-example/common.conf
+++ b/test-example/common.conf
@@ -8,8 +8,6 @@ resolver adns {
};
log-events "all";
random randomfile("/dev/urandom",no);
-transform serpent256-cbc {
- max-sequence-skew 10;
-};
+transform eax-serpent, serpent256-cbc;
include test-example/sites.conf
sites map(site,vpn/test-example/all-sites);
diff --git a/transform-cbcmac.c b/transform-cbcmac.c
index 5640ad8..302160f 100644
--- a/transform-cbcmac.c
+++ b/transform-cbcmac.c
@@ -279,6 +279,8 @@ static list_t *transform_apply(closure_t *self, struct cloc loc,
st->max_seq_skew=dict_read_number(dict, "max-sequence-skew",
False, "serpent-cbc256", loc, 10);
+ SET_CAPAB_TRANSFORMNUM(CAPAB_TRANSFORMNUM_SERPENT256CBC);
+
return new_closure(&st->cl);
}
diff --git a/transform-common.h b/transform-common.h
index de19817..52f6067 100644
--- a/transform-common.h
+++ b/transform-common.h
@@ -2,6 +2,8 @@
#ifndef TRANSFORM_COMMON_H
#define TRANSFORM_COMMON_H
+#include "magic.h"
+
#define KEYED_CHECK do{ \
if (!ti->keyed) { \
*errmsg="transform unkeyed"; \
@@ -40,6 +42,14 @@
free(st); \
}
+#define SET_CAPAB_TRANSFORMNUM(def) do{ \
+ st->ops.capab_transformnum=dict_read_number(dict, "capab-num", \
+ False, "transform", loc, def); \
+ if (st->ops.capab_transformnum > CAPAB_TRANSFORMNUM_MAX) \
+ cfgfatal(loc,"transform","capab-num out of range 0..%d\n", \
+ CAPAB_TRANSFORMNUM_MAX); \
+ }while(0)
+
#define TRANSFORM_CREATE_CORE \
struct transform_inst *ti; \
ti=safe_malloc(sizeof(*ti),"transform_create"); \
diff --git a/transform-eax.c b/transform-eax.c
index f126408..0a47d5f 100644
--- a/transform-eax.c
+++ b/transform-eax.c
@@ -218,6 +218,8 @@ static list_t *transform_apply(closure_t *self, struct cloc loc,
cfgfatal(loc,"eax-serpent","parameter must be a dictionary\n");
dict=item->data.dict;
+ SET_CAPAB_TRANSFORMNUM(CAPAB_TRANSFORMNUM_EAXSERPENT);
+
st->p.max_seq_skew=dict_read_number(dict, "max-sequence-skew",
False, "eax-serpent", loc, 10);
--
1.7.2.5
More information about the sgo-software-discuss
mailing list