Skip to content

Commit 87fcfc0

Browse files
author
Jared Hendrickson
committed
Fixed bug that unnecessarily required the protocol field when updating firewall rules, fixed bug that prevented ports from being behaving correctly when updating firewall rules, fixed bug that prevented firewall rules from being disabled, fixed bug that preventing clients from receiving access tokens when API is set to read only mode
1 parent 52ea261 commit 87fcfc0

5 files changed

Lines changed: 42 additions & 23 deletions

File tree

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class APIAuth {
2020
private $api_config;
2121
private $request;
2222
public $auth_mode;
23+
public $read_mode;
2324
public $req_privs;
2425
public $username;
2526
public $privs;
@@ -28,9 +29,10 @@ class APIAuth {
2829
public $is_authorized;
2930

3031
# Create our method constructor
31-
public function __construct($req_privs, $enforce_auth_mode=null){
32+
public function __construct($req_privs, $enforce_auth_mode=null, $read_mode=null){
3233
$this->api_config = APITools\get_api_config()[1];
3334
$this->auth_mode = (is_null($enforce_auth_mode)) ? $this->api_config["authmode"] : $enforce_auth_mode;
35+
$this->read_mode = (is_null($read_mode)) ? array_key_exists("readonly", $this->api_config) : $read_mode;
3436
$this->request = APITools\get_request_data();
3537
$this->req_privs = $req_privs;
3638
$this->privs = [];
@@ -113,17 +115,17 @@ class APIAuth {
113115
# AUTHORIZATION #
114116
# Checks if the current user has the necessary privileges to access this resource
115117
public function authorize() {
116-
// Local variables
118+
# Local variables
117119
$authorized = false;
118120
$client_config =& getUserEntry($this->username);;
119121
$this->privs = get_user_privileges($client_config);
120122
$read_only = array_key_exists("readonly", $this->api_config);
121123

122124
# If no require privileges were given, assume call is always authorized
123125
if (!empty($this->req_privs)) {
124-
// Ensure our API is not read only OR if it is read only, only authorize the request if it is a GET request
125-
if ($read_only === false or ($read_only === true and $_SERVER['REQUEST_METHOD'] === "GET")) {
126-
// Loop through each of our req privs and ensure the client has them, also check if access is read only
126+
# If API is in readonly mode, only allow GET requests
127+
if ($this->read_mode === false or ($this->read_mode === true and $_SERVER['REQUEST_METHOD'] === "GET")) {
128+
# Loop through each of our req privs and ensure the client has them, also check if access is read only
127129
foreach ($this->req_privs as &$priv) {
128130
if (in_array($priv, $this->privs)) {
129131
$authorized = true;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class APIModel {
3030
public $change_note;
3131
public $requires_auth;
3232
public $set_auth_mode;
33+
public $set_read_mode;
3334

3435
public function __construct() {
3536
global $config;
@@ -39,6 +40,7 @@ class APIModel {
3940
$this->client = null;
4041
$this->requires_auth = true;
4142
$this->set_auth_mode = null;
43+
$this->set_read_mode = null;
4244
$this->change_note = "Made unknown change via API";
4345
$this->id = null;
4446
$this->validate_id = true;
@@ -95,7 +97,7 @@ class APIModel {
9597
}
9698

9799
private function check_authentication() {
98-
$this->client = new APIAuth($this->privileges, $this->set_auth_mode);
100+
$this->client = new APIAuth($this->privileges, $this->set_auth_mode, $this->set_read_mode);
99101
if ($this->requires_auth === true) {
100102
if (!$this->client->is_authenticated) {
101103
$this->errors[] = APIResponse\get(3);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class APIAccessTokenCreate extends APIModel {
2121
public function __construct() {
2222
parent::__construct();
2323
$this->set_auth_mode = "local";
24+
$this->set_read_mode = false;
2425
}
2526

2627
# Validate our API configurations auth mode (must be JWT)

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ class APIFirewallRuleCreate extends APIModel {
3434
}
3535

3636
public function validate_payload() {
37+
# Include static rule ID
38+
$this->validated_data["id"] = "";
39+
3740
# Check for our required 'type' payload value
3841
if (isset($this->initial_data["type"])) {
3942
$type_options = array("pass", "block", "reject");
@@ -179,8 +182,8 @@ class APIFirewallRuleCreate extends APIModel {
179182

180183

181184
# Check for our optional 'disabled' payload value
182-
if ($this->initial_data['disabled'] !== true) {
183-
$this->validated_data["id"] = "";
185+
if ($this->initial_data['disabled'] === true) {
186+
$this->validated_data["disabled"] = "";
184187
}
185188

186189
# Check for our optional 'descr' payload value

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

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class APIFirewallRuleUpdate extends APIModel {
5151
$this->errors[] = APIResponse\get(4031);
5252
}
5353

54-
# Check for our required 'type' payload value
54+
# Check for our optional 'type' payload value
5555
if (isset($this->initial_data["type"])) {
5656
$type_options = array("pass", "block", "reject");
5757
# Ensure our type is a valid option
@@ -62,7 +62,7 @@ class APIFirewallRuleUpdate extends APIModel {
6262
}
6363
}
6464

65-
# Check for our required 'interface' payload value
65+
# Check for our optional 'interface' payload value
6666
if (isset($this->initial_data['interface'])) {
6767
$this->initial_data['interface'] = APITools\get_pfsense_if_id($this->initial_data['interface']);
6868
# Check that we found the request pfSense interface ID
@@ -73,7 +73,7 @@ class APIFirewallRuleUpdate extends APIModel {
7373
}
7474
}
7575

76-
# Check for our required 'ipprotocol' payload value
76+
# Check for our optional 'ipprotocol' payload value
7777
if (isset($this->initial_data['ipprotocol'])) {
7878
$ipprotocol_options = array("inet", "inet6", "inet46");
7979
# Check that our ipprotocol value is a support option
@@ -84,13 +84,21 @@ class APIFirewallRuleUpdate extends APIModel {
8484
}
8585
}
8686

87-
# Check for our required 'protocol' payload value
87+
# Check for our optional 'protocol' payload value
8888
if (isset($this->initial_data['protocol'])) {
89-
$port_required = (!in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) ? true : false;
89+
$missing_port = (!in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) ? true : false;
90+
$requires_port = (in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"])) ? true : false;
9091
$protocol_options = [
9192
"any", "tcp", "udp", "tcp/udp", "icmp", "esp", "ah",
9293
"gre", "ipv6", "igmp", "pim", "ospf", "carp", "pfsync"
9394
];
95+
96+
# If a new protocol was chosen that doesn't require a port, remove existing ports from the rule
97+
if ((!in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"]))) {
98+
unset($this->validated_data["source"]["port"]);
99+
unset($this->validated_data["destination"]["port"]);
100+
}
101+
94102
# Check that our protocol value is a support option
95103
if (in_array($this->initial_data["protocol"], $protocol_options)) {
96104
# Don't add a specific protocol if any
@@ -102,9 +110,10 @@ class APIFirewallRuleUpdate extends APIModel {
102110
} else {
103111
$this->errors[] = APIResponse\get(4042);
104112
}
105-
$protocol = $this->initial_data['protocol'];
106113
} else {
107-
$this->errors[] = APIResponse\get(4036);
114+
# If a new protocol was not selected don't validate ports
115+
$requires_port = false;
116+
$missing_port = false;
108117
}
109118

110119
# Check for our optional 'icmpsubtype' payload value when our protocol is set to ICMP
@@ -130,7 +139,7 @@ class APIFirewallRuleUpdate extends APIModel {
130139
}
131140
}
132141

133-
# Check for our required 'src' payload value
142+
# Check for our optional 'src' payload value
134143
if (isset($this->initial_data['src'])) {
135144
// Check if our source and destination values are valid
136145
$dir_check = APITools\is_valid_rule_addr($this->initial_data['src'], "source");
@@ -141,7 +150,7 @@ class APIFirewallRuleUpdate extends APIModel {
141150
}
142151
}
143152

144-
# Check for our required 'src' payload value
153+
# Check for our optional 'src' payload value
145154
if (isset($this->initial_data['dst'])) {
146155
// Check if our source and destination values are valid
147156
$dir_check = APITools\is_valid_rule_addr($this->initial_data['dst'], "destination");
@@ -152,16 +161,16 @@ class APIFirewallRuleUpdate extends APIModel {
152161
}
153162
}
154163

155-
# Check for our required 'srcport' and 'dstport' payload values if protocol is TCP, UDP or TCP/UDP
156-
if ($port_required && in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
164+
# Check for our optional 'srcport' and 'dstport' payload values if protocol is TCP, UDP or TCP/UDP
165+
if ($requires_port) {
157166
if (isset($this->initial_data['srcport'])) {
158167
$val = str_replace("-", ":", $this->initial_data['srcport']);
159168
if (!is_port_or_range($val) and $val !== "any") {
160169
$this->errors[] = APIResponse\get(4048);
161170
} elseif ($val !== "any") {
162171
$this->validated_data["source"]["port"] = str_replace(":", "-", $val);;
163172
}
164-
} else {
173+
} elseif ($missing_port) {
165174
$this->errors[] = APIResponse\get(4047);
166175
}
167176

@@ -172,7 +181,7 @@ class APIFirewallRuleUpdate extends APIModel {
172181
} elseif ($val !== "any") {
173182
$this->validated_data["destination"]["port"] = str_replace(":", "-", $val);;
174183
}
175-
} else {
184+
} elseif ($missing_port) {
176185
$this->errors[] = APIResponse\get(4047);
177186
}
178187
}
@@ -189,8 +198,10 @@ class APIFirewallRuleUpdate extends APIModel {
189198

190199

191200
# Check for our optional 'disabled' payload value
192-
if ($this->initial_data['disabled'] !== true) {
193-
$this->validated_data["id"] = "";
201+
if ($this->initial_data['disabled'] === true) {
202+
$this->validated_data["disabled"] = "";
203+
} elseif ($this->initial_data['disabled'] === false) {
204+
unset($this->validated_data["disabled"]);
194205
}
195206

196207
# Check for our optional 'descr' payload value

0 commit comments

Comments
 (0)