permission_broker: Remove firewall from permission broker
Remove all references of firewall inside port_tracker
and remove firewall from permission broker.
Instead of calling firewall, port_tracker now calls
patchpanel's DBus API ModifyPortRule. With this change
firewall fuzzer is inactive.
BUG=b:160130580
TEST=FEATURES=test emerge-rammus permission_broker
TEST=USE="asan fuzzer" emerge-atlas permission_broker
TEST=/usr/libexec/fuzzers/port_tracker_fuzzer
TEST=tast run <DUT> platform.Firewall
Cq-Depend: chromium:2291496
Change-Id: Ibb2e4d278c9003af931df19d73c7d1c55bbcb28d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2291865
Commit-Queue: Jason Jeremy Iman <jasongustaman@chromium.org>
Tested-by: Jason Jeremy Iman <jasongustaman@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
diff --git a/permission_broker/BUILD.gn b/permission_broker/BUILD.gn
index 50955fd..9215149 100644
--- a/permission_broker/BUILD.gn
+++ b/permission_broker/BUILD.gn
@@ -11,10 +11,7 @@
deps += [ ":permission_broker_test" ]
}
if (use.fuzzer) {
- deps += [
- ":firewall_fuzzer",
- ":port_tracker_fuzzer",
- ]
+ deps += [ ":port_tracker_fuzzer" ]
}
}
@@ -64,7 +61,6 @@
"deny_unsafe_hidraw_device_rule.cc",
"deny_usb_device_class_rule.cc",
"deny_usb_vendor_id_rule.cc",
- "firewall.cc",
"hidraw_subsystem_udev_rule.cc",
"libusb_wrapper.cc",
"permission_broker.cc",
@@ -112,9 +108,7 @@
"deny_usb_device_class_rule_test.cc",
"deny_usb_vendor_id_rule_test.cc",
"fake_libusb_wrapper.cc",
- "firewall_test.cc",
"group_tty_device_rule_test.cc",
- "mock_firewall.cc",
"port_tracker_test.cc",
"rule_engine_test.cc",
"rule_test.cc",
@@ -128,15 +122,6 @@
pkg_config("permission_broker_fuzzer_config") {
pkg_deps = [ "libchrome-test-${libbase_ver}" ]
}
- executable("firewall_fuzzer") {
- configs += [
- "//common-mk/common_fuzzer:common_fuzzer",
- ":permission_broker_fuzzer_config",
- ":target_defaults",
- ]
- deps = [ ":libpermission_broker" ]
- sources = [ "firewall_fuzzer.cc" ]
- }
executable("port_tracker_fuzzer") {
configs += [
"//common-mk/common_fuzzer:common_fuzzer",
diff --git a/permission_broker/firewall.cc b/permission_broker/firewall.cc
deleted file mode 100644
index f82fd7b..0000000
--- a/permission_broker/firewall.cc
+++ /dev/null
@@ -1,403 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "permission_broker/firewall.h"
-
-#include <arpa/inet.h>
-#include <linux/capability.h>
-#include <netinet/in.h>
-
-#include <string>
-#include <vector>
-
-#include <base/bind.h>
-#include <base/bind_helpers.h>
-#include <base/callback.h>
-#include <base/logging.h>
-#include <base/strings/string_number_conversions.h>
-#include <base/strings/string_util.h>
-#include <base/strings/stringprintf.h>
-#include <brillo/minijail/minijail.h>
-
-namespace {
-
-// Interface names must be shorter than 'IFNAMSIZ' chars.
-// See http://man7.org/linux/man-pages/man7/netdevice.7.html
-// 'IFNAMSIZ' is 16 in recent kernels.
-// See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/if.h?h=v4.14#n33
-constexpr size_t kInterfaceNameSize = 16;
-
-// Interface names are passed directly to the 'iptables' command. Rather than
-// auditing 'iptables' source code to see how it handles malformed names,
-// do some sanitization on the names beforehand.
-bool IsValidInterfaceName(const std::string& iface) {
- // |iface| should be shorter than |kInterfaceNameSize| chars and have only
- // alphanumeric characters (embedded hypens and periods are also permitted).
- if (iface.length() >= kInterfaceNameSize) {
- return false;
- }
- if (base::StartsWith(iface, "-", base::CompareCase::SENSITIVE) ||
- base::EndsWith(iface, "-", base::CompareCase::SENSITIVE) ||
- base::StartsWith(iface, ".", base::CompareCase::SENSITIVE) ||
- base::EndsWith(iface, ".", base::CompareCase::SENSITIVE)) {
- return false;
- }
- for (auto c : iface) {
- if (!std::isalnum(c) && (c != '-') && (c != '.')) {
- return false;
- }
- }
- return true;
-}
-
-} // namespace
-
-namespace permission_broker {
-
-const char kIpTablesPath[] = "/sbin/iptables";
-const char kIp6TablesPath[] = "/sbin/ip6tables";
-
-const std::string ProtocolName(Protocol proto) {
- if (proto == ModifyPortRuleRequest::INVALID_PROTOCOL) {
- NOTREACHED() << "Unexpected L4 protocol value";
- }
- return base::ToLowerASCII(ModifyPortRuleRequest::Protocol_Name(proto));
-}
-
-bool Firewall::AddAcceptRules(Protocol protocol,
- uint16_t port,
- const std::string& interface) {
- if (port == 0U) {
- LOG(ERROR) << "Port 0 is not a valid port";
- return false;
- }
-
- if (!IsValidInterfaceName(interface)) {
- LOG(ERROR) << "Invalid interface name '" << interface << "'";
- return false;
- }
-
- if (!AddAcceptRule(kIpTablesPath, protocol, port, interface)) {
- LOG(ERROR) << "Could not add ACCEPT rule using '" << kIpTablesPath << "'";
- return false;
- }
-
- if (!AddAcceptRule(kIp6TablesPath, protocol, port, interface)) {
- LOG(ERROR) << "Could not add ACCEPT rule using '" << kIp6TablesPath
- << "', aborting operation";
- DeleteAcceptRule(kIpTablesPath, protocol, port, interface);
- return false;
- }
-
- return true;
-}
-
-bool Firewall::DeleteAcceptRules(Protocol protocol,
- uint16_t port,
- const std::string& interface) {
- if (port == 0U) {
- LOG(ERROR) << "Port 0 is not a valid port";
- return false;
- }
-
- if (!IsValidInterfaceName(interface)) {
- LOG(ERROR) << "Invalid interface name '" << interface << "'";
- return false;
- }
-
- bool ip4_success = DeleteAcceptRule(kIpTablesPath, protocol, port,
- interface);
- bool ip6_success = DeleteAcceptRule(kIp6TablesPath, protocol,
- port, interface);
- return ip4_success && ip6_success;
-}
-
-bool Firewall::AddIpv4ForwardRule(Protocol protocol,
- const std::string& input_ip,
- uint16_t port,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port) {
- if (!ModifyIpv4DNATRule(protocol, input_ip, port, interface, dst_ip, dst_port, "-I")) {
- return false;
- }
-
- if (!ModifyIpv4ForwardChain(protocol, interface, dst_ip, dst_port, "-A")) {
- ModifyIpv4DNATRule(protocol, input_ip, port, interface, dst_ip, dst_port, "-D");
- return false;
- }
-
- return true;
-}
-
-bool Firewall::DeleteIpv4ForwardRule(Protocol protocol,
- const std::string& input_ip,
- uint16_t port,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port) {
- bool success = true;
- if (!ModifyIpv4DNATRule(protocol, input_ip, port, interface, dst_ip, dst_port, "-D")) {
- success = false;
- }
- if (!ModifyIpv4ForwardChain(protocol, interface, dst_ip, dst_port, "-D")) {
- success = false;
- }
- return success;
-}
-
-bool Firewall::ModifyIpv4DNATRule(Protocol protocol,
- const std::string& input_ip,
- uint16_t port,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port,
- const std::string& operation) {
- struct in_addr addr;
- if (!input_ip.empty() && inet_pton(AF_INET, input_ip.c_str(), &addr) != 1) {
- LOG(ERROR) << "Invalid input IPv4 address '" << input_ip << "'";
- return false;
- }
-
- if (port == 0U) {
- LOG(ERROR) << "Port 0 is not a valid port";
- return false;
- }
-
- if (!IsValidInterfaceName(interface) || interface.empty()) {
- LOG(ERROR) << "Invalid interface name '" << interface << "'";
- return false;
- }
-
- if (inet_pton(AF_INET, dst_ip.c_str(), &addr) != 1) {
- LOG(ERROR) << "Invalid destination IPv4 address '" << dst_ip << "'";
- return false;
- }
-
- if (dst_port == 0U) {
- LOG(ERROR) << "Destination port 0 is not a valid port";
- return false;
- }
-
- // Only support deleting existing forwarding rules or inserting rules in the
- // first position: ARC++ generic inbound DNAT rule always need to go last.
- if (operation != "-I" && operation != "-D") {
- LOG(ERROR) << "Invalid chain operation '" << operation << "'";
- return false;
- }
-
- std::vector<std::string> argv{kIpTablesPath,
- "-t",
- "nat",
- operation,
- "PREROUTING",
- "-i",
- interface,
- "-p", // protocol
- ProtocolName(protocol)};
- if (!input_ip.empty()) {
- argv.push_back("-d"); // input destination ip
- argv.push_back(input_ip);
- }
- argv.push_back("--dport"); // input destination port
- argv.push_back(std::to_string(port));
- argv.push_back("-j");
- argv.push_back("DNAT");
- argv.push_back("--to-destination"); // new output destination ip:port
- argv.push_back(dst_ip + ":" + std::to_string(dst_port));
- argv.push_back("-w"); // Wait for xtables lock.
- return RunInMinijail(argv) == 0;
-}
-
-bool Firewall::ModifyIpv4ForwardChain(Protocol protocol,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port,
- const std::string& operation) {
- if (!IsValidInterfaceName(interface) || interface.empty()) {
- LOG(ERROR) << "Invalid interface name '" << interface << "'";
- return false;
- }
-
- struct in_addr addr;
- if (inet_pton(AF_INET, dst_ip.c_str(), &addr) != 1) {
- LOG(ERROR) << "Invalid IPv4 destination address '" << dst_ip << "'";
- return false;
- }
-
- if (dst_port == 0U) {
- LOG(ERROR) << "Destination port 0 is not a valid port";
- return false;
- }
-
- // Order does not matter for the FORWARD chain: both -A or -I are possible.
- if (operation != "-A" && operation != "-I" && operation != "-D") {
- LOG(ERROR) << "Invalid chain operation '" << operation << "'";
- return false;
- }
-
- std::vector<std::string> argv{
- kIpTablesPath,
- "-t",
- "filter",
- operation,
- "FORWARD",
- "-i",
- interface,
- "-p", // protocol
- ProtocolName(protocol),
- "-d", // destination ip
- dst_ip,
- "--dport", // destination port
- std::to_string(dst_port),
- "-j",
- "ACCEPT",
- "-w"}; // Wait for xtables lock.
- return RunInMinijail(argv) == 0;
-}
-
-bool Firewall::AddLoopbackLockdownRules(Protocol protocol, uint16_t port) {
- if (port == 0U) {
- LOG(ERROR) << "Port 0 is not a valid port";
- return false;
- }
-
- if (!AddLoopbackLockdownRule(kIpTablesPath, protocol, port)) {
- LOG(ERROR) << "Could not add loopback REJECT rule using '" << kIpTablesPath
- << "'";
- return false;
- }
-
- if (!AddLoopbackLockdownRule(kIp6TablesPath, protocol, port)) {
- LOG(ERROR) << "Could not add loopback REJECT rule using '" << kIp6TablesPath
- << "', aborting operation";
- DeleteLoopbackLockdownRule(kIpTablesPath, protocol, port);
- return false;
- }
-
- return true;
-}
-
-bool Firewall::DeleteLoopbackLockdownRules(Protocol protocol, uint16_t port) {
- if (port == 0U) {
- LOG(ERROR) << "Port 0 is not a valid port";
- return false;
- }
-
- bool ip4_success = DeleteLoopbackLockdownRule(kIpTablesPath, protocol, port);
- bool ip6_success = DeleteLoopbackLockdownRule(kIp6TablesPath, protocol, port);
- return ip4_success && ip6_success;
-}
-
-bool Firewall::AddAcceptRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port,
- const std::string& interface) {
- std::vector<std::string> argv{executable_path,
- "-I", // insert
- "INPUT",
- "-p", // protocol
- ProtocolName(protocol),
- "--dport", // destination port
- std::to_string(port)};
- if (!interface.empty()) {
- argv.push_back("-i"); // interface
- argv.push_back(interface);
- }
- argv.push_back("-j");
- argv.push_back("ACCEPT");
- argv.push_back("-w"); // Wait for xtables lock.
-
- return RunInMinijail(argv) == 0;
-}
-
-bool Firewall::DeleteAcceptRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port,
- const std::string& interface) {
- std::vector<std::string> argv{executable_path,
- "-D", // delete
- "INPUT",
- "-p", // protocol
- ProtocolName(protocol),
- "--dport", // destination port
- std::to_string(port)};
- if (!interface.empty()) {
- argv.push_back("-i"); // interface
- argv.push_back(interface);
- }
- argv.push_back("-j");
- argv.push_back("ACCEPT");
- argv.push_back("-w"); // Wait for xtables lock.
-
- return RunInMinijail(argv) == 0;
-}
-
-bool Firewall::AddLoopbackLockdownRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port) {
- std::vector<std::string> argv{
- executable_path,
- "-I", // insert
- "OUTPUT",
- "-p", // protocol
- ProtocolName(protocol),
- "--dport", // destination port
- std::to_string(port),
- "-o", // output interface
- "lo",
- "-m", // match extension
- "owner",
- "!",
- "--uid-owner",
- "chronos",
- "-j",
- "REJECT",
- "-w", // Wait for xtables lock.
- };
-
- return RunInMinijail(argv) == 0;
-}
-
-bool Firewall::DeleteLoopbackLockdownRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port) {
- std::vector<std::string> argv{
- executable_path,
- "-D", // delete
- "OUTPUT",
- "-p", // protocol
- ProtocolName(protocol),
- "--dport", // destination port
- std::to_string(port),
- "-o", // output interface
- "lo",
- "-m", // match extension
- "owner",
- "!",
- "--uid-owner",
- "chronos",
- "-j",
- "REJECT",
- "-w", // Wait for xtables lock.
- };
-
- return RunInMinijail(argv) == 0;
-}
-
-int Firewall::RunInMinijail(const std::vector<std::string>& argv) {
- brillo::Minijail* m = brillo::Minijail::GetInstance();
- minijail* jail = m->New();
-
- std::vector<char*> args;
- for (const auto& arg : argv) {
- args.push_back(const_cast<char*>(arg.c_str()));
- }
- args.push_back(nullptr);
-
- int status;
- return m->RunSyncAndDestroy(jail, args, &status) ? status : -1;
-}
-
-} // namespace permission_broker
diff --git a/permission_broker/firewall.h b/permission_broker/firewall.h
deleted file mode 100644
index c04fb35..0000000
--- a/permission_broker/firewall.h
+++ /dev/null
@@ -1,102 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef PERMISSION_BROKER_FIREWALL_H_
-#define PERMISSION_BROKER_FIREWALL_H_
-
-#include <stdint.h>
-
-#include <set>
-#include <string>
-#include <utility>
-#include <vector>
-
-#include <base/macros.h>
-#include <brillo/errors/error.h>
-#include <gtest/gtest_prod.h>
-#include <patchpanel/proto_bindings/patchpanel_service.pb.h>
-
-namespace permission_broker {
-
-using patchpanel::ModifyPortRuleRequest;
-using Operation = patchpanel::ModifyPortRuleRequest::Operation;
-using Protocol = patchpanel::ModifyPortRuleRequest::Protocol;
-using RuleType = patchpanel::ModifyPortRuleRequest::RuleType;
-
-extern const char kIpTablesPath[];
-extern const char kIp6TablesPath[];
-
-const std::string ProtocolName(Protocol proto);
-
-class Firewall {
- public:
- typedef std::pair<uint16_t, std::string> Hole;
-
- Firewall() = default;
- ~Firewall() = default;
-
- bool AddAcceptRules(Protocol protocol,
- uint16_t port,
- const std::string& interface);
- bool DeleteAcceptRules(Protocol protocol,
- uint16_t port,
- const std::string& interface);
- bool AddLoopbackLockdownRules(Protocol protocol, uint16_t port);
- bool DeleteLoopbackLockdownRules(Protocol protocol, uint16_t port);
- bool AddIpv4ForwardRule(Protocol protocol,
- const std::string& input_ip,
- uint16_t port,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port);
- bool DeleteIpv4ForwardRule(Protocol protocol,
- const std::string& input_ip,
- uint16_t port,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port);
-
- private:
- friend class FirewallTest;
- // Adds ACCEPT chain rules to the filter INPUT chain.
- virtual bool AddAcceptRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port,
- const std::string& interface);
- // Removes ACCEPT chain rules from the filter INPUT chain.
- virtual bool DeleteAcceptRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port,
- const std::string& interface);
- // Adds or removes MASQUERADE chain rules to/from the nat PREROUTING chain.
- virtual bool ModifyIpv4DNATRule(Protocol protocol,
- const std::string& input_ip,
- uint16_t port,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port,
- const std::string& operation);
- // Adds or removes ACCEPT chain rules to/from the filter FORWARD chain.
- virtual bool ModifyIpv4ForwardChain(Protocol protocol,
- const std::string& interface,
- const std::string& dst_ip,
- uint16_t dst_port,
- const std::string& operation);
- virtual bool AddLoopbackLockdownRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port);
- virtual bool DeleteLoopbackLockdownRule(const std::string& executable_path,
- Protocol protocol,
- uint16_t port);
-
- // Even though permission_broker runs as a regular user, it can still add
- // other restrictions when launching 'iptables'.
- virtual int RunInMinijail(const std::vector<std::string>& argv);
-
- DISALLOW_COPY_AND_ASSIGN(Firewall);
-};
-
-} // namespace permission_broker
-
-#endif // PERMISSION_BROKER_FIREWALL_H_
diff --git a/permission_broker/firewall_fuzzer.cc b/permission_broker/firewall_fuzzer.cc
deleted file mode 100644
index 995459c..0000000
--- a/permission_broker/firewall_fuzzer.cc
+++ /dev/null
@@ -1,117 +0,0 @@
-// Copyright 2018 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <arpa/inet.h>
-#include <net/if.h>
-
-#include <fuzzer/FuzzedDataProvider.h>
-#include <set>
-
-#include "base/logging.h"
-
-#include "permission_broker/firewall.h"
-
-using patchpanel::ModifyPortRuleRequest;
-using Protocol = patchpanel::ModifyPortRuleRequest::Protocol;
-
-namespace permission_broker {
-
-class FakeFirewall : public Firewall {
- public:
- FakeFirewall() = default;
- ~FakeFirewall() = default;
-
- private:
- // The fake's implementation always succeeds.
- int RunInMinijail(const std::vector<std::string>& argv) override {
- return 0;
- }
-
- DISALLOW_COPY_AND_ASSIGN(FakeFirewall);
-};
-
-} // namespace permission_broker
-
-struct Environment {
- Environment() {
- logging::SetMinLogLevel(logging::LOG_FATAL);
- }
-};
-
-void FuzzAcceptRules(permission_broker::FakeFirewall& fake_firewall,
- const uint8_t* data,
- size_t size) {
- FuzzedDataProvider data_provider(data, size);
- while (data_provider.remaining_bytes() > 0) {
- Protocol proto = data_provider.ConsumeBool() ? ModifyPortRuleRequest::TCP
- : ModifyPortRuleRequest::UDP;
- uint16_t port = data_provider.ConsumeIntegral<uint16_t>();
- std::string iface = data_provider.ConsumeRandomLengthString(IFNAMSIZ - 1);
- if (data_provider.ConsumeBool()) {
- fake_firewall.AddAcceptRules(proto, port, iface);
- } else {
- fake_firewall.DeleteAcceptRules(proto, port, iface);
- }
- }
-}
-
-void FuzzForwardRules(permission_broker::FakeFirewall& fake_firewall,
- const uint8_t* data,
- size_t size) {
- FuzzedDataProvider data_provider(data, size);
- while (data_provider.remaining_bytes() > 0) {
- Protocol proto = data_provider.ConsumeBool() ? ModifyPortRuleRequest::TCP
- : ModifyPortRuleRequest::UDP;
- uint16_t forwarded_port = data_provider.ConsumeIntegral<uint16_t>();
- uint16_t dst_port = data_provider.ConsumeIntegral<uint16_t>();
- struct in_addr input_ip_addr = {
- .s_addr = data_provider.ConsumeIntegral<uint32_t>()};
- struct in_addr dst_ip_addr = {
- .s_addr = data_provider.ConsumeIntegral<uint32_t>()};
- char input_buffer[INET_ADDRSTRLEN];
- char dst_buffer[INET_ADDRSTRLEN];
- memset(input_buffer, 0, INET_ADDRSTRLEN);
- memset(dst_buffer, 0, INET_ADDRSTRLEN);
- inet_ntop(AF_INET, &input_ip_addr, input_buffer, INET_ADDRSTRLEN);
- inet_ntop(AF_INET, &dst_ip_addr, dst_buffer, INET_ADDRSTRLEN);
- std::string input_ip = input_buffer;
- std::string dst_ip = dst_buffer;
- std::string iface = data_provider.ConsumeRandomLengthString(IFNAMSIZ - 1);
- if (data_provider.ConsumeBool()) {
- fake_firewall.AddIpv4ForwardRule(proto, input_ip, forwarded_port, iface,
- dst_ip, dst_port);
- } else {
- fake_firewall.DeleteIpv4ForwardRule(proto, input_ip, forwarded_port,
- iface, dst_ip, dst_port);
- }
- }
-}
-
-void FuzzLoopbackLockdownRules(permission_broker::FakeFirewall& fake_firewall,
- const uint8_t* data,
- size_t size) {
- FuzzedDataProvider data_provider(data, size);
- while (data_provider.remaining_bytes() > 0) {
- Protocol proto = data_provider.ConsumeBool() ? ModifyPortRuleRequest::TCP
- : ModifyPortRuleRequest::UDP;
- uint16_t port = data_provider.ConsumeIntegral<uint16_t>();
- if (data_provider.ConsumeBool()) {
- fake_firewall.AddLoopbackLockdownRules(proto, port);
- } else {
- fake_firewall.DeleteLoopbackLockdownRules(proto, port);
- }
- }
-}
-
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
- static Environment env;
-
- permission_broker::FakeFirewall fake_firewall;
-
- FuzzAcceptRules(fake_firewall, data, size);
- FuzzForwardRules(fake_firewall, data, size);
- FuzzLoopbackLockdownRules(fake_firewall, data, size);
-
- return 0;
-}
diff --git a/permission_broker/firewall_test.cc b/permission_broker/firewall_test.cc
deleted file mode 100644
index 8c0829d..0000000
--- a/permission_broker/firewall_test.cc
+++ /dev/null
@@ -1,496 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "permission_broker/firewall.h"
-
-#include <string>
-#include <vector>
-
-#include <gtest/gtest.h>
-
-#include "permission_broker/mock_firewall.h"
-
-namespace permission_broker {
-
-using testing::_;
-using testing::Return;
-
-class FirewallTest : public testing::Test {
- public:
- FirewallTest() = default;
- ~FirewallTest() override = default;
-
- protected:
- void SetMockExpectations(MockFirewall* firewall, bool success) {
- // Empty criterion matches all commands.
- firewall->SetRunInMinijailFailCriterion(std::vector<std::string>(), true,
- success);
- }
-
- void SetMockExpectationsPerExecutable(MockFirewall* firewall,
- bool ip4_success,
- bool ip6_success) {
- if (!ip4_success)
- firewall->SetRunInMinijailFailCriterion(
- std::vector<std::string>({kIpTablesPath}), true /* repeat */,
- false /* omit_failure */);
- if (!ip6_success)
- firewall->SetRunInMinijailFailCriterion(
- std::vector<std::string>({kIp6TablesPath}), true, false);
- }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(FirewallTest);
-};
-
-TEST_F(FirewallTest, Port0Fails) {
- MockFirewall mock_firewall;
- // Don't fail on anything.
- int id = mock_firewall.SetRunInMinijailFailCriterion(
- std::vector<std::string>(), true, true);
-
- // Try to punch hole for TCP port 0, port 0 is not a valid port.
- EXPECT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 0, "iface"));
- // Try to punch hole for UDP port 0, port 0 is not a valid port.
- EXPECT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 0, "iface"));
-
- // Try to plug hole for TCP port 0, port 0 is not a valid port.
- EXPECT_FALSE(
- mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 0, "iface"));
- // Try to plug hole for UDP port 0, port 0 is not a valid port.
- EXPECT_FALSE(
- mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 0, "iface"));
-
- // We should not be adding/removing any rules for port 0.
- EXPECT_EQ(mock_firewall.GetRunInMinijailCriterionMatchCount(id), 0);
-}
-
-TEST_F(FirewallTest, ValidInterfaceName) {
- MockFirewall mock_firewall;
- SetMockExpectations(&mock_firewall, true /* success */);
-
- EXPECT_TRUE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "shortname"));
- EXPECT_TRUE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "shortname"));
- EXPECT_TRUE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "middle-dash"));
- EXPECT_TRUE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "middle-dash"));
- EXPECT_TRUE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "middle.dot"));
- EXPECT_TRUE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "middle.dot"));
-}
-
-TEST_F(FirewallTest, InvalidInterfaceName) {
- MockFirewall mock_firewall;
- int id = mock_firewall.SetRunInMinijailFailCriterion(
- std::vector<std::string>(), true, true);
-
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "reallylonginterfacename"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "with spaces"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "with$ymbols"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "-startdash"));
- EXPECT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80, "enddash-"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80,
- ".startdot"));
- EXPECT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80, "enddot."));
-
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "reallylonginterfacename"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "with spaces"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "with$ymbols"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "-startdash"));
- EXPECT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53, "enddash-"));
- EXPECT_FALSE(mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53,
- ".startdot"));
- EXPECT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53, "enddot."));
-
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "reallylonginterfacename"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "with spaces"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "with$ymbols"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "-startdash"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "enddash-"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- ".startdot"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::TCP, 80,
- "enddot."));
-
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "reallylonginterfacename"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "with spaces"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "with$ymbols"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "-startdash"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "enddash-"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- ".startdot"));
- EXPECT_FALSE(mock_firewall.DeleteAcceptRules(ModifyPortRuleRequest::UDP, 53,
- "enddot."));
-
- // We should not be adding/removing any rules for invalid interface names.
- EXPECT_EQ(mock_firewall.GetRunInMinijailCriterionMatchCount(id), 0);
-}
-
-TEST_F(FirewallTest, AddAcceptRules_Fails) {
- MockFirewall mock_firewall;
- SetMockExpectations(&mock_firewall, false /* success */);
-
- // Punch hole for TCP port 80, should fail.
- ASSERT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80, "iface"));
- // Punch hole for UDP port 53, should fail.
- ASSERT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53, "iface"));
-}
-
-TEST_F(FirewallTest, AddAcceptRules_Ipv6Fails) {
- MockFirewall mock_firewall;
- SetMockExpectationsPerExecutable(&mock_firewall, true /* ip4_success */,
- false /* ip6_success */);
-
- // Punch hole for TCP port 80, should fail because 'ip6tables' fails.
- ASSERT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::TCP, 80, "iface"));
- // Punch hole for UDP port 53, should fail because 'ip6tables' fails.
- ASSERT_FALSE(
- mock_firewall.AddAcceptRules(ModifyPortRuleRequest::UDP, 53, "iface"));
-}
-
-TEST_F(FirewallTest, Port0LockdownFails) {
- MockFirewall mock_firewall;
- // Don't fail on anything.
- int id = mock_firewall.SetRunInMinijailFailCriterion(
- std::vector<std::string>(), true, true);
-
- // Try to lock down TCP port 0, port 0 is not a valid port.
- EXPECT_FALSE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::TCP, 0));
- // Try to lock down UDP port 0, port 0 is not a valid port.
- EXPECT_FALSE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::UDP, 0));
-
- // We should not be adding/removing any rules for port 0.
- EXPECT_EQ(mock_firewall.GetRunInMinijailCriterionMatchCount(id), 0);
-}
-
-TEST_F(FirewallTest, AddLoopbackLockdownRules_Success) {
- MockFirewall mock_firewall;
- SetMockExpectations(&mock_firewall, true /* success */);
- ASSERT_TRUE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::TCP, 80));
- ASSERT_TRUE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::UDP, 53));
- ASSERT_TRUE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::TCP, 1234));
- ASSERT_TRUE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::TCP, 8080));
-}
-
-TEST_F(FirewallTest, AddLoopbackLockdownRules_Fails) {
- MockFirewall mock_firewall;
- SetMockExpectations(&mock_firewall, false /* success */);
-
- // Lock down TCP port 80, should fail.
- ASSERT_FALSE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::TCP, 80));
- // Lock down UDP port 53, should fail.
- ASSERT_FALSE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::UDP, 53));
-}
-
-TEST_F(FirewallTest, AddLoopbackLockdownRules_Ipv6Fails) {
- MockFirewall mock_firewall;
- SetMockExpectationsPerExecutable(&mock_firewall, true /* ip4_success */,
- false /* ip6_success */);
-
- // Lock down TCP port 80, should fail because 'ip6tables' fails.
- ASSERT_FALSE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::TCP, 80));
- // Lock down UDP port 53, should fail because 'ip6tables' fails.
- ASSERT_FALSE(
- mock_firewall.AddLoopbackLockdownRules(ModifyPortRuleRequest::UDP, 53));
-}
-
-TEST_F(FirewallTest, AddIpv4ForwardRules_InvalidArguments) {
- MockFirewall mock_firewall;
- // Don't fail on anything.
- mock_firewall.SetRunInMinijailFailCriterion({}, true, true);
-
- // Invalid input interface. No iptables commands are issued.
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "-startdash", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "enddash-", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, ".startdot", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "enddot.", "100.115.92.5", 8080));
- ASSERT_TRUE(mock_firewall.GetAllCommands().empty());
- mock_firewall.ResetStoredCommands();
-
- // Empty interface. No iptables commands are issused.
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(ModifyPortRuleRequest::TCP, "",
- 80, "", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(ModifyPortRuleRequest::UDP, "",
- 80, "", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "", "100.115.92.5", 8080));
- ASSERT_TRUE(mock_firewall.GetAllCommands().empty());
- mock_firewall.ResetStoredCommands();
-
- // Invalid input dst address. No iptables commands are issused.
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(ModifyPortRuleRequest::TCP,
- "256.256.256.256", 80, "iface",
- "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(ModifyPortRuleRequest::UDP,
- "qodpjqwpod", 80, "iface",
- "100.115.92.5", 8080));
- // Trying to delete an IPv4 forward rule with an invalid input address will
- // still trigger an explicit iptables -D command for the associated FORWARD
- // ACCEPT rule. Two such commands are expected.
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "1.1", 80, "iface", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(ModifyPortRuleRequest::UDP,
- "2001:db8::1", 80, "iface",
- "100.115.92.5", 8080));
- {
- std::vector<std::string> expected_commands = {
- "/sbin/iptables -t filter -D FORWARD -i iface -p tcp -d 100.115.92.5 "
- "--dport 8080 -j ACCEPT -w",
- "/sbin/iptables -t filter -D FORWARD -i iface -p udp -d 100.115.92.5 "
- "--dport 8080 -j ACCEPT -w",
- };
- std::vector<std::string> issued_commands = mock_firewall.GetAllCommands();
- EXPECT_EQ(expected_commands.size(), issued_commands.size());
- for (int i = 0; i < expected_commands.size(); i++) {
- ASSERT_EQ(expected_commands[i], issued_commands[i]);
- }
- }
- mock_firewall.ResetStoredCommands();
-
- // Invalid input dst port.
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 0, "iface", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 0, "iface", "100.115.92.5", 8080));
- // Trying to delete an IPv4 forward rule with an invalid input port will
- // still trigger an explicit iptables -D command for the associated FORWARD
- // ACCEPT rule. Two such commands are expected.
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 0, "iface", "100.115.92.5", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 0, "iface", "100.115.92.5", 8080));
- {
- std::vector<std::string> expected_commands = {
- "/sbin/iptables -t filter -D FORWARD -i iface -p tcp -d 100.115.92.5 "
- "--dport 8080 -j ACCEPT -w",
- "/sbin/iptables -t filter -D FORWARD -i iface -p udp -d 100.115.92.5 "
- "--dport 8080 -j ACCEPT -w",
- };
- std::vector<std::string> issued_commands = mock_firewall.GetAllCommands();
- EXPECT_EQ(expected_commands.size(), issued_commands.size());
- for (int i = 0; i < expected_commands.size(); i++) {
- ASSERT_EQ(expected_commands[i], issued_commands[i]);
- }
- }
- mock_firewall.ResetStoredCommands();
-
- // Invalid output dst address. No iptables commands are issused.
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(ModifyPortRuleRequest::TCP, "",
- 80, "iface", "", 8080));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "iface", "qodpjqwpod", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "iface", "1.1", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "iface", "2001:db8::1", 8080));
- ASSERT_TRUE(mock_firewall.GetAllCommands().empty());
- mock_firewall.ResetStoredCommands();
-
- // Invalid output dst port. No iptables commands are issused.
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "iface", "100.115.92.5", 0));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "iface", "100.115.92.5", 0));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "iface", "100.115.92.5", 0));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "iface", "100.115.92.5", 0));
- ASSERT_TRUE(mock_firewall.GetAllCommands().empty());
- mock_firewall.ResetStoredCommands();
-}
-
-TEST_F(FirewallTest, AddIpv4ForwardRules_IptablesFails) {
- MockFirewall mock_firewall;
- mock_firewall.SetRunInMinijailFailCriterion({}, true, false);
-
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "iface", "100.115.92.6", 8080));
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "iface", "100.115.92.6", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "iface", "100.115.92.6", 8080));
- ASSERT_FALSE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 80, "iface", "100.115.92.6", 8080));
-
- // AddIpv4ForwardRule: Firewall should return at the first failure and issue
- // only a single command.
- // DeleteIpv4ForwardRule: Firewall should try to delete both the DNAT rule
- // and the FORWARD rule regardless of iptables failures.
- std::vector<std::string> expected_commands = {
- // Failed first AddIpv4ForwardRule call
- "/sbin/iptables -t nat -I PREROUTING -i iface -p tcp --dport 80 -j DNAT "
- "--to-destination 100.115.92.6:8080 -w",
- // Failed second AddIpv4ForwardRule call
- "/sbin/iptables -t nat -I PREROUTING -i iface -p udp --dport 80 -j DNAT "
- "--to-destination 100.115.92.6:8080 -w",
- // Failed first DeleteIpv4ForwardRule call
- "/sbin/iptables -t nat -D PREROUTING -i iface -p tcp --dport 80 -j DNAT "
- "--to-destination 100.115.92.6:8080 -w",
- "/sbin/iptables -t filter -D FORWARD -i iface -p tcp -d 100.115.92.6 "
- "--dport 8080 -j ACCEPT -w",
- // Failed second DeleteIpv4ForwardRule call
- "/sbin/iptables -t nat -D PREROUTING -i iface -p udp --dport 80 -j DNAT "
- "--to-destination 100.115.92.6:8080 -w",
- "/sbin/iptables -t filter -D FORWARD -i iface -p udp -d 100.115.92.6 "
- "--dport 8080 -j ACCEPT -w",
- };
- std::vector<std::string> issued_commands = mock_firewall.GetAllCommands();
- EXPECT_EQ(expected_commands.size(), issued_commands.size());
- for (int i = 0; i < expected_commands.size(); i++) {
- ASSERT_EQ(expected_commands[i], issued_commands[i]);
- }
-}
-
-TEST_F(FirewallTest, AddIpv4ForwardRules_ValidRules) {
- MockFirewall mock_firewall;
- mock_firewall.SetRunInMinijailFailCriterion({}, true, true);
-
- ASSERT_TRUE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "wlan0", "100.115.92.2", 8080));
- ASSERT_TRUE(mock_firewall.AddIpv4ForwardRule(ModifyPortRuleRequest::TCP,
- "100.115.92.2", 5555, "vmtap0",
- "127.0.0.1", 5550));
- ASSERT_TRUE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 5353, "eth0", "192.168.1.1", 5353));
- ASSERT_TRUE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 5000, "mlan0", "10.0.0.24", 5001));
- ASSERT_TRUE(mock_firewall.DeleteIpv4ForwardRule(ModifyPortRuleRequest::TCP,
- "100.115.92.2", 5555,
- "vmtap0", "127.0.0.1", 5550));
- ASSERT_TRUE(mock_firewall.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::UDP, "", 443, "eth1", "1.2.3.4", 443));
-
- std::vector<std::string> expected_commands = {
- "/sbin/iptables -t nat -I PREROUTING -i wlan0 -p tcp --dport 80 -j DNAT "
- "--to-destination 100.115.92.2:8080 -w",
- "/sbin/iptables -t filter -A FORWARD -i wlan0 -p tcp -d 100.115.92.2 "
- "--dport 8080 -j ACCEPT -w",
- "/sbin/iptables -t nat -I PREROUTING -i vmtap0 -p tcp -d 100.115.92.2 "
- "--dport 5555 -j DNAT --to-destination 127.0.0.1:5550 -w",
- "/sbin/iptables -t filter -A FORWARD -i vmtap0 -p tcp -d 127.0.0.1 "
- "--dport 5550 -j ACCEPT -w",
- "/sbin/iptables -t nat -I PREROUTING -i eth0 -p udp --dport 5353 -j DNAT "
- "--to-destination 192.168.1.1:5353 -w",
- "/sbin/iptables -t filter -A FORWARD -i eth0 -p udp -d 192.168.1.1 "
- "--dport 5353 -j ACCEPT -w",
- "/sbin/iptables -t nat -D PREROUTING -i mlan0 -p tcp --dport 5000 -j "
- "DNAT --to-destination 10.0.0.24:5001 -w",
- "/sbin/iptables -t filter -D FORWARD -i mlan0 -p tcp -d 10.0.0.24 "
- "--dport 5001 -j ACCEPT -w",
- "/sbin/iptables -t nat -D PREROUTING -i vmtap0 -p tcp -d 100.115.92.2 "
- "--dport 5555 -j DNAT --to-destination 127.0.0.1:5550 -w",
- "/sbin/iptables -t filter -D FORWARD -i vmtap0 -p tcp -d 127.0.0.1 "
- "--dport 5550 -j ACCEPT -w",
- "/sbin/iptables -t nat -D PREROUTING -i eth1 -p udp --dport 443 -j DNAT "
- "--to-destination 1.2.3.4:443 -w",
- "/sbin/iptables -t filter -D FORWARD -i eth1 -p udp -d 1.2.3.4 "
- "--dport 443 -j ACCEPT -w",
- };
- std::vector<std::string> issued_commands = mock_firewall.GetAllCommands();
- EXPECT_EQ(expected_commands.size(), issued_commands.size());
- for (int i = 0; i < expected_commands.size(); i++) {
- ASSERT_EQ(expected_commands[i], issued_commands[i]);
- }
-}
-
-TEST_F(FirewallTest, AddIpv4ForwardRules_PartialFailure) {
- MockFirewall mock_firewall;
- mock_firewall.SetRunInMinijailFailCriterion({"FORWARD"}, true, false);
-
- ASSERT_FALSE(mock_firewall.AddIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "wlan0", "100.115.92.2", 8080));
-
- // When the second issued FORWARD command fails, expect a delete command to
- // cleanup the PREROUTING command that succeeded.
- std::vector<std::string> expected_commands = {
- "/sbin/iptables -t nat -I PREROUTING -i wlan0 -p tcp --dport 80 -j DNAT "
- "--to-destination 100.115.92.2:8080 -w",
- "/sbin/iptables -t filter -A FORWARD -i wlan0 -p tcp -d 100.115.92.2 "
- "--dport 8080 -j ACCEPT -w",
- "/sbin/iptables -t nat -D PREROUTING -i wlan0 -p tcp --dport 80 -j DNAT "
- "--to-destination 100.115.92.2:8080 -w",
- };
- std::vector<std::string> issued_commands = mock_firewall.GetAllCommands();
- ASSERT_EQ(expected_commands.size(), issued_commands.size());
- for (int i = 0; i < expected_commands.size(); i++) {
- ASSERT_EQ(expected_commands[i], issued_commands[i]);
- }
-}
-
-TEST_F(FirewallTest, DeleteIpv4ForwardRules_PartialFailure) {
- MockFirewall mock_firewall1;
- MockFirewall mock_firewall2;
-
- mock_firewall1.SetRunInMinijailFailCriterion({"FORWARD"}, false, false);
- mock_firewall2.SetRunInMinijailFailCriterion({"PREROUTING"}, false, false);
-
- ASSERT_FALSE(mock_firewall1.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "wlan0", "100.115.92.2", 8080));
- ASSERT_FALSE(mock_firewall2.DeleteIpv4ForwardRule(
- ModifyPortRuleRequest::TCP, "", 80, "wlan0", "100.115.92.2", 8080));
-
- // Cleanup commands for both FORWARD and PREROUTING rules are both issued
- // regardless of any iptables failures.
- std::vector<std::string> expected_commands = {
- "/sbin/iptables -t nat -D PREROUTING -i wlan0 -p tcp --dport 80 -j DNAT "
- "--to-destination 100.115.92.2:8080 -w",
- "/sbin/iptables -t filter -D FORWARD -i wlan0 -p tcp -d 100.115.92.2 "
- "--dport 8080 -j ACCEPT -w",
- };
- std::vector<std::string> issued_commands1 = mock_firewall1.GetAllCommands();
- std::vector<std::string> issued_commands2 = mock_firewall2.GetAllCommands();
- ASSERT_EQ(expected_commands.size(), issued_commands1.size());
- ASSERT_EQ(expected_commands.size(), issued_commands2.size());
- for (int i = 0; i < expected_commands.size(); i++) {
- ASSERT_EQ(expected_commands[i], issued_commands1[i]);
- ASSERT_EQ(expected_commands[i], issued_commands2[i]);
- }
-}
-} // namespace permission_broker
diff --git a/permission_broker/mock_firewall.cc b/permission_broker/mock_firewall.cc
deleted file mode 100644
index f7dd6d9..0000000
--- a/permission_broker/mock_firewall.cc
+++ /dev/null
@@ -1,140 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "permission_broker/mock_firewall.h"
-
-#include <algorithm>
-
-namespace permission_broker {
-
-int MockFirewall::SetRunInMinijailFailCriterion(
- const std::vector<std::string>& keywords, bool repeat, bool omit_failure) {
- match_criteria_.push_back(Criterion{keywords, repeat, omit_failure, 0});
- return match_criteria_.size() - 1;
-}
-
-int MockFirewall::GetRunInMinijailCriterionMatchCount(int id) {
- if (id < 0 || id >= static_cast<int>(match_criteria_.size()))
- return -1;
- return match_criteria_[id].match_count;
-}
-
-bool MockFirewall::MatchAndUpdate(const std::vector<std::string>& argv) {
- // Make a copy of the container so that we can delete elements while
- // iterating.
- auto criteria = match_criteria_;
- for (auto& criterion : criteria) {
- bool match = true;
- // Empty criterion is a catch all -- fail on any RunInMinijail.
- for (const std::string& keyword : criterion.keywords) {
- match =
- match && (std::find(argv.begin(), argv.end(), keyword) != argv.end());
- }
- if (match) {
- criterion.match_count++;
- if (!criterion.repeat) {
- match_criteria_.erase(std::remove(match_criteria_.begin(),
- match_criteria_.end(), criterion),
- match_criteria_.end());
- }
- // If not negating, treat as fail criterion,
- // otherwise ignore (return false).
- return !criterion.omit_failure;
- }
- }
- return false;
-}
-
-std::vector<std::string> MockFirewall::GetAllCommands() {
- std::vector<std::string> commands;
- commands.reserve(commands_.size());
- for (const auto& argv : commands_) {
- std::string joined_argv = "";
- std::string separator = "";
- for (const auto& arg : argv) {
- joined_argv += separator + arg;
- separator = " ";
- }
- commands.push_back(joined_argv);
- }
- return commands;
-}
-
-int MockFirewall::RunInMinijail(const std::vector<std::string>& argv) {
- commands_.push_back(argv);
- return MatchAndUpdate(argv) ? 1 : 0;
-}
-
-// Returns the inverse of a given command. For an insert or append returns a
-// command to delete that rule, for a deletion returns a command to insert it at
-// the start. Assumes that the inverse of -D is always -I and that -I|--insert
-// is used without index arguments. This holds as of 2019-11-20 but if you start
-// using them you'll need to update this method.
-std::vector<std::string> MockFirewall::GetInverseCommand(
- const std::vector<std::string>& argv) {
- std::vector<std::string> inverse;
- if (argv.size() == 0) {
- return inverse;
- }
-
- bool isIpTablesCommand = argv[0] == kIpTablesPath;
- for (const auto& arg : argv) {
- if (isIpTablesCommand && (arg == "-I" || arg == "-A" || arg == "--insert" ||
- arg == "--append")) {
- inverse.push_back("-D");
- } else if (isIpTablesCommand && arg == "-D") {
- inverse.push_back("-I");
- } else {
- inverse.push_back(arg);
- }
- }
- return inverse;
-}
-
-// This check does not enforce ordering. It only checks
-// that for each command that adds a rule/mark with
-// ip/iptables, there is a later match command that
-// deletes that same rule/mark.
-bool MockFirewall::CheckCommandsUndone() {
- return CountActiveCommands() == 0;
-}
-
-// For each command, if it's an insert or an append it checks if there's a
-// corresponding delete later on, then it returns a count of all rules without
-// deletes. Will skip any rule that's not an append or insert e.g. delete
-// without an insert first will return 0 not -1.
-int MockFirewall::CountActiveCommands() {
- int count = 0;
- for (std::vector<std::vector<std::string>>::iterator it = commands_.begin();
- it != commands_.end(); it++) {
- // For each command that adds a rule, check that its inverse
- // exists later in the log of commands.
- if ((std::find(it->begin(), it->end(), "-A") != it->end()) ||
- (std::find(it->begin(), it->end(), "--append") != it->end()) ||
- (std::find(it->begin(), it->end(), "-I") != it->end()) ||
- (std::find(it->begin(), it->end(), "--insert") != it->end())) {
- bool found = false;
- std::vector<std::string> inverse = GetInverseCommand(*it);
- // If GetInverseCommand returns the same command, then it was
- // not an ip/iptables command that added/removed a rule/mark.
- if (std::equal(it->begin(), it->end(), inverse.begin()))
- continue;
- for (auto it_next = it + 1; it_next != commands_.end(); it_next++) {
- if (*it_next == inverse) {
- found = true;
- break;
- }
- }
- if (!found)
- count++;
- }
- }
- return count;
-}
-
-void MockFirewall::ResetStoredCommands() {
- commands_.clear();
-}
-
-} // namespace permission_broker
diff --git a/permission_broker/mock_firewall.h b/permission_broker/mock_firewall.h
deleted file mode 100644
index 97e6f76..0000000
--- a/permission_broker/mock_firewall.h
+++ /dev/null
@@ -1,78 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef PERMISSION_BROKER_MOCK_FIREWALL_H_
-#define PERMISSION_BROKER_MOCK_FIREWALL_H_
-
-#include <string>
-#include <vector>
-
-#include <base/macros.h>
-#include <gmock/gmock.h>
-
-#include "permission_broker/firewall.h"
-
-namespace permission_broker {
-
-class MockFirewall : public Firewall {
- public:
- MockFirewall() = default;
- ~MockFirewall() = default;
-
- int SetRunInMinijailFailCriterion(
- const std::vector<std::string>& keywords, bool repeat, bool omit_failure);
- int GetRunInMinijailCriterionMatchCount(int id);
- bool CheckCommandsUndone();
- int CountActiveCommands();
- void ResetStoredCommands();
-
- // Check if the current command matches a failure rule,
- // if the failure rule is not a repeat rule, remove it
- // from the match criteria.
- bool MatchAndUpdate(const std::vector<std::string>& argv);
-
- // Returns all commands issued with RunInMinijail during the test.
- std::vector<std::string> GetAllCommands();
-
- private:
- struct Criterion {
- std::vector<std::string> keywords;
- // If false, remove the criterion after it's matched.
- bool repeat;
- // If false, treat matching commands as failures, otherwise,
- // omit failing.
- bool omit_failure;
- // Count the number of times the criterion is matched.
- int match_count;
-
- // Don't take into account match_count.
- bool operator==(const Criterion& c) const {
- return (std::equal(keywords.begin(), keywords.end(),
- c.keywords.begin()) &&
- (repeat == c.repeat) &&
- (c.omit_failure == omit_failure));
- }
- };
-
- // A list of collections of keywords that a command
- // must have in order for it to fail.
- std::vector<Criterion> match_criteria_;
-
- // A log of commands issued with RunInMinijail during the test.
- std::vector<std::vector<std::string>> commands_;
-
- // The mock's implementation simply logs the command issued if
- // successful.
- int RunInMinijail(const std::vector<std::string>& argv) override;
-
- // Given an ip/iptables command, return the command that undoes its effect.
- std::vector<std::string> GetInverseCommand(
- const std::vector<std::string>& command);
-
- DISALLOW_COPY_AND_ASSIGN(MockFirewall);
-};
-
-} // namespace permission_broker
-
-#endif // PERMISSION_BROKER_MOCK_FIREWALL_H_
diff --git a/permission_broker/permission_broker.cc b/permission_broker/permission_broker.cc
index 0c95417..19d0f74 100644
--- a/permission_broker/permission_broker.cc
+++ b/permission_broker/permission_broker.cc
@@ -72,7 +72,7 @@
rule_engine_(udev_run_path, poll_interval),
dbus_object_(
nullptr, bus, dbus::ObjectPath(kPermissionBrokerServicePath)),
- port_tracker_(&firewall_),
+ port_tracker_(),
usb_control_(std::make_unique<UsbDeviceManager>()),
session_manager_proxy_(
new org::chromium::SessionManagerInterfaceProxy(bus)) {
diff --git a/permission_broker/permission_broker.h b/permission_broker/permission_broker.h
index e378fa0..9c93414 100644
--- a/permission_broker/permission_broker.h
+++ b/permission_broker/permission_broker.h
@@ -19,7 +19,6 @@
#include <session_manager/dbus-proxies.h>
#include "permission_broker/dbus_adaptors/org.chromium.PermissionBroker.h"
-#include "permission_broker/firewall.h"
#include "permission_broker/port_tracker.h"
#include "permission_broker/rule_engine.h"
#include "permission_broker/usb_control.h"
@@ -97,7 +96,6 @@
RuleEngine rule_engine_;
brillo::dbus_utils::DBusObject dbus_object_;
- Firewall firewall_;
PortTracker port_tracker_;
UsbControl usb_control_;
UsbDriverTracker usb_driver_tracker_;
diff --git a/permission_broker/port_tracker.cc b/permission_broker/port_tracker.cc
index 7626d53..cf6e36e 100644
--- a/permission_broker/port_tracker.cc
+++ b/permission_broker/port_tracker.cc
@@ -59,6 +59,13 @@
constexpr const uint16_t kAdbServerPort = 5555;
constexpr const uint16_t kAdbProxyPort = 5550;
+const std::string ProtocolName(Protocol proto) {
+ if (proto == ModifyPortRuleRequest::INVALID_PROTOCOL) {
+ NOTREACHED() << "Unexpected L4 protocol value";
+ }
+ return base::ToLowerASCII(ModifyPortRuleRequest::Protocol_Name(proto));
+}
+
std::string RuleTypeName(PortTracker::PortRuleType type) {
switch (type) {
case PortTracker::kUnknownRule:
@@ -95,15 +102,13 @@
}
} // namespace
-PortTracker::PortTracker(Firewall* firewall)
+PortTracker::PortTracker()
: task_runner_{base::ThreadTaskRunnerHandle::Get()},
- epfd_{kInvalidHandle},
- firewall_{firewall} {}
+ epfd_{kInvalidHandle} {}
// Test-only.
-PortTracker::PortTracker(scoped_refptr<base::SequencedTaskRunner> task_runner,
- Firewall* firewall)
- : task_runner_{task_runner}, epfd_{kInvalidHandle}, firewall_{firewall} {}
+PortTracker::PortTracker(scoped_refptr<base::SequencedTaskRunner> task_runner)
+ : task_runner_{task_runner}, epfd_{kInvalidHandle} {}
PortTracker::~PortTracker() {
RevokeAllPortRules();
@@ -224,28 +229,7 @@
port_rules_[key].lifeline_fd = lifeline_fd;
lifeline_fds_[lifeline_fd] = key;
- bool success = false;
- switch (rule.type) {
- case kAccessRule:
- success = firewall_->AddAcceptRules(rule.proto, rule.input_dst_port,
- rule.input_ifname);
- break;
- case kLockdownRule:
- success =
- firewall_->AddLoopbackLockdownRules(rule.proto, rule.input_dst_port);
- break;
- case kForwardingRule:
- case kAdbForwardingRule:
- success = firewall_->AddIpv4ForwardRule(
- rule.proto, rule.input_dst_ip, rule.input_dst_port, rule.input_ifname,
- rule.dst_ip, rule.dst_port);
- break;
- default:
- LOG(ERROR) << "Unknown port rule type " << rule.type;
- break;
- }
-
- if (!success) {
+ if (!ModifyPortRule(ModifyPortRuleRequest::CREATE, rule)) {
// If we fail to punch the hole in the firewall, stop tracking the lifetime
// of the process.
LOG(ERROR) << "Failed to create rule " << rule;
@@ -572,22 +556,7 @@
}
port_rules_.erase(key);
lifeline_fds_.erase(rule.lifeline_fd);
- switch (rule.type) {
- case kAccessRule:
- return deleted && firewall_->DeleteAcceptRules(
- key.proto, key.input_dst_port, key.input_ifname);
- case kLockdownRule:
- return deleted && firewall_->DeleteLoopbackLockdownRules(
- key.proto, key.input_dst_port);
- case kForwardingRule:
- case kAdbForwardingRule:
- return deleted && firewall_->DeleteIpv4ForwardRule(
- rule.proto, rule.input_dst_ip, rule.input_dst_port,
- rule.input_ifname, rule.dst_ip, rule.dst_port);
- default:
- LOG(ERROR) << "Unknown port rule entry type " << rule.type;
- return false;
- }
+ return deleted && ModifyPortRule(ModifyPortRuleRequest::DELETE, rule);
}
bool PortTracker::InitializeEpollOnce() {
diff --git a/permission_broker/port_tracker.h b/permission_broker/port_tracker.h
index 2c0c510..9d50b1d 100644
--- a/permission_broker/port_tracker.h
+++ b/permission_broker/port_tracker.h
@@ -16,10 +16,13 @@
#include <base/sequenced_task_runner.h>
#include <patchpanel/proto_bindings/patchpanel_service.pb.h>
-#include "permission_broker/firewall.h"
-
namespace permission_broker {
+using patchpanel::ModifyPortRuleRequest;
+using Operation = patchpanel::ModifyPortRuleRequest::Operation;
+using Protocol = patchpanel::ModifyPortRuleRequest::Protocol;
+using RuleType = patchpanel::ModifyPortRuleRequest::RuleType;
+
class PortTracker {
public:
struct PortRuleKey {
@@ -71,7 +74,7 @@
uint16_t dst_port;
};
- explicit PortTracker(Firewall* firewall);
+ PortTracker();
virtual ~PortTracker();
bool AllowTcpPortAccess(uint16_t port, const std::string& iface, int dbus_fd);
@@ -103,8 +106,7 @@
void RevokeAllPortRules();
protected:
- PortTracker(scoped_refptr<base::SequencedTaskRunner> task_runner,
- Firewall* firewall);
+ explicit PortTracker(scoped_refptr<base::SequencedTaskRunner> task_runner);
private:
// Call patchpanel's DBus API to create or remove firewall rule.
@@ -138,10 +140,6 @@
// it requested.
std::map<int, PortRuleKey> lifeline_fds_;
- // |firewall_| is owned by the PermissionBroker object owning this instance
- // of PortTracker.
- Firewall* firewall_;
-
DISALLOW_COPY_AND_ASSIGN(PortTracker);
};
diff --git a/permission_broker/port_tracker_fuzzer.cc b/permission_broker/port_tracker_fuzzer.cc
index 5feabab..a38c7e7 100644
--- a/permission_broker/port_tracker_fuzzer.cc
+++ b/permission_broker/port_tracker_fuzzer.cc
@@ -14,24 +14,15 @@
namespace permission_broker {
-class FakeFirewall : public Firewall {
- public:
- FakeFirewall() = default;
- ~FakeFirewall() = default;
-
- private:
- // The fake's implementation always succeeds.
- int RunInMinijail(const std::vector<std::string>& argv) override { return 0; }
-
- DISALLOW_COPY_AND_ASSIGN(FakeFirewall);
-};
-
class FakePortTracker : public PortTracker {
public:
- explicit FakePortTracker(Firewall* firewall)
- : PortTracker(nullptr, firewall), next_fd_{1} {}
+ FakePortTracker() : PortTracker(nullptr), next_fd_{1} {}
~FakePortTracker() override = default;
+ bool ModifyPortRule(patchpanel::ModifyPortRuleRequest::Operation,
+ const PortRule& rule) override {
+ return true;
+ }
int AddLifelineFd(int dbus_fd) override { return next_fd_++; }
bool DeleteLifelineFd(int fd) override { return true; }
void CheckLifelineFds(bool reschedule_check) override {}
@@ -155,8 +146,7 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
static Environment env;
- permission_broker::FakeFirewall firewall;
- permission_broker::FakePortTracker port_tracker(&firewall);
+ permission_broker::FakePortTracker port_tracker;
std::set<permission_broker::FuzzRequest> existing_rules;
FuzzedDataProvider provider(data, size);
diff --git a/permission_broker/port_tracker_test.cc b/permission_broker/port_tracker_test.cc
index 2fb5e13..f5a344d 100644
--- a/permission_broker/port_tracker_test.cc
+++ b/permission_broker/port_tracker_test.cc
@@ -9,8 +9,6 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
-#include "permission_broker/mock_firewall.h"
-
using ::testing::_;
using ::testing::Return;
using ::testing::SetArgPointee;
@@ -19,10 +17,14 @@
class MockPortTracker : public PortTracker {
public:
- explicit MockPortTracker(Firewall* firewall)
- : PortTracker(nullptr, firewall) {}
+ MockPortTracker() : PortTracker(nullptr) {}
~MockPortTracker() override = default;
+ MOCK_METHOD(bool,
+ ModifyPortRule,
+ (patchpanel::ModifyPortRuleRequest::Operation, const PortRule&),
+ (override));
+
MOCK_METHOD(int, AddLifelineFd, (int), (override));
MOCK_METHOD(bool, DeleteLifelineFd, (int), (override));
MOCK_METHOD(void, CheckLifelineFds, (bool), (override));
@@ -36,29 +38,10 @@
class PortTrackerTest : public testing::Test {
public:
- PortTrackerTest() : port_tracker_{&mock_firewall_} {}
+ PortTrackerTest() = default;
~PortTrackerTest() override = default;
protected:
- void SetMockExpectations(MockFirewall* firewall, bool success) {
- // Empty criterion matches all commands.
- firewall->SetRunInMinijailFailCriterion(std::vector<std::string>(), true,
- success);
- }
-
- void SetMockExpectationsPerExecutable(MockFirewall* firewall,
- bool ip4_success,
- bool ip6_success) {
- if (!ip4_success)
- firewall->SetRunInMinijailFailCriterion(
- std::vector<std::string>({kIpTablesPath}), true /* repeat */,
- false /* omit_failure */);
- if (!ip6_success)
- firewall->SetRunInMinijailFailCriterion(
- std::vector<std::string>({kIp6TablesPath}), true, false);
- }
-
- MockFirewall mock_firewall_;
MockPortTracker port_tracker_;
uint16_t tcp_port = 8080;
@@ -81,21 +64,21 @@
};
TEST_F(PortTrackerTest, AllowTcpPortAccess_Success) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(0));
ASSERT_TRUE(port_tracker_.AllowTcpPortAccess(tcp_port, interface, dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
}
TEST_F(PortTrackerTest, AllowUdpPortAccess_Success) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(0));
ASSERT_TRUE(port_tracker_.AllowUdpPortAccess(udp_port, interface, dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
}
TEST_F(PortTrackerTest, AllowTcpPortAccess_Twice) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(0));
EXPECT_CALL(port_tracker_, CheckLifelineFds(false));
ASSERT_TRUE(port_tracker_.AllowTcpPortAccess(tcp_port, interface, dbus_fd));
@@ -104,7 +87,7 @@
}
TEST_F(PortTrackerTest, AllowUdpPortAccess_Twice) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(0));
EXPECT_CALL(port_tracker_, CheckLifelineFds(false));
ASSERT_TRUE(port_tracker_.AllowUdpPortAccess(udp_port, interface, dbus_fd));
@@ -113,8 +96,9 @@
}
TEST_F(PortTrackerTest, AllowTcpPortAccess_FirewallFailure) {
- // Make 'iptables' fail.
- SetMockExpectations(&mock_firewall_, false /* success */);
+ // Make DBus call to patchpanel fail.
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillRepeatedly(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
EXPECT_CALL(port_tracker_, DeleteLifelineFd(tracked_fd))
@@ -124,8 +108,9 @@
}
TEST_F(PortTrackerTest, AllowUdpPortAccess_FirewallFailure) {
- // Make 'iptables' fail.
- SetMockExpectations(&mock_firewall_, false /* success */);
+ // Make DBus call to patchpanel fail.
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillRepeatedly(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
EXPECT_CALL(port_tracker_, DeleteLifelineFd(tracked_fd))
@@ -135,7 +120,7 @@
}
TEST_F(PortTrackerTest, AllowTcpPortAccess_EpollFailure) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
// Make epoll(7) fail.
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(-1));
ASSERT_FALSE(port_tracker_.AllowTcpPortAccess(tcp_port, interface, dbus_fd));
@@ -143,14 +128,14 @@
}
TEST_F(PortTrackerTest, AllowUdpPortAccess_EpollFailure) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
// Make epoll(7) fail.
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(-1));
ASSERT_FALSE(port_tracker_.AllowUdpPortAccess(udp_port, interface, dbus_fd));
}
TEST_F(PortTrackerTest, RevokeTcpPortAccess_Success) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
ASSERT_TRUE(port_tracker_.AllowTcpPortAccess(tcp_port, interface, dbus_fd));
@@ -162,7 +147,7 @@
}
TEST_F(PortTrackerTest, RevokeUdpPortAccess_Success) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
ASSERT_TRUE(port_tracker_.AllowUdpPortAccess(udp_port, interface, dbus_fd));
@@ -174,10 +159,10 @@
}
TEST_F(PortTrackerTest, RevokeTcpPortAccess_FirewallFailure) {
- // Make plugging the firewall hole fail.
- mock_firewall_.SetRunInMinijailFailCriterion(
- std::vector<std::string>({"-D", "-p", "tcp"}), true /* repeat */,
- false /* omit_failure */);
+ // Make revoke iptables rules DBus call fail.
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillOnce(Return(true))
+ .WillOnce(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
@@ -190,10 +175,10 @@
}
TEST_F(PortTrackerTest, RevokeUdpPortAccess_DbusFailure) {
- // Make plugging the firewall hole fail.
- mock_firewall_.SetRunInMinijailFailCriterion(
- std::vector<std::string>({"-D", "-p", "udp"}), true /* repeat */,
- false /* omit_failure */);
+ // Make revoke iptables rules DBus call fail.
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillOnce(Return(true))
+ .WillOnce(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
@@ -206,7 +191,7 @@
}
TEST_F(PortTrackerTest, RevokeTcpPortAccess_EpollFailure) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
@@ -220,7 +205,7 @@
}
TEST_F(PortTrackerTest, RevokeUdpPortAccess_EpollFailure) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
@@ -234,14 +219,14 @@
}
TEST_F(PortTrackerTest, LockDownLoopbackTcpPort_Success) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(0));
ASSERT_TRUE(port_tracker_.LockDownLoopbackTcpPort(tcp_port, dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
}
TEST_F(PortTrackerTest, LockDownLoopbackTcpPort_Twice) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(0));
EXPECT_CALL(port_tracker_, CheckLifelineFds(false));
ASSERT_TRUE(port_tracker_.LockDownLoopbackTcpPort(tcp_port, dbus_fd));
@@ -250,8 +235,9 @@
}
TEST_F(PortTrackerTest, LockDownLoopbackTcpPort_FirewallFailure) {
- // Make 'iptables' fail.
- SetMockExpectations(&mock_firewall_, false /* success */);
+ // Make DBus call to patchpanel fail.
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillRepeatedly(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
EXPECT_CALL(port_tracker_, DeleteLifelineFd(tracked_fd))
@@ -261,7 +247,7 @@
}
TEST_F(PortTrackerTest, LockDownLoopbackTcpPort_EpollFailure) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
// Make epoll(7) fail.
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(-1));
ASSERT_FALSE(port_tracker_.LockDownLoopbackTcpPort(tcp_port, dbus_fd));
@@ -269,7 +255,7 @@
}
TEST_F(PortTrackerTest, ReleaseLoopbackTcpPort_Success) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
ASSERT_TRUE(port_tracker_.LockDownLoopbackTcpPort(tcp_port, dbus_fd));
@@ -281,11 +267,10 @@
}
TEST_F(PortTrackerTest, ReleaseLoopbackTcpPort_FirewallFailure) {
- // Make releasing the lockdown fail.
- mock_firewall_.SetRunInMinijailFailCriterion(
- std::vector<std::string>({"-D", "-p", "tcp"}), true /* repeat */,
- false /* omit_failure */);
-
+ // Make revoke iptables rules DBus call fail.
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillOnce(Return(true))
+ .WillOnce(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
ASSERT_TRUE(port_tracker_.LockDownLoopbackTcpPort(tcp_port, dbus_fd));
@@ -297,8 +282,7 @@
}
TEST_F(PortTrackerTest, ReleaseLoopbackTcpPort_EpollFailure) {
- SetMockExpectations(&mock_firewall_, true /* success */);
-
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd))
.WillOnce(Return(tracked_fd));
ASSERT_TRUE(port_tracker_.LockDownLoopbackTcpPort(tcp_port, dbus_fd));
@@ -311,7 +295,7 @@
}
TEST_F(PortTrackerTest, StartPortForwarding_BaseSuccessCase) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(5));
ASSERT_TRUE(port_tracker_.StartTcpPortForwarding(
tcp_port, "eth0", crosvm_addr, tcp_port, dbus_fd));
@@ -329,19 +313,18 @@
udp_port, "wlan0", pluginvm_addr, udp_port, dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
- ASSERT_EQ(8, mock_firewall_.CountActiveCommands());
}
TEST_F(PortTrackerTest, StartAdbPortForwarding_BaseSuccessCase) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(7));
ASSERT_TRUE(port_tracker_.StartAdbPortForwarding("vmtap0", dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
- ASSERT_EQ(2, mock_firewall_.CountActiveCommands());
}
TEST_F(PortTrackerTest, StartPortForwarding_LifelineFdFailure) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(-1));
ASSERT_FALSE(port_tracker_.StartTcpPortForwarding(
tcp_port, "eth0", crosvm_addr, tcp_port, dbus_fd));
@@ -354,6 +337,7 @@
}
TEST_F(PortTrackerTest, StartAdbPortForwarding_LifelineFdFailure) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(-1));
ASSERT_FALSE(port_tracker_.StartAdbPortForwarding("vmtap0", dbus_fd));
@@ -361,7 +345,8 @@
}
TEST_F(PortTrackerTest, StartPortForwarding_IptablesFailure) {
- SetMockExpectations(&mock_firewall_, false /* failure */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillRepeatedly(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(5));
EXPECT_CALL(port_tracker_, DeleteLifelineFd(5)).WillOnce(Return(true));
@@ -377,7 +362,8 @@
}
TEST_F(PortTrackerTest, StartAdbPortForwarding_IptablesFailure) {
- SetMockExpectations(&mock_firewall_, false /* failure */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _))
+ .WillRepeatedly(Return(false));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(7));
EXPECT_CALL(port_tracker_, DeleteLifelineFd(7)).WillOnce(Return(true));
@@ -387,6 +373,8 @@
}
TEST_F(PortTrackerTest, StartPortForwarding_InputPortValidation) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
+
ASSERT_FALSE(port_tracker_.StartTcpPortForwarding(
reserved_port, "eth0", crosvm_addr, tcp_port, dbus_fd));
ASSERT_FALSE(port_tracker_.StartUdpPortForwarding(
@@ -400,10 +388,11 @@
ASSERT_TRUE(port_tracker_.StartUdpPortForwarding(
tcp_port, "eth0", crosvm_addr, reserved_port, dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
- ASSERT_EQ(4, mock_firewall_.CountActiveCommands());
}
TEST_F(PortTrackerTest, StartPortForwarding_InputInterfaceValidation) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
+
ASSERT_FALSE(port_tracker_.StartTcpPortForwarding(tcp_port, "", crosvm_addr,
tcp_port, dbus_fd));
ASSERT_FALSE(port_tracker_.StartUdpPortForwarding(tcp_port, "", crosvm_addr,
@@ -436,10 +425,11 @@
tcp_port, "mlan0", crosvm_addr, tcp_port, dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
- ASSERT_EQ(10, mock_firewall_.CountActiveCommands());
}
TEST_F(PortTrackerTest, StartAdbPortForwarding_InputInterfaceValidation) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
+
ASSERT_FALSE(port_tracker_.StartAdbPortForwarding("", dbus_fd));
ASSERT_FALSE(port_tracker_.StartAdbPortForwarding("lo", dbus_fd));
ASSERT_FALSE(port_tracker_.StartAdbPortForwarding("iface0", dbus_fd));
@@ -448,10 +438,11 @@
ASSERT_TRUE(port_tracker_.StartAdbPortForwarding("vmtap1", dbus_fd));
ASSERT_TRUE(port_tracker_.HasActiveRules());
- ASSERT_EQ(4, mock_firewall_.CountActiveCommands());
}
TEST_F(PortTrackerTest, StartPortForwarding_TargetIpAddressValidation) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
+
ASSERT_FALSE(port_tracker_.StartTcpPortForwarding(
tcp_port, "eth0", "not_an_ip", tcp_port, dbus_fd));
ASSERT_FALSE(port_tracker_.StartTcpPortForwarding(tcp_port, "eth0", ipv4_any,
@@ -469,6 +460,7 @@
}
TEST_F(PortTrackerTest, StopPortForwarding) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
// Cannot stop before starting.
ASSERT_FALSE(port_tracker_.StopTcpPortForwarding(tcp_port, "eth0"));
ASSERT_FALSE(port_tracker_.StopUdpPortForwarding(tcp_port, "eth0"));
@@ -493,6 +485,7 @@
}
TEST_F(PortTrackerTest, StopAdbPortForwarding) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
// Cannot stop before starting.
ASSERT_FALSE(port_tracker_.StopAdbPortForwarding("vmtap0"));
@@ -509,6 +502,7 @@
}
TEST_F(PortTrackerTest, StartPortForwarding_PreventOverwrite) {
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(5));
ASSERT_TRUE(port_tracker_.StartTcpPortForwarding(
tcp_port, "eth0", crosvm_addr, tcp_port, dbus_fd));
@@ -546,7 +540,7 @@
}
TEST_F(PortTrackerTest, StartPortForwarding_PreventForwardingOpenPort) {
- SetMockExpectations(&mock_firewall_, true /* success */);
+ EXPECT_CALL(port_tracker_, ModifyPortRule(_, _)).WillRepeatedly(Return(true));
// Open TCP port, that port cannot be forwarded for the same interface.
EXPECT_CALL(port_tracker_, AddLifelineFd(dbus_fd)).WillOnce(Return(5));
