fix: address remaining review issues from PR #477

- Remove custom wireguard.PrivateKey type; use wgtypes.Key everywhere
- Fix ReadOrCreatePrivateKey to write/read base64 format (not raw bytes)
- Fix double base64 encoding in enrollgateway.go
- Replace fragile manual base64+copy with wgtypes.ParseKey in EnsurePrivateKey
- Add strings.TrimSpace when reading key files to handle trailing newlines
- Fix error wrapping: %v -> %w for proper errors.Is/As support
- Make Configure retry loop context-aware (select on ctx.Done vs time.Sleep)
- Clean up Linux interface on partial SetupInterface failure
- Make IPv6 address configuration conditional on all platforms
- Change GatewayRequest.WireGuardPublicKey from []byte to string

Author: sechmann

cmd/apiserver/main.go

@@ -158,7 +158,7 @@ func run(log *logrus.Entry, cfg config.Config) error {
 		}
 		cfg.WireGuardPrivateKey = key
 
-		netConf, err := wg.NewConfigurer(log.WithField("component", "network-configurer"), cfg.WireGuardConfigPath, cfg.WireGuardIPv4Prefix, cfg.WireGuardIPv6Prefix, string(cfg.WireGuardPrivateKey.Private()), "wg0", 51820, nil, nil, nil)
+		netConf, err := wg.NewConfigurer(log.WithField("component", "network-configurer"), cfg.WireGuardConfigPath, cfg.WireGuardIPv4Prefix, cfg.WireGuardIPv6Prefix, cfg.WireGuardPrivateKey.String(), "wg0", 51820, nil, nil, nil)
 		if err != nil {
 			return fmt.Errorf("create WireGuard configurer: %w", err)
 		}

cmd/gateway-agent/main.go

