Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VoIP decoder #93

Merged
merged 5 commits into from
Nov 1, 2016
Merged

VoIP decoder #93

merged 5 commits into from
Nov 1, 2016

Conversation

1modm
Copy link
Contributor

@1modm 1modm commented Oct 3, 2016

SIP and RTP decoders

@dev195
Copy link
Contributor

dev195 commented Oct 5, 2016

Very nice! Handling VoIP traffic in Dshell would be very useful, and this seems like a good starting point.

The decoders seem to be working with the traffic samples I threw at them. Some of my thoughts:

rtp.py

  • Maybe change payload_type function into a dictionary that is established in the preModule function. This would be somewhat faster than a really long if-statement in a function.
  • Can this logic be implemented using an IPDecoder as the super class instead of just a raw Decoder? I feel like using the IPHandler function would take care of a lot of the redundant code in your rawHandler (e.g. parsing IP addresses, determining protocols, etc.).
  • The decoder is specifically defined as alerting for every packet. Are you willing to make a connection based one to reduce sheer quantity of output? If not, we might be able to, once we have some free cycles.

sip.py

  • layer5_header function can be replaced with the python dictionary's own 'get' function.
  • Similar thoughts about using IPDecoder as the super class.
  • I think your two if-statements for REQUEST and RESPONSE will always evaluate the same as each other. Also, the logic for both seems very similar. Might be able to merge them.
  • The 'port' argument in optiondict can probably be 'type':'int'. Not a big deal. Just a nicety. Saves you from needing to do the conversion later on.
  • I definitely think this one has potential! I'm seeing something in the future similar to the HTTPDecoder. If we can automatically tie together the requests and responses, that could be very handy.

General

  • Can you make some of the exception-handling more specific? except Exception is usually frowned upon.
  • kw already has the two MAC addresses, so there's no need to generate them again (kw['smac'] and kw['dmac'] for source and destination, respectively)
  • Also, hanging whitespace is in a couple of lines.

In short, the idea is great! It just needs some cleaning up before we can outright accept the pull request.

@1modm
Copy link
Contributor Author

1modm commented Oct 5, 2016

Thank you for the tips! very useful. I think this can be a first approach to start to create more decoders, I will try h323 also.

rtp.py

Maybe change payload_type function into a dictionary that is established in the preModule function. This would be somewhat faster than a really long if-statement in a function.

[+] Done

Can this logic be implemented using an IPDecoder as the super class instead of just a raw Decoder? I feel like using the IPHandler function would take care of a lot of the redundant code in your rawHandler (e.g. parsing IP addresses, determining protocols, etc.).

[-] I tried but I have some samples using Linux Cooked Capture instead ethernet and was the only way to handle. For packets using Linux Cooked Capture I received the next error when was trying to set any filter like ip or udp:

Traceback (most recent call last):
File "Dshell/bin/decode", line 895, in main
decoder.capture.setfilter(decoder.filter)
File "pcap.pyx", line 255, in pcap.pcap.setfilter
OSError: no VLAN support for data link type 113

The decoder is specifically defined as alerting for every packet. Are you willing to make a connection based one to reduce sheer quantity of output? If not, we might be able to, once we have some free cycles.

[-] Do you mean different type of output defined in parameters

Can you make some of the exception-handling more specific? except Exception is usually frowned upon.

[+] Done

kw already has the two MAC addresses, so there's no need to generate them again (kw['smac'] and kw['dmac'] for source and destination, respectively)

[+] Done

sip.py

layer5_header function can be replaced with the python dictionary's own 'get' function.

[+] Done

Similar thoughts about using IPDecoder as the super class.

[-] I tried but I have some samples using Linux Cooked Capture instead ethernet and was the only way to handle. For packets using Linux Cooked Capture I received the next error when was trying to set any filter like ip or udp:

Traceback (most recent call last):
File "Dshell/bin/decode", line 895, in main
decoder.capture.setfilter(decoder.filter)
File "pcap.pyx", line 255, in pcap.pcap.setfilter
OSError: no VLAN support for data link type 113

I think your two if-statements for REQUEST and RESPONSE will always evaluate the same as each other. Also, the logic for both seems very similar. Might be able to merge them.

[+] Done

The 'port' argument in optiondict can probably be 'type':'int'. Not a big deal. Just a nicety. Saves you from needing to do the conversion later on.

[+] Done

I definitely think this one has potential! I'm seeing something in the future similar to the HTTPDecoder. If we can automatically tie together the requests and responses, that could be very handy.

[+] Great idea, I will try!

Can you make some of the exception-handling more specific? except Exception is usually frowned upon.

[+] Done

kw already has the two MAC addresses, so there's no need to generate them again (kw['smac'] and kw['dmac'] for source and destination, respectively)

[+] Done

@dev195
Copy link
Contributor

dev195 commented Oct 6, 2016

Thanks! Appreciate the quick response. The changes look good.

[-] I tried but I have some samples using Linux Cooked Capture instead ethernet and was the only way to handle. For packets using Linux Cooked Capture I received the next error when was trying to set any filter like ip or udp:

We have a flag to help with that. By default, decode.py adds a VLAN wrapper around the Berkeley packet filter (BPF) in use. For cooked capture, that causes issues. If you call decode.py with the --no-vlan flag, it skips over that logic and should run properly (line).

Also, you can explicitly tell Dshell which dpkt module to use with --layer2. Point it to the module to use, like 'ethernet.Ethernet' or 'sll.SLL', and it will automatically attach it to 'dpkt.' and import the module (line).

Here's an example with an old sample capture I have that is also a cooked capture:

$ tcpdump -nnr ~/pcap/tftp-cooked.pcap |head -n5
reading from file /home/user/pcap/tftp-cooked.pcap, link-type LINUX_SLL (Linux cooked)
09:57:30.560643 IP6 ::1.55731 > ::1.55731: UDP, length 16
09:57:35.720876 IP 10.10.10.4.53861 > 10.10.10.2.69:  19 RRQ "sunset.jpg" octet 
09:57:35.722013 IP 10.10.10.2.32778 > 10.10.10.4.53861: UDP, length 516
09:57:35.722781 IP 10.10.10.4.53861 > 10.10.10.2.32778: UDP, length 4
09:57:35.723231 IP 10.10.10.2.32778 > 10.10.10.4.53861: UDP, length 516
$ ./dshell
Dshell> decode -d tftp ~/pcap/tftp-cooked.pcap 
Traceback (most recent call last):
  File "/home/user/dshell/bin/decode", line 895, in main
    decoder.capture.setfilter(decoder.filter)
  File "pcap.pyx", line 224, in pcap.pcap.setfilter (pcap.c:2209)
OSError: no VLAN support for data link type 113
Dshell>
Dshell> decode -d tftp ~/pcap/tftp-cooked.pcap --no-vlan
Dshell>
Dshell> decode -d tftp ~/pcap/tftp-cooked.pcap --no-vlan --layer2=sll.SLL
tftp 2013-03-13 09:57:35       10.10.10.4:53861 --       10.10.10.2:32778 ** read sunset.jpg (90752 bytes)  **
tftp 2013-03-13 09:57:56       10.10.10.4:41710 --       10.10.10.2:32779 ** read palindrome.txt (779 bytes)  **
tftp 2013-03-13 09:58:09       10.10.10.4:34445 --       10.10.10.2:32780 ** read doesnotexist.exe (0 bytes) File not found **
tftp 2013-03-13 09:58:41       10.10.10.4:50915 --       10.10.10.2:32781 ** write sunset2.jpg (90752 bytes)  **

[-] Do you mean different type of output defined in parameters

There are several different types of decoders you can write, but I unofficially keep them straight in my head by referring to them as packet-based decoders and connection-based decoders.

The packet-based decoders (e.g. Decoder, IPDecoder) look at packets individually. Connection-based decoders (e.g. TCPDecoder, HTTPDecoder) attempt to reassemble the connections into unified streams; it does not necessarily look at the packets individually, but the stream as a whole.

I'm not sure which would work best for these protocols (I don't know much about them, personally). My main worry is that, when alerting on individual packets, the decoder can get very noisy. For small traffic samples, it may not seem like much, but it can be overwhelming at professional scales.

On the other hand, if the decoders are just looking for the initial requests and responses, it might not be too bad. It might just be a situation where we see how it goes, and make updates as needed. Judgment call?

p.s. socket.inet_ntoa only works for IPv4. For IPv6, you will need socket.inet_ntop. Dshell handles that internally, but you have to keep that in mind when handling IPs manually. That tripped me up several times, too.

@1modm
Copy link
Contributor Author

1modm commented Oct 11, 2016

Thank you for the tips, I have re-write the decoders using the UDPDecoder, for sip decoder I tried to write using a connection-based decoder but SIP is a text-based protocol with syntax similar to that of HTTP, so you can use the followstream decoder: decode -d followstream --ebpf 'port 5060' --bpf 'udp'

Any help will be appreciated

@dev195
Copy link
Contributor

dev195 commented Oct 13, 2016

rtp.py

Looking good!

