Skip to content
Merged
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
6 changes: 3 additions & 3 deletions debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,9 @@ Depends:
kbd,
# End "system option keyboard-layout"
# For "container"
podman (>=4.9.5),
netavark,
aardvark-dns,
podman (>=5.8),
netavark (>=1.14.0),
aardvark-dns (>=1.14.0),
# iptables is only used for containers now, not the the firewall CLI
iptables,
# End container
Expand Down
16 changes: 16 additions & 0 deletions op-mode-definitions/container.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@
</node>
</children>
</node>
<node name="interface">
<properties>
<help>Show host-side network interface used by each container</help>
</properties>
<!-- no admin check -->
<command>${vyos_op_scripts_dir}/container.py show_interface</command>
<children>
<node name="json">
<properties>
<help>Show host-side network interface used by each container in JSON format</help>
</properties>
<!-- no admin check -->
<command>${vyos_op_scripts_dir}/container.py show_interface --raw</command>
</node>
</children>
</node>
<tagNode name="log">
<properties>
<help>Show logs from a given container</help>
Expand Down
16 changes: 16 additions & 0 deletions python/vyos/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,27 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from hashlib import sha256

from vyos.config import Config
from vyos.ifconfig import Interface
from vyos.utils.dict import dict_search
from vyos.utils.network import interface_exists

def get_container_host_ifname(name: str) -> str:
"""
Deterministic host-side veth interface name for a container's network
attachment (verify() only allows one network per container). Kept within
IFNAMSIZ and - thanks to the leading "veth-" (a hyphen can never appear in
a VyOS "vethN" interface name) - guaranteed to never collide with the
"virtual-ethernet" naming scheme..
"""
prefix = f'veth-{name}'
if len(prefix) <= 15:
return prefix
digest = sha256(name.encode()).hexdigest()[:4]
return f'veth-{name[:5]}-{digest}'

def restart_network(config: Config) -> None:
"""
Start network and assign it to given VRF if requested.
Expand Down
52 changes: 52 additions & 0 deletions smoketest/scripts/cli/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,58 @@ def test_user_defined_mac(self):
n = cmd_to_json(['container', 'inspect', 'test2'])
self.assertEqual(n['NetworkSettings']['Networks']['bridge1']['MacAddress'], '02:00:00:00:00:02')

def test_long_name_host_interface_uniqueness(self):
# T7736: the deterministic host-side veth interface name derived
# from a container name is truncated to fit IFNAMSIZ. Two distinct
# but similarly-prefixed long names must not truncate to the same
# interface name - Podman would then refuse to attach the second
# container's network, and its systemd unit would fail to start.
net_name = 'longiftest'
prefix = '192.0.2.0/24'
name_1 = 'abcdefghij-1'
name_2 = 'abcdefghij-2'

self.cli_set(base_path + ['network', net_name, 'prefix', prefix])
self.cli_set(base_path + ['name', name_1, 'image', busybox_image])
self.cli_set(base_path + ['name', name_1, 'network', net_name, 'address', str(ip_interface(prefix).ip + 2)])
self.cli_set(base_path + ['name', name_2, 'image', busybox_image])
self.cli_set(base_path + ['name', name_2, 'network', net_name, 'address', str(ip_interface(prefix).ip + 3)])
self.cli_commit()

# Both containers run a "conmon" process at once, so checking by
# process name alone can't distinguish which container it belongs
# to - verify each container's own recorded PID is still alive
for name in (name_1, name_2):
pid = 0
with open(PROCESS_PIDFILE.format(name)) as f:
pid = int(f.read())
self.assertTrue(os.path.exists(f'/proc/{pid}'))

Comment on lines +267 to +293

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the host-interface contract, not only PID existence.

This test can pass even if host_interface_name is removed: Podman may allocate two distinct vethN interfaces automatically. /proc/<pid> existence also does not prove that the expected container is running. Assert each container’s actual host-side interface is present and distinct, and validate container state through Podman or systemd.

🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 289-289: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(PROCESS_PIDFILE.format(name))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(open-filename-from-request)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@smoketest/scripts/cli/test_container.py` around lines 267 - 293, The
test_long_name_host_interface_uniqueness test currently verifies only PID
existence, so it must also validate the host-interface contract and container
state. After cli_commit, obtain each container’s actual host-side interface
through the existing Podman or systemd inspection mechanism, assert both
interfaces are present and distinct, and verify each expected container is
running via Podman or systemd rather than relying on /proc/<pid> alone.

def test_colliding_host_interface_names(self):
# T7736: the host-side veth name is "veth-<name[:5]>-<hash[:4]>" for
# long container names - two distinct names can still (rarely) hash
# to the same result. These two are a confirmed real collision
# (both produce "veth-aaaa9-4ded") - verify() must reject the
# commit with a clear error instead of leaving it to Podman to fail
# obscurely when the second container's network attachment clashes
# with the first's interface name.
net_name = 'collidetest'
prefix = '192.0.2.0/24'
name_1 = 'aaaa9000005'
name_2 = 'aaaa9000336'

self.cli_set(base_path + ['network', net_name, 'prefix', prefix])
self.cli_set(base_path + ['name', name_1, 'image', busybox_image])
self.cli_set(base_path + ['name', name_1, 'network', net_name, 'address', str(ip_interface(prefix).ip + 2)])
self.cli_set(base_path + ['name', name_2, 'image', busybox_image])
self.cli_set(base_path + ['name', name_2, 'network', net_name, 'address', str(ip_interface(prefix).ip + 3)])

with self.assertRaises(ConfigSessionError):
self.cli_commit()

self.cli_delete(base_path + ['name', name_2])
self.cli_commit()

def test_ipv4_network(self):
prefix = '192.0.2.0/24'
base_name = 'ipv4'
Expand Down
36 changes: 32 additions & 4 deletions src/conf_mode/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from vyos.configdict import node_changed
from vyos.configdict import is_node_changed
from vyos.configverify import verify_vrf
from vyos.container import get_container_host_ifname
from vyos.container import restart_network
from vyos.utils.configfs import delete_cli_node
from vyos.utils.configfs import add_cli_node
Expand Down Expand Up @@ -122,6 +123,7 @@ def verify(container):
net_dict = {}
net_dict['mac'] = {}
net_dict['address'] = {}
net_dict['host_ifname'] = {}

for name, container_config in container['name'].items():
# Container image is a mandatory option
Expand Down Expand Up @@ -158,6 +160,19 @@ def verify(container):
if network_name not in container.get('network', {}):
raise ConfigError(f'Container network "{network_name}" does not exist!')

# T7736: two distinct (long) container names could truncate to
# the same host_interface_name - not applicable to macvlan networks,
# they attach without a paired host veth
network_type = dict_search(f'{network_name}.type', container['network'])
if dict_search('macvlan', network_type) is None:
host_ifname = get_container_host_ifname(name)
if host_ifname in net_dict['host_ifname']:
raise ConfigError(
f'Container "{name}" and "{net_dict["host_ifname"][host_ifname]}" '
f'both generate the host interface name "{host_ifname}" - please '
f'use less similar container names!')
net_dict['host_ifname'][host_ifname] = name

if 'name_server' in container_config and 'no_name_server' not in container['network'][network_name]:
raise ConfigError(f'Setting name server has no effect when attached container network has DNS enabled!')

Expand Down Expand Up @@ -361,7 +376,7 @@ def verify(container):
return None


def generate_run_arguments(name, container_config, host_ident):
def generate_run_arguments(name, container_config, host_ident, network_config):
image = container_config['image']
cpu_quota = container_config['cpu_quota']
memory = container_config['memory']
Expand Down Expand Up @@ -511,9 +526,19 @@ def generate_run_arguments(name, container_config, host_ident):
else:
ip_param = ''
addr_info = ''
networks = ",".join(container_config['network'])
network_opts = []
for network in container_config['network']:
network_name = network
# T7736: give the host-side veth a name that can never collide
# with a VyOS "virtual-ethernet vethN" interface.
type_config = dict_search(f'{network}.type', network_config)
is_macvlan = dict_search('macvlan', type_config) is not None
net_opt = network
if not is_macvlan:
ifname = get_container_host_ifname(name)
net_opt += f':host_interface_name={ifname}'
network_opts.append(net_opt)

if 'address' not in container_config['network'][network]:
continue
for address in container_config['network'][network]['address']:
Expand All @@ -524,6 +549,8 @@ def generate_run_arguments(name, container_config, host_ident):

addr_info = ''.join(container_config['network'][network]['address'])

networks = ' '.join(f'--network {opt}' for opt in network_opts)

get_mac = dict_search(f'network.{network_name}.mac', container_config)
if get_mac == 'auto' or get_mac is None:
mac_add = gen_mac(name, addr_info, host_ident)
Expand All @@ -546,7 +573,7 @@ def generate_run_arguments(name, container_config, host_ident):
delete_cli_node(mac_config_path)
add_cli_node(mac_config_path, value=mac_add)

net = f'--net {networks} {ip_param} {mac_address}'
net = f'{networks} {ip_param} {mac_address}'

return f'{container_base_cmd} {healthcheck} {net} {entrypoint} {image} {command} {command_arguments}'.strip()

Expand Down Expand Up @@ -626,12 +653,13 @@ def generate(container):

if 'name' in container:
host_ident = get_host_identity()
network_config = container.get('network', {})
for name, container_config in container['name'].items():
if 'disable' in container_config:
continue

file_path = os.path.join(systemd_unit_path, f'vyos-container-{name}.service')
run_args = generate_run_arguments(name, container_config, host_ident)
run_args = generate_run_arguments(name, container_config, host_ident, network_config)
render(file_path, 'container/systemd-unit.j2', {'name': name, 'run_args': run_args, },
formatter=lambda _: _.replace("&quot;", '"').replace("&apos;", "'"))

Expand Down
39 changes: 39 additions & 0 deletions src/op_mode/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import subprocess

from pathlib import Path
from tabulate import tabulate

from vyos.defaults import directories
from vyos.utils.process import cmdl
from vyos.utils.process import rc_cmd
Expand Down Expand Up @@ -163,6 +165,43 @@ def show_network(raw: bool):
else:
return cmdl(command.split())

def show_interface(raw: bool):
""" Show the deterministic host-side veth interface name (T7736) VyOS
assigns to each configured container's network attachment """
from vyos.configquery import ConfigTreeQuery
from vyos.container import get_container_host_ifname
from vyos.utils.dict import dict_search

conf = ConfigTreeQuery()
container = conf.get_config_dict(['container'], key_mangling=('-', '_'),
no_tag_node_value_mangle=True,
get_first_key=True,
with_recursive_defaults=True)

data = []
for name, container_config in container.get('name', {}).items():
if 'allow_host_networks' in container_config:
data.append({'name': name, 'network': None, 'interface': None})
continue
if 'network' not in container_config:
continue

network_name = list(container_config['network'])[0]
network_type = dict_search(f'network.{network_name}.type', container)
is_macvlan = dict_search('macvlan', network_type) is not None
interface = None if is_macvlan else get_container_host_ifname(name)
data.append({'name': name, 'network': network_name, 'interface': interface})

if raw:
return data

if not data:
return 'No containers configured!'

headers = ['Container', 'Network', 'Host Interface']
rows = [[d['name'], d['network'] or 'host', d['interface'] or 'n/a'] for d in data]
return tabulate(rows, headers)

def restart(name: str):
from vyos.utils.process import rc_cmd
from vyos.config import Config
Expand Down
Loading