-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Trailing backslash results in non-FQDN targets #1528
Comments
but |
An even number of slashes definitely seems legal. Digging a bit, it looks like FQDN-ing seems fine to me since |
(on my phone)
Returning an error when escape is true looks like a good thing to do.
Unsure if there is RFC text on that corner case.
…On Fri, 19 Jan 2024, 16:10 Janik Rabe, ***@***.***> wrote:
An even number of slashes definitely seems legal.
Digging a bit, it looks like CNAME.parse() receives an example.com\
token. Maybe (*zlexer).Next() should return an error when escape is still
true when returning? I need to look more into how (and if) this would
work.
FQDN-ing seems fine to me since CNAME.parse() calls toAbsoluteName based
on the provided origin, right?
—
Reply to this email directly, view it on GitHub
<#1528 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACWIWZ3Q72K43FDUVOOICLYPKEGZAVCNFSM6AAAAABB6KWFNKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBQGU4TONRUG4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
It's not a corner case in the RFC; CNAME RDATA format is just a <domain-name> and per RFC 1035 section 5.1:
(where «<character-string> is expressed in one or two ways: as a contiguous set of characters without interior spaces, or as a string beginning with a " and ending with a "… [and] \X where X is any character other than a digit (0-9), is used to quote that character so that its special meaning does not apply») So in a zone file, |
ack
you agree with returning an error from the parser is |
Yes, I think |
Keep track if the escape, if still true when returning isDomainName should return false. TODO: - Should still be done in packDomainName as well. - And that should be tested - Some tests now fail There are multiple other places that supposedly also check for this, but they are not called in the parsing. Fixes: #1528 Signed-off-by: Miek Gieben <miek@miek.nl>
Keep track if the escape, if still true when returning isDomainName should return false. TODO: - Should still be done in packDomainName as well. - And that should be tested - Some tests now fail There are multiple other places that supposedly also check for this, but they are not called in the parsing. Fixes: #1528 Signed-off-by: Miek Gieben <miek@miek.nl>
Calling
NewRR(". 1 IN CNAME example.com")
normally turnsexample.com
into an FQDN,example.com.
, before storing it in the RR'sTarget
variable.However,
NewRR(". 1 IN CNAME example.com\\")
with a trailing backslash results inexample.com\.
, which is not an FQDN since the last dot is escaped.It seems to me like the parser should either add another dot in these cases, or (perhaps better) reject the trailing backslash with an error.
This may be related to #1384.
The text was updated successfully, but these errors were encountered: