Ticket History #786: Keytab code should cache last-read key

From jik@kamens.brookline.ma.us Wed Feb 12 12:16:35 1997
Received: from MIT.EDU (PACIFIC-CARRIER-ANNEX.MIT.EDU [18.69.0.28]) by rt-11.MIT.EDU (8.7.5/8.7.3) with SMTP id MAA10162 for <bugs@RT-11.MIT.EDU>; Wed, 12 Feb 1997 12:16:35 -0500
Received: from jik.saturn.net by MIT.EDU with SMTP
id AA22887; Wed, 12 Feb 97 12:16:32 EST
Received: (from jik@localhost)
by jik.saturn.net (8.8.4/8.8.4)
id MAA22859; Wed, 12 Feb 1997 12:20:21 -0500
Message-Id: <199702121720.MAA22859@jik.saturn.net>
Date: Wed, 12 Feb 1997 12:20:21 -0500
From: "Jonathan I. Kamens" <jik@kamens.brookline.ma.us>
Reply-To: jik@kamens.brookline.ma.us
To: krb5-bugs@MIT.EDU
Subject: keytab library should cache most-recently-used keytab entry
X-Send-Pr-Version: 3.99

OpenVision Technologies, Inc.
N/A
The speed of krb5_kt_get_entry() for persistent applications
can be greatly improved by making it cache the keytab entry most
recently returned to a caller, so that if a subsequent request is for
the same keytab entry, it can be returned without needing to open and
scan the keytab file again.

This doesn't provide a significant performance improvement on
systems which cache files in memory and whose disks aren't very busy,
but it will improve performance on systems which don't cache files
and/or which are busy enough that the keytab file isn't kept in the
cache all the time.

--- krb5-1.0/src/lib/krb5/keytab/file/ktf_close.c Thu Apr 13 20:53:11 1995
+++ krb5-1.0/src/lib/krb5/keytab/file/ktf_close.c Wed Feb 12 09:30:54 1997
@@ -41,6 +41,8 @@
*/
{
krb5_xfree(KTFILENAME(id));
+ if (KTCACHE(id).magic)
+ krb5_kt_free_entry(context, &KTCACHE(id));
krb5_xfree(id->data);
id->ops = 0;
krb5_xfree(id);
--- krb5-1.0/src/lib/krb5/keytab/file/ktf_g_ent.c Tue Nov 19 17:07:30 1996
+++ krb5-1.0/src/lib/krb5/keytab/file/ktf_g_ent.c Wed Feb 12 09:17:08 1997
@@ -41,6 +41,42 @@
krb5_keytab_entry cur_entry, new_entry;
krb5_error_code kerror = 0;
int found_wrong_kvno = 0;
+ krb5_enctype entry_type;
+
+ switch (enctype) {
+ case ENCTYPE_DES_CBC_CRC:
+ case ENCTYPE_DES_CBC_MD5:
+ case ENCTYPE_DES_CBC_MD4:
+ case ENCTYPE_DES_CBC_RAW:
+ enctype = ENCTYPE_DES_CBC_CRC;
+ break;
+ }
+
+ if (KTCACHE(id).magic) {
+ new_entry = KTCACHE(id);
+
+ entry_type = new_entry.key.enctype;
+ switch(entry_type) {
+ case ENCTYPE_DES_CBC_CRC:
+ case ENCTYPE_DES_CBC_MD5:
+ case ENCTYPE_DES_CBC_MD4:
+ case ENCTYPE_DES_CBC_RAW:
+ entry_type = ENCTYPE_DES_CBC_CRC;
+ break;
+ }
+
+ if (((enctype == IGNORE_ENCTYPE) || (enctype == entry_type)) &&
+ krb5_principal_compare(context, principal, new_entry.principal) &&
+ ((kvno == IGNORE_VNO) || (kvno == new_entry.vno))) {
+ if ((kerror = krb5_ktfileint_copy_entry(context, &new_entry, entry)))
+ return(kerror);
+ return 0;
+ }
+ else {
+ krb5_kt_free_entry(context, &new_entry);
+ KTCACHE(id).magic = 0;
+ }
+ }

/* Open the keyfile for reading */
if ((kerror = krb5_ktfileint_openr(context, id)))
@@ -54,20 +90,9 @@
cur_entry.vno = 0;
cur_entry.key.contents = 0;
while (TRUE) {
- krb5_enctype entry_type;
-
if ((kerror = krb5_ktfileint_read_entry(context, id, &new_entry)))
break;

- switch (enctype) {
- case ENCTYPE_DES_CBC_CRC:
- case ENCTYPE_DES_CBC_MD5:
- case ENCTYPE_DES_CBC_MD4:
- case ENCTYPE_DES_CBC_RAW:
- enctype = ENCTYPE_DES_CBC_CRC;
- break;
- }
-
entry_type = new_entry.key.enctype;
switch(entry_type) {
case ENCTYPE_DES_CBC_CRC:
@@ -118,5 +143,6 @@
return kerror;
}
*entry = cur_entry;
+ (void) krb5_ktfileint_copy_entry(context, &cur_entry, &KTCACHE(id));
return 0;
}
--- krb5-1.0/src/lib/krb5/keytab/file/ktf_resolv.c Tue Aug 29 14:34:32 1995
+++ krb5-1.0/src/lib/krb5/keytab/file/ktf_resolv.c Wed Feb 12 09:17:33 1997
@@ -53,6 +53,7 @@

(void) strcpy(data->name, name);
data->openf = 0;
+ memset(&data->cache, 0, sizeof(data->cache));

(*id)->data = (krb5_pointer)data;
(*id)->magic = KV5M_KEYTAB;
--- krb5-1.0/src/lib/krb5/keytab/file/ktf_util.c Mon Sep 11 21:25:09 1995
+++ krb5-1.0/src/lib/krb5/keytab/file/ktf_util.c Wed Feb 12 09:24:54 1997
@@ -771,3 +771,31 @@
return 0;
}

+/*
+ * Copy a keytab entry's data from an old structure into a new
+ * (already allocated) structure.
+ */
+
+krb5_error_code
+krb5_ktfileint_copy_entry(context, old_entry, new_entry)
+ krb5_context context;
+ krb5_keytab_entry *old_entry, *new_entry;
+{
+ krb5_error_code code;
+ krb5_keytab_entry tmp_entry;
+
+ tmp_entry = *old_entry;
+
+ if ((code = krb5_copy_principal(context, old_entry->principal,
+ &tmp_entry.principal)))
+ return code;
+
+ if ((code = krb5_copy_keyblock_contents(context, &old_entry->key,
+ &tmp_entry.key))) {
+ (void) krb5_free_principal(context, tmp_entry.principal);
+ return code;
+ }
+
+ *new_entry = tmp_entry;
+ return 0;
+}
--- krb5-1.0/src/lib/krb5/keytab/file/ktf_wreslv.c Tue Aug 29 14:34:35 1995
+++ krb5-1.0/src/lib/krb5/keytab/file/ktf_wreslv.c Wed Feb 12 09:18:15 1997
@@ -53,6 +53,7 @@

(void) strcpy(data->name, name);
data->openf = 0;
+ memset(&data->cache, 0, sizeof(data->cache));

(*id)->data = (krb5_pointer)data;
(*id)->magic = KV5M_KEYTAB;
--- krb5-1.0/src/lib/krb5/keytab/file/ktfile.h Fri Jan 26 00:20:17 1996
+++ krb5-1.0/src/lib/krb5/keytab/file/ktfile.h Wed Feb 12 09:16:18 1997
@@ -49,6 +49,7 @@
char *name; /* Name of the file */
FILE *openf; /* open file, if any. */
int version; /* Version number of keytab */
+ krb5_keytab_entry cache; /* Cached last keytab entry returned by get_entry */
} krb5_ktfile_data;

/*
@@ -58,6 +59,7 @@
#define KTFILENAME(id) (((krb5_ktfile_data *)(id)->data)->name)
#define KTFILEP(id) (((krb5_ktfile_data *)(id)->data)->openf)
#define KTVERSION(id) (((krb5_ktfile_data *)(id)->data)->version)
+#define KTCACHE(id) (((krb5_ktfile_data *)(id)->data)->cache)

extern struct _krb5_kt_ops krb5_ktf_ops;
extern struct _krb5_kt_ops krb5_ktf_writable_ops;
@@ -160,6 +162,11 @@
krb5_keytab,
krb5_int32 *,
krb5_int32 *));
+
+krb5_error_code krb5_ktfileint_copy_entry
+ PROTOTYPE((krb5_context,
+ krb5_keytab_entry *,
+ krb5_keytab_entry *));


#endif /* KRB5_KTFILE__ */

From: "Jonathan I. Kamens" <jik@cam.ov.com>
To: krb5-bugs@MIT.EDU
Cc: Subject: Re: krb5-libs/371: keytab code should cache the most-recently-used keytab entry
Date: Mon, 11 Aug 1997 16:03:29 -0400

Has this patch been merged back into the main branch of the MIT tree?
Will it be included in the next MIT release?

jik

From jik@kamens.brookline.ma.us Tue Nov 23 16:48:18 1999
Received: from MIT.EDU (PACIFIC-CARRIER-ANNEX.MIT.EDU [18.69.0.28]) by rt-11.MIT.EDU (8.7.5/8.7.3) with SMTP id QAA02226 for <bugs@RT-11.MIT.EDU>; Tue, 23 Nov 1999 16:48:18 -0500
Received: from nautilus.shore.net by MIT.EDU with SMTP
id AA03876; Tue, 23 Nov 99 16:48:45 EST
Received: from jik.shore.net [206.243.167.15]
by nautilus.shore.net with esmtp (Exim)
for krb5-bugs@mit.edu
id 11qNnE-0001v5-00; Tue, 23 Nov 1999 16:48:08 -0500
Received: from jik2.kamens.brookline.ma.us (IDENT:root@jik2.kamens.brookline.ma.us [192.168.0.3])
by jik.shore.net (8.9.0/8.9.0) with ESMTP id QAA20904;
Tue, 23 Nov 1999 16:47:34 -0500
Received: (from jik@localhost)
by jik2.kamens.brookline.ma.us (8.9.3/8.9.3) id QAA20937;
Tue, 23 Nov 1999 16:48:23 -0500
Message-Id: <199911232148.QAA20937@jik2.kamens.brookline.ma.us>
Date: Tue, 23 Nov 1999 16:48:23 -0500
From: Jonathan Kamens <jik@kamens.brookline.ma.us>
Reply-To: jik@kamens.brookline.ma.us
To: krb5-bugs@MIT.EDU
Cc: jik@kamens.brookline.ma.us
Subject: Keytab code should cache last-read key
X-Send-Pr-Version: 3.99



System: Linux jik2 2.2.12-20 #14 Thu Nov 18 20:57:02 EST 1999 i686 unknown
Architecture: i686

Every time a Kerberos application server gets a request, it
has to read its ticket file to process the request. For a
very busy server, this is very inefficient. It's also
unnecessary, since the vast majority of servers receive
requests for the same service principal over and over.

The patch below caches the last key read from the keytab, so
that the keytab does not have to be read over and over as long
as requests keep coming in for the same service principal.

Note that these patches were previously submitted against krb5
1.0. I tested them pretty extensively before submitting them
against 1.0, but I have *not* tested them extensively against
1.1, since I'm not currently using any krb5 application
servers. I've simply merged my changes from 1.0 into a
current 1.1 tree, which required some additional changes since
the 1.1 code in question had changed a bit.

Therefore, somebody who is more actively involved in doing
krb5 development than I am right now should test these
changes before committing and releasing them.

