Added more structure to JSON result when policy errors are found.

ssh-audit.py

@@ -204,6 +204,15 @@ class Policy:
             raise ValueError('The policy does not have a version field.')
 
 
+    @staticmethod
+    def _append_error(errors: List[Any], mismatched_field: str, expected_required: Optional[List[str]], expected_optional: Optional[List[str]], actual: List[str]) -> None:
+        if expected_required is None:
+            expected_required = ['']
+        if expected_optional is None:
+            expected_optional = ['']
+        errors.append({'mismatched_field': mismatched_field, 'expected_required': expected_required, 'expected_optional': expected_optional, 'actual': actual})
+
+
     @staticmethod
     def create(source: Optional[str], banner: Optional['SSH.Banner'], kex: Optional['SSH2.Kex'], client_audit: bool) -> str:
         '''Creates a policy based on a server configuration.  Returns a string.'''
@@ -290,24 +299,24 @@ macs = %s
         return policy_data
 
 
-    def evaluate(self, banner: Optional['SSH.Banner'], kex: Optional['SSH2.Kex']) -> Tuple[bool, List[str]]:
+    def evaluate(self, banner: Optional['SSH.Banner'], kex: Optional['SSH2.Kex']) -> Tuple[bool, List[Dict[str, str]], str]:
         '''Evaluates a server configuration against this policy.  Returns a tuple of a boolean (True if server adheres to policy) and an array of strings that holds error messages.'''
 
         ret = True
-        errors = []
+        errors = []  # type: List[Any]
 
         banner_str = str(banner)
         if (self._banner is not None) and (banner_str != self._banner):
             ret = False
-            errors.append('Banner did not match. Expected: [%s]; Actual: [%s]' % (self._banner, banner_str))
+            self._append_error(errors, 'Banner', [self._banner], None, [banner_str])
 
         # All subsequent tests require a valid kex, so end here if we don't have one.
         if kex is None:
-            return ret, errors
+            return ret, errors, self._get_error_str(errors)
 
         if (self._compressions is not None) and (kex.server.compression != self._compressions):
             ret = False
-            errors.append('Compression types did not match. Expected: %s; Actual: %s' % (self._compressions, kex.server.compression))
+            self._append_error(errors, 'Compression', self._compressions, None, kex.server.compression)
 
         # If a list of optional host keys was given in the policy, remove any of its entries from the list retrieved from the server.  This allows us to do an exact comparison with the expected list below.
         pruned_host_keys = kex.key_algorithms
@@ -316,7 +325,7 @@ macs = %s
 
         if (self._host_keys is not None) and (pruned_host_keys != self._host_keys):
             ret = False
-            errors.append('Host key types did not match. Expected (required): %s; Expected (optional): %s; Actual: %s' % (self._host_keys, self._optional_host_keys, kex.key_algorithms))
+            self._append_error(errors, 'Host keys', self._host_keys, self._optional_host_keys, kex.key_algorithms)
 
         if self._hostkey_sizes is not None:
             hostkey_types = list(self._hostkey_sizes.keys())
@@ -327,7 +336,7 @@ macs = %s
                     actual_hostkey_size, actual_cakey_size = kex.rsa_key_sizes()[hostkey_type]
                     if actual_hostkey_size != expected_hostkey_size:
                         ret = False
-                        errors.append('RSA hostkey (%s) sizes did not match.  Expected: %d; Actual: %d' % (hostkey_type, expected_hostkey_size, actual_hostkey_size))
+                        self._append_error(errors, 'RSA host key (%s) sizes' % hostkey_type, [str(expected_hostkey_size)], None, [str(actual_hostkey_size)])
 
         if self._cakey_sizes is not None:
             hostkey_types = list(self._cakey_sizes.keys())
@@ -338,19 +347,19 @@ macs = %s
                     actual_hostkey_size, actual_cakey_size = kex.rsa_key_sizes()[hostkey_type]
                     if actual_cakey_size != expected_cakey_size:
                         ret = False
-                        errors.append('RSA CA key (%s) sizes did not match.  Expected: %d; Actual: %d' % (hostkey_type, expected_cakey_size, actual_cakey_size))
+                        self._append_error(errors, 'RSA CA key (%s) sizes' % hostkey_type, [str(expected_cakey_size)], None, [str(actual_cakey_size)])
 
         if kex.kex_algorithms != self._kex:
             ret = False
