Bug 117214 - [PATCH] krb5_parse_name() doesn't track changes to default realm
 Summary: [PATCH] krb5_parse_name() doesn't track changes to default realm
 Status: Product: CLOSED ERRATA Aliases: None Red Hat Enterprise Linux 3 Red Hat krb5 (Show other bugs) --- 3.0 All Linux medium Severity medium --- Target Release: --- Nalin Dahyabhai Brian Brock RHEL3U8MustFix Show dependency tree / graph

 Reported: 2004-03-01 12:53 EST by John Haxby 2007-11-30 17:07 EST (History) 2 users (show) pgraner tao RHBA-2006-0404 Bug Fix --- 2006-07-20 11:00:15 EDT --- --- --- --- --- --- ---

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

External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2006:0404 normal SHIPPED_LIVE krb5 bug fix update 2006-07-19 15:10:00 EDT

None (edit)
 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 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. 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. 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. 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! 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. Nalin Dahyabhai 2006-01-23 15:15:12 EST Oops, the docs use "local" and "default" to refer to the same realm. 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. 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. 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.