o->push_continuation = 0;
o->push_option_types_found = 0;
+ o->push_update_options_found = 0;
o->imported_protocol_flags = 0;
}
update_option(struct context *c, struct options *options, char *p[], bool is_inline,
const char *file, int line, const int level, const msglvl_t msglevel,
const unsigned int permission_mask, unsigned int *option_types_found,
- struct env_set *es, unsigned int *update_options_found)
+ struct env_set *es)
{
const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);
ASSERT(MAX_PARMS >= 7);
if (streq(p[0], "route") && p[1] && !p[5])
{
- if (!(*update_options_found & OPT_P_U_ROUTE))
+ if (!(options->push_update_options_found & OPT_P_U_ROUTE))
{
VERIFY_PERMISSION(OPT_P_ROUTE);
if (!check_route_option(options, p, msglevel, pull_mode))
es, &c->net_ctx);
RESET_OPTION_ROUTES(options->routes, routes);
}
- *update_options_found |= OPT_P_U_ROUTE;
+ options->push_update_options_found |= OPT_P_U_ROUTE;
}
}
else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
{
- if (!(*update_options_found & OPT_P_U_ROUTE6))
+ if (!(options->push_update_options_found & OPT_P_U_ROUTE6))
{
VERIFY_PERMISSION(OPT_P_ROUTE);
if (!check_route6_option(options, p, msglevel, pull_mode))
ROUTE_OPTION_FLAGS(&c->options), es, &c->net_ctx);
RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
}
- *update_options_found |= OPT_P_U_ROUTE6;
+ options->push_update_options_found |= OPT_P_U_ROUTE6;
}
}
else if (streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private"))
{
- if (!(*update_options_found & OPT_P_U_REDIR_GATEWAY))
+ if (!(options->push_update_options_found & OPT_P_U_REDIR_GATEWAY))
{
VERIFY_PERMISSION(OPT_P_ROUTE);
if (options->routes)
}
env_set_del(es, "route_redirect_gateway_ipv4");
env_set_del(es, "route_redirect_gateway_ipv6");
- *update_options_found |= OPT_P_U_REDIR_GATEWAY;
+ options->push_update_options_found |= OPT_P_U_REDIR_GATEWAY;
}
}
else if (streq(p[0], "dns") && p[1])
{
- if (!(*update_options_found & OPT_P_U_DNS))
+ if (!(options->push_update_options_found & OPT_P_U_DNS))
{
VERIFY_PERMISSION(OPT_P_DHCPDNS);
if (!check_dns_option(options, p, msglevel, pull_mode))
}
gc_free(&options->dns_options.gc);
CLEAR(options->dns_options);
- *update_options_found |= OPT_P_U_DNS;
+ options->push_update_options_found |= OPT_P_U_DNS;
}
}
#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
{
- if (!(*update_options_found & OPT_P_U_DHCP))
+ if (!(options->push_update_options_found & OPT_P_U_DHCP))
{
struct tuntap_options *o = &options->tuntap_options;
VERIFY_PERMISSION(OPT_P_DHCPDNS);
o->http_proxy_port = 0;
o->http_proxy = NULL;
#endif
- *update_options_found |= OPT_P_U_DHCP;
+ options->push_update_options_found |= OPT_P_U_DHCP;
}
}
#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
{
- if (!(*update_options_found & OPT_P_U_DHCP))
+ if (!(options->push_update_options_found & OPT_P_U_DHCP))
{
VERIFY_PERMISSION(OPT_P_DHCPDNS);
delete_all_dhcp_fo(options, &es->list);
- *update_options_found |= OPT_P_U_DHCP;
+ options->push_update_options_found |= OPT_P_U_DHCP;
}
}
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
bool pull; /* client pull of config options from server */
int push_continuation;
unsigned int push_option_types_found;
+ unsigned int push_update_options_found; /* tracks which option types have been reset in current PUSH_UPDATE sequence */
const char *auth_user_pass_file;
bool auth_user_pass_file_inline;
struct options_pre_connect *pre_connect;
* @param option_types_found A pointer to the variable where the flags corresponding to the options
* found are stored.
* @param es The environment set structure.
- * @param update_options_found A pointer to the variable where the flags corresponding to the update
- * options found are stored, used to check if an option of the same type has already been processed
- * by update_option() within the same push-update message.
*/
void update_option(struct context *c, struct options *options, char *p[], bool is_inline,
const char *file, int line, const int level, const msglvl_t msglevel,
const unsigned int permission_mask, unsigned int *option_types_found,
- struct env_set *es, unsigned int *update_options_found);
+ struct env_set *es);
void parse_argv(struct options *options, const int argc, char *argv[], const msglvl_t msglevel,
const unsigned int permission_mask, unsigned int *option_types_found,
int line_num = 0;
const char *file = "[PUSH-OPTIONS]";
const msglvl_t msglevel = D_PUSH_ERRORS | M_OPTERR;
- unsigned int update_options_found = 0;
while (buf_parse(buf, ',', line, sizeof(line)))
{
}
else
{
+ /*
+ * Use persistent push_update_options_found from options struct to track
+ * which option types have been reset across continuation messages.
+ * This prevents routes from being reset on each continuation message.
+ */
update_option(c, options, p, false, file, line_num, 0, msglevel, permission_mask,
- option_types_found, es, &update_options_found);
+ option_types_found, es);
}
}
}
}
else
{
- if (!option_types_found)
+ /* Clear push_update_options_found for next PUSH_UPDATE sequence */
+ c->options.push_update_options_found = 0;
+
+ /* Use accumulated option_types_found for the entire PUSH_UPDATE sequence */
+ if (!c->options.push_option_types_found)
{
msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message");
}
- else if (!do_update(c, option_types_found))
+ else if (!do_update(c, c->options.push_option_types_found))
{
msg(D_PUSH_ERRORS, "Failed to update options");
goto error;
update_option(struct context *c, struct options *options, char *p[], bool is_inline,
const char *file, int line, const int level, const msglvl_t msglevel,
const unsigned int permission_mask, unsigned int *option_types_found,
- struct env_set *es, unsigned int *update_options_found)
+ struct env_set *es)
{
}
return true;
}
+/*
+ * Counters to track route accumulation across continuation messages.
+ * Used to verify the bug where update_options_found resets per message.
+ */
+static int route_reset_count = 0;
+static int route_add_count = 0;
+
+static void
+reset_route_counters(void)
+{
+ route_reset_count = 0;
+ route_add_count = 0;
+}
+
bool
apply_push_options(struct context *c, struct options *options, struct buffer *buf,
unsigned int permission_mask, unsigned int *option_types_found,
{
char line[OPTION_PARM_SIZE];
+ /*
+ * Use persistent push_update_options_found from options struct to track
+ * which option types have been reset across continuation messages.
+ * This is the FIXED behavior - routes are only reset once per PUSH_UPDATE sequence.
+ */
+
while (buf_parse(buf, ',', line, sizeof(line)))
{
unsigned int push_update_option_flags = 0;
return false; /* Cause push/pull error and stop push processing */
}
}
- /*
- * No need to test also the application part here
- * (add_option/remove_option/update_option)
- */
+
+ /* Simulate route handling from update_option() in options.c */
+ if (strncmp(&line[i], "route ", 6) == 0)
+ {
+ if (!(options->push_update_options_found & OPT_P_U_ROUTE))
+ {
+ /* First route in entire PUSH_UPDATE sequence - reset routes once */
+ route_reset_count++;
+ options->push_update_options_found |= OPT_P_U_ROUTE;
+ }
+ route_add_count++;
+ }
+ /* Simulate add_option() push-continuation logic */
+ else if (!strcmp(&line[i], "push-continuation 2"))
+ {
+ options->push_continuation = 2;
+ }
+ else if (!strcmp(&line[i], "push-continuation 1"))
+ {
+ options->push_continuation = 1;
+ }
}
return true;
}
free_buf(&buf);
}
+/**
+ * Test that routes accumulate correctly across multiple continuation messages.
+ * This test exposes a bug where update_options_found is reset to 0 for each
+ * continuation message, causing routes to be reset on each message instead
+ * of accumulating.
+ *
+ * Expected behavior: routes should only be reset ONCE (when the first route is received),
+ * then all subsequent routes should accumulate.
+ *
+ * Current bug: routes are reset on the first route of EACH continuation message.
+ */
+static void
+test_incoming_push_continuation_route_accumulation(void **state)
+{
+ struct context *c = *state;
+ unsigned int option_types_found = 0;
+
+ reset_route_counters();
+
+ /* Message 1: first batch of routes, continuation 2 (more coming) */
+ struct buffer buf1 = alloc_buf(512);
+ const char *msg1 = "PUSH_UPDATE, route 10.1.0.0 255.255.0.0, route 10.2.0.0 255.255.0.0, route 10.3.0.0 255.255.0.0,push-continuation 2";
+ buf_write(&buf1, msg1, strlen(msg1));
+
+ assert_int_equal(process_incoming_push_msg(c, &buf1, c->options.pull, pull_permission_mask(c),
+ &option_types_found),
+ PUSH_MSG_CONTINUATION);
+ free_buf(&buf1);
+
+ /* Message 2: more routes, continuation 2 (more coming) */
+ struct buffer buf2 = alloc_buf(512);
+ const char *msg2 = "PUSH_UPDATE, route 10.4.0.0 255.255.0.0, route 10.5.0.0 255.255.0.0, route 10.6.0.0 255.255.0.0,push-continuation 2";
+ buf_write(&buf2, msg2, strlen(msg2));
+
+ assert_int_equal(process_incoming_push_msg(c, &buf2, c->options.pull, pull_permission_mask(c),
+ &option_types_found),
+ PUSH_MSG_CONTINUATION);
+ free_buf(&buf2);
+
+ /* Message 3: final batch of routes, continuation 1 (last message) */
+ struct buffer buf3 = alloc_buf(512);
+ const char *msg3 = "PUSH_UPDATE, route 10.7.0.0 255.255.0.0, route 10.8.0.0 255.255.0.0, route 10.9.0.0 255.255.0.0,push-continuation 1";
+ buf_write(&buf3, msg3, strlen(msg3));
+
+ assert_int_equal(process_incoming_push_msg(c, &buf3, c->options.pull, pull_permission_mask(c),
+ &option_types_found),
+ PUSH_MSG_UPDATE);
+ free_buf(&buf3);
+
+ /* Verify: all 9 routes should have been added */
+ assert_int_equal(route_add_count, 9);
+
+ /*
+ * Verify: route option is reset only one time in the first message
+ * if a push-continuation is present.
+ */
+ assert_int_equal(route_reset_count, 1);
+}
+
#ifdef ENABLE_MANAGEMENT
char *r0[] = {
"PUSH_UPDATE,redirect-gateway local,route 192.168.1.0 255.255.255.0",
cmocka_unit_test_setup_teardown(test_incoming_push_message_bad_format, setup, teardown),
cmocka_unit_test_setup_teardown(test_incoming_push_message_mix, setup, teardown),
cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown),
+ cmocka_unit_test_setup_teardown(test_incoming_push_continuation_route_accumulation, setup,
+ teardown),
#ifdef ENABLE_MANAGEMENT
cmocka_unit_test_setup_teardown(test_send_push_msg0, setup2, teardown2),
projects / thirdparty / openvpn.git / commitdiff
