From: Mark Wooding Date: Sat, 29 Apr 2017 12:55:40 +0000 (+0100) Subject: Introduce negotiation for Diffie--Hellman groups. X-Git-Url: https://git.distorted.org.uk/~mdw/secnet/commitdiff_plain/9c6af4eca6bfb7bed6f86b1f32479f933979c080 Introduce negotiation for Diffie--Hellman groups. For the most part, this slots into the space previously prepared for it. However, there are a few subtleties. The most significant one is that existing Secnets don't pay attention to the high 16 cap bits. To bring them into availability, we introduce a signalling system. If bit 15 is set, then * all of the bits are scanned for capabilities, and * it is expected that sender has advertised its DH groups explicitly. If the bit is clear, then we have the old situation: * firstly, only the low 16 bits are scanned for transform cap bits, and * secondly, it is assumed that the sender only implements traditional integer Diffie--Hellman, cap 10, with some appropriately determined group. We also set the explicit bit if one of the high capability bits is set. As part of this, add a parameter to the `diffie-hellman' closure to configure its advertised group cap. Signed-off-by: Mark Wooding --- diff --git a/NOTES b/NOTES index 884ae30..313a0ff 100644 --- a/NOTES +++ b/NOTES @@ -218,16 +218,20 @@ Capability flag bits must be in one the following two categories: applicable. They may also appear in MSG1, but this is not guaranteed. MSG4 must advertise the same set as MSG2. -Currently, the low 16 bits are allocated for negotiating bulk-crypto -transforms. Bits 8 to 15 are used by Secnet as default capability -numbers for the various kinds of transform closures: bit 8 is for the -original CBCMAC-based transform, and bit 9 for the new EAX transform; -bits 10 to 15 are reserved for future expansion. The the low eight bits -are reserved for local use, e.g., to allow migration from one set of -parameters for a particular transform to a different, incompatible set -of parameters for the same transform. Bit 31, if advertised by both -ends, indicates that a mobile end gets priority in case of crossed MSG1. -The remaining bits have not yet been assigned a purpose. +Capability bits 8 to 31 are used by Secnet as default capability numbers +for various features: bit 8 is for the original CBCMAC-based transform, +and bit 9 for the new EAX transform; bit 10 for traditional finite-field +Diffie--Hellman; bits 11 to 14 and 16 to 30 are reserved for future +expansion. The the low eight bits are reserved for local use, e.g., to +allow migration from one set of parameters for a particular transform to +a different, incompatible set of parameters for the same transform. Bit +31, if advertised by both ends, indicates that a mobile end gets +priority in case of crossed MSG1. + +Bit 15 is special: it signifies (a) that the sender is reporting all of +its transforms and DH groups explicitly, and (b) that it uses all 32 +capability bits to do so. Older Secnets only checked the low 16 bits +for known capabilities. Whether a capability number is early depends on its meaning, rather than being a static property of its number. That said, the mobile-end-gets diff --git a/dh.c b/dh.c index 11f1d35..0b5e4d2 100644 --- a/dh.c +++ b/dh.c @@ -32,6 +32,7 @@ #include #include "secnet.h" +#include "magic.h" #include "util.h" struct dh { @@ -169,6 +170,15 @@ static list_t *dh_apply(closure_t *self, struct cloc loc, dict_t *context, /* According to the docs, mpz_sizeinbase(,256) is allowed to return * an answer which is 1 too large. But mpz_sizeinbase(,2) isn't. */ + if (!dict) + st->ops.capab_bit = CAPAB_BIT_TRADZP; + else + st->ops.capab_bit = dict_read_number(dict, "capab-num", False, + "dh", loc, CAPAB_BIT_TRADZP); + if (st->ops.capab_bit > CAPAB_BIT_MAX) + cfgfatal(loc,"dh","capab-num out of range 0..%d\n", + CAPAB_BIT_MAX); + return new_closure(&st->cl); } diff --git a/magic.h b/magic.h index 15d8498..9c071c0 100644 --- a/magic.h +++ b/magic.h @@ -75,6 +75,7 @@ #define LABEL_MSG2 MSGCODE( 2, 0) #define LABEL_MSG3 MSGCODE( 3, 0) #define LABEL_MSG3BIS MSGCODE( 3, 1) +#define LABEL_MSG3TER MSGCODE( 3, 2) #define LABEL_MSG4 MSGCODE( 4, 0) #define LABEL_MSG5 MSGCODE( 5, 0) #define LABEL_MSG6 MSGCODE( 6, 0) @@ -99,7 +100,8 @@ */ /* uses of the 32-bit capability bitmap */ -#define CAPAB_TRANSFORM_MASK 0x0000ffff +#define CAPAB_INEXPLICIT_TRANSFORM_MASK 0x0000ffff /* DH group implicit */ +#define CAPAB_EXPLICIT_TRANSFORM_DH 0x00008000 /* Explicit xform and DH */ #define CAPAB_PRIORITY_MOBILE 0x80000000 /* mobile site has MSG1 priority */ /* remaining bits are unused */ @@ -108,7 +110,9 @@ #define CAPAB_BIT_USER_MAX 7 #define CAPAB_BIT_SERPENT256CBC 8 #define CAPAB_BIT_EAXSERPENT 9 -#define CAPAB_BIT_MAX 15 +#define CAPAB_BIT_TRADZP 10 +#define CAPAB_BIT_EXPLICIT_TRANSFORM_DH 15 +#define CAPAB_BIT_MAX 31 #define CAPAB_BIT_ANCIENTTRANSFORM CAPAB_BIT_SERPENT256CBC diff --git a/secnet-wireshark.lua b/secnet-wireshark.lua index 62739bc..5166918 100644 --- a/secnet-wireshark.lua +++ b/secnet-wireshark.lua @@ -162,7 +162,7 @@ end local function dump_algs(algs) -- Dump the algorithms selection ALGS from a site structure. - return "xform=" .. algs.transform + return "xform=" .. algs.transform .. "; dh=" .. algs.dhgroup end local function dump_str(str) return str end @@ -224,6 +224,8 @@ local CAPTAB = { desc = "Deprecated Serpent256-CBC transform" }, [9] = { name = "eaxserpent", kind = "transform", desc = "Serpent256-EAX transform" }, + [10] = { name = "tradzp", kind = "dhgroup", + desc = "Traditional Z_p Diffie--Hellman key agreement" }, [31] = { name = "mobile-priority", kind = "early", desc = "Mobile site takes priority in case of MSG1 crossing" } } @@ -248,10 +250,11 @@ local function notice_alg_selection(st) -- Record the algorithm selections declared in the packet described by ST. local transform = get_algname("transform", st.transform, "serpent256cbc") + local dhgroup = get_algname("dhgroup", st.dhgroup, "tradzp") local site = get_site_create(st.sndname) local peer = get_site_create(st.rcvname) local now = st.pinfo.rel_ts - local algs = { transform = transform } + local algs = { transform = transform, dhgroup = dhgroup } tl_add(get_timeline_create(site.algs, st.rcvname), now, algs) tl_add(get_timeline_create(peer.algs, st.sndname), now, algs) end @@ -302,6 +305,7 @@ local M = { NAK = msgcode( 0, 0), MSG2 = msgcode( 2, 0), MSG3 = msgcode( 3, 0), MSG3BIS = msgcode( 3, 1), + MSG3TER = msgcode( 3, 2), MSG4 = msgcode( 4, 0), MSG5 = msgcode( 5, 0), MSG6 = msgcode( 6, 0), @@ -346,6 +350,7 @@ do -- Firstly, build, in `caps', a list of the capability names and their -- numbers. local i = 1 + caps[i] = { i = 15, cap = "explicit" }; i = 1 + 1 for j, cap in pairs(CAPTAB) do caps[i] = { i = j, cap = cap.name } i = i + 1 @@ -451,6 +456,14 @@ local function dissect_transform(st, buf, tree, pos, sz) return pos end +local function dissect_dhgroup(st, buf, tree, pos, sz) + -- Dissect the selected DH group. Note this in the packet state for later. + + st.dhgroup = buf(pos, 1):uint() + tree:add(PF["secnet.kx.dhgroup"], buf(pos, 1)); pos = pos + 1 + return pos +end + local function dissect_lenstr(st, buf, tree, label, pos, sz) -- Dissect a simple string given its length. local len = buf(pos, 2):uint() @@ -620,6 +633,23 @@ local PKTINFO = { dissect_wtf }, hook = notice_alg_selection }, + [M.MSG3TER] = { + label = "MSG3TER", + info = "MSG3TER", + dissect = { make_dissect_name_xinfo("secnet.kx.sndname", + { dissect_caps, + dissect_mtu, + dissect_wtf }, + notice_sndname), + make_dissect_name_xinfo("secnet.kx.rcvname", + { dissect_wtf }, + notice_rcvname), + dissect_sndnonce, dissect_rcvnonce, + dissect_transform, dissect_dhgroup, + dissect_dhval, dissect_sig, + dissect_wtf }, + hook = notice_alg_selection + }, [M.MSG4] = { label = "MSG4", info = "MSG4", @@ -665,7 +695,7 @@ do local msgtab = { } for i, v in pairs(PKTINFO) do msgtab[i] = v.label end - local capmap = { transform = { }, early = { } } + local capmap = { transform = { }, dhgroup = { }, early = { } } for i, v in pairs(CAPTAB) do capmap[v.kind][i] = v.desc end local ftab = { @@ -726,6 +756,10 @@ do name = "User-assigned capability bits", type = ftypes.UINT32, mask = 0x000000ff, base = base.HEX }, + ["secnet.cap.explicit"] = { + name = "Transforms listed explicitly; all capability bits used", + type = ftypes.BOOLEAN, mask = 0x00008000, base = 32 + }, ["secnet.mtu"] = { name = "Sender's requested MTU", type = ftypes.UINT16, base = base.DEC }, @@ -739,6 +773,10 @@ do name = "Selected bulk-crypto transform", type = ftypes.UINT8, base = base.DEC, tab = capmap.transform }, + ["secnet.kx.dhgroup"] = { + name = "Selected Diffie--Hellman group kind", type = ftypes.UINT8, + base = base.DEC, tab = capmap.dhgroup + }, ["secnet.kx.dhval"] = { name = "Sender's public Diffie--Hellman value", type = ftypes.NONE }, diff --git a/secnet.8 b/secnet.8 index f8cd48f..525e854 100644 --- a/secnet.8 +++ b/secnet.8 @@ -331,6 +331,12 @@ If \fBtrue\fR (the default) then check if \fIp\fR is prime. Corresponds to the .I CHECK argument. +.TP +.B capab-num +The capability number to use when advertising +this Diffie\(enHellman group. +The default capability number is 10. + .PP A \fIdh closure\fR defines a group to be used for key exchange. @@ -609,8 +615,18 @@ 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. -The group to use in key exchange. +A list of one or more \fIdh closure\fRs. +The groups to use in key exchange. +These should all have distinct +.B capab-num +values, +and the same +.B capab-num +value should have the same (or a compatible) meaning 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 hash The hash function used during setup. diff --git a/secnet.h b/secnet.h index ee6af9c..554df4b 100644 --- a/secnet.h +++ b/secnet.h @@ -632,6 +632,7 @@ struct dh_if { void *st; int32_t secret_len; /* Size of random secret to generate */ int32_t shared_len; /* Size of generated shared secret */ + int capab_bit; dh_makepublic_fn *makepublic; dh_makeshared_fn *makeshared; }; diff --git a/site.c b/site.c index 4f954e6..4da99e3 100644 --- a/site.c +++ b/site.c @@ -107,7 +107,7 @@ #define SITE_SENTMSG5 7 #define SITE_WAIT 8 -#define CASES_MSG3_KNOWN LABEL_MSG3: case LABEL_MSG3BIS +#define CASES_MSG3_KNOWN LABEL_MSG3: case LABEL_MSG3BIS: case LABEL_MSG3TER int32_t site_max_start_pad = 4*4; @@ -316,7 +316,8 @@ struct site { struct rsapubkey_if *pubkey; struct transform_if **transforms; int ntransforms; - struct dh_if *dh; + struct dh_if **dhs; + int ndhs; struct hash_if *hash; uint32_t index; /* Index of this site */ @@ -361,6 +362,7 @@ struct site { uint32_t remote_capabilities; uint16_t remote_adv_mtu; struct transform_if *chosen_transform; + struct dh_if *chosen_dh; uint32_t setup_session_id; transport_peers setup_peers; uint8_t localN[NONCELEN]; /* Nonces for key exchange */ @@ -528,6 +530,7 @@ struct msg { uint32_t remote_capabilities; uint16_t remote_mtu; int capab_transformnum; + int capab_dhnum; uint8_t *nR; uint8_t *nL; int32_t pklen; @@ -553,16 +556,20 @@ static _Bool set_new_transform(struct site *st, char *pk) /* Generate the shared key */ assert(!st->sharedsecret); - st->sharedsecret = safe_malloc(st->dh->shared_len, "site:sharedsecret"); - if (!st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->secret_len, - pk, st->sharedsecret,st->dh->shared_len)) + st->sharedsecret = safe_malloc(st->chosen_dh->shared_len, + "site:sharedsecret"); + if (!st->chosen_dh->makeshared(st->chosen_dh->st, + st->dhsecret,st->chosen_dh->secret_len, + pk, + st->sharedsecret, + st->chosen_dh->shared_len)) return False; /* Set up the transform */ struct transform_if *generator=st->chosen_transform; struct transform_inst_if *generated=generator->create(generator->st); ok = generated->setkey(generated->st,st->sharedsecret, - st->dh->shared_len,st->our_name_later); + st->chosen_dh->shared_len,st->our_name_later); dispose_transform(&st->new_transform); if (!ok) return False; @@ -643,9 +650,12 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what) minor = MSGMINOR(type); if (minor < 1) break; buf_append_uint8(&st->buffer,st->chosen_transform->capab_bit); + if (minor < 2) break; + buf_append_uint8(&st->buffer,st->chosen_dh->capab_bit); } while (0); - dhpub=st->dh->makepublic(st->dh->st,st->dhsecret,st->dh->secret_len); + dhpub=st->chosen_dh->makepublic(st->chosen_dh->st, + st->dhsecret,st->chosen_dh->secret_len); buf_append_string(&st->buffer,dhpub); free(dhpub); hash=safe_malloc(st->hash->len, "generate_msg"); @@ -680,7 +690,7 @@ static bool_t unpick_msg(struct site *st, uint32_t type, { unsigned minor; - m->capab_transformnum=-1; + m->capab_transformnum=m->capab_dhnum=-1; m->hashstart=msg->start; CHECK_AVAIL(msg,4); m->dest=buf_unprepend_uint32(msg); @@ -726,6 +736,7 @@ static bool_t unpick_msg(struct site *st, uint32_t type, } \ } while (0) MAYBE_READ_CAP(1, transform, CAPAB_BIT_ANCIENTTRANSFORM); + MAYBE_READ_CAP(2, dh, CAPAB_BIT_TRADZP); #undef MAYBE_READ_CAP } while (0); CHECK_AVAIL(msg,2); @@ -831,12 +842,17 @@ 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 */ + /* Select the transform and DH group to use */ - uint32_t remote_crypto_caps = st->remote_capabilities & CAPAB_TRANSFORM_MASK; + uint32_t remote_crypto_caps = st->remote_capabilities; + if (!(remote_crypto_caps & CAPAB_EXPLICIT_TRANSFORM_DH)) + remote_crypto_caps &= CAPAB_INEXPLICIT_TRANSFORM_MASK; if (!remote_crypto_caps) /* old secnets only had this one transform */ remote_crypto_caps = 1UL << CAPAB_BIT_ANCIENTTRANSFORM; + if (!(remote_crypto_caps & CAPAB_EXPLICIT_TRANSFORM_DH)) + /* old secnets only had this one kind of group */ + remote_crypto_caps |= 1UL << CAPAB_BIT_TRADZP; #define CHOOSE_CRYPTO(kind, whats) do { \ struct kind##_if *iface; \ @@ -857,6 +873,7 @@ kind##_found: \ } while (0) CHOOSE_CRYPTO(transform, "transforms"); + CHOOSE_CRYPTO(dh, "Diffie--Hellman groups"); #undef CHOOSE_CRYPTO @@ -866,9 +883,14 @@ kind##_found: \ static void generate_dhsecret(struct site *st) { + slog(st,LOG_SETUP_INIT,"key exchange negotiated DH group" + " %d (capabilities ours=%#"PRIx32" theirs=%#"PRIx32")", + st->chosen_dh->capab_bit, + st->local_capabilities, st->remote_capabilities); assert(!st->dhsecret); - st->dhsecret = safe_malloc(st->dh->secret_len, "site:dhsecret"); - st->random->generate(st->random->st, st->dh->secret_len,st->dhsecret); + st->dhsecret = safe_malloc(st->chosen_dh->secret_len, "site:dhsecret"); + st->random->generate(st->random->st, + st->chosen_dh->secret_len,st->dhsecret); } static bool_t generate_msg3(struct site *st) @@ -877,7 +899,11 @@ static bool_t generate_msg3(struct site *st) and create message number 3. */ generate_dhsecret(st); return generate_msg(st, - (st->remote_capabilities & CAPAB_TRANSFORM_MASK) + (st->remote_capabilities & + CAPAB_EXPLICIT_TRANSFORM_DH) + ? LABEL_MSG3TER + : (st->remote_capabilities & + CAPAB_INEXPLICIT_TRANSFORM_MASK) ? LABEL_MSG3BIS : LABEL_MSG3, "site:MSG3"); @@ -949,6 +975,7 @@ kind##_found: \ } while (0) CHOSE_CRYPTO(transform, "transform"); + CHOSE_CRYPTO(dh, "Diffie--Hellman group"); #undef CHOSE_CRYPTO @@ -1520,12 +1547,12 @@ static void enter_state_run(struct site *st) FILLZERO(st->remoteN); dispose_transform(&st->new_transform); if (st->dhsecret) { - memset(st->dhsecret, 0, st->dh->secret_len); + memset(st->dhsecret, 0, st->chosen_dh->secret_len); free(st->dhsecret); st->dhsecret = 0; } if (st->sharedsecret) { - memset(st->sharedsecret, 0, st->dh->shared_len); + memset(st->sharedsecret, 0, st->chosen_dh->shared_len); free(st->sharedsecret); st->sharedsecret = 0; } @@ -2151,7 +2178,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context, assert(index_sequence < 0xffffffffUL); st->index = ++index_sequence; - st->local_capabilities = 0; + st->local_capabilities = CAPAB_EXPLICIT_TRANSFORM_DH; st->early_capabilities = CAPAB_PRIORITY_MOBILE; st->netlink=find_cl_if(dict,"link",CL_NETLINK,True,"site",loc); @@ -2195,8 +2222,8 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context, st->pubkey=find_cl_if(dict,"key",CL_RSAPUBKEY,True,"site",loc); GET_CLOSURE_LIST("transform",transforms,ntransforms,CL_TRANSFORM); + GET_CLOSURE_LIST("dh",dhs,ndhs,CL_DH); - st->dh=find_cl_if(dict,"dh",CL_DH,True,"site",loc); st->hash=find_cl_if(dict,"hash",CL_HASH,True,"site",loc); #define DEFAULT(D) (st->peer_mobile || st->local_mobile \ @@ -2260,6 +2287,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context, st->remote_capabilities=0; st->chosen_transform=0; + st->chosen_dh=0; st->current.key_timeout=0; st->auxiliary_key.key_timeout=0; transport_peers_clear(st,&st->peers); @@ -2277,6 +2305,8 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context, for (i=0; intransforms; i++) SET_CAPBIT(st->transforms[i]->capab_bit); + for (i=0; indhs; i++) + SET_CAPBIT(st->dhs[i]->capab_bit); #undef SET_CAPBIT