8 files changed, 143 insertions, 19 deletions

diff --git a/man5/opentracker.conf.5 b/man5/opentracker.conf.5
index 67e624f..2250fe5 100644
--- a/man5/opentracker.conf.5
+++ b/man5/opentracker.conf.5
@@ -79,7 +79,20 @@ Specifies the path to the stats location. You can configure opentracker to appea
 Specifies the IP address or network of the reverse proxies. Opentracker will take the X-Forwarded-For address instead of the source IP address. Can be added more than once.
 .Pp
 .It livesync.cluster.listen Ar ip_address:port
-Specifies the IP address and port opentracker will listen on for incoming live sync packets to keep a cluster of opentrackers synchronized.
+Specifies the IP address and port opentracker will listen on for incoming live
+sync packets to keep a cluster of opentrackers synchronized. Every member of a
+cluster has to agree on the port, which is also the one packets are sent to.
+.Pp
+.It livesync.cluster.group Ar ip_address
+Specifies the multicast group a cluster talks in. For reaosns of backwards
+comaptibility with legacy trackers, it defaults to 224.0.23.5, which,
+technically sits in a range IANA assigns on request.
+
+A cluster of its own is better served by an address out of 239.192.0.0/14,
+which RFC 2365 sets aside for private use. Packets carry a ttl of 1 either
+way and never leave the local link. Must be a valid mulitcast address from
+.Pa 224.0.0.0/4
+.
 .Pp
 .It livesync.cluster.node_ip Ar ip_address
 Specifies one trusted IP address for sync between trackers running in a cluster. Can be added more than once.
diff --git a/opentracker.conf.sample b/opentracker.conf.sample
index 3cd30dc..7b35369 100644
--- a/opentracker.conf.sample
+++ b/opentracker.conf.sample
@@ -138,14 +138,22 @@
 # livesync.cluster.listen 192.168.0.1:9696
 #
 #      Note that two udp sockets will be opened. One on ip address 0.0.0.0
-#      port 9696, that will join the multicast group 224.0.23.5 for incoming
-#      udp packets and one on ip address 192.168.0.1 port 9696 for outgoing
-#      udp packets. The group is the same for every opentracker, so a cluster
-#      is kept to itself by the node_ip list below and by the ttl of 1 that
-#      keeps packets on the local link.
+#      port 9696, that will join the multicast group for incoming udp packets
+#      and one on ip address 192.168.0.1 port 9696 for outgoing ones. Every
+#      member of a cluster has to agree on the port as well as the group.
+#
+#      The group a cluster talks in.
+#
+# livesync.cluster.group 239.192.23.5
+#
+#      For reaosns of backwards comaptibility with legacy trackers, it
+#      defaults to 224.0.23.5, which, technically sits in a range IANA
+#      assigns on request.
+#
+#      A cluster of your own is better served by an address out of
+#      239.192.0.0/14, which RFC 2365 sets aside for private use. Packets
+#      carry a ttl of 1 either way, so they never leave the local link.
 #
-#      As of now one and only one ip address must be given, if opentracker
-#      was built with the WANT_SYNC_LIVE feature.
 #
 
 # IV)  Sync between trackers running in a cluster is restricted to packets
