Skip to content
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

Add field(s) representing DNS header flags #9

Open
neilcook opened this issue Mar 25, 2024 · 5 comments
Open

Add field(s) representing DNS header flags #9

neilcook opened this issue Mar 25, 2024 · 5 comments

Comments

@neilcook
Copy link
Contributor

There are various DNS flags that are not represented in the protobuf dnsmessage. For example Recursion Desired (RD), Recursion Available (RA), Truncated Response (TC) etc.

It would be good if these were available in the protobuf in some form, specifically:

AA, RD, RA, TC, AD, CD (from regular DNS header flags)

And also it would be nice if the protobuf indicated:

  • The EDNS0 version number (if applicable)
  • Any EDNS0 header flags, i.e. DO
@omoerbeek
Copy link
Member

omoerbeek commented Apr 8, 2024

I can see two ways of doing this:

  1. Define the flags and other values of interest and add them to DNSQuestion and maybe DNSResponse as bool or other integer values, eg. add optional bool aaFlag, optional bool doFlag, optional uint8 extendedRCode,
    optional uint8 ednsVersion, ...
  2. Expose the request header and EDNS always present data (encoded in the wire format in the TTL fields of the OPT record) as-is, relying on the protobuf client to further decode by adding wire format fields to DNSQuestion and maybe DNSResponse. eg. optional bytes dnsHeader, optional uint32 ednsRCodeVersionAndFlags

Alternatively the values of interest could be added as Meta values. Encoding could be either using method 1 or 2

I think the amount of work to add these values does not make a lot of difference from the rec point of view. Methods 2 is more compact of course, but does involve work for the client to decode. Encoding in Meta values wastes a bit more bytes.

@neilcook
Copy link
Contributor Author

neilcook commented Apr 8, 2024

Thanks Otto. I lean toward option 2, mainly because it means we don't have to do any additional work in future to add for example new EDNS0 flags, even if that does mean a bit more work on behalf of the protobuf client.

Adding in Meta is a nice idea, my only worry with that is that it's essentially undocumented, unless we added something to this repo to document all the things we add to Meta. Which we probably should do, in which case it would probably be my preferred way (with option 2 encoding).

@omoerbeek
Copy link
Member

Currently we add nothing to Meta in rec. We could reserve a portion of the namespace for rec itself, e.g. any name starting with underscore of some other prefix. Then properly document the fields rec adds by itself.

@neilcook
Copy link
Contributor Author

neilcook commented Apr 8, 2024

I think dnsdist could also potentially add these fields?

@omoerbeek
Copy link
Member

omoerbeek commented Apr 8, 2024

yes, atm only configurable in RemoteLogAction and RemoteLogResponseAction. So specific product name should likely not be part of the reserved prefix. We want to coordinate things between rec and dnsdist (if dnsdist is also going to be adding Meta values as well by its own).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants