Merge pull request #39 from kytos-ng/fix/get_cookie_overflow

fix: 2022.3.1 release: coloring `get_cookie` can generate an cookie that overflows 8 bytes

CHANGELOG.rst

@@ -6,6 +6,23 @@ All notable changes to the coloring NApp will be documented in this file.
 [UNRELEASED] - Under development
 ********************************
 
+[2022.3.1] - 2023-02-17
+***********************
+
+Fixed
+=====
+- ``get_cookie`` could overflow 8 bytes for certain dpid values
+- Made sure that the generated matched ``dl_src`` is a local unicast address
+
+General Information
+===================
+
+If you have been running this NApp version 2022.3.0 or prior in production it's recommended that you delete the previous flows before you start ``kytosd`` again since it this new version can end up pushing flows might not completely overwrite the old ones depending on dpids values:
+
+.. code:: bash
+
+  $ curl -X DELETE http://127.0.0.1:8181/api/kytos/flow_manager/v2/flows/ -H 'Content-type: application/json' -d '{ "flows": [ { "cookie": 12393906174523604992, "cookie_mask": 18374686479671623680 } ] }'
+
 [2022.3.0] - 2022-12-15
 ***********************
 

kytos.json

@@ -3,7 +3,7 @@
   "username": "amlight",
   "name": "coloring",
   "description": "NApp to color a network topology",
-  "version": "2022.3.0",
+  "version": "2022.3.1",
   "napp_dependencies": ["amlight/flow_stats", "kytos/flow_manager"],
   "license": "",
   "tags": [],

main.py

@@ -15,6 +15,7 @@
 from kytos.core import KytosNApp, log, rest
 from kytos.core.helpers import listen_to
 from napps.amlight.coloring import settings
+from napps.amlight.coloring.utils import make_unicast_local_mac
 from pyof.v0x04.common.port import PortNo
 
 
@@ -121,10 +122,10 @@ def color_to_field(color, field='dl_src'):
         :return: A representation of the color suitable for the given field
         """
         if field in ('dl_src', 'dl_dst'):
-            color_48bits = color & 0xffffffffffffffff
-            int_mac = struct.pack('!Q', color_48bits)[2:]
-            color_value = ':'.join([f'{b:02x}' for b in int_mac])
-            return color_value.replace('00', 'ee')
+            color_64bits = color & 0xffffffffffffffff
+            int_mac_6bytes = struct.pack('!Q', color_64bits)[2:]
+            color_value = ':'.join([f'{b:02x}' for b in int_mac_6bytes])
+            return make_unicast_local_mac(color_value.replace('00', 'ee'))
         if field in ('nw_src', 'nw_dst'):
             color_32bits = color & 0xffffffff
             int_ip = struct.pack('!L', color_32bits)
@@ -181,6 +182,6 @@ def return_settings():
 
     @staticmethod
     def get_cookie(dpid):
-        """Get cookie integer"""
-        return (int(dpid.replace(":", ""), 16)) + \
-               (settings.COOKIE_PREFIX << 56)
+        """Get 8-byte integer cookie."""
+        int_dpid = int(dpid.replace(":", ""), 16)
+        return (0x00FFFFFFFFFFFFFF & int_dpid) | (settings.COOKIE_PREFIX << 56)

tests/unit/test_main.py

@@ -218,3 +218,10 @@ def test_rest_colors_without_switches(self):
         json_response = json.loads(response.data)
 
         self.assertEqual(json_response['colors'], {})
+
+    def test_get_cookie(self) -> None:
+        """test get_cookie."""
+        dpid = "cc4e244b11000000"
+        assert Main.get_cookie(dpid) == 0xac4e244b11000000
+        dpid = "0000000000000001"
+        assert Main.get_cookie(dpid) == 0xac00000000000001

tests/unit/test_utils.py

@@ -0,0 +1,18 @@
+"""Test utils.py."""
+import pytest
+
+
+from napps.amlight.coloring.utils import make_unicast_local_mac
+
+
+def test_make_unicast_local_mac_valid() -> None:
+    """test make_unicast_local_mac."""
+    mac = "31:94:06:ee:ee:ee"
+    assert make_unicast_local_mac(mac) == "3e:94:06:ee:ee:ee"
+
+
+@pytest.mark.parametrize("mac", ["31:94:06:ee:ee:eea", "a", ""])
+def test_make_unicast_local_mac_errors(mac) -> None:
+    """test make_unicast_local_mac."""
+    with pytest.raises(ValueError):
+        assert make_unicast_local_mac(mac)

utils.py

@@ -0,0 +1,17 @@
+"""Utilities."""
+import re
+
+MAC_ADDR = re.compile("([0-9A-Fa-f]{2}[-:]){5}[0-9A-Fa-f]{2}$")
+
+
+def make_unicast_local_mac(mac: str) -> str:
+    """
+    The first two bits (b0, b1) of the most significant MAC address byte is for
+    its uniquiness and wether its locally administered or not. This functions
+    ensures it's a unicast (b0 -> 0) and locally administered (b1 -> 1).
+    """
+    if not re.search(MAC_ADDR, mac):
+        msg = "Invalid mac '{mac}': expected this regex format: {MAC_ADDR}"
+        raise ValueError(msg)
+    mac = mac.lower()
+    return mac[:1] + "e" + mac[2:]