Erik-Lamers1 / vnet-manager

Virtual network manager - Manages containers and VMs to create a virtual network setup
MIT License
11 stars 7 forks source link

MAC address validation fails with TypeError, exception not caught for proper logging #62

Open Southparkfan opened 10 months ago

Southparkfan commented 10 months ago

If you supply an invalid MAC address to an interface, vnet-manager should complain to you in clear words:

2023-11-29 15:55:39,221 [vnet_manager.config.validate] [ERROR] MAC 33212743742 for interface eth123 on machine router2, does not seem to be valid. Please check your settings

Instead, a TypeError is thrown, which is not caught by the validation code:

Traceback (most recent call last):
  File "/usr/local/bin/vnet-manager", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/vnet_manager.py", line 47, in main
    return manager.execute(args["action"])
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/actions/manager.py", line 72, in execute
    if self.config_path and action not in ("connect", "list") and not self.parse_config():
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/actions/manager.py", line 88, in parse_config
    check_result, self.config = self.check_and_update_config()
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/actions/manager.py", line 101, in check_and_update_config
    validator.validate()
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/config/validate.py", line 61, in validate
    self.validate_machine_config()
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/config/validate.py", line 122, in validate_machine_config
    self.validate_interface_config(name)
  File "/usr/local/lib/python3.10/dist-packages/vnet_manager/config/validate.py", line 248, in validate_interface_config
    elif not fullmatch(r"^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$", int_vals["mac"]):
  File "/usr/lib/python3.10/re.py", line 195, in fullmatch
    return _compile(pattern, flags).fullmatch(string)
TypeError: expected string or bytes-like object

Quick fix:

            #elif not fullmatch(r"^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$", int_vals["mac"]):
            try:
                fullmatch(r"^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$", int_vals["mac"])
            except TypeError:

One of the offending line was mac: 42:42:42:42:00:01. Wrapping the MAC address in single quotes resolved the issue.

Erik-Lamers1 commented 10 months ago

Nice find :) The current implementation of the config validation leaves a lot to be desired. I once started an effort to refactor this using Pydantic, but I didn't finish it.

Please raise a PR and I'll check and merge it.

Southparkfan commented 10 months ago

... and now I understand what's wrong here: https://en.wikipedia.org/wiki/Sexagesimal#YAML

42:42:42:42:09:01 will be parsed as (((((42*60)+42)*60+42)*60+42)*60+9)*60+1 (= 33212743741).

I have changed the validation logic and added tests for MAC addresses on my laptop, but if we can switch to YAML 1.2, which does not contain support for sexagesimal numbers, we don't need to tell users to wrap certain MAC addresses (i.e. where every two hexadecimals are in the range of 0.0-5.9) between quotes. If I'm correct, pyyaml doesn't support 1.2 yet, though :(.