From b1657c2b2a89e0b6c8d815f958f472ac5fae7451 Mon Sep 17 00:00:00 2001
From: Gunjan Gupta <viraniac@gmail.com>
Date: Fri, 3 Jan 2025 20:52:31 +0530
Subject: [PATCH 1/6] dhcpd: Fix slaac configuration which was not working due
 to invalid icmp packet

Signed-off-by: Gunjan Gupta <viraniac@gmail.com>
---
 internal/dhcpd/routeradv.go      | 23 ++++++++++-------------
 internal/dhcpd/routeradv_test.go |  7 +++----
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/internal/dhcpd/routeradv.go b/internal/dhcpd/routeradv.go
index 2b480b46..32b7e6dc 100644
--- a/internal/dhcpd/routeradv.go
+++ b/internal/dhcpd/routeradv.go
@@ -50,16 +50,7 @@ func hwAddrToLinkLayerAddr(hwa net.HardwareAddr) (lla []byte, err error) {
 		return nil, err
 	}
 
-	if len(hwa) == 6 || len(hwa) == 8 {
-		lla = make([]byte, 8)
-		copy(lla, hwa)
-
-		return lla, nil
-	}
-
-	// Assume that netutil.ValidateMAC prevents lengths other than 20 by
-	// now.
-	lla = make([]byte, 24)
+	lla = make([]byte, len(hwa))
 	copy(lla, hwa)
 
 	return lla, nil
@@ -109,9 +100,15 @@ func createICMPv6RAPacket(params icmpv6RA) (data []byte, err error) {
 		return nil, fmt.Errorf("converting source link layer address: %w", err)
 	}
 
+	// Calculate length of source link layer address section
+	// This is based on calculation from radvd's add_ra_option_sllao function
+	sllao_bytes := len(lla) + 2
+	sllao_len := (sllao_bytes + 7) / 8
+	sllao_pad_bytes := sllao_len * 8 - sllao_bytes
+
 	// TODO(a.garipov): Don't use a magic constant here.  Refactor the code
 	// and make all constants named instead of all those comments..
-	data = make([]byte, 82+len(lla))
+	data = make([]byte, 80+sllao_bytes+sllao_pad_bytes)
 	i := 0
 
 	// ICMPv6:
@@ -176,11 +173,11 @@ func createICMPv6RAPacket(params icmpv6RA) (data []byte, err error) {
 	// Option=Source link-layer address:
 
 	data[i] = 1   // Type
-	data[i+1] = 1 // Length
+	data[i+1] = byte(sllao_len) // Length
 	i += 2
 
 	copy(data[i:], lla) // Link-Layer Address[8/24]
-	i += len(lla)
+	i += len(lla) + sllao_pad_bytes
 
 	// Option=Recursive DNS Server:
 
diff --git a/internal/dhcpd/routeradv_test.go b/internal/dhcpd/routeradv_test.go
index 94f7afd3..e007d460 100644
--- a/internal/dhcpd/routeradv_test.go
+++ b/internal/dhcpd/routeradv_test.go
@@ -17,10 +17,9 @@ func TestCreateICMPv6RAPacket(t *testing.T) {
 		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 		0x05, 0x01, 0x00, 0x00, 0x00, 0x00, 0x05, 0xdc,
 		0x01, 0x01, 0x0a, 0x00, 0x27, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x19, 0x03, 0x00, 0x00, 0x00, 0x00,
-		0x0e, 0x10, 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x08, 0x00, 0x27, 0xff, 0xfe, 0x00,
-		0x00, 0x00,
+		0x19, 0x03, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x10,
+		0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x08, 0x00, 0x27, 0xff, 0xfe, 0x00, 0x00, 0x00,
 	}
 
 	gotData, err := createICMPv6RAPacket(icmpv6RA{

From 0c72cde4c3f0f6ca076c96d5941b4971349642a1 Mon Sep 17 00:00:00 2001
From: Gunjan Gupta <viraniac@gmail.com>
Date: Sat, 4 Jan 2025 00:21:49 +0530
Subject: [PATCH 2/6] Change comment message to match implementation

Signed-off-by: Gunjan Gupta <viraniac@gmail.com>
---
 internal/dhcpd/routeradv.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/internal/dhcpd/routeradv.go b/internal/dhcpd/routeradv.go
index 32b7e6dc..b2376f70 100644
--- a/internal/dhcpd/routeradv.go
+++ b/internal/dhcpd/routeradv.go
@@ -15,8 +15,8 @@ import (
 )
 
 type raCtx struct {
-	raAllowSLAAC     bool   // send RA packets without MO flags
-	raSLAACOnly      bool   // send RA packets with MO flags
+	raAllowSLAAC     bool   // send RA packets with MO flags
+	raSLAACOnly      bool   // send RA packets without MO flags
 	ipAddr           net.IP // source IP address (link-local-unicast)
 	dnsIPAddr        net.IP // IP address for DNS Server option
 	prefixIPAddr     net.IP // IP address for Prefix option

From 74640fb06c466e4d0352fc1046b4e2e52ab4b845 Mon Sep 17 00:00:00 2001
From: Eugene Burkov <E.Burkov@AdGuard.COM>
Date: Thu, 6 Mar 2025 15:15:06 +0300
Subject: [PATCH 3/6] all: imp code & test, log changes

---
 CHANGELOG.md                              |  2 +
 internal/dhcpd/routeradv.go               | 31 ++++++-----
 internal/dhcpd/routeradv_internal_test.go | 63 +++++++++++++++++++++++
 internal/dhcpd/routeradv_test.go          | 37 -------------
 4 files changed, 80 insertions(+), 53 deletions(-)
 create mode 100644 internal/dhcpd/routeradv_internal_test.go
 delete mode 100644 internal/dhcpd/routeradv_test.go

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf02db7b..3c5a8945 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,9 +28,11 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- Invalid ICMPv6 Router Advertisement messages ([#7547]).
 - The formatting of large numbers on the dashboard ([#7329]).
 
 [#7329]: https://github.com/AdguardTeam/AdGuardHome/issues/7329
+[#7547]: https://github.com/AdguardTeam/AdGuardHome/issues/7547
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.
diff --git a/internal/dhcpd/routeradv.go b/internal/dhcpd/routeradv.go
index b2376f70..cdda0f42 100644
--- a/internal/dhcpd/routeradv.go
+++ b/internal/dhcpd/routeradv.go
@@ -4,6 +4,7 @@ import (
 	"encoding/binary"
 	"fmt"
 	"net"
+	"slices"
 	"sync/atomic"
 	"time"
 
@@ -14,6 +15,9 @@ import (
 	"golang.org/x/net/ipv6"
 )
 
+// raCtx is a context for the Router Advertisement logic.
+//
+// TODO(e.burkov):  !! Improve docs
 type raCtx struct {
 	raAllowSLAAC     bool   // send RA packets with MO flags
 	raSLAACOnly      bool   // send RA packets without MO flags
@@ -50,10 +54,8 @@ func hwAddrToLinkLayerAddr(hwa net.HardwareAddr) (lla []byte, err error) {
 		return nil, err
 	}
 
-	lla = make([]byte, len(hwa))
-	copy(lla, hwa)
-
-	return lla, nil
+	// TODO(e.burkov):  Check if it's safe to use the original slice.
+	return slices.Clone(hwa), nil
 }
 
 // Create an ICMPv6.RouterAdvertisement packet with all necessary options.
@@ -94,21 +96,19 @@ func hwAddrToLinkLayerAddr(hwa net.HardwareAddr) (lla []byte, err error) {
 //
 // TODO(a.garipov): Replace with an existing implementation from a dependency.
 func createICMPv6RAPacket(params icmpv6RA) (data []byte, err error) {
-	var lla []byte
-	lla, err = hwAddrToLinkLayerAddr(params.sourceLinkLayerAddress)
+	lla, err := hwAddrToLinkLayerAddr(params.sourceLinkLayerAddress)
 	if err != nil {
 		return nil, fmt.Errorf("converting source link layer address: %w", err)
 	}
 
-	// Calculate length of source link layer address section
-	// This is based on calculation from radvd's add_ra_option_sllao function
-	sllao_bytes := len(lla) + 2
-	sllao_len := (sllao_bytes + 7) / 8
-	sllao_pad_bytes := sllao_len * 8 - sllao_bytes
+	// Calculate length of the source link-layer address option.
+	srcLLAOptLen := len(lla) + 2
+	srcLLAOptLenValue := (srcLLAOptLen + 7) / 8
+	srcLLAPadLen := srcLLAOptLenValue*8 - srcLLAOptLen
 
 	// TODO(a.garipov): Don't use a magic constant here.  Refactor the code
 	// and make all constants named instead of all those comments..
-	data = make([]byte, 80+sllao_bytes+sllao_pad_bytes)
+	data = make([]byte, 80+srcLLAOptLen+srcLLAPadLen)
 	i := 0
 
 	// ICMPv6:
@@ -172,12 +172,11 @@ func createICMPv6RAPacket(params icmpv6RA) (data []byte, err error) {
 
 	// Option=Source link-layer address:
 
-	data[i] = 1   // Type
-	data[i+1] = byte(sllao_len) // Length
+	data[i] = 1                         // Type
+	data[i+1] = byte(srcLLAOptLenValue) // Length
 	i += 2
-
 	copy(data[i:], lla) // Link-Layer Address[8/24]
-	i += len(lla) + sllao_pad_bytes
+	i += len(lla) + srcLLAPadLen
 
 	// Option=Recursive DNS Server:
 
diff --git a/internal/dhcpd/routeradv_internal_test.go b/internal/dhcpd/routeradv_internal_test.go
new file mode 100644
index 00000000..c876a67a
--- /dev/null
+++ b/internal/dhcpd/routeradv_internal_test.go
@@ -0,0 +1,63 @@
+package dhcpd
+
+import (
+	"net"
+	"testing"
+
+	"github.com/google/gopacket"
+	"github.com/google/gopacket/layers"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCreateICMPv6RAPacket(t *testing.T) {
+	raConf := icmpv6RA{
+		managedAddressConfiguration: false,
+		otherConfiguration:          true,
+		mtu:                         1500,
+		prefix:                      net.ParseIP("1234::"),
+		prefixLen:                   64,
+		recursiveDNSServer:          net.ParseIP("fe80::800:27ff:fe00:0"),
+		sourceLinkLayerAddress:      []byte{0x0A, 0x00, 0x27, 0x00, 0x00, 0x00},
+	}
+
+	pkt, err := createICMPv6RAPacket(raConf)
+	require.NoError(t, err)
+
+	icmpPkt := &layers.ICMPv6{}
+	err = icmpPkt.DecodeFromBytes(pkt, gopacket.NilDecodeFeedback)
+	require.NoError(t, err)
+
+	require.Equal(t, layers.LayerTypeICMPv6RouterAdvertisement, icmpPkt.NextLayerType())
+	raPkt := &layers.ICMPv6RouterAdvertisement{}
+	err = raPkt.DecodeFromBytes(icmpPkt.LayerPayload(), gopacket.NilDecodeFeedback)
+	require.NoError(t, err)
+
+	assert.Equal(t, raConf.managedAddressConfiguration, raPkt.ManagedAddressConfig())
+	assert.Equal(t, raConf.otherConfiguration, raPkt.OtherConfig())
+
+	wantOpts := layers.ICMPv6Options{{
+		Type: layers.ICMPv6OptPrefixInfo,
+		Data: []uint8{
+			0x40, 0xC0, 0x00, 0x00, 0x0E, 0x10, 0x00, 0x00,
+			0x0E, 0x10, 0x00, 0x00, 0x00, 0x00, 0x12, 0x34,
+			0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		},
+	}, {
+		Type: layers.ICMPv6OptMTU,
+		Data: []uint8{0x00, 0x00, 0x00, 0x00, 0x05, 0xDC},
+	}, {
+		Type: layers.ICMPv6OptSourceAddress,
+		Data: []uint8{0x0A, 0x00, 0x27, 0x00, 0x00, 0x0},
+	}, {
+		// Package layers declares no constant for Recursive DNS Server option.
+		Type: layers.ICMPv6Opt(25),
+		Data: []uint8{
+			0x00, 0x00, 0x00, 0x00, 0x0E, 0x10, 0xFE, 0x80,
+			0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00,
+			0x27, 0xFF, 0xFE, 0x00, 0x00, 0x00,
+		},
+	}}
+	assert.Equal(t, wantOpts, raPkt.Options)
+}
diff --git a/internal/dhcpd/routeradv_test.go b/internal/dhcpd/routeradv_test.go
deleted file mode 100644
index e007d460..00000000
--- a/internal/dhcpd/routeradv_test.go
+++ /dev/null
@@ -1,37 +0,0 @@
-package dhcpd
-
-import (
-	"net"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestCreateICMPv6RAPacket(t *testing.T) {
-	wantData := []byte{
-		0x86, 0x00, 0x00, 0x00, 0x40, 0x40, 0x07, 0x08,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x03, 0x04, 0x40, 0xc0, 0x00, 0x00, 0x0e, 0x10,
-		0x00, 0x00, 0x0e, 0x10, 0x00, 0x00, 0x00, 0x00,
-		0x12, 0x34, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x05, 0x01, 0x00, 0x00, 0x00, 0x00, 0x05, 0xdc,
-		0x01, 0x01, 0x0a, 0x00, 0x27, 0x00, 0x00, 0x00,
-		0x19, 0x03, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x10,
-		0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x08, 0x00, 0x27, 0xff, 0xfe, 0x00, 0x00, 0x00,
-	}
-
-	gotData, err := createICMPv6RAPacket(icmpv6RA{
-		managedAddressConfiguration: false,
-		otherConfiguration:          true,
-		mtu:                         1500,
-		prefix:                      net.ParseIP("1234::"),
-		prefixLen:                   64,
-		recursiveDNSServer:          net.ParseIP("fe80::800:27ff:fe00:0"),
-		sourceLinkLayerAddress:      []byte{0x0a, 0x00, 0x27, 0x00, 0x00, 0x00},
-	})
-
-	assert.NoError(t, err)
-	assert.Equal(t, wantData, gotData)
-}

From ed947a048e4aa98d183bf2b43f09b338a967d231 Mon Sep 17 00:00:00 2001
From: Eugene Burkov <E.Burkov@AdGuard.COM>
Date: Thu, 6 Mar 2025 15:54:02 +0300
Subject: [PATCH 4/6] dhcpd: imp docs

---
 internal/dhcpd/routeradv.go | 51 ++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/internal/dhcpd/routeradv.go b/internal/dhcpd/routeradv.go
index cdda0f42..1ae1922b 100644
--- a/internal/dhcpd/routeradv.go
+++ b/internal/dhcpd/routeradv.go
@@ -16,20 +16,47 @@ import (
 )
 
 // raCtx is a context for the Router Advertisement logic.
-//
-// TODO(e.burkov):  !! Improve docs
 type raCtx struct {
-	raAllowSLAAC     bool   // send RA packets with MO flags
-	raSLAACOnly      bool   // send RA packets without MO flags
-	ipAddr           net.IP // source IP address (link-local-unicast)
-	dnsIPAddr        net.IP // IP address for DNS Server option
-	prefixIPAddr     net.IP // IP address for Prefix option
-	ifaceName        string
-	iface            *net.Interface
-	packetSendPeriod time.Duration // how often RA packets are sent
+	// raAllowSLAAC is used to determine if the ICMP Router Advertisement
+	// messages should be sent.
+	//
+	// If both raAllowSLAAC and raSLAACOnly are false, the Router Advertisement
+	// messages aren't sent.
+	raAllowSLAAC bool
 
-	conn *icmp.PacketConn // ICMPv6 socket
-	stop atomic.Value     // stop the packet sending loop
+	// raSLAACOnly is used to determine if the ICMP Router Advertisement
+	// messages should set M and O flags, see RFC 4861, section 4.2.
+	//
+	// If both raAllowSLAAC and raSLAACOnly are false, the Router Advertisement
+	// messages aren't sent.
+	raSLAACOnly bool
+
+	// ipAddr is an IP address used within the Source Link-Layer Address option.
+	// See RFC 4861, section 4.6.1.
+	ipAddr net.IP
+
+	// dnsIPAddr is an IP address used within the DNS Server option.
+	dnsIPAddr net.IP
+
+	// prefixIPAddr is an IP address used within the Prefix Information option.
+	// See RFC 4861, section 4.6.2.
+	prefixIPAddr net.IP
+
+	// ifaceName is the name of the interface used as a scope of the IP
+	// addresses.
+	ifaceName string
+
+	// iface is the network interface used to send the ICMPv6 packets.
+	iface *net.Interface
+
+	// packetSendPeriod is the interval between sending the ICMPv6 packets.
+	packetSendPeriod time.Duration
+
+	// conn is the ICMPv6 socket.
+	conn *icmp.PacketConn
+
+	// stop is used to stop the packet sending loop.
+	stop atomic.Value
 }
 
 type icmpv6RA struct {

From 777b310a4be55f6946fae03abdec3bbfb8319681 Mon Sep 17 00:00:00 2001
From: Eugene Burkov <E.Burkov@AdGuard.COM>
Date: Thu, 6 Mar 2025 16:12:10 +0300
Subject: [PATCH 5/6] all: fix changelog

---
 CHANGELOG.md | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f2cb08ff..d7043103 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,9 +20,13 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- Invalid ICMPv6 Router Advertisement messages ([#7547]).
+
 - Changes to global upstream DNS settings not applying to custom client upstream configurations.
+
 - The formatting of large numbers in the clients tables on the *Client settings* page ([#7583]).
 
+[#7547]: https://github.com/AdguardTeam/AdGuardHome/issues/7547
 [#7583]: https://github.com/AdguardTeam/AdGuardHome/issues/7583
 
 <!--
@@ -48,6 +52,7 @@ See also the [v0.107.57 GitHub milestone][ms-v0.107.57].
 ### Fixed
 
 - The hostnames of DHCP clients not being shown in the *Top clients* table on the dashboard ([#7627]).
+
 - The formatting of large numbers in the upstream table and query log ([#7590]).
 
 [#7590]: https://github.com/AdguardTeam/AdGuardHome/issues/7590
@@ -74,15 +79,12 @@ See also the [v0.107.56 GitHub milestone][ms-v0.107.56].
 
 ### Fixed
 
-- Invalid ICMPv6 Router Advertisement messages ([#7547]).
-
 - Request count link in the clients table ([#7513]).
 
 - The formatting of large numbers on the dashboard ([#7329]).
 
 [#7329]: https://github.com/AdguardTeam/AdGuardHome/issues/7329
 [#7513]: https://github.com/AdguardTeam/AdGuardHome/issues/7513
-[#7547]: https://github.com/AdguardTeam/AdGuardHome/issues/7547
 
 [go-1.23.5]: https://groups.google.com/g/golang-announce/c/sSaUhLA-2SI
 [ms-v0.107.56]: https://github.com/AdguardTeam/AdGuardHome/milestone/91?closed=1

From 4b0db6d397f0f7d5de06d71b0452a38e369e32c8 Mon Sep 17 00:00:00 2001
From: Eugene Burkov <E.Burkov@AdGuard.COM>
Date: Fri, 7 Mar 2025 13:01:34 +0300
Subject: [PATCH 6/6] dhcpd: imp docs

---
 internal/dhcpd/routeradv.go | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/internal/dhcpd/routeradv.go b/internal/dhcpd/routeradv.go
index 1ae1922b..5aad4901 100644
--- a/internal/dhcpd/routeradv.go
+++ b/internal/dhcpd/routeradv.go
@@ -69,10 +69,11 @@ type icmpv6RA struct {
 	mtu                         uint32
 }
 
-// hwAddrToLinkLayerAddr converts a hardware address into a form required by
-// RFC4861.  That is, a byte slice of length divisible by 8.
+// hwAddrToLinkLayerAddr clones the hardware address and returns it as a byte
+// slice suitable for the Source Link-Layer Address option in the ICMPv6
+// Router Advertisement packet.
 //
-// See https://tools.ietf.org/html/rfc4861#section-4.6.1.
+// TODO(e.burkov):  Check if it's safe to use the original slice.
 func hwAddrToLinkLayerAddr(hwa net.HardwareAddr) (lla []byte, err error) {
 	err = netutil.ValidateMAC(hwa)
 	if err != nil {
@@ -81,7 +82,6 @@ func hwAddrToLinkLayerAddr(hwa net.HardwareAddr) (lla []byte, err error) {
 		return nil, err
 	}
 
-	// TODO(e.burkov):  Check if it's safe to use the original slice.
 	return slices.Clone(hwa), nil
 }
 
@@ -125,16 +125,21 @@ func hwAddrToLinkLayerAddr(hwa net.HardwareAddr) (lla []byte, err error) {
 func createICMPv6RAPacket(params icmpv6RA) (data []byte, err error) {
 	lla, err := hwAddrToLinkLayerAddr(params.sourceLinkLayerAddress)
 	if err != nil {
-		return nil, fmt.Errorf("converting source link layer address: %w", err)
+		return nil, fmt.Errorf("converting source link-layer address: %w", err)
 	}
 
-	// Calculate length of the source link-layer address option.
+	// Calculate length of the source link-layer address option.  As per RFC
+	// 4861, section 4.6.1, the length should be in units of 8 octets, including
+	// the type and length fields.
+	//
+	// See https://datatracker.ietf.org/doc/html/rfc4861#section-4.6.1.
 	srcLLAOptLen := len(lla) + 2
+	// Make sure the value is rounded up to the nearest multiple of 8.
 	srcLLAOptLenValue := (srcLLAOptLen + 7) / 8
 	srcLLAPadLen := srcLLAOptLenValue*8 - srcLLAOptLen
 
 	// TODO(a.garipov): Don't use a magic constant here.  Refactor the code
-	// and make all constants named instead of all those comments..
+	// and make all constants named instead of all those comments.
 	data = make([]byte, 80+srcLLAOptLen+srcLLAPadLen)
 	i := 0