[1743] in Kerberos-V5-bugs

home help back first fref pref prev next nref lref last post

Bug fixes for May beta 5 code

daemon@ATHENA.MIT.EDU (Mark Carson)
Tue Dec 19 13:47:08 1995

Date: Tue, 19 Dec 1995 13:46:57 -0500 (EST)
From: Mark Carson <carson@cs.umd.edu>
To: krb5-bugs@MIT.EDU

I have gotten the May 1995 Kerberos 5 beta 5 code running on AIX
version 3.2.5.  I haven't tested everything yet, but at least the basic
items are working.  Getting things to work required some bug and
configuration fixing.  In this note, I report the generic bugs I fixed;
in a followup, I will list AIX-specific changes.

Notes: 1. The code I worked from was the May krb5b5 distribution with
only the telnet patch applied.  I wouldn't be surprised if the bugs I
report here have been fixed elsewhere; I'd appreciate pointers to other
bodies of fixes.

2. I used an aggressive debugging version of malloc to isolate many of
the bugs.  This isolates every allocated block on its own page(s), and
permanently invalidates every freed block.  A good stocking-stuffer for
the programmer who has everything.  (Unfortunately this version is
quite AIX-specific.)

Mark Carson

Bugs noted:

1. src/lib/krb4/kparse.c -- two variables are declared both static and
(in the header file) extern.  I assumed they were meant to be extern.

2. src/appl/bsd/kcmd.c -- includes to unistd.h and stdlib.h were omitted.

3. src/appl/telnet/libtelnet/kerberos5.c: line 276 --  comment close
was * / instead of */.

4. src/lib/krb5/os/mk_faddr.c: line 52 -- wrong length used for
allocation.

5. src/lib/gssapi/krb5/init_sec_context.c: lines 431-455 -- were
essentially duplicated.  The killer here was a block was freed twice.

6. src/lib/krb5/krb/mk_cred.c etc. -- every call to the macro
CLEANUP_PUSH was being given a pointer to a pointer instead of a
pointer (e.g. CLEANUP_PUSH(&local_fulladdr.contents, free) instead of
CLEANUP_PUSH(local_fulladdr.contents, free)).

7. src/lib/krb5/krb/rd_cred.c: line 132 -- missing parentheses, so
wrong length was allocated.

8. src/lib/krb5/krb/rd_cred.c: line 173 -- the forwarded ticket was
being freed immediately after being received and parsed, but before
being stored.  I simply dropped the free call, but presumably it really
should be moved elsewhere.

Context diffs:

*** src/lib/krb4/kparse.c.orig	Thu Apr 27 11:49:48 1995
--- src/lib/krb4/kparse.c	Tue Dec 19 11:07:13 1995
***************
*** 55,62 ****
  static char *strsave();
  #endif
  
! static int LineNbr=1;		/* current line nbr in parameter file */
! static char ErrorMsg[80];	/* meaningful only when KV_SYNTAX, PS_SYNTAX,
  				 * or PS_BAD_KEYWORD is returned by
  				 * fGetKeywordValue or fGetParameterSet */
  
--- 55,65 ----
  static char *strsave();
  #endif
  
! /* @@ Mark: These are marked static here, but extern in kparse.h.
!  * I decided to remove the static marking.
!  */
! /*@@static*/ int LineNbr=1;		/* current line nbr in parameter file */
! /*@@static*/ char ErrorMsg[80];	/* meaningful only when KV_SYNTAX, PS_SYNTAX,
  				 * or PS_BAD_KEYWORD is returned by
  				 * fGetKeywordValue or fGetParameterSet */
  
*** src/appl/bsd/kcmd.c.orig	Tue May  2 22:14:11 1995
--- src/appl/bsd/kcmd.c	Fri Dec  1 09:31:19 1995
***************
*** 21,26 ****
--- 21,33 ----
  
  /* derived from @(#)rcmd.c	5.17 (Berkeley) 6/27/88 */
       
+ #ifdef HAVE_UNISTD_H
+ #include <unistd.h>
+ #endif
+ #ifdef HAVE_STDLIB_H
+ #include <stdlib.h>
+ #endif
+ 
  #include <stdio.h>
  #include <ctype.h>
  #include <string.h>
*** src/appl/telnet/libtelnet/kerberos5.c.orig	Thu Apr 27 15:53:53 1995
--- src/appl/telnet/libtelnet/kerberos5.c	Tue Dec  5 10:50:18 1995
***************
*** 273,279 ****
  	/*
  	 * keep the key in our private storage, but don't use it yet
  	 * ---see kerberos5_reply() below 
! 	 * /
  	if (newkey && (newkey->keytype != KEYTYPE_DES)) {
  	    if (new_creds->keyblock.keytype == KEYTYPE_DES)
  		/* use the session key in credentials instead */
--- 273,279 ----
  	/*
  	 * keep the key in our private storage, but don't use it yet
  	 * ---see kerberos5_reply() below 
! 	 */
  	if (newkey && (newkey->keytype != KEYTYPE_DES)) {
  	    if (new_creds->keyblock.keytype == KEYTYPE_DES)
  		/* use the session key in credentials instead */
*** src/lib/krb5/os/mk_faddr.c.orig	Mon May  1 17:09:36 1995
--- src/lib/krb5/os/mk_faddr.c	Tue Dec  5 14:35:34 1995
***************
*** 49,55 ****
  	return EINVAL;
  
      raddr->length = kaddr->length + kport->length + (4 * sizeof(krb5_int32));
!     if (!(raddr->contents = (krb5_octet *)malloc(kaddr->length)))
  	return ENOMEM;
  
      raddr->addrtype = ADDRTYPE_ADDRPORT;
--- 49,55 ----
  	return EINVAL;
  
      raddr->length = kaddr->length + kport->length + (4 * sizeof(krb5_int32));
!     if (!(raddr->contents = (krb5_octet *)malloc(raddr->length)))
  	return ENOMEM;
  
      raddr->addrtype = ADDRTYPE_ADDRPORT;
*** src/lib/gssapi/krb5/init_sec_context.c.orig	Mon May  1 16:48:26 1995
--- src/lib/gssapi/krb5/init_sec_context.c	Tue Dec 19 11:13:03 1995
***************
*** 428,454 ****
  	    }
        	}
  
!       /* store away the sequence number */
!       ctx->seq_recv = ap_rep_data->seq_number;
  
-       /* free the ap_rep_data */
-       krb5_free_ap_rep_enc_part(context, ap_rep_data);
- 
-       /* set established */
-       ctx->established = 1;
- 
-       /* set returns */
- 
-       if (time_rec) {
- 	 if (code = krb5_timeofday(context, &now)) {
- 	    (void)krb5_gss_delete_sec_context(context, minor_status, 
- 					      (gss_ctx_id_t) ctx, NULL);
- 	    *minor_status = code;
- 	    return(GSS_S_FAILURE);
- 
- 	 }
-       }
- 
        /* store away the sequence number */
        ctx->seq_recv = ap_rep_data->seq_number;
  
--- 428,437 ----
  	    }
        	}
  
!       /* @@ Mark -- the following code was basically in here twice, so I
!        * chopped out the duplicate copy.
!        */
  
        /* store away the sequence number */
        ctx->seq_recv = ap_rep_data->seq_number;
  
***************
*** 470,475 ****
--- 453,459 ----
  	 }
  	 *time_rec = ctx->endtime - now;
        }
+       /* @@ Mark -- end of code that was duplicated */
  
        if (ret_flags)
  	 *ret_flags = GSS_C_CONF_FLAG | GSS_C_INTEG_FLAG | GSS_C_MUTUAL_FLAG;
*** src/lib/krb5/krb/mk_cred.c.orig	Tue May  2 21:18:50 1995
--- src/lib/krb5/krb/mk_cred.c	Mon Dec 11 15:33:22 1995
***************
*** 270,276 ****
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))) {
!             	CLEANUP_PUSH(&local_fulladdr.contents, free);
  	    	plocal_fulladdr = &local_fulladdr;
              } else {
                  goto error;
--- 270,276 ----
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))) {
!             	CLEANUP_PUSH(local_fulladdr.contents, free);
  	    	plocal_fulladdr = &local_fulladdr;
              } else {
                  goto error;
***************
*** 285,291 ****
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(&remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
                  CLEANUP_DONE();
--- 285,291 ----
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
                  CLEANUP_DONE();
*** src/lib/krb5/krb/mk_priv.c.orig	Tue May  2 21:18:52 1995
--- src/lib/krb5/krb/mk_priv.c	Mon Dec 11 15:38:54 1995
***************
*** 197,203 ****
  	    if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
  				  	      auth_context->local_port, 
  					      &local_fulladdr))) {
! 	    	CLEANUP_PUSH(&local_fulladdr.contents, free);
  	    	plocal_fulladdr = &local_fulladdr;
              } else {
      	    	goto error;
--- 197,203 ----
  	    if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
  				  	      auth_context->local_port, 
  					      &local_fulladdr))) {
! 	    	CLEANUP_PUSH(local_fulladdr.contents, free);
  	    	plocal_fulladdr = &local_fulladdr;
              } else {
      	    	goto error;
***************
*** 212,218 ****
  	    if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
  				 	      auth_context->remote_port, 
  					      &remote_fulladdr))){
! 	    	CLEANUP_PUSH(&remote_fulladdr.contents, free);
  	    	premote_fulladdr = &remote_fulladdr;
   	    } else {
  	        CLEANUP_DONE();
--- 212,218 ----
  	    if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
  				 	      auth_context->remote_port, 
  					      &remote_fulladdr))){
! 	    	CLEANUP_PUSH(remote_fulladdr.contents, free);
  	    	premote_fulladdr = &remote_fulladdr;
   	    } else {
  	        CLEANUP_DONE();
*** src/lib/krb5/krb/mk_safe.c.orig	Tue May  2 19:31:41 1995
--- src/lib/krb5/krb/mk_safe.c	Mon Dec 11 15:39:40 1995
***************
*** 180,186 ****
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!             	CLEANUP_PUSH(&local_fulladdr.contents, free);
  	    	plocal_fulladdr = &local_fulladdr;
              } else {
                  goto error;
--- 180,186 ----
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!             	CLEANUP_PUSH(local_fulladdr.contents, free);
  	    	plocal_fulladdr = &local_fulladdr;
              } else {
                  goto error;
***************
*** 196,202 ****
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!             	CLEANUP_PUSH(&remote_fulladdr.contents, free);
  	    	premote_fulladdr = &remote_fulladdr;
              } else {
                  CLEANUP_DONE();
--- 196,202 ----
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!             	CLEANUP_PUSH(remote_fulladdr.contents, free);
  	    	premote_fulladdr = &remote_fulladdr;
              } else {
                  CLEANUP_DONE();
*** src/lib/krb5/krb/rd_cred.c.orig	Tue May  2 21:18:53 1995
--- src/lib/krb5/krb/rd_cred.c	Tue Dec 19 11:17:58 1995
***************
*** 129,135 ****
      for (ncreds = 0; pcred->tickets[ncreds]; ncreds++);
  	
      if ((*pppcreds = 
!         (krb5_creds **)malloc((size_t)(sizeof(krb5_creds *) * ncreds + 1))) == NULL) {
          retval = ENOMEM;
          goto cleanup_cred;
      }
--- 129,135 ----
      for (ncreds = 0; pcred->tickets[ncreds]; ncreds++);
  	
      if ((*pppcreds = 
!         (krb5_creds **)malloc((size_t)(sizeof(krb5_creds *) * (ncreds + 1)))) == NULL) {
          retval = ENOMEM;
          goto cleanup_cred;
      }
***************
*** 170,176 ****
  	    goto cleanup;
  
  	pcur->ticket = *pdata;
! 	krb5_free_data(context, pdata);
  
  
          pcur->is_skey = FALSE;
--- 170,180 ----
  	    goto cleanup;
  
  	pcur->ticket = *pdata;
! 	/* @@ The code was freeing the data here, but it can't be freed
! 	 * until we store it.  For now I will just delete the call here.
! 	 */
! 	/* krb5_free_data(context, pdata);*/
! 	/* @@ Perhaps it would be appropriate to free pdata itself here? */
  
  
          pcur->is_skey = FALSE;
***************
*** 245,251 ****
              if (!(retval = krb5_make_fulladdr(context,auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!                 CLEANUP_PUSH(&local_fulladdr.contents, free);
  	        plocal_fulladdr = &local_fulladdr;
              } else {
  	        return retval;
--- 249,255 ----
              if (!(retval = krb5_make_fulladdr(context,auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!                 CLEANUP_PUSH(local_fulladdr.contents, free);
  	        plocal_fulladdr = &local_fulladdr;
              } else {
  	        return retval;
***************
*** 260,266 ****
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(&remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
  	        return retval;
--- 264,270 ----
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
  	        return retval;
*** src/lib/krb5/krb/rd_priv.c.orig	Tue May  2 21:18:55 1995
--- src/lib/krb5/krb/rd_priv.c	Mon Dec 11 15:41:34 1995
***************
*** 210,216 ****
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!                 CLEANUP_PUSH(&local_fulladdr.contents, free);
  	        plocal_fulladdr = &local_fulladdr;
              } else {
  	        return retval;
--- 210,216 ----
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!                 CLEANUP_PUSH(local_fulladdr.contents, free);
  	        plocal_fulladdr = &local_fulladdr;
              } else {
  	        return retval;
***************
*** 225,231 ****
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(&remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
                  CLEANUP_DONE();
--- 225,231 ----
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
                  CLEANUP_DONE();
*** src/lib/krb5/krb/rd_safe.c.orig	Tue May  2 21:18:57 1995
--- src/lib/krb5/krb/rd_safe.c	Mon Dec 11 15:42:30 1995
***************
*** 201,207 ****
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!                 CLEANUP_PUSH(&local_fulladdr.contents, free);
  	        plocal_fulladdr = &local_fulladdr;
              } else {
  	        return retval;
--- 201,207 ----
              if (!(retval = krb5_make_fulladdr(context, auth_context->local_addr,
                                   	      auth_context->local_port, 
  					      &local_fulladdr))){
!                 CLEANUP_PUSH(local_fulladdr.contents, free);
  	        plocal_fulladdr = &local_fulladdr;
              } else {
  	        return retval;
***************
*** 216,222 ****
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(&remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
  	        return retval;
--- 216,222 ----
              if (!(retval = krb5_make_fulladdr(context,auth_context->remote_addr,
                                   	      auth_context->remote_port, 
  					      &remote_fulladdr))){
!                 CLEANUP_PUSH(remote_fulladdr.contents, free);
  	        premote_fulladdr = &remote_fulladdr;
              } else {
  	        return retval;



Mark Carson	carson@cs.umd.edu	301-240-7348
Department of Computer Science   University of Maryland, College Park -or-
FSC 182/3F42   700 N. Frederick Ave.   Gaithersburg MD 20879


home help back first fref pref prev next nref lref last post