We are one of the few that need to preserve case in kerberos realm names. The following seems to make sense - administrators ought to use the proper case when configuring a realm name. diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 579f847..f5fd985 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -147,6 +147,8 @@ struct global { char *szPasswordServer; char *szSocketOptions; char *szRealm; + char *szEffectiveRealm; + bool *bRealmPreserveCase; char *szAfsUsernameMap; int iAfsTokenLifetime; char *szLogNtTokenCommand; @@ -995,13 +997,22 @@ static struct parm_struct parm_table[] = { #ifdef WITH_ADS { .label = "realm", - .type = P_USTRING, + .type = P_STRING, .p_class = P_GLOBAL, - .ptr = &Globals.szRealm, - .special = NULL, + .ptr = &Globals.szEffectiveRealm, + .special = handle_realm, .enum_list = NULL, .flags = FLAG_BASIC | FLAG_ADVANCED | FLAG_WIZARD, }, + { + .label = "realm preserve case", + .type = P_BOOL, + .p_class = P_GLOBAL, + .ptr = &Globals.bRealmPreserveCase, + .special = handle_realm_preserve_case, + .enum_list = NULL, + .flags = FLAG_BASIC | FLAG_ADVANCED, + }, #endif { .label = "netbios name", @@ -5351,7 +5362,7 @@ FN_GLOBAL_STRING(lp_passwd_program, &Globals.szPasswdProgram) FN_GLOBAL_STRING(lp_passwd_chat, &Globals.szPasswdChat) FN_GLOBAL_STRING(lp_passwordserver, &Globals.szPasswordServer) FN_GLOBAL_STRING(lp_name_resolve_order, &Globals.szNameResolveOrder) -FN_GLOBAL_STRING(lp_realm, &Globals.szRealm) +FN_GLOBAL_STRING(lp_realm, &Globals.szEffectiveRealm) FN_GLOBAL_CONST_STRING(lp_afs_username_map, &Globals.szAfsUsernameMap) FN_GLOBAL_INTEGER(lp_afs_token_lifetime, &Globals.iAfsTokenLifetime) FN_GLOBAL_STRING(lp_log_nt_token_command, &Globals.szLogNtTokenCommand) @@ -7228,6 +7239,25 @@ static bool handle_workgroup(int snum, const char *pszParmValue, char **ptr) return ret; } +static bool handle_realm(int snum, const char *pszParmValue, char **ptr) +{ + string_set(ptr, pszParmValue); + string_set(&Globals.szRealm, pszParmValue); + if (!Globals.bRealmPreserveCase) + strupper_m(*ptr); + + return True; +} + +static bool handle_realm_preserve_case(int snum, const char *pszParmValue, char **ptr) +{ + string_set(&Globals.szEffectiveRealm, Globals.szRealm); + if (!(*(bool *)ptr = lp_bool(pszParmValue))) + strupper_m(Globals.szEffectiveRealm); + + return True; +} + static bool handle_netbios_scope(int snum, const char *pszParmValue, char **ptr) { bool ret;
Created attachment 5619 [details] Here's a fix to preserve realm case
Why don't you replace lp_realm() with a call that looks at "realm preserve case", doing the strupper on demand? Seems simpler to me. Volker
You'd rather strupper a value every time you need it than do it once? It looks like the other lp_ variables point to memory, not a function, so that's the way I did it. It's also close to the way samba4 uses two pointers for szRealm_lower and szRealm_upper. Or, are you suggesting that you'd replace every call to lp_realm?
I would suggest to take away the FN_GLOBAL_STRING(lp_realm, &Globals.szRealm) definition an replace it with a function of the prototype char *lp_realm(void) { /* new code */ } that does the new magic. If you are concerned about the performance, you might also introduce a new parameter "case insensitive realm" that takes precedence over the "realm" parameter when set. "case insensitive realm" would then be a P_STRING instead of a P_USTRING. Your definition of the lp_realm function would then look if "case insensitive realm" is defined. If so, then it would return that value. If it is not defined (the default), then it would return the value of the "realm" parameter. BTW, please also add a smb.conf manual page entry of any new parameter you choose. You might want to look at docs-xml/smbdotconf for templates. Thanks, Volker
Created attachment 5632 [details] Second try Here's another shot at it. Thanks for the guidance.
Comment on attachment 5632 [details] Second try This patch is the right approach, but it misses an important point: The existing definition of lp_realm goes through the function lp_string, which the new patch drops. lp_string does the %-Macro substitutions. It might be arguable if % macro substitutions are required for the realm parameter, but for compatibility reasons I would argue that this is necessary. If you want this patch in this form, you might want to start the pulic discussion about this issue on samba-technical@samba.org. Maybe I am just wrong with my 100% compatibility argument in this case.
Created attachment 5635 [details] using lp_string Thanks for looking this over, that was an oversight on my part. Here's the same approach including lp_string.
Re-assigning to Volker for patch review.
(In reply to Volker Lendecke from comment #6) In the meantime lp_realm() has become a 'constant' string parameter and no longer goes via lp_string(). The generation of the loadparm system makes this hard to find, but it is declared in docs-xml/smbdotconf/base/realm.xml via 'constant=1' and is visible in the generated bin/default/lib/param/param_functions.c.
(In reply to Andrew Bartlett from comment #9) > (In reply to Volker Lendecke from comment #6) > In the meantime lp_realm() has become a 'constant' string parameter and no > longer goes via lp_string(). The generation of the loadparm system makes > this hard to find, but it is declared in docs-xml/smbdotconf/base/realm.xml > via 'constant=1' and is visible in the generated > bin/default/lib/param/param_functions.c. As far as I can see, the "realm" parameter is now handled via the handle_realm() function where the strupper() happens unconditionally. Question to the authentication developers: Do we still need/want this parameter? If not, what shall we do with this bug report?
This really feels like a local patch to me. So much in the rest of Samba has come to assume the force-uppercase that while your particular use case might work, things like the AD DC are more likely to become unhappy (I've not checked). As force-uppercase is well entrenched in the AD DC, I think testing this (and untested code is broken code) would be quite difficult. Sorry!
I would note that the thread at http://marc.info/?t=126772172500006&r=1&w=2 linked in the URL is quite insightful and useful context.
Andrew, handing this over to you. I think you have much more to say about all of this. Thanks for taking care!
Benjamin, I'm going to, regretfully, close this as WONTFIX as we can't afford to carry and test the additional complexity in our authentication code. Sorry, Andrew Bartlett