Index: ./src/lib/krb5/keytab/file/ktf_close.c
--- ktf_close.c 1999/11/23 20:25:26 1.1.1.1
+++ ktf_close.c 1999/11/23 21:37:04 1.3
@@ -44,6 +44,8 @@
*/
{
krb5_xfree(KTFILENAME(id));
+ if (KTCACHE(id).magic)
+ krb5_kt_free_entry(context, &KTCACHE(id));
krb5_xfree(id->data);
id->ops = 0;
krb5_xfree(id);
Index: ./src/lib/krb5/keytab/file/ktf_g_ent.c
--- ktf_g_ent.c 1999/11/23 20:25:26 1.1.1.1
+++ ktf_g_ent.c 1999/11/23 21:37:04 1.3
@@ -45,11 +45,11 @@
krb5_error_code kerror = 0;
int found_wrong_kvno = 0;
krb5_boolean similar;
+ /* 0 - haven't checked the cache yet
+ 1 - checked the cache, but haven't opened the ticket file yet
+ 2 - opened the ticket file */
+ int checking_state = 0;

- /* Open the keyfile for reading */
- if ((kerror = krb5_ktfileint_openr(context, id)))
- return(kerror);
-
/*
* For efficiency and simplicity, we'll use a while true that
* is exited with a break statement.
@@ -59,8 +59,28 @@
cur_entry.key.contents = 0;

while (TRUE) {
- if ((kerror = krb5_ktfileint_read_entry(context, id, &new_entry)))
+ if (checking_state == 0) {
+ checking_state++;
+ if (! KTCACHE(id).magic)
+ continue;
+ krb5_ktfileint_copy_entry(context, &KTCACHE(id), &new_entry);
+ }
+ else {
+ if (checking_state == 1) {
+ /* Free the cache entry, if there is one */
+ if (KTCACHE(id).magic) {
+ krb5_kt_free_entry(context, &KTCACHE(id));
+ KTCACHE(id).magic = 0;
+ }
+ /* Open the keyfile for reading */
+ if ((kerror = krb5_ktfileint_openr(context, id)))
+ return(kerror);
+ checking_state++;
+ }
+
+ if ((kerror = krb5_ktfileint_read_entry(context, id, &new_entry)))
break;
+ }

/* by the time this loop exits, it must either free cur_entry,
and copy new_entry there, or free new_entry. Otherwise, it
@@ -137,6 +157,8 @@
krb5_kt_free_entry(context, &cur_entry);
return kerror;
}
+ if (checking_state == 2)
+ (void) krb5_ktfileint_copy_entry(context, &cur_entry, &KTCACHE(id));
*entry = cur_entry;
return 0;
}
Index: ./src/lib/krb5/keytab/file/ktfile.h
--- ktfile.h 1999/11/23 20:25:26 1.1.1.1
+++ ktfile.h 1999/11/23 21:37:04 1.3
@@ -52,6 +52,7 @@
char *name; /* Name of the file */
FILE *openf; /* open file, if any. */
int version; /* Version number of keytab */
+ krb5_keytab_entry cache; /* Cached last keytab entry returned by get_entry */
} krb5_ktfile_data;

/*
@@ -61,6 +62,7 @@
#define KTFILENAME(id) (((krb5_ktfile_data *)(id)->data)->name)
#define KTFILEP(id) (((krb5_ktfile_data *)(id)->data)->openf)
#define KTVERSION(id) (((krb5_ktfile_data *)(id)->data)->version)
+#define KTCACHE(id) (((krb5_ktfile_data *)(id)->data)->cache)

extern struct _krb5_kt_ops krb5_ktf_ops;
extern struct _krb5_kt_ops krb5_ktf_writable_ops;
@@ -163,6 +165,11 @@
krb5_keytab,
krb5_int32 *,
krb5_int32 *));
+
+krb5_error_code krb5_ktfileint_copy_entry
+ PROTOTYPE((krb5_context,
+ krb5_keytab_entry *,
+ krb5_keytab_entry *));


#endif /* KRB5_KTFILE__ */
Index: ./src/lib/krb5/keytab/file/ktf_resolv.c
--- ktf_resolv.c 1999/11/23 20:25:26 1.1.1.1
+++ ktf_resolv.c 1999/11/23 21:37:04 1.3
@@ -56,6 +56,7 @@

(void) strcpy(data->name, name);
data->openf = 0;
+ memset(&data->cache, 0, sizeof(data->cache));
data->version = 0;

(*id)->data = (krb5_pointer)data;
Index: ./src/lib/krb5/keytab/file/ktf_util.c
--- ktf_util.c 1999/11/23 20:25:26 1.1.1.1
+++ ktf_util.c 1999/11/23 21:37:04 1.3
@@ -774,3 +774,31 @@
return 0;
}

