-
Notifications
You must be signed in to change notification settings - Fork 292
CA-399256: Ensure AD domain name check is case insensitive #6047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CA-399256: Ensure AD domain name check is case insensitive #6047
Conversation
No need to close/reopen the PR, you can just mark it as a draft while working things out. Here in particular you need to run |
fec9f97
to
bf7d76d
Compare
Signed-off-by: Elijah Sadorra <[email protected]>
bf7d76d
to
aa09cb8
Compare
@@ -322,7 +322,8 @@ let pre_join_checks ~__context ~rpc ~session_id ~force = | |||
slavetobe_auth_type slavetobe_auth_service_name ; | |||
if | |||
slavetobe_auth_type <> master_auth_type | |||
|| slavetobe_auth_service_name <> master_auth_service_name | |||
|| String.lowercase_ascii slavetobe_auth_service_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight simplification:
String.(lowercase_ascii slavetobe_auth_service_name <> lowercase_ascii master_auth_service_name)
but fine as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in person, this does not work for UTF8 but it still covers the common case to get over the hurdle that someone is using not all lowercase to begin with. We need to be sure that it is indeed correct to compare these names as lowercase such that foo
and FOO
are indeed semantically equal and that we don't make names equal that in fact designate different entities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading Microsoft specifications domain names come from Netbios names and/or DNS names which are both defined in terms of ASCII. See https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou although you need to refer also to Netbios/DNS specifications. For DNS names "All characters preserve their case formatting except for ASCII characters.".
No description provided.