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>

diff --git a/site.c b/site.c
index 3b8f34d47c8ff455a0d69bf931598093cacc6a25..edf4c50d11cfd7fe3c101d851e27fed22e1a67a1 100644 (file)
--- 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 */