+/*
+ * Copy a keytab entry's data from an old structure into a new
+ * (already allocated) structure.
+ */
+
+krb5_error_code
+krb5_ktfileint_copy_entry(context, old_entry, new_entry)
+ krb5_context context;
+ krb5_keytab_entry *old_entry, *new_entry;
+{
+ krb5_error_code code;
+ krb5_keytab_entry tmp_entry;
+
+ tmp_entry = *old_entry;
+
+ if ((code = krb5_copy_principal(context, old_entry->principal,
+ &tmp_entry.principal)))
+ return code;
+
+ if ((code = krb5_copy_keyblock_contents(context, &old_entry->key,
+ &tmp_entry.key))) {
+ (void) krb5_free_principal(context, tmp_entry.principal);
+ return code;
+ }
+
+ *new_entry = tmp_entry;
+ return 0;
+}
Index: ./src/lib/krb5/keytab/file/ktf_wreslv.c
--- ktf_wreslv.c 1999/11/23 20:25:26 1.1.1.1
+++ ktf_wreslv.c 1999/11/23 21:37:04 1.3
@@ -56,6 +56,7 @@

(void) strcpy(data->name, name);
data->openf = 0;
+ memset(&data->cache, 0, sizeof(data->cache));
data->version = 0;

(*id)->data = (krb5_pointer)data;

From: tytso@MIT.EDU
To: jik@kamens.brookline.ma.us
Cc: krb5-bugs@MIT.EDU, krb5-unassigned@RT-11.MIT.EDU,
jik@kamens.brookline.ma.us, gnats-admin@RT-11.MIT.EDU,
krb5-prs@RT-11.MIT.EDU
Subject: Re: krb5-libs/786: Keytab code should cache last-read key
Date: Mon, 29 Nov 1999 14:25:23 -0500

Date: Tue, 23 Nov 1999 16:48:23 -0500
From: Jonathan Kamens <jik@kamens.brookline.ma.us>

Every time a Kerberos application server gets a request, it
has to read its ticket file to process the request. For a
very busy server, this is very inefficient. It's also
unnecessary, since the vast majority of servers receive
requests for the same service principal over and over.

The patch below caches the last key read from the keytab, so
that the keytab does not have to be read over and over as long
as requests keep coming in for the same service principal.

Should there perhaps be a stat() test to see if the keytab has changed,
or is the assumption that you will kill and restart all application
daemons when the keytab file is changed?

- Ted

From: Jonathan Kamens <jik@kamens.brookline.ma.us>
To: tytso@MIT.EDU
Cc: krb5-bugs@MIT.EDU, krb5-unassigned@RT-11.MIT.EDU,
gnats-admin@RT-11.MIT.EDU, krb5-prs@RT-11.MIT.EDU
Subject: Re: krb5-libs/786: Keytab code should cache last-read key
Date: Mon, 29 Nov 1999 14:40:44 -0500


It doesn't really matter if the keytab has changed, since the caching
I implemented checks the key version number of the cached key. That
is, if a new key is issued for the server, then the tickets issued to
clients will use the new key, and therefore they will not match the
cached key, so the keytab file will be read.

I suppose you could say that a site might bump the version number on
an application server key to lock out clients which snooped an old key
version or something like that, but that seems exceedingly unlikely,
and I don't think protecting against that remote possibility is worth
the additional disk activity of a stat() for every client connection.

jik

From: tytso@MIT.EDU
To: jik@kamens.brookline.ma.us
Cc: krb5-bugs@MIT.EDU, krb5-unassigned@RT-11.MIT.EDU,
gnats-admin@RT-11.MIT.EDU, krb5-prs@RT-11.MIT.EDU
Subject: Re: krb5-libs/786: Keytab code should cache last-read key
Date: Tue, 30 Nov 1999 15:55:54 -0500

Date: Mon, 29 Nov 1999 14:40:44 -0500
From: Jonathan Kamens <jik@kamens.brookline.ma.us>

It doesn't really matter if the keytab has changed, since the caching
I implemented checks the key version number of the cached key. That
is, if a new key is issued for the server, then the tickets issued to
clients will use the new key, and therefore they will not match the
cached key, so the keytab file will be read.

Good point, and if a site wants to bump the version number to lock out a
compromised key, it's probably not unreasonable to require that the
adminsitrator kill and restart the application server. (And of course,
none of this applies for servers run out of inetd.)

- Ted