Skip to content

openvpn: restore netifd route handling for pulled defaults#29198

Draft
pesa1234 wants to merge 2 commits intoopenwrt:masterfrom
pesa1234:fix-ovpn-route
Draft

openvpn: restore netifd route handling for pulled defaults#29198
pesa1234 wants to merge 2 commits intoopenwrt:masterfrom
pesa1234:fix-ovpn-route

Conversation

@pesa1234
Copy link
Copy Markdown
Contributor

@pesa1234 pesa1234 commented Apr 20, 2026

📦 Package Details

Maintainer: Alexandru Ardelean ardeleanalex@gmail.com

Description:
The netifd proto wrapper installs its own hotplug scripts and runs OpenVPN with --route-noexec and --ifconfig-noexec. Since the migration, client configs that rely on pushed redirect-gateway routes could lose the route back to the VPN server or add default routes even when the server did not request them.

Teach the hotplug helper to add a direct route to the VPN peer only when a full/default route is pulled, including redirect-gateway def1 and explicit default routes. Keep that route in a runtime state file so it can be removed on down/teardown, and tag the state with the OpenVPN daemon start time and pid to avoid stale hooks racing with a reconnect.

Also keep user supplied hook scripts behind the default hotplug wrapper. This avoids passing duplicate --up/--down hooks while still executing the user hooks through hotplug, and restores the pre-netifd script-security default of 2.

Tested with a NordVPN client pushing redirect-gateway def1. Also checked with sh -n on the shell scripts and git diff --check.


🧪 Run Testing Details

  • OpenWrt Version: snapshot
  • OpenWrt Target/Subtarget: mediatek/filogic
  • OpenWrt Device: gl-mt6000

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

If your PR contains a patch:

  • It can be applied using git am
  • It has been refreshed to avoid offsets, fuzzes, etc., using
    make package/<your-package>/refresh V=s
  • It is structured in a way that it is potentially upstreamable
    (e.g., subject line, commit description, etc.)
    We must try to upstream patches to reduce maintenance burden.

Copy link
Copy Markdown
Member

@feckert feckert left a comment

Choose a reason for hiding this comment

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

  • Please remove PKG_RELEASE from the commit body
  • Please check the 75-character limit in the commit message. The lines are simply too long.
  • As I see it, the changes are independent of one another. Could you please split them into two commits? That way, it will be easier to follow and review the chang

Comment thread net/openvpn/files/lib/netifd/proto/openvpn.sh Outdated
Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug Outdated
return 0
fi

if ip "-$family" route replace "$target/$prefix" via "$gateway" >/dev/null 2>&1; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be worth explaining why this is being done here with a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, I added a short comment explaining why the host route is needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I mean in the source code as a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

proto_add_ipv4_route "$trusted_ip" 32 "$route_net_gateway" should add the route
why you have to add the route via ip route ... commnad?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add the route via proto_add_ipv4_route or proto_add_ipv6_route
do not add it via ip ... command

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with ptpt52. We should try to manage that in netifd? To be able to say for certain whether that’s actually possible, I’ll need to take a closer look at it. It might well be that it doesn’t work, as you’ve already mentioned.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe yes in my opinion.
If you want I'm also available to do some check if you want.

@pesa1234 pesa1234 force-pushed the fix-ovpn-route branch 2 times, most recently from 2702366 to 2913d64 Compare April 20, 2026 12:36
@pesa1234
Copy link
Copy Markdown
Contributor Author

  • Please remove PKG_RELEASE from the commit body

    • Please check the 75-character limit in the commit message. The lines are simply too long.

    • As I see it, the changes are independent of one another. Could you please split them into two commits? That way, it will be easier to follow and review the chang

Thanks a lot for the review. I force-pushed an updated version.

I split the changes into two commits, removed the PKG_RELEASE note from
the commit body and reduced the commit messages to 75.

The first commit now handles the hotplug/script-security change, while
the second commit handles the pulled route/default route logic.

Copy link
Copy Markdown
Member

@feckert feckert left a comment

Choose a reason for hiding this comment

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

Here are a few more comments.

Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug
Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug
return 0
fi

if ip "-$family" route replace "$target/$prefix" via "$gateway" >/dev/null 2>&1; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I mean in the source code as a comment

Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug Outdated
Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug
Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug
Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug Outdated
Comment thread net/openvpn/files/usr/libexec/openvpn-hotplug Outdated
@pesa1234 pesa1234 force-pushed the fix-ovpn-route branch 4 times, most recently from bbec587 to 1cf17f8 Compare April 20, 2026 18:33
@pesa1234
Copy link
Copy Markdown
Contributor Author

Here are a few more comments.

Thanks again for your review and for your time.
Let me know if is necessary some other things to be changed.

Andrea

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 20, 2026

Sorry, I didn't fully understand your question. Could you provide a concrete configuration example? For instance, your current OpenVPN client config file, the server-side pushed route settings, and any specific error messages or symptoms you're seeing — that would help me assist you more accurately.

return 0
fi

if ip "-$family" route replace "$target/$prefix" via "$gateway" >/dev/null 2>&1; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add the route via proto_add_ipv4_route or proto_add_ipv6_route
do not add it via ip ... command

@pesa1234
Copy link
Copy Markdown
Contributor Author

Sorry, I didn't fully understand your question. Could you provide a concrete configuration example? For instance, your current OpenVPN client config file, the server-side pushed route settings, and any specific error messages or symptoms you're seeing — that would help me assist you more accurately.

Thanks for your reply. I hope this clarifies the issue; please ask if more details would help.

The goal of this PR is to restore the expected OpenVPN client behavior after the netifd proto handler changes.

There are two separate problems:

  1. Restore default hotplug handling

The proto handler installs its own up/down/route-up/route-pre-down hooks for netifd integration. If the user also configures custom OpenVPN hooks, OpenVPN only keeps the last hook and reports:

Multiple --up scripts defined. The previously configured script is overridden.
Multiple --down scripts defined. The previously configured script is overridden.

So the netifd hook can be overwritten, or the user hook can be lost.

  1. Restore pulled default route handling

The proto handler starts OpenVPN with route-noexec, so OpenVPN imports pushed routes but does not install them itself. This means the hotplug script must recreate the relevant routes: normal VPN routes through netifd, and the peer host route through the pre-tunnel gateway.

For a server pushing redirect-gateway def1, OpenVPN reports:

PUSH_REPLY,redirect-gateway def1,route-gateway 10.100.0.1,
ifconfig 10.1xx.0.2 255.255.240.0

and resolves the pre-tunnel gateway before opening tun0:

net_route_v4_best_gw query: dst 0.0.0.0
net_route_v4_best_gw result: via 192.168.178.1 dev eth1
net_route_v4_best_gw query: dst 1xx.160.247.60
net_route_v4_best_gw result: via 192.168.178.1 dev eth1

The expected route table needs both the VPN default split routes and a host route to the OpenVPN peer through the pre-tunnel gateway:

0.0.0.0/1 via 10.1xx.0.1 dev tun0
128.0.0.0/1 via 10.1xx.0.1 dev tun0
1xx.160.247.60 via 192.168.178.1 dev eth1

Without the peer host route, traffic to the VPN server can follow the new default route into tun0 and break the tunnel.

I also tested the suggested proto_add_ipv4_route() variant for the peer route.
It installs the def1 routes, but the peer host route is missing:

0.0.0.0/1 via 10.1xx.0.1 dev tun0 proto static
128.0.0.0/1 via 10.1xx.0.1 dev tun0 proto static

The required route is not present:

1xx.160.247.60 via 192.168.178.1 dev eth1

So a plain proto_add_ipv4_route() in the OpenVPN interface update does not reproduce the needed behavior for this route.
This PR keeps normal VPN routes managed by netifd, but installs the peer host route through the pre-tunnel gateway so the OpenVPN transport remains reachable.

this is .ovpn client config file:

client
dev tun
proto udp
remote 1xx.160.247.60 1194
resolv-retry infinite
remote-random
nobind
tun-mtu 1500
tun-mtu-extra 32
mssfix 1450
persist-key
persist-tun
ping 15
ping-restart 0
ping-timer-rem
reneg-sec 0
comp-lzo no
verify-x509-name CN=servnumber.nordvpn.com

remote-cert-tls server

auth-user-pass user_pass_nord
verb 3
pull
fast-io
cipher AES-256-CBC
auth SHA512
<ca>
....

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 21, 2026

ok, i get the point.
for trusted_ip, use ip route add
for push route, use proto_add_ipv4_route

@pesa1234
Copy link
Copy Markdown
Contributor Author

Thanks for your time on check this pull request.
I added comment on source.

