From e32c18faab81fb08cafe88324c7049cad5b47cf7 Mon Sep 17 00:00:00 2001
From: Ainar Garipov <a.garipov@adguard.com>
Date: Tue, 16 Feb 2021 17:58:42 +0300
Subject: [PATCH] Pull request: dhcpd: fix ip option parsing

Merge in DNS/adguard-home from 2688-dhcp-opt-ip to master

Updates #2688.

Squashed commit of the following:

commit e17e003a3a61c1f4ed55617bb61df721cbca12c1
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Feb 16 17:10:32 2021 +0300

    dhcpd: fix ip option parsing
---
 CHANGELOG.md                 |  8 ++++++++
 internal/dhcpd/dhcpd.go      | 12 ++++++++++--
 internal/dhcpd/dhcpd_test.go | 34 ++++++++++++++++++++++++----------
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 39217416..6d11db20 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,14 @@ and this project adheres to
 ## [v0.105.2] - 2021-02-24
 -->
 
+### Fixed
+
+- Wrong parsing of DHCP options of the `ip` type ([#2688]).
+
+[#2688]: https://github.com/AdguardTeam/AdGuardHome/issues/2688
+
+
+
 ## [v0.105.1] - 2021-02-15
 
 ### Changed
diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go
index bcd35d1c..e1d5ab1b 100644
--- a/internal/dhcpd/dhcpd.go
+++ b/internal/dhcpd/dhcpd.go
@@ -293,14 +293,22 @@ func parseOptionString(s string) (uint8, []byte) {
 		if err != nil {
 			return 0, nil
 		}
-
 	case "ip":
 		ip := net.ParseIP(sval)
 		if ip == nil {
 			return 0, nil
 		}
-		val = ip
 
+		// Most DHCP options require IPv4, so do not put the 16-byte
+		// version if we can.  Otherwise, the clients will receive weird
+		// data that looks like four IPv4 addresses.
+		//
+		// See https://github.com/AdguardTeam/AdGuardHome/issues/2688.
+		if ip4 := ip.To4(); ip4 != nil {
+			val = ip4
+		} else {
+			val = ip
+		}
 	default:
 		return 0, nil
 	}
diff --git a/internal/dhcpd/dhcpd_test.go b/internal/dhcpd/dhcpd_test.go
index e9b44518..cca733d9 100644
--- a/internal/dhcpd/dhcpd_test.go
+++ b/internal/dhcpd/dhcpd_test.go
@@ -3,7 +3,6 @@
 package dhcpd
 
 import (
-	"bytes"
 	"net"
 	"os"
 	"testing"
@@ -130,36 +129,51 @@ func TestOptions(t *testing.T) {
 	testCases := []struct {
 		name     string
 		optStr   string
-		wantCode uint8
 		wantVal  []byte
+		wantCode uint8
 	}{{
-		name:     "all_right_hex",
-		optStr:   " 12  hex  abcdef ",
-		wantCode: 12,
+		name:     "success_hex",
+		optStr:   "12 hex abcdef",
 		wantVal:  []byte{0xab, 0xcd, 0xef},
+		wantCode: 12,
 	}, {
 		name:     "bad_hex",
-		optStr:   " 12  hex  abcdef1 ",
+		optStr:   "12 hex abcdefx",
+		wantVal:  nil,
 		wantCode: 0,
 	}, {
-		name:     "all_right_ip",
+		name:     "success_ip",
 		optStr:   "123 ip 1.2.3.4",
+		wantVal:  net.IP{1, 2, 3, 4},
+		wantCode: 123,
+	}, {
+		name:   "success_ipv6",
+		optStr: "123 ip ::1234",
+		wantVal: net.IP{
+			0, 0, 0, 0,
+			0, 0, 0, 0,
+			0, 0, 0, 0,
+			0, 0, 0x12, 0x34,
+		},
 		wantCode: 123,
-		wantVal:  net.IPv4(1, 2, 3, 4),
 	}, {
 		name:     "bad_code",
 		optStr:   "256 ip 1.1.1.1",
+		wantVal:  nil,
 		wantCode: 0,
 	}, {
 		name:     "negative_code",
 		optStr:   "-1 ip 1.1.1.1",
+		wantVal:  nil,
 		wantCode: 0,
 	}, {
 		name:     "bad_ip",
 		optStr:   "12 ip 1.1.1.1x",
+		wantVal:  nil,
 		wantCode: 0,
 	}, {
 		name:     "bad_mode",
+		wantVal:  nil,
 		optStr:   "12 x 1.1.1.1",
 		wantCode: 0,
 	}}
@@ -167,9 +181,9 @@ func TestOptions(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			code, val := parseOptionString(tc.optStr)
-			require.EqualValues(t, tc.wantCode, code)
+			require.Equal(t, tc.wantCode, code)
 			if tc.wantVal != nil {
-				assert.True(t, bytes.Equal(tc.wantVal, val))
+				assert.Equal(t, tc.wantVal, val)
 			}
 		})
 	}