From 9fae870260ee0067fd09840bc696b9459c4dba52 Mon Sep 17 00:00:00 2001 From: Joe Testa Date: Tue, 19 Mar 2024 14:45:19 -0400 Subject: [PATCH] Added allow_larger_keys flag to custom policies to control whether targets can have larger keys, and added Docker tests to complete work started in PR #242. --- README.md | 1 + docker_test.sh | 3 +++ src/ssh_audit/policy.py | 19 ++++++++++++++----- .../openssh_8.0p1_custom_policy_test17.json | 6 ++++++ .../openssh_8.0p1_custom_policy_test17.txt | 3 +++ test/docker/policies/policy_test17.txt | 15 +++++++++++++++ test/test_policy.py | 2 +- 7 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 test/docker/expected_results/openssh_8.0p1_custom_policy_test17.json create mode 100644 test/docker/expected_results/openssh_8.0p1_custom_policy_test17.txt create mode 100644 test/docker/policies/policy_test17.txt diff --git a/README.md b/README.md index 0047412..5a630c8 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,7 @@ For convenience, a web front-end on top of the command-line tool is available at - Snap builds are now architecture-independent. - Changed Docker base image from `python:3-slim` to `python:3-alpine`, resulting in a 59% reduction in image size; credit [Daniel Thamdrup](https://github.com/dallemon). - Custom policies now support the `allow_algorithm_subset_and_reordering` directive to allow targets to pass with a subset and/or re-ordered list of host keys, kex, ciphers, and MACs. This allows for the creation of a baseline policy where targets can optionally implement stricter controls; partial credit [yannik1015](https://github.com/yannik1015). + - Custom policies now support the `allow_larger_keys` directive to allow targets to pass with larger host keys, CA keys, and Diffie-Hellman keys. This allows for the creation of a baseline policy where targets can optionally implement stricter controls; partial credit [Damian Szuberski](https://github.com/szubersk). - Added 1 new key exchange algorithm: `gss-nistp384-sha384-*`. ### v3.1.0 (2023-12-20) diff --git a/docker_test.sh b/docker_test.sh index f169b64..3c14ef0 100755 --- a/docker_test.sh +++ b/docker_test.sh @@ -790,6 +790,9 @@ run_custom_policy_test "config2" "test15" "${PROGRAM_RETVAL_GOOD}" # Failing test with algorithm subset matching. run_custom_policy_test "config2" "test16" "${PROGRAM_RETVAL_FAILURE}" +# Passing test with larger key matching. +run_custom_policy_test "config2" "test17" "${PROGRAM_RETVAL_GOOD}" + # Failing test for built-in OpenSSH 8.0p1 server policy (RSA host key size is 3072 instead of 4096). run_builtin_policy_test "Hardened OpenSSH Server v8.0 (version 4)" "8.0p1" "test1" "-o HostKeyAlgorithms=rsa-sha2-512,rsa-sha2-256,ssh-ed25519 -o KexAlgorithms=curve25519-sha256,curve25519-sha256@libssh.org,diffie-hellman-group16-sha512,diffie-hellman-group18-sha512,diffie-hellman-group-exchange-sha256 -o Ciphers=chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr -o MACs=hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,umac-128-etm@openssh.com" "${PROGRAM_RETVAL_FAILURE}" diff --git a/src/ssh_audit/policy.py b/src/ssh_audit/policy.py index ef3b155..bd0bee6 100644 --- a/src/ssh_audit/policy.py +++ b/src/ssh_audit/policy.py @@ -55,6 +55,7 @@ class Policy: self._dh_modulus_sizes: Optional[Dict[str, int]] = None self._server_policy = True self._allow_algorithm_subset_and_reordering = False + self._allow_larger_keys = False self._errors: List[Any] = [] self._name_and_version: str = '' @@ -114,7 +115,7 @@ class Policy: key = key.strip() val = val.strip() - if key not in ['name', 'version', 'banner', 'compressions', 'host keys', 'optional host keys', 'key exchanges', 'ciphers', 'macs', 'client policy', 'host_key_sizes', 'dh_modulus_sizes', 'allow_algorithm_subset_and_reordering'] and not key.startswith('hostkey_size_') and not key.startswith('cakey_size_') and not key.startswith('dh_modulus_size_'): + if key not in ['name', 'version', 'banner', 'compressions', 'host keys', 'optional host keys', 'key exchanges', 'ciphers', 'macs', 'client policy', 'host_key_sizes', 'dh_modulus_sizes', 'allow_algorithm_subset_and_reordering', 'allow_larger_keys'] and not key.startswith('hostkey_size_') and not key.startswith('cakey_size_') and not key.startswith('dh_modulus_size_'): raise ValueError("invalid field found in policy: %s" % line) if key in ['name', 'banner']: @@ -209,6 +210,8 @@ class Policy: self._server_policy = False elif key == 'allow_algorithm_subset_and_reordering' and val.lower() == 'true': self._allow_algorithm_subset_and_reordering = True + elif key == 'allow_larger_keys' and val.lower() == 'true': + self._allow_larger_keys = True if self._name is None: raise ValueError('The policy does not have a name field.') @@ -296,9 +299,12 @@ name = "Custom Policy (based on %s on %s)" # The version of this policy (displayed in the output during scans). Not parsed, and may be any value, including strings. version = 1 -# When false, host keys, kex, ciphers, and MAC lists must match exactly. When true, the target host may support a subset of the specified algorithms and/or algorithms may appear in a different order; this is useful for specifying a baseline and allowing some hosts the option to implement stricter controls. +# When false, host keys, kex, ciphers, and MAC lists must match exactly. When true, the target host may support a subset of the specified algorithms and/or algorithms may appear in a different order; this feature is useful for specifying a baseline and allowing some hosts the option to implement stricter controls. allow_algorithm_subset_and_reordering = false +# When false, host keys, CA keys, and Diffie-Hellman key sizes must exactly match what's specified in this policy. When true, target systems are allowed to have larger keys; this feature is useful for specifying a baseline and allowing some hosts the option to implement stricter controls. +allow_larger_keys = false + # The banner that must match exactly. Commented out to ignore banners, since minor variability in the banner is sometimes normal. # banner = "%s" @@ -371,7 +377,8 @@ macs = %s server_host_keys = kex.host_keys() if hostkey_type in server_host_keys: actual_hostkey_size = cast(int, server_host_keys[hostkey_type]['hostkey_size']) - if actual_hostkey_size < expected_hostkey_size: + if (self._allow_larger_keys and actual_hostkey_size < expected_hostkey_size) or \ + (not self._allow_larger_keys and actual_hostkey_size != expected_hostkey_size): ret = False self._append_error('Host key (%s) sizes' % hostkey_type, [str(expected_hostkey_size)], None, [str(actual_hostkey_size)]) @@ -387,7 +394,8 @@ macs = %s ret = False self._append_error('CA signature type', [expected_ca_key_type], None, [actual_ca_key_type]) # Ensure that the actual and expected signature sizes match. - elif actual_ca_key_size < expected_ca_key_size: + elif (self._allow_larger_keys and actual_ca_key_size < expected_ca_key_size) or \ + (not self._allow_larger_keys and actual_ca_key_size != expected_ca_key_size): ret = False self._append_error('CA signature size (%s)' % actual_ca_key_type, [str(expected_ca_key_size)], None, [str(actual_ca_key_size)]) @@ -446,7 +454,8 @@ macs = %s expected_dh_modulus_size = self._dh_modulus_sizes[dh_modulus_type] if dh_modulus_type in kex.dh_modulus_sizes(): actual_dh_modulus_size = kex.dh_modulus_sizes()[dh_modulus_type] - if expected_dh_modulus_size > actual_dh_modulus_size: + if (self._allow_larger_keys and actual_dh_modulus_size < expected_dh_modulus_size) or \ + (not self._allow_larger_keys and actual_dh_modulus_size != expected_dh_modulus_size): ret = False self._append_error('Group exchange (%s) modulus sizes' % dh_modulus_type, [str(expected_dh_modulus_size)], None, [str(actual_dh_modulus_size)]) diff --git a/test/docker/expected_results/openssh_8.0p1_custom_policy_test17.json b/test/docker/expected_results/openssh_8.0p1_custom_policy_test17.json new file mode 100644 index 0000000..b7286af --- /dev/null +++ b/test/docker/expected_results/openssh_8.0p1_custom_policy_test17.json @@ -0,0 +1,6 @@ +{ + "errors": [], + "host": "localhost", + "passed": true, + "policy": "Docker policy: test17 (version 1)" +} diff --git a/test/docker/expected_results/openssh_8.0p1_custom_policy_test17.txt b/test/docker/expected_results/openssh_8.0p1_custom_policy_test17.txt new file mode 100644 index 0000000..7957c73 --- /dev/null +++ b/test/docker/expected_results/openssh_8.0p1_custom_policy_test17.txt @@ -0,0 +1,3 @@ +Host: localhost:2222 +Policy: Docker policy: test17 (version 1) +Result: ✔ Passed diff --git a/test/docker/policies/policy_test17.txt b/test/docker/policies/policy_test17.txt new file mode 100644 index 0000000..6cfc9d9 --- /dev/null +++ b/test/docker/policies/policy_test17.txt @@ -0,0 +1,15 @@ +# +# Docker policy: test17 +# + +name = "Docker policy: test17" +version = 1 +allow_larger_keys = true +banner = "SSH-2.0-OpenSSH_8.0" +compressions = none, zlib@openssh.com +host keys = rsa-sha2-512, rsa-sha2-256, ssh-rsa, ecdsa-sha2-nistp256, ssh-ed25519 +key exchanges = curve25519-sha256, curve25519-sha256@libssh.org, ecdh-sha2-nistp256, ecdh-sha2-nistp384, ecdh-sha2-nistp521, diffie-hellman-group-exchange-sha256, diffie-hellman-group16-sha512, diffie-hellman-group18-sha512, diffie-hellman-group14-sha256, diffie-hellman-group14-sha1 +ciphers = chacha20-poly1305@openssh.com, aes128-ctr, aes192-ctr, aes256-ctr, aes128-gcm@openssh.com, aes256-gcm@openssh.com +macs = umac-64-etm@openssh.com, umac-128-etm@openssh.com, hmac-sha2-256-etm@openssh.com, hmac-sha2-512-etm@openssh.com, hmac-sha1-etm@openssh.com, umac-64@openssh.com, umac-128@openssh.com, hmac-sha2-256, hmac-sha2-512, hmac-sha1 +host_key_sizes = {"ssh-rsa": {"hostkey_size": 2048}, "rsa-sha2-256": {"hostkey_size": 2048}, "rsa-sha2-512": {"hostkey_size": 2048}, "ssh-ed25519": {"hostkey_size": 256}} +dh_modulus_sizes = {"diffie-hellman-group-exchange-sha256": 2048} diff --git a/test/test_policy.py b/test/test_policy.py index 9cb9385..cca9e3e 100644 --- a/test/test_policy.py +++ b/test/test_policy.py @@ -297,7 +297,7 @@ macs = mac_alg1, mac_alg2, mac_alg3''' pol_data = pol_data.replace(date.today().strftime('%Y/%m/%d'), '[todays date]') # Instead of writing out the entire expected policy--line by line--just check that it has the expected hash. - assert hashlib.sha256(pol_data.encode('ascii')).hexdigest() == '4b504b799f6b964a20ccbe8af7edd26c7b5f0e0b98070e754ea41dccdace33b4' + assert hashlib.sha256(pol_data.encode('ascii')).hexdigest() == 'fb84bce442cff2bce9bf653d6373a8a938e3bfcfbd1e876f51a08c1842df3cff' def test_policy_evaluate_passing_1(self):