From 11905ed44afcd173d77483c7c57cf63e3e2f0beb Mon Sep 17 00:00:00 2001 From: Joe Testa Date: Sun, 13 Mar 2022 20:58:22 -0400 Subject: [PATCH] Fixed pylint errors, consolidated error checking for granular GEX tests, renamed functions for better readability. --- README.md | 1 + src/ssh_audit/gextest.py | 40 ++++++++++------------------- src/ssh_audit/ssh_audit.py | 52 +++++++++++++++++++++----------------- ssh-audit.1 | 23 ++++++----------- 4 files changed, 51 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index 0a252b0..683c12a 100644 --- a/README.md +++ b/README.md @@ -172,6 +172,7 @@ For convenience, a web front-end on top of the command-line tool is available at ## ChangeLog ### v2.6.0-dev + - Added `-g` and `--gex-test` for granular GEX modulus size tests; credit [Adam Russell](https://github.com/thecliguy). - Snap packages now print more user-friendly error messages when permission errors are encountered. - JSON 'target' field now always includes port number; credit [tomatohater1337](https://github.com/tomatohater1337). - Added 24 new key exchanges: `ecdh-sha2-1.3.132.0.1`, `ecdh-sha2-1.2.840.10045.3.1.1`, `ecdh-sha2-1.3.132.0.33`, `ecdh-sha2-1.3.132.0.26`, `ecdh-sha2-1.3.132.0.27`, `ecdh-sha2-1.2.840.10045.3.1.7`, `ecdh-sha2-1.3.132.0.16`, `ecdh-sha2-1.3.132.0.34`, `ecdh-sha2-1.3.132.0.36`, `ecdh-sha2-1.3.132.0.37`, `ecdh-sha2-1.3.132.0.35`, `ecdh-sha2-1.3.132.0.38`, `ecdh-sha2-4MHB+NBt3AlaSRQ7MnB4cg==`, `ecdh-sha2-5pPrSUQtIaTjUSt5VZNBjg==`, `ecdh-sha2-VqBg4QRPjxx1EXZdV0GdWQ==`, `ecdh-sha2-zD/b3hu/71952ArpUG4OjQ==`, `ecdh-sha2-qCbG5Cn/jjsZ7nBeR7EnOA==`, `ecdh-sha2-9UzNcgwTlEnSCECZa7V1mw==`, `ecdh-sha2-wiRIU8TKjMZ418sMqlqtvQ==`, `ecdh-sha2-qcFQaMAMGhTziMT0z+Tuzw==`, `ecdh-sha2-m/FtSAmrV4j/Wy6RVUaK7A==`, `ecdh-sha2-D3FefCjYoJ/kfXgAyLddYA==`, `ecdh-sha2-h/SsxnLCtRBh7I9ATyeB3A==`, `ecdh-sha2-mNVwCXAoS1HGmHpLvBC94w==`. diff --git a/src/ssh_audit/gextest.py b/src/ssh_audit/gextest.py index b8e203c..7cf6f27 100644 --- a/src/ssh_audit/gextest.py +++ b/src/ssh_audit/gextest.py @@ -21,13 +21,12 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ +import traceback # pylint: disable=unused-import from typing import Dict, List, Set, Sequence, Tuple, Iterable # noqa: F401 from typing import Callable, Optional, Union, Any # noqa: F401 -import traceback - from ssh_audit.kexdh import KexGroupExchange_SHA1, KexGroupExchange_SHA256 from ssh_audit.ssh2_kexdb import SSH2_KexDB from ssh_audit.ssh2_kex import SSH2_Kex @@ -71,11 +70,10 @@ class GEXTest: return True - # Tests for modulus size in bits against the specified target. @staticmethod - def modulus_size_test(out: 'OutputBuffer', s: 'SSH_Socket', kex: 'SSH2_Kex', bits_min: int, bits_pref: int, bits_max: int, modulus_dict: Dict[str, List[int]]) -> int: + def granular_modulus_size_test(out: 'OutputBuffer', s: 'SSH_Socket', kex: 'SSH2_Kex', bits_min: int, bits_pref: int, bits_max: int, modulus_dict: Dict[str, List[int]]) -> int: ''' - Tests for modulus size in bits against the target target. + Tests for granular modulus sizes. Builds a dictionary, where a key represents a DH algorithm name and the values are the modulus sizes (in bits) that have been returned by the target server. @@ -89,10 +87,6 @@ class GEXTest: out.d("Bits Pref: " + str(bits_pref)) out.d("Bits Max: " + str(bits_max)) - if all(x < 0 for x in (bits_min, bits_pref, bits_max)): - out.fail("min, pref and max values cannot be negative.") - return exitcodes.FAILURE - GEX_ALGS = { 'diffie-hellman-group-exchange-sha1': KexGroupExchange_SHA1, 'diffie-hellman-group-exchange-sha256': KexGroupExchange_SHA256, @@ -100,11 +94,11 @@ class GEXTest: # Check if the server supports any of the group-exchange # algorithms. If so, test each one. - for gex_alg in GEX_ALGS: + for gex_alg, kex_group_class in GEX_ALGS.items(): if gex_alg not in kex.kex_algorithms: out.d('Server does not support the algorithm "' + gex_alg + '".', write_now=True) else: - kex_group = GEX_ALGS[gex_alg]() + kex_group = kex_group_class() out.d('Preparing to perform DH group exchange using ' + gex_alg + ' with min, pref and max modulus sizes of ' + str(bits_min) + ' bits, ' + str(bits_pref) + ' bits and ' + str(bits_max) + ' bits...', write_now=True) # It has been observed that reconnecting to some SSH servers @@ -121,11 +115,8 @@ class GEXTest: kex_group.recv_reply(s, False) modulus_size_returned = kex_group.get_dh_modulus_size() out.d('Modulus size returned by server: ' + str(modulus_size_returned) + ' bits', write_now=True) - except Exception as e: - out.d('[exception] ' + str(e), write_now=True) - # import traceback - # print(traceback.format_exc()) - pass + except Exception: + out.d('[exception] ' + str(traceback.format_exc()), write_now=True) finally: # The server is in a state that is not re-testable, # so there's nothing else to do with this open @@ -159,8 +150,7 @@ class GEXTest: # algorithms. If so, test each one. for gex_alg, kex_group_class in GEX_ALGS.items(): if gex_alg in kex.kex_algorithms: - weak_sizes = 512, 1024, 1536 - out.d('Preparing to perform DH group exchange using ' + gex_alg + ' with min, pref and max modulus sizes of ' + str(weak_sizes[0]) + ' bits, ' + str(weak_sizes[1]) + ' bits and ' + str(weak_sizes[2]) + ' bits...', write_now=True) + out.d('Preparing to perform DH group exchange using ' + gex_alg + ' with min, pref and max modulus sizes of 512 bits, 1024 bits and 1536 bits...', write_now=True) if GEXTest.reconnect(out, s, kex, gex_alg) is False: break @@ -170,7 +160,7 @@ class GEXTest: # First try a range of weak sizes. try: - kex_group.send_init_gex(s, weak_sizes[0], weak_sizes[1], weak_sizes[2]) + kex_group.send_init_gex(s, 512, 1024, 1536) kex_group.recv_reply(s, False) # Its been observed that servers will return a group @@ -179,9 +169,8 @@ class GEXTest: smallest_modulus = kex_group.get_dh_modulus_size() out.d('Modulus size returned by server: ' + str(smallest_modulus) + ' bits', write_now=True) - except Exception as e: - out.d('[exception] ' + str(e), write_now=True) - pass + except Exception: + out.d('[exception] ' + str(traceback.format_exc()), write_now=True) finally: s.close() @@ -205,11 +194,8 @@ class GEXTest: kex_group.recv_reply(s, False) smallest_modulus = kex_group.get_dh_modulus_size() out.d('Modulus size returned by server: ' + str(smallest_modulus) + ' bits', write_now=True) - except Exception as e: - out.d('[exception] ' + str(e), write_now=True) - # import traceback - # print(traceback.format_exc()) - pass + except Exception: + out.d('[exception] ' + str(traceback.format_exc()), write_now=True) finally: # The server is in a state that is not re-testable, # so there's nothing else to do with this open diff --git a/src/ssh_audit/ssh_audit.py b/src/ssh_audit/ssh_audit.py index 6616267..6fe2d12 100755 --- a/src/ssh_audit/ssh_audit.py +++ b/src/ssh_audit/ssh_audit.py @@ -87,7 +87,7 @@ def usage(err: Optional[str] = None) -> None: uout.info(' -b, --batch batch output') uout.info(' -c, --client-audit starts a server on port 2222 to audit client\n software config (use -p to change port;\n use -t to change timeout)') uout.info(' -d, --debug debug output') - uout.info(' -g, --gex-test= dh gex modulus size test') + uout.info(' -g, --gex-test= dh gex modulus size test') uout.info(' -j, --json JSON output (use -jj to enable indents)') uout.info(' -l, --level= minimum output level (info|warn|fail)') uout.info(' -L, --list-policies list all the official, built-in policies') @@ -654,10 +654,29 @@ def process_commandline(out: OutputBuffer, args: List[str], usage_cb: Callable[. aconf.debug = True out.debug = True elif o in ('-g', '--gex-test'): - if not((any(re.search(regex_str, a) for regex_str in get_permitted_syntax_for_gex_test().values()))): + permitted_syntax = get_permitted_syntax_for_gex_test() + + if not any(re.search(regex_str, a) for regex_str in permitted_syntax.values()): usage_cb('{} {} is not valid'.format(o, a)) + + if re.search(permitted_syntax['RANGE'], a): + extracted_digits = re.findall(r'\d+', a) + bits_left_bound = int(extracted_digits[0]) + bits_right_bound = int(extracted_digits[1]) + + bits_step = 1 + if (len(extracted_digits)) == 3: + bits_step = int(extracted_digits[2]) + + if bits_step <= 0: + usage_cb('{} {} is not valid'.format(o, bits_step)) + + if all(x < 0 for x in (bits_left_bound, bits_right_bound)): + usage_cb('{} {} {} is not valid'.format(o, bits_left_bound, bits_right_bound)) + aconf.gex_test = a + if len(args) == 0 and aconf.client_audit is False and aconf.target_file is None and aconf.list_policies is False and aconf.lookup == '' and aconf.manual is False: usage_cb() @@ -928,8 +947,7 @@ def audit(out: OutputBuffer, aconf: AuditConf, sshv: Optional[int] = None, print if aconf.client_audit is False: HostKeyTest.run(out, s, kex) if aconf.gex_test != '': - program_retval = invoke_modulus_size_test(out, s, kex, aconf) - return program_retval + return run_gex_granular_modulus_size_test(out, s, kex, aconf) else: GEXTest.run(out, s, kex) @@ -1079,15 +1097,11 @@ def get_permitted_syntax_for_gex_test() -> Dict[str, str]: return syntax -def invoke_modulus_size_test(out: OutputBuffer, s: 'SSH_Socket', kex: 'SSH2_Kex', aconf: AuditConf) -> int: +def run_gex_granular_modulus_size_test(out: OutputBuffer, s: 'SSH_Socket', kex: 'SSH2_Kex', aconf: AuditConf) -> int: '''Extracts the user specified modulus sizes and submits them for testing against the target server. Returns an exitcodes.* flag.''' permitted_syntax = get_permitted_syntax_for_gex_test() - if not((any(re.search(regex_str, aconf.gex_test) for regex_str in permitted_syntax.values()))): - out.fail("Invalid syntax.") - return exitcodes.FAILURE - mod_dict: Dict[str, List[int]] = {} # Range syntax. @@ -1096,18 +1110,9 @@ def invoke_modulus_size_test(out: OutputBuffer, s: 'SSH_Socket', kex: 'SSH2_Kex' bits_left_bound = int(extracted_digits[0]) bits_right_bound = int(extracted_digits[1]) + bits_step = 1 if (len(extracted_digits)) == 3: bits_step = int(extracted_digits[2]) - else: - bits_step = 1 - - if bits_step <= 0: - out.fail("Step value must be greater than zero.") - return exitcodes.FAILURE - - if all(x < 0 for x in (bits_left_bound, bits_right_bound)): - out.fail("Start and end values cannot be negative.") - return exitcodes.FAILURE # If the left value is greater than the right value, then the sequence # operates from right to left. @@ -1119,7 +1124,7 @@ def invoke_modulus_size_test(out: OutputBuffer, s: 'SSH_Socket', kex: 'SSH2_Kex' out.v("A separate test will be performed against each of the following modulus sizes: " + ", ".join([str(x) for x in bits_in_range_to_test]) + ".", write_now=True) for i_bits in bits_in_range_to_test: - program_retval = GEXTest.modulus_size_test(out, s, kex, i_bits, i_bits, i_bits, mod_dict) + program_retval = GEXTest.granular_modulus_size_test(out, s, kex, i_bits, i_bits, i_bits, mod_dict) if program_retval != exitcodes.GOOD: return program_retval @@ -1128,15 +1133,16 @@ def invoke_modulus_size_test(out: OutputBuffer, s: 'SSH_Socket', kex: 'SSH2_Kex' bits_in_list_to_test = aconf.gex_test.split(',') out.v("A separate test will be performed against each of the following modulus sizes: " + ", ".join([str(x) for x in bits_in_list_to_test]) + ".", write_now=True) for s_bits in bits_in_list_to_test: - program_retval = GEXTest.modulus_size_test(out, s, kex, int(s_bits), int(s_bits), int(s_bits), mod_dict) + program_retval = GEXTest.granular_modulus_size_test(out, s, kex, int(s_bits), int(s_bits), int(s_bits), mod_dict) if program_retval != exitcodes.GOOD: return program_retval + if re.search(permitted_syntax['LIST_WITH_MIN_PREF_MAX'], aconf.gex_test): sets_of_min_pref_max = aconf.gex_test.split(',') out.v("A separate test will be performed against each of the following sets of 'min:pref:max' modulus sizes: " + ', '.join(sets_of_min_pref_max), write_now=True) for set_of_min_pref_max in sets_of_min_pref_max: bits_in_list_to_test = set_of_min_pref_max.split(':') - program_retval = GEXTest.modulus_size_test(out, s, kex, int(bits_in_list_to_test[0]), int(bits_in_list_to_test[1]), int(bits_in_list_to_test[2]), mod_dict) + program_retval = GEXTest.granular_modulus_size_test(out, s, kex, int(bits_in_list_to_test[0]), int(bits_in_list_to_test[1]), int(bits_in_list_to_test[2]), mod_dict) if program_retval != exitcodes.GOOD: return program_retval @@ -1150,7 +1156,7 @@ def invoke_modulus_size_test(out: OutputBuffer, s: 'SSH_Socket', kex: 'SSH2_Kex' for key, value in mod_dict.items(): padding = (max_key_len - len(key)) + 1 - out.info(key + " " * padding + '--> ' + ', '.join(map(str, value))) + out.info(key + " " * padding + '--> ' + ', '.join([str(i) for i in value])) return program_retval diff --git a/ssh-audit.1 b/ssh-audit.1 index f4136a1..43348b8 100644 --- a/ssh-audit.1 +++ b/ssh-audit.1 @@ -1,4 +1,4 @@ -.TH SSH-AUDIT 1 "February 13, 2022" +.TH SSH-AUDIT 1 "March 13, 2022" .SH NAME \fBssh-audit\fP \- SSH server & client configuration auditor .SH SYNOPSIS @@ -52,36 +52,29 @@ Starts a server on port 2222 to audit client software configuration. Use -p/--p Enable debug output. .TP -.B -g, \-\-gex-test= +.B -g, \-\-gex-test= .br Runs a Diffie-Hellman Group Exchange modulus size test against a server. -Diffie-Hellman requires the client and server to agree on a generator value and -a modulus value. In the "Group Exchange" implementation of Diffie-Hellman, the -client specifies the size of the modulus in bits by providing the server with -minimum, preferred and maximum values. The server then finds a group that best -matches the client's request, returning the corresponding generator and modulus. -For a full explanation of this process see RFC 4419 and its successors. +Diffie-Hellman requires the client and server to agree on a generator value and a modulus value. In the "Group Exchange" implementation of Diffie-Hellman, the client specifies the size of the modulus in bits by providing the server with minimum, preferred and maximum values. The server then finds a group that best matches the client's request, returning the corresponding generator and modulus. For a full explanation of this process see RFC 4419 and its successors. -This test acts as a client by providing an SSH server with the size of a modulus -and then obtains the size of the modulus returned by the server. +This test acts as a client by providing an SSH server with the size of a modulus and then obtains the size of the modulus returned by the server. Three types of syntax are supported: 1. - + A comma delimited list of modulus sizes. A test is performed against each value in the list where it acts as the minimum, preferred and maximum modulus size. - 2. - + A set of three colon delimited values denoting minimum, preferred and maximum modulus size. A test is performed against each set. Multiple sets can specified as a comma separated list. - 3. - + 3. + A range of modulus sizes with an optional step value. Step defaults to 1 if omitted. If the left value is greater than the right value, then the sequence operates from right to left. A test is performed against each value in the range where it acts as the minimum, preferred and maximum modulus size.