Pull request: dhcpd: add ips and text option types

Updates #2385.

Squashed commit of the following:

commit ce8467f1c013c6b3fef59667084e2c6569a7213c
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Mar 15 19:02:17 2021 +0300

    dhcpd: add ips and text option types

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to
 
 ### Added
 
+- `ips` and `text` DHCP server options ([#2385]).
 - `SRV` records support in `$dnsrewrite` filters ([#2533]).
 
 ### Changed
@@ -30,6 +31,7 @@ and this project adheres to
 
 - Go 1.14 support.
 
+[#2385]: https://github.com/AdguardTeam/AdGuardHome/issues/2385
 [#2412]: https://github.com/AdguardTeam/AdGuardHome/issues/2412
 [#2498]: https://github.com/AdguardTeam/AdGuardHome/issues/2498
 [#2533]: https://github.com/AdguardTeam/AdGuardHome/issues/2533

internal/dhcpd/dhcpd.go

@@ -2,18 +2,14 @@
 package dhcpd
 
 import (
-	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"net"
 	"net/http"
 	"path/filepath"
 	"runtime"
-	"strconv"
-	"strings"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/util"
 	"github.com/AdguardTeam/golibs/log"
 )
 
@@ -283,48 +279,3 @@ func (s *Server) FindMACbyIP(ip net.IP) net.HardwareAddr {
 func (s *Server) AddStaticLease(lease Lease) error {
 	return s.srv4.AddStaticLease(lease)
 }
-
-// Parse option string
-// Format:
-// CODE TYPE VALUE
-func parseOptionString(s string) (uint8, []byte) {
-	s = strings.TrimSpace(s)
-	scode := util.SplitNext(&s, ' ')
-	t := util.SplitNext(&s, ' ')
-	sval := util.SplitNext(&s, ' ')
-
-	code, err := strconv.Atoi(scode)
-	if err != nil || code <= 0 || code > 255 {
-		return 0, nil
-	}
-
-	var val []byte
-
-	switch t {
-	case "hex":
-		val, err = hex.DecodeString(sval)
-		if err != nil {
-			return 0, nil
-		}
-	case "ip":
-		ip := net.ParseIP(sval)
-		if ip == nil {
-			return 0, nil
-		}
-
-		// 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
-	}
-
-	return uint8(code), val
-}

internal/dhcpd/dhcpd_test.go

@@ -124,67 +124,3 @@ func TestNormalizeLeases(t *testing.T) {
 	assert.Equal(t, leases[1].HWAddr, staticLeases[1].HWAddr)
 	assert.Equal(t, leases[2].HWAddr, dynLeases[1].HWAddr)
 }
-
-func TestOptions(t *testing.T) {
-	testCases := []struct {
-		name     string
-		optStr   string
-		wantVal  []byte
-		wantCode uint8
-	}{{
-		name:     "success_hex",
-		optStr:   "12 hex abcdef",
-		wantVal:  []byte{0xab, 0xcd, 0xef},
-		wantCode: 12,
-	}, {
-		name:     "bad_hex",
-		optStr:   "12 hex abcdefx",
-		wantVal:  nil,
-		wantCode: 0,
-	}, {
-		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,
-	}, {
-		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,
-	}}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			code, val := parseOptionString(tc.optStr)
-			require.Equal(t, tc.wantCode, code)
-			if tc.wantVal != nil {
-				assert.Equal(t, tc.wantVal, val)
-			}
-		})
-	}
-}

internal/dhcpd/options.go

@@ -0,0 +1,138 @@
+package dhcpd
+
+import (
+	"encoding/hex"
+	"fmt"
+	"net"
+	"strconv"
+	"strings"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
+)
+
+// hexDHCPOptionParserHandler parses a DHCP option as a hex-encoded string.
+// For example:
+//
+//   252 hex 736f636b733a2f2f70726f78792e6578616d706c652e6f7267
+//
+func hexDHCPOptionParserHandler(s string) (data []byte, err error) {
+	data, err = hex.DecodeString(s)
+	if err != nil {
+		return nil, fmt.Errorf("decoding hex: %w", err)
+	}
+
+	return data, nil
+}
+
+// ipDHCPOptionParserHandler parses a DHCP option as a single IP address.
+// For example:
+//
+//   6 ip 192.168.1.1
+//
+func ipDHCPOptionParserHandler(s string) (data []byte, err error) {
+	ip := net.ParseIP(s)
+	if ip == nil {
+		return nil, agherr.Error("invalid 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 {
+		data = ip4
+	} else {
+		data = ip
+	}
+
+	return data, nil
+}
+
+// textDHCPOptionParserHandler parses a DHCP option as a simple UTF-8 encoded
+// text.  For example:
+//
+//   252 text http://192.168.1.1/wpad.dat
+//
+func ipsDHCPOptionParserHandler(s string) (data []byte, err error) {
+	ipStrs := strings.Split(s, ",")
+	for i, ipStr := range ipStrs {
+		var ipData []byte
+		ipData, err = ipDHCPOptionParserHandler(ipStr)
+		if err != nil {
+			return nil, fmt.Errorf("parsing ip at index %d: %w", i, err)
+		}
+
+		data = append(data, ipData...)
+	}
+
+	return data, nil
+}
+
+// ipsDHCPOptionParserHandler parses a DHCP option as a comma-separates list of
+// IP addresses.  For example:
+//
+//   6 ips 192.168.1.1,192.168.1.2
+//
+func textDHCPOptionParserHandler(s string) (data []byte, err error) {
+	return []byte(s), nil
+}
+
+// dhcpOptionParserHandler is a parser for a single dhcp option type.
+type dhcpOptionParserHandler func(s string) (data []byte, err error)
+
+// dhcpOptionParser parses DHCP options.
+type dhcpOptionParser struct {
+	handlers map[string]dhcpOptionParserHandler
+}
+
+// newDHCPOptionParser returns a new dhcpOptionParser.
+func newDHCPOptionParser() (p *dhcpOptionParser) {
+	return &dhcpOptionParser{
+		handlers: map[string]dhcpOptionParserHandler{
+			"hex":  hexDHCPOptionParserHandler,
+			"ip":   ipDHCPOptionParserHandler,
+			"ips":  ipsDHCPOptionParserHandler,
+			"text": textDHCPOptionParserHandler,
+		},
+	}
+}
+
+// parse parses an option.  See the handlers' documentation for more info.
+func (p *dhcpOptionParser) parse(s string) (code uint8, data []byte, err error) {
+	defer func() {
+		if err != nil {
+			err = fmt.Errorf("invalid option string %q: %w", s, err)
+		}
+	}()
+
+	s = strings.TrimSpace(s)
+	parts := strings.SplitN(s, " ", 3)
+	if len(parts) < 3 {
+		return 0, nil, agherr.Error("need at least three fields")
+	}
+
+	codeStr := parts[0]
+	typ := parts[1]
+	val := parts[2]
+
+	var code64 uint64
+	code64, err = strconv.ParseUint(codeStr, 10, 8)
+	if err != nil {
+		return 0, nil, fmt.Errorf("parsing option code: %w", err)
+	}
+
+	code = uint8(code64)
+
+	h, ok := p.handlers[typ]
+	if !ok {
+		return 0, nil, fmt.Errorf("unknown option type %q", typ)
+	}
+
+	data, err = h(val)
+	if err != nil {
+		return 0, nil, err
+	}
+
+	return uint8(code), data, nil
+}

internal/dhcpd/options_test.go

@@ -0,0 +1,109 @@
+package dhcpd
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDHCPOptionParser(t *testing.T) {
+	testCasesA := []struct {
+		name       string
+		in         string
+		wantErrMsg string
+		wantData   []byte
+		wantCode   uint8
+	}{{
+		name:       "hex_success",
+		in:         "6 hex c0a80101c0a80102",
+		wantErrMsg: "",
+		wantData:   []byte{0xC0, 0xA8, 0x01, 0x01, 0xC0, 0xA8, 0x01, 0x02},
+		wantCode:   6,
+	}, {
+		name:       "ip_success",
+		in:         "6 ip 1.2.3.4",
+		wantErrMsg: "",
+		wantData:   []byte{0x01, 0x02, 0x03, 0x04},
+		wantCode:   6,
+	}, {
+		name:       "ip_success_v6",
+		in:         "6 ip ::1234",
+		wantErrMsg: "",
+		wantData: []byte{
+			0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x12, 0x34,
+		},
+		wantCode: 6,
+	}, {
+		name:       "ips_success",
+		in:         "6 ips 192.168.1.1,192.168.1.2",
+		wantErrMsg: "",
+		wantData:   []byte{0xC0, 0xA8, 0x01, 0x01, 0xC0, 0xA8, 0x01, 0x02},
+		wantCode:   6,
+	}, {
+		name:       "text_success",
+		in:         "252 text http://192.168.1.1/",
+		wantErrMsg: "",
+		wantData:   []byte("http://192.168.1.1/"),
+		wantCode:   252,
+	}, {
+		name:       "bad_parts",
+		in:         "6 ip",
+		wantErrMsg: `invalid option string "6 ip": need at least three fields`,
+		wantCode:   0,
+		wantData:   nil,
+	}, {
+		name: "bad_code",
+		in:   "256 ip 1.1.1.1",
+		wantErrMsg: `invalid option string "256 ip 1.1.1.1": parsing option code: ` +
+			`strconv.ParseUint: parsing "256": value out of range`,
+		wantCode: 0,
+		wantData: nil,
+	}, {
+		name:       "bad_type",
+		in:         "6 bad 1.1.1.1",
+		wantErrMsg: `invalid option string "6 bad 1.1.1.1": unknown option type "bad"`,
+		wantCode:   0,
+		wantData:   nil,
+	}, {
+		name: "hex_error",
+		in:   "6 hex ZZZ",
+		wantErrMsg: `invalid option string "6 hex ZZZ": decoding hex: ` +
+			`encoding/hex: invalid byte: U+005A 'Z'`,
+		wantData: nil,
+		wantCode: 0,
+	}, {
+		name:       "ip_error",
+		in:         "6 ip 1.2.3.x",
+		wantErrMsg: `invalid option string "6 ip 1.2.3.x": invalid ip`,
+		wantData:   nil,
+		wantCode:   0,
+	}, {
+		name: "ips_error",
+		in:   "6 ips 192.168.1.1,192.168.1.x",
+		wantErrMsg: `invalid option string "6 ips 192.168.1.1,192.168.1.x": ` +
+			`parsing ip at index 1: invalid ip`,
+		wantData: nil,
+		wantCode: 0,
+	}}
+
+	p := newDHCPOptionParser()
+
+	for _, tc := range testCasesA {
+		t.Run(tc.name, func(t *testing.T) {
+			code, data, err := p.parse(tc.in)
+			if tc.wantErrMsg == "" {
+				assert.Nil(t, err)
+			} else {
+				require.NotNil(t, err)
+				assert.Equal(t, tc.wantErrMsg, err.Error())
+			}
+
+			assert.Equal(t, tc.wantCode, code)
+			assert.Equal(t, tc.wantData, data)
+		})
+	}
+}

internal/dhcpd/server.go

@@ -96,5 +96,5 @@ type V6ServerConf struct {
 
 type dhcpOption struct {
 	code uint8
-	val  []byte
+	data []byte
 }

internal/dhcpd/v4.go

@@ -521,7 +521,7 @@ func (s *v4Server) process(req, resp *dhcpv4.DHCPv4) int {
 	resp.UpdateOption(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 
 	for _, opt := range s.conf.options {
-		resp.Options[opt.code] = opt.val
+		resp.Options[opt.code] = opt.data
 	}
 	return 1
 }
@@ -631,7 +631,7 @@ func (s *v4Server) Stop() {
 }
 
 // Create DHCPv4 server
-func v4Create(conf V4ServerConf) (DHCPServer, error) {
+func v4Create(conf V4ServerConf) (srv DHCPServer, err error) {
 	s := &v4Server{}
 	s.conf = conf
 
@@ -639,7 +639,6 @@ func v4Create(conf V4ServerConf) (DHCPServer, error) {
 		return s, nil
 	}
 
-	var err error
 	s.conf.routerIP, err = tryTo4(s.conf.GatewayIP)
 	if err != nil {
 		return s, fmt.Errorf("dhcpv4: %w", err)
@@ -675,17 +674,23 @@ func v4Create(conf V4ServerConf) (DHCPServer, error) {
 		s.conf.leaseTime = time.Second * time.Duration(conf.LeaseDuration)
 	}
 
-	for _, o := range conf.Options {
+	p := newDHCPOptionParser()
-		code, val := parseOptionString(o)
+
-		if code == 0 {
+	for i, o := range conf.Options {
-			log.Debug("dhcpv4: bad option string: %s", o)
+		var code uint8
+		var data []byte
+		code, data, err = p.parse(o)
+		if err != nil {
+			log.Error("dhcpv4: bad option string at index %d: %s", i, err)
+
 			continue
 		}
 
 		opt := dhcpOption{
 			code: code,
-			val:  val,
+			data: data,
 		}
+
 		s.conf.options = append(s.conf.options, opt)
 	}