DRIVERS-3329: Configurable DNS domain validation for SRV records

[sleepyStick]

Please complete the following before merging:

• Is the relevant DRIVERS ticket in the PR title?

• Update changelog.
• Test changes in at least one language driver.
  Python PR: PYTHON-5814 Configurable DNS domain validation for SRV records mongo-python-driver#2868
• Test these changes against all server versions and topologies (including standalone, replica set, and sharded
  clusters).

[rozza]

It looks good, I have one comment about if an invalid srvAllowedHostsSuffix should throw an error explicitly? (if so should we update the yml?)

Also the yml and json has diverged for srvAllowedHostsSuffix-without_dot_pass - which is probably the lint error.

[rozza]

Just to clarify, if a host is invalid eg: ..example.com should this error or just return an empty list of hosts?

[sleepyStick]

oh good question, my current implementation has it eventually erroring (with a configuration error) when the driver goes to validate the SRV hosts -- i didn't add a new "check valid host" type of function. (though this probably wouldn't be too hard with some regex? if you think it'd be beneficial to add something like this.) My initial thought process was this new parameter is just a user configurable way to denote what the domain should be (as opposed to the previous, now default, logic) -- they should know what they're doing if they're using this param and if they happened to make a typo / silly mistake, the existing "invalid SRV host" error message that occurs when there is an invalid host given the previous logic would still apply.

[rozza]

I think the eventually error part will cover this, so no need for a regex. I can never understand regexes and someone always seems to find a usecase where the regex fails where it shouldn't!

[matthewdale]

Optional: Consider linking to the Querying DNS section, which is where the DNS validation is described.

[sleepyStick]

added! thanks!

[codeowners-service-app]

Assigned vector-of-bool for team dbx-spec-maintainers-connection-string because aclark4life is out of office.

[matthewdale]

Looks good! 👍

[aclark4life]

Looks like the convention in yml is no quotes around uri

[aclark4life]

Missing space

[aclark4life]

Also a convention apparently is new lines at the end of yaml files.

[aclark4life]

Pending nit fixes LGTM

[ajcvickers]

Setting srvAllowedHostsSuffix to a public suffix neuters SRV anti-spoofing, turning mongodb+srv:// into an unbounded DNS redirect. A merely broad private domain doesn't make it unbounded, but widens the redirect to anywhere under that domain. There is no PSL/minimum-label guard to prevent either. Example:

Preconditions:

• Misconfiguration. App sets a too-broad suffix:
  mongodb+srv://mycluster.mongodb.example.com/?srvAllowedHostsSuffix=.com
• Attacker runs a malicious/compromised resolver, or poisons the cache. (DNSSEC is not enforced per SECURITY-488).
• Attacker obtains a CA-valid cert for a domain they own.

Step-by-step:

• Client SRV-queries _mongodb._tcp.mycluster.mongodb.example.com.
• Attacker forges the SRV response: target db1.attacker-evil.com:27017.
• Suffix validation passes (db1.attacker-evil.com ends in .com).
• mongodb+srv forces TLS; the client validates the cert against the SRV-returned name (db1.attacker-evil.com), not the typed name (mycluster.mongodb.example.com).
• Attacker owns that name and holds a valid cert → TLS verification passes → client connects to the attacker's server.

[ajcvickers]

I think the parsing needs to be much better specified and tested. It's important to understand what DNS entries allow and how to safely handle these things. These should be specified at the spec level, so all drivers will get the parsing correct. For example, most of these are not specified or tested:

