Bug 117214 - [PATCH] krb5_parse_name() doesn't track changes to default realm
[PATCH] krb5_parse_name() doesn't track changes to default realm
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 3
Classification: Red Hat
Component: krb5 (Show other bugs)
3.0
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nalin Dahyabhai
Brian Brock
:
Depends On:
Blocks: RHEL3U8MustFix
  Show dependency treegraph
 
Reported: 2004-03-01 12:53 EST by John Haxby
Modified: 2007-11-30 17:07 EST (History)
2 users (show)

See Also:
Fixed In Version: RHBA-2006-0404
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-20 11:00:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to allow changes to the default domain to take effect (458 bytes, patch)
2004-03-01 15:20 EST, John Haxby
no flags Details | Diff
Updated patch (1.19 KB, patch)
2004-03-02 12:15 EST, John Haxby
no flags Details | Diff

  None (edit)
Description John Haxby 2004-03-01 12:53:04 EST
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030922

Description of problem:
I have to Kerberos servers, one at work and one at home, and I have
the following (among other things) in /etc/pam.d/system-auth:

auth        sufficient    /lib/security/$ISA/pam_krb5.so \
                          realm=HOME.EXAMPLE.COM
auth        sufficient    /lib/security/$ISA/pam_krb5.so \
                          use_first_pass realm=WORK.EXAMPLE.COM

(backslash-linefeed added for clarity)

pam_krb5.so called krb5_set_default_realm() for each instance. 
However, the subsequence krb5_parse_name() takes a copy of the default
realm the first time it is called.

This is bad because when I log in at work, the first PAM module tries
to find "jch@HOME.EXAMPLE.COM" (as it should), however, the second one
also tries to find "jch@HOME.EXAMPLE.COM" because krb5_parse_name()
has cached the realm.

I hacked my way around this by forcing the default realm for the
principal after each call to krb5_parse_name() in pam-kr5b-1.70's
./pam_krb5afs.c but it's not pretty and it's not really correct (for
example, it prevents me logging in as jch@ELSEWHERE.EXAMPLE.COM).

I'll have a patch for you shortly ...


Version-Release number of selected component (if applicable): 1.2.7-19


How reproducible:
Always
Comment 1 John Haxby 2004-03-01 15:20:53 EST
Created attachment 98173 [details]
Patch to allow changes to the default domain to take effect

The patch is trivial -- just make the default_realm and its associated length
non-static.

I've now tested this in both Kerberos realms -- both at work and at home -- and
it works beautifully.	I don't think the slight loss of performance really
makes much difference.
Comment 2 John Haxby 2004-03-02 12:12:54 EST
Comment on attachment 98173 [details]
Patch to allow changes to the default domain to take effect

--- ./src/lib/krb5/krb/parse.c.realm-change	2004-03-02 16:54:34.563715147
+0000
+++ ./src/lib/krb5/krb/parse.c	2004-03-02 16:59:12.023582016 +0000
@@ -76,8 +76,8 @@
	const char	*parsed_realm = NULL;
	int		fcompsize[FCOMPNUM];
	int		realmsize = 0;
-	static char	*default_realm = NULL;
-	static int	default_realm_size = 0;
+	char		*default_realm = NULL;
+	int		default_realm_size = 0;
	char		*tmpdata;
	krb5_principal	principal;
	krb5_error_code retval;
@@ -215,6 +215,7 @@
	if (tmpdata == 0) {
		krb5_xfree(principal->data);
		krb5_xfree(principal);
+		krb5_xfree(default_realm);
		return ENOMEM;
	}
	krb5_princ_set_realm_length(context, principal, realmsize);
@@ -228,6 +229,7 @@
			krb5_xfree(krb5_princ_realm(context, principal)->data);
			krb5_xfree(principal->data);
			krb5_xfree(principal);
+			krb5_xfree(default_realm);
			return(ENOMEM);
		}
		krb5_princ_component(context, principal, i)->data = tmpdata;