The pushed VPN routes are still handled through netifd with proto_add_ipv4_route()/proto_add_ipv6_route(). The direct ip route handling is only used for the peer host route to trusted_ip/trusted_ip6 via the pre-tunnel gateway, because that route must stay outside the OpenVPN interface itself.

I used ip route replace instead of ip route add because it can run from both up and route-up/reconnect paths, and replace avoids failing on an already existing peer route while keeping the same final route.
If you prefer strict add semantics, I can change it and add an explicit existing-route check.

Thanks again!

Andrea

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 21, 2026

The route's next hop for trusted_ip is not necessarily the route_net_gateway. For example, trusted_ip could be directly routed to a specific router's next hop, or in scenarios with multiple gateways (with different metrics), its next hop might be a gateway other than the route_net_gateway.

To correctly determine the next hop for a trusted_ip, you will likely need to use a command like ip route get $trusted_ip.

ip route get 100.64.0.2
100.64.0.2 via 100.64.0.1 dev pppoe-wan src 100.64.230.231 uid 0 
    cache 

ip route get 192.168.16.224
192.168.16.224 dev br-lan src 192.168.16.1 uid 0 
    cache

@pesa1234
Copy link
Copy Markdown
Contributor Author

pesa1234 commented Apr 21, 2026

The route's next hop for trusted_ip is not necessarily the route_net_gateway. For example, trusted_ip could be directly routed to a specific router's next hop, or in scenarios with multiple gateways (with different metrics), its next hop might be a gateway other than the route_net_gateway.

To correctly determine the next hop for a trusted_ip, you will likely need to use a command like ip route get $trusted_ip.

ip route get 100.64.0.2
100.64.0.2 via 100.64.0.1 dev pppoe-wan src 100.64.230.231 uid 0 
    cache 

ip route get 192.168.16.224
192.168.16.224 dev br-lan src 192.168.16.1 uid 0 
    cache

Thanks, you were right.

I updated the peer host route handling to resolve the actual path to trusted_ip/trusted_ip6 with ip route get instead of assuming route_net_gateway / route_ipv6_gateway is the correct next hop.

route_net_gateway was removed. The script now installs the peer route only from the resolved route-get result, supporting both:

  • via <gateway> dev <iface>
  • directly connected dev <iface>

I tested it with a redirect-gateway def1 setup and now get:

0.0.0.0/1 via 10.100.0.1 dev tun0
128.0.0.0/1 via 10.100.0.1 dev tun0
193.160.247.60 via 192.168.178.1 dev eth1

So the pushed default routes go through the VPN, while the OpenVPN peer stays reachable through the pre-tunnel path.

Thanks again!

Andrea

eval "net=\$route_network_$i mask=\$route_netmask_$i gw=\$route_gateway_$i"
[ -z "$net" ] && break
[ -z "$mask" ] && continue
if [ "$net" = "0.0.0.0" ] && [ "$mask" = "0.0.0.0" ]; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When $net equals "0.0.0.0", just execute proto_add_ipv4_route "0.0.0.0" 0 "$gateway". Please avoid splitting the default route into 0.0.0.0/1 and 128.0.0.0/1. It is better to let netifd manage the default routing. This ensures that if option defaultroute 0 is configured, netifd will properly ignore the 0.0.0.0 default route.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and we don't need to check redirect_gateway_ipv4_enabled, openvpn should pass the default route info to hotplug script if redirect_gateway_ipv4_enabled

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review.

I think we should keep the current def1 handling here.

When OpenVPN receives redirect-gateway def1, it normally installs two more
specific routes:

0.0.0.0/1
128.0.0.0/1

This is different from replacing the real default route with 0.0.0.0/0.
The original WAN default route stays in the routing table, while normal
traffic still prefers the VPN because the /1 routes are more specific.

This also keeps useful behaviour such as:

ping -I eth1 www.google.com

this is useful for example to understand if a website can be reach by vpn or not.

With a real 0.0.0.0/0 route via the VPN, forcing traffic through the
physical interface may no longer work as expected.

So for redirect-gateway def1 I think it is better to preserve OpenVPN's
native behaviour and let the hotplug script add the same /1 routes that
OpenVPN would have added without route-noexec.

For an explicit pushed 0.0.0.0/0 route, using proto_add_ipv4_route
"0.0.0.0" 0 "$gateway" makes sense. But for def1, keeping the split
default routes is intentional.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

