Remove the GSSAPI specific members out of packet_info structure.

The last piece was the NTLMSSP dissector and that is now handled by passing a pointer to a tvbuff* as dissector data for the NTLMSSP dissector to (possibly) "return" a tvbuff* with decrypted data.

Change-Id: I2606172e4d0ebb5fc6353921d5b5f41a4792f9e5
Reviewed-on: https://code.wireshark.org/review/12232
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

4 files changed, 47 insertions, 156 deletions

diff --git a/epan/dissectors/packet-gssapi.c b/epan/dissectors/packet-gssapi.c
index ec0c54e8f9..1f556af7d1 100644
--- a/epan/dissectors/packet-gssapi.c
+++ b/epan/dissectors/packet-gssapi.c
@@ -336,9 +336,9 @@ dissect_gssapi_work(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
 									pinfo, subtree);
 				}
 				else if( encrypt_info->gssapi_encrypted_tvb ) {
-					return_offset = call_dissector(ntlmssp_data_only_handle,
+					return_offset = call_dissector_with_data(ntlmssp_data_only_handle,
 									tvb_new_subset_remaining(encrypt_info->gssapi_encrypted_tvb, 0),
-									pinfo, subtree);
+									pinfo, subtree, &encrypt_info->gssapi_decrypted_tvb);
 					encrypt_info->gssapi_data_encrypted = TRUE;
 				}
 		   		goto done;
@@ -521,49 +521,29 @@ dissect_gssapi_work(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
 	return return_offset;
 }
 
-/* XXX - This should be TEMPORARY until these members in are removed from packet_info */
-static void packet_info_to_gssapi_encrypt(packet_info *pinfo, gssapi_encrypt_info_t* encrypt_info)
-{
-	encrypt_info->decrypt_gssapi_tvb = pinfo->decrypt_gssapi_tvb;
-	encrypt_info->gssapi_wrap_tvb = pinfo->gssapi_wrap_tvb;
-	encrypt_info->gssapi_encrypted_tvb = pinfo->gssapi_encrypted_tvb;
-	encrypt_info->gssapi_decrypted_tvb = pinfo->gssapi_decrypted_tvb;
-	encrypt_info->gssapi_data_encrypted = pinfo->gssapi_data_encrypted;
-}
-
 static int
 dissect_gssapi_work_wrapper(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, gssapi_encrypt_info_t* encrypt_info, gboolean is_verifier)
 {
 	int ret;
 	gssapi_encrypt_info_t pass_encrypt_info;
 
-	/* XXX - This is setup to hopefully remove the need for these members in packet_info
-	 * Setup the dissector to take them as arguments and for now, convert to
-	 * packet_info
-	 */
+	/* Ensure a non-null encryption structure */
 	if (encrypt_info != NULL)
 	{
 		pass_encrypt_info = *encrypt_info;
 	}
 	else
 	{
-		packet_info_to_gssapi_encrypt(pinfo, &pass_encrypt_info);
+		memset(&pass_encrypt_info, 0, sizeof(pass_encrypt_info));
 	}
 
 	ret = dissect_gssapi_work(tvb, pinfo, tree, is_verifier, &pass_encrypt_info);
 
+	/* Restore any changes to provided encryption structure */
 	if (encrypt_info != NULL)
 	{
 		*encrypt_info = pass_encrypt_info;
 	}
-	else
-	{
-		/* Just clean up */
-		pinfo->decrypt_gssapi_tvb=0;
-		pinfo->gssapi_wrap_tvb=NULL;
-		pinfo->gssapi_encrypted_tvb=NULL;
-		pinfo->gssapi_decrypted_tvb=NULL;
-	}
 
 	return ret;
 }
diff --git a/epan/dissectors/packet-gssapi.h b/epan/dissectors/packet-gssapi.h
index 1801bef07f..a62caaf840 100644
--- a/epan/dissectors/packet-gssapi.h
+++ b/epan/dissectors/packet-gssapi.h
@@ -34,11 +34,19 @@ typedef struct _gssapi_oid_value {
 	const gchar *comment;  /* For the comment */
 } gssapi_oid_value;
 
-/* Created as an attempt to remove members out of packet_info.
-   Data structure to be passed between dissectors */
 #define DECRYPT_GSSAPI_NORMAL   1
 #define DECRYPT_GSSAPI_DCE  2
 
+/**< Extra data for handling of decryption of GSSAPI wrapped tvbuffs.
+	Caller sets decrypt_gssapi_tvb if this service is requested.
+	If gssapi_encrypted_tvb is NULL, then the rest of the tvb data following
+	the gssapi blob itself is decrypted othervise the gssapi_encrypted_tvb
+	tvb will be decrypted (DCERPC has the data before the gssapi blob)
+	If, on return, gssapi_data_encrypted is FALSE, the wrapped tvbuff
+	was signed (i.e., an encrypted signature was present, to check
+	whether the data was modified by a man in the middle) but not sealed
+	(i.e., the data itself wasn't encrypted).
+*/
 typedef struct _gssapi_encrypt_info
 {
 	guint16 decrypt_gssapi_tvb;
diff --git a/epan/dissectors/packet-ntlmssp.c b/epan/dissectors/packet-ntlmssp.c
index 9199d9274c..4b7c2ef950 100644
--- a/epan/dissectors/packet-ntlmssp.c
+++ b/epan/dissectors/packet-ntlmssp.c
@@ -1888,7 +1888,7 @@ get_encrypted_state(packet_info *pinfo, int cryptpeer)
   }
 }
 
-static void
+static tvbuff_t*
 decrypt_data_payload(tvbuff_t *tvb, int offset, guint32 encrypted_block_length,
                      packet_info *pinfo, proto_tree *tree _U_, gpointer key);
 static void
@@ -1974,7 +1974,7 @@ dissect_ntlmssp_payload(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi
   return offset;
 }
 
-static void
+static tvbuff_t*
 decrypt_data_payload(tvbuff_t *tvb, int offset, guint32 encrypted_block_length,
                      packet_info *pinfo, proto_tree *tree _U_, gpointer key)
 {
@@ -1999,19 +1999,19 @@ decrypt_data_payload(tvbuff_t *tvb, int offset, guint32 encrypted_block_length,
                                      pinfo->destport, 0);
     if (conversation == NULL) {
       /* There is no conversation, thus no encryption state */
-      return ;
+      return NULL;
     }
 
     conv_ntlmssp_info = (ntlmssp_info *)conversation_get_proto_data(conversation,
                                                     proto_ntlmssp);
     if (conv_ntlmssp_info == NULL) {
       /* There is no NTLMSSP state tied to the conversation */
-      return ;
+      return NULL;
     }
     if (conv_ntlmssp_info->rc4_state_initialized != 1) {
       /* The crypto sybsystem is not initialized.  This means that either
          the conversation did not include a challenge, or that we do not have the right password */
-      return;
+      return NULL;
     }
     if (key != NULL) {
       stored_packet_ntlmssp_info = (ntlmssp_packet_info *)g_hash_table_lookup(hash_packet, key);
@@ -2040,7 +2040,7 @@ decrypt_data_payload(tvbuff_t *tvb, int offset, guint32 encrypted_block_length,
 
       if (rc4_state == NULL) {
         /* There is no encryption state, so we cannot decrypt */
-        return ;
+        return NULL;
       }
 
       /* Store the decrypted contents in the packet state struct
@@ -2080,7 +2080,7 @@ decrypt_data_payload(tvbuff_t *tvb, int offset, guint32 encrypted_block_length,
 
   add_new_data_source(pinfo, decr_tvb,
                       "Decrypted data");
-  pinfo->gssapi_decrypted_tvb =  decr_tvb;
+  return decr_tvb;
 }
 
 static int
@@ -2343,12 +2343,16 @@ decrypt_verifier(tvbuff_t *tvb, int offset, guint32 encrypted_block_length,
 
 /* Used when NTLMSSP is done over DCE/RPC because in this case verifier and real payload are not contigious*/
 static int
-dissect_ntlmssp_payload_only(tvbuff_t *tvb, packet_info *pinfo, _U_ proto_tree *tree, void *data _U_)
+dissect_ntlmssp_payload_only(tvbuff_t *tvb, packet_info *pinfo, _U_ proto_tree *tree, void *data)
 {
   volatile int          offset       = 0;
   proto_tree *volatile  ntlmssp_tree = NULL;
   guint32               encrypted_block_length;
+  tvbuff_t *volatile    decr_tvb;
+  tvbuff_t**            ret_decr_tvb = (tvbuff_t**)data;
 
+  if (ret_decr_tvb)
+    *ret_decr_tvb = NULL;
   /* the magic ntlm is the identifier of a NTLMSSP packet that's 00 00 00 01
    */
   encrypted_block_length = tvb_captured_length (tvb);
@@ -2381,7 +2385,9 @@ dissect_ntlmssp_payload_only(tvbuff_t *tvb, packet_info *pinfo, _U_ proto_tree *
     /* Version number */
 
     /* Try to decrypt */
-    decrypt_data_payload (tvb, offset, encrypted_block_length, pinfo, ntlmssp_tree, NULL);
+    decr_tvb = decrypt_data_payload (tvb, offset, encrypted_block_length, pinfo, ntlmssp_tree, NULL);
+    if (ret_decr_tvb)
+       *ret_decr_tvb = decr_tvb;
     /* let's try to hook ourselves here */
 
   } CATCH_NONFATAL_ERRORS {
@@ -2463,11 +2469,11 @@ static tvbuff_t *
 wrap_dissect_ntlmssp_payload_only(tvbuff_t *tvb, tvbuff_t *auth_tvb _U_,
                                   int offset, packet_info *pinfo, dcerpc_auth_info *auth_info _U_)
 {
-  tvbuff_t *data_tvb;
+  tvbuff_t *data_tvb, *decrypted_tvb;
 
   data_tvb = tvb_new_subset_remaining(tvb, offset);
-  dissect_ntlmssp_payload_only(data_tvb, pinfo, NULL, NULL);
-  return pinfo->gssapi_decrypted_tvb;
+  dissect_ntlmssp_payload_only(data_tvb, pinfo, NULL, &decrypted_tvb);
+  return decrypted_tvb;
 }
 
 #if 0
diff --git a/epan/dissectors/packet-spnego.c b/epan/dissectors/packet-spnego.c
index a2a95bf667..9b660f41a7 100644
--- a/epan/dissectors/packet-spnego.c
+++ b/epan/dissectors/packet-spnego.c
@@ -622,28 +622,6 @@ dissect_spnego_krb5_cfx_getmic_base(tvbuff_t *tvb, int offset, packet_info *pinf
 static int
 dissect_spnego_krb5_cfx_wrap_base(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *tree, guint16 token_id, gssapi_encrypt_info_t* gssapi_encrypt);
 
-
-/* XXX - This should be TEMPORARY until these members in are removed from packet_info */
-static void packet_info_to_gssapi_encrypt(packet_info *pinfo, gssapi_encrypt_info_t* encrypt_info)
-{
-	encrypt_info->decrypt_gssapi_tvb = pinfo->decrypt_gssapi_tvb;
-	encrypt_info->gssapi_wrap_tvb = pinfo->gssapi_wrap_tvb;
-	encrypt_info->gssapi_encrypted_tvb = pinfo->gssapi_encrypted_tvb;
-	encrypt_info->gssapi_decrypted_tvb = pinfo->gssapi_decrypted_tvb;
-	encrypt_info->gssapi_data_encrypted = pinfo->gssapi_data_encrypted;
-}
-
-static void gssapi_encrypt_to_packet_info(packet_info *pinfo, gssapi_encrypt_info_t* encrypt_info)
-{
-	pinfo->decrypt_gssapi_tvb = encrypt_info->decrypt_gssapi_tvb;
-	pinfo->gssapi_wrap_tvb = encrypt_info->gssapi_wrap_tvb;
-	pinfo->gssapi_encrypted_tvb = encrypt_info->gssapi_encrypted_tvb;
-	pinfo->gssapi_decrypted_tvb = encrypt_info->gssapi_decrypted_tvb;
-	pinfo->gssapi_data_encrypted = encrypt_info->gssapi_data_encrypted;
-}
-
-
-
 static int
 dissect_spnego_krb5(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data)
 {
@@ -658,7 +636,6 @@ dissect_spnego_krb5(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* d
 	gint32 tag;
 	guint32 len;
 	gssapi_encrypt_info_t* encrypt_info = (gssapi_encrypt_info_t*)data;
-	gssapi_encrypt_info_t pass_encrypt_info;
 	asn1_ctx_t asn1_ctx;
 	asn1_ctx_init(&asn1_ctx, ASN1_ENC_BER, TRUE, pinfo);
 
@@ -762,28 +739,7 @@ dissect_spnego_krb5(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* d
 	  break;
 
 	case KRB_TOKEN_WRAP:
-		/* XXX - This is setup to hopefully remove the need for these members in packet_info
-		* If data is supplied to dissector, use it.  Otherwise convert to packet_info
-		*/
-		if (encrypt_info != NULL)
-		{
-			pass_encrypt_info = *encrypt_info;
-		}
-		else
-		{
-			packet_info_to_gssapi_encrypt(pinfo, &pass_encrypt_info);
-		}
-
-		offset = dissect_spnego_krb5_wrap_base(tvb, offset, pinfo, subtree, token_id, &pass_encrypt_info);
-
-		if (encrypt_info != NULL)
-		{
-			*encrypt_info = pass_encrypt_info;
-		}
-		else
-		{
-			gssapi_encrypt_to_packet_info(pinfo, &pass_encrypt_info);
-		}
+		offset = dissect_spnego_krb5_wrap_base(tvb, offset, pinfo, subtree, token_id, encrypt_info);
 	  break;
 
 	case KRB_TOKEN_DELETE_SEC_CONTEXT:
@@ -795,28 +751,7 @@ dissect_spnego_krb5(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* d
 	  break;
 
 	case KRB_TOKEN_CFX_WRAP:
-		/* XXX - This is setup to hopefully remove the need for these members in packet_info
-		* If data is supplied to dissector, use it.  Otherwise convert to packet_info
-		*/
-		if (encrypt_info != NULL)
-		{
-			pass_encrypt_info = *encrypt_info;
-		}
-		else
-		{
-			packet_info_to_gssapi_encrypt(pinfo, &pass_encrypt_info);
-		}
-
-		offset = dissect_spnego_krb5_cfx_wrap_base(tvb, offset, pinfo, subtree, token_id, &pass_encrypt_info);
-
-		if (encrypt_info != NULL)
-		{
-			*encrypt_info = pass_encrypt_info;
-		}
-		else
-		{
-			gssapi_encrypt_to_packet_info(pinfo, &pass_encrypt_info);
-		}
+		offset = dissect_spnego_krb5_cfx_wrap_base(tvb, offset, pinfo, subtree, token_id, encrypt_info);
 	  break;
 
 	default:
@@ -1321,11 +1256,12 @@ dissect_spnego_krb5_wrap_base(tvbuff_t *tvb, int offset, packet_info *pinfo
 	}
 
 	/* Is the data encrypted? */
-	gssapi_encrypt->gssapi_data_encrypted=(seal_alg!=KRB_SEAL_ALG_NONE);
+	if (gssapi_encrypt != NULL)
+		gssapi_encrypt->gssapi_data_encrypted=(seal_alg!=KRB_SEAL_ALG_NONE);
 
 #ifdef HAVE_KERBEROS
 #define GSS_ARCFOUR_WRAP_TOKEN_SIZE 32
-	if(gssapi_encrypt->decrypt_gssapi_tvb){
+	if(gssapi_encrypt && gssapi_encrypt->decrypt_gssapi_tvb){
 		/* if the caller did not provide a tvb, then we just use
 		   whatever is left of our current tvb.
 		*/
@@ -1493,7 +1429,8 @@ dissect_spnego_krb5_cfx_wrap_base(tvbuff_t *tvb, int offset, packet_info *pinfo
 	flags = tvb_get_guint8(tvb, offset);
 	offset = dissect_spnego_krb5_cfx_flags(tvb, offset, tree, flags);
 
-	gssapi_encrypt->gssapi_data_encrypted=(flags & 2);
+	if (gssapi_encrypt != NULL)
+		gssapi_encrypt->gssapi_data_encrypted=(flags & 2);
 
 	/* Skip the filler */
 
@@ -1521,6 +1458,9 @@ dissect_spnego_krb5_cfx_wrap_base(tvbuff_t *tvb, int offset, packet_info *pinfo
 			    ENC_BIG_ENDIAN);
 	offset += 8;
 
+	if (gssapi_encrypt == NULL) /* Probably shoudn't happen, but just protect ourselves */
+		return offset;
+
 	/* Checksum of plaintext padded data */
 
 	if (gssapi_encrypt->gssapi_data_encrypted) {
@@ -1706,7 +1646,6 @@ dissect_spnego_krb5_wrap(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree
 	int offset = 0;
 	guint16 token_id;
 	gssapi_encrypt_info_t* encrypt_info = (gssapi_encrypt_info_t*)data;
-	gssapi_encrypt_info_t pass_encrypt_info;
 
 	item = proto_tree_add_item(tree, hf_spnego_krb5, tvb, 0, -1, ENC_NA);
 
@@ -1732,28 +1671,7 @@ dissect_spnego_krb5_wrap(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree
 	  break;
 
 	case KRB_TOKEN_WRAP:
-		/* XXX - This is setup to hopefully remove the need for these members in packet_info
-		* If data is supplied to dissector, use it.  Otherwise convert to packet_info
-		*/
-		if (encrypt_info != NULL)
-		{
-			pass_encrypt_info = *encrypt_info;
-		}
-		else
-		{
-			packet_info_to_gssapi_encrypt(pinfo, &pass_encrypt_info);
-		}
-
-		offset = dissect_spnego_krb5_wrap_base(tvb, offset, pinfo, subtree, token_id, &pass_encrypt_info);
-
-		if (encrypt_info != NULL)
-		{
-			*encrypt_info = pass_encrypt_info;
-		}
-		else
-		{
-			gssapi_encrypt_to_packet_info(pinfo, &pass_encrypt_info);
-		}
+		offset = dissect_spnego_krb5_wrap_base(tvb, offset, pinfo, subtree, token_id, encrypt_info);
 	  break;
 
 	case KRB_TOKEN_CFX_GETMIC:
@@ -1761,28 +1679,7 @@ dissect_spnego_krb5_wrap(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree
 	  break;
 
 	case KRB_TOKEN_CFX_WRAP:
-		/* XXX - This is setup to hopefully remove the need for these members in packet_info
-		* If data is supplied to dissector, use it.  Otherwise convert to packet_info
-		*/
-		if (encrypt_info != NULL)
-		{
-			pass_encrypt_info = *encrypt_info;
-		}
-		else
-		{
-			packet_info_to_gssapi_encrypt(pinfo, &pass_encrypt_info);
-		}
-
-		offset = dissect_spnego_krb5_cfx_wrap_base(tvb, offset, pinfo, subtree, token_id, &pass_encrypt_info);
-
-		if (encrypt_info != NULL)
-		{
-			*encrypt_info = pass_encrypt_info;
-		}
-		else
-		{
-			gssapi_encrypt_to_packet_info(pinfo, &pass_encrypt_info);
-		}
+		offset = dissect_spnego_krb5_cfx_wrap_base(tvb, offset, pinfo, subtree, token_id, encrypt_info);
 	  break;
 
 	default:
@@ -2064,7 +1961,7 @@ void proto_register_spnego(void) {
         NULL, HFILL }},
 
 /*--- End of included file: packet-spnego-hfarr.c ---*/
-#line 1519 "../../asn1/spnego/packet-spnego-template.c"
+#line 1416 "../../asn1/spnego/packet-spnego-template.c"
 	};
 
 	/* List of subtrees */
@@ -2087,7 +1984,7 @@ void proto_register_spnego(void) {
     &ett_spnego_InitialContextToken_U,
 
 /*--- End of included file: packet-spnego-ettarr.c ---*/
-#line 1529 "../../asn1/spnego/packet-spnego-template.c"
+#line 1426 "../../asn1/spnego/packet-spnego-template.c"
 	};
 
 	static ei_register_info ei[] = {