getdns icon indicating copy to clipboard operation
getdns copied to clipboard

Better recognize IPv6 address primitives in the JSON like format

Open bortzmeyer opened this issue 7 years ago • 15 comments

A comment in the sample stubby configuration file says "IPv6 addresses ending in :: are not yet supported (use ::0)". But it is also true for addresses STARTING in '::'.

'0::1' is accepted, but '::1' triggers a very unhelpful "Could not parse config file [...] Generic error".

bortzmeyer avatar Nov 16 '17 07:11 bortzmeyer

127.0.0.1@8053 generates the same error here.

keltia avatar Nov 16 '17 20:11 keltia

@keltia Are you sure you've configured everything correctly? The following works for me:

  • 127.0.0.1@453
  • 0::1@453
  • 192.168.66.1@453

zyrill avatar Nov 22 '17 19:11 zyrill

Here is what I get with my stubby.yaml file.

364 [20:45] roberto@lonrach:local/etc> stubby -C /usr/local/etc/stubby.yaml
Could not parse config file # Specifies whether to run as a recursive or stub resolver
# For stubby this MUST be set to GETDNS_RESOLUTION_STUB
resolution_type: GETDNS_RESOLUTION_STUB

dns_transport_list:
  - GETDNS_TRANSPORT_TLS

listen_addresses:
  - 127.0.0.1@8053

upstream_recursive_servers:
# Quad9
  - address_data: 9.9.9.9
    tls_auth_name: "dns.quad9.net"
  - address_data: 2620:fe::fe
    tls_auth_name: "dns.quad9.net"
, "Generic error"
Could not parse config file "/usr/local/etc/stubby.yaml": Generic error

What is interesting is that if I use the default stubby.yml file, it works.

keltia avatar Nov 22 '17 19:11 keltia

You’ve likely made a typo in the file. Can you share the raw stubby.yaml file?

ArchangeGabriel avatar Nov 22 '17 19:11 ArchangeGabriel

The Quad9 stuff is wrong. Delete it.

zyrill avatar Nov 22 '17 20:11 zyrill

Here is the entire file.

# Specifies whether to run as a recursive or stub resolver
# For stubby this MUST be set to GETDNS_RESOLUTION_STUB
resolution_type: GETDNS_RESOLUTION_STUB

dns_transport_list:
  - GETDNS_TRANSPORT_TLS

listen_addresses:
  - 127.0.0.1@8053

upstream_recursive_servers:
# Quad9
  - address_data: 9.9.9.9
    tls_auth_name: "dns.quad9.net"
  - address_data: 2620:fe::fe
    tls_auth_name: "dns.quad9.net"

keltia avatar Nov 22 '17 20:11 keltia

see above. Delete Quad9 and use one of the provided servers that comes with the example file.

zyrill avatar Nov 22 '17 20:11 zyrill

@zyrill the whole point is to use Quad9 :)

keltia avatar Nov 22 '17 20:11 keltia

@zyrill You're clearly wrong. I use this configuration file and it works:


listen_addresses:
  - 0::1@8053
# https://github.com/getdnsapi/getdns/issues/358

dns_transport_list:
  - GETDNS_TRANSPORT_TLS

upstream_recursive_servers:
# Quad9
   - address_data: 9.9.9.9
     tls_auth_name: "dns.quad9.net"
   - address_data: 2620:fe::fe
     tls_auth_name: "dns.quad9.net"

bortzmeyer avatar Nov 22 '17 20:11 bortzmeyer

Rly... interesting. I used those and since there weren't any keys provided, the config check said invalid. Must have been using it wrong. I hope there's going to be better documentation - I obviously don't understand well enough how to use it.

zyrill avatar Nov 22 '17 20:11 zyrill

I think it is just the extension - if I rename my working stubby.yml file to stubby.yaml I too get a generic error: stubby -C /usr/etc/stubby/stubby.yaml

, "Generic error" Could not parse config file "/usr/etc/stubby/stubby.yaml": Generic error

hanvinke avatar Nov 22 '17 23:11 hanvinke

Ok, if I rename the file as stubby.yml it works. I'll dig into this.

I do not see the point of line https://github.com/getdnsapi/stubby/blob/a43be56e28f3a802f74b7c5b19b4b4c5fbaa908a/src/stubby.c#L270. Why would you care whether it is named .yml or not? If it parse as YAML, fine.

keltia avatar Nov 23 '17 09:11 keltia

Well... Stubby parsed its configuration in a JSON like format before we had parsing in YAML format. So to not break backwards compatibility and to make the distinction between JSON and YAML we decided to test for filename extension. The default config filenames end in .yml, so that is where the inflexibility in filename extension came from. Addressed in this commit getdnsapi/stubby@70832ecc Thanks for discovering and reporting!

wtoorop avatar Nov 23 '17 09:11 wtoorop

Thanks for the commit. Maybe the error message needs to be more specific?

keltia avatar Nov 23 '17 09:11 keltia

Yes the error message is very "generic" ;) The difficulty in returning line numbers is that the whole configuration is converted to a getdns_dict first before it is used to configure the getdns_context used with resolving. I can see an approach to potentially address this though... (turning the YAML parser into an iterator that returns a getdns_dict for each individual key/value pair augmented with the line number).

B.t.w. the title of this issue is wrong. It is not the IPv6 parser that is questionable, but the JSON like parser. I changed the title of this issue to reflect this.

Stubby converts the YAML to JSON and then converts to a getdns_dict to parse the configuration file. The getdns_dict's deal with IP addresses (and domain names etc.) in network/wire format and not in string format. The JSON like format came from the original spec. We always say "JSON like" and not just "JSON" because it contains all sorts of primitive types which are not in JSON, like the binary blobs.

I'd like to have one more attempt in recognizing the IPv6 primitive data from the JSON parser used internally before abandoning this approach and reverting to some form of second phase parsing by parsing the IP addresses in string format from within the getdns dictionary.

Another primitive I would love to see recognized by the JSON like parser is the IPv6/port specifications with square brackets I.e.: [2001:470:1c:76d::53]:443 or maybe even [2001:470:1c:76d::53]:https

wtoorop avatar Nov 23 '17 10:11 wtoorop