Skip to content

nstun: fail closed on invalid policy rule fields #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
carrerasdarren-cell wants to merge 1 commit into
base: master
Choose a base branch
from
+254 −69
Open

nstun: fail closed on invalid policy rule fields #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,15 @@ OLD_EF := --experimental_mnt=old
NEW_EF := --experimental_mnt=new
UID := $(shell id -u)

.PHONY: test
test: $(BIN)
.PHONY: test nstun_policy_test
nstun_policy_test: $(SRCS_PB_CXX)
$(CXX) -std=c++20 -Wall -Wextra -Werror -Wno-unused -Wno-unused-parameter \
-Wno-c99-designator -I. \
$(PROTOBUF_CFLAGS) tests/nstun_policy_test.cc nstun/policy.cc logs.cc util.cc \
config.pb.cc -o /tmp/nsjail_nstun_policy_test $(PROTOBUF_LIBS) -lpthread
/tmp/nsjail_nstun_policy_test

test: $(BIN) nstun_policy_test
# --- Basic sanity tests ---
$(call run_test, ./nsjail -q -Mo --chroot / --user 99999 --group 99999 -- /bin/true, 0)
$(call run_test, ./nsjail -q -Mo --chroot / --user 99999 --group 99999 -- /bin/false, 1)
Expand Down
92 changes: 30 additions & 62 deletions nstun/nstun.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include <sys/wait.h>
#include <unistd.h>

#include <charconv>
#include <thread>

#include "core.h"
Expand Down Expand Up @@ -204,14 +203,22 @@ bool nstun_init_parent(int sock, nsj_t* nsj) {
auto assign_ip = [](const std::string& str, uint32_t* ip) {
if (inet_pton(AF_INET, str.c_str(), ip) != 1) {
LOG_E("Failed to parse IP: %s", str.c_str());
return false;
}
return true;
};

if (!nsj->njc.user_net().ip4().empty()) {
assign_ip(nsj->njc.user_net().ip4(), &ctx->guest_ip4);
if (!assign_ip(nsj->njc.user_net().ip4(), &ctx->guest_ip4)) {
close(tap_fd);
return false;
}
}
if (!nsj->njc.user_net().gw4().empty()) {
assign_ip(nsj->njc.user_net().gw4(), &ctx->host_ip4);
if (!assign_ip(nsj->njc.user_net().gw4(), &ctx->host_ip4)) {
close(tap_fd);
return false;
}
}

if (!nsj->njc.user_net().ip6().empty()) {
Expand All @@ -231,57 +238,6 @@ bool nstun_init_parent(int sock, nsj_t* nsj) {
}
}

auto parse_ip = [](const std::string& str, uint32_t* ip, uint32_t* mask) {
std::string ip_str = str;
int bits = 32;
size_t pos = str.find('/');
if (pos != std::string::npos) {
ip_str = str.substr(0, pos);
const char* p = str.c_str() + pos + 1;
auto [ptr, ec] = std::from_chars(p, str.c_str() + str.length(), bits);
if (ec != std::errc()) {
LOG_E("Failed to parse mask bits: %s", p);
return;
}
}
if (inet_pton(AF_INET, ip_str.c_str(), ip) != 1) {
LOG_E("Failed to parse IP string: %s", ip_str.c_str());
return;
}
*mask = (bits == 0) ? 0 : htonl(~((1ULL << (32 - bits)) - 1));
};

auto parse_ip6 = [](const std::string& str, uint8_t* ip6, uint8_t* mask6) {
std::string ip_str = str;
int bits = 128;
size_t pos = str.find('/');
if (pos != std::string::npos) {
ip_str = str.substr(0, pos);
const char* p = str.c_str() + pos + 1;
auto [ptr, ec] = std::from_chars(p, str.c_str() + str.length(), bits);
if (ec != std::errc()) {
LOG_E("Failed to parse mask bits: %s", p);
return;
}
}
if (inet_pton(AF_INET6, ip_str.c_str(), ip6) != 1) {
LOG_E("Failed to parse IPv6 string: %s", ip_str.c_str());
return;
}
memset(mask6, 0, nstun::IPV6_ADDR_LEN);
for (int i = 0; i < (int)nstun::IPV6_ADDR_LEN; i++) {
if (bits >= 8) {
mask6[i] = 0xFF;
bits -= 8;
} else if (bits > 0) {
mask6[i] = (uint8_t)(0xFF << (8 - bits));
bits = 0;
} else {
mask6[i] = 0;
}
}
};

ctx->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
if (ctx->epoll_fd == -1) {
PLOG_E("epoll_create1(EPOLL_CLOEXEC)");
Expand All @@ -307,14 +263,20 @@ bool nstun_init_parent(int sock, nsj_t* nsj) {
if (status == nstun::RuleParseStatus::IGNORE) continue;

if (r.has_src_ip()) {
parse_ip(r.src_ip(), &nr.src_ip4, &nr.src_mask4);
if (!nstun::parse_ip4_cidr(r.src_ip(), &nr.src_ip4, &nr.src_mask4)) {
return cleanup_and_fail();
}
}
if (r.has_dst_ip()) {
parse_ip(r.dst_ip(), &nr.dst_ip4, &nr.dst_mask4);
if (!nstun::parse_ip4_cidr(r.dst_ip(), &nr.dst_ip4, &nr.dst_mask4)) {
return cleanup_and_fail();
}
}

if (inet_pton(AF_INET, r.redirect_ip().c_str(), &nr.redirect_ip4) != 1) {
LOG_E("Failed to parse redirect IP: %s", r.redirect_ip().c_str());
if (r.has_redirect_ip() &&
inet_pton(AF_INET, r.redirect_ip().c_str(), &nr.redirect_ip4) != 1) {
LOG_E("Invalid redirect IPv4 address: %s", r.redirect_ip().c_str());
return cleanup_and_fail();
}
nr.redirect_port = r.has_redirect_port() ? r.redirect_port() : 0;

Expand Down Expand Up @@ -396,10 +358,14 @@ bool nstun_init_parent(int sock, nsj_t* nsj) {
if (status == nstun::RuleParseStatus::IGNORE) continue;

if (r.has_src_ip()) {
parse_ip6(r.src_ip(), nr.src_ip6, nr.src_mask6);
if (!nstun::parse_ip6_cidr(r.src_ip(), nr.src_ip6, nr.src_mask6)) {
return cleanup_and_fail();
}
}
if (r.has_dst_ip()) {
parse_ip6(r.dst_ip(), nr.dst_ip6, nr.dst_mask6);
if (!nstun::parse_ip6_cidr(r.dst_ip(), nr.dst_ip6, nr.dst_mask6)) {
return cleanup_and_fail();
}
}

if (r.has_redirect_ip()) {
Expand All @@ -408,15 +374,17 @@ bool nstun_init_parent(int sock, nsj_t* nsj) {
/* Proxy is always IPv4 */
if (inet_pton(AF_INET, r.redirect_ip().c_str(), &nr.redirect_ip4) !=
1) {
LOG_E("Failed to parse redirect IP: %s",
LOG_E("Invalid redirect IPv4 address: %s",
r.redirect_ip().c_str());
return cleanup_and_fail();
}
} else {
/* REDIRECT: target is IPv6 */
if (inet_pton(AF_INET6, r.redirect_ip().c_str(), nr.redirect_ip6) !=
1) {
LOG_E("Failed to parse redirect IPv6: %s",
LOG_E("Invalid redirect IPv6 address: %s",
r.redirect_ip().c_str());
return cleanup_and_fail();
}
}
}
Expand Down
113 changes: 108 additions & 5 deletions nstun/policy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
#include "policy.h"

#include <arpa/inet.h>
#include <string.h>

#include <charconv>
#include <limits>
#include <string>

#include "core.h"
#include "logs.h"
#include "nstun.h"
Expand All @@ -12,6 +17,72 @@

namespace nstun {

static bool parse_prefix(const std::string& str, int max_bits, std::string* ip_str, int* bits) {
*ip_str = str;
*bits = max_bits;

size_t pos = str.find('/');
if (pos == std::string::npos) return true;

*ip_str = str.substr(0, pos);
const char* begin = str.data() + pos + 1;
const char* end = str.data() + str.size();
int parsed_bits = 0;
auto [ptr, ec] = std::from_chars(begin, end, parsed_bits);
if (begin == end || ec != std::errc() || ptr != end || parsed_bits < 0 ||
parsed_bits > max_bits) {
LOG_E("Invalid CIDR prefix length: %s", str.c_str());
return false;
}

*bits = parsed_bits;
return true;
}

bool parse_ip4_cidr(const std::string& str, uint32_t* ip, uint32_t* mask) {
std::string ip_str;
int bits;
if (!parse_prefix(str, 32, &ip_str, &bits)) return false;

uint32_t parsed_ip;
if (inet_pton(AF_INET, ip_str.c_str(), &parsed_ip) != 1) {
LOG_E("Invalid IPv4 address: %s", str.c_str());
return false;
}

uint32_t host_mask = bits == 0 ? 0 : std::numeric_limits<uint32_t>::max() << (32 - bits);
*ip = parsed_ip;
*mask = htonl(host_mask);
return true;
}

bool parse_ip6_cidr(const std::string& str, uint8_t* ip6, uint8_t* mask6) {
std::string ip_str;
int bits;
if (!parse_prefix(str, 128, &ip_str, &bits)) return false;

uint8_t parsed_ip6[IPV6_ADDR_LEN];
if (inet_pton(AF_INET6, ip_str.c_str(), parsed_ip6) != 1) {
LOG_E("Invalid IPv6 address: %s", str.c_str());
return false;
}

uint8_t parsed_mask6[IPV6_ADDR_LEN] = {};
for (size_t i = 0; i < IPV6_ADDR_LEN; i++) {
if (bits >= 8) {
parsed_mask6[i] = 0xFF;
bits -= 8;
} else if (bits > 0) {
parsed_mask6[i] = (uint8_t)(0xFF << (8 - bits));
bits = 0;
}
}

memcpy(ip6, parsed_ip6, IPV6_ADDR_LEN);
memcpy(mask6, parsed_mask6, IPV6_ADDR_LEN);
return true;
}

RuleResult evaluate_rules4(Context* ctx, nstun_direction_t dir, nstun_proto_t proto,
uint32_t src_ip4, uint32_t dst_ip4, uint16_t sport, uint16_t dport) {
for (const auto& r : ctx->rules) {
Expand Down Expand Up @@ -85,6 +156,31 @@ RuleResult evaluate_rules6(Context* ctx, nstun_direction_t dir, nstun_proto_t pr

template <typename RuleMsg>
RuleParseStatus fill_rule_common(const RuleMsg& r, nstun_rule_t* nr) {
auto set_port_range = [](const char* name, bool has_start, uint32_t start, bool has_end,
uint32_t end, uint16_t* out_start, uint16_t* out_end) {
if (has_end && !has_start) {
LOG_E("%s_end requires %s", name, name);
return false;
}
if (!has_start) {
*out_start = 0;
*out_end = 0;
return true;
}
if (start == 0 || start > std::numeric_limits<uint16_t>::max()) {
LOG_E("Invalid %s: %u", name, start);
return false;
}
uint32_t range_end = has_end ? end : start;
if (range_end < start || range_end > std::numeric_limits<uint16_t>::max()) {
LOG_E("Invalid %s range: %u-%u", name, start, range_end);
return false;
}
*out_start = (uint16_t)start;
*out_end = (uint16_t)range_end;
return true;
};

if ((r.action() == nsjail::NsJailConfig_UserNet_NstunRule_Action_ENCAP_SOCKS5 ||
r.action() == nsjail::NsJailConfig_UserNet_NstunRule_Action_ENCAP_CONNECT) &&
r.proto() == nsjail::NsJailConfig_UserNet_NstunRule_Protocol_ICMP) {
Expand Down Expand Up @@ -124,11 +220,18 @@ RuleParseStatus fill_rule_common(const RuleMsg& r, nstun_rule_t* nr) {
nr->proto = NSTUN_PROTO_ANY;
}

nr->sport_start = r.has_sport() ? r.sport() : 0;
nr->sport_end = r.has_sport_end() ? r.sport_end() : nr->sport_start;

nr->dport_start = r.has_dport() ? r.dport() : 0;
nr->dport_end = r.has_dport_end() ? r.dport_end() : nr->dport_start;
if (!set_port_range("sport", r.has_sport(), r.sport(), r.has_sport_end(), r.sport_end(),
&nr->sport_start, &nr->sport_end)) {
return RuleParseStatus::ABORT;
}
if (!set_port_range("dport", r.has_dport(), r.dport(), r.has_dport_end(), r.dport_end(),
&nr->dport_start, &nr->dport_end)) {
return RuleParseStatus::ABORT;
}
if (r.has_redirect_port() && r.redirect_port() > std::numeric_limits<uint16_t>::max()) {
LOG_E("Invalid redirect_port: %u", r.redirect_port());
return RuleParseStatus::ABORT;
}

return RuleParseStatus::OK;
}
Expand Down
5 changes: 5 additions & 0 deletions nstun/policy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef NSTUN_POLICY_H_
#define NSTUN_POLICY_H_

#include <string>

#include "core.h"
#include "nstun.h"

Expand All @@ -18,6 +20,9 @@ RuleResult evaluate_rules4(Context* ctx, nstun_direction_t dir, nstun_proto_t pr
RuleResult evaluate_rules6(Context* ctx, nstun_direction_t dir, nstun_proto_t proto,
const uint8_t* src_ip6, const uint8_t* dst_ip6, uint16_t sport, uint16_t dport);

bool parse_ip4_cidr(const std::string& str, uint32_t* ip, uint32_t* mask);
bool parse_ip6_cidr(const std::string& str, uint8_t* ip6, uint8_t* mask6);

template <typename RuleMsg>
RuleParseStatus fill_rule_common(const RuleMsg& r, nstun_rule_t* nr);

Expand Down
Loading