Fixes to multicast server behavior

A server will now no longer send RSTs in response to unmatched responses
from a multicast source, and not show warnings any more when sending
responses to multicast requests.

.gitlab-ci.yml

@@ -30,6 +30,7 @@ test:3.7:
     - python3 -m coverage report --include=aiocoap/\*
     - mkdir collected-coverage/3.7/ -p
     - mv .coverage* collected-coverage/3.7/
+    - "AIOCOAP_TEST_MCIF=\"$(ip -j -6 route list default | python3 -c 'import sys, json; print(json.load(sys.stdin)[0][\"dev\"])')\" ./setup.py test"
   artifacts:
     paths:
       - collected-coverage

aiocoap/interfaces.py

@@ -145,6 +145,16 @@ def scheme(Self):
     message size when the block handlers can get serialization length
     predictions from the remote. Must be divisible by 1024."""
 
+    def as_response_address(self):
+        """Address to be assigned to a response to messages that arrived with
+        this message
+
+        This can (and does, by default) return self, but gives the protocol the
+        opportunity to react to create a modified copy to deal with variations
+        from multicast.
+        """
+        return self
+
 class MessageManager(metaclass=abc.ABCMeta):
     """The interface an entity that drives a MessageInterface provides towards
     the MessageInterface for callbacks and object acquisition."""

aiocoap/messagemanager.py

@@ -98,10 +98,13 @@ def dispatch_message(self, message):
                 if message.mtype is CON:
                     self._send_empty_ack(message.remote, message.mid, reason="acknowledging incoming response")
             else:
-                self.log.info("Response not recognized - sending RST.")
-                rst = Message(mtype=RST, mid=message.mid, code=EMPTY, payload='')
-                rst.remote = message.remote
-                self._send_initially(rst)
+                if message.remote.is_multicast_locally:
+                    self.log.info("Ignoring response incoming with multicast destination.")
+                else:
+                    self.log.info("Response not recognized - sending RST.")
+                    rst = Message(mtype=RST, mid=message.mid, code=EMPTY, payload='')
+                    rst.remote = message.remote.as_response_address()
+                    self._send_initially(rst)
         else:
             self.log.warning("Received a message with code %s and type %s (those don't fit) from %s, ignoring it."%(message.code, message.mtype, message.remote))
 
@@ -276,7 +279,7 @@ def _retransmit(self, message, timeout, retransmission_counter):
     def _process_ping(self, message):
         self.log.info('Received CoAP Ping from %s, replying with RST.'%(message.remote,))
         rst = Message(mtype=RST, mid=message.mid, code=EMPTY, payload=b'')
-        rst.remote = message.remote
+        rst.remote = message.remote.as_response_address()
         # not going via send_message because that would strip the mid, and we
         # already know that it can go straight to the wire
         self._send_initially(rst)
@@ -358,7 +361,7 @@ def send_message(self, message, exchange_monitor=None):
 
                 if no_response:
                     new_message = Message(code=EMPTY, mid=mid, mtype=ACK)
-                    new_message.remote = message.remote
+                    new_message.remote = message.remote.as_response_address()
                     message = new_message
                     self.log.debug("Turning to-be-sent message into an empty ACK due to no_response option.")
                 else:
@@ -441,7 +444,7 @@ def _send_empty_ack(self, remote, mid, reason):
                 code=EMPTY,
                 payload=b"",
                 )
-        ack.remote = remote
+        ack.remote = remote.as_response_address()
         ack.mid = mid
         # not going via send_message because that would strip the mid, and we
         # already know that it can go straight to the wire

aiocoap/tokenmanager.py

@@ -127,7 +127,7 @@ async def run():
                     m = ev.message
                     # FIXME: should this code warn if token or remote are set?
                     m.token = request.token
-                    m.remote = request.remote
+                    m.remote = request.remote.as_response_address()
                     self.token_interface.send_message(m)
                 else:
                     self.log.error("Requests shouldn't receive errors at the level of a TokenManager any more, but this did: %s", ev.exception)

aiocoap/transports/udp6.py

@@ -144,6 +144,15 @@ def is_multicast(self):
     def is_multicast_locally(self):
         return ipaddress.ip_address(self._plainaddress_local()).is_multicast
 
+    def as_response_address(self):
+        if not self.is_multicast_locally:
+            return self
+
+        # Create a copy without pktinfo, as responses to messages received to
+        # multicast addresses can not have their request's destination address
+        # as source address
+        return type(self)(self.sockaddr, self.interface)
+
 
 class SockExtendedErr(namedtuple("_SockExtendedErr", "ee_errno ee_origin ee_type ee_code ee_pad ee_info ee_data")):
     _struct = struct.Struct("IbbbbII")
@@ -255,14 +264,8 @@ async def shutdown(self):
     def send(self, message):
         ancdata = []
         if message.remote.pktinfo is not None:
-            if message.remote.is_multicast_locally:
-                # this is kind of a last-resort location; the `response.remote
-                # = request.remote` places should better consider this.
-                self.log.warn("Dropping pktinfo from ancdata because it" \
-                        " indicates a multicast address")
-            else:
-                ancdata.append((socket.IPPROTO_IPV6, socket.IPV6_PKTINFO,
-                    message.remote.pktinfo))
+            ancdata.append((socket.IPPROTO_IPV6, socket.IPV6_PKTINFO,
+                message.remote.pktinfo))
         self.transport.sendmsg(message.encode(), ancdata, 0, message.remote.sockaddr)
 
     async def recognize_remote(self, remote):

tests/test_noncoap_client.py

@@ -13,11 +13,22 @@
 import asyncio
 import signal
 import contextlib
+import os
+import unittest
 
 import aiocoap
 
 from .test_server import WithTestServer, precise_warnings, no_warnings, asynctest
 
+# For some reasons site-local requests do not work on my test setup, resorting
+# to link-local; that means a link needs to be given, and while we never need
+# to find the default multicast interface to join MC groups, we need to know it
+# to address them. This needs support from outside the test suite right now.
+_skip_unless_defaultmcif = unittest.skipIf(
+        "AIOCOAP_TEST_MCIF" not in os.environ,
+        "Multicast tests require AIOCOAP_TEST_MCIF environment variable to tell"
+        " the default multicast interface")
+
 class TimeoutError(RuntimeError):
     """Raised when a non-async operation times out"""
 
@@ -34,17 +45,22 @@ def after(cls, time):
         signal.alarm(0)
         signal.signal(signal.SIGALRM, old)
 
-class TestNoncoapClient(WithTestServer):
+class WithMockSock(unittest.TestCase):
     def setUp(self):
-        super(TestNoncoapClient, self).setUp()
+        super().setUp()
 
         self.mocksock = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
-        self.mocksock.connect((self.serveraddress, aiocoap.COAP_PORT))
 
     def tearDown(self):
         self.mocksock.close()
 
-        super(TestNoncoapClient, self).tearDown()
+        super().tearDown()
+
+class TestNoncoapClient(WithTestServer, WithMockSock):
+    def setUp(self):
+        super().setUp()
+
+        self.mocksock.connect((self.serveraddress, aiocoap.COAP_PORT))
 
     @precise_warnings(["Ignoring unparsable message from ..."])
     @asynctest
@@ -101,3 +117,44 @@ async def test_noresponse(self):
             self.assertTrue(False, "Response was sent when No-Response should have suppressed it")
         except TimeoutError:
             pass
+
+    @no_warnings
+    @asynctest
+    async def test_unknownresponse_reset(self):
+        self.mocksock.send(bytes.fromhex("4040ffff"))
+        await asyncio.sleep(0.1)
+        with TimeoutError.after(1):
+            response = self.mocksock.recv(1024)
+        self.assertEqual(response, bytes.fromhex("7000ffff"), "Unknown CON Response did not trigger RST")
+
+# Skipping the whole class when no multicast address was given (as otherwise
+# it'd try binding :: which is bound to fail with a simplesocketserver setting)
+@_skip_unless_defaultmcif
+class TestNoncoapMulticastClient(WithTestServer, WithMockSock):
+    # This exposes the test server to traffic from the environment system for
+    # some time; it's only run if a default multicast inteface is given
+    # explicitly, though.
+    serveraddress = '::'
+
+    @no_warnings
+    @asynctest
+    async def test_mutlicast_ping(self):
+        # exactly like the unicast case -- just to verify we're actually reaching our server
+        self.mocksock.sendto(b'\x40\x00\x99\x9a', (aiocoap.numbers.constants.MCAST_IPV6_LINKLOCAL_ALLNODES, aiocoap.COAP_PORT, 0, socket.if_nametoindex(os.environ['AIOCOAP_TEST_MCIF'])))
+        await asyncio.sleep(0.1)
+        with TimeoutError.after(1):
+            response = self.mocksock.recv(1024)
+        assert response == b'\x70\x00\x99\x9a'
+
+    @no_warnings
+    @asynctest
+    async def test_multicast_unknownresponse_noreset(self):
+        self.mocksock.sendto(bytes.fromhex("4040ffff"), (aiocoap.numbers.constants.MCAST_IPV6_LINKLOCAL_ALLNODES, aiocoap.COAP_PORT, 0, socket.if_nametoindex(os.environ['AIOCOAP_TEST_MCIF'])))
+        await asyncio.sleep(0.1)
+        try:
+            with TimeoutError.after(1):
+                response = self.mocksock.recv(1024)
+        except TimeoutError:
+            pass
+        else:
+            self.assertEqual(False, "Message was sent back responding to CON response to multicast address")