From d915e0b407adcfd24df6e28be22f095909749aa3 Mon Sep 17 00:00:00 2001
From: Eugene Burkov <E.Burkov@AdGuard.COM>
Date: Mon, 18 Oct 2021 14:46:20 +0300
Subject: [PATCH] dhcpd: fix options priority

---
 CHANGELOG.md              |  3 ++
 internal/dhcpd/v4.go      |  7 ++--
 internal/dhcpd/v4_test.go | 74 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7b017cf0b87..45ea5c9b08a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -114,6 +114,8 @@ In this release, the schema version has changed from 10 to 12.
 
 ### Fixed
 
+- Too low priority for explicitly configured domain name server DHCP option
+  ([#3744]).
 - Occasional panic during shutdown ([#3655]).
 - Addition of IPs into only one as opposed to all matching ipsets on Linux
   ([#3638]).
@@ -204,6 +206,7 @@ In this release, the schema version has changed from 10 to 12.
 [#3607]: https://github.com/AdguardTeam/AdGuardHome/issues/3607
 [#3638]: https://github.com/AdguardTeam/AdGuardHome/issues/3638
 [#3655]: https://github.com/AdguardTeam/AdGuardHome/issues/3655
+[#3744]: https://github.com/AdguardTeam/AdGuardHome/issues/3744
 
 
 
diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go
index d7e4dfae976..8ddfe98ba07 100644
--- a/internal/dhcpd/v4.go
+++ b/internal/dhcpd/v4.go
@@ -900,9 +900,10 @@ func (s *v4Server) process(req, resp *dhcpv4.DHCPv4) int {
 			resp.UpdateOption(dhcpv4.OptGeneric(code, configured.Get(code)))
 		}
 	}
-	// Update the value of Domain Name Server option separately from others
-	// since its value is set after server's creating.
-	if requested.Has(dhcpv4.OptionDomainNameServer) {
+	// Update the value of Domain Name Server option separately from others if
+	// not assigned yet since its value is set after server's creating.
+	if requested.Has(dhcpv4.OptionDomainNameServer) &&
+		!resp.Options.Has(dhcpv4.OptionDomainNameServer) {
 		resp.UpdateOption(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 	}
 
diff --git a/internal/dhcpd/v4_test.go b/internal/dhcpd/v4_test.go
index d20e715c410..47ed334a7c5 100644
--- a/internal/dhcpd/v4_test.go
+++ b/internal/dhcpd/v4_test.go
@@ -5,8 +5,10 @@ package dhcpd
 
 import (
 	"net"
+	"strings"
 	"testing"
 
+	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/mdlayher/raw"
 	"github.com/stretchr/testify/assert"
@@ -118,8 +120,78 @@ func TestV4_AddReplace(t *testing.T) {
 	}
 }
 
+func TestV4Server_Process_optionsPriority(t *testing.T) {
+	defaultIP := net.IP{192, 168, 1, 1}
+	knownIP := net.IP{1, 2, 3, 4}
+
+	prepareSrv := func(opt6IPs []net.IP) (s *v4Server) {
+		conf := V4ServerConf{
+			Enabled:    true,
+			RangeStart: net.IP{192, 168, 10, 100},
+			RangeEnd:   net.IP{192, 168, 10, 200},
+			GatewayIP:  net.IP{192, 168, 10, 1},
+			SubnetMask: net.IP{255, 255, 255, 0},
+			notify:     notify4,
+		}
+		if len(opt6IPs) > 0 {
+			b := &strings.Builder{}
+			stringutil.WriteToBuilder(b, "6 ips ", opt6IPs[0].String())
+			for _, ip := range opt6IPs[1:] {
+				stringutil.WriteToBuilder(b, ",", ip.String())
+			}
+			conf.Options = []string{b.String()}
+		}
+
+		ss, err := v4Create(conf)
+		require.NoError(t, err)
+
+		var ok bool
+		s, ok = ss.(*v4Server)
+		require.True(t, ok)
+
+		s.conf.dnsIPAddrs = []net.IP{defaultIP}
+
+		return s
+	}
+
+	checkResp := func(t *testing.T, s *v4Server, wantIPs []net.IP) {
+		mac := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}
+		req, err := dhcpv4.NewDiscovery(mac, dhcpv4.WithRequestedOptions(
+			dhcpv4.OptionDomainNameServer,
+		))
+		require.NoError(t, err)
+
+		var resp *dhcpv4.DHCPv4
+		resp, err = dhcpv4.NewReplyFromRequest(req)
+		require.NoError(t, err)
+
+		res := s.process(req, resp)
+		require.Equal(t, 1, res)
+
+		o := resp.GetOneOption(dhcpv4.OptionDomainNameServer)
+		require.NotEmpty(t, o)
+
+		wantData := []byte{}
+		for _, ip := range wantIPs {
+			wantData = append(wantData, ip...)
+		}
+		assert.Equal(t, o, wantData)
+	}
+
+	t.Run("default", func(t *testing.T) {
+		s := prepareSrv(nil)
+
+		checkResp(t, s, []net.IP{defaultIP})
+	})
+
+	t.Run("explicitly_configured", func(t *testing.T) {
+		s := prepareSrv([]net.IP{knownIP, knownIP})
+
+		checkResp(t, s, []net.IP{knownIP, knownIP})
+	})
+}
+
 func TestV4StaticLease_Get(t *testing.T) {
-	var err error
 	sIface, err := v4Create(V4ServerConf{
 		Enabled:    true,
 		RangeStart: net.IP{192, 168, 10, 100},