Wireshark · Ethereal-users: Re: [ethereal-users] Bad NETBIOS Packets

[Guy Harris <guy@xxxxxxxxxx>]

> Actually, when handed a U frame, shouldn't the LLC dissector just call
> the per-SAP dissectors for UI frames, not for other U frames?  This
> means that "dissect_xdlc_control()" should return not just XDLC_S,
> XDLC_U, or XDLC_I, but, for U frames, return something that includes the
> modifier field as well.

I've checked in a change to make the LLC dissector call the per-SAP, and
SNAP per-Ethertype, dissectors only for I and UI frames, not for other
frames.

(We may also want to do the same for LAPB.)

You can get that from the CVS tree if you can use anonymous CVS (see the
Ethereal home page for information on that), or apply the following
patch (this is the change checked into the CVS tree, your
mileage^H^H^H^H^H^H^Hline numbers may vary):

Index: packet-llc.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-llc.c,v
retrieving revision 1.19
retrieving revision 1.20
diff -c -r1.19 -r1.20
*** packet-llc.c	1999/08/10 20:05:40	1.19
--- packet-llc.c	1999/08/23 22:47:13	1.20
***************
*** 162,190 ****
  void
  capture_llc(const u_char *pd, int offset, guint32 cap_len, packet_counts *ld) {
  
- 	guint16		etype;
  	int		is_snap;
  	capture_func_t	*capture;
  
  	is_snap = (pd[offset] == 0xAA) && (pd[offset+1] == 0xAA);
- 	if (is_snap) {
- 		etype  = (pd[offset+6] << 8) | pd[offset+7];
- 		offset += 8;
- 		capture_ethertype(etype, offset, pd, cap_len, ld);
- 	}		
- 	else {
- 		capture = sap_capture_func(pd[offset]);
  
! 		/* non-SNAP */
! 		offset += 3;
  
! 		if (capture) {
! 			capture(pd, offset, cap_len, ld);
  		}
! 		else {
! 			ld->other++;
  		}
- 
  	}
  }
  
--- 162,220 ----
  void
  capture_llc(const u_char *pd, int offset, guint32 cap_len, packet_counts *ld) {
  
  	int		is_snap;
+ 	int		control;
+ 	guint16		etype;
  	capture_func_t	*capture;
  
  	is_snap = (pd[offset] == 0xAA) && (pd[offset+1] == 0xAA);
  
! 	/*
! 	 * The low-order bit of the SSAP apparently determines whether this
! 	 * is a request or a response.  (RFC 1390, "Transmission of IP and
! 	 * ARP over FDDI Networks", says
! 	 *
! 	 *	Command frames are identified by having the low order
! 	 *	bit of the SSAP address reset to zero.  Response frames
! 	 *	have the low order bit of the SSAP address set to one.
! 	 *
! 	 * and a page I've seen seems to imply that's part of 802.2.)
! 	 *
! 	 * XXX - that page also implies that LLC Type 2 always uses
! 	 * extended operation, so we don't need to determine whether
! 	 * it's basic or extended operation; is that the case?
! 	 */
! 	control = get_xdlc_control(pd, offset+2, pd[offset+1] & 0x01, TRUE);
  
! 	if (is_snap) {
! 		if (control == XDLC_I || control == (XDLC_U|XDLC_UI)) {
! 			/*
! 			 * Unnumbered Information - analyze it based on
! 			 * the Ethernet packet type.
! 			 */
! 			etype  = (pd[offset+6] << 8) | pd[offset+7];
! 			offset += 8;
! 			capture_ethertype(etype, offset, pd, cap_len, ld);
  		}
! 	}		
! 	else {
! 		if (control == XDLC_I || control == (XDLC_U|XDLC_UI)) {
! 			/*
! 			 * Unnumbered Information - analyze it based on
! 			 * the DSAP.
! 			 */
! 			capture = sap_capture_func(pd[offset]);
! 
! 			/* non-SNAP */
! 			offset += 3;
! 
! 			if (capture) {
! 				capture(pd, offset, cap_len, ld);
! 			}
! 			else {
! 				ld->other++;
! 			}
  		}
  	}
  }
  
***************
*** 193,200 ****
  
  	proto_tree	*llc_tree = NULL;
  	proto_item	*ti;
- 	guint16		etype;
  	int		is_snap;
  	dissect_func_t	*dissect;
  
  	is_snap = (pd[offset] == 0xAA) && (pd[offset+1] == 0xAA);
--- 223,231 ----
  
  	proto_tree	*llc_tree = NULL;
  	proto_item	*ti;
  	int		is_snap;
+ 	int		control;
+ 	guint16		etype;
  	dissect_func_t	*dissect;
  
  	is_snap = (pd[offset] == 0xAA) && (pd[offset+1] == 0xAA);
***************
*** 226,242 ****
  	 * extended operation, so we don't need to determine whether
  	 * it's basic or extended operation; is that the case?
  	 */
