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

Dns_trie.entries doesn't include glue records #360

Open
RyanGibb opened this issue Dec 2, 2024 · 5 comments
Open

Dns_trie.entries doesn't include glue records #360

RyanGibb opened this issue Dec 2, 2024 · 5 comments

Comments

@RyanGibb
Copy link
Contributor

RyanGibb commented Dec 2, 2024

Hi, hope you're well :-)

For example, consider the following zone file:

$ORIGIN example.org.
$TTL 3600
@ IN SOA ns1 dns (
  1        ; Serial No.
  3600     ; 1hr Refresh
  900      ; 15m Retry
  1814400  ; 21d Expire
  3600     ; 1hr Negative Cache TTL
)
ns.subdomain IN  A 128.232.113.136
subdomain IN  NS ns.subdomain

On a trie built from this zonefile Dns_trie.entries will return the SOA and NS records, but not the A record.

Changing false to true on this line would include it, but would also include all other records of the subdomain. I think a 'selective recurse' could solve this.

I'm using Dns_trie.entries to get a list of all RRs in a zone, and I think it should include the glue records. But perhaps this this behaviour is intentional? Any insight would be much appreciated.

@RyanGibb
Copy link
Contributor Author

RyanGibb commented Dec 4, 2024

RyanGibb/eon@1d5769d seems to be a solution to this.

For the zonefile,

$ORIGIN example.org.
$TTL 3600
@ IN SOA ns1 dns (
  1        ; Serial No.
  3600     ; 1hr Refresh
  900      ; 15m Retry
  1814400  ; 21d Expire
  3600     ; 1hr Negative Cache TTL
)
ns.subdomain IN  A 128.232.113.136
sub.subdomain IN  A 128.232.113.136
subdomain IN  NS ns.subdomain

It includes only the A record for ns.subdomain.example.org and not the record for sub.subdomain.example.org.

@hannesm
Copy link
Member

hannesm commented Dec 4, 2024

The entries is not supposed to return glue. Is there a specific reason (performance? convenience?) why that should return the glue as well?

There's a separate function available:

val lookup_glue : 'a Domain_name.t -> t ->
  (int32 * Ipaddr.V4.Set.t) option * (int32 * Ipaddr.V6.Set.t) option
(** [lookup_glue k t] finds glue records (A, AAAA) for [k] in [t]. It ignores
    potential DNS invariants, e.g. that there is no surrounding zone. *)

From what I remember, there's not so much added value to return the glue: getting the glue is not so much added complexity/performance (usually domain name tries are shallow). Also, a server should carry the glue in the additional section (not in the answer), so if entries would return it, it would need to be separate. Taking a look at the users of entries, i.e. AXFR shouldn't include the glue as glue... Does this make sense?

Btw, in dns_server.ml there is a function that collects the glue.

let lookup_glue_for_zone zone data =
  match Dns_trie.lookup zone Rr_map.Ns data with
  | Error _ -> Domain_name.Map.empty
  | Ok (_, names) ->
    Domain_name.Host_set.fold (fun ns acc ->
        match Dns_trie.lookup ns Rr_map.A data with
        | Ok _ -> acc
        | Error _ -> match Dns_trie.lookup_glue ns data with
          | None, None -> acc
          | Some v4, None -> Name_rr_map.add (Domain_name.raw ns) Rr_map.A v4 acc
          | None, Some v6 -> Name_rr_map.add (Domain_name.raw ns) Rr_map.Aaaa v6 acc
          | Some v4, Some v6 ->
            Name_rr_map.add (Domain_name.raw ns) Rr_map.A v4
              (Name_rr_map.add (Domain_name.raw ns) Rr_map.Aaaa v6 acc))
      names Domain_name.Map.empty

Which may be what you're looking for?

RyanGibb/eon@1d5769d seems to be a solution to this.

Hmm, as described above, I think that changing the semantics of entries isn't straightforward - looking at the places where it is being used.

@RyanGibb
Copy link
Contributor Author

RyanGibb commented Dec 4, 2024

Hi Hannes, thanks for the response. I'm using Dns_trie.entries to get a list of all records to transfer to a secondary nameserver.

Btw, in dns_server.ml there is a function that collects the glue.
...
Which may be what you're looking for?

Yes, I basically included this same logic in the referenced commit. I could do this outside the function call myself, but;

Taking a look at the users of entries, i.e. AXFR shouldn't include the glue as glue... Does this make sense?

I'm a bit confused about this. It seems the RFC specifies that glue records should be included? https://www.rfc-editor.org/rfc/rfc5936#section-3.3

@hannesm
Copy link
Member

hannesm commented Dec 4, 2024

I'm a bit confused about this. It seems the RFC specifies that glue records should be included? https://www.rfc-editor.org/rfc/rfc5936#section-3.3

Ah, thanks. My brain was more in RFC 1034. Maybe we need some additional test cases and add the glue to AXFR then. Thanks for bringing this up.

@RyanGibb
Copy link
Contributor Author

RyanGibb commented Dec 4, 2024

No worries!

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