Wireshark-dev: [Wireshark-dev] I think the following patch fixes the longstanding bug in the SP
From: Richard Sharpe <realrichardsharpe@xxxxxxxxx>
Date: Mon, 26 May 2014 10:44:45 -0700
Hi folks, Thanks to the hint from Thomas Kukosa, I now seem to have the SPNEGO dissector doing the correct things. It now dissects the NegTokenInit2 in a NegotiateProtocol response and the NegTokenInit in a SessionSetupRequest. Attached is a patch. I will add the individual patch files to the bug when I find it again. -- Regards, Richard Sharpe (何以解憂?唯有杜康。--曹操)
diff --git a/asn1/spnego/packet-spnego-template.c b/asn1/spnego/packet-spnego-template.c index ead29a3..ac2d9b1 100644 --- a/asn1/spnego/packet-spnego-template.c +++ b/asn1/spnego/packet-spnego-template.c @@ -96,13 +96,16 @@ static gint ett_spnego_krb5_cfx_flags = -1; #include "packet-spnego-ett.c" /* - * Unfortunately, we have to have a forward declaration of this, + * Unfortunately, we have to have forward declarations of thess, * as the code generated by asn2wrs includes a call before the * definition. */ -static int dissect_spnego_PrincipalSeq(gboolean implicit_tag, tvbuff_t *tvb, +static int dissect_spnego_NegTokenInit(gboolean implicit_tag, tvbuff_t *tvb, int offset, asn1_ctx_t *actx _U_, proto_tree *tree, int hf_index); +static int dissect_spnego_NegTokenInit2(gboolean implicit_tag, tvbuff_t *tvb, + int offset, asn1_ctx_t *actx _U_, + proto_tree *tree, int hf_index); #include "packet-spnego-fn.c" /* diff --git a/asn1/spnego/spnego.asn b/asn1/spnego/spnego.asn index 190b3f1..99c119f 100644 --- a/asn1/spnego/spnego.asn +++ b/asn1/spnego/spnego.asn @@ -13,21 +13,11 @@ NegotiationToken ::= CHOICE { MechTypeList ::= SEQUENCE OF MechType -- --- In some cases, the mechListMIC is a sequence of GeneralString, --- rather than an OCTET STRING. We define that sequence here so --- that we can call its dissector. --- The IRC discussion at +-- MS-SPNG tells us that the format of a negTokenInit is actually +-- negTokenInit2 if a negTokenInit is seen in a response. It might need +-- to be the first negTokenInit seen in a response, but I am not sure. +-- It will only occur in a NegotiateProtocol response in CIFS/SMB or SMB2. -- --- http://irc.vernstok.nl/samba-technical.dy --- --- seems to suggest that it's a Kerberos principal of some sort, thanks --- to some flavor of "embrace, extend, expectorate" sequence from --- Microsoft. --- -PrincipalSeq ::= SEQUENCE { - principal [0] GeneralString -} - NegTokenInit ::= SEQUENCE { mechTypes [0] MechTypeList OPTIONAL, reqFlags [1] ContextFlags OPTIONAL, @@ -35,6 +25,19 @@ NegTokenInit ::= SEQUENCE { mechListMIC [3] OCTET STRING OPTIONAL } +NegHints ::= SEQUENCE { + hintName [0] GeneralString OPTIONAL, + hintAddress [1] OCTET STRING OPTIONAL +} + +NegTokenInit2 ::= SEQUENCE { + mechTypes [0] MechTypeList OPTIONAL, + reqFlags [1] ContextFlags OPTIONAL, + mechToken [2] OCTET STRING OPTIONAL, + negHints [3] NegHints OPTIONAL, + mechListMIC [4] OCTET STRING OPTIONAL +} + ContextFlags ::= BIT STRING { delegFlag (0), mutualFlag (1), diff --git a/asn1/spnego/spnego.cnf b/asn1/spnego/spnego.cnf index a4fcc0b..5c633f5 100644 --- a/asn1/spnego/spnego.cnf +++ b/asn1/spnego/spnego.cnf @@ -8,11 +8,22 @@ #.NO_EMIT ONLY_VALS NegotiationToken -#.TYPE_RENAME -NegTokenInit/mechListMIC T_NegTokenInit_mechListMIC +#.FN_BODY NegotiationToken/negTokenInit + gboolean is_response = actx->pinfo->ptype == PT_TCP && + actx->pinfo->srcport < 1024; -#.FIELD_RENAME -NegTokenInit/mechListMIC negTokenInit_mechListMIC + /* + * We decode as negTokenInit2 or negTokenInit depending on whether or not + * we are in a response or a request. That is essentially what MS-SPNG + * says. + */ + if (is_response) { + return dissect_spnego_NegTokenInit2(%(IMPLICIT_TAG)s, %(TVB)s, %(OFFSET)s, + %(ACTX)s, %(TREE)s, %(HF_INDEX)s); + } else { + return dissect_spnego_NegTokenInit(%(IMPLICIT_TAG)s, %(TVB)s, %(OFFSET)s, + %(ACTX)s, %(TREE)s, %(HF_INDEX)s); + } #.FN_PARS MechType @@ -121,42 +132,28 @@ NegTokenInit/mechListMIC negTokenInit_mechListMIC call_dissector(next_level_value->handle, mechToken_tvb, actx->pinfo, tree); +#.FN_PARS NegTokenInit/mechListMIC + + VAL_PTR = &mechListMIC_tvb + #.FN_BODY NegTokenInit/mechListMIC - gint8 ber_class; - gboolean pc; - gint32 tag; tvbuff_t *mechListMIC_tvb; + +%(DEFAULT_BODY)s + + /* - * There seems to be two different forms this can take, - * one as an octet string, and one as a general string in a - * sequence. - * - * Peek at the header, and then decide which it is we're seeing. + * Now, we should be able to dispatch, if we've gotten a tvbuff for + * the MIC and we have information on how to dissect its contents. */ - get_ber_identifier(tvb, offset, &ber_class, &pc, &tag); - if (ber_class == BER_CLASS_UNI && pc && tag == BER_UNI_TAG_SEQUENCE) { - /* - * It's a sequence. - */ - return dissect_spnego_PrincipalSeq(FALSE, tvb, offset, actx, tree, - hf_spnego_mechListMIC); - } else { - /* - * It's not a sequence, so dissect it as an octet string, - * which is what it's supposed to be; that'll cause the - * right error report if it's not an octet string, either. - */ - offset = dissect_ber_octet_string(FALSE, actx, tree, tvb, offset, - hf_spnego_mechListMIC, &mechListMIC_tvb); - - /* - * Now, we should be able to dispatch with that tvbuff. - */ - if (mechListMIC_tvb && next_level_value) - call_dissector(next_level_value->handle, mechListMIC_tvb, actx->pinfo, tree); - return offset; + if (mechListMIC_tvb && (tvb_reported_length(mechListMIC_tvb) > 0) ){ + gssapi_oid_value *value=next_level_value; + + if(value){ + call_dissector(value->handle, mechListMIC_tvb, actx->pinfo, tree); + } } #.FN_BODY NegTokenTarg/supportedMech
- Follow-Ups:
- Prev by Date: Re: [Wireshark-dev] Fixing the problem where Wireshark misdissects the SPNEGO negTokenInit
- Next by Date: Re: [Wireshark-dev] I think the following patch fixes the longstanding bug in the SPNEGO dissector
- Previous by thread: Re: [Wireshark-dev] Byte matching
- Next by thread: Re: [Wireshark-dev] I think the following patch fixes the longstanding bug in the SPNEGO dissector
- Index(es):