-            errors.append('Key exchanges did not match. Expected: %s; Actual: %s' % (self._kex, kex.kex_algorithms))
+            self._append_error(errors, 'Key exchanges', self._kex, None, kex.kex_algorithms)
 
         if (self._ciphers is not None) and (kex.server.encryption != self._ciphers):
             ret = False
-            errors.append('Ciphers did not match. Expected: %s; Actual: %s' % (self._ciphers, kex.server.encryption))
+            self._append_error(errors, 'Ciphers', self._ciphers, None, kex.server.encryption)
 
         if (self._macs is not None) and (kex.server.mac != self._macs):
             ret = False
-            errors.append('MACs did not match. Expected: %s; Actual: %s' % (self._macs, kex.server.mac))
+            self._append_error(errors, 'MACs', self._macs, None, kex.server.mac)
 
         if self._dh_modulus_sizes is not None:
             dh_modulus_types = list(self._dh_modulus_sizes.keys())
@@ -361,9 +370,32 @@ macs = %s
                     actual_dh_modulus_size, _ = kex.dh_modulus_sizes()[dh_modulus_type]
                     if expected_dh_modulus_size != actual_dh_modulus_size:
                         ret = False
-                        errors.append('Group exchange (%s) modulus sizes did not match.  Expected: %d; Actual: %d' % (dh_modulus_type, expected_dh_modulus_size, actual_dh_modulus_size))
+                        self._append_error(errors, 'Group exchange (%s) modulus sizes' % dh_modulus_type, [str(expected_dh_modulus_size)], None, [str(actual_dh_modulus_size)])
 
-        return ret, errors
+        return ret, errors, self._get_error_str(errors)
+
+
+    @staticmethod
+    def _get_error_str(errors: List[Any]) -> str:
+        '''Transforms an error struct to a flat string of error messages.'''
+
+        error_list = []
+        for e in errors:
+            e_str = "%s did not match. " % e['mismatched_field']
+            if ('expected_optional' in e) and (e['expected_optional'] != ['']):
+                e_str += "Expected (required): %s; Expected (optional): %s" % (Policy._normalize_error_field(e['expected_required']), Policy._normalize_error_field(e['expected_optional']))
+            else:
+                e_str += "Expected: %s" % Policy._normalize_error_field(e['expected_required'])
+            e_str += "; Actual: %s" % Policy._normalize_error_field(e['actual'])
+            error_list.append(e_str)
+
+        error_list.sort()  # To ensure repeatable results for testing.
+
+        error_str = ''
+        if len(error_list) > 0:
+            error_str = "  * %s" % '\n  * '.join(error_list)
+
+        return error_str
 
 
     def get_name_and_version(self) -> str:
@@ -376,6 +408,18 @@ macs = %s
         return self._server_policy
 
 
+    @staticmethod
+    def _normalize_error_field(field: List[str]) -> Any:
+        '''If field is an array with a string parsable as an integer, return that integer.  Otherwise, return the field unmodified.'''
+        if len(field) == 1:
+            try:
+                return int(field[0])
+            except ValueError:
+                return field
+        else:
+            return field
+
+
     def __str__(self) -> str:
         undefined = '{undefined}'
 