diff --git a/src/ot_config.c b/src/ot_config.c
index 95136b5..6b51c61 100644
--- a/src/ot_config.c
+++ b/src/ot_config.c
@@ -208,6 +208,7 @@ typedef enum {
     CONFIG_TRACKER_REDIRECT_URL,
     CONFIG_LIVESYNC_CLUSTER_NODE_IP,
     CONFIG_LIVESYNC_CLUSTER_LISTEN,
+    CONFIG_LIVESYNC_CLUSTER_GROUP,
 } config_kind;
 
 typedef struct {
@@ -247,6 +248,7 @@ static const config_key config_keys[] = {
 #ifdef WANT_SYNC_LIVE
     CONFIG_KEY("livesync.cluster.node_ip", CONFIG_LIVESYNC_CLUSTER_NODE_IP),
     CONFIG_KEY("livesync.cluster.listen",  CONFIG_LIVESYNC_CLUSTER_LISTEN),
+    CONFIG_KEY("livesync.cluster.group",   CONFIG_LIVESYNC_CLUSTER_GROUP),
 #endif
 };
 static const size_t config_entry_count = sizeof(config_keys) / sizeof(config_keys[0]);
@@ -318,7 +320,7 @@ static int parse_configfile(char *config_filename) {
         int off     = scan_ip6_port(p, tmpip, &tmpport);
 
         if (!off || !listen_scan_options(p + off, &shared, &workers))
-          goto parse_error;
+          exerr("listen.tcp, listen.udp and listen.tcp_udp want an address, optionally a port and options.");
         if (kind != CONFIG_LISTEN_UDP)
           listen_append(tmpip, tmpport, FLAG_TCP, shared, workers), ++bound;
         if (kind != CONFIG_LISTEN_TCP)
@@ -378,6 +380,10 @@ static int parse_configfile(char *config_filename) {
         set_config_option(&g_redirecturl, p);
         break;
 #ifdef WANT_SYNC_LIVE
+      case CONFIG_LIVESYNC_CLUSTER_GROUP:
+        if (!ip_parse(p, tmpip) || livesync_set_group(tmpip) == -1)
+          exerr("livesync.cluster.group wants a multicast address, out of 224.0.0.0/4.");
+        break;
       case CONFIG_LIVESYNC_CLUSTER_NODE_IP:
         if (!scan_ip6_net(p, &tmpnet))
           goto parse_error;
@@ -440,7 +446,7 @@ static void help(char *name) {
   fprintf(stderr, "\nExample:   ./opentracker -i 127.0.0.1 -p 6969 -P 6969 -f ./opentracker.conf -i 10.1.1.23 -p 2710 -p 80\n");
   fprintf(stderr, "           Here -i 127.0.0.1 selects the ip address for the next -p 6969 and -P 6969.\n");
   fprintf(stderr, "           If no port is bound from config file or command line, the last address given\n");
-  fprintf(stderr, "           (or ::1 if none is set) will be used on port 6969.\n");
+  fprintf(stderr, "           is used on port 6969, and every interface if none was given.\n");
 }
 #undef HELPLINE
 
diff --git a/src/ot_livesync.c b/src/ot_livesync.c
index 11b75d2..85577f7 100644
--- a/src/ot_livesync.c
+++ b/src/ot_livesync.c
@@ -22,8 +22,10 @@
 
 #ifdef WANT_SYNC_LIVE
 
-ot_ip6 groupip_1 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 224, 0, 23, 5};
-static uint16_t g_port = LIVESYNC_PORT;
+/* The multicast group, legacy opentrackers joined by default 224.0.23.5, so it
+   stays the default */
+static ot_ip6   g_group = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 224, 0, 23, 5};
+static uint16_t g_port  = LIVESYNC_PORT;
 
 #define LIVESYNC_INCOMING_BUFFSIZE        (256 * 256)
 
@@ -96,6 +98,14 @@ void livesync_deinit(void) {
     close(g_socket_out);
 }
 
+int livesync_set_group(const ot_ip6 group) {
+  /* v4 multicast is 224.0.0.0/4, and the socket layer has nothing else */
+  if (!ip_is_v4mapped(group) || (group[12] & 0xf0) != 0xe0)
+    return -1;
+  memcpy(g_group, group, sizeof(ot_ip6));
+  return 0;
+}
+
 void livesync_bind_mcast(ot_ip6 ip, uint16_t port) {
   if (!ip_is_v4mapped(ip))
     exerr("v6 mcast support not yet available.");
@@ -105,7 +115,7 @@ void livesync_bind_mcast(ot_ip6 ip, uint16_t port) {
 
   g_port = port;
 
-  if ((g_socket_in = socket_mcast_join(groupip_1, ip, port)) == -1)
+  if ((g_socket_in = socket_mcast_join(g_group, ip, port)) == -1)
     exerr("Error: Cant bind live sync incoming socket.");
 
   if ((g_socket_out = socket_mcast_bind(ip, port, 0)) == -1)
@@ -125,7 +135,7 @@ static void livesync_issue_peersync(sync_buffer *buf) {
      has modified the protected element */
   pthread_mutex_unlock(&g_outbuf_mutex);
 
-  socket_mcast_send(g_socket_out, mycopy, fill, groupip_1, g_port);
+  socket_mcast_send(g_socket_out, mycopy, fill, g_group, g_port);
 }
 
 static void livesync_handle_peersync(struct ot_workstruct *ws, size_t peer_size) {
diff --git a/src/ot_livesync.h b/src/ot_livesync.h
index d8d20b3..cf3c64c 100644
--- a/src/ot_livesync.h
+++ b/src/ot_livesync.h
@@ -48,6 +48,10 @@
 void livesync_init(void);
 void livesync_deinit(void);
 
+/* The group to join and to send to, (defaults to 224.0.23.5).
+   Returns -1 if group is not a v4 multicast address. */
+int  livesync_set_group(const ot_ip6 group);
+
 /* Join multicast group for listening and create sending socket */
 void livesync_bind_mcast(ot_ip6 ip, uint16_t port);
 
diff --git a/tests/integration/harness.py b/tests/integration/harness.py
index a9d38ca..10aa6de 100644
--- a/tests/integration/harness.py
+++ b/tests/integration/harness.py
@@ -192,7 +192,7 @@ class Tracker:
     def torrent_count(self):
         return int(self.stats("torr").split()[0])
 
-    def livesync_send(self, payload):
+    def livesync_send(self, payload, group=LIVESYNC_GROUP):
         """Put a packet on the live sync group.
 
         It has to go to the group rather than straight at the port: the
@@ -204,7 +204,7 @@ class Tracker:
             sock.setsockopt(socket.IPPROTO_IP, socket.IP_MULTICAST_IF, socket.inet_aton("127.0.0.1"))
             sock.setsockopt(socket.IPPROTO_IP, socket.IP_MULTICAST_LOOP, 1)
             sock.setsockopt(socket.IPPROTO_IP, socket.IP_MULTICAST_TTL, 1)
-            sock.sendto(payload, (LIVESYNC_GROUP, self.livesync_port))
+            sock.sendto(payload, (group, self.livesync_port))
         finally:
             sock.close()
 
diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py
index 1a84fa7..2f6bdc4 100644
--- a/tests/integration/test_config.py
+++ b/tests/integration/test_config.py
@@ -1,7 +1,9 @@
 """Settings the tracker has to accept, and still work with.
 
-An unrecognised key makes opentracker refuse to start, so a test that it
-comes up at all is a test that the key was understood.
+An unrecognised key only warns, so coming up proves nothing on its own and a
+test that cares has to look at what the tracker said. A key that is
+recognised but cannot be parsed is another matter, and for the listen
+statements it stops the tracker.
 """
 
 import unittest
@@ -14,6 +16,7 @@ class TcpWorkers(TrackerTestCase):
 
     def test_the_tracker_serves_with_tcp_workers_asked_for(self):
         self.assertEqual(self.tracker.announce(self.hash_for("tcpworkers"))[b"complete"], 1)
+        self.assertNotIn("Unhandled line", self.tracker.output())
 
 
 class WorkersOnAListenStatement(unittest.TestCase):
@@ -27,3 +30,22 @@ class WorkersOnAListenStatement(unittest.TestCase):
             self.assertEqual(complaints, [], "the tracker reported:\n" + "\n".join(complaints))
         finally:
             tracker._cleanup()
+
+
+class AListenLineThatDoesNotParse(unittest.TestCase):
+    """Warning and carrying on left a tracker that had been told where to
+    listen coming up on its default instead, which is every interface."""
+
+    def refuses(self, line):
+        with self.assertRaises(AssertionError) as caught:
+            Tracker(line)
+        return str(caught.exception)
+
+    def test_an_address_that_is_not_one_stops_the_tracker(self):
+        self.assertIn("want an address", self.refuses("listen.tcp 10.0.0.300:7999\n"))
+
+    def test_so_does_a_port_out_of_range(self):
+        self.assertIn("want an address", self.refuses("listen.udp 127.0.0.1:99999\n"))
+
+    def test_so_does_an_option_it_does_not_know(self):
+        self.assertIn("want an address", self.refuses("listen.tcp 127.0.0.1:7999 wokers=2\n"))
diff --git a/tests/integration/test_livesync.py b/tests/integration/test_livesync.py
index 3a9ee58..f4cb2db 100644
--- a/tests/integration/test_livesync.py
+++ b/tests/integration/test_livesync.py
@@ -10,7 +10,7 @@ import struct
 import subprocess
 import unittest
 
-from harness import TRACKER, TrackerTestCase
+from harness import TRACKER, Tracker, TrackerTestCase
 
 # enum { OT_SYNC_PEER4, OT_SYNC_PEER6 } in src/ot_livesync.c
 SYNC_PEER4 = 0
@@ -164,3 +164,64 @@ class LiveSyncUnconfigured(TrackerTestCase):
         for i in range(200):
             self.tracker.announce(self.hash_for("no-cluster-%d" % i), port=6881 + i, left=0)
         self.assertEqual(self.tracker.torrent_count(), 200)
+
+
+# RFC 2365 sets 239.192.0.0/14 aside for exactly this kind of use
+OWN_GROUP = "239.192.0.7"
+# A group nothing in this suite ever joins. Deliberately not the default one:
+# other trackers here do join that, on ports the harness picks at random, and
+# a socket bound to the wildcard address shares a port's multicast traffic
+# with anything else on the host that has joined the group it is addressed
+# to. Two clusters sharing a host and a port is pathological in the field but
+# routine in a test suite.
+UNJOINED_GROUP = "239.192.0.99"
+
+
+@unittest.skipUnless(has_livesync(), "this build has no WANT_SYNC_LIVE")
+class LiveSyncGroup(TrackerTestCase):
+    config = ("livesync.cluster.group %s\n"
+              "livesync.cluster.listen 127.0.0.1:{livesync_port}\n"
+              "livesync.cluster.node_ip 127.0.0.1\n" % OWN_GROUP)
+
+    def test_the_configured_group_is_joined(self):
+        info_hash = self.hash_for("own-group")
+        self.tracker.livesync_send(packet(SYNC_PEER4, [(info_hash, peer4("10.1.3.1", 6881, FLAG_SEEDING))]),
+                                   group=OWN_GROUP)
+        self.assertTrue(self.tracker.wait_until(lambda: self.tracker.torrent_count() == 1))
+
+    def test_another_group_is_left_alone(self):
+        """A cluster that picked its own group hears nothing from the others."""
+        info_hash = self.hash_for("not-our-group")
+        self.tracker.livesync_send(packet(SYNC_PEER4, [(info_hash, peer4("10.1.3.2", 6881, FLAG_SEEDING))]),
+                                   group=UNJOINED_GROUP)
+        self.assertFalse(self.tracker.wait_until(lambda: self.tracker.torrent_count() != 0, timeout=1))
+
+
+@unittest.skipUnless(has_livesync(), "this build has no WANT_SYNC_LIVE")
+class LiveSyncGroupRefused(unittest.TestCase):
+    """A cluster that asked for a group of its own must not quietly end up on
+    the one every other opentracker is on, so a line that does not name a
+    group stops the tracker rather than falling back."""
+
+    def start_with(self, group):
+        return Tracker("livesync.cluster.group %s\n"
+                       "livesync.cluster.listen 127.0.0.1:{livesync_port}\n"
+                       "livesync.cluster.node_ip 127.0.0.1\n" % group)
+
+    def test_a_unicast_address_stops_the_tracker(self):
+        with self.assertRaises(AssertionError) as caught:
+            self.start_with("10.0.0.1")
+        self.assertIn("multicast address", str(caught.exception))
+
+    def test_so_does_something_that_is_not_an_address(self):
+        with self.assertRaises(AssertionError) as caught:
+            self.start_with("not-an-address")
+        self.assertIn("multicast address", str(caught.exception))
+
+    def test_a_group_at_either_end_of_the_range_is_taken(self):
+        for group in ("224.0.0.1", "239.255.255.255"):
+            tracker = self.start_with(group)
+            try:
+                self.assertEqual(tracker.torrent_count(), 0)
+            finally:
+                tracker.stop()