jtesta / ssh-audit

SSH server & client security auditing (banner, key exchange, encryption, mac, compression, compatibility, security, etc)
MIT License
3.23k stars 165 forks source link

Make host key test class reusable #278

Closed dlenskiSB closed 12 hours ago

dlenskiSB commented 4 days ago

Because the HostKeyTest class was mutating its static/global HOST_KEY_TYPES dict, this class could not actually be used more than once in a single thread!

Rather than mutate this dict after parsing each key type (HOST_KEY_TYPES[host_key_type]['parsed'] = True), the perform_test method should simple add the parsed key types to a local set().

jtesta commented 3 days ago

Thanks for this. I'm not entirely clear on what the practical problem would be, though. Does the current code result in a crash? Or inaccurate results?

dlenskiSB commented 2 days ago

Does the current code result in a crash? Or inaccurate results?

Yes, the latter. Here's a test.py that should demonstrate the problem: try running test.py host1 host2 … hostN (without merging this PR).

You'll see that it cannot find any key type more than once (total!!) because it has marked that key type as already found/parsed in a global data structure.

import sys
from pprint import pprint
from binascii import b2a_base64

from ssh_audit.outputbuffer import OutputBuffer
from ssh_audit.ssh_socket import SSH_Socket
from ssh_audit.ssh2_kex import SSH2_Kex
from ssh_audit.hostkeytest import HostKeyTest

for ii in range(1, len(sys.argv)):
    host, *port = sys.argv[ii].rsplit(':', 1)
    port = int(port) if port else 22

    print("\n******** Trying HostKeyTest for %s:%d\n" % (host, port))

    # Do initial connect and KEX
    out = OutputBuffer()
    #out.debug = True
    ssh = SSH_Socket(out, host, port)
    ssh.connect()
    banner, header, err = ssh.get_banner(2)
    ssh.send_kexinit()
    packet_type, payload = ssh.read_packet(2)

    kex = SSH2_Kex.parse(out, payload)
    print("Host supports these key_algorithms:")
    pprint(kex.key_algorithms)

    # 🚨🚨 This assertion will fail on ii > 0 if uncommented: 🚨🚨
    # assert not any(v.get('parsed') for v in HostKeyTest.HOST_KEY_TYPES.values())

    # Get keys of specific algorithms
    HostKeyTest.run(out, ssh, kex)
    print("Host keys:")
    for kt, d in kex.host_keys().items():
        blob = d['raw_hostkey_bytes']
        assert len(blob) > 4
        kt_len = int.from_bytes(blob[:4], 'big')
        assert len(blob) >= 4 + kt_len
        keyline = blob[4: 4+kt_len] + b' ' + b2a_base64(blob, newline=False)
        print(keyline.decode())
jtesta commented 12 hours ago

Merged. Thanks for working on this!