We found today that if you have credentials like "MUSIC\PROCAL HAREM", users with these credentials can authenticate via NTLM but not KRB5. This is because pull_utf8_talloc in smb_krb5_unparse_name converts the above to "MUSIC\PROCAL\ HAREM". Since Windows seems to have no problems with such names, we should be able to deal with them. I have a patch but it is a bit ugly.
The implementation of pull_utf8_talloc passes CH_UNIX. Is there some other symbol we can use that will not escape spaces, or do we just have to post process the string? If so, here is a patch that does the job, but looks ugly: --- samba-3.6.6/source3/libsmb/clikrb5.c 2012-06-24 10:21:16.000000000 -0700 +++ clikrb5.c 2013-05-09 01:52:06.861677204 -0700 @@ -113,6 +113,7 @@ krb5_error_code ret; char *utf8_name; size_t converted_size; + int i = 0; *unix_name = NULL; ret = krb5_unparse_name(context, principal, &utf8_name); @@ -124,6 +125,17 @@ krb5_free_unparsed_name(context, utf8_name); return ENOMEM; } + for (i = 0; i < converted_size; i++) { + if ((*unix_name)[i] == '\\') + if (((i + 1) < converted_size) && + ((*unix_name)[i+1] == ' ')) { + /* get rid of the slash */ + memcpy(&((*unix_name)[i]),&((*unix_name)[i+1]), + converted_size - i - 2); + converted_size--; + } + + } krb5_free_unparsed_name(context, utf8_name); return 0; }
A simple iconv shouldn't escape spaces. Let me take a look to see if there's some Samba code that is doing this. Jeremy.
What is inserting the escape sequences into the string ? In this function: krb5_error_code smb_krb5_unparse_name(TALLOC_CTX *mem_ctx, krb5_context context, krb5_const_principal principal, char **unix_name) { krb5_error_code ret; char *utf8_name; size_t converted_size; *unix_name = NULL; ret = krb5_unparse_name(context, principal, &utf8_name); if (ret) { return ret; } Are the '\' escape sequences in utf8_name after krb5_unparse_name() is called ? if (!pull_utf8_talloc(mem_ctx, unix_name, utf8_name, &converted_size)) { krb5_free_unparsed_name(context, utf8_name); return ENOMEM; } or added inside pull_utf8_talloc() ?
OK, looked through the call chain, and we should be ending up in a call to the system iconv(). The system iconv() should *never* be escaping anything. We need to know where these escape characters are being added. If you think it's inside pull_utf8_talloc() then you need to set a breakpoint before and after your system iconv() call and if that is escaping characters then you need a working iconv :-). Jeremy.
Follow-up. Looks like a Heimdal bug. From the MIT krb5 source code, function copy_component_quoting(): case COMPONENT_SEP: case '\\': *q++ = '\\'; *q++ = *cp; break; case '\t': *q++ = '\\'; *q++ = 't'; break; case '\n': *q++ = '\\'; *q++ = 'n'; break; case '\b': *q++ = '\\'; *q++ = 'b'; break; #if 0 /* Heimdal escapes spaces in principal names upon unparsing */ case ' ': *q++ = '\\'; *q++ = ' '; break; #endif
So inside smb_krb5_unparse_name() after the call to krb5_unparse_name() add a call: string_sub(utf8_name, "\\ ", " ", 0); before the pull_utf8_talloc() call. That should fix it. Jeremy.
(In reply to comment #6) > So inside smb_krb5_unparse_name() after the call to > krb5_unparse_name() add a call: > > string_sub(utf8_name, "\\ ", " ", 0); > > before the pull_utf8_talloc() call. > > That should fix it. > > Jeremy. Testing now.
(In reply to comment #6) > So inside smb_krb5_unparse_name() after the call to > krb5_unparse_name() add a call: > > string_sub(utf8_name, "\\ ", " ", 0); > > before the pull_utf8_talloc() call. > > That should fix it. This works.
Ok can you post it as a git format-patch patch to samba-technical and I'll push it. Then we'll get it into 3.6.next and 4.0.next. Cheers, Jeremy.
Actually, looking at master there are several places that use the raw krb5_unparse_name() when they should be using the wrapper (IMHO). We'll need to fix all these. Jeremy.
(In reply to comment #10) > Actually, looking at master there are several places that use the raw > krb5_unparse_name() when they should be using the wrapper (IMHO). > > We'll need to fix all these. Sure, I'll generate a patch for them.
Hmmm, do we need to do this: diff --git a/auth/credentials/credentials_krb5.c b/auth/credentials/credentials_ index cc51f56..62e8e16 100644 --- a/auth/credentials/credentials_krb5.c +++ b/auth/credentials/credentials_krb5.c @@ -101,7 +101,7 @@ static int cli_credentials_set_from_ccache(struct cli_creden return ret; } - ret = krb5_unparse_name(ccache->smb_krb5_context->krb5_context, princ, & + ret = smb_krb5_unparse_name(cred, ccache->smb_krb5_context->krb5_context if (ret) { (*error_string) = talloc_asprintf(cred, "failed to unparse princ smb_get_krb5_error_message(cca @@ -111,7 +111,7 @@ static int cli_credentials_set_from_ccache(struct cli_creden cli_credentials_set_principal(cred, name, obtained); - free(name); + talloc_free(name); krb5_free_principal(ccache->smb_krb5_context->krb5_context, princ);
Created attachment 8878 [details] A patch that 'fixes' two instances of naked krb5_unparse_name calls and handles escaped space This is an initial patch to see if I am on the right path here.
There also seems to be no wrapper for krb5_unparse_name_flags.
Hmmm, It might be better to use krb5_unparse_name_flags and pass KRB5_PRINCIPAL_UNPARSE_DISPLAY as a flag. This does not quote special chars. The questions are: 1. Does Heimdal support this. That is easy to check. It looks like they support krb5_unparse_name_flags, but I am not yet sure whether they support that flag. 2. Do any of the other special chars cause problems with Unix account lookups.
I agree that krb5_unparse_name_flags is probably the right approach, but in a different way. In master, we don't actually use this a lot, except for debugging, and for those the display flag certainly makes sense. Other flags that would be worth using are for example the NO_REALM flag, and then extract the realm directly. That would remove any confusion about an @ that somehow ends up in a username. Doing this at the pain point in 3.6 and master might help get past the immediate issue. http://technet.microsoft.com/en-us/library/bb726984.aspx indicates that @ would be valid within a username! I need to think carefully about the implications of using smb_unparse_name in the credentials code, as we have to think carefully about the character set we are coming from and going to. The code used here (like the rest of the AD DC) blindly assumes that everything is UTF8, and we can't simply assert that 'kerberos is UTF8, but unix might be different', because userspace kerberos tools would operate them in 'unix' charset. It is also the case that we will pass the princpal string back to kerberos, calling krb5_parse_name on it. These issues come from the fact that "MIT" (ie traditional) kerberos treats these things as a bag of bytes, while AD they are a case-insensitive UTF8 string. Essentially I'm saying 'here be dragons', but I'm happy to keep working with you on this, to try and define some clearer lines.