Skip to content

Commit 92156e3

Browse files
jaredhendrickson13jaredhendrickson13
committed
Fixed bug that reinitialized the NAT configuration unexpectedly, allow firewall rules to be created with port aliases, restructured alias endpoints and improved input validation
1 parent c324d42 commit 92156e3

10 files changed

Lines changed: 308 additions & 241 deletions

‎pfSense-pkg-API/files/etc/inc/api/framework/APIResponse.inc‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ function get($id, $data=[], $all=false) {
12121212
"status" => "bad request",
12131213
"code" => 400,
12141214
"return" => $id,
1215-
"message" => "Firewall alias name must be type string"
1215+
"message" => "Invalid firewall alias name"
12161216
],
12171217
4054 => [
12181218
"status" => "bad request",
@@ -1254,7 +1254,7 @@ function get($id, $data=[], $all=false) {
12541254
"status" => "bad request",
12551255
"code" => 400,
12561256
"return" => $id,
1257-
"message" => "Invalid firewall alias port"
1257+
"message" => "Invalid firewall alias port or port range"
12581258
],
12591259
4061 => [
12601260
"status" => "bad request",
@@ -1532,6 +1532,12 @@ function get($id, $data=[], $all=false) {
15321532
"return" => $id,
15331533
"message" => "Alias details cannot contain more items than alias addresses"
15341534
],
1535+
4107 => [
1536+
"status" => "bad request",
1537+
"code" => 400,
1538+
"return" => $id,
1539+
"message" => "Alias type cannot be changed while in use"
1540+
],
15351541

15361542
//5000-5999 reserved for /users API calls
15371543
5000 => [

‎pfSense-pkg-API/files/etc/inc/api/framework/APITools.inc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,8 +593,6 @@ function alias_in_use($alias_name) {
593593
alias_find_references(array('nat', 'rule'), array('target'), $alias_name, $is_alias_referenced, $referenced_by);
594594
alias_find_references(array('nat', 'rule'), array('local-port'), $alias_name, $is_alias_referenced, $referenced_by);
595595
// NAT 1:1 Rules
596-
//alias_find_references(array('nat', 'onetoone'), array('external'), $alias_name, $is_alias_referenced, $referenced_by);
597-
//alias_find_references(array('nat', 'onetoone'), array('source', 'address'), $alias_name, $is_alias_referenced, $referenced_by);
598596
alias_find_references(array('nat', 'onetoone'), array('destination', 'address'), $alias_name, $is_alias_referenced, $referenced_by);
599597
// NAT Outbound Rules
600598
alias_find_references(array('nat', 'outbound', 'rule'), array('source', 'network'), $alias_name, $is_alias_referenced, $referenced_by);

‎pfSense-pkg-API/files/etc/inc/api/models/APIFirewallAliasCreate.inc‎

Lines changed: 106 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -32,147 +32,140 @@ class APIFirewallAliasCreate extends APIModel {
3232
send_event("filter reload"); // Ensure our firewall filter is reloaded
3333
return APIResponse\get(0, $this->validated_data);
3434
}
35-
36-
public function validate_payload() {
3735

38-
$allowed_alias_types = array("host", "network", "port"); // Array of allowed alias types
36+
private function __validate_name() {
37+
# Require clients to specify a valid name for the alias
3938
if (isset($this->initial_data['name'])) {
40-
$name = $this->initial_data['name'];
41-
$name = APITools\sanitize_str($name);
39+
# Ensure an alias with this name does not already exist
40+
if (!is_alias($this->initial_data['name'])) {
41+
# Ensure the requested name is valid and not reserved by the system
42+
if (is_validaliasname($this->initial_data["name"])) {
43+
$this->validated_data["name"] = $this->initial_data['name'];
44+
} else {
45+
$this->errors[] = APIResponse\get(4053);
46+
}
47+
} else {
48+
$this->errors[] = APIResponse\get(4056);
49+
}
4250
} else {
4351
$this->errors[] = APIResponse\get(4050);
4452
}
53+
}
54+
55+
private function __validate_type() {
56+
# Require clients to specify a valid type for this alias
4557
if (isset($this->initial_data['type'])) {
46-
$type = $this->initial_data['type'];
47-
$type = trim($type);
58+
# Require this type to be supported
59+
if (in_array($this->initial_data['type'], ["host", "network", "port"])) {
60+
$this->validated_data["type"] = $this->initial_data["type"];
61+
} else {
62+
$this->errors[] = APIResponse\get(4057);
63+
}
4864
} else {
4965
$this->errors[] = APIResponse\get(4061);
5066
}
67+
}
68+
69+
private function __validate_address() {
5170
if (isset($this->initial_data['address'])) {
52-
$address = $this->initial_data['address'];
53-
// Convert string to array
54-
if (!is_array($address)) {
55-
$address = array($address);
71+
# Convert value to array if it is not already
72+
if (!is_array($this->initial_data['address'])) {
73+
$this->initial_data['address'] = [$this->initial_data['address']];
5674
}
57-
} else {
58-
$this->errors[] = APIResponse\get(4052);
5975

60-
}
61-
if (isset($this->initial_data['descr'])) {
62-
$descr = $this->initial_data['descr'];
63-
}
64-
if (isset($this->initial_data['detail'])) {
65-
$detail = $this->initial_data['detail'];
66-
// Convert string to array
67-
if (!is_array($detail)) {
68-
$detail = array($detail);
69-
}
70-
}
71-
// Check that our input is valid
72-
if (!is_string($name)) {
73-
$this->errors[] = APIResponse\get(4053);
74-
} elseif (!is_string($type)) {
75-
$this->errors[] = APIResponse\get(4062);
76-
} elseif (!in_array($type, $allowed_alias_types)) {
77-
$this->errors[] = APIResponse\get(4057);
78-
} elseif (isset($descr) and !is_string($descr)) {
79-
$this->errors[] = APIResponse\get(4063);
80-
} elseif (!is_array($address)) {
81-
$this->errors[] = APIResponse\get(4054);
82-
} elseif (isset($detail) and !is_array($detail)) {
83-
$this->errors[] = APIResponse\get(4063);
84-
}
85-
if (!isset($type_err)) {
86-
// Loop through our arrays and ensure the values are valid
87-
$a_count = 0; // Define a loop counter
88-
foreach ($address as $ae) {
89-
// Conditions for alias type 'port'
90-
if ($type === "port") {
91-
// Check that our value is numeric
92-
if (is_numeric($ae)) {
93-
if (1 <= intval($ae) and intval($ae) <= 65535) {
94-
$address[$a_count] = strval($ae);
95-
} else {
96-
$this->errors[] = APIResponse\get(4065);
97-
98-
}
76+
# Loop through each address and ensure it is valid for our specified type
77+
foreach($this->initial_data['address'] as $address) {
78+
# Validation for host type aliases
79+
if ($this->validated_data["type"] === "host") {
80+
# Require this address to be a valid IPv4, IPv6, or FQDN
81+
if (is_ipaddrv4($address) or is_ipaddrv6($address) or is_fqdn($address)) {
82+
$this->validated_data["address"][] = $address;
9983
} else {
100-
$this->errors[] = APIResponse\get(4066);
101-
84+
$this->errors[] = APIResponse\get(4058);
10285
}
10386
}
104-
// Conditionals for alias type 'network'
105-
if ($type === "network") {
106-
// Check that values are strings
107-
if (is_string($ae)) {
108-
// Check that string is a network CIDR
109-
if (strpos($ae, "/")) {
110-
$net_ip = explode("/", $ae)[0]; // Save our network IP
111-
$bit_mask = explode("/", $ae)[1]; // Save our subnet bit mask
112-
// Check if our IP is IPv4
113-
if (is_ipaddrv4($net_ip)) {
114-
$max_bits = 32; // Assign our maximum IPv4 bitmask
115-
} elseif (is_ipaddrv6($net_ip)) {
116-
$max_bits = 128; // Assign our maximum IPv4 bitmask
117-
} else {
118-
$this->errors[] = APIResponse\get(4067);
119-
120-
}
121-
// Check if our bitmask is numeric and in range
122-
if (is_numeric($bit_mask)) {
123-
if (1 <= intval($bit_mask) and intval($bit_mask) <= $max_bits) {
124-
continue;
125-
} else {
126-
$this->errors[] = APIResponse\get(4068);
127-
}
128-
} else {
129-
$this->errors[] = APIResponse\get(4069);
130-
}
131-
} else {
132-
$this->errors[] = APIResponse\get(4069);
133-
134-
}
87+
# Validation for network type aliases
88+
elseif ($this->validated_data["type"] === "network") {
89+
# Require this address to be a valid IPv4 subnet, IPv6 subnet, or FQDN
90+
if (is_subnetv4($address) or is_subnetv6($address) or is_fqdn($address)) {
91+
$this->validated_data["address"][] = $address;
13592
} else {
136-
$this->errors[] = APIResponse\get(4070);
137-
93+
$this->errors[] = APIResponse\get(4059);
13894
}
13995
}
140-
// Conditions for alias type 'host'
141-
if ($type === "host") {
142-
// Check that values are strings
143-
if (is_string($ae)) {
144-
$address[$a_count] = APITools\sanitize_str($ae);
145-
} else {
146-
$this->errors[] = APIResponse\get(4070);
96+
# Validation for port type aliases
97+
elseif ($this->validated_data["type"] === "port") {
98+
# Convert integers to string expected by is_port_or_range() function
99+
$address = (is_int($address)) ? strval($address) : $address;
147100

101+
# Replace hyphen with colon
102+
$address = str_replace("-", ":", $address);
103+
104+
# Require this address to be a valid port or port range
105+
if (is_port_or_range($address)) {
106+
$this->validated_data["address"][] = $address;
107+
} else {
108+
$this->errors[] = APIResponse\get(4060);
148109
}
149110
}
150-
// Increase our counter
151-
$a_count++;
111+
152112
}
153-
// Check each of our alias details
154-
foreach ($detail as $de) {
155-
if (!is_string($de)) {
156-
$this->errors[] = APIResponse\get(4071);
157113

158-
}
114+
# Convert our array to a space separated string as expected by the XML config
115+
$this->validated_data["address"] = implode(" ", $this->validated_data["address"]);
116+
} else {
117+
$this->errors[] = APIResponse\get(4052);
118+
}
119+
}
120+
121+
private function __validate_descr() {
122+
# Optionally allow clients to specify an alias description
123+
if (isset($this->initial_data['descr'])) {
124+
$this->validated_data["descr"] = $this->initial_data['descr'];
125+
} else {
126+
$this->validated_data["descr"] = "";
127+
}
128+
}
129+
130+
private function __validate_detail() {
131+
# Convert single values to an array
132+
if (!is_array($this->initial_data["detail"])) {
133+
$this->initial_data["detail"] = [$this->initial_data["detail"]];
134+
}
135+
136+
# If we have less detail values than address values, add some defaults
137+
while(true) {
138+
# Check if we have the correct number of detail values
139+
if (count($this->initial_data["detail"]) < count($this->initial_data["address"])) {
140+
$this->initial_data["detail"][] = "Entry added " . date('r');
141+
} else {
142+
break;
159143
}
160144
}
161-
// Check our existing aliases
162-
if (is_array($this->config["aliases"])) {
163-
$c_count = 0;
164-
// Loop through each alias and see if alias already exists
165-
foreach ($this->config["aliases"]["alias"] as $ce) {
166-
if ($ce["name"] === $name) {
167-
$this->errors[] = APIResponse\get(4056);
168145

146+
# Ensure the number of detail values is less than or equal to the number of address values
147+
if (count($this->initial_data["detail"]) <= count($this->initial_data["address"])) {
148+
# Loop through each alias detail and ensure it is a string
149+
foreach ($this->initial_data["detail"] as $detail) {
150+
if (is_string($detail)) {
151+
$this->validated_data["detail"][] = $detail;
152+
} else {
153+
$this->errors[] = APIResponse\get(4071);
169154
}
170155
}
156+
157+
# Convert our array to pfSense's expected XML string format
158+
$this->validated_data["detail"] = implode("||", $this->validated_data["detail"]);
159+
} else {
160+
$this->errors[] = APIResponse\get(4106);
171161
}
172-
$this->validated_data["name"] = $name; // Save our alias name
173-
$this->validated_data["type"] = $type; // Save our type
174-
$this->validated_data["descr"] = $descr; // Save our description
175-
$this->validated_data["address"] = implode(" ", $address); // Join array in to space seperated string
176-
$this->validated_data["detail"] = implode("||", $detail); // Join array in to || seperated string
162+
}
163+
164+
public function validate_payload() {
165+
$this->__validate_name();
166+
$this->__validate_type();
167+
$this->__validate_descr();
168+
$this->__validate_address();
169+
$this->__validate_detail();
177170
}
178171
}

‎pfSense-pkg-API/files/etc/inc/api/models/APIFirewallAliasDelete.inc‎

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,45 +25,36 @@ class APIFirewallAliasDelete extends APIModel {
2525
}
2626

2727
public function action() {
28-
$del_conf = $this->config["aliases"]["alias"][$this->id]; // Capture alias config before deleting
29-
unset($this->config["aliases"]["alias"][$this->id]); // Remove this alias from our configuration
30-
$this->config["aliases"]["alias"] = array_values($this->config["aliases"]["alias"]); // Reindex array
31-
$this->write_config(); // Apply our configuration change
32-
send_event("filter reload"); // Ensure our firewall filter is reloaded
28+
$del_conf = $this->config["aliases"]["alias"][$this->id];
29+
unset($this->config["aliases"]["alias"][$this->id]);
30+
$this->config["aliases"]["alias"] = array_values($this->config["aliases"]["alias"]);
31+
$this->write_config();
32+
send_event("filter reload");
3333
return APIResponse\get(0, $del_conf);
3434
}
35-
36-
public function validate_payload() {
37-
38-
if (isset($this->initial_data['id'])) {
39-
$name = $this->initial_data['id'];
40-
$name = APITools\sanitize_str($name);
4135

42-
// Check that alias is not in use in our configuration
43-
if (!APITools\alias_in_use($name)) {
44-
// Loop through our current config and find the index ID for our alias to delete
45-
$c_count = 0; // Init loop counter
46-
foreach ($this->config["aliases"]["alias"] as $ce) {
47-
// Check if this entry matches our requested value
48-
if ($ce["name"] === $name) {
49-
$del_index = $c_count;
50-
break;
51-
}
52-
$c_count++;
36+
private function __validate_id() {
37+
# Require clients to pass in an ID field containing the alias name or index value
38+
if (isset($this->initial_data["id"])) {
39+
# Loop through each alias and check for a match
40+
foreach ($this->config["aliases"]["alias"] as $id=>$alias) {
41+
# First check if the ID matches the index value or the alias name
42+
if ($this->initial_data["id"] === $id or $this->initial_data["id"] === $alias["name"]) {
43+
$this->id = $id;
44+
$this->validated_data = $alias;
45+
break;
5346
}
54-
55-
if (is_numeric($del_index)) {
56-
$this->id = $del_index;
57-
} else {
58-
$this->errors[] = APIResponse\get(4055);
59-
}
60-
61-
} else {
62-
$this->errors[] = APIResponse\get(4051);
47+
}
48+
# If we did not find an ID in the loop, return a not found error
49+
if (is_null($this->id)) {
50+
$this->errors[] = APIResponse\get(4055);
6351
}
6452
} else {
65-
$this->errors = APIResponse\get(4074);
53+
$this->errors[] = APIResponse\get(4050);
6654
}
67-
55+
}
56+
57+
public function validate_payload() {
58+
$this->__validate_id();
6859
}
6960
}

‎pfSense-pkg-API/files/etc/inc/api/models/APIFirewallAliasEntryDelete.inc‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,12 @@ class APIFirewallAliasEntryDelete extends APIModel {
3434
private function __validate_name() {
3535
# Require clients to pass in a name value to locate the alias to add addresses to
3636
if (isset($this->initial_data['name'])) {
37-
# Ensure our alias name is a string
38-
if (is_string($this->initial_data['name'])) {
39-
# Ensure this alias exists
40-
$this->id = $this->__get_alias_id($this->initial_data["name"]);
41-
if (isset($this->id)) {
42-
$this->validated_data = $this->config["aliases"]["alias"][$this->id];
43-
} else {
44-
$this->errors[] = APIResponse\get(4055);
45-
}
37+
# Ensure this alias exists
38+
$this->id = $this->__get_alias_id($this->initial_data["name"]);
39+
if (isset($this->id)) {
40+
$this->validated_data = $this->config["aliases"]["alias"][$this->id];
4641
} else {
47-
$this->errors[] = APIResponse\get(4053);
42+
$this->errors[] = APIResponse\get(4055);
4843
}
4944
} else {
5045
$this->errors[] = APIResponse\get(4050);

0 commit comments

Comments
 (0)