Pull request 2750: AGDNS-3720-imp-cert-has-ip

Squashed commit of the following:

commit fb4eef0ca4f90c01f0935138ccf69337c83422f2
Merge: 4d3772f51 e0b474f9d
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Thu Aug 13 17:50:58 2026 +0300

    Merge branch 'master' into AGDNS-3720-imp-cert-has-ip

commit 4d3772f511d2fdfc2679d47122e8ec7415b98d8d
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Thu Aug 13 17:28:29 2026 +0300

    home: imp tests;

commit 7f2d3fe91fbde242d56f124c168aecc7fd7c8ace
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Thu Aug 13 11:55:48 2026 +0300

    home: fix test;

commit a1459d011f03ecf8565daab9d9a3d428cb249505
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Thu Aug 13 11:29:57 2026 +0300

    home: imp test;

commit c24faddaed3d7347e4bb9fdc0ab4b33025b3a25b
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Wed Aug 12 15:17:04 2026 +0300

    dnsforward: imp code;

commit d9ad570cfe371820e7d641fa954fa12f64d3d4e2
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Wed Aug 12 15:14:23 2026 +0300

    home: cover has ip addrs with tests; rm invalid todo;
This commit is contained in:
Maksim Kazantsev committed 2026-08-13 15:02:00 +00:00
1 parent e0b474f9d2
commit 0c3cc9070a
3 files changed
+133 -22

No files matched your search

+21 -20
View File
@@ -251,28 +251,29 @@ func (s *Server) makeDDRResponse(req *dns.Msg) (resp *dns.Msg) {
// server's TLS certificate contains IP addresses. req and resp must not be
// nil.
func (s *Server) appendDoTResolvers(req, resp *dns.Msg, domainName string) {
hasIPAddrs := s.tlsConfigProvider.HasIPAddrs()
if !s.tlsConfigProvider.HasIPAddrs() {
return
}
if hasIPAddrs {
// Only add DNS-over-TLS resolvers in case the certificate contains IP
// addresses.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/4927.
for _, addr := range s.dnsProxy.TLSListenAddr {
values := []dns.SVCBKeyValue{
&dns.SVCBAlpn{Alpn: []string{"dot"}},
&dns.SVCBPort{Port: uint16(addr.Port)},
}
ans := &dns.SVCB{
Hdr: s.hdr(req, dns.TypeSVCB),
Priority: 1,
Target: domainName,
Value: values,
}
resp.Answer = append(resp.Answer, ans)
// Only add DNS-over-TLS resolvers in case the certificate contains IP
// addresses.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/4927.
for _, addr := range s.dnsProxy.TLSListenAddr {
values := []dns.SVCBKeyValue{
&dns.SVCBAlpn{Alpn: []string{"dot"}},
&dns.SVCBPort{Port: uint16(addr.Port)},
}
ans := &dns.SVCB{
Hdr: s.hdr(req, dns.TypeSVCB),
Priority: 1,
Target: domainName,
Value: values,
}
resp.Answer = append(resp.Answer, ans)
}
}
-2
View File
@@ -916,8 +916,6 @@ func (m *tlsManager) HasIPAddrs() (ok bool) {
return false
}
// TODO(m.kazantsev): Consider storing the value instead of parsing each
// time.
return aghtls.CertificateHasIP(m.tlsCert.Leaf)
}
+112
View File
@@ -8,6 +8,7 @@ import (
"crypto/x509"
"encoding/pem"
"math/big"
"net"
"os"
"path/filepath"
"testing"
@@ -200,6 +201,35 @@ func newCertWithoutIP(tb testing.TB) (
return caCert, buf.Bytes(), leafKeyPEM
}
// newCertWithIP generates a self-signed certificate with an IP address in its
// SAN extension and returns the PEM-encoded certificate and private key.
func newCertWithIP(tb testing.TB) (certPEM, keyPEM []byte) {
tb.Helper()
key, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(tb, err)
now := time.Now()
tmpl := &x509.Certificate{
SerialNumber: big.NewInt(1),
NotBefore: now.Add(-time.Hour),
NotAfter: now.Add(time.Hour),
KeyUsage: x509.KeyUsageDigitalSignature,
IPAddresses: []net.IP{net.ParseIP("192.0.2.1")},
}
certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key)
require.NoError(tb, err)
certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
keyPEM = pem.EncodeToMemory(&pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: x509.MarshalPKCS1PrivateKey(key),
})
return certPEM, keyPEM
}
// newCertAndKey is a helper function that generates certificate and key.
func newCertAndKey(tb testing.TB, n int64) (certDER []byte, key *rsa.PrivateKey) {
tb.Helper()
@@ -348,3 +378,85 @@ func TestTLSManager_Reload(t *testing.T) {
extTLSConf = m.ExtendedTLSConfig()
assertCertSerialNumber(t, extTLSConf, snAfter)
}
func TestTLSManager_HasIPAddrs(t *testing.T) {
t.Parallel()
_, noIPChainPEM, noIPKeyPEM := newCertWithoutIP(t)
ipChainPEM, ipKeyPEM := newCertWithIP(t)
noIPSettings := &aghtls.ExtendedTLSConfig{
Enabled: true,
CertificateChain: string(noIPChainPEM),
PrivateKey: string(noIPKeyPEM),
}
ipSettings := &aghtls.ExtendedTLSConfig{
Enabled: true,
CertificateChain: string(ipChainPEM),
PrivateKey: string(ipKeyPEM),
}
testCases := []struct {
want assert.BoolAssertionFunc
name string
certificateChainData []byte
privateKeyData []byte
settings *aghtls.ExtendedTLSConfig
}{{
name: "no_ip_in_cert",
settings: noIPSettings,
want: assert.False,
}, {
name: "has_ip_in_cert",
settings: ipSettings,
want: assert.True,
}, {
name: "updated_to_ip",
settings: noIPSettings,
certificateChainData: ipChainPEM,
privateKeyData: ipKeyPEM,
want: assert.True,
}, {
name: "updated_to_no_ip",
settings: ipSettings,
certificateChainData: noIPChainPEM,
privateKeyData: noIPKeyPEM,
want: assert.False,
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Do not run in parallel because the test modifies the TLS
// manager's state.
ctx := testutil.ContextWithTimeout(t, testTimeout)
m, err := newTLSManager(ctx, &tlsManagerConfig{
logger: testLogger,
confModifier: agh.EmptyConfigModifier{},
manager: aghtls.EmptyManager{},
extTLSConf: tc.settings,
servePlainDNS: false,
})
require.NoError(t, err)
if tc.certificateChainData == nil && tc.privateKeyData == nil {
tc.want(t, m.HasIPAddrs())
return
}
func() {
m.mu.Lock()
defer m.mu.Unlock()
var cert tls.Certificate
cert, err = tls.X509KeyPair(tc.certificateChainData, tc.privateKeyData)
require.NoError(t, err)
m.tlsCert = &cert
}()
tc.want(t, m.HasIPAddrs())
})
}
}