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] 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)
-}