The first time I read through it, I wondered why you needed to handle LookupError exceptions. It made me realize that Dshell does not parse out the MAC address from cooked capture (as far as I can tell, SLL only captures one address per packet). I will try and get that updated in the core library sometime this week.

I don't think you need to use as many self. variables in the handler function. The smac and dmac will probably change with each packet, so storing it as a class attribute is probably overkill.

Also, I don't think ethernet = dpkt.ethernet.Ethernet(data) works the way you think it does. At this point, data only contains the packet's data after all of the headers are removed. Trying to parse Ethernet data out of it will not work.

sip.py

This one is very nice. I like the use of colorout. It makes it much easier to separate the requests and responses at a glance.

Aside from the same comments about LookupError exceptions and self. variables, this decoder should be okay, too.

I will try to push an update to Dshell soon to handle SLL MAC addresses. I think I will store the one I get in the 'smac' key.

@1modm
Copy link
Contributor Author

1modm commented Oct 13, 2016

Great, yes I was checking that the MACs for my SLL packet were not correct 👍

Thank you for the help, I will wait an update and I will update the decoders.

My TODO: make another decoder for H323, it will be more complicated because h323 has not a packet handle and use h245 and h225.

@1modm
Copy link
Contributor Author

1modm commented Oct 16, 2016

Thanks for the update, it works but I have seen some packets without smac also, check Linux cooked capture section in the next example, it could be possible to send smac as None if doesn exist?

Frame 32: 1363 bytes on wire (10904 bits), 1363 bytes captured (10904 bits)
Encapsulation type: Linux cooked-mode capture (25)
Arrival Time: Sep 21, 2016 23:44:25.360170000 IST
[Time shift for this packet: 0.000000000 seconds]
Epoch Time: 1474497865.360170000 seconds
[Time delta from previous captured frame: 0.000069000 seconds]
[Time delta from previous displayed frame: 0.000069000 seconds]
[Time since reference or first frame: 3.699828000 seconds]
Frame Number: 32
Frame Length: 1363 bytes (10904 bits)
Capture Length: 1363 bytes (10904 bits)
[Frame is marked: False]
[Frame is ignored: False]
[Protocols in frame: sll:ethertype:ip:udp:sip:sdp]
Linux cooked capture
Packet type: Unicast to us (0)
Link-layer address type: 65534
Link-layer address length: 0
Protocol: IPv4 (0x0800)
Internet Protocol Version 4, Src: 10.5.1.8, Dst: 10.5.1.7
0100 .... = Version: 4
.... 0101 = Header Length: 20 bytes
Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
0000 00.. = Differentiated Services Codepoint: Default (0)
.... ..00 = Explicit Congestion Notification: Not ECN-Capable Transport (0)
Total Length: 1347
Identification: 0x0f44 (3908)
Flags: 0x00
0... .... = Reserved bit: Not set
.0.. .... = Don't fragment: Not set
..0. .... = More fragments: Not set
Fragment offset: 0
Time to live: 128
Protocol: UDP (17)
Header checksum: 0x104e [validation disabled]
[Good: False]
[Bad: False]
Source: 10.5.1.8
Destination: 10.5.1.7
[Source GeoIP: Unknown]
[Destination GeoIP: Unknown]
User Datagram Protocol, Src Port: 5060 (5060), Dst Port: 5060 (5060)
Source Port: 5060
Destination Port: 5060
Length: 1327
Checksum: 0x42cf [validation disabled]
[Good Checksum: False]
[Bad Checksum: False]
[Stream index: 7]
Session Initiation Protocol (INVITE)
Request-Line: INVITE sip:demo-alice@10.5.1.7 SIP/2.0
Method: INVITE
Request-URI: sip:demo-alice@10.5.1.7
Request-URI User Part: demo-alice
Request-URI Host Part: 10.5.1.7
[Resent Packet: False]

@1modm
Copy link
Contributor Author

1modm commented Oct 18, 2016

Updated decoders

@dev195
Copy link
Contributor

dev195 commented Oct 25, 2016

Okay, everything seems to work as expected and looks good! Once I get the proper approvals from the rest of the team, I'll accept the pull request.

@1modm
Copy link
Contributor Author

1modm commented Oct 25, 2016

Cool, thank you for all the help, and I will try to contribute with more decoders. I would like to try h323 (h245, h225, Q.931) and codecs (melpe, g711, etc) or secure protocols.

@dev195
Copy link
Contributor

dev195 commented Nov 1, 2016

Thanks for your patience. This pull request is approved. Thanks for your contribution!

@dev195 dev195 merged commit 4011487 into USArmyResearchLab:master Nov 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants