dns
dns copied to clipboard
miekg
Use golang.org/x/crypto/cryptobyte to unpack DNS records
This substantially simplifies the machinery and state tracking around DNS record unpacking. Gone are the off int arguments! Gone are the manual slice length checks!
For the most part care has been taken to maintain existing behaviour, including quirks, with one of the few notable intentional change being to UnpackDomainName where we now strictly enforce that pointers point backwards into the message. We also fix a bug here or there (see unpackQuestion) and are more strict in our unpacking of certain EDNS0 options.
holy moly this cleans stuff up.
You know I thought it would, hence the PR, but even I was surprised just how much it improves things.
Would be good to run the coredns testsuite with this version.
Yep that's actually my hold up at the moment. I've been debugging and working through a few scattered CoreDNS test failures.
regarding this errors: I think the est. pattern in this lib is: "bad.. ", with maybe slightly more context than just 'bad SVCB'
Yeah I've gone back and forth on the errors and ended up with something quite different. There may be compatibility concerns, but ultimately I think we need to be far more semantic with our unpacking errors. I've gone and distinguished between failures to unpack the message framing and failures to unpack the record data specifically. Once we're in a length-delimited section, it doesn't make sense to use the same error anymore as we're not dealing with potential truncation but rather just corrupt marshalling.
Edit: The error changes very much could be backward incompatible. It's hard to tell exactly. If so, I think we can work around that while still mostly keeping the semantic distinctions.
I've left quite a few TODOs and comments where I've discovered incorrect or problematic behaviour. I've successfully run the CoreDNS test suite against this change now as well. Figuring out why it was failing was quite difficult and led down the RFC rabbit hole and exposed some bugs in this library. I'll aim to address all the TODOs in future PRs.
think this could go in and the fix some fallout and the dyn. update stuff?
(pointer to self should still be checked I guess, but can also be done in followup)
