Support parsing known RR types in RFC 3597 format #1211
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the format used for "Unknown DNS Resource Records", but it's also useful to support parsing known RR types in this way. RFC 3597 says: An implementation MAY also choose to represent some RRs of known type using the above generic representations for the type, class and/or RDATA, which carries the benefit of making the resulting master file portable to servers where these types are unknown. Using the generic representation for the RDATA of an RR of known type can also be useful in the case of an RR type where the text format varies depending on a version, protocol, or similar field (or several) embedded in the RDATA when such a field has a value for which no text format is known, e.g., a LOC RR [RFC1876] with a VERSION other than 0. Even though an RR of known type represented in the \# format is effectively treated as an unknown type for the purpose of parsing the RDATA text representation, all further processing by the server MUST treat it as a known type and take into account any applicable type- specific rules regarding compression, canonicalization, etc.
This was spotted when writing TestParseKnownRRAsRFC3597.
This has the advantage that concrete types will now be returned for parsed ANY, NULL, OPT and TSIG records.
This ensures we're properly handling empty RDATA for RFC 3597 parsed records.
miekg
approved these changes
Jan 30, 2021
tmthrgd
added a commit
to tmthrgd/miekgdns
that referenced
this pull request
Jan 31, 2021
This was a bug found by oss-fuzz. My bad (miekg#1211).
miekg
pushed a commit
that referenced
this pull request
Feb 1, 2021
* Set Rdlength in fromRFC3597 This was a bug found by oss-fuzz. My bad (#1211). * Limit maximum length of Rdata in (*RFC3597).parse RDATA must be a 16-bit unsigned integer. * Validate Rdlength and off in UnpackRRWithHeader * Revert "Validate Rdlength and off in UnpackRRWithHeader" This reverts commit 2f6a881. * Use hex.DecodedLen in (*RFC3597).fromRFC3597 While this isn't done elsewhere, it is clearer and more obvious.
aanm
pushed a commit
to cilium/dns
that referenced
this pull request
Jul 29, 2022
* Support parsing known RR types in RFC 3597 format This is the format used for "Unknown DNS Resource Records", but it's also useful to support parsing known RR types in this way. RFC 3597 says: An implementation MAY also choose to represent some RRs of known type using the above generic representations for the type, class and/or RDATA, which carries the benefit of making the resulting master file portable to servers where these types are unknown. Using the generic representation for the RDATA of an RR of known type can also be useful in the case of an RR type where the text format varies depending on a version, protocol, or similar field (or several) embedded in the RDATA when such a field has a value for which no text format is known, e.g., a LOC RR [RFC1876] with a VERSION other than 0. Even though an RR of known type represented in the \# format is effectively treated as an unknown type for the purpose of parsing the RDATA text representation, all further processing by the server MUST treat it as a known type and take into account any applicable type- specific rules regarding compression, canonicalization, etc. * Correct mistakes in TestZoneParserAddressAAAA This was spotted when writing TestParseKnownRRAsRFC3597. * Eliminate canParseAsRR This has the advantage that concrete types will now be returned for parsed ANY, NULL, OPT and TSIG records. * Expand TestDynamicUpdateParsing for RFC 3597 This ensures we're properly handling empty RDATA for RFC 3597 parsed records.
aanm
pushed a commit
to cilium/dns
that referenced
this pull request
Jul 29, 2022
* Set Rdlength in fromRFC3597 This was a bug found by oss-fuzz. My bad (miekg#1211). * Limit maximum length of Rdata in (*RFC3597).parse RDATA must be a 16-bit unsigned integer. * Validate Rdlength and off in UnpackRRWithHeader * Revert "Validate Rdlength and off in UnpackRRWithHeader" This reverts commit 2f6a881. * Use hex.DecodedLen in (*RFC3597).fromRFC3597 While this isn't done elsewhere, it is clearer and more obvious.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RFC 3597 says we should allow parsing known record types in the "Unknown DNS Resource Record Type" format. This makes sense, zone files shouldn't become invalid simply because we learn a new concrete RR type or because other software doesn't recognise an RR type we support. The RFC also mentions instances where it can be preferable to use this format even in widely known RR types.
The concrete RR type will be returned for known record types. This is true even of ANY, NULL, OPT and TSIG records which would previously return a
*RFC3597record.Fixes #1209
/cc @madestro