Enhance L2TP filters to include assigned IDs

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

Original bug information:

Reporter: Andy Karch
Status: RESOLVED FIXED
Product: Wireshark
Component: GTK+ UI
OS: All
Platform: All
Version: unspecified

Attachments:

CSCts22400_feb06_2012.filtered.pcap: Filtered output for L2TPv2 filters.
wireshark.l2tp_assigned_id.diff: patch to add filters
l2tpv3_ut.pcap: Packet capture used to unit-test L2TPv3

Andy Karch said:

Created attachment 7832 Filtered output for L2TPv2 filters.

Build Information: Version 1.6.6 (SVN Rev 40982 from /trunk-1.6)

Copyright 1998-2012 Gerald Combs <gerald@‍wireshark.org> and contributors. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Compiled (64-bit) with GTK+ 2.10.4, with GLib 2.12.3, with libpcap (version unknown), with libz 1.2.3, with POSIX capabilities (Linux), without libpcre, without SMI, without c-ares, without ADNS, without Lua, without Python, with GnuTLS 1.4.1, with Gcrypt 1.4.4, with MIT Kerberos, without GeoIP, without PortAudio, without AirPcap. NOTE: this build doesn't support the "matches" operator for Wireshark filter syntax.

Running on Linux 2.6.18-194.8.1.el5, with libpcap version 0.9.4, with libz 1.2.3, GnuTLS 1.4.1, Gcrypt 1.4.4.

Built using gcc 4.1.2 20080704 (Red Hat 4.1.2-48).

Wireshark is Open Source Software released under the GNU General Public License.

Check the man page and http://www.wireshark.org for more information.

Currently, filters only work on the Tunnel/Session ID specified in the packet header.  This presents a limitation where the ID hasn't yet been propagated to the peer and is thus set to '0'.  This occurs in SCCRQ, ICRQ, and OCRQ packets.

To solve this problem, the filters need to be enhanced to use the AVPs that propagate locally allocated IDs to the peer.

L2TPv2 (RFC22661):
    - Assigned Tunnel ID (2 bytes)
    - Assigned Session ID (2 bytes)

L2TPv3 (RFC3931):
    - Assigned Control Connection ID (4 bytes)
    - Local Session ID (4 bytes)

Further, Session ID filtering doesn't work for L2TPv3 because it is not in the control message header for L2TPv3 over IP.  A control message is identified by a session ID of '0'.  Instead the control message Session ID is obtained from the Remote Session ID AVP, specified in RFC3931.

Again the filters need to be enhanced to solve this problem.

L2TPv3 (RFC3931):
    - Remote Session ID (4 bytes)

added enhancement uigtk labels

Andy Karch said:

Created attachment 7833 patch to add filters

When this is reviewed and eventually approved, do you commit, or do I?

Andy Karch said:

Created attachment 7836 Packet capture used to unit-test L2TPv3

I tested the filters on this L2TPv3 packet capture.

Bill Meier said:

(In reply to comment #1)  
>  
> When this is reviewed and eventually approved, do you commit, or do I?  

Wireshark commits are done by the Wireshark "core developers" ...

Andy Karch said:

(In reply to comment #3)  
> (In reply to comment #1)  
> >  
> > When this is reviewed and eventually approved, do you commit, or do I?  
>  
> Wireshark commits are done by the Wireshark "core developers" ...  

No problem, sounds good.

As I'm new to wireshark's developement and review process, please let me know if there are any further tests or attachments that you require.

I ran 'checkAPIs.pl', and it returned no violoations.

    wireshark-1.6-svn$ perl tools/checkAPIs.pl epan/dissectors/packet-l2tp.c
    wireshark-1.6-svn$

I also included captures for L2TPv2 and L2TPv3 for testing.

Thanks,
-Andy
akarch@cisco.com

Anders Broman said:

Committed revision 41031.

Andy Karch said:

akarch@cisco.com(In reply to comment #5)
> Committed revision 41031.  

Hi Anders,

Thanks for the check-in.

I see you've committed to 1.7.x (trunk).  Can this also be committed to 1.6.x (trunk-1.6)?

Thanks,
-Andy
akarch@cisco.com

Jeff Morriss said:

(In reply to comment #6)  
> akarch@cisco.com(In reply to comment #5)  
> > Committed revision 41031.  
>  
> Hi Anders,  
>  
> Thanks for the check-in.  
>  
> I see you've committed to 1.7.x (trunk).  Can this also be committed to 1.6.x  
> (trunk-1.6)?  

Normally only bug fixes are back-ported to the "stable" branches.  So the question becomes: is replacing add_text()'s with add_item()'s an enhancement or a bug?  I do like to think of add_text()'s as bugs, but I still can't convince myself this warrants being back-ported.

closed