Sanitize MAC addresses

This patch sanitizes the MAC address coming from a user input:
- The "base_mac" address configuration parameter.
- The "port.mac_address" stored in the database, if the script
  provided is not executed.

This patch relays on [1], that will sanitize any input coming from
the server API.

This patch adds a new script to sanitize all "port.mac_address"
registers stored in the dabatabase.

[1]https://review.opendev.org/c/openstack/neutron-lib/+/788300

Related-Bug: #1926273

Change-Id: I8572906cc435feda1f82263fd94dda47fc1526e1

neutron/agent/l2/extensions/dhcp/base.py

@@ -18,6 +18,7 @@ import math
 import struct
 
 import netaddr
+from neutron_lib.api import converters
 from os_ken.lib import addrconv
 from os_ken.lib.packet import dhcp
 from os_ken.lib.packet import dhcp6
@@ -47,7 +48,8 @@ class DHCPResponderBase(base_oskenapp.BaseNeutronAgentOSKenApp):
         self.version = version
         self.name = "DHCP%sResponder" % version
 
-        self.hw_addr = cfg.CONF.base_mac
+        self.hw_addr = converters.convert_to_sanitized_mac_address(
+            cfg.CONF.base_mac)
         self.register_packet_in_handler(self._packet_in_handler)
 
     def _packet_in_handler(self, event):

neutron/cmd/sanitize_port_mac_addresses.py

@@ -0,0 +1,55 @@
+#  Licensed under the Apache License, Version 2.0 (the "License"); you may
+#  not use this file except in compliance with the License. You may obtain
+#  a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#  License for the specific language governing permissions and limitations
+#  under the License.
+
+from neutron_lib.api import converters
+from neutron_lib import context
+from neutron_lib.db import api as db_api
+from oslo_config import cfg
+from oslo_db import options as db_options
+from oslo_log import log as logging
+
+from neutron.db import models_v2
+
+
+LOG = logging.getLogger(__name__)
+
+
+def setup_conf():
+    conf = cfg.CONF
+    db_group, neutron_db_opts = db_options.list_opts()[0]
+    cfg.CONF.register_cli_opts(neutron_db_opts, db_group)
+    conf()
+
+
+def main():
+    """Main method for sanitizing the database ``port.mac_address`` column.
+
+    This script will sanitize all ``port.mac_address`` columns existing in the
+    database. The output format will be xx:xx:xx:xx:xx:xx.
+    """
+    setup_conf()
+    admin_ctx = context.get_admin_context()
+    with db_api.CONTEXT_WRITER.using(admin_ctx):
+        for port in admin_ctx.session.query(models_v2.Port.id,
+                                            models_v2.Port.mac_address).all():
+            if port[1] == converters.convert_to_sanitized_mac_address(port[1]):
+                continue
+
+            query = admin_ctx.session.query(models_v2.Port)
+            port_db = query.filter(models_v2.Port.id == port[0]).first()
+            if not port_db:
+                continue
+
+            mac_address = converters.convert_to_sanitized_mac_address(port[1])
+            port_db.update({'mac_address': mac_address})
+            LOG.info('Port %s updated, MAC address: %s', port[0],
+                     mac_address)

neutron/cmd/upgrade_checks/checks.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from neutron_lib.api import converters
 from neutron_lib import constants
 from neutron_lib import context
 from neutron_lib.db import model_query
@@ -83,6 +84,12 @@ def count_vlan_allocations_invalid_segmentation_id():
     return query.count()
 
 
+def port_mac_addresses():
+    ctx = context.get_admin_context()
+    return [port[0] for port in
+            ctx.session.query(models_v2.Port.mac_address).all()]
+
+
 class CoreChecks(base.BaseChecks):
 
     def get_checks(self):
@@ -100,6 +107,8 @@ class CoreChecks(base.BaseChecks):
              self.vlan_allocations_segid_check),
             (_('Policy File JSON to YAML Migration'),
              (common_checks.check_policy_json, {'conf': cfg.CONF})),
+            (_('Port MAC address sanity check'),
+             self.port_mac_address_sanity),
         ]
 
     @staticmethod
@@ -284,3 +293,29 @@ class CoreChecks(base.BaseChecks):
             upgradecheck.Code.SUCCESS,
             _("All 'ml2_vlan_allocations' registers have a valid "
               "segmentation ID."))
