mirror of
https://github.com/linux-system-roles/network.git
synced 2026-01-23 02:15:17 +00:00
fix: Refine MAC validation using interface name
When a user provides both an interface name and a MAC address, the current validation process retrieves sysfs link info separately using the interface name and the MAC address, then compares the results. If the information doesn't match, an error is raised. However, this approach may trigger false alarms because retrieving the link info by MAC might return the link info that only matches the current MAC instead of the permanent MAC. Since the interface name is unique within the kernel, a more robust validation method is to fetch the MAC address using the interface name and then compare it directly with the user-provided MAC address. Resolves: https://issues.redhat.com/browse/RHEL-84362 Signed-off-by: Wen Liang <liangwen12year@gmail.com>
This commit is contained in:
Wen Liang
liangwen12year
parent
cac2bbb43e
commit
fe7c6c6fd5
5 changed files with 129 additions and 9 deletions
|
|
@ -95,6 +95,7 @@ ABSENT_STATE = "absent"
|
|||
|
||||
DEFAULT_ACTIVATION_TIMEOUT = 90
|
||||
DEFAULT_TIMEOUT = 10
|
||||
NULL_MAC = "00:00:00:00:00:00"
|
||||
|
||||
|
||||
class CheckMode:
|
||||
|
|
@ -166,6 +167,20 @@ class SysUtil:
|
|||
c = SysUtil._sysctl_read("/sys/class/net/" + ifname + "/address")
|
||||
return Util.mac_norm(c.strip())
|
||||
|
||||
@staticmethod
|
||||
def _link_read_bond_port_perm_hwaddr(ifname):
|
||||
filename = os.path.join(
|
||||
"/sys/class/net",
|
||||
ifname,
|
||||
# wokeignore:rule=slave
|
||||
"bonding_slave",
|
||||
"perm_hwaddr",
|
||||
)
|
||||
if not os.path.exists(filename):
|
||||
return None
|
||||
c = SysUtil._sysctl_read(filename)
|
||||
return Util.mac_norm(c.strip())
|
||||
|
||||
@staticmethod
|
||||
def _link_read_permaddress(ifname):
|
||||
return ethtool.get_perm_addr(ifname)
|
||||
|
|
@ -186,6 +201,13 @@ class SysUtil:
|
|||
"ifname": ifname,
|
||||
"address": SysUtil._link_read_address(ifname),
|
||||
"perm-address": SysUtil._link_read_permaddress(ifname),
|
||||
# When an interface is added as a port of a bonding device, its MAC
|
||||
# address might change, we need to retrieve and preserve the original
|
||||
# MAC address to ensure the user-provided interface name and MAC match
|
||||
# correctly.
|
||||
"bond-port-perm-hwaddr": SysUtil._link_read_bond_port_perm_hwaddr(
|
||||
ifname
|
||||
),
|
||||
}
|
||||
return links
|
||||
|
||||
|
|
@ -2130,14 +2152,13 @@ class Cmd(object):
|
|||
for idx, connection in enumerate(self.connections):
|
||||
if "type" in connection and connection["check_iface_exists"]:
|
||||
# when the profile is tied to a certain interface via
|
||||
# 'interface_name' or 'mac', check that such an interface
|
||||
# 'interface_name' and 'mac', check that such an interface
|
||||
# exists.
|
||||
#
|
||||
# This check has many flaws, as we don't check whether the
|
||||
# existing interface has the right device type. Also, there is
|
||||
# some ambiguity between the current MAC address and the
|
||||
# permanent MAC address.
|
||||
li_mac = None
|
||||
li_ifname = None
|
||||
if connection["interface_name"]:
|
||||
li_ifname = SysUtil.link_info_find(connection["interface_name"])
|
||||
|
|
@ -2156,13 +2177,23 @@ class Cmd(object):
|
|||
"infiniband interface exists"
|
||||
% (connection["interface_name"]),
|
||||
)
|
||||
if li_mac and li_ifname and li_mac != li_ifname:
|
||||
self.log_fatal(
|
||||
idx,
|
||||
"profile specifies interface_name '%s' and mac '%s' but no "
|
||||
"such interface exists"
|
||||
% (connection["interface_name"], connection["mac"]),
|
||||
)
|
||||
elif connection["mac"]:
|
||||
perm_address = li_ifname.get("perm-address", NULL_MAC)
|
||||
current_address = li_ifname.get("address", NULL_MAC)
|
||||
bond_port_perm_hwaddr = li_ifname.get(
|
||||
"bond-port-perm-hwaddr", NULL_MAC
|
||||
)
|
||||
if (perm_address not in (NULL_MAC, connection["mac"])) or (
|
||||
perm_address == NULL_MAC
|
||||
and connection["mac"]
|
||||
not in (current_address, bond_port_perm_hwaddr)
|
||||
):
|
||||
self.log_fatal(
|
||||
idx,
|
||||
"profile specifies interface_name '%s' and mac '%s' "
|
||||
"but no such interface exists"
|
||||
% (connection["interface_name"], connection["mac"]),
|
||||
)
|
||||
|
||||
def start_transaction(self):
|
||||
"""Hook before making changes"""
|
||||
|
|
|
|||
|
|
@ -74,6 +74,7 @@ ibution_major_version | int < 9",
|
|||
},
|
||||
"playbooks/tests_ignore_auto_dns.yml": {},
|
||||
"playbooks/tests_bond_options.yml": {},
|
||||
"playbooks/tests_bond_port_match_by_mac.yml": {},
|
||||
"playbooks/tests_eth_dns_support.yml": {},
|
||||
"playbooks/tests_dummy.yml": {}, # wokeignore:rule=dummy
|
||||
"playbooks/tests_infiniband.yml": {},
|
||||
|
|
|
|||
40
tests/playbooks/tests_bond_port_match_by_mac.yml
Normal file
40
tests/playbooks/tests_bond_port_match_by_mac.yml
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
# SPDX-License-Identifier: BSD-3-Clause
|
||||
---
|
||||
- name: Play for creating the connection to match the port device
|
||||
based on the perm_hwaddr
|
||||
hosts: all
|
||||
vars:
|
||||
controller_profile: bond0
|
||||
controller_device: nm-bond
|
||||
port1_profile: bond0.0
|
||||
dhcp_interface1: test1
|
||||
port2_profile: bond0.1
|
||||
dhcp_interface2: test2
|
||||
profile: test2conn
|
||||
interface: test2
|
||||
tasks:
|
||||
- name: Test creating the connection to match the port device
|
||||
based on the perm_hwaddr
|
||||
tags:
|
||||
- tests::bond:create
|
||||
block:
|
||||
- name: Include the task 'run_test.yml'
|
||||
include_tasks: tasks/run_test.yml
|
||||
vars:
|
||||
lsr_description: Given two DHCP-enabled network interfaces,
|
||||
when creating a bond profile with them,
|
||||
then we can still create the connection to match the port device
|
||||
based on the perm_hwaddr
|
||||
lsr_setup:
|
||||
- tasks/create_test_interfaces_with_dhcp.yml
|
||||
- tasks/assert_dhcp_device_present.yml
|
||||
lsr_test:
|
||||
- tasks/create_bond_profile.yml
|
||||
- tasks/create_bond_port_match_by_mac.yml
|
||||
lsr_assert:
|
||||
- tasks/assert_controller_device_present.yml
|
||||
- tasks/assert_profile_present.yml
|
||||
lsr_cleanup:
|
||||
- tasks/cleanup_bond_profile+device.yml
|
||||
- tasks/remove_test_interfaces_with_dhcp.yml
|
||||
- tasks/check_network_dns.yml
|
||||
25
tests/tasks/create_bond_port_match_by_mac.yml
Normal file
25
tests/tasks/create_bond_port_match_by_mac.yml
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
# SPDX-License-Identifier: BSD-3-Clause
|
||||
---
|
||||
- name: Retrieve perm_hwaddr using ethtool
|
||||
# wokeignore:rule=slave
|
||||
command: cat /sys/class/net/{{ interface }}/bonding_slave/perm_hwaddr
|
||||
register: mac_address_result
|
||||
changed_when: false
|
||||
failed_when: mac_address_result.rc != 0
|
||||
- name: Set the MAC address variable
|
||||
set_fact:
|
||||
mac: "{{ mac_address_result.stdout_lines[-1].split(' ')[-1] }}"
|
||||
- name: Display the retrieved MAC address
|
||||
debug:
|
||||
msg: "Retrieved MAC address for {{ interface }}: {{ mac }}"
|
||||
- name: Test matching the port device based on the perm_hwaddr
|
||||
import_role:
|
||||
name: linux-system-roles.network
|
||||
vars:
|
||||
network_connections:
|
||||
- name: "{{ profile }}"
|
||||
state: up
|
||||
type: ethernet
|
||||
interface_name: "{{ interface }}"
|
||||
mac: "{{ mac }}"
|
||||
...
|
||||
23
tests/tests_bond_port_match_by_mac_nm.yml
Normal file
23
tests/tests_bond_port_match_by_mac_nm.yml
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
# SPDX-License-Identifier: BSD-3-Clause
|
||||
# This file was generated by ensure_provider_tests.py
|
||||
---
|
||||
# set network provider and gather facts
|
||||
# yamllint disable rule:line-length
|
||||
- name: Run playbook 'playbooks/tests_bond_port_match_by_mac.yml' with nm as provider
|
||||
hosts: all
|
||||
tasks:
|
||||
- name: Include the task 'el_repo_setup.yml'
|
||||
include_tasks: tasks/el_repo_setup.yml
|
||||
- name: Set network provider to 'nm'
|
||||
set_fact:
|
||||
network_provider: nm
|
||||
tags:
|
||||
- always
|
||||
|
||||
|
||||
# The test requires or should run with NetworkManager, therefore it cannot run
|
||||
# on RHEL/CentOS 6
|
||||
- name: Import the playbook 'playbooks/tests_bond_port_match_by_mac.yml'
|
||||
import_playbook: playbooks/tests_bond_port_match_by_mac.yml
|
||||
when:
|
||||
- ansible_distribution_major_version != '6'
|
||||
Loading…
Add table
Add a link
Reference in a new issue