Re: [Wireshark-dev] Improving TCAP session matching
On Sun, Jan 04, 2015 at 12:33:45PM +0100, Alexis La Goutte wrote: |The better is create a bug in bugtracker with subject like : Enhance |TCAP dissector (and list the list of issue/change) |and push your patch on gerrit with in commit log the reference to this |bug (Ping-Bug: ) |Also attach in bug, some pcap sample for try. I've done that; as bug 10841. A sample ERF is attached. I'll push a patch to Gerrit to review. Thanks for the suggestion. cheers, Luke. pgpQ5Ljm8v4KD.pgp Description: PGP signature ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Improving TCAP session matching
On 01/03/2015 07:32 PM, Luke Mewburn wrote: 2. TCAP session matching incomplete. DETAILS: TCAP session matching doesn't cope with TCAP dialogue confirmation. (See "TCAP background" below for more information about dialogue confirmation). This is because the current implementation uses the destination address for TCAP BEGIN matching (tcaphash_begin_info_key_t.dpc_hash), and origin address for TCAP END matching (tcaphash_end_info_key_t.opc_hash). When TCAP sessions (transactions) aren't correctly matched, then higher level dissectors can mistakenly decode packets part-way through the session because of assumptions about protocol versions (etc). There are existing Wireshark bugs in TCAP (and higher layer) that may be fixed by addressing this PROPOSAL: I have some changes to be submitted for review which resolves this, and greatly improves transaction matching when sccp.set_addresses:TRUE and tcap.srt:TRUE, even when the destination address of the TCAP BEGIN changes as part of dialogue (transaction) confirmation. Great, thanks for the efforts in this area. I've been bothered by problems in this code but never enough to wade into the (murky) depths to try to fix it. Future proposals (These could be moved into separate discussions). 1. SCCP addresses not set by default. DETAILS: By default, the SCCP GT is not set as the source and destination addresses for higher layer protocols. The MTP3 (or M3UA) point codes (PCs) are left as these addresses. This can be changed with the preference option sccp.set_addresses. For TCAP transaction matching, the SCCP GT is more correct than the MTP3 PC. PROPOSAL: The sccp.set_addresses default could change to TRUE (and possibly the SUA equivalent, although I haven't any experience with SUA), but I'm not sure what ramifications that this has on protocols other than TCAP that use SCCP. When I added this behavior (and the preference): https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=3be92af2d91692706f5d4531f33a7420224d9a6b I noted that: This may help (if the PCs change but the GT does not) or hurt (if the GT or RI change but the PCs do not) TCAP's ability to identify which messages belong to which TCAP "session." Your "SCCP GT not set if route-on-SSN" change presumably fixes the "if the RI changes" case. If your "TCAP session matching incomplete" change fixes the "if the GT changes" case then, yes, at least for TCAP it would make sense to change the default. Like you I'm not sure how other (protocol) users of SCCP might be affected. Of course users who are used to seeing the PCs there may also get confused by the behavior change but they'd likely get over it (or change the preference if they really don't like it). 3. TCAP session matching not enabled by default. DETAILS: The session matching isn't enabled by default. I suspect that this is because it has a performance impact and that it doesn't work reliably (without my fixes :). PROPOSAL: Once the session matching is fixed, it could be enabled by default. Sure. 4. TCAP converted to conversation.h (after the latter is enhanced) [...] PROPOSAL: Further investigation and discussion is required about how to enhance conversation.h to resolve this. I suspect we need special-case handling in find_conversation() (etc) for PT_TCAP, including possibly a separate hashtable or keeping the TCAP BEGIN in the conversation_hashtable_no_addr2_or_port2 in parallel to the entry in conversation_hashtable_exact. This sounds like a good idea. I don't like the existing TCAP SRT code. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Improving TCAP session matching
On Sun, Jan 4, 2015 at 12:33 PM, Alexis La Goutte wrote: > > > On Sun, Jan 4, 2015 at 1:32 AM, Luke Mewburn wrote: > >> Hi all. >> >> I've observed that Wireshark's TCAP session (transaction) matching >> (enabled with tcap.srt:TRUE) doesn't operate correctly in various cases. >> >> I've been working on improving Wireshark's TCAP session handling, >> and have some questions about the "best" way to do this as a new >> contributor to the Wireshark development community. >> >> I'm not sure if it's preferable to describe all of the issues >> in one 'wall of text' email or split up the discussions across >> multiple messages; I've gone with the former :) >> >> Please let me know if the issues should be split out into separate >> discussions, or if there's a better forum for this (e.g. bugzilla >> tickets). >> >> regards, >> Luke. >> >> >> Specific fixes >> -- >> >> 1. SCCP GT not set if route-on-SSN. >> >> DETAILS:: When sccp.set_addresses is TRUE, the GT isn't used if present >> when the SCCP route-on-SSN bit is set (i.e not route-on-GT). >> The GT is still useful for TCAP matching even in this case. >> >> PROPOSAL: I've submitted a change for this at: >> https://code.wireshark.org/review/#/c/6053/ >> (The second patch attempt is a minor improvement to the wording >> of the preference after the changes in the original patch). >> >> >> 2. TCAP session matching incomplete. >> >> DETAILS: TCAP session matching doesn't cope with TCAP dialogue >> confirmation. (See "TCAP background" below for more information >> about dialogue confirmation). >> >> This is because the current implementation uses the destination >> address for TCAP BEGIN matching (tcaphash_begin_info_key_t.dpc_hash), >> and origin address for TCAP END matching >> (tcaphash_end_info_key_t.opc_hash). >> >> When TCAP sessions (transactions) aren't correctly matched, >> then higher level dissectors can mistakenly decode packets >> part-way through the session because of assumptions about >> protocol versions (etc). There are existing Wireshark >> bugs in TCAP (and higher layer) that may be fixed by addressing this >> >> PROPOSAL: I have some changes to be submitted for review which >> resolves this, and greatly improves transaction matching when >> sccp.set_addresses:TRUE and tcap.srt:TRUE, even when the destination >> address of the TCAP BEGIN changes as part of dialogue (transaction) >> confirmation. >> >> >> Future proposals >> >> >> (These could be moved into separate discussions). >> >> 1. SCCP addresses not set by default. >> >> DETAILS: By default, the SCCP GT is not set as the source and destination >> addresses for higher layer protocols. The MTP3 (or M3UA) point codes >> (PCs) are left as these addresses. >> This can be changed with the preference option sccp.set_addresses. >> For TCAP transaction matching, the SCCP GT is more correct than the >> MTP3 PC. >> >> PROPOSAL: The sccp.set_addresses default could change to TRUE (and >> possibly the SUA equivalent, although I haven't any experience with >> SUA), but I'm not sure what ramifications that this has on protocols >> other than TCAP that use SCCP. >> >> >> 2. TCAP filters for party addresses. >> >> DETAILS: I've experimented with adding new filter nodes to contain the >> (SCCP GT) addresses of the original calling party (from TCAP BEGIN), >> the original called party (from TCAP BEGIN), and the confirmed party >> (from first TCAP CONTINUE or END-after-BEGIN). >> >> These depend upon the SCCP addresses set (see above) and >> the session matching fixed. >> >> PROPOSAL: Add new filters. I have a work in progress for this >> which could be submitted for review. >> >> >> 3. TCAP session matching not enabled by default. >> >> DETAILS: The session matching isn't enabled by default. >> I suspect that this is because it has a performance impact >> and that it doesn't work reliably (without my fixes :). >> >> PROPOSAL: Once the session matching is fixed, it could be >> enabled by default. >> >> >> 4. TCAP converted to conversation.h (after the latter is enhanced) >> >> DETAILS: TCAP uses its own session matching logic. If the standard >> "conversation.h" implementation was used, then there may be other >> benefits such as following conversation flows. >> >> I've experimented with this in a private branch, but ran into issues in >> the current conversation.h implementation that need to be addressed >> for TCAP. >> >> I implemented a new port type - PT_TCAP - to contain the TCAP >> transaction ID (TID). >> >> As described below in "TCAP background": >> >> - TCAP BEGIN contains a src addr, otid (src port), and dst addr. >> The dst addr may change in the first CONTINUE. >> >> We have to create the conversation with NO_ADDR2 as well as NO_PORT2, >> which results in the conversation added to >> conversation_hashtable_no_addr2_or_port2. >> >> - TCAP CONTINUE contains src addr, otid, dst addr, dtid. >> >> Looking this up with find_conversation() seems to work;
Re: [Wireshark-dev] Improving TCAP session matching
On Sun, Jan 4, 2015 at 1:32 AM, Luke Mewburn wrote: > Hi all. > > I've observed that Wireshark's TCAP session (transaction) matching > (enabled with tcap.srt:TRUE) doesn't operate correctly in various cases. > > I've been working on improving Wireshark's TCAP session handling, > and have some questions about the "best" way to do this as a new > contributor to the Wireshark development community. > > I'm not sure if it's preferable to describe all of the issues > in one 'wall of text' email or split up the discussions across > multiple messages; I've gone with the former :) > > Please let me know if the issues should be split out into separate > discussions, or if there's a better forum for this (e.g. bugzilla > tickets). > > regards, > Luke. > > > Specific fixes > -- > > 1. SCCP GT not set if route-on-SSN. > > DETAILS:: When sccp.set_addresses is TRUE, the GT isn't used if present > when the SCCP route-on-SSN bit is set (i.e not route-on-GT). > The GT is still useful for TCAP matching even in this case. > > PROPOSAL: I've submitted a change for this at: > https://code.wireshark.org/review/#/c/6053/ > (The second patch attempt is a minor improvement to the wording > of the preference after the changes in the original patch). > > > 2. TCAP session matching incomplete. > > DETAILS: TCAP session matching doesn't cope with TCAP dialogue > confirmation. (See "TCAP background" below for more information > about dialogue confirmation). > > This is because the current implementation uses the destination > address for TCAP BEGIN matching (tcaphash_begin_info_key_t.dpc_hash), > and origin address for TCAP END matching > (tcaphash_end_info_key_t.opc_hash). > > When TCAP sessions (transactions) aren't correctly matched, > then higher level dissectors can mistakenly decode packets > part-way through the session because of assumptions about > protocol versions (etc). There are existing Wireshark > bugs in TCAP (and higher layer) that may be fixed by addressing this > > PROPOSAL: I have some changes to be submitted for review which > resolves this, and greatly improves transaction matching when > sccp.set_addresses:TRUE and tcap.srt:TRUE, even when the destination > address of the TCAP BEGIN changes as part of dialogue (transaction) > confirmation. > > > Future proposals > > > (These could be moved into separate discussions). > > 1. SCCP addresses not set by default. > > DETAILS: By default, the SCCP GT is not set as the source and destination > addresses for higher layer protocols. The MTP3 (or M3UA) point codes > (PCs) are left as these addresses. > This can be changed with the preference option sccp.set_addresses. > For TCAP transaction matching, the SCCP GT is more correct than the > MTP3 PC. > > PROPOSAL: The sccp.set_addresses default could change to TRUE (and > possibly the SUA equivalent, although I haven't any experience with > SUA), but I'm not sure what ramifications that this has on protocols > other than TCAP that use SCCP. > > > 2. TCAP filters for party addresses. > > DETAILS: I've experimented with adding new filter nodes to contain the > (SCCP GT) addresses of the original calling party (from TCAP BEGIN), > the original called party (from TCAP BEGIN), and the confirmed party > (from first TCAP CONTINUE or END-after-BEGIN). > > These depend upon the SCCP addresses set (see above) and > the session matching fixed. > > PROPOSAL: Add new filters. I have a work in progress for this > which could be submitted for review. > > > 3. TCAP session matching not enabled by default. > > DETAILS: The session matching isn't enabled by default. > I suspect that this is because it has a performance impact > and that it doesn't work reliably (without my fixes :). > > PROPOSAL: Once the session matching is fixed, it could be > enabled by default. > > > 4. TCAP converted to conversation.h (after the latter is enhanced) > > DETAILS: TCAP uses its own session matching logic. If the standard > "conversation.h" implementation was used, then there may be other > benefits such as following conversation flows. > > I've experimented with this in a private branch, but ran into issues in > the current conversation.h implementation that need to be addressed > for TCAP. > > I implemented a new port type - PT_TCAP - to contain the TCAP > transaction ID (TID). > > As described below in "TCAP background": > > - TCAP BEGIN contains a src addr, otid (src port), and dst addr. > The dst addr may change in the first CONTINUE. > > We have to create the conversation with NO_ADDR2 as well as NO_PORT2, > which results in the conversation added to > conversation_hashtable_no_addr2_or_port2. > > - TCAP CONTINUE contains src addr, otid, dst addr, dtid. > > Looking this up with find_conversation() seems to work; > if it's the first CONTINUE the conversation is moved from > conversation_hashtable_no_addr2_or_port2 to > conversation_hashtable_exact. > > - TCAP END & ABORT contains the dst addr, dtid
Re: [Wireshark-dev] Improving TCAP session matching
On Sat, Jan 03, 2015 at 04:49:06PM -0800, Guy Harris wrote: | | On Jan 3, 2015, at 4:32 PM, Luke Mewburn wrote: | | > I suspect we need special-case handling in find_conversation() (etc) | > for PT_TCAP, including possibly a separate hashtable or keeping | > the TCAP BEGIN in the conversation_hashtable_no_addr2_or_port2 in | > parallel to the entry in conversation_hashtable_exact. | | I suspect what we need is to separate the general concept of | conversations from the details of particular protocols, so that each | type of conversation would supply its own matching code. That sounds better. That could replace the existing special cases for PT_UDP and AT_FC too. pgpbPVMMsJqII.pgp Description: PGP signature ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] Improving TCAP session matching
On Jan 3, 2015, at 4:32 PM, Luke Mewburn wrote: > I suspect we need special-case handling in find_conversation() (etc) > for PT_TCAP, including possibly a separate hashtable or keeping > the TCAP BEGIN in the conversation_hashtable_no_addr2_or_port2 in > parallel to the entry in conversation_hashtable_exact. I suspect what we need is to separate the general concept of conversations from the details of particular protocols, so that each type of conversation would supply its own matching code. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
[Wireshark-dev] Improving TCAP session matching
Hi all. I've observed that Wireshark's TCAP session (transaction) matching (enabled with tcap.srt:TRUE) doesn't operate correctly in various cases. I've been working on improving Wireshark's TCAP session handling, and have some questions about the "best" way to do this as a new contributor to the Wireshark development community. I'm not sure if it's preferable to describe all of the issues in one 'wall of text' email or split up the discussions across multiple messages; I've gone with the former :) Please let me know if the issues should be split out into separate discussions, or if there's a better forum for this (e.g. bugzilla tickets). regards, Luke. Specific fixes -- 1. SCCP GT not set if route-on-SSN. DETAILS:: When sccp.set_addresses is TRUE, the GT isn't used if present when the SCCP route-on-SSN bit is set (i.e not route-on-GT). The GT is still useful for TCAP matching even in this case. PROPOSAL: I've submitted a change for this at: https://code.wireshark.org/review/#/c/6053/ (The second patch attempt is a minor improvement to the wording of the preference after the changes in the original patch). 2. TCAP session matching incomplete. DETAILS: TCAP session matching doesn't cope with TCAP dialogue confirmation. (See "TCAP background" below for more information about dialogue confirmation). This is because the current implementation uses the destination address for TCAP BEGIN matching (tcaphash_begin_info_key_t.dpc_hash), and origin address for TCAP END matching (tcaphash_end_info_key_t.opc_hash). When TCAP sessions (transactions) aren't correctly matched, then higher level dissectors can mistakenly decode packets part-way through the session because of assumptions about protocol versions (etc). There are existing Wireshark bugs in TCAP (and higher layer) that may be fixed by addressing this PROPOSAL: I have some changes to be submitted for review which resolves this, and greatly improves transaction matching when sccp.set_addresses:TRUE and tcap.srt:TRUE, even when the destination address of the TCAP BEGIN changes as part of dialogue (transaction) confirmation. Future proposals (These could be moved into separate discussions). 1. SCCP addresses not set by default. DETAILS: By default, the SCCP GT is not set as the source and destination addresses for higher layer protocols. The MTP3 (or M3UA) point codes (PCs) are left as these addresses. This can be changed with the preference option sccp.set_addresses. For TCAP transaction matching, the SCCP GT is more correct than the MTP3 PC. PROPOSAL: The sccp.set_addresses default could change to TRUE (and possibly the SUA equivalent, although I haven't any experience with SUA), but I'm not sure what ramifications that this has on protocols other than TCAP that use SCCP. 2. TCAP filters for party addresses. DETAILS: I've experimented with adding new filter nodes to contain the (SCCP GT) addresses of the original calling party (from TCAP BEGIN), the original called party (from TCAP BEGIN), and the confirmed party (from first TCAP CONTINUE or END-after-BEGIN). These depend upon the SCCP addresses set (see above) and the session matching fixed. PROPOSAL: Add new filters. I have a work in progress for this which could be submitted for review. 3. TCAP session matching not enabled by default. DETAILS: The session matching isn't enabled by default. I suspect that this is because it has a performance impact and that it doesn't work reliably (without my fixes :). PROPOSAL: Once the session matching is fixed, it could be enabled by default. 4. TCAP converted to conversation.h (after the latter is enhanced) DETAILS: TCAP uses its own session matching logic. If the standard "conversation.h" implementation was used, then there may be other benefits such as following conversation flows. I've experimented with this in a private branch, but ran into issues in the current conversation.h implementation that need to be addressed for TCAP. I implemented a new port type - PT_TCAP - to contain the TCAP transaction ID (TID). As described below in "TCAP background": - TCAP BEGIN contains a src addr, otid (src port), and dst addr. The dst addr may change in the first CONTINUE. We have to create the conversation with NO_ADDR2 as well as NO_PORT2, which results in the conversation added to conversation_hashtable_no_addr2_or_port2. - TCAP CONTINUE contains src addr, otid, dst addr, dtid. Looking this up with find_conversation() seems to work; if it's the first CONTINUE the conversation is moved from conversation_hashtable_no_addr2_or_port2 to conversation_hashtable_exact. - TCAP END & ABORT contains the dst addr, dtid (dst port), and src addr without src port (otid). The dst addr may change in the first CONTINUE or an END after BEGIN. If the TCAP END is after a BEGIN, the conversation may be found from conversation_hashtable_no_addr2_or_port2 if the addresses are swapped, but if
