[1743] in Kerberos-V5-bugs
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