From 0aae8f7a1935b589258a52c4728a434d87537da5 Mon Sep 17 00:00:00 2001 From: Mark Wooding Date: Sat, 28 Sep 2019 16:13:45 +0100 Subject: [PATCH] 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 --- site.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/site.c b/site.c index 3b8f34d..edf4c50 100644 --- a/site.c +++ b/site.c @@ -913,7 +913,6 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3, capab_adv_late, st->remote_capabilities, m.remote_capabilities); return False; } - st->remote_capabilities|=m.remote_capabilities; #define CHOSE_CRYPTO(kind, what) do { \ struct kind##_if *iface; \ @@ -937,6 +936,16 @@ kind##_found: \ if (!process_msg3_msg4(st,&m)) return False; + /* Update our idea of the remote site's capabilities, now that we've + * verified that its message was authentic. + * + * Our previous idea of the remote site's capabilities came from the + * unauthenticated MSG1. We've already checked that this new message + * doesn't change any of the bits we relied upon in the past, but it may + * also have set additional capability bits. We simply throw those away + * now, and use the authentic capabilities from this MSG3. */ + st->remote_capabilities=m.remote_capabilities; + /* Terminate their DH public key with a '0' */ m.pk[m.pklen]=0; /* Invent our DH secret key */ -- 2.30.2