site.c (we_have_priority): Fix unintended `&&'.

`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>

diff --git a/site.c b/site.c
index 4ec56dfa5076da4a8a59eea7d7a7ffde74e7e87b..d0dd909d57aac307373df6c3132997f795f1c270 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;
     }