Improve handling of VOIP VLAN queries and replies.

For queries, there appear to be two different versions, one with a
2-byte value of some unknown type and one with a 1-byte value that
appears to be an "appliance type" code followed by a 2-byte VLAN ID.

For replies, there only appears to be a version with a 1-byte "appliance
type" followed by a 2-byte VLAN ID, but handle a too-short payload.

Also point to http://www.rhyshaden.com/cdp.htm in some comments.

Change-Id: If1b476d5e6b23c7e0ba027835c6f0c84c8b723b7
Reviewed-on: https://code.wireshark.org/review/25249
Reviewed-by: Guy Harris <guy@alum.mit.edu>

1 files changed, 50 insertions, 33 deletions

diff --git a/epan/dissectors/packet-cdp.c b/epan/dissectors/packet-cdp.c
index 17bd381c47..c389e606e3 100644
--- a/epan/dissectors/packet-cdp.c
+++ b/epan/dissectors/packet-cdp.c
@@ -38,9 +38,13 @@
  *
  * and
  *
- *      http://www.cisco.com/c/en/us/support/docs/switches/catalyst-4500-series-switches/13414-103.html#cdp
+ *    http://www.cisco.com/c/en/us/support/docs/switches/catalyst-4500-series-switches/13414-103.html#cdp
  *
  * for some more information on CDP version 2 (a superset of version 1).
+ *
+ * Also see
+ *
+ *    http://www.rhyshaden.com/cdp.htm
  */
 
 void proto_register_cdp(void);
@@ -577,52 +581,58 @@ dissect_cdp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_)
             break;
 
         case TYPE_VOIP_VLAN_REPLY:
+            tlvi = NULL;
             if (tree) {
-                if (length >= 7) {
-                    tlv_tree = proto_tree_add_subtree_format(cdp_tree, tvb, offset, length, ett_cdp_tlv, NULL,
-                                               "VoIP VLAN Reply: %u", tvb_get_ntohs(tvb, offset + 5));
+                guint32 vlan_id;
+
+                tlv_tree = proto_tree_add_subtree(cdp_tree, tvb,
+                                           offset, length, ett_cdp_tlv, &tlvi,
+                                           "VoIP VLAN Reply");
+                proto_tree_add_item(tlv_tree, hf_cdp_tlvtype, tvb, offset + TLV_TYPE, 2, ENC_BIG_ENDIAN);
+                proto_tree_add_item(tlv_tree, hf_cdp_tlvlength, tvb, offset + TLV_LENGTH, 2, ENC_BIG_ENDIAN);
+                if (length == 6) {
+                    /*
+                     * XXX - this doesn't appear to happen, so report it
+                     * as an error.
+                     */
+                    proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 2, ENC_NA);
                 } else {
                     /*
-                     * XXX - what are these?  I've seen them in some captures;
-                     * they have a length of 6, and run up to the end of
-                     * the packet, so if we try to dissect it the same way
-                     * we dissect the 7-byte ones, we report a malformed
-                     * frame.
+                     * XXX - the first byte appears to be a 1-byte
+                     * "appliance type" code.
                      */
-                    tlv_tree = proto_tree_add_subtree(cdp_tree, tvb,
-                                               offset, length, ett_cdp_tlv, NULL, "VoIP VLAN Reply");
-                }
-                proto_tree_add_item(tlv_tree, hf_cdp_tlvtype, tvb, offset + TLV_TYPE, 2, ENC_BIG_ENDIAN);
-                proto_tree_add_item(tlv_tree, hf_cdp_tlvlength, tvb, offset + TLV_LENGTH, 2, ENC_BIG_ENDIAN);
-                proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 1, ENC_NA);
-                if (length >= 7) {
-                    proto_tree_add_item(tlv_tree, hf_cdp_voice_vlan, tvb, offset + 5, 2, ENC_BIG_ENDIAN);
+                    proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 1, ENC_NA);
+                    proto_tree_add_item_ret_uint(tlv_tree, hf_cdp_voice_vlan, tvb, offset + 5, 2, ENC_BIG_ENDIAN, &vlan_id);
+                    proto_item_append_text(tlvi, ": VLAN %u", vlan_id);
                 }
             }
             offset += length;
             break;
 
         case TYPE_VOIP_VLAN_QUERY:
+            tlvi = NULL;
             if (tree) {
-                if (length >= 7) {
-                    tlv_tree = proto_tree_add_subtree_format(cdp_tree, tvb, offset, length,
-                                               ett_cdp_tlv, NULL, "VoIP VLAN Query: %u", tvb_get_ntohs(tvb, offset + 5));
+                guint32 vlan_id;
+
+                tlv_tree = proto_tree_add_subtree(cdp_tree, tvb,
+                                           offset, length, ett_cdp_tlv, &tlvi,
+                                           "VoIP VLAN Query");
+                proto_tree_add_item(tlv_tree, hf_cdp_tlvtype, tvb, offset + TLV_TYPE, 2, ENC_BIG_ENDIAN);
+                proto_tree_add_item(tlv_tree, hf_cdp_tlvlength, tvb, offset + TLV_LENGTH, 2, ENC_BIG_ENDIAN);
+                if (length == 6) {
+                    /*
+                     * This is some unknown value; it's typically 0x20 0x00,
+                     * which, as a big-endian value, is not a VLAN ID, as
+                     * VLAN IDs are 12 bits long.
+                     */
+                    proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 2, ENC_BIG_ENDIAN);
                 } else {
                     /*
-                     * XXX - what are these?  I've seen them in some captures;
-                     * they have a length of 6, and run up to the end of
-                     * the packet, so if we try to dissect it the same way
-                     * we dissect the 7-byte ones, we report a malformed
-                     * frame.
+                     * XXX - is this a 1-byte "appliance type" code?
                      */
-                    tlv_tree = proto_tree_add_subtree(cdp_tree, tvb,
-                                               offset, length, ett_cdp_tlv, NULL, "VoIP VLAN Query");
-                }
-                proto_tree_add_item(tlv_tree, hf_cdp_tlvtype, tvb, offset + TLV_TYPE, 2, ENC_BIG_ENDIAN);
-                proto_tree_add_item(tlv_tree, hf_cdp_tlvlength, tvb, offset + TLV_LENGTH, 2, ENC_BIG_ENDIAN);
-                proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 1, ENC_NA);
-                if (length >= 7) {
-                    proto_tree_add_item(tlv_tree, hf_cdp_voice_vlan, tvb, offset + 5, 2, ENC_BIG_ENDIAN);
+                    proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 1, ENC_NA);
+                    proto_tree_add_item_ret_uint(tlv_tree, hf_cdp_voice_vlan, tvb, offset + 5, 2, ENC_BIG_ENDIAN, &vlan_id);
+                    proto_item_append_text(tlvi, ": VLAN %u", vlan_id);
                 }
             }
             offset += length;
@@ -1123,6 +1133,13 @@ dissect_address_tlv(tvbuff_t *tvb, int offset, int length, proto_tree *tree)
         }
     }
     if ((protocol_type == PROTO_TYPE_IEEE_802_2) && (protocol_length == 8) && (etypeid > 0)) {
+        /*
+         * See also:
+         *
+         *    http://www.rhyshaden.com/cdp.htm
+         *
+         * where other Ethertypes are mentioned.
+         */
         switch (etypeid) {
 
         case ETHERTYPE_IPv6: