Handle RFC 6761 special-use domain names in Dns resolution #123076
Conversation
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/ncl |
There was a problem hiding this comment.
Pull request overview
This PR implements RFC 6761 compliant handling for special-use domain names in System.Net.Dns. The implementation adds logic to intercept DNS queries for invalid/*.invalid domains (returning NXDOMAIN) and *.localhost subdomains (returning loopback addresses) before they reach the OS resolver. Plain localhost continues to use the OS resolver to preserve /etc/hosts customizations.
Changes:
- Added RFC 6761 special-use domain name handling to DNS resolution with proper telemetry integration
- Implemented helper methods for efficient string matching and loopback address allocation
- Added comprehensive test coverage for invalid domains, localhost subdomains, and address family filtering
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs | Added helper methods (IsReservedName, GetLoopbackAddresses, TryHandleRfc6761SpecialName) and integrated RFC 6761 handling into both sync and async resolution paths |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostEntryTest.cs | Added tests for invalid domain rejection, localhost subdomain resolution, address family filtering, and edge cases |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostAddressesTest.cs | Added parallel tests for GetHostAddresses API covering invalid domains and localhost subdomains |
| // Matches "reservedName" exactly, or "*.reservedName" (subdomain) | ||
| return hostName.EndsWith(reservedName, StringComparison.OrdinalIgnoreCase) && | ||
| (hostName.Length == reservedName.Length || | ||
| hostName[hostName.Length - reservedName.Length - 1] == '.'); |
There was a problem hiding this comment.
The IsReservedName function has a bug with edge case hostnames starting with a dot. For example, ".localhost" or ".invalid" would incorrectly match because the function checks if the character before the reserved name is a dot, but doesn't verify there's actual content before that dot. This would cause ".localhost" to be treated as a reserved name when it shouldn't be.
To fix this, add a check to ensure the dot is not at position 0. The condition should be:
hostName[hostName.Length - reservedName.Length - 1] == '.' && hostName.Length > reservedName.Length + 1
Or alternatively, check that the position of the dot is greater than 0:
(hostName.Length - reservedName.Length - 1) > 0 && hostName[hostName.Length - reservedName.Length - 1] == '.'
| hostName[hostName.Length - reservedName.Length - 1] == '.'); | |
| (hostName.Length > reservedName.Length + 1 && | |
| hostName[hostName.Length - reservedName.Length - 1] == '.')); |
There was a problem hiding this comment.
hmm, this brings up more questions. We probably don't want to return loopback address for cases like:
.localhost
.something.localhost
something..localhost
so.me..thing.localhost
as they are all malformed, but validating that a string is a valid hostname is nontrivial (label lengths, punycode....) and we don't have that implemented anywhere IIRC. @wfurt any thoughts here?
There was a problem hiding this comment.
I'll add validation to reject malformed hostnames (those starting with ., containing .., or ending with .) and let them fall through to the OS resolver.
There was a problem hiding this comment.
or ending with .
Ending with . is valid (i.e. localhost and localhost. are equivalent) the empty label at the end is the root label which does not need to be explicitly stated in the API (but is always part of the encoded query), but that is not necessarily what I meant. I am not sure we should implement partial hostname validation.
If any, we should implement complete hostname validation with all it's edgecases (label length restrictions, total length restriction, punycode encoding, etc.), which is complicated and I suggest you wait a bit while we discuss in the team whether that is something that we want.
There was a problem hiding this comment.
We've discussed this in the team, and have consensus on the following
- we want to preserve the existing behavior for folks who are using /etc/hosts.conf to redirect localhost subdomains to specific IPs. So 'attempting to resolve those subdomains in the native resolver first, and if resolves to empty set of addresses, then falling back to whatever "localhost" resolves to' seems a decent middle ground.
- there is no need to implement exhaustive hostname validation, rejecting just obviously malformed names should be enough.
There was a problem hiding this comment.
Done! The implementation now rejects obviously malformed hostnames (starting with . or containing ..) and passes them to the OS resolver, which will reject them appropriately. I'm not doing exhaustive hostname validation as discussed.
| } | ||
|
|
||
| // RFC 6761 Section 6.3: "*.localhost" subdomains must resolve to loopback. | ||
| // Note: Plain "localhost" is handled by the OS resolver (preserves /etc/hosts customizations). |
There was a problem hiding this comment.
It feels weird to allow customizations for "localhost" name itself, but not for subdomains.
It also means that without any customization, names "localhost" and "a.localhost" may return different results (e.g. IPv4 and IPv6 addresses in different order)
There was a problem hiding this comment.
You're right that this creates an inconsistency where localhost and a.localhost may return different results.
The current approach defers plain \localhost\ to the OS resolver intentionally — handling it in code would be a breaking change for users with custom hosts file entries (e.g., mapping localhost to a specific IP).
Handling both identically would be more RFC 6761 compliant and provide consistent behavior, but at the cost of backward compatibility.
My preference would be to avoid the breaking change, but I'm not sure what the general practice is in dotnet for situations like this. Is there guidance on when RFC compliance should take precedence over backward compatibility?
There was a problem hiding this comment.
what if we strip the subdomains and return the same result as simply querying "localhost"? that would preserve the existing behavior and in majority of the cases the subdomains would be resolved to loopback addresses anyway (and we no longer need to care about whether IPv4 or IPv6 is enabled as that would be handled by the native layer)
There was a problem hiding this comment.
Done - this is exactly what the updated implementation does.
Implement RFC 6761 compliant handling for special-use domain names: - 'invalid' and '*.invalid' domains return SocketError.HostNotFound (NXDOMAIN) - '*.localhost' subdomains resolve to loopback addresses (127.0.0.1 and/or ::1) - Plain 'localhost' continues to use OS resolver to preserve /etc/hosts customizations Implementation details: - Pre-allocated loopback address arrays to avoid allocations on hot path - Respects AddressFamily parameter for filtered results - Includes telemetry for diagnostics consistency - Case-insensitive matching per RFC specification Added tests for invalid domains, localhost subdomains, AddressFamily filtering, and edge cases like 'notlocalhost' to ensure similar names aren't incorrectly matched.
- Fix IPv6 address order: use [IPv6Loopback, Loopback] to match Windows resolver behavior - Add validation to reject malformed hostnames (starting with '.' or containing '..') in IsReservedName, letting them fall through to OS resolver - Add tests for malformed hostname edge cases (.localhost, foo..localhost, .invalid, test..invalid)
laveeshb
force-pushed
the
fix/rfc6761-special-domains
branch
from
84b6b50 to
8fbc05e
Compare
- RFC 6761 Section 6.4: 'invalid' and '*.invalid' domains immediately return NXDOMAIN - RFC 6761 Section 6.3: '*.localhost' subdomains try OS resolver first, fall back to plain 'localhost' resolution if OS returns failure or empty results - Handles trailing dots (DNS root notation) correctly - Rejects malformed hostnames (starting with '.' or containing '..') by passing to OS resolver - Comprehensive test coverage for all edge cases
|
Thanks for the feedback @rzikm! I've updated the implementation based on your suggestion: Changes made:
Benefits of this approach:
The \invalid\ domain handling remains unchanged - those immediately return NXDOMAIN without hitting the OS resolver, as per RFC 6761 Section 6.4. All 133 tests pass, including new tests that verify localhost subdomains return the same addresses as plain \localhost. |
|
Correction to formatting above: Thanks for the feedback @rzikm! I've updated the implementation based on your suggestion: Changes made:
Benefits of this approach:
The All 133 tests pass, including new tests that verify localhost subdomains return the same addresses as plain |
| bool isLocalhostSubdomain = IsLocalhostSubdomain(hostName); | ||
| Task? t; | ||
| if (NameResolutionTelemetry.AnyDiagnosticsEnabled()) | ||
| if (NameResolutionTelemetry.AnyDiagnosticsEnabled() || isLocalhostSubdomain) |
There was a problem hiding this comment.
Why do we need the telemetry-enabled path for localhost subdomains? What is preventing us from using the non-telemetry path?
There was a problem hiding this comment.
The telemetry-enabled path (GetAddrInfoWithTelemetryAsync) contains the localhost subdomain fallback logic - if the OS resolver returns an empty address list or throws a SocketException, it falls back to resolving plain localhost instead. This is the RFC 6761 Section 6.3 compliant behavior for *.localhost subdomains.
The non-telemetry path (NameResolutionPal.GetAddrInfoAsync) doesn't have this fallback handling - it just returns whatever the OS resolver returns. So we need to route localhost subdomains through the telemetry-enabled path to ensure the fallback mechanism is invoked when needed.
The condition could perhaps be renamed to be clearer, e.g., GetAddrInfoWithFallbackAsync or add a comment explaining that the method handles both telemetry and RFC 6761 fallback. Would that help clarify the intent?
2 participants
Handle RFC 6761 special-use domain names in Dns resolution
Description
This PR implements RFC 6761 compliant handling for special-use domain names in
System.Net.Dns.RFC 6761 compliance:
invalidand*.invalid- ReturnSocketError.HostNotFound(NXDOMAIN) per Section 6.4*.localhostsubdomains - Try OS resolver first, fall back to plainlocalhostresolution if OS returns failure or empty (per Section 6.3)localhost- Continues to use OS resolver to preserve/etc/hostscustomizationsBehavior Change
Before this PR:
Dns.GetHostAddresses("test.invalid")- Would query DNS servers and eventually failDns.GetHostAddresses("foo.localhost")- Would fail with HostNotFound (most systems don't have subdomains configured)After this PR:
Dns.GetHostAddresses("test.invalid")- Immediately returnsSocketError.HostNotFoundwithout network I/ODns.GetHostAddresses("foo.localhost")- Tries OS resolver first, falls back to returning loopback addresses if OS fails/returns emptyThis is an intentional breaking change for RFC 6761 compliance. Applications that previously caught
SocketExceptionfor*.localhostsubdomains will now get successful results (loopback addresses). This enables local development scenarios where services use*.localhostsubdomains.Implementation Notes
Updated based on team feedback from @rzikm:
The previous implementation returned pre-allocated loopback arrays immediately for
*.localhostsubdomains. This was changed to:/etc/hostsor local DNS) to work correctlylocalhostresolution - If OS resolver fails or returns empty results, resolve plainlocalhostinstead to guarantee loopback addressesThis approach:
localhoston typical systemsAddressFamilyfilteringTesting
Added tests covering:
invalid,test.invalid,foo.bar.invalid,invalid.,test.invalid.)foo.localhost,bar.foo.localhost,foo.localhost.)INVALID,Test.INVALID,FOO.LOCALHOST)AddressFamilyfiltering for localhost subdomainsnotlocalhost,localhostfooto ensure similar names aren't incorrectly matched.localhost,..invalid) passed to OS resolverlocalhostFixes #118569