Repository: Azure/WALinuxAgent (python)

Issue:
agent breaks ssh configuration
The agent indiscriminately adds

 PasswordAuthentication no
ChallengeResponseAuthentication no

to the end of sshd_config. However, if the configuration file happens to contain a "Match" block at the end then the added configuration options become part of the match block creating a syntax error in the configuration file and thus preventing sshd from running. This results in a running instance that the user cannot access.

Pull request:
Add support for Match blocks in sshd config
We do not support `Match` blocks in sshd config, and if they occur anywhere but the end of the file, we can potentially break the config rules. In addition, we do not have unit tests for conf_sshd. These ensure the correct behavior.

- fixes #544 

/cc @jinhyunr @brendandixon 

Patch:
diff --git a/azurelinuxagent/common/utils/textutil.py b/azurelinuxagent/common/utils/textutil.py
index c683a13c..2d99f6f6 100644
--- a/azurelinuxagent/common/utils/textutil.py
+++ b/azurelinuxagent/common/utils/textutil.py
@@ -221,15 +221,24 @@ def hexstr_to_bytearray(a):
 
 
 def set_ssh_config(config, name, val):
-    notfound = True
+    found = False
+    no_match = -1
+
+    match_start = no_match
     for i in range(0, len(config)):
-        if config[i].startswith(name):
+        if config[i].startswith(name) and match_start == no_match:
             config[i] = "{0} {1}".format(name, val)
-            notfound = False
-        elif config[i].startswith("Match"):
-            # Match block must be put in the end of sshd config
-            break
-    if notfound:
+            found = True
+        elif config[i].lower().startswith("match"):
+            if config[i].lower().startswith("match all"):
+                # outside match block
+                match_start = no_match
+            elif match_start == no_match:
+                # inside match block
+                match_start = i
+    if not found:
+        if match_start != no_match:
+            i = match_start
         config.insert(i, "{0} {1}".format(name, val))
     return config
 


Test patch:
diff --git a/tests/common/osutil/test_default.py b/tests/common/osutil/test_default.py
index d982b7e2..933787ba 100644
--- a/tests/common/osutil/test_default.py
+++ b/tests/common/osutil/test_default.py
@@ -21,6 +21,7 @@ import mock
 import azurelinuxagent.common.osutil.default as osutil
 import azurelinuxagent.common.utils.shellutil as shellutil
 from azurelinuxagent.common.osutil import get_osutil
+from azurelinuxagent.common.utils import fileutil
 from tests.tools import *
 
 
@@ -164,5 +165,155 @@ class TestOSUtil(AgentTestCase):
         else:
             self.fail("Cannot retrieve number of process cores using shell command.")
 
+    def test_conf_sshd(self):
+        new_file = "\
+Port 22\n\
+Protocol 2\n\
+ChallengeResponseAuthentication yes\n\
+#PasswordAuthentication yes\n\
+UsePAM yes\n\
+"
+        expected_output = "\
+Port 22\n\
+Protocol 2\n\
+ChallengeResponseAuthentication no\n\
+#PasswordAuthentication yes\n\
+UsePAM yes\n\
+PasswordAuthentication no\n\
+ClientAliveInterval 180\n\
+"
+
+        with patch.object(fileutil, 'write_file') as patch_write:
+            with patch.object(fileutil, 'read_file', return_value=new_file):
+                osutil.DefaultOSUtil().conf_sshd(disable_password=True)
+                patch_write.assert_called_once_with(
+                    conf.get_sshd_conf_file_path(),
+                    expected_output)
+
+    def test_conf_sshd_with_match(self):
+        new_file = "\
+Port 22\n\
+ChallengeResponseAuthentication yes\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+"
+        expected_output = "\
+Port 22\n\
+ChallengeResponseAuthentication no\n\
+PasswordAuthentication no\n\
+ClientAliveInterval 180\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+"
+
+        with patch.object(fileutil, 'write_file') as patch_write:
+            with patch.object(fileutil, 'read_file', return_value=new_file):
+                osutil.DefaultOSUtil().conf_sshd(disable_password=True)
+                patch_write.assert_called_once_with(
+                    conf.get_sshd_conf_file_path(),
+                    expected_output)
+
+    def test_conf_sshd_with_match_last(self):
+        new_file = "\
+Port 22\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+"
+        expected_output = "\
+Port 22\n\
+PasswordAuthentication no\n\
+ChallengeResponseAuthentication no\n\
+ClientAliveInterval 180\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+"
+
+        with patch.object(fileutil, 'write_file') as patch_write:
+            with patch.object(fileutil, 'read_file', return_value=new_file):
+                osutil.DefaultOSUtil().conf_sshd(disable_password=True)
+                patch_write.assert_called_once_with(
+                    conf.get_sshd_conf_file_path(),
+                    expected_output)
+
+    def test_conf_sshd_with_match_middle(self):
+        new_file = "\
+Port 22\n\
+match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+match all\n\
+#Other config\n\
+"
+        expected_output = "\
+Port 22\n\
+match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+match all\n\
+#Other config\n\
+PasswordAuthentication no\n\
+ChallengeResponseAuthentication no\n\
+ClientAliveInterval 180\n\
+"
+
+        with patch.object(fileutil, 'write_file') as patch_write:
+            with patch.object(fileutil, 'read_file', return_value=new_file):
+                osutil.DefaultOSUtil().conf_sshd(disable_password=True)
+                patch_write.assert_called_once_with(
+                    conf.get_sshd_conf_file_path(),
+                    expected_output)
+
+    def test_conf_sshd_with_match_multiple(self):
+        new_file = "\
+Port 22\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+Match host 192.168.1.2\n\
+  ChallengeResponseAuthentication yes\n\
+Match all\n\
+#Other config\n\
+"
+        expected_output = "\
+Port 22\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+Match host 192.168.1.2\n\
+  ChallengeResponseAuthentication yes\n\
+Match all\n\
+#Other config\n\
+PasswordAuthentication no\n\
+ChallengeResponseAuthentication no\n\
+ClientAliveInterval 180\n\
+"
+
+        with patch.object(fileutil, 'write_file') as patch_write:
+            with patch.object(fileutil, 'read_file', return_value=new_file):
+                osutil.DefaultOSUtil().conf_sshd(disable_password=True)
+                patch_write.assert_called_once_with(
+                    conf.get_sshd_conf_file_path(),
+                    expected_output)
+
+    def test_conf_sshd_with_match_multiple_first_last(self):
+        new_file = "\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+Match host 192.168.1.2\n\
+  ChallengeResponseAuthentication yes\n\
+"
+        expected_output = "\
+PasswordAuthentication no\n\
+ChallengeResponseAuthentication no\n\
+ClientAliveInterval 180\n\
+Match host 192.168.1.1\n\
+  ChallengeResponseAuthentication yes\n\
+Match host 192.168.1.2\n\
+  ChallengeResponseAuthentication yes\n\
+"
+
+        with patch.object(fileutil, 'write_file') as patch_write:
+            with patch.object(fileutil, 'read_file', return_value=new_file):
+                osutil.DefaultOSUtil().conf_sshd(disable_password=True)
+                patch_write.assert_called_once_with(
+                    conf.get_sshd_conf_file_path(),
+                    expected_output)
+
 if __name__ == '__main__':
     unittest.main()