Repository: Azure/WALinuxAgent
Pull request #2147 (2021-01-25 20:27:16)

Add a binary file for setting up firewall rules on the VM
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description
This is the 1st PR for the change to add firewall rules to the VM on system reboot (since iptables are not persisted on reboots).

This PR deals with - 
- Creating a new binary file for the sole purpose of setting up waagent-network rules
- Separating out the bare minimum needed for setting up the firewall rules

Issue # <!-- if any -->
<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->

---

### PR information
- [ ] The title of the PR is clear and informative.
- [ ] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [ ] Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made and Travis.CI is passing.

### Quality of Code and Contribution Guidelines
- [ ] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=h1) Report
> Merging [#2147](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=desc) (a08ced3) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/f519183e0b62a5dce87d591f69322c0747371602?el=desc) (f519183) will **increase** coverage by `0.01%`.
> The diff coverage is `78.57%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/graphs/tree.svg?width=650&height=150&src=pr&token=ae90E2KYCw)](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           develop    #2147      +/-   ##
===========================================
+ Coverage    70.68%   70.70%   +0.01%     
===========================================
  Files           95       95              
  Lines        12934    12965      +31     
  Branches      1841     1844       +3     
===========================================
+ Hits          9143     9167      +24     
- Misses        3386     3391       +5     
- Partials       405      407       +2     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/agent.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2FnZW50LnB5) | `48.95% <ø> (ø)` | |
| [azurelinuxagent/common/osutil/default.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9vc3V0aWwvZGVmYXVsdC5weQ==) | `59.86% <57.14%> (+0.15%)` | :arrow_up: |
| [azurelinuxagent/common/utils/networkutil.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi91dGlscy9uZXR3b3JrdXRpbC5weQ==) | `89.65% <82.85%> (-4.58%)` | :arrow_down: |
| [azurelinuxagent/ga/collect\_telemetry\_events.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfdGVsZW1ldHJ5X2V2ZW50cy5weQ==) | `90.00% <0.00%> (-1.38%)` | :arrow_down: |
| [azurelinuxagent/common/protocol/goal\_state.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC9nb2FsX3N0YXRlLnB5) | `91.48% <0.00%> (-0.47%)` | :arrow_down: |
| [azurelinuxagent/common/exception.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9leGNlcHRpb24ucHk=) | `98.87% <0.00%> (+0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=footer). Last update [f519183...a08ced3](https://codecov.io/gh/Azure/WALinuxAgent/pull/2147?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/azurelinuxagent/agent.py b/azurelinuxagent/agent.py
index 69bb2ec4..26ad7b21 100644
--- a/azurelinuxagent/agent.py
+++ b/azurelinuxagent/agent.py
@@ -249,7 +249,7 @@ def parse_args(sys_args):
             conf_file_path = m.group(1)
             if not os.path.exists(conf_file_path):
                 print("Error: Configuration file {0} does not exist".format(
-                        conf_file_path), file=sys.stderr) 
+                        conf_file_path), file=sys.stderr)
                 print(usage())
                 sys.exit(1)
         elif re.match("^([-/]*)deprovision\\+user", arg):
diff --git a/azurelinuxagent/common/osutil/default.py b/azurelinuxagent/common/osutil/default.py
index 35c06356..82c71fae 100644
--- a/azurelinuxagent/common/osutil/default.py
+++ b/azurelinuxagent/common/osutil/default.py
@@ -45,7 +45,7 @@ from azurelinuxagent.common.exception import OSUtilError
 from azurelinuxagent.common.future import ustr, array_to_bytes
 from azurelinuxagent.common.utils.cryptutil import CryptUtil
 from azurelinuxagent.common.utils.flexible_version import FlexibleVersion
-from azurelinuxagent.common.utils.networkutil import RouteEntry, NetworkInterfaceCard
+from azurelinuxagent.common.utils.networkutil import RouteEntry, NetworkInterfaceCard, AddFirewallRules
 from azurelinuxagent.common.utils.shellutil import CommandError
 
 __RULES_FILES__ = ["/lib/udev/rules.d/75-persistent-net-generator.rules",
@@ -75,14 +75,11 @@ def _get_iptables_version_command():
 
 
 def _get_firewall_accept_command(wait, command, destination, owner_uid):
-    return _add_wait(wait,
-                     ["iptables", "-t", "security", command, "OUTPUT", "-d", destination, "-p", "tcp", "-m", "owner",
-                      "--uid-owner", str(owner_uid), "-j" "ACCEPT"])  # pylint: disable=W1404,W1403
+    return AddFirewallRules.get_iptables_accept_command(wait, command, destination, owner_uid)
 
 
 def _get_firewall_drop_command(wait, command, destination):
-    return _add_wait(wait, ["iptables", "-t", "security", command, "OUTPUT", "-d", destination, "-p", "tcp", "-m",
-                            "conntrack", "--ctstate", "INVALID,NEW", "-j", "DROP"])
+    return AddFirewallRules.get_iptables_drop_command(wait, command, destination)
 
 
 def _get_firewall_list_command(wait):
@@ -263,20 +260,10 @@ class DefaultOSUtil(object):
 
             # Otherwise, append both rules
             try:
-                accept_rule = _get_firewall_accept_command(wait, "-A", dst_ip, uid)
-                shellutil.run_command(accept_rule)
-            except Exception as e:
-                msg = "Unable to add ACCEPT firewall rule '{0}' - {1}".format(accept_rule, ustr(e))
-                logger.warn(msg)
-                raise Exception(msg)
-
-            try:
-                drop_rule = _get_firewall_drop_command(wait, "-A", dst_ip)
-                shellutil.run_command(drop_rule)
-            except Exception as e:
-                msg = "Unable to add DROP firewall rule '{0}' - {1}".format(drop_rule, ustr(e))
-                logger.warn(msg)
-                raise Exception(msg)
+                AddFirewallRules.add_iptables_rules(wait, dst_ip, uid)
+            except Exception as error:
+                logger.warn(ustr(error))
+                raise
 
             logger.info("Successfully added Azure fabric firewall rules")
 
diff --git a/azurelinuxagent/common/utils/networkutil.py b/azurelinuxagent/common/utils/networkutil.py
index a5f49be5..d3617732 100644
--- a/azurelinuxagent/common/utils/networkutil.py
+++ b/azurelinuxagent/common/utils/networkutil.py
@@ -16,6 +16,9 @@
 # Requires Python 2.6+ and Openssl 1.0+
 #
 
+from azurelinuxagent.common.future import ustr
+from azurelinuxagent.common.utils import shellutil
+
 
 class RouteEntry(object):
     """
@@ -51,7 +54,7 @@ class RouteEntry(object):
     def to_json(self):
         f = '{{"Iface": "{0}", "Destination": "{1}", "Gateway": "{2}", "Mask": "{3}", "Flags": "{4:#06x}", "Metric": "{5}"}}'
         return f.format(self.interface, self.destination_quad(), self.gateway_quad(), self.mask_quad(),
-                       self.flags, self.metric) 
+                       self.flags, self.metric)
 
     def __str__(self):
         f = "Iface: {0}\tDestination: {1}\tGateway: {2}\tMask: {3}\tFlags: {4:#06x}\tMetric: {5}"
@@ -93,3 +96,49 @@ class NetworkInterfaceCard:
         if len(self.ipv6) > 0:
             entries.append('"ipv6": {0}'.format(self._json_array(self.ipv6)))
         return "{{ {0} }}".format(", ".join(entries))
+
+
+class AddFirewallRules(object):
+
+    @staticmethod
+    def _add_wait(wait, command):
+        """
+        If 'wait' is True, adds the wait option (-w) to the given iptables command line
+        """
+        if wait:
+            command.insert(1, "-w")
+        return command
+
+    @staticmethod
+    def get_iptables_accept_command(wait, command, destination, owner_uid):
+        cmd = ["iptables", "-t", "security", command, "OUTPUT", "-d", destination, "-p", "tcp", "-m", "owner",
+               "--uid-owner", str(owner_uid), "-j", "ACCEPT"]
+        return AddFirewallRules._add_wait(wait, cmd)
+
+    @staticmethod
+    def get_iptables_drop_command(wait, command, destination):
+        cmd = ["iptables", "-t", "security", command, "OUTPUT", "-d", destination, "-p", "tcp", "-m", "conntrack",
+               "--ctstate", "INVALID,NEW", "-j", "DROP"]
+        return AddFirewallRules._add_wait(wait, cmd)
+
+    @staticmethod
+    def add_iptables_rules(wait, dst_ip, uid):
+        def _raise_if_empty(val, name):
+            if val == "":
+                raise Exception("{0} should not be empty".format(name))
+
+        _raise_if_empty(dst_ip, "Destination IP")
+        _raise_if_empty(uid, "User ID")
+        accept_rule = AddFirewallRules.get_iptables_accept_command(wait, "-A", dst_ip, uid)
+        try:
+            shellutil.run_command(accept_rule)
+        except Exception as e:
+            msg = "Unable to add ACCEPT firewall rule '{0}' - {1}".format(accept_rule, ustr(e))
+            raise Exception(msg)
+
+        drop_rule = AddFirewallRules.get_iptables_drop_command(wait, "-A", dst_ip)
+        try:
+            shellutil.run_command(drop_rule)
+        except Exception as e:
+            msg = "Unable to add DROP firewall rule '{0}' - {1}".format(drop_rule, ustr(e))
+            raise Exception(msg)
diff --git a/bin/waagent_network_setup.py b/bin/waagent_network_setup.py
new file mode 100644
index 00000000..f123a05a
--- /dev/null
+++ b/bin/waagent_network_setup.py
@@ -0,0 +1,85 @@
+#!/usr/bin/env python
+#
+# Azure Linux Agent
+#
+# Copyright 2015 Microsoft Corporation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Requires Python 2.6 and Openssl 1.0+
+#
+# Implements parts of RFC 2131, 1541, 1497 and
+# http://msdn.microsoft.com/en-us/library/cc227282%28PROT.10%29.aspx
+# http://msdn.microsoft.com/en-us/library/cc227259%28PROT.13%29.aspx
+#
+
+import sys
+import re
+from azurelinuxagent.common.future import ustr
+from azurelinuxagent.common.utils.networkutil import AddFirewallRules
+
+
+def __usage():
+    """
+    Return the usage for this script
+    """
+    return "\nUsage: {0} [-help] -dst_ip=<Wireserver IP> -uid=[User ID] [-w|--wait]\n".format(sys.argv[0])
+
+
+def _setup_firewall_rules():
+    print("Setting up firewall rules")
+    try:
+        args = sys.argv[1:]
+        dst_ip = ""
+        uid = ""
+        wait = ""
+
+        # The command to call this would look something like this -
+        # ExecStart=PYPATH /usr/sbin/waagent_network_setup.py --dst_ip=1.2.3.4 --uid=9999 [-w]
+        for arg in args:
+            dst_ip_match = re.match("^([-/]*)dst_ip=(?P<dst_ip>[\d.]{7,})", arg)
+            uid_match = re.match("^([-/]*)uid=(?P<uid>[\d]+)", arg)
+            wait_match = re.match("^([-/]*)(w|wait)$", arg)
+
+            if dst_ip_match:
+                dst_ip = dst_ip_match.group('dst_ip')
+            elif uid_match:
+                uid = uid_match.group('uid')
+            elif wait_match:
+                wait = "-w"
+            else:
+                # If any command is not recognised, just print the help command and exit
+                print(__usage())
+                sys.exit(0)
+
+        AddFirewallRules.add_iptables_rules(wait, dst_ip, uid)
+        print("Setting Firewall rules completed")
+    except Exception as error:
+        print("Unable to setup firewall rules: {0}".format(ustr(error)))
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    """
+    This file would be called by the waagent-network-setup service to bridge the gap between the system reboot and 
+    waagent.service start. The sole purpose of this file is to setup firewall rules for the VM.
+    
+    The parameters for this file would be set in the unit file for the service, which would be created dynamically the 
+    first time waagent.service starts. On every service restart, we would monitor the Wireserver IP and reset it in the 
+    unit file if it changes.
+    
+    This service would run before the network would be setup, so we need to maintain state of the previous Wireserver IP
+    to ensure we restrict access to the Wire IP.
+    """
+    _setup_firewall_rules()
+
diff --git a/setup.py b/setup.py
index 2db10c10..be4e5c59 100755
--- a/setup.py
+++ b/setup.py
@@ -40,7 +40,7 @@ def set_files(data_files, dest=None, src=None):
 
 def set_bin_files(data_files, dest="/usr/sbin", src=None):
     if src is None:
-        src = ["bin/waagent", "bin/waagent2.0"]
+        src = ["bin/waagent", "bin/waagent2.0", "bin/waagent_network_setup.py"]
     data_files.append((dest, src))
 
 


Test patch:
diff --git a/tests/utils/test_network_util.py b/tests/utils/test_network_util.py
index ba5c630c..9836d0ee 100644
--- a/tests/utils/test_network_util.py
+++ b/tests/utils/test_network_util.py
@@ -14,9 +14,11 @@
 #
 # Requires Python 2.6+ and Openssl 1.0+
 #
+import subprocess
 
-import azurelinuxagent.common.utils.networkutil as networkutil
+from mock.mock import patch
 
+import azurelinuxagent.common.utils.networkutil as networkutil
 from tests.tools import AgentTestCase
 
 
@@ -60,3 +62,36 @@ class TestNetworkOperations(AgentTestCase):
         nic.add_ipv6("ipv6-1")
         nic.add_ipv4("ipv4-1")
         self.assertEqual(str(nic), '{ "name": "test0", "link": "link INFO", "ipv4": ["ipv4-1"], "ipv6": ["ipv6-1"] }')
+
+
+class TestAddFirewallRules(AgentTestCase):
+
+    def test_it_should_add_firewall_rules(self):
+        test_dst_ip = "1.2.3.4"
+        test_uid = 9999
+        test_wait = "-w"
+        original_popen = subprocess.Popen
+        commands_called = []
+
+        def mock_popen(command, *args, **kwargs):
+            if "iptables" in command:
+                commands_called.append(command)
+                command = ["echo", "iptables"]
+            return original_popen(command, *args, **kwargs)
+
+        with patch("azurelinuxagent.common.utils.shellutil.subprocess.Popen", side_effect=mock_popen):
+            networkutil.AddFirewallRules.add_iptables_rules(test_wait, test_dst_ip, test_uid)
+
+        self.assertTrue(all(test_dst_ip in cmd for cmd in commands_called), "Dest IP was not set correctly in iptables")
+        self.assertTrue(all(test_wait in cmd for cmd in commands_called), "The wait was not set properly")
+        self.assertTrue(all(str(test_uid) in cmd for cmd in commands_called if "ACCEPT" in cmd),
+                        "The UID is not set for the accept command")
+
+    def test_it_should_raise_if_invalid_data(self):
+        with self.assertRaises(Exception) as context_manager:
+            networkutil.AddFirewallRules.add_iptables_rules(wait="", dst_ip="", uid=9999)
+        self.assertIn("Destination IP should not be empty", str(context_manager.exception))
+
+        with self.assertRaises(Exception) as context_manager:
+            networkutil.AddFirewallRules.add_iptables_rules(wait="", dst_ip="1.2.3.4", uid="")
+        self.assertIn("User ID should not be empty", str(context_manager.exception))