search

jafayer / DinoDNS

https://dinodns.dev
2 stars 0 forks source link

Add the Ability to Return Authority Records in the Packet Response #29

Closed zicklag closed 2 days ago

zicklag commented 1 week ago

Packet type allows you to add answer and additional records, but it doesn't allow you to add authority records.

I think I might need this, but I haven't learned enough about DNS to know for sure yet, they might be optional.

zicklag commented 1 week ago

I validated that records in the authority section are required to comply with RFC 2308 Section 2.2.

zicklag commented 1 week ago

Oh, I just found the packet.raw.authorities :grinning:

That works, but we might want to setup a similar wrapper as we have for the answers and additionals.

jafayer commented 1 week ago

Ah good catch, just forgot to implement that in the packet wrapper! Simple fix. Thanks for flagging!

jafayer commented 1 week ago

For context (both for you and anyone else who happens to be browsing), the wrapper mostly exists for two reasons:

  1. it was difficult to enforce the no-edit-after-sent behavior with the raw packet. The set wrappers around the raw packet properties allow us to check for this.frozen before allowing modifications and to throw the custom ModifiedAfterSentError without implementing an arguably more complicated proxy.
  2. The typing of the answers property is arguably overly permissive with the answers that are allowed in the dns-packet typings. Specifically, the OPT pseudo record type violates the interface that all other records have (it lacks a data attribute), and TypeScript REALLY did not like plugins that tried to access the data property of the answers array with OPT records in that interface. Indeed, the RFC seems to specify that OPT records may only be added "to the additional data section of a request". It actually looks like there's an open issue about this on the dns-packet library, but that library is also distinct from the types which would have to be updated as well.
jafayer commented 2 days ago

Just closing the loop on this -- this was just deployed in version 0.0.12!