From cf6c3c99e84db3d55bb9c05595e6f2e0c8fd3c47 Mon Sep 17 00:00:00 2001 From: evgeny Date: Wed, 21 Oct 2026 19:48:54 +0200 Subject: [PATCH] Reject invalid firewall configuration and preserve policy on reload failure --- src/config_parser.c | 43 +++++++++++++++++++++++++++++++++++-------- src/utun_instance.c | 11 +++++++++-- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/config_parser.c b/src/config_parser.c index 0f6ddbb0..7482359d 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -201,12 +201,16 @@ static int add_route_entry(struct CFG_ROUTE_ENTRY **list, const char *subnet_str return 0; } +/* Разрешение добавляется только после проверки всей строки; ошибочный порт не становится wildcard. */ static int parse_firewall_rule(const char *rule_str, struct global_config *global) { - if (!rule_str || !global) return -1; + if (!rule_str || !global) { DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: missing rule/config"); return -1; } char rule_copy[64]; - strncpy(rule_copy, rule_str, sizeof(rule_copy) - 1); - rule_copy[sizeof(rule_copy) - 1] = '\0'; + if (strlen(rule_str) >= sizeof(rule_copy)) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: rule is too long: %s", rule_str); + return -1; + } + strcpy(rule_copy, rule_str); config_ini_trim(rule_copy); if (strcasecmp(rule_copy, "all") == 0) { @@ -219,8 +223,15 @@ static int parse_firewall_rule(const char *rule_str, struct global_config *globa char *port_str = strchr(rule_copy, ':'); uint16_t port = 0; if (port_str) { - *port_str = '\0'; - port = (uint16_t)atoi(port_str + 1); + *port_str++ = '\0'; + char *end = NULL; + errno = 0; + unsigned long parsed = strtoul(port_str, &end, 10); + if (!isdigit((unsigned char)*port_str) || errno || *end || parsed > UINT16_MAX) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: invalid port in rule: %s", rule_str); + return -1; + } + port = (uint16_t)parsed; } // Parse IP to uint32_t (host order) @@ -234,6 +245,10 @@ static int parse_firewall_rule(const char *rule_str, struct global_config *globa // Reallocate array if needed if ((global->firewall_rule_count % INITIAL_ARRAY_CAPACITY) == 0) { + if ((uint32_t)global->firewall_rule_count > UINT32_MAX / sizeof(struct CFG_FIREWALL_RULE) - INITIAL_ARRAY_CAPACITY) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Firewall: too many rules"); + return -1; + } int new_cap = global->firewall_rule_count + INITIAL_ARRAY_CAPACITY; struct CFG_FIREWALL_RULE *new_rules = u_realloc(global->firewall_rules, new_cap * sizeof(struct CFG_FIREWALL_RULE)); @@ -247,7 +262,6 @@ static int parse_firewall_rule(const char *rule_str, struct global_config *globa struct CFG_FIREWALL_RULE *rule = &global->firewall_rules[global->firewall_rule_count]; rule->ip = ip; rule->port = port; - rule->bypass = 0; global->firewall_rule_count++; DEBUG_DEBUG(DEBUG_CATEGORY_CONFIG, "Added firewall rule: %s:%d", rule_str, port); @@ -1060,7 +1074,11 @@ struct utun_config* parse_config_from_buf(const char *buf, size_t len, const cha continue; } char key[MAX_LINE_LEN], value[MAX_LINE_LEN]; - if (config_ini_key_value(trimmed, key, sizeof(key), value, sizeof(value)) < 0) { DEBUG_WARN(DEBUG_CATEGORY_CONFIG, "%s:%d: Invalid key=value format", filename, line_num); continue; } + if (config_ini_key_value(trimmed, key, sizeof(key), value, sizeof(value)) < 0) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Invalid key=value format", filename, line_num); + if (cur_section == SECTION_FIREWALL) goto error; + continue; + } switch (cur_section) { case SECTION_GLOBAL: parse_global(key, value, &cfg->global, filename, line_num); break; case SECTION_SERVER: if (cur_server) parse_server(key, value, cur_server, filename, line_num); break; @@ -1073,7 +1091,16 @@ struct utun_config* parse_config_from_buf(const char *buf, size_t len, const cha strncpy(cfg->global.debug_levels.level[index], value, 15); cfg->global.debug_levels.level[index][15] = '\0'; } else DEBUG_WARN(DEBUG_CATEGORY_CONFIG, "%s:%d: Too many debug categories, ignored %s=%s", filename, line_num, key, value); break; - case SECTION_FIREWALL: if (strcmp(key, "allow") == 0) parse_firewall_rule(value, &cfg->global); else DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Unknown firewall option '%s'", filename, line_num, key); break; + case SECTION_FIREWALL: + if (strcmp(key, "allow") != 0) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Unknown firewall option '%s'", filename, line_num, key); + goto error; + } + if (parse_firewall_rule(value, &cfg->global) < 0) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Invalid firewall rule allow=%s", filename, line_num, value); + goto error; + } + break; case SECTION_CONTROL: parse_control(key, value, &cfg->global, filename, line_num); break; case SECTION_ALLOWED_KEYS: if (strcmp(key, "allow_all") == 0) cfg->global.allowed_keys_allow_all = strcasecmp(value, "yes") == 0 || strcasecmp(value, "1") == 0 || strcasecmp(value, "true") == 0; else if (strcmp(key, "key") == 0) { struct CFG_ALLOWED_KEY *nk = u_calloc(1, sizeof(struct CFG_ALLOWED_KEY)); if (nk && strlen(value) == SC_PUBKEY_SIZE * 2 && config_ini_hex(value, nk->key_bin, SC_PUBKEY_SIZE) == 0) { nk->next = cfg->global.allowed_keys; cfg->global.allowed_keys = nk; cfg->global.allowed_keys_count++; } else u_free(nk); } else DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "%s:%d: Unknown allowed_keys option '%s'", filename, line_num, key); break; case SECTION_NAT: cfg->global.nat_enabled = 1; parse_nat(key, value, &cfg->global, filename, line_num); break; diff --git a/src/utun_instance.c b/src/utun_instance.c index 65f917ec..2f51947e 100644 --- a/src/utun_instance.c +++ b/src/utun_instance.c @@ -996,6 +996,12 @@ struct UTUN_INSTANCE *utun_instance_reload(struct UTUN_INSTANCE *instance, struc DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Failed to parse new config"); return instance; } + struct firewall_ctx next_fw = {0}; + if (fw_load_rules(&next_fw, &new_config->global) < 0) { + DEBUG_ERROR(DEBUG_CATEGORY_CONFIG, "Reload rejected: cannot prepare firewall; previous configuration retained"); + free_config(new_config); + return instance; + } // Reset + apply debug categories from [debug] section (always, for partial and full paths). // Mirrors utun_instance_create exactly. Reset ensures removed categories fall back to global. @@ -1022,6 +1028,7 @@ struct UTUN_INSTANCE *utun_instance_reload(struct UTUN_INSTANCE *instance, struc } if (needs_full) { DEBUG_INFO(DEBUG_CATEGORY_CONFIG, "Incompatible global - full reload"); + fw_free(&next_fw); utun_instance_destroy(instance); struct UTUN_INSTANCE *new_instance = utun_instance_create(ua, config_file); if (!new_instance || utun_instance_init(new_instance) < 0) { @@ -1102,8 +1109,8 @@ struct UTUN_INSTANCE *utun_instance_reload(struct UTUN_INSTANCE *instance, struc DEBUG_INFO(DEBUG_CATEGORY_CONFIG, "Networks reloaded"); } fw_free(&instance->fw); - fw_init(&instance->fw); - fw_load_rules(&instance->fw, &new_config->global); + instance->fw = next_fw; + DEBUG_INFO(DEBUG_CATEGORY_CONFIG, "Firewall reload applied: rules=%d bypass_all=%d", instance->fw.count, instance->fw.bypass_all); // routing_destroy(instance); // commented per request - BGP self-restarts on conn events // routing_create(instance); // commented per request stun_nat_destroy(instance);