Don't make duplicate SVCB records for DDR responses

[linuxgemini]

Prerequisites

• I have checked the Wiki and Discussions and found no answer

• I have searched other issues and found no duplicates

• 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:

AdGuardHome/internal/dnsforward/process.go

Line 251 in e5d0f0b

for _, addr := range s.conf.TLSConf.HTTPSListenAddrs {

AdGuardHome/internal/dnsforward/process.go

Line 273 in e5d0f0b

for _, addr := range s.dnsProxy.TLSListenAddr {

AdGuardHome/internal/dnsforward/process.go

Line 290 in e5d0f0b

for _, addr := range s.dnsProxy.QUICListenAddr {

AdGuardHome/internal/dnsforward/config.go

Lines 621 to 622 in e5d0f0b

	proxyConfig.TLSListenAddr = s.conf.TLSConf.TLSListenAddrs
	proxyConfig.QUICListenAddr = s.conf.TLSConf.QUICListenAddrs

AdGuardHome/internal/home/dns.go

Lines 325 to 335 in e5d0f0b

	if conf.PortHTTPS != 0 {
	dnsConf.HTTPSListenAddrs = ipsToTCPAddrs(addrs, conf.PortHTTPS)
	}
	if conf.PortDNSOverTLS != 0 {
	dnsConf.TLSListenAddrs = ipsToTCPAddrs(addrs, conf.PortDNSOverTLS)
	}
	if conf.PortDNSOverQUIC != 0 {
	dnsConf.QUICListenAddrs = ipsToUDPAddrs(addrs, conf.PortDNSOverQUIC)
	}

AdGuardHome/internal/home/dns.go

Lines 243 to 256 in e5d0f0b

	func newServerConfig(
	dnsConf *dnsConfig,
	clientSrcConf *clientSourcesConfig,
	tlsConf *tlsConfigSettings,
	tlsMgr *tlsManager,
	httpReg aghhttp.RegisterFunc,
	clientsContainer dnsforward.ClientsContainer,
	) (newConf *dnsforward.ServerConfig, err error) {
	hosts := aghalg.CoalesceSlice(dnsConf.BindHosts, []netip.Addr{netutil.IPv4Localhost()})
	fwdConf := dnsConf.Config
	fwdConf.ClientsContainer = clientsContainer
	intTLSConf, err := newDNSTLSConfig(tlsConf, hosts)

---

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.

[meghadeep-com]

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?

[linuxgemini]

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?

@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

[linuxgemini]

@meghadeep-com Ah I missed out one thing, it is actually correctly set:

AdGuardHome/internal/dnsforward/process.go

Line 255 in e5d0f0b

&dns.SVCBDoHPath{Template: "/dns-query{?dns}"},

Its just that dig hasn't renamed the key from key7 to dohpath yet.

[meghadeep-com]

@linuxgemini Ahh gotchu, thank you!

Apologies for crowding the thread.