[Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>]

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.