Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate unique MAC for bridges used for external networking #34

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to review if this OUI makes sense to use, we also must make sure we set the unicast/multicast and universal/local bits correctly.

# 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