From 827cca2ed75b159c3a1a2e5f193150fe32491b1b Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 5 May 2021 10:45:36 +0000 Subject: [PATCH] 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 | 4 +- neutron/cmd/sanitize_port_mac_addresses.py | 55 +++++++++++++++++++ neutron/cmd/upgrade_checks/checks.py | 35 ++++++++++++ neutron/db/db_base_plugin_v2.py | 12 ++-- .../unit/cmd/upgrade_checks/test_checks.py | 11 ++++ ...ize-port-mac-address-732f451942483e21.yaml | 11 ++++ setup.cfg | 1 + 7 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 neutron/cmd/sanitize_port_mac_addresses.py create mode 100644 releasenotes/notes/sanitize-port-mac-address-732f451942483e21.yaml diff --git a/neutron/agent/l2/extensions/dhcp/base.py b/neutron/agent/l2/extensions/dhcp/base.py index 5525718fe37..357e07d09e1 100644 --- a/neutron/agent/l2/extensions/dhcp/base.py +++ b/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): diff --git a/neutron/cmd/sanitize_port_mac_addresses.py b/neutron/cmd/sanitize_port_mac_addresses.py new file mode 100644 index 00000000000..61762e5350e --- /dev/null +++ b/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) diff --git a/neutron/cmd/upgrade_checks/checks.py b/neutron/cmd/upgrade_checks/checks.py index a478bc6d0f4..4645b398ce2 100644 --- a/neutron/cmd/upgrade_checks/checks.py +++ b/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.")) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index ac269f76a98..020b7c15d73 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/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] - query = query.filter(func.lower(Port.mac_address).in_( - lowered_macs)) + sanitized_macs = [converters.convert_to_sanitized_mac_address(x) + for x in mac_address] + 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))) diff --git a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py b/neutron/tests/unit/cmd/upgrade_checks/test_checks.py index 9744c133b22..dd33f6c1eee 100644 --- a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py +++ b/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) diff --git a/releasenotes/notes/sanitize-port-mac-address-732f451942483e21.yaml b/releasenotes/notes/sanitize-port-mac-address-732f451942483e21.yaml new file mode 100644 index 00000000000..4b7a929d685 --- /dev/null +++ b/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 `_. diff --git a/setup.cfg b/setup.cfg index 9cbb883f3ae..fb3770b1e3b 100644 --- a/setup.cfg +++ b/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