chiark / gitweb /
site.c (we_have_priority): Fix unintended `&&'.
authorMark Wooding <mdw@distorted.org.uk>
Thu, 19 Sep 2019 20:01:23 +0000 (20:01 +0000)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Sat, 21 Sep 2019 10:02:16 +0000 (11:02 +0100)
`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>
site.c

diff --git a/site.c b/site.c
index 4ec56df..d0dd909 100644 (file)
--- a/site.c
+++ b/site.c
@@ -1781,8 +1781,8 @@ static bool_t named_for_us(struct site *st, const struct buffer_if *buf_in,
 }
 
 static bool_t we_have_priority(struct site *st, const struct msg *m) {
-    if ((st->local_capabilities & m->remote_capabilities)
-       && CAPAB_PRIORITY_MOBILE) {
+    if (st->local_capabilities & m->remote_capabilities &
+       CAPAB_PRIORITY_MOBILE) {
        if (st->local_mobile) return True;
        if (st-> peer_mobile) return False;
     }