! 	dissect_xdlc_control(pd, offset+2, fd, llc_tree, hf_llc_ctrl,
  	    pd[offset+1] & 0x01, TRUE);
  
  	/*
  	 * XXX - do we want to append the SAP information to the stuff
  	 * "dissect_xdlc_control()" put in the COL_INFO column, rather
  	 * than overwriting it?
- 	 *
- 	 * XXX - we shouldn't, as far as I know, pass S frames to
- 	 * "ethertype" or "dissect", and we may have to treat I frames
- 	 * differently from U frames.
  	 */
  	if (is_snap) {
  		if (check_col(fd, COL_INFO)) {
--- 257,269 ----
  	 * extended operation, so we don't need to determine whether
  	 * it's basic or extended operation; is that the case?
  	 */
! 	control = dissect_xdlc_control(pd, offset+2, fd, llc_tree, hf_llc_ctrl,
  	    pd[offset+1] & 0x01, TRUE);
  
  	/*
  	 * XXX - do we want to append the SAP information to the stuff
  	 * "dissect_xdlc_control()" put in the COL_INFO column, rather
  	 * than overwriting it?
  	 */
  	if (is_snap) {
  		if (check_col(fd, COL_INFO)) {
***************
*** 245,255 ****
  		if (tree) {
  			proto_tree_add_item(llc_tree, hf_llc_oui, offset+3, 3,
  				pd[offset+3] << 16 | pd[offset+4] << 8 | pd[offset+5]);
  		}
- 		etype = pntohs(&pd[offset+6]);
- 		offset += 8;
- 		/* w/o even checking, assume OUI is ethertype */
- 		ethertype(etype, offset, pd, fd, tree, llc_tree, hf_llc_type);
  	}		
  	else {
  		if (check_col(fd, COL_INFO)) {
--- 272,289 ----
  		if (tree) {
  			proto_tree_add_item(llc_tree, hf_llc_oui, offset+3, 3,
  				pd[offset+3] << 16 | pd[offset+4] << 8 | pd[offset+5]);
+ 		}
+ 		if (control == (XDLC_U|XDLC_UI)) {
+ 			/*
+ 			 * Unnumbered Information - dissect it based on
+ 			 * the Ethernet packet type.
+ 			 */
+ 			etype = pntohs(&pd[offset+6]);
+ 			offset += 8;
+ 			/* w/o even checking, assume OUI is ethertype */
+ 			ethertype(etype, offset, pd, fd, tree, llc_tree,
+ 			    hf_llc_type);
  		}
  	}		
  	else {
  		if (check_col(fd, COL_INFO)) {
***************
*** 257,274 ****
  				val_to_str(pd[offset], sap_vals, "%02x"));
  		}
  
! 		dissect = sap_dissect_func(pd[offset]);
! 
! 		/* non-SNAP */
! 		offset += 3;
! 
! 		if (dissect) {
! 			dissect(pd, offset, fd, tree);
  		}
- 		else {
- 			dissect_data(pd, offset, fd, tree);
- 		}
- 
  	}
  }
  
--- 291,313 ----
  				val_to_str(pd[offset], sap_vals, "%02x"));
  		}
  
! 		if (control == (XDLC_U|XDLC_UI)) {
! 			/*
! 			 * Unnumbered Information - dissect it based on
! 			 * the DSAP.
! 			 */
! 			dissect = sap_dissect_func(pd[offset]);
! 
! 			/* non-SNAP */
! 			offset += 3;
! 
! 			if (dissect) {
! 				dissect(pd, offset, fd, tree);
! 			}
! 			else {
! 				dissect_data(pd, offset, fd, tree);
! 			}
  		}
  	}
  }
  
Index: xdlc.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/xdlc.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -c -r1.3 -r1.4
*** xdlc.c	1999/08/16 05:54:32	1.3
--- xdlc.c	1999/08/23 22:47:13	1.4
***************
*** 74,104 ****
      { 0,         NULL }
  };
  
- /*
-  * U-format modifiers.
-  */
- #define XDLC_U_MODIFIER_MASK	0xEC
- #define XDLC_UI		0x00	/* Unnumbered Information */
- #define XDLC_UP		0x20	/* Unnumbered Poll */
- #define XDLC_DISC	0x40	/* Disconnect (command) */
- #define XDLC_RD		0x40	/* Request Disconnect (response) */
- #define XDLC_UA		0x60	/* Unnumbered Acknowledge */
- #define XDLC_SNRM	0x80	/* Set Normal Response Mode */
- #define XDLC_TEST	0xC0	/* Test */
- #define XDLC_SIM	0x04	/* Set Initialization Mode (command) */
- #define XDLC_RIM	0x04	/* Request Initialization Mode (response) */
- #define XDLC_FRMR	0x84	/* Frame reject */
- #define XDLC_CFGR	0xC4	/* Configure */
- #define XDLC_SARM	0x0C	/* Set Asynchronous Response Mode (command) */
- #define XDLC_DM		0x0C	/* Disconnected mode (response) */
- #define XDLC_SABM	0x2C	/* Set Asynchronous Balanced Mode */
- #define XDLC_SARME	0x4C	/* Set Asynchronous Response Mode Extended */
- #define XDLC_SABME	0x6C	/* Set Asynchronous Balanced Mode Extended */
- #define XDLC_RESET	0x8C	/* Reset */
- #define XDLC_XID	0xAC	/* Exchange identification */
- #define XDLC_SNRME	0xCC	/* Set Normal Response Mode Extended */
- #define XDLC_BCN	0xEC	/* Beacon */
- 
  static const value_string modifier_short_vals_cmd[] = {
      { XDLC_UI,    "UI" },
      { XDLC_UP,    "UP" },
--- 74,79 ----
***************
*** 184,189 ****
--- 159,205 ----
  };
  
  int
+ get_xdlc_control(const u_char *pd, int offset, int is_response, int is_extended)
+ {
+     guint16 control;
+ 
+     switch (pd[offset] & 0x03) {
+ 
+     case XDLC_S:
+         /*
+ 	 * Supervisory frame.
+ 	 */
+ 	return XDLC_S;
+ 
+     case XDLC_U:
+ 	/*
+ 	 * Unnumbered frame.
+ 	 *
+ 	 * XXX - is this two octets, with a P/F bit, in HDLC extended
+ 	 * operation?  It's one octet in LLC, even though the control
+ 	 * field of I and S frames is a 2-byte extended-operation field
+ 	 * in LLC.  Given that there are no sequence numbers in the
+ 	 * control field of a U frame, there doesn't appear to be any
+ 	 * need for it to be 2 bytes in extended operation.
+ 	 */
+ 	control = pd[offset];
+ 
+ 	/*
+ 	 * Return the modifier as well as the XDLC_U bits, so that
+ 	 * our caller knows whether the packet is UI or something
+ 	 * else.
+ 	 */
+ 	return control & (XDLC_U_MODIFIER_MASK|0x03);
+ 
+     default:
+ 	/*
+ 	 * Information frame.
+ 	 */
+ 	return XDLC_I;
+     }
+ }
+ 
+ int
  dissect_xdlc_control(const u_char *pd, int offset, frame_data *fd,
    proto_tree *xdlc_tree, int hf_xdlc_control, 
    int is_response, int is_extended)
***************
*** 331,337 ****
  		decode_boolean_bitfield(control, 0x03, 1*8,
  		    "Unnumbered frame", NULL));
  	}
! 	return XDLC_U;
  
      default:
  	/*
--- 347,359 ----
  		decode_boolean_bitfield(control, 0x03, 1*8,
  		    "Unnumbered frame", NULL));
  	}
! 
! 	/*
! 	 * Return the modifier as well as the XDLC_U bits, so that
! 	 * our caller knows whether the packet is UI or something
! 	 * else.
! 	 */
! 	return control & (XDLC_U_MODIFIER_MASK|0x03);
  
      default:
  	/*
Index: xdlc.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/xdlc.h,v
retrieving revision 1.1
retrieving revision 1.2
diff -c -r1.1 -r1.2
*** xdlc.h	1999/08/04 04:37:46	1.1
--- xdlc.h	1999/08/23 22:47:13	1.2
***************
*** 31,36 ****
--- 31,64 ----
  #define XDLC_I		0x00	/* Information frames */
  #define XDLC_S		0x01	/* Supervisory frames */
  #define XDLC_U		0x03	/* Unnumbered frames */
+ 
+ /*
+  * U-format modifiers.
+  */
+ #define XDLC_U_MODIFIER_MASK	0xEC
+ #define XDLC_UI		0x00	/* Unnumbered Information */
+ #define XDLC_UP		0x20	/* Unnumbered Poll */
+ #define XDLC_DISC	0x40	/* Disconnect (command) */
+ #define XDLC_RD		0x40	/* Request Disconnect (response) */
+ #define XDLC_UA		0x60	/* Unnumbered Acknowledge */
+ #define XDLC_SNRM	0x80	/* Set Normal Response Mode */
+ #define XDLC_TEST	0xC0	/* Test */
+ #define XDLC_SIM	0x04	/* Set Initialization Mode (command) */
+ #define XDLC_RIM	0x04	/* Request Initialization Mode (response) */
+ #define XDLC_FRMR	0x84	/* Frame reject */
+ #define XDLC_CFGR	0xC4	/* Configure */
+ #define XDLC_SARM	0x0C	/* Set Asynchronous Response Mode (command) */
+ #define XDLC_DM		0x0C	/* Disconnected mode (response) */
+ #define XDLC_SABM	0x2C	/* Set Asynchronous Balanced Mode */
+ #define XDLC_SARME	0x4C	/* Set Asynchronous Response Mode Extended */
+ #define XDLC_SABME	0x6C	/* Set Asynchronous Balanced Mode Extended */
+ #define XDLC_RESET	0x8C	/* Reset */
+ #define XDLC_XID	0xAC	/* Exchange identification */
+ #define XDLC_SNRME	0xCC	/* Set Normal Response Mode Extended */
+ #define XDLC_BCN	0xEC	/* Beacon */
+ 
+ int get_xdlc_control(const u_char *pd, int offset, int is_response,
+   int extended);
  
  int dissect_xdlc_control(const u_char *pd, int offset, frame_data *fd,
    proto_tree *xdlc_tree, int hf_xdlc_control, int is_response, int extended);