RTP Stream Analysis does not work for TURN-encapsulated RTP

This issue was migrated from bug 6322 in our old bug tracker.

Original bug information:

Reporter: Ken Cox
Status: RESOLVED FIXED
Product: Wireshark
Component: TShark
OS: All
Platform: All
Version: 1.6.1

Attachments:

rtp_turn.pcap: pcap containing TURN-encapsulated RTP
vivox.patch: proosed patch
vivox.patch: proposed patch, take 2

Ken Cox said:

Created attachment 6953

pcap containing TURN-encapsulated RTP

Build Information:

TShark 1.6.1 (SVN Rev 38096 from /trunk-1.6)

I also tried my trunk build:

TShark 1.7.0-kenstir-20110831 (SVN Rev 38820 from /trunk)

If you have an RTP stream encapsulated in TURN ChannelData messages, then RTP Stream Analysis does not work:

c:\src\wireshark\trunk>"c:\Program Files\Wireshark\tshark.exe" -qr c:\pcaps\rtp_turn.pcap -z rtp,streams
========================= RTP Streams ========================
    Src IP addr  Port    Dest IP addr  Port       SSRC          Payload  Pkts         Lost   Max Delta(ms)  Max Jitter(ms) Mean Jitter(ms) Problems?
==============================================================

With this small patch it does:

c:\src\wireshark\trunk>wireshark-gtk2\tshark.exe -qr c:\pcaps\rtp_turn.pcap -z rtp,streams
========================= RTP Streams ========================
    Src IP addr  Port    Dest IP addr  Port       SSRC          Payload  Pkts         Lost   Max Delta(ms)  Max Jitter(ms) Mean Jitter(ms) Problems?
     10.0.7.209  4577   74.201.99.151  3478 0x00004BE4     Unknown(127)   109     0 (0.0%)         1000.42            7.72            2.48 X
  74.201.99.151  3478      10.0.7.209  4577 0x00C0F7E8     Unknown(127)   908     0 (0.0%)           25.29            0.72            0.34 X
==============================================================

The change causes dissect_stun_message() to pass control to subdissectors when 'tree' is NULL.

added clitshark version1.6 labels

Ken Cox said:

Created attachment 6954 proosed patch

Anders Broman said:

Hi, Just a quick review: I do not think that the changes to tap-rtp-common.c and packet-rtp.c are the right thing to do. I don't remember if it was possible to call the "tap" requesting the tree to be built which would make the stun patch obsolete too.

Ken Cox said:

Created attachment 6958 proposed patch, take 2

Oops, thanks for the catch. I did not mean to include my local changes to tap-rtp-common.c. Those changes contain assumptions of nonstandard payload types, but they enable me to analyze RTP streams in the absence of the SIP negotiation.

However, I don't understand your comment about the packet-rtp.c and packet-stun.c changes. The change to packet-stun.c is required because the TAP calls dissect_stun_message() with a NULL tree. Without the patch, during TAP, dissect_stun_message() would return immediately without calling dissector_try_heuristic() and the encapsulated RTP would not be recognized. The change the packet-rtp.c is required because when RTP is tunneled though TURN with a TCP transport, the local port number may not be even numbered.

Anders Broman said:

Hi, If there is a way to have the tap to call dissect_stun_message() with a non NULL tree the patch would not be needed, I may be mixing this up with the VoIP call functionality. I'm not sure the RTP changes ar OK for non STUN traffic. That said I haven't looked to deply into the patch I just wanted to convey the information to Bill to save him some effort. Regards Anders

Guy Harris said:

The STUN dissector should most definitely hand off the packet to subdissectors regardless of whether "tree" is null or not - *ALL* subdissector calls should be done, everywhere in Wireshark, regardless of whether a protocol tree is being built or no, as the subdissector might, for example, build internal state necessary to dissect subsequent packets or even to correctly re-dissect the same packet.

Bill Meier said:

(In reply to comment #4)  

> That said I haven't looked too deeply into the patch I just wanted to convey  
> the information to Bill to save him some effort.  

Thanks, Anders  (I'm not familiar with STUN, RTP, etc) so the help is appreciated).


> Hi,  
> If there is a way to have the tap to call dissect_stun_message() with a non  
> NULL tree the patch would not be needed, I may be mixing this up with the VoIP  
> call functionality.  

As noted by Guy in Comment #5, the patch to packet-stun to always call the subdissector should be done. I've tested it and it works as advertised.


> I'm not sure the RTP changes are OK for non STUN traffic.  

I've looked at this a bit and note the following:

- dissect_rtp_heur() is registered only for the  UDP and STUN dissectors.
- dissect_stun_heur() is registered for TCP, UDP and STUN.
- So: It appears that RTP could be carried over UDP STUN.
  Can/Will this happen ?
  If so, will the STUN UDP port always be even so that the test for an even UDP
  port in dissect_rtp_heuristic succeeds ?
  (Or do I misunderstand how this works ?)

Ken Cox said:

RTP can be carried over UDP STUN, and if it is the UDP port number can be even or odd. RFC 3550 says: For UDP and similar protocols, RTP should use an even destination port number and the corresponding RTCP stream should use the next higher (odd) destination port number. Adding (pinfo->ptype == PT_UDP) to the heuristic was a lame attempt to maintain the spirit of the even-port check without missing my odd-port TCP packets. I don't see a problem in practice with removing the even-port check altogether, aside from possibly more false positives.

Anders Broman said:

The STUN patch Committed revision 38950. I'm not sure what to do with the RTP heuristic it is very weak if B7-B6 of Octet 1 is 0 or 2 and the port is even the it's RTP. Weaken it even more seems a bad idea to me.

Regards Anders

Bill Meier said:

(In reply to comment #8)  
> I'm not sure what to do with  
> the RTP heuristic it is very weak if B7-B6 of Octet 1 is 0 or 2 and  
> the port is even the it's RTP. Weaken it even more seems a bad idea to me.  
>  

I've committed a change to the RTP heuristic so that the 'port is even' check is done if testing for RTP over UDP but not done if testing for RTP over STUN.

IOW, the heuristic hasn't been weakened for for RTP over UDP.

(See SVN #38953).

I also note that the heuristic test in the RTP dissector is not even enabled unless the "heuristic rtp" preference is explicitly changed to TRUE from the normal default of FALSE.

closed