From 7a3f6db654e351f46ceadc5e54af872fd2e151c1 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Thu, 21 Jan 2021 22:05:59 +0100 Subject: [PATCH] Generate unique MAC for bridges used for external networking When creating a bridge in Open vSwitch, a interface representing that bridge will appear in the system. Open vSwitch will use the lowest MAC address of the interfaces added to the bridge as MAC address of the bridge representor interface. Since the advent of predictable interface naming in Linux it has become common for network configuration renderers and backends to express network configuration in such a way that users will use the MAC address of an interface to match where a certain network config belongs. These two factors together creates a situation where the backend Netplan.io configures may choose to rename and use the Open vSwitch bridge representor interface and apply network config to it instead of using the real interface. To work around this issue we generate an unique MAC address for the bridges we add physical network interfaces to. Related-Bug: #1912643 --- lib/charms/ovn_charm.py | 52 +++++++++++++++++++++++++ unit_tests/test_lib_charms_ovn_charm.py | 26 ++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/lib/charms/ovn_charm.py b/lib/charms/ovn_charm.py index 9cef5d4..a9e9ba6 100644 --- a/lib/charms/ovn_charm.py +++ b/lib/charms/ovn_charm.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. import collections +import functools +import hashlib +import hmac import ipaddress import os import subprocess @@ -678,6 +681,10 @@ def configure_bridges(self): # for bridges used for external connectivity we want the # datapath to act like an ordinary MAC-learning switch. **{'fail-mode': 'standalone'}, + # Workaround for netplan LP: #1912643 + **{'other-config': { + 'hwaddr': self.unique_bridge_mac( + self.get_hashed_machine_id('charm-ovn-chassis'), br)}}, }) for port in bpi[br]: ifdatamap = bpi.get_ifdatamap(br, port) @@ -729,6 +736,51 @@ def render_nrpe(self): charm_nrpe, self.nrpe_check_services, current_unit) charm_nrpe.write() + @staticmethod + def get_hashed_machine_id(app_id): + """Get local machine ID. + + The machine ID must be treated as confidential information and we + cannot expose it or parts of it, especially not on the network. + + :param app_id: Application specific ID used when hashing machine ID. + :type app_id: str + :returns: machine ID + :rtype: bytearray + :raises: OSError + """ + with open('/etc/machine-id', 'r') as fin: + return hmac.new( + bytes.fromhex(fin.read().rstrip()), + msg=bytes(app_id, 'utf-8'), + digestmod=hashlib.sha256).digest() + + @staticmethod + def unique_bridge_mac(machine_id, bridge_name): + """Generate uniqe mac address for use on a bridge interface. + + The bridge interface will be visible in the datapath and as such the + address we choose must be globally unique. We accomplish this by + composing a MAC address from the local machine-id(5), a prefix and the + name of the bridge. + + :param machine_id: Local machine ID. + :type machine_id: bytearray + :param bridge_name: Name of bridge for which the address will be used. + :type bridge_name: str + :returns: String representation of generated MAC address. + :rtype: str + """ + # prefix from the IANA 64-bit MAC Unassigned range + generated = bytearray.fromhex('b61d9e') + # extend two last bytes of hashed machine ID + generated.extend(machine_id[-2:]) + # append checksum of bridge name + generated.append( + functools.reduce( + lambda x, y: x ^ y, [ord(c) for c in bridge_name])) + return ':'.join('{:02x}'.format(b) for b in generated) + class BaseTrainOVNChassisCharm(BaseOVNChassisCharm): """Train incarnation of the OVN Chassis base charm class.""" diff --git a/unit_tests/test_lib_charms_ovn_charm.py b/unit_tests/test_lib_charms_ovn_charm.py index 1c4549d..f0176b7 100644 --- a/unit_tests/test_lib_charms_ovn_charm.py +++ b/unit_tests/test_lib_charms_ovn_charm.py @@ -266,6 +266,8 @@ def test_configure_bridges(self): self.patch_object(ovn_charm.ch_ovs, 'add_bridge_port') self.patch_target('check_if_paused') self.check_if_paused.return_value = ('some', 'reason') + self.patch_target('unique_bridge_mac') + self.unique_bridge_mac.return_value = 'fa:ke:ma:ca:dd:rs' self.target.configure_bridges() self.BridgePortInterfaceMap.assert_not_called() self.check_if_paused.return_value = (None, None) @@ -296,6 +298,7 @@ def test_configure_bridges(self): 'datapath-type': 'netdev', 'protocols': 'OpenFlow13,OpenFlow15', 'fail-mode': 'standalone', + 'other-config': {'hwaddr': 'fa:ke:ma:ca:dd:rs'}, }), mock.call( 'br-ex', @@ -304,6 +307,7 @@ def test_configure_bridges(self): 'datapath-type': 'netdev', 'protocols': 'OpenFlow13,OpenFlow15', 'fail-mode': 'standalone', + 'other-config': {'hwaddr': 'fa:ke:ma:ca:dd:rs'}, }), ], any_order=True) self.add_bridge_bond.assert_called_once_with( @@ -567,6 +571,8 @@ def test_configure_bridges(self): self.patch_object(ovn_charm.ch_ovs, 'add_bridge_port') self.patch_target('check_if_paused') self.check_if_paused.return_value = ('some', 'reason') + self.patch_target('unique_bridge_mac') + self.unique_bridge_mac.return_value = 'fa:ke:ma:ca:dd:rs' self.target.configure_bridges() self.BridgePortInterfaceMap.assert_not_called() self.check_if_paused.return_value = (None, None) @@ -598,6 +604,7 @@ def test_configure_bridges(self): 'datapath-type': 'system', 'protocols': 'OpenFlow13,OpenFlow15', 'fail-mode': 'standalone', + 'other-config': {'hwaddr': 'fa:ke:ma:ca:dd:rs'}, }), mock.call( 'br-other', @@ -606,6 +613,7 @@ def test_configure_bridges(self): 'datapath-type': 'system', 'protocols': 'OpenFlow13,OpenFlow15', 'fail-mode': 'standalone', + 'other-config': {'hwaddr': 'fa:ke:ma:ca:dd:rs'}, }), ], any_order=True) self.add_bridge_port.assert_has_calls([ @@ -654,6 +662,23 @@ def test_resume(self): self.execl.assert_called_once_with( '/usr/bin/env', 'python3', '/some/path/hooks/config-changed') + def test_get_hashed_machine_id(self): + self.maxDiff = None + mocked_open = mock.mock_open(read_data='deadbeefcafe\n') + with mock.patch('builtins.open', mocked_open): + self.assertEquals( + self.target.get_hashed_machine_id('app'), + b'l\xee\xe7\x06+\x89\xf2*\x84\xe9\xaf\xc2to\xad\xc0\x07\xbapK' + b'\x93_\xb8Es\x08\xec7\x0fQT\x98') + mocked_open.assert_called_once_with( + '/etc/machine-id', 'r') + + def test_unique_bridge_mac(self): + self.assertEquals( + self.target.unique_bridge_mac( + bytearray.fromhex('deadbeef'), 'br-ex'), + 'b6:1d:9e:be:ef:20') + class TestSRIOVOVNChassisCharm(Helper): @@ -668,7 +693,6 @@ def setUp(self): self.enable_openstack.return_value = True def test__init__(self): - self.maxDiff = None self.assertEquals(self.target.packages, [ 'ovn-host', 'sriov-netplan-shim',