From: Selva Nair <selva.n...@gmail.com> Makes it possible to report management state as CONNECTED,ROUTE_ERROR instead of CONNECTED,SUCCESS in case of routing errors.
This depends on treating "route already exists" as not an error which right now works when using netlink on Linux and IPAPI or iservice on Windows. For route set via command line there is no easy way to get this information and current behaviour is unchanged: i.e., the management state continues to be reported as CONNECTED,SUCCESS. Status notification to systemd is not affected. To test on Linux, build with netlink and use a --route option with an unreachable gateway like: "--route 192.168.122.0 255.255.255.0 1.1.1.1" Notes: On windows, if the route method is "exe", setting a route that exists *may* get logged as error and this patch will lead to a slightly misleading CONNECTED,ROUTE_ERROR state message. This is considered tolerable as no one should be using "exe" (i.e. route.exe) as the route method. Signed-off-by: Selva Nair <selva.n...@gmail.com> --- src/openvpn/forward.c | 10 +++-- src/openvpn/init.c | 42 +++++++++++++------ src/openvpn/init.h | 10 ++--- src/openvpn/route.c | 97 +++++++++++++++++-------------------------- src/openvpn/route.h | 18 +++----- 5 files changed, 85 insertions(+), 92 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index af4ed05d..d7b0a2d3 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -405,12 +405,16 @@ send_control_channel_string(struct context *c, const char *str, int msglevel) static void check_add_routes_action(struct context *c, const bool errors) { - do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, - c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + bool route_status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + + int flags = (errors ? ISC_ERRORS : 0); + flags |= (!route_status ? ISC_ROUTE_ERRORS : 0); + update_time(); event_timeout_clear(&c->c2.route_wakeup); event_timeout_clear(&c->c2.route_wakeup_expire); - initialization_sequence_completed(c, errors ? ISC_ERRORS : 0); /* client/p2p --route-delay was defined */ + initialization_sequence_completed(c, flags); /* client/p2p --route-delay was defined */ } static void diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 2e95256c..a5e7399a 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1648,6 +1648,15 @@ initialization_sequence_completed(struct context *c, const unsigned int flags) { detail = "ERROR"; } + /* Flag route error only on platforms where trivial "already exists" errors + * are filtered out. Currently this is the case on Windows or if usng netlink. + */ +#if defined(_WIN32) || defined(ENABLE_SITNL) + else if (flags & ISC_ROUTE_ERRORS) + { + detail = "ROUTE_ERROR"; + } +#endif CLEAR(local); actual = &get_link_socket_info(c)->lsa->actual; @@ -1697,7 +1706,7 @@ initialization_sequence_completed(struct context *c, const unsigned int flags) * Possibly add routes and/or call route-up script * based on options. */ -void +bool do_route(const struct options *options, struct route_list *route_list, struct route_ipv6_list *route_ipv6_list, @@ -1706,10 +1715,11 @@ do_route(const struct options *options, struct env_set *es, openvpn_net_ctx_t *ctx) { + bool ret = true; if (!options->route_noexec && ( route_list || route_ipv6_list ) ) { - add_routes(route_list, route_ipv6_list, tt, ROUTE_OPTION_FLAGS(options), - es, ctx); + ret = add_routes(route_list, route_ipv6_list, tt, ROUTE_OPTION_FLAGS(options), + es, ctx); setenv_int(es, "redirect_gateway", route_did_redirect_default_gateway(route_list)); } #ifdef ENABLE_MANAGEMENT @@ -1748,6 +1758,7 @@ do_route(const struct options *options, show_adapters(D_SHOW_NET|M_NOPREFIX); } #endif + return ret; } /* @@ -1798,10 +1809,11 @@ can_preserve_tun(struct tuntap *tt) } static bool -do_open_tun(struct context *c) +do_open_tun(struct context *c, int *error_flags) { struct gc_arena gc = gc_new(); bool ret = false; + *error_flags = 0; if (!can_preserve_tun(c->c1.tuntap)) { @@ -1868,8 +1880,9 @@ do_open_tun(struct context *c) if (route_order() == ROUTE_BEFORE_TUN) { /* Ignore route_delay, would cause ROUTE_BEFORE_TUN to be ignored */ - do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, - c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + bool status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + *error_flags |= (status ? 0 : ISC_ROUTE_ERRORS); } #ifdef TARGET_ANDROID /* Store the old fd inside the fd so open_tun can use it */ @@ -1930,8 +1943,9 @@ do_open_tun(struct context *c) /* possibly add routes */ if ((route_order() == ROUTE_AFTER_TUN) && (!c->options.route_delay_defined)) { - do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, - c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + int status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + *error_flags |= (status ? 0 : ISC_ROUTE_ERRORS); } ret = true; @@ -2227,6 +2241,7 @@ do_deferred_options_part2(struct context *c) bool do_up(struct context *c, bool pulled_options, unsigned int option_types_found) { + int error_flags = 0; if (!c->c2.do_up_ran) { reset_coarse_timers(c); @@ -2243,7 +2258,7 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) /* if --up-delay specified, open tun, do ifconfig, and run up script now */ if (c->options.up_delay || PULL_DEFINED(&c->options)) { - c->c2.did_open_tun = do_open_tun(c); + c->c2.did_open_tun = do_open_tun(c, &error_flags); update_time(); /* @@ -2272,7 +2287,7 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) else { management_sleep(1); - c->c2.did_open_tun = do_open_tun(c); + c->c2.did_open_tun = do_open_tun(c, &error_flags); update_time(); } } @@ -2345,12 +2360,12 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) } else { - initialization_sequence_completed(c, 0); /* client/p2p --route-delay undefined */ + initialization_sequence_completed(c, error_flags); /* client/p2p --route-delay undefined */ } } else if (c->options.mode == MODE_POINT_TO_POINT) { - initialization_sequence_completed(c, 0); /* client/p2p restart with --persist-tun */ + initialization_sequence_completed(c, error_flags); /* client/p2p restart with --persist-tun */ } c->c2.do_up_ran = true; @@ -4483,7 +4498,8 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f * open tun/tap device, ifconfig, run up script, etc. */ if (!(options->up_delay || PULL_DEFINED(options)) && (c->mode == CM_P2P || c->mode == CM_TOP)) { - c->c2.did_open_tun = do_open_tun(c); + int error_flags = 0; + c->c2.did_open_tun = do_open_tun(c, &error_flags); } c->c2.frame_initial = c->c2.frame; diff --git a/src/openvpn/init.h b/src/openvpn/init.h index d0fb6ea1..2315b3ca 100644 --- a/src/openvpn/init.h +++ b/src/openvpn/init.h @@ -71,12 +71,9 @@ void init_instance(struct context *c, const struct env_set *env, const unsigned */ void init_query_passwords(const struct context *c); -void do_route(const struct options *options, - struct route_list *route_list, - struct route_ipv6_list *route_ipv6_list, - const struct tuntap *tt, - const struct plugin_list *plugins, - struct env_set *es, +bool do_route(const struct options *options, struct route_list *route_list, + struct route_ipv6_list *route_ipv6_list, const struct tuntap *tt, + const struct plugin_list *plugins, struct env_set *es, openvpn_net_ctx_t *ctx); void close_instance(struct context *c); @@ -116,6 +113,7 @@ void free_context_buffers(struct context_buffers *b); #define ISC_ERRORS (1<<0) #define ISC_SERVER (1<<1) +#define ISC_ROUTE_ERRORS (1<<2) void initialization_sequence_completed(struct context *c, const unsigned int flags); #ifdef ENABLE_MANAGEMENT diff --git a/src/openvpn/route.c b/src/openvpn/route.c index b4a9d56a..d406770d 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -907,7 +907,7 @@ init_route_ipv6_list(struct route_ipv6_list *rl6, return ret; } -static void +static bool add_route3(in_addr_t network, in_addr_t netmask, in_addr_t gateway, @@ -923,7 +923,7 @@ add_route3(in_addr_t network, r.network = network; r.netmask = netmask; r.gateway = gateway; - add_route(&r, tt, flags, rgi, es, ctx); + return add_route(&r, tt, flags, rgi, es, ctx); } static void @@ -945,7 +945,7 @@ del_route3(in_addr_t network, delete_route(&r, tt, flags, rgi, es, ctx); } -static void +static bool add_bypass_routes(struct route_bypass *rb, in_addr_t gateway, const struct tuntap *tt, @@ -954,21 +954,16 @@ add_bypass_routes(struct route_bypass *rb, const struct env_set *es, openvpn_net_ctx_t *ctx) { - int i; - for (i = 0; i < rb->n_bypass; ++i) + int ret = true; + for (int i = 0; i < rb->n_bypass; ++i) { if (rb->bypass[i]) { - add_route3(rb->bypass[i], - IPV4_NETMASK_HOST, - gateway, - tt, - flags | ROUTE_REF_GW, - rgi, - es, - ctx); + ret = add_route3(rb->bypass[i], IPV4_NETMASK_HOST, gateway, tt, + flags | ROUTE_REF_GW, rgi, es, ctx) && ret; } } + return ret; } static void @@ -997,12 +992,13 @@ del_bypass_routes(struct route_bypass *rb, } } -static void +static bool redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx) { const char err[] = "NOTE: unable to redirect IPv4 default gateway --"; + bool ret = true; if (rl && rl->flags & RG_ENABLE) { @@ -1011,6 +1007,7 @@ redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, if (!(rl->spec.flags & RTSA_REMOTE_ENDPOINT) && (rl->flags & RG_REROUTE_GW)) { msg(M_WARN, "%s VPN gateway parameter (--route-gateway or --ifconfig) is missing", err); + ret = false; } /* * check if a default route is defined, unless: @@ -1021,6 +1018,7 @@ redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, && (rl->spec.flags & RTSA_REMOTE_HOST)) { msg(M_WARN, "%s Cannot read current default gateway from system", err); + ret = false; } else { @@ -1047,14 +1045,9 @@ redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, if ((rl->spec.flags & RTSA_REMOTE_HOST) && rl->spec.remote_host != IPV4_INVALID_ADDR) { - add_route3(rl->spec.remote_host, - IPV4_NETMASK_HOST, - rl->rgi.gateway.addr, - tt, - flags | ROUTE_REF_GW, - &rl->rgi, - es, - ctx); + ret = add_route3(rl->spec.remote_host, IPV4_NETMASK_HOST, + rl->rgi.gateway.addr, tt, flags | ROUTE_REF_GW, + &rl->rgi, es, ctx); rl->iflags |= RL_DID_LOCAL; } else @@ -1065,32 +1058,20 @@ redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, #endif /* ifndef TARGET_ANDROID */ /* route DHCP/DNS server traffic through original default gateway */ - add_bypass_routes(&rl->spec.bypass, rl->rgi.gateway.addr, tt, flags, - &rl->rgi, es, ctx); + ret = add_bypass_routes(&rl->spec.bypass, rl->rgi.gateway.addr, tt, flags, + &rl->rgi, es, ctx); if (rl->flags & RG_REROUTE_GW) { if (rl->flags & RG_DEF1) { /* add new default route (1st component) */ - add_route3(0x00000000, - 0x80000000, - rl->spec.remote_endpoint, - tt, - flags, - &rl->rgi, - es, - ctx); + ret = add_route3(0x00000000, 0x80000000, rl->spec.remote_endpoint, + tt, flags, &rl->rgi, es, ctx) && ret; /* add new default route (2nd component) */ - add_route3(0x80000000, - 0x80000000, - rl->spec.remote_endpoint, - tt, - flags, - &rl->rgi, - es, - ctx); + ret = add_route3(0x80000000, 0x80000000, rl->spec.remote_endpoint, + tt, flags, &rl->rgi, es, ctx) && ret; } else { @@ -1103,14 +1084,8 @@ redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, } /* add new default route */ - add_route3(0, - 0, - rl->spec.remote_endpoint, - tt, - flags, - &rl->rgi, - es, - ctx); + ret = add_route3(0, 0, rl->spec.remote_endpoint, tt, + flags, &rl->rgi, es, ctx) && ret; } } @@ -1118,6 +1093,7 @@ redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt, rl->iflags |= RL_DID_REDIRECT_DEFAULT_GATEWAY; } } + return ret; } static void @@ -1194,12 +1170,12 @@ undo_redirect_default_route_to_vpn(struct route_list *rl, } } -void +bool add_routes(struct route_list *rl, struct route_ipv6_list *rl6, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx) { - redirect_default_route_to_vpn(rl, tt, flags, es, ctx); + bool ret = redirect_default_route_to_vpn(rl, tt, flags, es, ctx); if (rl && !(rl->iflags & RL_ROUTES_ADDED) ) { struct route_ipv4 *r; @@ -1232,7 +1208,7 @@ add_routes(struct route_list *rl, struct route_ipv6_list *rl6, { delete_route(r, tt, flags, &rl->rgi, es, ctx); } - add_route(r, tt, flags, &rl->rgi, es, ctx); + ret = add_route(r, tt, flags, &rl->rgi, es, ctx) && ret; } rl->iflags |= RL_ROUTES_ADDED; } @@ -1254,10 +1230,11 @@ add_routes(struct route_list *rl, struct route_ipv6_list *rl6, { delete_route_ipv6(r, tt, flags, es, ctx); } - add_route_ipv6(r, tt, flags, es, ctx); + ret = add_route_ipv6(r, tt, flags, es, ctx) && ret; } rl6->iflags |= RL_ROUTES_ADDED; } + return ret; } void @@ -1569,7 +1546,7 @@ is_on_link(const int is_local_route, const unsigned int flags, const struct rout return rgi && (is_local_route == LR_MATCH || ((flags & ROUTE_REF_GW) && (rgi->flags & RGI_ON_LINK))); } -void +bool add_route(struct route_ipv4 *r, const struct tuntap *tt, unsigned int flags, @@ -1582,7 +1559,7 @@ add_route(struct route_ipv4 *r, if (!(r->flags & RT_DEFINED)) { - return; + return true; /* no error */ } struct argv argv = argv_new(); @@ -1635,7 +1612,7 @@ add_route(struct route_ipv4 *r, { openvpn_snprintf(out, sizeof(out), "%s %s %s", network, netmask, gateway); } - management_android_control(management, "ROUTE", out); + status = management_android_control(management, "ROUTE", out); #elif defined (_WIN32) { @@ -1845,6 +1822,8 @@ done: gc_free(&gc); /* release resources potentially allocated during route setup */ net_ctx_reset(ctx); + + return (status != 0); } @@ -1871,7 +1850,7 @@ route_ipv6_clear_host_bits( struct route_ipv6 *r6 ) } } -void +bool add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx) @@ -1882,7 +1861,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, if (!(r6->flags & RT_DEFINED) ) { - return; + return true; /* no error */ } struct argv argv = argv_new(); @@ -1972,7 +1951,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, openvpn_snprintf(out, sizeof(out), "%s/%d %s", network, r6->netbits, device); - management_android_control(management, "ROUTE6", out); + status = management_android_control(management, "ROUTE6", out); #elif defined (_WIN32) @@ -2092,6 +2071,8 @@ done: gc_free(&gc); /* release resources potentially allocated during route setup */ net_ctx_reset(ctx); + + return (status != 0); } static void diff --git a/src/openvpn/route.h b/src/openvpn/route.h index 74ecd343..1c940a9b 100644 --- a/src/openvpn/route.h +++ b/src/openvpn/route.h @@ -259,15 +259,12 @@ void copy_route_ipv6_option_list(struct route_ipv6_option_list *dest, void route_ipv6_clear_host_bits( struct route_ipv6 *r6 ); -void add_route_ipv6(struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx); +bool add_route_ipv6(struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx); void delete_route_ipv6(const struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx); -void add_route(struct route_ipv4 *r, - const struct tuntap *tt, - unsigned int flags, - const struct route_gateway_info *rgi, - const struct env_set *es, +bool add_route(struct route_ipv4 *r, const struct tuntap *tt, unsigned int flags, + const struct route_gateway_info *rgi, const struct env_set *es, openvpn_net_ctx_t *ctx); void add_route_to_option_list(struct route_option_list *l, @@ -301,12 +298,9 @@ void route_list_add_vpn_gateway(struct route_list *rl, struct env_set *es, const in_addr_t addr); -void add_routes(struct route_list *rl, - struct route_ipv6_list *rl6, - const struct tuntap *tt, - unsigned int flags, - const struct env_set *es, - openvpn_net_ctx_t *ctx); +bool add_routes(struct route_list *rl, struct route_ipv6_list *rl6, + const struct tuntap *tt, unsigned int flags, + const struct env_set *es, openvpn_net_ctx_t *ctx); void delete_routes(struct route_list *rl, struct route_ipv6_list *rl6, -- 2.34.1 _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel