| Age | Commit message (Collapse) | Author |
|---|
|
Address the round-2 adversarial-review finding: warnings.catch_warnings()
mutates the process-global filter list for the duration of the block (still
the case on Python 3.13), so scoping suppression inside the library was not
actually isolated.
- http_api_client no longer touches warning filters at all. The
_suppress_insecure_warning helper is removed and post() calls
session.post directly again. The unused warnings, contextmanager and
urllib3 imports are dropped. The class docstring now states that callers
disabling verification own InsecureRequestWarning handling. The OSError
wrapping and verify_tls validation are unchanged.
- config_sync ignores InsecureRequestWarning in its __main__ block only,
before opmode.run(). That one-shot op-mode process deliberately connects
without verification (verify_tls=False) until T9396. Importing the module
adds no filter.
🤖 Generated by [robots](https://vyos.io)
|
|
Address the adversarial-review findings on the TLS verification change:
- Finding 1: validate ApiClientConfig.verify_tls in __post_init__. Only a
bool or a non-empty CA bundle path is accepted; anything else (including
"", None and ints) raises ValueError. requests treats "" as falsy and
would silently disable verification. Strings are never coerced: "false"
is a path, so callers must pass a real bool.
- Finding 2: stop calling urllib3.disable_warnings() process-wide from
ApiClient.__init__. InsecureRequestWarning is now ignored only around the
request itself, via warnings.catch_warnings(), and only when verify_tls
is False. post() is the only request path in the client.
- Finding 3: wrap OSError (raised by requests' cert_verify for a missing
CA bundle path) in ApiTransportError. The handler sits after the
RequestException handlers, since those subclass OSError.
- Finding 4: config-sync hard-codes verify_tls = False instead of reading
an unmapped 'verify_tls' key from the runtime config, whose leaf values
are strings. Mapping a CLI option to a bool or CA path is T9396.
🤖 Generated by [robots](https://vyos.io)
|
|
ApiClientConfig.verify_tls defaulted to False and ApiClient unconditionally
silenced urllib3's InsecureRequestWarning, so TLS peer verification was off by
default. The sole consumer, src/op_mode/config_sync.py, builds the client
without passing verify_tls and therefore connected to a *remote* secondary VyOS
node over HTTPS without verifying the certificate -- a MITM exposure on the
channel that carries the API key and the full configuration.
- ApiClientConfig.verify_tls now defaults to True and accepts bool | str;
a string is forwarded to requests as a CA bundle path (verify=<path>).
- InsecureRequestWarning is suppressed only when verification is explicitly
disabled (verify_tls is False), not when a CA path or True is set.
- config_sync reads an optional verify_tls from the secondary runtime settings,
defaulting to False to preserve current behaviour for self-signed secondaries.
The insecurity is now explicit at the call site instead of hidden in the
library default. Exposing a CA certificate (PKI reference) and peer
fingerprint pinning under service config-sync secondary is follow-up T9396.
🤖 Generated by [robots](https://vyos.io)
|
|
|
|
T9341: Fix consistency of legacy templates, xml_ref cache, and reference tree
|
|
modem_connect() only ran the --3gpp-set-initial-eps-bearer-settings
pre-step (added by T9326) when the requested ip-type included IPv6,
gated on options.partition('ip-type=')[2].partition(',')[0] in ('ipv6',
'ipv4v6'). A bare 'ipv4' request skipped it entirely.
Confirmed live (same MC7455/Google Fi setup as T9326's original report)
that the underlying ip-version-mismatch rejection isn't IPv6-specific:
with the attach committed to ip-type=ipv6, a bare ip-type=ipv4
--simple-connect fails outright with the same QMI CallEndReason, not
just the ipv4v6 case's silent single-family drop. A WWAN interface
whose configured address family changes (e.g. dropping dhcpv6 down to
dhcp-only) can hit exactly this, and won't self-heal on retry since
nothing about a failed connect touches the attach-committed PDN type -
vyos-netlinkd's reconcile loop would back off and retry the identical
failing request forever.
Removed the gate entirely: the pre-step already fails silently and
falls through unchanged on a modem/network that doesn't need it, so
there's no reason to skip it for any particular family.
|
|
wwan: T7487: identify modem by port ownership, not interface number
|
|
mmcli can return empty or non-JSON output while ModemManager is still
starting (T6604), and json.loads() raising on that is a ValueError, not an
OSError - it was propagating straight out of _modem_owning_port() uncaught.
Under wait=True that reaches wait_for()'s callback and aborts the poll loop
instead of retrying, exactly the startup race this module exists to ride
out.
Co-authored-by: Christian Breunig <christian@breunig.cc>
|
|
Bugfix which allows for BaseX link modes reported by ethtool to be correctly
validated, so config can be commited.
|
|
Requested by c-po: json.loads and wait_for were only used by
_modem_owning_port() and modem_index()'s wait branch respectively, so
importing them at module level cost every other caller of this module an
import it does not need.
|
|
modem_index() unconditionally polled for up to modem_wait_timeout before
returning None, so every caller that expects "no modem" as a routine
outcome - interface teardown, admin-disconnect markers, disconnect/connect
status checks, and vyos-netlinkd's reconcile loop checking every configured
interface once per pass - blocked on it too. The reconcile loop in
particular calls is_wwan_connected() synchronously, not per-interface, so
one interface with no modem stalled the whole daemon's reconcile pass,
including unrelated DHCP interfaces, every cycle.
Add a wait parameter, default False (single check, returns immediately).
Only start_modem_manager() passes wait=True, where waiting for a
still-initializing modem is the actual point.
|
|
modem_index() in python/vyos/utils/wwan.py derived the ModemManager modem index straight from the interface name (wwan0 -> 0). The kernel-assigned WWAN interface number and ModemManager's own modem index are two independently enumerated values and are not guaranteed to match - on a box with more than one modem, they can enumerate in different orders, so modem index 0 does not necessarily own wwan0. Every function in this module funnels through modem_index() (start_modem_manager, modem_disconnect, modem_connect, and both admin-disconnect marker helpers), and is_wwan_connected() in vyos.utils.network had its own separate copy of the same assumption - so a single reversed enumeration order silently commands the wrong physical modem for every one of them.
modem_index() now lists every modem ModemManager currently knows about and returns whichever one actually has ifname in its own port list, instead of assuming the correspondence. A modem that has not shown up on the bus yet looks the same as one that will never exist, so this waits up to the same modem_wait_timeout budget start_modem_manager() already gives a modem to become dialable, then returns None if nothing claimed the interface. is_wwan_connected() now calls the same resolver instead of keeping its own copy of the old assumption.
None is a real, expected return now, not just a theoretical one - the three callers that previously assumed modem_index() always produced *a* string (right or wrong) are updated: start_modem_manager() bails out early (matching the tolerance its own caller's w.exists() check already has for hardware that isn't detected yet), modem_disconnect() and modem_connect() treat it as nothing to disconnect / a failed dial, matching each function's own existing return contract. show_wwan.py's modem_info() raises a clear, specific error instead, which its own top-level exception handler already reports cleanly. set_admin_disconnected()/clear_admin_disconnected() only ever used modem_index() to validate the interface name looks like a WWAN interface in the first place, so they're unaffected.
|
|
wwan: T9350: possible dead interface after boot, disconnect does not stick
|
|
op-mode: T9354: replace "reset connection" with "reconnect interface"
|
|
T8977: Fix regression with is_addr_assigned and is_listen_port_bind_service
|
|
(#5504)
The Router Solicitation kicked off when an IPv6 autoconf PPPoE link comes up
is best effort - the peer may answer late or not at all. rdisc6(8) then blocks
for the duration of all retries and exits non-zero, which aborted the commit:
FileNotFoundError: [Errno 2] failed to run command: rdisc6 --single --retry 3 pppoe1000
returned: Soliciting ff02::2 (ff02::2) on pppoe1000...
Timed out.
No response.
exit code: 2
Run rdisc6(8) detached in a transient systemd unit, so neither the delay nor
the exit code reaches the commit. The outcome is logged to the journal.
|
|
accel-ppp: T9357: do not require local users when any-login is configured
|
|
create_private_key() took one "size" for every key type, but the number
means entirely different things per type: for RSA and DSA it is a modulus
size, for EC it selects a curve. CodeQL is not path sensitive across the
key_type comparison, so it follows the 256 an EC caller passes into the
RSA and DSA branches that caller can never reach, and reports pki.py as
generating 256 bit RSA and DSA keys.
Build elliptic curve keys in create_ec_private_key() instead and route the
op mode generator and the OpenVPN smoketest there, so no curve size ever
enters the function that also builds RSA and DSA keys.
create_private_key() now refuses a key type it does not build rather than
returning None, so a caller cannot drift back into the old shape
unnoticed.
|
|
The "default to SECP256R1" branch stored an instance where the other branch
stored the class, so the shared call at the end invoked the instance and
raised "'SECP256R1' object is not callable". Any curve size without a
matching SECP<size>R1 - the very case the default exists for - aborted
instead of falling back.
Take the default through getattr() so both paths carry the class.
|
|
Commit a9e9c4acfa ("pki: openvpn: T3642: Migrate OpenVPN to PKI and refactor")
was the last caller of verify_diffie_hellman_length() despite the build time
testcases.
Unused code is dead code, drop it.
|
|
The Diffie-Hellman length check stringified the requested minimum keysize
into a variable it never used, then converted the original argument again
for the comparison. A non-numeric value therefore escaped as an uncaught
ValueError from the comparison rather than being reported where it was
passed in.
The argument is now typed and validated up front: a non-integer raises
TypeError, booleans included, and a non-positive integer raises
ValueError. Both indicate a logic error in the calling script and should
fail loudly instead of silently reporting an undersized key. The only
callers today are the unit tests, so the contract is free to change.
A file that exists but holds no readable parameters is user input rather
than a logic error, and now fails the check instead of propagating the
error from openssl.
The bit-count pattern becomes a raw string, and the tests pass integer
keysizes and cover the new paths.
|
|
Whether an interface belongs to a VRF was only established when a VRF was
named explicitly. A lookup for the default VRF - what most callers ask for -
thus reported an address living in an arbitrary VRF, or on a VRF device
itself, as locally assigned, and accepted a configuration the daemon cannot
bind later on. Naming the VRF device remains the way to ask for the
addresses it carries.
Probing the next address family after a failed bind was unreachable, as the
argument is validated as a literal beforehand and then resolves to exactly
one family. The early return is restored.
An interface address carries host bits and must stay valid, but the range
variants classify untrusted input handed over to nftables, which only
accepts a prefix in canonical form. Those reject host bits again.
All of it is covered by tests.
|
|
The list of listening sockets is a snapshot, and the owning process was
looked up afterwards. A process exiting in between, or one belonging to
another user when we do not run privileged, raised an exception that
escaped and aborted the conf-mode script asking whether a port is taken.
Neither case can be the service being looked for, so such a socket is
skipped now.
|
|
Whether an address may be bound is not the same question as whether it is
currently configured somewhere: the kernel permits binding a foreign address
once the non-local bind sysctl is set, which is how a failover address is
commonly handed over after the commit. Callers can now opt into that, the
default is unchanged.
A lookup no longer skips the VRF device itself, which carries the addresses
of its own L3 domain, and an interface name can be retrieved for an address
without the caller asking a yes/no question in the first place.
Address and range validation is split into separate checks, so a caller no
longer has to accept ranges where only an address is meaningful. The two
duplicated version checks are dropped in favour of the existing ones.
Co-authored-by: Christian Breunig <christian@breunig.cc>
|
|
Committing "service pppoe-server authentication any-login" failed with
"Authentication mode local requires local users to be configured!" even
though any-login accepts any username/password combination, making local
user accounts pointless.
verify_accel_ppp_authentication() raised unconditionally whenever
authentication mode was "local" - the default - without ever consulting
the any-login option. Skip that requirement when any-login is set, while
keeping the password and rate-limit validation for users that are
configured.
|
|
"connect interface" and "disconnect interface" cannot be combined to restart a
dialer interface when the CLI session runs over that very interface, yet the
command which does cover it was filed under an unrelated top-level node and
was hard to find.
It is now "reconnect interface <ifname>", mirroring the existing connect and
disconnect commands. "reset connection" is removed, no alias is kept.
The removed command was reachable through the REST API as a "reset" path. A
dedicated "/reconnect" endpoint takes its place, so the operation stays
available to API consumers.
|
|
vpp: T9146: Enable DPDK no-multi-seg when a full frame fits the buffer
|
|
ppp: T9060: do not derive a link-local address from a synthetic MAC
|
|
A modem is on the bus while still probing, so a dial could start too early and
leave the interface without a session until something re-dialled it.
|
|
A PPP link has no hardware address. RFC 5072 defines the IPv6 interface
identifier of such a link as the one negotiated via IPV6CP, and that
negotiation is also what guarantees its uniqueness - which is why duplicate
address detection is skipped for the link-local address on PPP links.
VyOS additionally derived an EUI-64 link-local address from a synthetic MAC
address and assigned it to PPPoE and SSTP client interfaces. The peer knows
nothing about that address and its uniqueness was never verified. With two
link-local addresses present, RFC 6724 source address selection falls through
to rule 8 and may pick the synthetic one, so DHCPv6 prefix delegation and
router solicitations leave from an address the BRAS never negotiated. ISPs
validating the source address silently drop them and delegation stalls.
Both interface types now carry only the address negotiated via IPV6CP. A
leftover address on an established session is removed on commit.
|
|
vxlan: T9319: add VXLAN-GBP support
|
|
Two things kept an operator disconnect from taking effect.
vyos-netlinkd re-dials every configured WWAN interface that has lost its
session. It cannot tell a session lost to RF signal loss from one that was
taken down on purpose, so the disconnect was undone on the next reconcile
pass, at most 30 seconds later:
$ disconnect interface wwan0
08:53:33 t+10s state=registered
08:53:43 t+20s state=registered
08:53:53 t+30s state=connected <- re-dialled
08:54:03 t+40s state=connected
vyos-netlinkd: WWAN interface wwan0 is not connected, re-dialling...
vyos-netlinkd: Re-dialled WWAN interface wwan0
Record the intent instead of inferring it from the session state. op-mode
"disconnect" drops a marker in /run/vyos-wwan, which _reconcile_wwan() honours
and "connect" removes again. conf_mode removes it too, so a commit keeps
re-asserting the configured state - the marker only survives for as long as
nobody says otherwise, and never across a reboot.
|
|
/etc/cron.d/vyos-wwan re-ran the conf_mode script every 5 minutes via
op-mode "connect interface wwanN" to re-establish a session lost to RF
signal loss. Re-entering conf_mode from cron is unsafe: the cron job knows
nothing about the boot commit or about a commit in flight, so when a tick
lands before the running config is populated, get_config() returns nothing
but {'ifname', 'deleted'} and apply() takes the teardown branch instead of
dialling. It then stops the DHCP clients, stops ModemManager and unlinks its
own cron file, leaving wwan0 down with no way to recover until the next
commit:
CRON[1869]: (root) CMD (/usr/libexec/vyos/vyos-check-wwan.py)
systemd[1]: Stopping DHCP client on wwan0...
systemd[1]: Stopping Modem Manager...
This is timing dependent - it only bites when a */5 tick coincides with
boot - which is why it presents as an intermittently dead WWAN interface.
Move the re-dial into vyos-netlinkd, which already tracks these interfaces
for DHCP and already gates all of its work behind boot_configuration_complete()
and commit_in_progress2(). A re-dial can therefore no longer land on a
half-applied configuration. It also dials ModemManager directly rather than
re-entering conf_mode, so the teardown branch is no longer reachable from
the recovery path at all.
|
|
|
|
|
|
|
|
Expose the Group Policy extension as a valueless gbp CLI node and pass
it to iproute2 when creating VXLAN links. Recreate links when GBP
changes and reject configurations combining GBP with GPE.
Reject conflicting GBP settings on a shared UDP port, accounting for
address families and underlay VRF socket bindings. Warn that external
mode carries GBP in tunnel metadata instead of the packet mark.
Add smoketest coverage for GBP creation, removal and re-enabling,
retained interface settings, rejection without changing the running
tunnel, shared ports, separate address families, and external mode.
|
|
configtree: T9307: serialize libvyosconfig calls across threads
|
|
test_dhcp_vrf and test_dhcpv6_vrf resolved the DHCP client's PID, then read
the VRF's process list and asserted the PID appeared in it. The client
re-executes itself at moments of its own choosing, so between those two
steps it can come back with a new PID - the process is in the VRF, it simply
is not the process we asked about:
AssertionError: '13502' not found in '14322 dhclient\n
14324 dhclient-script\n
14485 dhclient...'
Observed failing and passing on the same image, so it costs a rerun rather
than telling anyone anything.
get_process_cmdline() already fights the same race for the /proc read.
Resolve the PID and read the VRF's list as one step and retry, so a restart
costs a retry instead of a failure.
The probe itself was also loose: substring-matching the raw output of
"ip vrf pids <name>" lets a PID check be satisfied by the process name
column or by a longer PID containing ours. Add
vyos.utils.network.get_vrf_pids(), which parses the command's
"<pid> <name>" lines into (pid, process_name) tuples and returns an empty
list for an unknown VRF, and compare the PID field as an int.
|
|
T8264: openvpn dco lifecycle
|
|
ipsec: T9254: do not emit ESN transforms in IKE proposals
|
|
assertDcoDataPath() checks the link kind, the operating mode and that the
daemon is up, and none of those change when the offload carries nothing.
A client whose server is not up yet never opens its tun, so VyOS creates the
"ovpn" device for it and OpenVPN has to adopt that device once the server
appears. Get the adoption wrong and the daemon takes the device but not its
ifindex: dco_new_peer() fails, the daemon exits and deletes the interface on
its way out, systemd starts it again and the second attempt creates a device
of its own. The tunnel comes up either way, so the state it ends in proves
nothing - the restart it cost does.
Ask the Kernel for its peers as well, which is the one thing no amount of
userspace bookkeeping can fake.
|
|
apply() restarts the daemon and then creates the interface itself, as a
tun device. OpenVPN declines the offload when it finds a device of the
wrong type, so DCO worked only until the first reconfiguration.
The Kernel module takes its operating mode at creation time and iproute2
can neither set nor report it, so go through netlink - multipoint for a
server, point-to-point otherwise, matching what OpenVPN asks for itself.
A server adopting a point-to-point device rejects every client, and the
attribute is dropped silently if it is ever unrecognised, so read the
mode back.
A raw option does not change this. verify() turns away the ones known to
drop the offload, and beyond those "openvpn-option" is documented as the
user's own responsibility - so an offloaded interface is created either
way. A raw option that does decline the offload now leaves the daemon
unable to open the device it is given, which is at least visible, rather
than quietly carrying traffic in userspace.
|
|
EthernetIf.remove() snapshotted Section.sub_interfaces() once and then removed
every entry of that list. For Q-in-Q setups the list contains both the vif-s
(eth0.10) and its vif-c children (eth0.10.20), as Section.sub_interfaces()
matches every configured interface whose name starts with "<ifname>.".
Removing eth0.10 cascades in the Kernel and also deletes eth0.10.20. Once the
loop reached eth0.10.20, Interface.__init__() no longer found it and - as
"create" defaults to True, re-created it via a bare "ip link add dev eth0.10.20"
without any type. This failed and left a stranded interface behind which in
turn broke all subsequent smoketests.
The same cascade silently killed DHCP clients running on sub-interfaces of any
other interface type, as only the interface itself got its addresses flushed:
* deletion of a bond or bridge interface carrying vif/vif-s/vif-c
* deletion of an entire vif-s, as get_removed_vlans() only populates
"vif_c_remove" for a vif-s which still exists in the new configuration
Move the sub-interface teardown from EthernetIf.remove() into the base class
Interface.remove(), where it benefits every interface type. The list is walked
deepest first (vif-c before vif-s/vif), which is what T7813 requires to stop
the DHCP client and return the lease, and interfaces which vanished in the
meantime are skipped so they are never accidentally re-created.
As a side effect of this move, an ethernet interface is now only placed into
A/D state after the base class has released the addresses - previously the
lease of the interface itself was returned over an already downed link.
|
|
ethernet: T9228: only warn about unsupported NIC features when node changed
|
|
get_ike_cipher()
get_esp_ike_cipher() took an esn flag defaulting to True (ESP/CHILD_SA
semantics), and every IKE_SA call site had to remember to pass
esn=False. Nothing enforced that, so a future call site could reintroduce
the ESN-in-IKE bug this task fixed.
Following review feedback from hedrok, split it into two public filters:
get_esp_cipher(group_config, ike_group=None) ESP/CHILD_SA, esn=True
get_ike_cipher(group_config) IKE_SA, esn=False always
The shared implementation moves to _get_esp_ike_cipher() (no
register_filter), so a template can no longer call it directly and
skip picking one of the two entry points.
Updated call sites: peer.j2, profile.j2, remote_access.j2, l2tp.j2
(proposals -> get_ike_cipher, esp_proposals -> get_esp_cipher), and
test_template.py.
|
|
libvyosconfig is not safe for concurrent entry. ctypes releases the GIL
for each foreign call, so threads in one process can be inside the
library at the same time; two concurrent from_string calls, or
from_string racing destroy, corrupt the shared parser state and
crash the process with SIGSEGV. vyos-http-api reaches this when a
configure operation runs in the threadpool while a retrieve is served
on the event loop.
Serialize every entry point on one process-wide RLock in the _Lib
wrapper. For the entry points that report through the library's global
error buffer, read the buffer while the lock is held and keep it per
thread, so get_error() returns the caller's own error; destroy, which
can run from __del__ between a failed call and get_error(), leaves it
untouched.
|
|
configquery: T8409: resolve op-mode config paths independent of edit level
|
|
firewall: T9157: add fib-type match for destination/source
|
|
EthernetIf.update() re-applies every ethernet knob on every commit touching
the interface. The capability messages (offload, flow-control, ring-buffer,
coalesce, switchdev) are emitted whenever the CLI-desired state differs from
what the driver reports - and on drivers where a feature is fixed
(virtio_net, vmxnet3, vif, veth, ...) that is permanently true. The message
is correct the first time, but it then re-appears on unrelated commits like
an interface description or IP address change.
Record in interfaces_ethernet.py which of the affected CLI nodes were
altered - the mechanism already used for speed_duplex_changed - and pass it
to the setters via a "warn" argument. Settings are still applied
unconditionally, only the message is gated.
While at it, consolidate the seven offload setters onto a common helper and
use Warning() instead of print().
|