@@ -79,7 +79,7 @@ func run(log *logrus.Entry, cfg config.Config) error {
 
 		ecfg, err := enroll.NewGatewayClient(
 			ctx,
-			privateKey.Public(),
+			privateKey.PublicKey().String(),
 			hashedPassword,
 			wireguardListenPort,
 			log.WithField("component", "bootstrap"),
@@ -96,7 +96,7 @@ func run(log *logrus.Entry, cfg config.Config) error {
 		}
 
 		cfg.Name = ecfg.Name
-		cfg.PrivateKey = string(privateKey.Private())
+		cfg.PrivateKey = privateKey.String()
 		cfg.APIServerURL = enrollResp.APIServerGRPCAddress
 		cfg.DeviceIPv4 = enrollResp.WireGuardIPv4
 

internal/apiserver/config/config.go

@@ -11,6 +11,7 @@ import (
 	"github.com/nais/device/internal/wireguard"
 	"github.com/nais/device/pkg/pb"
 	"github.com/sirupsen/logrus"
+	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 )
 
 type Config struct {
@@ -44,7 +45,7 @@ type Config struct {
 	WireGuardIP                       string // for passing in raw string
 	WireGuardIPv6                     string // for passing in raw string
 	WireGuardConfigPath               string
-	WireGuardPrivateKey               wireguard.PrivateKey
+	WireGuardPrivateKey               wgtypes.Key
 	WireGuardPrivateKeyPath           string
 	WireGuardNetworkAddress           string
 	WireGuardIPv4Prefix               *netip.Prefix `ignored:"true"`
@@ -141,7 +142,7 @@ func (cfg *Config) APIServerPeer() *pb.Gateway {
 
 	return &pb.Gateway{
 		Name:      "apiserver",
-		PublicKey: string(cfg.WireGuardPrivateKey.Public()),
+		PublicKey: cfg.WireGuardPrivateKey.PublicKey().String(),
 		Endpoint:  cfg.Endpoint,
 		Ipv4:      cfg.WireGuardIPv4Prefix.Addr().String(),
 		Ipv6:      ipv6,

internal/controlplane-cli/enrollgateway.go

@@ -8,9 +8,9 @@ import (
 	"os"
 
 	"github.com/nais/device/internal/passwordhash"
-	"github.com/nais/device/internal/wireguard"
 	"github.com/nais/device/pkg/pb"
 	"github.com/urfave/cli/v2"
+	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
 )
@@ -133,18 +133,18 @@ func EnrollGateway(c *cli.Context) error {
 	key := passwordhash.HashPassword([]byte(password), salt)
 	passhash := passwordhash.FormatHash(key, salt)
 
-	privateKey, err := wireguard.GenKey()
+	privateKey, err := wgtypes.GeneratePrivateKey()
 	if err != nil {
 		return fmt.Errorf("generating private key: %w", err)
 	}
-	publicKey := privateKey.Public()
+	publicKey := privateKey.PublicKey().String()
 
 	req := &pb.ModifyGatewayRequest{
 		Username: AdminUsername,
 		Password: c.String(FlagAdminPassword),
 		Gateway: &pb.Gateway{
 			Name:         c.String(FlagName),
-			PublicKey:    string(publicKey),
+			PublicKey:    publicKey,
 			Endpoint:     c.String(FlagEndpoint),
 			PasswordHash: string(passhash),
 		},
@@ -177,7 +177,7 @@ func EnrollGateway(c *cli.Context) error {
 
 	fmt.Printf("GATEWAY_AGENT_NAME=\"%s\"\n", response.GetGateway().GetName())
 	fmt.Printf("GATEWAY_AGENT_APISERVERPASSWORD=\"%s\"\n", password)
-	fmt.Printf("GATEWAY_AGENT_PRIVATEKEY=\"%s\"\n", base64.StdEncoding.EncodeToString(privateKey))
+	fmt.Printf("GATEWAY_AGENT_PRIVATEKEY=\"%s\"\n", privateKey.String())
 	fmt.Printf("GATEWAY_AGENT_DEVICEIP=\"%s/21\"\n", response.GetGateway().GetIpv4())
 
 	return err

internal/deviceagent/wireguard/wireguard.go

@@ -1,9 +1,9 @@
 package wireguard
 
 import (
-	"encoding/base64"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/nais/device/internal/deviceagent/filesystem"
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
@@ -25,15 +25,13 @@ func EnsurePrivateKey(keyPath string) (wgtypes.Key, error) {
 
 	privateKeyEncoded, err := os.ReadFile(keyPath)
 	if err != nil {
-		return wgtypes.Key{}, fmt.Errorf("reading private key: %v", err)
+		return wgtypes.Key{}, fmt.Errorf("reading private key: %w", err)
 	}
 
-	var key wgtypes.Key
-	b, err := base64.StdEncoding.DecodeString(string(privateKeyEncoded))
+	key, err := wgtypes.ParseKey(strings.TrimSpace(string(privateKeyEncoded)))
 	if err != nil {
-		return wgtypes.Key{}, fmt.Errorf("decoding private key: %v", err)
+		return wgtypes.Key{}, fmt.Errorf("parsing private key: %w", err)
 	}
-	copy(key[:], b)
 
 	return key, nil
 }

internal/enroll/enroll.go

@@ -10,7 +10,7 @@ type DeviceRequest struct {
 }
 
 type GatewayRequest struct {
-	WireGuardPublicKey []byte `json:"wireguard_public_key"`
+	WireGuardPublicKey string `json:"wireguard_public_key"`
 	Name               string `json:"name"`
 	Endpoint           string `json:"endpoint"`
 	HashedPassword     string `json:"hashed_password"`

internal/enroll/gateway.go

@@ -16,7 +16,7 @@ import (
 )
 
 type GatewayClient struct {
-	wireGuardPublicKey []byte
+	wireGuardPublicKey string
 	port               int
 	hashedPassword     string
 	log                logrus.FieldLogger
@@ -28,7 +28,7 @@ type GatewayClient struct {
 	ExternalIP       string `json:"external_ip"`
 }
 
-func NewGatewayClient(ctx context.Context, publicKey []byte, hashedPassword string, wireguardListenPort int, log logrus.FieldLogger) (*GatewayClient, error) {
+func NewGatewayClient(ctx context.Context, publicKey string, hashedPassword string, wireguardListenPort int, log logrus.FieldLogger) (*GatewayClient, error) {
 	b, err := GetGoogleMetadata(ctx, "instance/attributes/enroll-config", log)
 	if err != nil {
 		return nil, err

internal/helper/helper.go

@@ -76,7 +76,11 @@ func (dhs *DeviceHelperServer) Configure(
 			backoff := time.Duration(attempt) * time.Second
 			dhs.log.WithError(loopErr).Error("synchronize WireGuard configuration")
 			dhs.log.WithField("attempt", attempt+1).WithField("backoff", backoff).Info("configuring failed, sleeping before retrying")
-			time.Sleep(backoff)
+			select {
+			case <-ctx.Done():
+				return nil, status.Errorf(codes.Canceled, "context canceled during WireGuard sync retry: %s", loopErr)
+			case <-time.After(backoff):
+			}
 			continue
 		}
 		break

internal/helper/helper_darwin.go

@@ -138,9 +138,11 @@ func (c *DarwinConfigurator) SetupInterface(ctx context.Context, cfg *pb.Configu
 	// tool used by the macOS networking stack itself.
 	commands := [][]string{
 		{"ifconfig", ifaceName, "inet", cfg.GetDeviceIPv4() + "/21", cfg.GetDeviceIPv4(), "alias"},
-		{"ifconfig", ifaceName, "inet6", cfg.GetDeviceIPv6() + "/64", "alias"},
-		{"ifconfig", ifaceName, "up"},
 	}
+	if cfg.GetDeviceIPv6() != "" {
+		commands = append(commands, []string{"ifconfig", ifaceName, "inet6", cfg.GetDeviceIPv6() + "/64", "alias"})
+	}
+	commands = append(commands, []string{"ifconfig", ifaceName, "up"})
 
 	for _, s := range commands {
 		cmd := exec.CommandContext(ctx, s[0], s[1:]...)

internal/helper/helper_linux.go

@@ -92,24 +92,34 @@ func (c *LinuxConfigurator) SetupInterface(ctx context.Context, cfg *pb.Configur
 		return fmt.Errorf("lookup interface after creation: %w", err)
 	}
 
+	// cleanup deletes the interface if any subsequent configuration step fails.
+	cleanup := func(cause error) error {
+		if delErr := netlink.LinkDel(link); delErr != nil {
+			return fmt.Errorf("%w (additionally, failed to delete interface: %v)", cause, delErr)
+		}
+		return cause
+	}
+
 	ipv4Addr, err := netlink.ParseAddr(cfg.DeviceIPv4 + "/21")
 	if err != nil {
-		return fmt.Errorf("parse IPv4 address: %w", err)
+		return cleanup(fmt.Errorf("parse IPv4 address: %w", err))
 	}
 	if err := netlink.AddrAdd(link, ipv4Addr); err != nil {
-		return fmt.Errorf("add IPv4 address: %w", err)
+		return cleanup(fmt.Errorf("add IPv4 address: %w", err))
 	}
 
-	ipv6Addr, err := netlink.ParseAddr(cfg.DeviceIPv6 + "/64")
-	if err != nil {
-		return fmt.Errorf("parse IPv6 address: %w", err)
-	}
-	if err := netlink.AddrAdd(link, ipv6Addr); err != nil {
-		return fmt.Errorf("add IPv6 address: %w", err)
+	if cfg.DeviceIPv6 != "" {
+		ipv6Addr, err := netlink.ParseAddr(cfg.DeviceIPv6 + "/64")
+		if err != nil {
+			return cleanup(fmt.Errorf("parse IPv6 address: %w", err))
+		}
+		if err := netlink.AddrAdd(link, ipv6Addr); err != nil {
+			return cleanup(fmt.Errorf("add IPv6 address: %w", err))
+		}
 	}
 
 	if err := netlink.LinkSetUp(link); err != nil {
-		return fmt.Errorf("bring interface up: %w", err)
+		return cleanup(fmt.Errorf("bring interface up: %w", err))
 	}
 
 	return nil