+
+    @staticmethod
+    def port_mac_address_sanity(checker):
+        """Checks the MAC address sanity of each port in the BD
+
+        All MAC addresses should be stored in the format xx:xx:xx:xx:xx:xx.
+        """
+        if not cfg.CONF.database.connection:
+            return upgradecheck.Result(
+                upgradecheck.Code.WARNING,
+                _("Database connection string is not set. Check for port MAC "
+                  "sanity can't be done."))
+
+        for mac in port_mac_addresses():
+            if mac != converters.convert_to_sanitized_mac_address(mac):
+                return upgradecheck.Result(
+                    upgradecheck.Code.WARNING,
+                    _("There port MAC addresses not correctly formated in the"
+                      "database. The script "
+                      "neutron-sanitize-port-mac-addresses should be "
+                      "executed"))
+
+        return upgradecheck.Result(
+            upgradecheck.Code.SUCCESS,
+            _("All port MAC addresses are correctly formated in the "
+              "database."))

neutron/db/db_base_plugin_v2.py

@@ -16,6 +16,7 @@
 import functools
 
 import netaddr
+from neutron_lib.api import converters
 from neutron_lib.api.definitions import external_net as extnet_def
 from neutron_lib.api.definitions import ip_allocation as ipalloc_apidef
 from neutron_lib.api.definitions import port as port_def
@@ -1477,7 +1478,9 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
                 context, current_owner, current_device_id,
                 db_port['tenant_id'])
 
-        if new_mac and new_mac != db_port['mac_address']:
+        if (new_mac and
+                new_mac != converters.convert_to_sanitized_mac_address(
+                    db_port['mac_address'])):
             self._check_mac_addr_update(context, db_port,
                                         new_mac, current_owner)
 
@@ -1565,9 +1568,10 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         if vif_type is not None:
             query = query.filter(Port.port_bindings.any(vif_type=vif_type))
         if mac_address:
-            lowered_macs = [x.lower() for x in mac_address]
+            sanitized_macs = [converters.convert_to_sanitized_mac_address(x)
-            query = query.filter(func.lower(Port.mac_address).in_(
+                              for x in mac_address]
-                                                    lowered_macs))
+            query = query.filter(
+                func.lower(Port.mac_address).in_(sanitized_macs))
         if ip_addresses:
             query = query.filter(
                 Port.fixed_ips.any(IPAllocation.ip_address.in_(ip_addresses)))

neutron/tests/unit/cmd/upgrade_checks/test_checks.py

@@ -190,3 +190,14 @@ class TestChecks(base.BaseTestCase):
                 result = checks.CoreChecks.vlan_allocations_segid_check(
                     mock.ANY)
                 self.assertEqual(returned_code, result.code)
+
+    def test_port_mac_address_sanity(self):
+        cases = ((['ca:fe:ca:fe:ca:fe'], Code.SUCCESS),
+                 (['ca:fe:ca:fe:ca:f'], Code.WARNING))
+        with mock.patch.object(
+                checks, 'port_mac_addresses') \
+                as mock_port_mac_addresses:
+            for mac_addresses, returned_code in cases:
+                mock_port_mac_addresses.return_value = mac_addresses
+                result = checks.CoreChecks.port_mac_address_sanity(mock.ANY)
+                self.assertEqual(returned_code, result.code)

releasenotes/notes/sanitize-port-mac-address-732f451942483e21.yaml

@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    The ``port.mac_address`` field is sanitized to have a common format
+    "xx:xx:xx:xx:xx:xx". The values stored in the database can be sanitized
+    executing the new script provided ``neutron-sanitize-port-mac-addresses``.
+    This script will read all ``port`` registers and fix, if needed, the
+    stored MAC address format.
+    The ``port`` API is also modified to sanitize the user input. This change
+    was added to neutron-lib 2.12.0 in
+    `788300 <https://review.opendev.org/c/openstack/neutron-lib/+/788300>`_.

setup.cfg

@@ -61,6 +61,7 @@ console_scripts =
     neutron-ovn-metadata-agent = neutron.cmd.eventlet.agents.ovn_metadata:main
     neutron-ovn-migration-mtu = neutron.cmd.ovn.migration_mtu:main
     neutron-ovn-db-sync-util = neutron.cmd.ovn.neutron_ovn_db_sync_util:main
+    neutron-sanitize-port-mac-addresses = neutron.cmd.sanitize_port_mac_addresses:main
     ml2ovn-trace = neutron.cmd.ovn.ml2ovn_trace:main
 neutron.core_plugins =
     ml2 = neutron.plugins.ml2.plugin:Ml2Plugin