openwall / john

John the Ripper jumbo - advanced offline password cracker, which supports hundreds of hash and cipher types, and runs on many operating systems, CPUs, GPUs, and even some FPGAs
https://www.openwall.com/john/
Other
10.33k stars 2.1k forks source link

pcap2john is not printing anything for HSRP #5499

Closed SynacktivCerv closed 4 months ago

SynacktivCerv commented 4 months ago

pcap2john is not printing anything for HSRP. This sample can be used as an example: https://github.com/openwall/john-samples/blob/main/HSRP/packet-captures/10-0.1-1408898406.pcap.

I already have a fix for Python 3 (but regarding this documentation: https://github.com/openwall/john/blob/bleeding-jumbo/CONTRIBUTING.md) I begin with an issue.

def pcap_parser_hsrp(fname):

    f = open(fname, "rb")
    pcap = dpkt.pcap.Reader(f)

    for _, buf in pcap:
        eth = dpkt.ethernet.Ethernet(buf)
        if eth.type == dpkt.ethernet.ETH_TYPE_IP:
            ip = eth.data

            if ip.v != 4:  # IPv6 is a fad
                continue

            if ip.p != dpkt.ip.IP_PROTO_UDP:
                continue

            udp = ip.data
            hsrp = udp.data

            if udp.dport != 1985:  # is this HSRP traffic?
                continue

            #OLD CODE
            #if ord(hsrp[0]) != 0:  # HSRP version
            #FIX
            if hsrp[0] != 0:  # HSRP version
                continue

            if len(hsrp) <= 28:  # doesn't use MD5 authentication
                continue

            if len(hsrp) != 50:  # 20 bytes HSRP + 30 bytes for the MD5 authentication payload
                continue

            #OLD CODE
            #auth_type = ord(hsrp[20])
            #FIX
            auth_type = hsrp[20]

            if auth_type != 4:
                continue
            #OLD CODE
            #h = hsrp[-16:].encode("hex")  # MD5 hash
            #FIX
            h = hsrp[-16:].hex()  # MD5 hash

            # 20 bytes (HSRP) + 14 (till "keyid") + zero padding (double-check this) to make 50 bytes!
            #OLD CODE
            #salt = hsrp.encode("hex")[:68] + ("\x00" * (50 - 20 - 14)).encode("hex")
            #FIX
            salt = hsrp.hex()[:68] + (b"\x00" * (50 - 20 - 14)).hex()
            print("$hsrp$%s$%s" % (salt, h))

    f.close()
solardiz commented 4 months ago

Thanks! Was the issue Python 3 incompatibility? Is your suggested code compatible with both 2 and 3?

@exploide Do you have any comments on this, should @SynacktivCerv go ahead and submit a PR with the proposed changes?

I see we seem to have had a similar(?) issue for vrrp:

commit db4bd780ad40d0085dbdda2981c7fab09fe43fe0
Author: exploide <me@exploide.net>
Date:   Mon Apr 12 18:59:39 2021 +0200

    pcap2john: fixed vrrp parsing

    didn't print anything with python3
    it's now compatible with python2 and python3
    simplyfied parsing using scapy
SynacktivCerv commented 4 months ago

Yes this is a Python 3 incompatibility. My suggested code was not tested in Python 2.

exploide commented 4 months ago

Was the issue Python 3 incompatibility? Is your suggested code compatible with both 2 and 3?

I don't think it is still compatible with Python 2 because indexing the (byte) string in Python 2 will return a char (hence the use of ord) but in Python 3 will return an int. Either we drop Python 2 support or need to apply a workaround to be compatible with both versions.

I see we seem to have had a similar(?) issue for vrrp

Similar yes. My fix was to rewrite the whole function using scapy instead of the dpkt library for parsing. But this is more work than the quick fix here.

should @SynacktivCerv go ahead and submit a PR with the proposed changes?

Generally yes, I also prefer code reviews in pull requests instead of issues. But it depends on your decision concerning Python 2 compatibility.

(By the way, reading the code would be easier in proper code blocks with syntax highlighting:

```python
code here
```

)

exploide commented 4 months ago

My fix was to rewrite the whole function using scapy instead of the dpkt library for parsing.

Did this again. Should be fixed with #5503.

solardiz commented 4 months ago

Thank you both! I've just merged @exploide's PR, trusting his testing as documented in the PR.