Wireshark · Ethereal-dev: Re: [Ethereal-dev] Radius protocol enhancements for Tunneling protocols

[Guy Harris <gharris@xxxxxxxxx>]

On Wed, Oct 10, 2001 at 11:33:45AM +0200, Novotny Pavel wrote:
> we needed to analyse RADIUS packets in our L2TP project.  Because
> there were not all atributes supported for tunnel accounting I have made
> some changes in module packet-radius.c.  These changes are made
> according to RFC 2867, 2868. 
> Mr Feyaerts, the author of this module, wrote that he has no
> objections to include these changes in the next release of Ethereal. 
> What can I do for this?

You can, for starters, fix the bugs that GCC found in the code.

The code that does

        case( RADIUS_INTEGER4_TAGGED ):
                intval = tvb_get_ntohl(tvb,offset+2);
                /* Tagged ? */
                if (intval >> 24) {
                        sprintf(textbuffer, "Tag:%u, Value:%s",
                                intval >> 24,
                                intval & 0xffffff);
                        break;
                }
                strcpy(cont,rd_match_strval(intval,valstrarr));
                break;

has two problems:

	1) the

                        sprintf(textbuffer, "Tag:%u, Value:%s",
                                intval >> 24,
                                intval & 0xffffff);

	   code is passing an integer value, "intval & 0xffffff", but
	   the matching part of the format string, "Value:%s", expects a
	   string value;

	2) the code uses "valstrarr", but never *sets* "valstrarr".

In addition, you can merge your changes into the current CVS version of
the RADIUS dissector, which is later than the version that you'd
modified; one of your changes collides with a change that was made after
the version you modified - RADIUS_TIMESTAMP is already defined as 21, so
RADIUS_INTEGER4_TAGGED would have to be defined as 22.

I've attached a patch that modifies the current CVS version of the
RADIUS dissector (which will be in the next Ethereal release, which
will probably come out soon) to include your changes (with
RADIUS_INTEGER4_TAGGED defined as 22, not 21).

In addition:
 
> I send this module in attachment. 

you can send a patch, rather than sending the entire file, so that if
other changes are made to the RADIUS dissector before you send your
fixed version of the changes, we don't have to, as I just did now, diff
your version of "packet-radius.c" against the version you'd modified,
and apply the results as a patch to the current version.

? errs.SUNC
? errs
? errs.MSVC
? packet-sdp.c.CONVERSATION
? packet-smb.c.NEW
? packet-sdp.c.NEW
? ABOUT-NLS
? packet-ipx.c.WTF
? HOME-DIRECTORY-HELL
? epan/charsets.h
Index: packet-radius.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-radius.c,v
retrieving revision 1.36
diff -c -r1.36 packet-radius.c
*** packet-radius.c	2001/10/01 08:47:50	1.36
--- packet-radius.c	2001/10/12 17:24:10
***************
*** 145,155 ****
--- 145,161 ----
  #define RD_TP_TUNNEL_MEDIUM_TYPE 65
  #define RD_TP_TUNNEL_CLIENT_ENDPOINT 66
  #define RD_TP_TUNNEL_SERVER_ENDPOINT 67
+ #define RD_TP_TUNNEL_CONNECTION 68
  #define RD_TP_TUNNEL_PASSWORD 69
  #define RD_TP_CONNECT_INFO 77
  #define RD_TP_MESSAGE_AUTHENTICATOR 80
+ #define RD_TP_TUNNEL_PRIVATE_GROUP_ID 81
  #define RD_TP_TUNNEL_ASSIGNMENT_ID 82
+ #define RD_TP_TUNNEL_TUNNEL_PREFERENCE 83
+ #define RD_TP_TUNNEL_PACKETS_LOST 86
  #define RD_TP_NAS_PORT_ID 87
+ #define RD_TP_TUNNEL_CLIENT_AUTH_ID 90
+ #define RD_TP_TUNNEL_SERVER_AUTH_ID 91
  #define RD_TP_ASCEND_MODEM_PORTNO 120
  #define RD_TP_ASCEND_MODEM_SLOTNO 121
  #define RD_TP_ASCEND_MULTILINK_ID 187
***************
*** 196,201 ****
--- 202,208 ----
  #define RADIUS_STRING_TAGGED 19
  #define RADIUS_VENDOR_SPECIFIC 20
  #define RADIUS_TIMESTAMP 21
+ #define RADIUS_INTEGER4_TAGGED 22
  
  static value_string radius_vals[] = {
   {RADIUS_ACCESS_REQUEST, "Access Request"},
***************
*** 321,326 ****
--- 328,339 ----
  {3, "Interim-Update"},
  {7,"Accounting-On"},
  {8,"Accounting-Off"},
+ {9, "Tunnel-Start"}, /* Tunnel accounting */
+ {10, "Tunnel-Stop"}, /* Tunnel accounting */
+ {11, "Tunnel-Reject"}, /* Tunnel accounting */
+ {12, "Tunnel-Link-Start"}, /* Tunnel accounting */
+ {13, "Tunnel-Link-Stop"}, /* Tunnel accounting */
+ {14, "Tunnel-Link-Reject"}, /* Tunnel accounting */
  {0,NULL}};
  
  static value_string radius_accounting_authentication_vals[]=