@@ -272,6 +274,7 @@
	*q++ = '\0';
	if (!parsed_realm)
		strcpy(krb5_princ_realm(context, principal)->data,
default_realm);
+	krb5_xfree(default_realm);
	/*
	 * Alright, we're done.  Now stuff a pointer to this monstrosity
	 * into the return variable, and let's get out of here.
Comment 3 John Haxby 2004-03-02 12:15:25 EST
Created attachment 98198 [details]
Updated patch

(Sorry about the previous comment, I thought I was editing the attachment).

I like valgrind -- the previous version of the patch lost the storage for the
default_realm if it was acquired.   This patch corrects that.
Comment 4 John Haxby 2004-07-28 05:48:11 EDT
I'm disappointed that this patch still hasn't made it into krb5; I'm
in the process of installing the beta and I've just been through the
problem of not being able to log in again because I forgot to rebuild
krb5 with the fix applied.

I've been using this patch since March and it hasn't given me any
problems at all.    Please include it in the RHEL3 U3.   Please!
Comment 8 Nalin Dahyabhai 2006-01-23 14:54:41 EST
This would change the behavior of krb5_parse_name() to conflict directly with
the API documentation, so I doubt upstream would go for it.  Rather, it looks to
me like the problem is squarely in pam_krb5.  Reassigning for tracking.
Comment 9 Nalin Dahyabhai 2006-01-23 15:15:12 EST
Oops, the docs use "local" and "default" to refer to the same realm.
Comment 10 John Haxby 2006-01-24 09:15:44 EST
I've spent an hour or two looking into my original fix.   Here's what I've found:

OK, so I'm looking at the comment for krb5_parse_name() in
src/lib/krb5/krb/parse.c (this is for krb5-1.3.4, not that it matters).  It
says, among other things:

 * get_default_realm() is called; it may return other errors.

It's lying, or at least being economical with the truth,
krb5_get_default_realm() is called just the once, the first time you call
krb_parse_name().

OK, so now in src/lib/krb5/os/def_realm.c we have krb5_get_default_realm() and
krb5_set_default_realm().  The comments say that get_default_realm() will get
the default realm from a config file, it actually only does that the first time,
subsequently it uses a static value (the one that set_default_realm()) sets
which rather makes parse_name()'s cache redundant.

The documentation for krb5_set_default_realm() says
-------------------
\begin{funcdecl}{krb5_set_default_realm}{krb5_error_code}{\funcin}
\funcarg{krb5_context}{context}
\funcarg{char *}{realm}
\end{funcdecl}

Sets the default realm to be used if no user-specified realm is
available (e.g. to interpret a user-typed principal name with the
realm omitted for convenience). (c.f. krb5_get_default_realm)

If \funcparam{realm} is NULL, then the operating system default value
will used.

Returns system errors.
------------------

It's interesting to note that it makes no mention of only being able to set the
default realm before the first call to krb5_parse_name() and it's this behaviour
that the current code enforces.   I'm sure that you can check doc/api/libos.tex
for yourself, but there's no similar sort of caveat for krb5_get_default_realm().


Interestingly in doc/api/krb5.tex, the documentation for krb5_parse_name() says
"If the realm is not specified, the local realm is used."   This is at odds with
the documentation for krb5_set_default_realm() (above) and with what the code
actually does.  Since the only reference (in the code or documentation) to a
function called get_local_realm() says that it returns a list of local realms
rather (which one would parse_name() use?) that does rather indicate that the
documentation should say "default" rather than "local".

Given all this, I  original fix still stands (and also applies to Kerberos 1.3,
slightly re-worked) and I would be amazed if it was rejected upstream.
Comment 11 Nalin Dahyabhai 2006-01-26 16:45:21 EST
My apologies for not being more verbose in comment 9. I'm agreeing with you.
Cloning for tracking against other releases.
Comment 23 Red Hat Bugzilla 2006-07-20 11:00:15 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2006-0404.html

Note You need to log in before you can comment on or make changes to this bug.