• DNS is case-insensitive. (Also, ASCII-only case-folding.)
• SRV targets often return trailing dots. For example: host.citi.net. These must be normalized.
• IDN / punycode / homographs. Don't compare raw Unicode.
• Probably more... (I'm not an expert, but the above are the obvious ones.)

There needs to be much better test coverage here.

[ajcvickers]

• When the suffix is set, does the "fewer than three parts" means "at least one more domain level" still apply?
• SRV polling reuses the same domain-matching and is not touched by this PR, nor mentioned in the decision doc. This seems fundamentally broken.
• How does setting the new option Interact with srvMaxHosts or a custom srvServiceName?

[ajcvickers]

From going through the history here, it seems we have acknowledged the security concerns but consciously accepted them and shifted the risk to the customer to meet customer demand. This is a weak security posture: it puts a lot of responsibility on the customer to get everything right, and rejects the safer alternatives (CNAMEs, RFC 4985 SANs).

If we are going to do this, then the risks I have highlighted need to be made front and center so that anyone opting into this is aware that it relaxes the naming-layer anti-spoofing defense and considerably increases their attack surface.

Beyond this, I think there needs to be a product-level guard against the worst misconfigurations (e.g. rejecting public-suffix values) and a fully specified safe-configuration recipe, so "the customer's responsibility" is achievable rather than a trap.

[sleepyStick]

Setting srvAllowedHostsSuffix to a public suffix neuters SRV anti-spoofing, turning mongodb+srv:// into an unbounded DNS redirect. A merely broad private domain doesn't make it unbounded, but widens the redirect to anywhere under that domain. There is no PSL/minimum-label guard to prevent either.

We're exploring some additional safeguards with respect to disallowing public suffixes but short of having a decision on that at the moment, I've added that " [srvAllowedHostsSuffix] MUST contain at least two dot-separated labels (i.e. the portion after the leading . must itself contain a .). For example, srvAllowedHostsSuffix=.net MUST raise an error."

I think the parsing needs to be much better specified and tested. It's important to understand what DNS entries allow and how to safely handle these things. These should be specified at the spec level, so all drivers will get the parsing correct. For example, most of these are not specified or tested:

• DNS is case-insensitive. (Also, ASCII-only case-folding.)
• SRV targets often return trailing dots. For example: host.citi.net. These must be normalized.
• IDN / punycode / homographs. Don't compare raw Unicode.
• Probably more... (I'm not an expert, but the above are the obvious ones.)

There needs to be much better test coverage here.

Great call out -- I definitely overlooked this for the spec. I relied on existing logic to handle the case insensitivity and trailing dots and overlooked adding the specifics of that to the specification. I've gone ahead and added a test for both these cases.
question about IDN / punycode / homographs -- even if a user passed in a non-ascii only value for srvAllowedHostsSuffix, wouldn't it not pass the check because DNS query would only return ascii values?

• When the suffix is set, does the "fewer than three parts" means "at least one more domain level" still apply?

The newly added two label minimum on srvAllowedHostsSuffix should satasify this concern therefore its implicitly applicable.

• SRV polling reuses the same domain-matching and is not touched by this PR, nor mentioned in the decision doc. This seems fundamentally broken.

ahh good point -- i had assumed the logic would be reused (because that's what pymongo does) -- this parameter should also be used in SRV polling and I've updated the parameter description accordingly to reflect this.

• How does setting the new option Interact with srvMaxHosts or a custom srvServiceName?

I don't believe there is any new interactions? Their logic is fairly separated and distinct to me? Am i missing something?

[ajcvickers]

We're exploring some additional safeguards with respect to disallowing public suffixes but short of having a decision on that at the moment, I've added that " [srvAllowedHostsSuffix] MUST contain at least two dot-separated labels (i.e. the portion after the leading . must itself contain a .). For example, srvAllowedHostsSuffix=.net MUST raise an error."

This doesn't really help very much. Consider .co.uk.

@damieng Suggested a different approach here. Rather than us trying to handle every case safely, we could instead provide a hook so that the customer could handle their specific case. It's much easier to handle very specific cases safely, because you are matching actual specific domains, rather than attempting to match a pattern. We could write the safe implementation ourselves and give it to important customers.

question about IDN / punycode / homographs -- even if a user passed in a non-ascii only value for srvAllowedHostsSuffix not pass the check because DNS query would only return ascii values?

I don't know. I'm not an expert on this type of parsing, or on what DNS actually does with non-Ascii characters. Do we have any domain experts in DNS that we can ask?

I don't believe there is any new interactions? Their logic is fairly separated and distinct to me? Am i missing something?

Sounds good.

[aclark4life]

question about IDN / punycode / homographs -- even if a user passed in a non-ascii only value for srvAllowedHostsSuffix not pass the check because DNS query would only return ascii values?

I don't know. I'm not an expert on this type of parsing, or on what DNS actually does with non-Ascii characters. Do we have any domain experts in DNS that we can ask?

DNS does not support non-ascii chars e.g.

mongo-python-driver|PYTHON-5867 ⇒ whois °.co.uk | tail -20

status:       ACTIVE
remarks:      Registration information: http://www.nic.uk/

created:      1985-07-24
changed:      2023-03-07
source:       IANA

# whois.nic.uk


    Error for "â°.co.uk".

    This domain cannot be registered because it contravenes the Nominet UK
    naming rules.  The reason is:
      Domain names may only comprise the characters A-Z, a-z, 0-9, hyphen (-)
    and dot (.)..

    WHOIS lookup made at 13:35:57 06-Jul-2026

[sleepyStick]

tldr: i don't think the hook is a better idea, the current approach achieves the same thing. but for more details:

We're exploring some additional safeguards with respect to disallowing public suffixes but short of having a decision on that at the moment, I've added that " [srvAllowedHostsSuffix] MUST contain at least two dot-separated labels (i.e. the portion after the leading . must itself contain a .). For example, srvAllowedHostsSuffix=.net MUST raise an error."

This doesn't really help very much. Consider .co.uk.

Agreed that the current one wouldn't help and that more is needed. I believe we're still contemplating the best way to "manage" a public suffix list across the drivers but the goal is to address this concern.

@damieng Suggested a different approach here. Rather than us trying to handle every case safely, we could instead provide a hook so that the customer could handle their specific case. It's much easier to handle very specific cases safely, because you are matching actual specific domains, rather than attempting to match a pattern. We could write the safe implementation ourselves and give it to important customers.

I messaged @damieng this morning after i read your reply (mainly because i didn't understand what the other approach was and was hoping for a clearer explanation) -- it appears that Damien didn't have the full context of the problem when he came up with this suggestion but I'll clarify on what this suggestion is (for anyone else reading) and then after share my thoughts on it for further discussion.
What the suggestion is: the aforementioned hook that would be provided to the customer would be like a function. So for example the code might look like this:

def is_allow_host(...) -> bool:
     # whatever the current validation is

and then cusomers can do the following:

@override
def is_allow_host(host: str, ...) -> bool:
     return host.endswith("<their allow host suffix>"))

notably this idea wouldn't involve adding srvAllowedHostsSuffix as a URI parameter. Thus the inferred host would be the existing logic.
my thoughts: okay so my first thought was, what's stopping the customer from doing "return true" in their overridden is_allow_host method? that would obviously be a silly thing for them to do but its basically the same thing as not adding any form of validation for the existing srvAllowedHostsSuffix, so my evolution of this idea is to basically always do some form of basic validation on the host (which i think would be the same type of validation that we need to perform on srvAllowedHostsSuffix if we were to go the URI approach) and thus the code would look like this:

def is_allow_host_hook(...) -> bool:
     # this is where we'd place "optional" / "recommended" validation that the customer can then override if they decide they know better

def is_allow_host(...) -> bool:
     # whatever validation we deem to be an absolute minimum safeguard that all allowed hosts should satisfy 
     # AND if those pass then
     return is_allow_host_hook(...)

and then customers can do the following:

@override
def is_allow_host_hook(host: str, ...) -> bool:
     return host.endswith("<their allow host suffix>")) # or whatever they want

But at this point, all that absolute minimum validation is the same as the validation that we need to figure out for srvAllowedHostsSuffix + the potential code architecture changes to allow said hook to exist for customers makes me feel like its not really a great alternative idea. Additionally, i feel like a URI option is more likely to be visible in logs making it easier to debug whereas this hook would be harder to, should a mistake be made somewhere?
Additionally, I presently don't know what I'd want to go into the is_allow_host_hook (from my evolution of the idea) -- this isn't to say we wouldn't think of something in the future? -- but if this function remains a no-op forever (aka return True) then it doesn't address the root cause of the entire project since it'd only allow customers to add more restrictive requirements to the host and not make it less restrictive?
Now, Damien did point out that this override would allow customers to more easily have different allow hosts depending on their various environments / configs but I'm personally not convinced how useful they'll be given the, what i believe would be, increased complexity of the code change? (I could probably be convinced otherwise tho? sometimes i'm pretty easy to convince lol)
But if I'm misunderstanding the suggestion, or otherwise failing to see a pro of this suggestion, please do let me know! (I am but a lowly code monkey after all haha)

[damieng]

what's stopping the customer from doing "return true" i

Nothing, but they would have explicitly chosen to have relaxed that security. It gives them the ultimate flexibility and power but only if they clearly opt into it.

The problem with doing things like srvAllowedHostsSuffix is it's very limited. The moment a customer wants to say - use two hosts under different suffixes perhaps on different cloud providers they're out of luck again. With a hook they can either hard-code it, read a comma-separated list in from their own configuration etc.

If the user wants to also perform our existing validation that's easy enough to do by them calling down to the base method/hook either before adding their own validation or after having allowed through their exception.