PowerDNS / pdns

PowerDNS Authoritative, PowerDNS Recursor, dnsdist
https://www.powerdns.com/
GNU General Public License v2.0
3.72k stars 912 forks source link

CAA/unQuotedText nits #6104

Open Habbie opened 6 years ago

Habbie commented 6 years ago
14:31Z <stbuehler> lieter: if lenField = false it skips the length byte anyway
14:31Z <stbuehler> (i know, it's never used like that)
14:32Z <Habbie> huh
14:32Z <Habbie> that seems bad

^ maybe just remove that feature?

14:42Z <stbuehler> looking at xfrUnquotedText it seems RecordTextReader accepts strings of any length, which might produce strange results in DNSPacketWriter, if the length is >= 256.
14:43Z <stbuehler> also RecordTextReader::xfrUnquotedText has a useless !val.empty() check - it just got cleared.
14:51Z <stbuehler> the tag field could be binary too, it just says "SHOULD NOT"
14:51Z <Habbie> uhuh
14:51Z <Habbie> this is all fine
14:51Z <stbuehler> which will break if you export as text into the db backend, and then try to read it back...
14:51Z <Habbie> it shouldn't though
14:51Z <Habbie> or don't we encode?
14:51Z <stbuehler> not afaics
14:51Z <Habbie> sigh
14:52Z <stbuehler> also no decoding on parse :)
stbuehler commented 6 years ago

RecordTextReader::xfrText and DNSPacketWriter::xfrText should make sure the quoted strings are shorter than 256 bytes too (if lenField is true); as the data is (should be for "unquoted text") stored in escaped form the check can only be done in DNSPacketWriter I guess. (appendSplit in dnslabeltext.rl autosplits content if it is too long)

Also it seems RecordTextReader::xfrText has a rather strict requirement for quotes, i.e. it doesn't accept data which would be considered valid by RFC 1035 (although this seems to be on purpose).

stbuehler commented 6 years ago

If DNSPacketWriter::xfrText gets an empty text it will always write a 0 byte, even is lenField == false. This will trigger malformed packets with URI and CAA, e.g. foo 300 IN URI 1 2.

zeha commented 2 years ago

Some notes:

I have some local changes to the CAA/URI code (really xfrText+xfrUnquotedText) but that suffers from this very correct existing comment in dnsparser.cc:

// exceptions thrown here do not result in logging in the main pdns auth server - just so you know!

... and is thus quite useless.