Skip to content

Commit

Permalink
Generate unique MAC for bridges used for external networking
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fnordahl committed Jan 22, 2021
1 parent b5a86ea commit 7a3f6db
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 1 deletion.
52 changes: 52 additions & 0 deletions lib/charms/ovn_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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."""
Expand Down
26 changes: 25 additions & 1 deletion unit_tests/test_lib_charms_ovn_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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',
Expand All @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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',
Expand All @@ -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([
Expand Down Expand Up @@ -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):

Expand All @@ -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',
Expand Down

0 comments on commit 7a3f6db

Please sign in to comment.