Use Implicit User Principal Name If Explicit Does Not Exist #332
Conversation
NoMoreFood
commented
Jul 18, 2018
NoMoreFood
commented
- Modified user principal name lookup to default to the implicit form (SamAccountName@DnsDomainName) if no explicit user principal name attribute is found on the account.
- Modified user principal name lookup to default to the implicit form (SamAccountName@DnsDomainName) if no explicit user principal name attribute is found on the account.
| wcscat_s(user_principal_name, MAX_UPN_LEN + 1, L"@"); | ||
| wcsncat_s(user_principal_name, MAX_UPN_LEN + 1, domain_upn, wcschr(domain_upn, L'/') - domain_upn); | ||
| debug3("%s: Successfully discovered implicit principal name: '%ls'=>'%ls'", | ||
| __FUNCTION__, sam_account_name, user_principal_name); |
There was a problem hiding this comment.
help me understand. How would the NameCanonical name be different from NameSamCompatible name ? As per msdn NameCanonical can have multiple '/'s.
There was a problem hiding this comment.
We're only using the fully qualified domain name part of NameCanonical (i.e. domain.local/other/path/stuff).
There was a problem hiding this comment.
Added some additional explanation to the code as well.
| lookup_error = GetLastError(); | ||
| domain_upn_len = ARRAYSIZE(domain_upn); | ||
| if (pTranslateNameW(sam_account_name, NameSamCompatible, NameCanonical, domain_upn, &domain_upn_len) != 0) { | ||
| wcscpy_s(user_principal_name, MAX_UPN_LEN + 1, wcschr(sam_account_name, L'\\') + 1); |
There was a problem hiding this comment.
Initialize user_principal_name to "\0".
There was a problem hiding this comment.
Not sure the intent here. String copy functions do not need an initialized string since they start copying immediately into the passed pointer without looking at what data might be there.
| /* report error and copy the sam account name into the buffer */ | ||
| error("%s: User principal name lookup failed for user '%ls' (explicit: %d, implicit: %d)", | ||
| __FUNCTION__, sam_account_name, lookup_error, GetLastError()); | ||
| wcscpy_s(user_principal_name, MAX_UPN_LEN + 1, sam_account_name); |
There was a problem hiding this comment.
remove this.. it will be clean to assume that nothing gets set on the failure path
| else | ||
| debug3("%s: Successfully discovered principal name: '%ls'=>'%ls'", __FUNCTION__, user_cpn, domain_upn); | ||
|
|
||
| lookup_principal_name(user_cpn, domain_upn); |
There was a problem hiding this comment.
handle failure (copy user_cpn to domain_upn on failure)
| @@ -715,4 +706,37 @@ get_custom_lsa_package() | |||
| return s_lsa_auth_pkg; | |||
| } | |||
|
|
|||
| int lookup_principal_name(const wchar_t * sam_account_name, wchar_t * user_principal_name) | |||
There was a problem hiding this comment.
please add a brief summary on what this routine does.
…ns 1 - Moved lookup_principal_name() fallback behavior out of the function and into the caller. - Added additional comments.
