Don't make duplicate SVCB records for DDR responses
Prerequisites
-
[x] I have checked the Wiki and Discussions and found no answer
-
[x] I have searched other issues and found no duplicates
-
[x] I want to request a feature or enhancement and not ask a question
The problem
The DDR response generated with makeDDRResponse can create multiples of the same record due to the generation method relying on dns.bind_hosts value inside AGH configuration:
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/dnsforward/process.go#L251
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/dnsforward/process.go#L273
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/dnsforward/process.go#L290
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/dnsforward/config.go#L621-L622
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/home/dns.go#L325-L335
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/home/dns.go#L243-L256
An example of this can be demonstrated by having a working AGH with encrypted services enabled. Only requirement is having more than one bind_host IP address (and assume that the TLS certificate does not contain any of the configured IP addresses):
dns:
bind_hosts:
# private ip for local operations
- 127.53.53.1
# public ipv4 to be used
- 192.0.2.1
# public ipv6 to be used
- "100::"
We can demo this by making the DDR query:
root@aghdemo:~# dig _dns.resolver.arpa SVCB @127.53.53.1 +short
1 aghdemo.example.org. alpn="h2" port=443 key7="/dns-query{?dns}"
1 aghdemo.example.org. alpn="h2" port=443 key7="/dns-query{?dns}"
1 aghdemo.example.org. alpn="h2" port=443 key7="/dns-query{?dns}"
1 aghdemo.example.org. alpn="doq" port=853
1 aghdemo.example.org. alpn="doq" port=853
1 aghdemo.example.org. alpn="doq" port=853
root@aghdemo:~#
Proposed solution
Making sure the DDR record(s) are deduplicated before answered. And ideally remove local IP addresses if a TLS certificate does contain one of the bound IPs.
Alternatives considered and additional information
Issue current as of v0.108.0-b.68. And loosely related to #6487 ((also, maybe no duplicated answers)), considering the h2 value is hardcoded while serve_http3 is not checked in makeDDRResponse.
Related, and I was about to open a new issue, but the dohpath of the DDR SVCB answer shows up as key7 (as clear from the screenshot).
I thought the RFC required dohpath to be present in the answer, am I misunderstanding something?
Related, and I was about to open a new issue, but the
dohpathof the DDR SVCB answer shows up askey7(as clear from the screenshot).I thought the RFC required
dohpathto be present in the answer, am I misunderstanding something?
~~@meghadeep-com You're right! It is required (https://www.rfc-editor.org/rfc/rfc9461.html#name-new-svcparamkey-dohpath). I suggest creating the new issue to be tracked anyway.~~
UPDATE: See next comment
@meghadeep-com Ah I missed out one thing, it is actually correctly set:
https://github.com/AdguardTeam/AdGuardHome/blob/e5d0f0b1196adb7af12cc18c2382764aee0af292/internal/dnsforward/process.go#L255
Its just that dig hasn't renamed the key from key7 to dohpath yet.
@linuxgemini Ahh gotchu, thank you!
Apologies for crowding the thread.