Ticket History #3099: error handling in keytab manipulation routines

From krb5-bugs-incoming-bounces@PCH.mit.edu Thu Jun 16 18:01:47 2005
Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90]) by krbdev.mit.edu (8.9.3p2) with ESMTP
id SAA14980; Thu, 16 Jun 2005 18:01:47 -0400 (EDT)
Received: from pch.mit.edu (pch.mit.edu [127.0.0.1])
by pch.mit.edu (8.12.8p2/8.12.8) with ESMTP id j5GM1CWn017301
for <krb5-send-pr@krbdev.mit.edu>; Thu, 16 Jun 2005 18:01:12 -0400
Received: from pacific-carrier-annex.mit.edu (PACIFIC-CARRIER-ANNEX.MIT.EDU
[18.7.21.83])
by pch.mit.edu (8.12.8p2/8.12.8) with ESMTP id j5G3g1Wn016589
for <krb5-bugs-incoming@PCH.mit.edu>; Wed, 15 Jun 2005 23:42:01 -0400
Received: from mx1.redhat.com (mx1.redhat.com [66.187.233.31])
j5G3fuaM001077
for <krb5-bugs@mit.edu>; Wed, 15 Jun 2005 23:41:56 -0400 (EDT)
Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com
[172.16.52.254])
by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id j5G3ft2T026753
for <krb5-bugs@mit.edu>; Wed, 15 Jun 2005 23:41:55 -0400
Received: from devserv.devel.redhat.com (devserv.devel.redhat.com
[172.16.58.1])
by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id j5G3ftu10092
for <krb5-bugs@mit.edu>; Wed, 15 Jun 2005 23:41:55 -0400
Received: from blade.boston.redhat.com (blade.boston.redhat.com
[172.16.80.50])j5G3ftP2023622
for <krb5-bugs@mit.edu>; Wed, 15 Jun 2005 23:41:55 -0400
Received: from blade.boston.redhat.com (localhost.localdomain [127.0.0.1])
j5G3ftDq007363
for <krb5-bugs@mit.edu>; Wed, 15 Jun 2005 23:41:55 -0400
Received: (from nalin@localhost)
by blade.boston.redhat.com (8.13.4/8.13.4/Submit) id j5G3ftbA007362;
Wed, 15 Jun 2005 23:41:55 -0400
Date: Wed, 15 Jun 2005 23:41:55 -0400
From: Nalin Dahyabhai <nalin@redhat.com>
Message-Id: <200506160341.j5G3ftbA007362@blade.boston.redhat.com>
To: krb5-bugs@mit.edu
X-send-pr-version: 3.99
X-Spam-Score: -2.601
X-Spam-Flag: NO
X-Scanned-By: MIMEDefang 2.42
X-Mailman-Approved-At: Thu, 16 Jun 2005 18:01:10 -0400
Subject: error handling in keytab manipulation routines
X-BeenThere: krb5-bugs-incoming@mailman.mit.edu
X-Mailman-Version: 2.1
Precedence: list
Reply-To: nalin@redhat.com
Sender: krb5-bugs-incoming-bounces@PCH.mit.edu
Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu



System: Linux blade.boston.redhat.com 2.6.11-1.1366_FC4smp #1 SMP Mon May 30 00:12:23 EDT 2005 i686 athlon i386 GNU/Linux
Architecture: i686

The routines which deal with keytab files don't react well to empty
files, which are an unfortunately common configuration error. An
empty file to which the user can't write triggers other errors.
I'm marking this confidential but non-critical because it's usually
triggered by a configuration or operator error, but as a crasher it
might have implications which I'm not aware of. Feel free to change
it to non-confidential if you wish.
Run "klist -k -t /dev/null".
When the file is closed after an error, make sure that an error code is
returned to the caller (short fread() or fwrite() may not set errno, so
my guess for a proper error code was EIO). If we fclose() the file,
clear the pointer so that if we accidentally try to close it again, we
at least don't chase into random heap memory.

--- krb5-1.4.1/src/lib/krb5/keytab/kt_file.c 2004-12-03 20:42:57.000000000 -0500
+++ krb5-1.4.1/src/lib/krb5/keytab/kt_file.c 2005-06-15 17:48:20.000000000 -0400
@@ -1099,17 +1099,19 @@
kt_vno = htons(krb5_kt_default_vno);
KTVERSION(id) = krb5_kt_default_vno;
if (!xfwrite(&kt_vno, sizeof(kt_vno), 1, KTFILEP(id))) {
- kerror = errno;
+ kerror = errno ? errno : EIO;
(void) krb5_unlock_file(context, fileno(KTFILEP(id)));
(void) fclose(KTFILEP(id));
+ KTFILEP(id) = 0;
return kerror;
}
} else {
/* gotta verify it instead... */
if (!xfread(&kt_vno, sizeof(kt_vno), 1, KTFILEP(id))) {
- kerror = errno;
+ kerror = errno ? errno : EIO;
(void) krb5_unlock_file(context, fileno(KTFILEP(id)));
(void) fclose(KTFILEP(id));
+ KTFILEP(id) = 0;
return kerror;
}
kt_vno = KTVERSION(id) = ntohs(kt_vno);
@@ -1117,6 +1119,7 @@
(kt_vno != KRB5_KT_VNO_1)) {
(void) krb5_unlock_file(context, fileno(KTFILEP(id)));
(void) fclose(KTFILEP(id));
+ KTFILEP(id) = 0;
return KRB5_KEYTAB_BADVNO;
}
}

This does need to be addressed, but I don't think Nalin's patch deals with a short write properly,
since the written length isn't actually checked. We should also check feof and/or ferror when
reads return 0.

I am working on a patch to clear KTFILEP any time we close the file.

Set KTFILEP field to null any time we close the file.

Commit By: raeburn



Revision: 19739
Changed Files:
_U trunk/
U trunk/src/lib/krb5/keytab/kt_file.c