@@ -3280,9 +3324,9 @@ def evaluate_policy(aconf: AuditConf, banner: Optional['SSH.Banner'], client_hos
     if aconf.policy is None:
         raise RuntimeError('Internal error: cannot evaluate against null Policy!')
 
-    passed, errors = aconf.policy.evaluate(banner, kex)
+    passed, error_struct, error_str = aconf.policy.evaluate(banner, kex)
     if aconf.json:
-        json_struct = {'host': aconf.host, 'policy': aconf.policy.get_name_and_version(), 'passed': passed, 'errors': errors}
+        json_struct = {'host': aconf.host, 'policy': aconf.policy.get_name_and_version(), 'passed': passed, 'errors': error_struct}
         print(json.dumps(json_struct, sort_keys=True))
     else:
         spacing = ''
@@ -3297,7 +3341,7 @@ def evaluate_policy(aconf: AuditConf, banner: Optional['SSH.Banner'], client_hos
             out.good("✔ Passed")
         else:
             out.fail("❌ Failed!")
-            out.warn("\nErrors:\n  * %s" % '\n  * '.join(errors))
+            out.warn("\nErrors:\n%s" % error_str)
 
     return passed
 

test/docker/expected_results/openssh_5.6p1_policy_test10.json

@@ -1 +1 @@
-{"errors": ["RSA hostkey (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 4096; Actual: 3072", "RSA CA key (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 4096; Actual: 1024"], "host": "localhost", "passed": false, "policy": "Docker poliicy: test10 (version 1)"}
+{"errors": [{"actual": ["3072"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "RSA host key (ssh-rsa-cert-v01@openssh.com) sizes"}, {"actual": ["1024"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "RSA CA key (ssh-rsa-cert-v01@openssh.com) sizes"}], "host": "localhost", "passed": false, "policy": "Docker poliicy: test10 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test10.txt

@@ -3,5 +3,5 @@ Policy: Docker poliicy: test10 (version 1)
 Result: ❌ Failed!
 
 Errors:
-  * RSA hostkey (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 4096; Actual: 3072
-  * RSA CA key (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 4096; Actual: 1024
+  * RSA CA key (ssh-rsa-cert-v01@openssh.com) sizes did not match. Expected: 4096; Actual: 1024
+  * RSA host key (ssh-rsa-cert-v01@openssh.com) sizes did not match. Expected: 4096; Actual: 3072

test/docker/expected_results/openssh_5.6p1_policy_test2.json

@@ -1 +1 @@
-{"errors": ["Key exchanges did not match. Expected: ['kex_alg1', 'kex_alg2']; Actual: ['diffie-hellman-group-exchange-sha256', 'diffie-hellman-group-exchange-sha1', 'diffie-hellman-group14-sha1', 'diffie-hellman-group1-sha1']"], "host": "localhost", "passed": false, "policy": "Docker policy: test2 (version 1)"}
+{"errors": [{"actual": ["diffie-hellman-group-exchange-sha256", "diffie-hellman-group-exchange-sha1", "diffie-hellman-group14-sha1", "diffie-hellman-group1-sha1"], "expected_optional": [""], "expected_required": ["kex_alg1", "kex_alg2"], "mismatched_field": "Key exchanges"}], "host": "localhost", "passed": false, "policy": "Docker policy: test2 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test3.json

@@ -1 +1 @@
-{"errors": ["Host key types did not match. Expected (required): ['ssh-rsa', 'ssh-dss', 'key_alg1']; Expected (optional): None; Actual: ['ssh-rsa', 'ssh-dss']"], "host": "localhost", "passed": false, "policy": "Docker policy: test3 (version 1)"}
+{"errors": [{"actual": ["ssh-rsa", "ssh-dss"], "expected_optional": [""], "expected_required": ["ssh-rsa", "ssh-dss", "key_alg1"], "mismatched_field": "Host keys"}], "host": "localhost", "passed": false, "policy": "Docker policy: test3 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test3.txt

@@ -3,4 +3,4 @@ Policy: Docker policy: test3 (version 1)
 Result: ❌ Failed!
 
 Errors:
-  * Host key types did not match. Expected (required): ['ssh-rsa', 'ssh-dss', 'key_alg1']; Expected (optional): None; Actual: ['ssh-rsa', 'ssh-dss']
+  * Host keys did not match. Expected: ['ssh-rsa', 'ssh-dss', 'key_alg1']; Actual: ['ssh-rsa', 'ssh-dss']

test/docker/expected_results/openssh_5.6p1_policy_test4.json

@@ -1 +1 @@
-{"errors": ["Ciphers did not match. Expected: ['cipher_alg1', 'cipher_alg2']; Actual: ['aes128-ctr', 'aes192-ctr', 'aes256-ctr', 'arcfour256', 'arcfour128', 'aes128-cbc', '3des-cbc', 'blowfish-cbc', 'cast128-cbc', 'aes192-cbc', 'aes256-cbc', 'arcfour', 'rijndael-cbc@lysator.liu.se']"], "host": "localhost", "passed": false, "policy": "Docker policy: test4 (version 1)"}
+{"errors": [{"actual": ["aes128-ctr", "aes192-ctr", "aes256-ctr", "arcfour256", "arcfour128", "aes128-cbc", "3des-cbc", "blowfish-cbc", "cast128-cbc", "aes192-cbc", "aes256-cbc", "arcfour", "rijndael-cbc@lysator.liu.se"], "expected_optional": [""], "expected_required": ["cipher_alg1", "cipher_alg2"], "mismatched_field": "Ciphers"}], "host": "localhost", "passed": false, "policy": "Docker policy: test4 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test5.json

@@ -1 +1 @@
-{"errors": ["MACs did not match. Expected: ['hmac-md5', 'hmac-sha1', 'umac-64@openssh.com', 'hmac-ripemd160', 'hmac-ripemd160@openssh.com', 'hmac_alg1', 'hmac-md5-96']; Actual: ['hmac-md5', 'hmac-sha1', 'umac-64@openssh.com', 'hmac-ripemd160', 'hmac-ripemd160@openssh.com', 'hmac-sha1-96', 'hmac-md5-96']"], "host": "localhost", "passed": false, "policy": "Docker policy: test5 (version 1)"}
+{"errors": [{"actual": ["hmac-md5", "hmac-sha1", "umac-64@openssh.com", "hmac-ripemd160", "hmac-ripemd160@openssh.com", "hmac-sha1-96", "hmac-md5-96"], "expected_optional": [""], "expected_required": ["hmac-md5", "hmac-sha1", "umac-64@openssh.com", "hmac-ripemd160", "hmac-ripemd160@openssh.com", "hmac_alg1", "hmac-md5-96"], "mismatched_field": "MACs"}], "host": "localhost", "passed": false, "policy": "Docker policy: test5 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test8.json

@@ -1 +1 @@
-{"errors": ["RSA CA key (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 2048; Actual: 1024"], "host": "localhost", "passed": false, "policy": "Docker poliicy: test8 (version 1)"}
+{"errors": [{"actual": ["1024"], "expected_optional": [""], "expected_required": ["2048"], "mismatched_field": "RSA CA key (ssh-rsa-cert-v01@openssh.com) sizes"}], "host": "localhost", "passed": false, "policy": "Docker poliicy: test8 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test9.json

@@ -1 +1 @@
-{"errors": ["RSA hostkey (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 4096; Actual: 3072"], "host": "localhost", "passed": false, "policy": "Docker poliicy: test9 (version 1)"}
+{"errors": [{"actual": ["3072"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "RSA host key (ssh-rsa-cert-v01@openssh.com) sizes"}], "host": "localhost", "passed": false, "policy": "Docker poliicy: test9 (version 1)"}

test/docker/expected_results/openssh_5.6p1_policy_test9.txt

@@ -3,4 +3,4 @@ Policy: Docker poliicy: test9 (version 1)
 Result: ❌ Failed!
 
 Errors:
-  * RSA hostkey (ssh-rsa-cert-v01@openssh.com) sizes did not match.  Expected: 4096; Actual: 3072
+  * RSA host key (ssh-rsa-cert-v01@openssh.com) sizes did not match. Expected: 4096; Actual: 3072

test/docker/expected_results/openssh_8.0p1_policy_test12.json

@@ -1 +1 @@
-{"errors": ["RSA hostkey (rsa-sha2-256) sizes did not match.  Expected: 4096; Actual: 3072", "RSA hostkey (rsa-sha2-512) sizes did not match.  Expected: 4096; Actual: 3072", "RSA hostkey (ssh-rsa) sizes did not match.  Expected: 4096; Actual: 3072"], "host": "localhost", "passed": false, "policy": "Docker policy: test12 (version 1)"}
+{"errors": [{"actual": ["3072"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "RSA host key (rsa-sha2-256) sizes"}, {"actual": ["3072"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "RSA host key (rsa-sha2-512) sizes"}, {"actual": ["3072"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "RSA host key (ssh-rsa) sizes"}], "host": "localhost", "passed": false, "policy": "Docker policy: test12 (version 1)"}

test/docker/expected_results/openssh_8.0p1_policy_test12.txt

@@ -3,6 +3,6 @@ Policy: Docker policy: test12 (version 1)
 Result: ❌ Failed!
 
 Errors:
-  * RSA hostkey (rsa-sha2-256) sizes did not match.  Expected: 4096; Actual: 3072
-  * RSA hostkey (rsa-sha2-512) sizes did not match.  Expected: 4096; Actual: 3072
-  * RSA hostkey (ssh-rsa) sizes did not match.  Expected: 4096; Actual: 3072
+  * RSA host key (rsa-sha2-256) sizes did not match. Expected: 4096; Actual: 3072
+  * RSA host key (rsa-sha2-512) sizes did not match. Expected: 4096; Actual: 3072
+  * RSA host key (ssh-rsa) sizes did not match. Expected: 4096; Actual: 3072

test/docker/expected_results/openssh_8.0p1_policy_test14.json

@@ -1 +1 @@
-{"errors": ["Group exchange (diffie-hellman-group-exchange-sha256) modulus sizes did not match.  Expected: 4096; Actual: 2048"], "host": "localhost", "passed": false, "policy": "Docker policy: test14 (version 1)"}
+{"errors": [{"actual": ["2048"], "expected_optional": [""], "expected_required": ["4096"], "mismatched_field": "Group exchange (diffie-hellman-group-exchange-sha256) modulus sizes"}], "host": "localhost", "passed": false, "policy": "Docker policy: test14 (version 1)"}

test/test_policy.py

@@ -202,9 +202,11 @@ macs = mac_alg1, mac_alg2, mac_alg3'''
         policy_data = self.Policy.create('www.l0l.com', None, kex, False)
         policy = self.Policy(policy_data=policy_data)
 
-        ret, errors = policy.evaluate('SSH Server 1.0', kex)
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', kex)
         assert ret is True
         assert len(errors) == 0
+        print(error_str)
+        assert len(error_str) == 0
 
 
     def test_policy_evaluate_failing_1(self):
@@ -220,10 +222,10 @@ ciphers = cipher_alg1, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 1
-        assert errors[0].find('Banner did not match.') != -1
+        assert error_str.find('Banner did not match.') != -1
 
 
     def test_policy_evaluate_failing_2(self):
@@ -238,10 +240,10 @@ ciphers = cipher_alg1, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 1
-        assert errors[0].find('Compression types did not match.') != -1
+        assert error_str.find('Compression did not match.') != -1
 
 
     def test_policy_evaluate_failing_3(self):
@@ -256,10 +258,10 @@ ciphers = cipher_alg1, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 1
-        assert errors[0].find('Host key types did not match.') != -1
+        assert error_str.find('Host keys did not match.') != -1
 
 
     def test_policy_evaluate_failing_4(self):
@@ -274,10 +276,10 @@ ciphers = cipher_alg1, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 1
-        assert errors[0].find('Key exchanges did not match.') != -1
+        assert error_str.find('Key exchanges did not match.') != -1
 
 
     def test_policy_evaluate_failing_5(self):
@@ -292,10 +294,10 @@ ciphers = cipher_alg1, XXXmismatched, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 1
-        assert errors[0].find('Ciphers did not match.') != -1
+        assert error_str.find('Ciphers did not match.') != -1
 
 
     def test_policy_evaluate_failing_6(self):
@@ -310,10 +312,10 @@ ciphers = cipher_alg1, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, XXXmismatched, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 1
-        assert errors[0].find('MACs did not match.') != -1
+        assert error_str.find('MACs did not match.') != -1
 
 
     def test_policy_evaluate_failing_7(self):
@@ -328,10 +330,8 @@ ciphers = cipher_alg1, cipher_alg2, cipher_alg3
 macs = mac_alg1, mac_alg2, XXXmismatchedXXX, mac_alg3'''
 
         policy = self.Policy(policy_data=policy_data)
-        ret, errors = policy.evaluate('SSH Server 1.0', self._get_kex())
+        ret, errors, error_str = policy.evaluate('SSH Server 1.0', self._get_kex())
         assert ret is False
         assert len(errors) == 2
-
-        errors_str = ', '.join(errors)
-        assert errors_str.find('Host key types did not match.') != -1
-        assert errors_str.find('MACs did not match.') != -1
+        assert error_str.find('Host keys did not match.') != -1
+        assert error_str.find('MACs did not match.') != -1