Using an SSL subdissector will cause SSL data to not be decoded (related to reassembly of application data)

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

Original bug information:

Reporter: Asad Mehmood
Status: RESOLVED FIXED
Product: Wireshark
Component: Dissection engine (libwireshark)
OS: Windows 10
Platform: x86-64
Version: 2.4.0

Attachments:

ssl.pcapng: SSL Capture
keys.keys: SSL Key LOG
test.lua: Simple Lua Dissector
Wireshark.rar: Screenshot + pcapng + self signed certificate

Asad Mehmood said:

Created attachment 15685 SSL Capture

Build Information: Version 2.4.0rc2 (v2.4.0rc2-0-g739ff757)

Compiled (64-bit) with Qt 5.6.2, with WinPcap (4_1_3), with GLib 2.42.0, with zlib 1.2.8, with SMI 0.4.8, with c-ares 1.12.0, with Lua 5.2.4, with GnuTLS 3.4.11, with Gcrypt 1.7.6, with MIT Kerberos, with GeoIP, with nghttp2 1.14.0, with LZ4, with Snappy, with libxml2 2.9.4, with QtMultimedia, with AirPcap, with SBC, with SpanDSP.

Running on 64-bit Windows 10, build 15063, with Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (with SSE4.2), with 12220 MB of physical memory, with locale English_United Kingdom.1252, with WinPcap version 4.1.3 (packet.dll version 4.1.0.2980), based on libpcap version 1.0 branch 1_0_rel0b (20091008), with GnuTLS 3.4.11, with Gcrypt 1.7.6, without AirPcap.

Built using Microsoft Visual C++ 14.0 build 24215

When a subdissector is used which needs to reassemble ssl data. The Data will not be reassembled and data will not be decoded in Follow SSL.

Attached:
Capture containing SSL Stream
SSL Key File
Example Simple Dissector

How to reproduce:

Open tcp capture
Decode TCP Stream as SSL
Use Follow SSL and see TWO way SSL Comunication
Look at Frame 27 and see two records decrypted

Decode SSL Stream as Lockdown
Use Follow SSL and see XML Missing from client.
Look at Frame 27 and see one records decrypted

added libwireshark oswindows version2.4 labels

Asad Mehmood said:

Created attachment 15686 SSL Key LOG

Asad Mehmood said:

Created attachment 15687 Simple Lua Dissector

Peter Wu said:

Thanks for the reproducer. It has something to do with reassembly of application data record fragments and can only be reproduced on the second pass (using the -2 option):

tshark -r ssl.pcapng -ossl.keylog_file:keys.keys -Xlua_script:test.lua -Y frame.number==27 -O ssl -x -dssl.port==62078,lockdown -2

The responsible function for reassembly is desegment_ssl in epan/dissectors/packet-ssl.c

If I may guess, the problem is related to multiple record fragments in the same packet while the subdissector requests more fragments. Apparently this edge case is not handled correctly.

Gerrit Code Review said:

Change 22631 had a related patch set uploaded by Asad Mehmood: SSL: Restore curr_layer_num after calling subdissector

https://code.wireshark.org/review/22631

Peter Wu said:

*** Bug #13943 (closed) has been marked as a duplicate of this bug. ***

Peter Wu said:

So the root cause is that "curr_layer_num") as in:

 SslPacketInfo *pi;
 p_add_proto_data(..., pinfo->curr_layer_num, pi);
 pi = p_get_proto_data(..., pinfo->curr_layer_num)

is different between two runs. This was changed in Wireshark 2.4, previously it would always use "0" (but potentially break with tunnels).

Let's consider the possible configurations how the SSL dissector can be called:

1. Multiple TLS records in a single TCP segment (realistic, see this bug).
2. TLS nested in TLS (e.g. TLS with stunnel, TCP/TLS with proxychains).

To handle both cases, I think it is best to save curr_layer_num somewhere (somewhere between dissect_ssl and before call_dissector) and use this for the p_*_proto_data functions.

This will only work when the parent layers maintain the same curr_layer_num value on subsequent dissections (e.g. the second pass) though, hopefully that is a valid assumption.

Gerrit Code Review said:

Change 23190 had a related patch set uploaded by Peter Wu: ssl: fix subdissection with multiple TLS records per packet

https://code.wireshark.org/review/23190

Anders Broman said:

(In reply to Peter Wu from comment #6)  
> So the root cause is that "curr_layer_num") as in:  
>  
>  SslPacketInfo *pi;  
>  p_add_proto_data(..., pinfo->curr_layer_num, pi);  
>  pi = p_get_proto_data(..., pinfo->curr_layer_num)  
>  
> is different between two runs. This was changed in Wireshark 2.4, previously  
> it would always use "0" (but potentially break with tunnels).  
>  
> Let's consider the possible configurations how the SSL dissector can be  
> called:  
>  
> 1. Multiple TLS records in a single TCP segment (realistic, see this bug).  
> 2. TLS nested in TLS (e.g. TLS with stunnel, TCP/TLS with proxychains).  
>  
> To handle both cases, I think it is best to save curr_layer_num somewhere  
> (somewhere between dissect_ssl and before call_dissector) and use this for  
> the p_*_proto_data functions.  
>  
> This will only work when the parent layers maintain the same curr_layer_num  
> value on subsequent dissections (e.g. the second pass) though, hopefully  
> that is a valid assumption.  

I think this is inline with how I thought p_get_proto_data() should work together with pinfo->curr_layer_num. If it does not work on the second path I'd consider that a bug.

Gerrit Code Review said:

Change 23190 merged by Michael Mann: ssl: fix subdissection with multiple TLS records per packet

https://code.wireshark.org/review/23190

Gerrit Code Review said:

Change 23473 had a related patch set uploaded by Michael Mann: ssl: fix subdissection with multiple TLS records per packet

https://code.wireshark.org/review/23473

Gerrit Code Review said:

Change 23473 merged by Anders Broman: ssl: fix subdissection with multiple TLS records per packet

https://code.wireshark.org/review/23473

Peter Wu said:

*** Bug #14111 (closed) has been marked as a duplicate of this bug. ***

c4rlosmarin said:

Created attachment 15901 Screenshot + pcapng + self signed certificate

c4rlosmarin said:

Related to bug #13943 (closed), version 2.4.2 is actually not showing any wcf response at all. I've attached the screenshot of what I see as well as the trace and the self-signed certificate (Password: P@$$‍w0rd)

Peter Wu said:

(In reply to c4rlosmarin from comment #‍14)
> Related to bug #13943 (closed), version 2.4.2 is actually not showing any wcf response
> at all.

You are using "ip.src == ..." in your display filter, to see a response you must use "ip.addr" instead. I can see a 200 OK message after that in both 2.4.2 and the current master branch, is this really a problem for you?

c4rlosmarin said:

(In reply to Peter Wu from comment #‍15)
> (In reply to c4rlosmarin from comment #‍14)
> > Related to bug #13943 (closed), version 2.4.2 is actually not showing any wcf response
> > at all.
>
> You are using "ip.src == ..." in your display filter, to see a response you
> must use "ip.addr" instead. I can see a 200 OK message after that in both
> 2.4.2 and the current master branch, is this really a problem for you?

Ups, you're right. It's working now. Tx! :)

closed

mentioned in issue #13943 (closed)