and we don't need to check redirect_gateway_ipv4_enabled, openvpn should pass the default route info to hotplug script if redirect_gateway_ipv4_enabled

This I fixed


if [ -n "$gateway" ]; then
if ip "-$family" route replace "$target/$prefix" via "$gateway" >/dev/null 2>&1; then
write_server_route_state "$state_file" "$target" "$gateway" "$gateway_dev"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we may write more data to

#sample
proto_add_data
    json_add_string "target" "$target"
    json_add_string "gateway" "$gateway"
    json_add_string "gateway_dev" "$gateway_dev"
proto_close_data

do not write to pid state file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I understand your point.

Just to clarify: the patch does not write to the OpenVPN pid file. It only stores the daemon pid/start time in the route state JSON as a guard, so stale down/route-pre-down hooks from an older process do not remove a newer route.

Using proto_add_data() for target/gateway/gateway_dev sounds cleaner. My only concern is that the same state is also needed later during down, route-pre-down and proto teardown cleanup.

Do you suggest reading it back from ifstatus data during cleanup, or is there a better netifd pattern for this?

The pid/start time are written here only in the route state JSON:
openvpn-hotplug lines 210-211

They are used only as an ownership guard before cleanup:
openvpn-hotplug line 438

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 21, 2026

A more accurate approach: when option defaultroute 0 is configured, adding a route for trusted_ip becomes unnecessary.

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 21, 2026

and the AI review:

Problem Description: Route Clobbering and Cross-Interface Limitations for trusted_ip
When establishing a VPN connection, the script must add a direct host route for the VPN server's IP (trusted_ip) via the underlying WAN interface to prevent traffic from looping back into the VPN tunnel. However, this introduces two main issues:

Cross-Interface Limitation: We cannot use netifd's native proto_add_ipv4_route function. netifd strictly binds these routes to the current logical VPN interface. Since the gateway for the trusted_ip resides on a different physical device (e.g., pppoe-wan), we are forced to use manual ip route commands.

Route Destruction on Teardown: A valid route to the trusted_ip might already exist natively in the system's routing table. If the VPN script simply executes ip route del $trusted_ip when the interface goes down, it acts too broadly and will accidentally destroy the system's pre-existing, legitimate route.

Solution Approach: Route Isolation via User-Defined Metrics
To resolve this safely and elegantly, we can implement Metric Isolation. By explicitly assigning a custom metric to the injected route, we can separate our temporary VPN route from the system's native routes.

The workflow is as follows:

Configuration: Allow the user to define a specific metric value in their UCI configuration.

Setup Phase (ifup): Manually inject the route with the user-defined metric using the ip command.
(Example: ip route add $trusted_ip/32 via $gateway dev $gateway_dev metric $user_metric)

Teardown Phase (ifdown): Perform an exact match deletion by specifying the same metric.
(Example: ip route del $trusted_ip/32 metric $user_metric)

Why this works: The Linux kernel allows multiple routes to the same destination as long as their metrics differ. By explicitly including the metric parameter during deletion, the kernel will only remove the exact route created by our script. Any pre-existing system routes (which typically have a different metric, such as 0) will remain completely untouched and protected.

@pesa1234 pesa1234 force-pushed the fix-ovpn-route branch 3 times, most recently from 1ff022d to 5817fcc Compare April 22, 2026 09:36
@pesa1234
Copy link
Copy Markdown
Contributor Author

A more accurate approach: when option defaultroute 0 is configured, adding a route for trusted_ip becomes unnecessary.

Correct, thanks. Done here: https://github.com/openwrt/packages/compare/6e27a52c61f0b8c2ab0d8046bb1647f766bfb700..1ff022d29248193029809bffc2b3627b58f64c09

@pesa1234
Copy link
Copy Markdown
Contributor Author

Thanks, I addressed the route clobbering concern too.

The peer host route is now added with ip route add instead of replace.
If an exact host route already exists, the hotplug script leaves it untouched and does not write route state for it. This way teardown only removes routes that were actually created by this hotplug run, and it does not delete pre-existing system/user routes.

json_add_string daemon_start_time "$(server_route_start_time)"
json_add_string daemon_pid "$(server_route_pid)"
json_dump
) > "$state_file"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

write data to proto_add_data instead of writing to $state_file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe

. /usr/share/libubox/jshn.sh

JSON_DATA=$(ubus call network.interface.wan status)

json_load "$JSON_DATA"
...

@pesa1234 pesa1234 force-pushed the fix-ovpn-route branch 2 times, most recently from d8b0cfe to 0d59ac4 Compare April 22, 2026 13:39
@pesa1234
Copy link
Copy Markdown
Contributor Author

pesa1234 commented Apr 22, 2026

@ptpt52 Thanks, I updated the patch.

With option defaultroute 0, the hotplug script now skips pushed
default/full-tunnel routes before adding them to netifd. This includes
0.0.0.0/0, redirect-gateway def1 /1 routes, and the IPv6 default-like
routes.

In that case it also skips the trusted_ip host route, because the VPN
default route is not installed and the peer should remain reachable via
the normal WAN routing.

Non-default pushed routes are still added as before.

Does this match what you had in mind?

local client tls_client tls_server
local tls_crypt_v2_verify mode learn_address client_connect
local client_crresponse client_disconnect auth_user_pass_verify
local defaultroute
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should add defaultroute to PROTO_BOOLS='

[ -z "$net" ] && break
[ -z "$mask" ] && continue

if ! default_route_enabled && ipv4_default_route "$net" "$mask"; then
Copy link
Copy Markdown
Contributor

@ptpt52 ptpt52 Apr 22, 2026

Choose a reason for hiding this comment

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

no need to check if default_route_enabled or not, just push all route to proto_add_ipv4_route and netifd would handle it if defaultroute is 0 or 1.

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 22, 2026

I’d like to clarify the logic here more clearly.

The OpenVPN server may push a set of routes, including either:

a standard default route 0.0.0.0/0, or
split default routes such as 128.0.0.0/1 and 0.0.0.0/1.

For the split default route form, we should convert it into a normal 0.0.0.0/0 default route, then pass it to proto_add_ipv4_route.

All routes can then be handed over to netifd for processing.

netifd can properly enforce the defaultroute option. If defaultroute=0, the routes that are actually applied by netifd will not include any default route.

case "$script_type" in
up)
install_needed_ipv4_server_route
[ "$IPV6" = "1" ] && install_needed_ipv6_server_route
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In fact, the IPv6 option determines whether the IPv6 protocol stack is enabled on the TUN interface — that is, whether IPv6 addresses are obtained and configured on the TUN device. It does not affect OpenVPN itself establishing the transport/data link over IPv6. Therefore, the IPv6 option should not be checked here.

@pesa1234 pesa1234 force-pushed the fix-ovpn-route branch 2 times, most recently from ff10b54 to 1f256e5 Compare April 22, 2026 18:45
@pesa1234
Copy link
Copy Markdown
Contributor Author

Thanks, I tested both behaviours on a real OpenWrt router with a VPN server pushing redirect-gateway def1.

I first tried converting redirect-gateway def1 to a single 0.0.0.0/0 route, as suggested. It made netifd install:

default via 10.100.0.1 dev tun0
193.160.247.60 via 192.168.178.1 dev eth1

The tunnel worked, but the original WAN default route disappeared from the main table, and this broke traffic explicitly bound to the WAN device:

ping -I eth1 -c 3 google.com
3 packets transmitted, 0 packets received, 100% packet loss

Keeping redirect-gateway def1 as OpenVPN normally applies it preserves the WAN default route while still preferring the VPN for normal traffic:

0.0.0.0/1 via 10.100.0.1 dev tun0
default via 192.168.178.1 dev eth1
128.0.0.0/1 via 10.100.0.1 dev tun0
193.160.247.60 via 192.168.178.1 dev eth1

With this, the same WAN-bound test works:

ping -I eth1 -c 3 google.com
3 packets transmitted, 3 packets received, 0% packet loss

So I kept the def1 split routes intentionally. This preserves the existing OpenVPN behaviour and avoids breaking WAN-bound commands, while the peer host route still keeps the VPN server reachable via the pre-tunnel path.

I also kept the other changes from your review:

  • defaultroute is now part of PROTO_BOOLS
  • the trusted_ip route is skipped when defaultroute=0
  • the IPv6 peer route is not gated by the TUN IPv6 option anymore

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 22, 2026

$route_vpn_gateway is the gateway IP whenever either redirect-gateway def1 or any route xxxx option is set.

The hotplug script should always check $route_vpn_gateway and configure the default route accordingly.

If the server pushes, or the client configures, routes in the form of 0.0.0.0/1 + 128.0.0.0/1, they should be merged into a single default route 0.0.0.0/0.

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 22, 2026

This should not be true. I just test it.

I first tried converting redirect-gateway def1 to a single 0.0.0.0/0 route, as suggested. It made netifd install:
default via 10.100.0.1 dev tun0
193.160.247.60 via 192.168.178.1 dev eth1

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 22, 2026

I suggest that, to better integrate with netifd network configuration, default route handling should be fully controlled by the defaultroute option.

The OpenVPN hotplug script should always install a default route 0.0.0.0/0, regardless of whether the client configuration includes redirect-gateway def1 or route, or whether the server pushes redirect-gateway def1 or route.

To achieve this, when OpenVPN starts, I would forcibly add the redirect-gateway def1 option so that the hotplug script can always obtain $route_vpn_gateway, and then consistently submit the default route 0.0.0.0/0 for netifd to manage.

@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 23, 2026

diff --git a/net/openvpn/files/lib/netifd/proto/openvpn.sh b/net/openvpn/files/lib/netifd/proto/openvpn.sh
index 2cf5dd718e..669178d762 100755
--- a/net/openvpn/files/lib/netifd/proto/openvpn.sh
+++ b/net/openvpn/files/lib/netifd/proto/openvpn.sh
@@ -92,6 +92,7 @@ option_builder() {
 # Not real config params used by openvpn - only by our proto handler
 PROTO_BOOLS='
 allow_deprecated
+defaultroute
 ipv6
 '
 
@@ -161,72 +162,30 @@ proto_openvpn_setup() {
 	json_get_vars auth_user_pass askpass username password cert_password
 
 	mkdir -p /var/run
-	# combine into --askpass:
-	if [ -n "$cert_password" ]; then
-		cp_file="/var/run/openvpn.$config.pass"
-		umask 077
-		printf '%s\n' "${cert_password:-}" > "$cp_file"
-		umask 022
-		append exec_params "--askpass $cp_file"
-	elif [ -n "$askpass" ]; then
-		append exec_params "--askpass $askpass"
-	fi
-
-	# combine into --auth-user-pass:
-	if [ -n "$username" ] || [ -n "$password" ]; then
-		auth_file="/var/run/openvpn.$config.auth"
-		umask 077
-		printf '%s\n' "${username:-}" "${password:-}" > "$auth_file"
-		umask 022
-		append exec_params "--auth-user-pass $auth_file"
-	elif [ -n "$auth_user_pass" ]; then
-		append exec_params "--auth-user-pass $auth_user_pass"
-	fi
 
 	# Testing option
 	# ${tls_exit:+--tls-exit} \
 
 	# Check 'script_security' option
 	json_get_var script_security script_security
-	[ -z "$script_security" ] && script_security=3
+	[ -z "$script_security" ] && script_security=2
 
-	# Add default hotplug handling if 'script_security' option is equal '3'
-	if [ "$script_security" -eq '3' ]; then
-		local ipv6
+	# Add default hotplug handling if 'script_security' option is ge '3'
+	if [ "$script_security" -ge '2' ]; then
 		local up down route_up route_pre_down
 		local client tls_client tls_server
 		local tls_crypt_v2_verify mode learn_address client_connect
 		local client_crresponse client_disconnect auth_user_pass_verify
 
-		logger -t "openvpn(proto)" \
-			-p daemon.info "Enabled default hotplug processing, as the openvpn configuration 'script_security' is '3'"
-
-		append exec_params "--setenv INTERFACE $config"
-		append exec_params "--script-security 3"
-
 		json_get_vars up down route_up route_pre_down
 		json_get_vars tls_crypt_v2_verify mode learn_address client_connect
 		json_get_vars client_crresponse client_disconnect auth_user_pass_verify
 
-		json_get_vars ipv6
-		#default ipv6 is enabled
-		[ -n "$ipv6" ] || ipv6=1
-		append exec_params "--setenv IPV6 '$ipv6'"
-
 		json_get_vars ifconfig_noexec route_noexec
-		[ -z "$ifconfig_noexec" ] && append exec_params "--ifconfig-noexec"
-		[ -z "$route_noexec" ] && append exec_params "--route-noexec"
 
-		append exec_params "--up '/usr/libexec/openvpn-hotplug'"
 		[ -n "$up" ] && append exec_params "--setenv user_up '$up'"
-
-		append exec_params "--down '/usr/libexec/openvpn-hotplug'"
 		[ -n "$down" ] && append exec_params "--setenv user_down '$down'"
-
-		append exec_params "--route-up '/usr/libexec/openvpn-hotplug'"
 		[ -n "$route_up" ] && append exec_params "--setenv user_route_up '$route_up'"
-
-		append exec_params "--route-pre-down '/usr/libexec/openvpn-hotplug'"
 		[ -n "$route_pre_down" ] && append exec_params "--setenv user_route_pre_down '$route_pre_down'"
 
 		append exec_params "--tls-crypt-v2-verify '/usr/libexec/openvpn-hotplug'"
@@ -260,16 +219,66 @@ proto_openvpn_setup() {
 			json_get_var tls_verify tls_verify
 			[ -n "$tls_verify" ] && append exec_params "--setenv user_tls_verify '$tls_verify'"
 		fi
-	else
-		logger -t "openvpn(proto)" \
-			-p daemon.warn "Default hotplug processing disabled, as the openvpn configuration 'script_security' is less than '3'"
 	fi
 
 	eval "set -- $exec_params"
 	umask 077
 	printf "%b\n" "${exec_params//--/\\n}" > "$conf_file"
 	umask 022
-	proto_run_command "$config" openvpn --config "$conf_file"
+
+	is_openvpn_client() {
+		grep -qE '^[[:space:]]*remote[[:space:]]+' "$conf_file" "$config_file" && return 0
+	}
+
+	local ipv6 defaultroute
+	exec_params=
+
+	# combine into --askpass:
+	if [ -n "$cert_password" ]; then
+		cp_file="/var/run/openvpn.$config.pass"
+		umask 077
+		printf '%s\n' "${cert_password:-}" > "$cp_file"
+		umask 022
+		append exec_params "--askpass $cp_file"
+	elif [ -n "$askpass" ]; then
+		append exec_params "--askpass $askpass"
+	fi
+
+	# combine into --auth-user-pass:
+	if [ -n "$username" ] || [ -n "$password" ]; then
+		auth_file="/var/run/openvpn.$config.auth"
+		umask 077
+		printf '%s\n' "${username:-}" "${password:-}" > "$auth_file"
+		umask 022
+		append exec_params "--auth-user-pass $auth_file"
+	elif [ -n "$auth_user_pass" ]; then
+		append exec_params "--auth-user-pass $auth_user_pass"
+	fi
+
+	#Always Override Options
+	append exec_params "--setenv INTERFACE $config"
+	append exec_params "--script-security 3"
+	append exec_params "--ifconfig-noexec"
+	append exec_params "--route-noexec"
+	append exec_params "--up '/usr/libexec/openvpn-hotplug'"
+	append exec_params "--down '/usr/libexec/openvpn-hotplug'"
+	append exec_params "--route-up '/usr/libexec/openvpn-hotplug'"
+	append exec_params "--route-pre-down '/usr/libexec/openvpn-hotplug'"
+
+	json_get_vars ipv6 defaultroute
+	#default ipv6 is enabled
+	[ -n "$ipv6" ] || ipv6=1
+	append exec_params "--setenv IPV6 '$ipv6'"
+
+	if is_openvpn_client then
+		append exec_params "--redirect-gateway"
+		[ -n "$defaultroute" ] || defaultroute=1
+	else
+		defaultroute=0
+	fi
+	append exec_params "--setenv DEFAULTROUTE '$defaultroute'"
+
+	proto_run_command "$config" openvpn $exec_params --config "$conf_file"
 
 	# last param wins; user provided status or syslog supersedes.
 }

@pesa1234
Copy link
Copy Markdown
Contributor Author

Thanks, I think I understand your point better now.

Maybe the cleanest direction is not to force redirect-gateway from the proto wrapper, but to step back a bit and make the OpenVPN/netifd boundary more explicit.

Before the netifd proto handler, OpenVPN did not force ifconfig-noexec or route-noexec by default. OpenVPN handled addresses and routes itself, and the OpenWrt scripts mostly handled hotplug/user hooks.

With the current netifd integration, these options are effectively used as the switch that moves address/route handling from OpenVPN to netifd.

So instead of forcing redirect-gateway globally, I think a better follow-up could be:

  • do not force ifconfig-noexec / route-noexec unconditionally
  • keep them configurable through UCI
  • let OpenVPN handle addresses/routes when noexec is disabled
  • let netifd/hotplug handle addresses/routes only when noexec is enabled
  • keep redirect-gateway behaviour as reported by OpenVPN, without changing split tunnel configs into full tunnel configs

That is a larger behavioural change than this PR currently does, so I think it may be better if I mark this PR as draft for now and rework it around that boundary more carefully.

Thanks again for pushing on this, I think this helped clarify the right shape of the fix.

@pesa1234 pesa1234 marked this pull request as draft April 23, 2026 07:36
@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 23, 2026

@pesa1234
Forcing ifconfig-noexec and route-noexec is required. OpenVPN must not handle interface IP configuration or routing setup—these responsibilities should be left to netifd. This is exactly the goal of integrating it as a netifd proto handler.

@pesa1234
Copy link
Copy Markdown
Contributor Author

Forcing ifconfig-noexec and route-noexec is required. OpenVPN must not handle interface IP configuration or routing setup—these responsibilities should be left to netifd. This is exactly the goal of integrating it as a netifd proto handler.

I agree with the general goal that netifd should own interface state for a proto handler.

However, I do not think forcing both --ifconfig-noexec and --route-noexec is behaviorally equivalent yet. In my tests, forcing --ifconfig-noexec changed the existing OpenVPN client behavior: the tunnel came up, but traffic through the WAN path did not behave correctly anymore, including simple checks such as pinging through the WAN interface.

So my concern is not about keeping OpenVPN in charge forever, but about avoiding a regression while moving this to netifd. The pre-netifd behavior allowed OpenVPN to configure the tunnel address itself, and that path worked correctly with common client configs.

For this reason I would prefer to keep --route-noexec enabled by default, but not force --ifconfig-noexec until the netifd path fully reproduces the previous behavior.

Thanks again.

Andrea

@pesa1234 pesa1234 force-pushed the fix-ovpn-route branch 2 times, most recently from a529aeb to 1f73554 Compare April 23, 2026 09:03
@ptpt52
Copy link
Copy Markdown
Contributor

ptpt52 commented Apr 24, 2026

related: #29220

Install the OpenVPN hotplug helper as the generated up/down/route
script and pass user configured hook paths through user_* environment
variables. This avoids emitting duplicate hook options in the generated
OpenVPN configuration, which otherwise causes OpenVPN to override the
helper.

Default script-security to 2, matching the pre-netifd init script and
the minimum level OpenVPN requires for external scripts. This keeps
common client configurations on the default hotplug path without
requiring script-security 3.

Only add auth-user-pass-verify when a user verifier is configured.

Signed-off-by: Andrea Pesaresi <andreapesaresi82@gmail.com>
Keep --route-noexec enabled by default in the netifd proto path and let the hotplug helper publish imported routes to netifd. Leave --ifconfig-noexec configurable instead of forcing it, so existing client configurations keep native OpenVPN tunnel address setup while the helper mirrors that address into netifd state.

Use OpenVPN route_* variables for pushed IPv4 routes and synthesize redirect-gateway def1 split routes when OpenVPN exposes only the redirect flag. Preserve def1 as 0.0.0.0/1 and 128.0.0.0/1 instead of collapsing it to 0.0.0.0/0, keeping the original WAN default route in the main table.

When a VPN default route is installed, add a host route to the VPN peer through the pre-tunnel path resolved by ip route get. Store this route in netifd protocol data so down, route-pre-down and teardown remove only the route owned by the current daemon generation, and leave pre-existing peer routes untouched.

Expose daemon_pid and ifname in protocol data alongside the saved server route state. Following ptpt52's MTU fix, apply OpenVPN's exported tun_mtu with ip link set in up and route-up instead of only publishing mtu in the update data.

Expose the proto defaultroute option to hotplug. When disabled, skip default-like routes and peer host route handling. Bump PKG_RELEASE for the behavior change.

Tested on OpenWrt with a NordVPN client pushing redirect-gateway def1: 0.0.0.0/1 and 128.0.0.0/1 were installed via tun0, the WAN default route remained via eth1, the VPN peer host route stayed via eth1, and ping -I eth1 google.com completed with 0% packet loss.

Checked with sh -n on the shell scripts and git diff --check.

Suggested-by: Chen Minqiang <ptpt52@gmail.com>

Signed-off-by: Andrea Pesaresi <andreapesaresi82@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants