firewalld: Add IPv6 firewall rules.

BUG=brillo:252
TEST=Unit tests.

Change-Id: I784472ce5f0c7d0649b38e48bd23b3acba9ffbbc
Reviewed-on: https://chromium-review.googlesource.com/249982
Trybot-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Jorge Lucangeli Obes <jorgelo@chromium.org>

3 files changed, 82 insertions, 32 deletions

diff --git a/iptables.cc b/iptables.cc
index d4641ef..c754a0a 100644
--- a/iptables.cc
+++ b/iptables.cc
@@ -12,7 +12,8 @@
 #include <chromeos/process.h>
 
 namespace {
-const char kIptablesPath[] = "/sbin/iptables";
+const char kIpTablesPath[] = "/sbin/iptables";
+const char kIp6TablesPath[] = "/sbin/ip6tables";
 
 // Interface names must be shorter than 'IFNAMSIZ' chars.
 // See http://man7.org/linux/man-pages/man7/netdevice.7.html
@@ -37,9 +38,10 @@ bool IsValidInterfaceName(const std::string& iface) {
 
 namespace firewalld {
 
-IpTables::IpTables() : IpTables{kIptablesPath} {}
+IpTables::IpTables() : IpTables{kIpTablesPath, kIp6TablesPath} {}
 
-IpTables::IpTables(const std::string& path) : executable_path_{path} {}
+IpTables::IpTables(const std::string& ip4_path, const std::string& ip6_path)
+    : ip4_exec_path_{ip4_path}, ip6_exec_path_{ip6_path} {}
 
 IpTables::~IpTables() {
   // Plug all holes when destructed.
@@ -65,7 +67,7 @@ bool IpTables::PlugUdpHole(uint16_t in_port, const std::string& in_interface) {
 bool IpTables::PunchHole(uint16_t port,
                          const std::string& interface,
                          std::set<Hole>* holes,
-                         enum ProtocolEnum protocol) {
+                         ProtocolEnum protocol) {
   if (port == 0) {
     // Port 0 is not a valid TCP/UDP port.
     return false;
@@ -86,9 +88,9 @@ bool IpTables::PunchHole(uint16_t port,
   std::string sprotocol = protocol == kProtocolTcp ? "TCP" : "UDP";
   LOG(INFO) << "Punching hole for " << sprotocol << " port " << port
             << " on interface '" << interface << "'";
-  if (!IpTables::AddAllowRule(protocol, port, interface)) {
+  if (!AddAcceptRules(protocol, port, interface)) {
     // If the 'iptables' command fails, this method fails.
-    LOG(ERROR) << "Calling 'iptables' failed";
+    LOG(ERROR) << "Adding ACCEPT rules failed";
     return false;
   }
 
@@ -101,7 +103,7 @@ bool IpTables::PunchHole(uint16_t port,
 bool IpTables::PlugHole(uint16_t port,
                         const std::string& interface,
                         std::set<Hole>* holes,
-                        enum ProtocolEnum protocol) {
+                        ProtocolEnum protocol) {
   if (port == 0) {
     // Port 0 is not a valid TCP/UDP port.
     return false;
@@ -119,9 +121,9 @@ bool IpTables::PlugHole(uint16_t port,
   std::string sprotocol = protocol == kProtocolTcp ? "TCP" : "UDP";
   LOG(INFO) << "Plugging hole for " << sprotocol << " port " << port
             << " on interface '" << interface << "'";
-  if (!IpTables::DeleteAllowRule(protocol, port, interface)) {
+  if (!DeleteAcceptRules(protocol, port, interface)) {
     // If the 'iptables' command fails, this method fails.
-    LOG(ERROR) << "Calling 'iptables' failed";
+    LOG(ERROR) << "Deleting ACCEPT rules failed";
     return false;
   }
 
@@ -151,19 +153,44 @@ void IpTables::PlugAllHoles() {
   CHECK(udp_holes_.size() == 0) << "Failed to plug all UDP holes.";
 }
 
-bool IpTables::AddAllowRule(enum ProtocolEnum protocol,
-                            uint16_t port,
-                            const std::string& interface) {
+bool IpTables::AddAcceptRules(ProtocolEnum protocol,
+                              uint16_t port,
+                              const std::string& interface) {
+  if (!AddAcceptRule(ip4_exec_path_, protocol, port, interface)) {
+    LOG(ERROR) << "Could not add ACCEPT rule using '" << ip4_exec_path_ << "'";
+    return false;
+  }
+  if (!AddAcceptRule(ip6_exec_path_, protocol, port, interface)) {
+    LOG(ERROR) << "Could not add ACCEPT rule using '" << ip6_exec_path_ << "'";
+    DeleteAcceptRule(ip4_exec_path_, protocol, port, interface);
+    return false;
+  }
+  return true;
+}
+
+bool IpTables::DeleteAcceptRules(ProtocolEnum protocol,
+                                 uint16_t port,
+                                 const std::string& interface) {
+  bool ip4_success = DeleteAcceptRule(ip4_exec_path_, protocol, port,
+                                      interface);
+  bool ip6_success = DeleteAcceptRule(ip6_exec_path_, protocol, port,
+                                      interface);
+  return ip4_success && ip6_success;
+}
+
+bool IpTables::AddAcceptRule(const std::string& executable_path,
+                             ProtocolEnum protocol,
+                             uint16_t port,
+                             const std::string& interface) {
   chromeos::ProcessImpl iptables;
-  iptables.AddArg(executable_path_);
+  iptables.AddArg(executable_path);
   iptables.AddArg("-I");  // insert
   iptables.AddArg("INPUT");
   iptables.AddArg("-p");  // protocol
   iptables.AddArg(protocol == kProtocolTcp ? "tcp" : "udp");
   iptables.AddArg("--dport");  // destination port
-  std::string port_number = base::StringPrintf("%d", port);
-  iptables.AddArg(port_number.c_str());
-  if (interface != "") {
+  iptables.AddArg(std::to_string(port));
+  if (!interface.empty()) {
     iptables.AddArg("-i");  // interface
     iptables.AddArg(interface);
   }
@@ -173,11 +200,12 @@ bool IpTables::AddAllowRule(enum ProtocolEnum protocol,
   return iptables.Run() == 0;
 }
 
-bool IpTables::DeleteAllowRule(enum ProtocolEnum protocol,
-                               uint16_t port,
-                               const std::string& interface) {
+bool IpTables::DeleteAcceptRule(const std::string& executable_path,
+                                ProtocolEnum protocol,
+                                uint16_t port,
+                                const std::string& interface) {
   chromeos::ProcessImpl iptables;
-  iptables.AddArg(executable_path_);
+  iptables.AddArg(executable_path);
   iptables.AddArg("-D");  // delete
   iptables.AddArg("INPUT");
   iptables.AddArg("-p");  // protocol
diff --git a/iptables.h b/iptables.h
index 51a7423..9be18b3 100644
--- a/iptables.h
+++ b/iptables.h
@@ -35,7 +35,7 @@ class IpTables : public org::chromium::FirewalldInterface {
 
  protected:
   // Test-only.
-  explicit IpTables(const std::string& path);
+  explicit IpTables(const std::string& ip4_path, const std::string& ip6_path);
 
  private:
   friend class IpTablesTest;
@@ -43,22 +43,31 @@ class IpTables : public org::chromium::FirewalldInterface {
   bool PunchHole(uint16_t port,
                  const std::string& interface,
                  std::set<Hole>* holes,
-                 enum ProtocolEnum protocol);
+                 ProtocolEnum protocol);
   bool PlugHole(uint16_t port,
                 const std::string& interface,
                 std::set<Hole>* holes,
-                enum ProtocolEnum protocol);
+                ProtocolEnum protocol);
 
   void PlugAllHoles();
 
-  bool AddAllowRule(enum ProtocolEnum protocol,
-                    uint16_t port,
-                    const std::string& interface);
-  bool DeleteAllowRule(enum ProtocolEnum protocol,
-                       uint16_t port,
-                       const std::string& interface);
-
-  std::string executable_path_;
+  bool AddAcceptRules(ProtocolEnum protocol,
+                      uint16_t port,
+                      const std::string& interface);
+  bool DeleteAcceptRules(ProtocolEnum protocol,
+                         uint16_t port,
+                         const std::string& interface);
+  bool AddAcceptRule(const std::string& executable_path,
+                     ProtocolEnum protocol,
+                     uint16_t port,
+                     const std::string& interface);
+  bool DeleteAcceptRule(const std::string& executable_path,
+                        ProtocolEnum protocol,
+                        uint16_t port,
+                        const std::string& interface);
+
+  std::string ip4_exec_path_;
+  std::string ip6_exec_path_;
 
   // Keep track of firewall holes to avoid adding redundant firewall rules.
   std::set<Hole> tcp_holes_;
diff --git a/iptables_unittest.cc b/iptables_unittest.cc
index f9f122c..e70fcb0 100644
--- a/iptables_unittest.cc
+++ b/iptables_unittest.cc
@@ -11,12 +11,15 @@ namespace firewalld {
 class IpTablesTest : public testing::Test {
  public:
   IpTablesTest()
-      : iptables_succeeds{"/bin/true"}, iptables_fails{"/bin/false"} {}
+      : iptables_succeeds{"/bin/true", "/bin/true"},
+        iptables_fails{"/bin/false", "/bin/false"},
+        ip4succeeds_ip6fails{"/bin/true", "/bin/false"} {}
   ~IpTablesTest() override = default;
 
  protected:
   IpTables iptables_succeeds;
   IpTables iptables_fails;
+  IpTables ip4succeeds_ip6fails;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(IpTablesTest);
@@ -85,4 +88,14 @@ TEST_F(IpTablesTest, PunchUdpHoleFails) {
   ASSERT_FALSE(iptables_fails.PunchUdpHole(53, "iface"));
 }
 
+TEST_F(IpTablesTest, PunchTcpHoleIpv6Fails) {
+  // Punch hole for TCP port 80, should fail because 'ip6tables' fails.
+  ASSERT_FALSE(ip4succeeds_ip6fails.PunchTcpHole(80, "iface"));
+}
+
+TEST_F(IpTablesTest, PunchUdpHoleIpv6Fails) {
+  // Punch hole for UDP port 53, should fail because 'ip6tables' fails.
+  ASSERT_FALSE(ip4succeeds_ip6fails.PunchUdpHole(53, "iface"));
+}
+
 }  // namespace firewalld