On Sat, Sep 20, 2003 at 02:44:09AM -0700, Guy Harris wrote:
> > I've still to complete the accounting part,
> Presumably that's why it gets a
>
> packet-tacacs.h:417: warning: `tacplus_acct_flags' defined but not used
>
> warning - the accounting part dissector will presumably use that
> variable.
Exactly :) May be I can keep that variable in my cvs version until I
complete the accounting part...
> I just checked in an additional change to arrange that the buffer
> allocated for the decrypted data be freed when the tvbuff to which it
> refers is freed (along the lines of what other decrypting dissectors
> do).
Ok, I found also a leek in md5_xor... It's possible to leave the function
without freeing md5_buff.
Attached you'll find the merge of my latest version and the one in current CVS tree.
I've replaced the buff=g_malloc + tvb_get_nstringz0 with buff=tvb_get_string.
Now the md5_xor function doesn't return value and use the CLEANUP call
Different RAS have different password with the same tacacs+ server, so it's
possible to have the need of different tacpluskey....
Where can I look for some hint about implementing this feature ?
It's possible to have a kind of popup to add the password to the tcp stream ?
--
Ciao,
Emanuele
--- mytacacs/packet-tacacs.c 2003-09-21 11:06:24.000000000 +0200
+++ /.1/cvs-mirror/ethereal/packet-tacacs.c 2003-09-21 09:41:25.000000000 +0200
@@ -47,7 +47,7 @@
#include "crypt-md5.h"
#include "packet-tacacs.h"
-static void md5_xor( u_char *data, char *key, int data_len, u_char *session_id, u_char version, u_char seq_no );
+static int md5_xor( u_char *data, char *key, int data_len, guint32 session_id, u_char version, u_char seq_no );
static int proto_tacacs = -1;
static int hf_tacacs_version = -1;
@@ -329,16 +329,15 @@
tacplus_tvb_setup( tvbuff_t *tvb, tvbuff_t **dst_tvb, packet_info *pinfo, guint32 len, guint8 version )
{
guint8 *buff;
- u_char session_id[4];
-
-/* session_id is in NETWORK Byte Order, and is used as byte array in the md5_xor */
-
- tvb_memcpy(tvb, (guint8*)session_id, 4,4);
+ guint32 tmp_sess;
+
+ buff=g_malloc(len);
- buff = tvb_memdup(tvb, TAC_PLUS_HDR_SIZE, len);
+ tvb_memcpy(tvb,(guint8*)&tmp_sess,4,4);
+ tvb_memcpy(tvb, buff, TAC_PLUS_HDR_SIZE, len);
- md5_xor( buff, tacplus_key, len, session_id,version, tvb_get_guint8(tvb,2) );
+ md5_xor( buff, tacplus_key, len, tmp_sess,version, tvb_get_guint8(tvb,2) );
/* Allocate a new tvbuff, referring to the decrypted data. */
*dst_tvb = tvb_new_real_data( buff, len, len );
@@ -556,7 +555,8 @@
val=tvb_get_ntohs( tvb, AUTHEN_C_USER_LEN_OFF );
proto_tree_add_text( tree, tvb, AUTHEN_C_USER_LEN_OFF, 2 , "User length: %d", val );
if( val ){
- buff=tvb_get_string( tvb, var_off, val );
+ buff=g_malloc(val+1);
+ tvb_get_nstringz0( tvb, var_off, val+1, buff );
proto_tree_add_text( tree, tvb, var_off, val, "User: %s", buff );
var_off+=val;
g_free(buff);
@@ -594,7 +594,8 @@
proto_tree_add_text( tree, tvb, AUTHEN_R_SRV_MSG_LEN_OFF, 2 ,
"Server message length: %d", val );
if( val ) {
- buff=tvb_get_string(tvb, var_off, val );
+ buff=g_malloc(val+1);
+ tvb_get_nstringz0(tvb, var_off, val+1, buff);
proto_tree_add_text(tree, tvb, var_off, val, "Server message: %s", buff );
var_off+=val;
g_free(buff);
@@ -703,7 +704,8 @@
proto_tree_add_text( tree, tvb, ACCT_R_SRV_MSG_LEN_OFF, 2 ,
"Server message length: %d", val );
if( val ) {
- buff=tvb_get_string( tvb, var_off, val );
+ buff=(guint8*)g_malloc(val+1);
+ tvb_get_nstringz0( tvb, var_off, val+1, buff );
proto_tree_add_text( tree, tvb, var_off,
val, "Server message: %s", buff );
var_off+=val;
@@ -715,7 +717,8 @@
proto_tree_add_text( tree, tvb, ACCT_R_DATA_LEN_OFF, 2 ,
"Data length: %d", val );
if( val ) {
- buff= tvb_get_string( tvb, var_off, val );
+ buff=(guint8*)g_malloc(val+1);
+ tvb_get_nstringz0( tvb, var_off, val+1, buff );
proto_tree_add_text( tree, tvb, var_off,
val, "Data: %s", buff );
g_free(buff);
@@ -914,8 +917,9 @@
proto_register_field_array(proto_tacplus, hf, array_length(hf));
proto_register_subtree_array(ett, array_length(ett));
tacplus_module = prefs_register_protocol (proto_tacplus, NULL);
- prefs_register_string_preference ( tacplus_module, "key",
- "TACACS+ Encryption Key", "TACACS+ Encryption Key", &tacplus_key );
+ prefs_register_string_preference(tacplus_module, "key",
+ "TACACS+ Encryption Key", "TACACS+ Encryption Key",
+ &tacplus_key);
}
void
@@ -928,11 +932,10 @@
dissector_add("tcp.port", TCP_PORT_TACACS, tacplus_handle);
}
-
#define MD5_LEN 16
-
-static void
-md5_xor( u_char *data, char *key, int data_len, u_char *session_id, u_char version, u_char seq_no )
+
+static int
+md5_xor( u_char *data, char *key, int data_len, guint32 session_id, u_char version, u_char seq_no )
{
int i,j,md5_len;
md5_byte_t *md5_buff;
@@ -940,21 +943,20 @@
md5_byte_t *mdp;
md5_state_t mdcontext;
- md5_len = 4 /* sizeof(session_id) */ + strlen(key)
+ md5_len = sizeof(session_id) + strlen(key)
+ sizeof(version) + sizeof(seq_no);
md5_buff = (md5_byte_t*)g_malloc(md5_len+MD5_LEN);
- CLEANUP_PUSH( g_free, md5_buff );
-
mdp = md5_buff;
- *(guint32*)mdp = *(guint32*)session_id;
- mdp += 4 ;
+ bcopy(&session_id, mdp, sizeof(session_id));
+ mdp += sizeof(session_id);
bcopy(key, mdp, strlen(key));
mdp += strlen(key);
- *mdp++ = version;
- *mdp++ = seq_no;
-
+ bcopy(&version, mdp, sizeof(version));
+ mdp += sizeof(version);
+ bcopy(&seq_no, mdp, sizeof(seq_no));
+ mdp += sizeof(seq_no);
md5_init(&mdcontext);
md5_append(&mdcontext, md5_buff, md5_len);
@@ -963,10 +965,8 @@
for (i = 0; i < data_len; i += 16) {
for (j = 0; j < 16; j++) {
- if ((i + j) >= data_len) {
- i = data_len+1; /* To exit from the external loop */
- break;
- }
+ if ((i + j) >= data_len)
+ return 0;
data[i + j] ^= hash[j];
}
bcopy( hash, mdp , MD5_LEN );
@@ -974,5 +974,6 @@
md5_append(&mdcontext, md5_buff, md5_len);
md5_finish(&mdcontext,hash);
}
- CLEANUP_CALL_AND_POP;
+ g_free( md5_buff );
+ return 0;
}