***************
*** 471,481 ****
--- 484,500 ----
  { RD_TP_TUNNEL_MEDIUM_TYPE, RADIUS_TUNNEL_MEDIUM_TYPE},
  { RD_TP_TUNNEL_CLIENT_ENDPOINT, RADIUS_STRING_TAGGED},
  { RD_TP_TUNNEL_SERVER_ENDPOINT, RADIUS_STRING_TAGGED},
+ { RD_TP_TUNNEL_CONNECTION, RADIUS_BINSTRING},
  { RD_TP_TUNNEL_PASSWORD, RADIUS_STRING_TAGGED},
  { RD_TP_CONNECT_INFO, RADIUS_STRING_TAGGED},
  { RD_TP_MESSAGE_AUTHENTICATOR, RADIUS_BINSTRING},
+ { RD_TP_TUNNEL_PRIVATE_GROUP_ID, RADIUS_STRING_TAGGED},
  { RD_TP_TUNNEL_ASSIGNMENT_ID, RADIUS_STRING_TAGGED},
+ { RD_TP_TUNNEL_TUNNEL_PREFERENCE, RADIUS_INTEGER4_TAGGED},
+ { RD_TP_TUNNEL_PACKETS_LOST, RADIUS_INTEGER4},
  { RD_TP_NAS_PORT_ID, RADIUS_STRING},
+ { RD_TP_TUNNEL_CLIENT_AUTH_ID, RADIUS_STRING_TAGGED},
+ { RD_TP_TUNNEL_SERVER_AUTH_ID, RADIUS_STRING_TAGGED},
  { RD_TP_ASCEND_MODEM_PORTNO, RADIUS_INTEGER4},
  { RD_TP_ASCEND_MODEM_SLOTNO, RADIUS_INTEGER4},
  { RD_TP_ASCEND_MULTILINK_ID, RADIUS_INTEGER4},
***************
*** 558,568 ****
--- 577,593 ----
  { RD_TP_TUNNEL_MEDIUM_TYPE, "Tunnel Medium Type"},
  { RD_TP_TUNNEL_CLIENT_ENDPOINT, "Tunnel Client Endpoint"},
  { RD_TP_TUNNEL_SERVER_ENDPOINT, "Tunnel Server Endpoint"},
+ { RD_TP_TUNNEL_CONNECTION, "Tunnel Connection"},
  { RD_TP_TUNNEL_PASSWORD, "Tunnel Password"},
  { RD_TP_CONNECT_INFO, "Connect-Info"},
  { RD_TP_MESSAGE_AUTHENTICATOR, "Message Authenticator"},
+ { RD_TP_TUNNEL_PRIVATE_GROUP_ID, "Tunnel Private Group ID"},
  { RD_TP_TUNNEL_ASSIGNMENT_ID, "Tunnel Assignment ID"},
+ { RD_TP_TUNNEL_TUNNEL_PREFERENCE, "Tunnel Preference"},
+ { RD_TP_TUNNEL_PACKETS_LOST, "Tunnel Packets Lost"},
  { RD_TP_NAS_PORT_ID, "NAS Port ID"},
+ { RD_TP_TUNNEL_CLIENT_AUTH_ID, "Tunnel Client Auth ID"},
+ { RD_TP_TUNNEL_SERVER_AUTH_ID, "Tunnel Server Auth ID"},
  { RD_TP_ASCEND_MODEM_PORTNO, "Ascend Modem Port No"},
  { RD_TP_ASCEND_MODEM_SLOTNO, "Ascend Modem Slot No"},
  { RD_TP_ASCEND_MULTILINK_ID, "Ascend Multilink ID"},
***************
*** 792,797 ****
--- 817,833 ----
  		rtimestamp=ctime((time_t*)&intval);
  		rtimestamp[strlen(rtimestamp)-1]=0;
  		sprintf(cont,"%d (%s %s)", tvb_get_ntohl(tvb,offset+2), rtimestamp, *tzname);
+ 		break;
+         case( RADIUS_INTEGER4_TAGGED ):
+ 		intval = tvb_get_ntohl(tvb,offset+2);
+ 		/* Tagged ? */
+ 		if (intval >> 24) {
+ 			sprintf(textbuffer, "Tag:%u, Value:%s",
+ 				intval >> 24,
+ 				intval & 0xffffff);
+ 			break;
+ 		}
+ 		strcpy(cont,rd_match_strval(intval,valstrarr));
  		break;
          case( RADIUS_UNKNOWN ):
          default: