Add per-server Interface/KeyPair edit hardening and OPNsense config import

Redact the private key from the /servers/:id/keypair response body - the UI never rendered it, but the raw key was still returned over the wire (json:"private_key,omitempty" plus explicit clearing before the JSON response). Add a new import flow: an admin can upload an OPNsense config.xml, preview the WireGuard servers/clients it defines (editable before committing), and confirm to create the corresponding Server/ServerSetting/Client records. Nothing is auto-applied - no wg-quick/systemctl call happens, matching the existing manual "Apply" step for regular server management. Schema verified against OPNsense core (WireGuard has been in core since 22.1, not a plugin) - see opnsense/parse.go for the confirmed tag reference. Public keys are always re-derived from private keys rather than trusted from the export; client public-key collisions against existing store data are skipped and reported per-batch rather than aborting the whole import. Since OPNsense stores DNS/MTU per-server and keepalive per-client, but this fork only had those app-wide (GlobalSetting), extended ServerSetting with DNSServers/MTU and Client with PersistentKeepalive as optional overrides that fall back to the global default when unset - existing single-server behavior is unchanged when the override is empty/zero. Manual UI editing of the per-client keepalive override outside the import flow is left for a later pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019VjwLYRA87o8m9a9zztgs3
2026-07-24 00:05:08 +02:00
parent 0dbb916866
commit 388a8377cd
10 changed files with 943 additions and 31 deletions
@@ -0,0 +1,373 @@
+package opnsense
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+	"time"
+
+	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
+
+	"github.com/ngoduykhanh/wireguard-ui/model"
+	"github.com/ngoduykhanh/wireguard-ui/util"
+)
+
+// PreviewClient is a single OPNsense client mapped into an admin-editable
+// staging structure. It is not a model.Client yet - ToModel performs the
+// final conversion at commit time.
+type PreviewClient struct {
+	SourceUUID          string   `json:"source_uuid,omitempty"`
+	Name                string   `json:"name"`
+	Email               string   `json:"email,omitempty"`
+	Enabled             bool     `json:"enabled"`
+	PublicKey           string   `json:"public_key"`
+	PresharedKey        string   `json:"preshared_key,omitempty"`
+	AllocatedIPs        []string `json:"allocated_ips"`
+	AllowedIPs          []string `json:"allowed_ips"`
+	PersistentKeepalive int      `json:"persistent_keepalive,omitempty"`
+	AdditionalNotes     string   `json:"additional_notes,omitempty"`
+	Warnings            []string `json:"warnings,omitempty"`
+}
+
+// PreviewServer is a single OPNsense WireGuard server instance mapped into
+// an admin-editable staging structure, together with the clients OPNsense
+// associated with it via its <peers> uuid list.
+type PreviewServer struct {
+	SourceUUID      string          `json:"source_uuid,omitempty"`
+	ID              string          `json:"id"`
+	Name            string          `json:"name"`
+	Interface       string          `json:"interface"`
+	Addresses       []string        `json:"addresses"`
+	ListenPort      int             `json:"listen_port"`
+	PrivateKey      string          `json:"private_key,omitempty"`
+	PublicKey       string          `json:"public_key,omitempty"`
+	EndpointAddress string          `json:"endpoint_address,omitempty"`
+	DNSServers      []string        `json:"dns_servers,omitempty"`
+	MTU             int             `json:"mtu,omitempty"`
+	Enabled         bool            `json:"enabled"`
+	Clients         []PreviewClient `json:"clients"`
+	Warnings        []string        `json:"warnings,omitempty"`
+}
+
+// PreviewResult is the full response of the preview step: every server
+// OPNsense defined, mapped and ready for the admin to review/edit before
+// confirming the import. It is never written to the store by itself.
+type PreviewResult struct {
+	Servers []PreviewServer `json:"servers"`
+}
+
+var slugInvalidChars = regexp.MustCompile(`[^a-zA-Z0-9_-]+`)
+
+// slugify turns a free-text name into a safe record-id/interface-name
+// candidate (see util.ValidateRecordID / util.ValidateInterfaceName).
+func slugify(name string) string {
+	s := slugInvalidChars.ReplaceAllString(strings.TrimSpace(name), "-")
+	s = strings.Trim(s, "-_")
+	if s == "" {
+		s = "opnsense-server"
+	}
+	return s
+}
+
+// uniqueID returns slug, or slug-2, slug-3, ... until it no longer collides
+// with existingIDs or anything already produced in this same batch (used
+// so importing several OPNsense servers whose names collide with each
+// other, or with already-existing wireguard-ui-multi servers, still
+// produces distinct IDs).
+func uniqueID(slug string, existingIDs map[string]bool) string {
+	candidate := slug
+	if !existingIDs[candidate] {
+		existingIDs[candidate] = true
+		return candidate
+	}
+	for i := 2; ; i++ {
+		candidate = fmt.Sprintf("%s-%d", slug, i)
+		if !existingIDs[candidate] {
+			existingIDs[candidate] = true
+			return candidate
+		}
+	}
+}
+
+// truncateInterfaceName shortens a candidate interface name to fit
+// util.ValidateInterfaceName's 15-char limit while staying unique within
+// this batch, reusing the numeric suffix strategy from uniqueID.
+func truncateInterfaceName(slug string, existingNames map[string]bool) string {
+	const maxLen = 15
+	base := slug
+	if len(base) > maxLen {
+		base = base[:maxLen]
+	}
+	if !existingNames[base] && util.ValidateInterfaceName(base) {
+		existingNames[base] = true
+		return base
+	}
+	for i := 2; ; i++ {
+		suffix := fmt.Sprintf("-%d", i)
+		cut := maxLen - len(suffix)
+		if cut < 1 {
+			cut = 1
+		}
+		truncated := slug
+		if len(truncated) > cut {
+			truncated = truncated[:cut]
+		}
+		candidate := truncated + suffix
+		if !existingNames[candidate] && util.ValidateInterfaceName(candidate) {
+			existingNames[candidate] = true
+			return candidate
+		}
+	}
+}
+
+// ToPreview maps a ParsedConfig into admin-editable PreviewServer/
+// PreviewClient structures. existingServerIDs should list every server ID
+// already present in the store (see store.IStore.GetServers), so imported
+// IDs never collide with them.
+func ToPreview(cfg *ParsedConfig, existingServerIDs []string) *PreviewResult {
+	usedIDs := make(map[string]bool, len(existingServerIDs))
+	for _, id := range existingServerIDs {
+		usedIDs[id] = true
+	}
+	usedIfaceNames := make(map[string]bool)
+
+	// index clients by their OPNsense uuid so each server's <peers> list
+	// (the only place the client<->server association is recorded) can
+	// pull in the right ones.
+	clientsByUUID := make(map[string]rawClient, len(cfg.Clients))
+	for _, c := range cfg.Clients {
+		clientsByUUID[c.UUID] = c
+	}
+
+	result := &PreviewResult{}
+
+	for _, rs := range cfg.Servers {
+		ps := PreviewServer{
+			SourceUUID: rs.UUID,
+			Name:       rs.Name,
+			Enabled:    isTruthy(rs.Enabled),
+			DNSServers: splitList(rs.DNS),
+		}
+		if ps.Name == "" {
+			ps.Name = fmt.Sprintf("opnsense-server-%s", rs.Instance)
+		}
+
+		slug := slugify(ps.Name)
+		ps.ID = uniqueID(slug, usedIDs)
+		ps.Interface = truncateInterfaceName(slug, usedIfaceNames)
+
+		ps.Addresses = splitList(rs.TunnelAddress)
+		if len(ps.Addresses) == 0 {
+			ps.Warnings = append(ps.Warnings, "no tunnel address found; please set the server's address range manually")
+		} else if !util.ValidateServerAddresses(ps.Addresses) {
+			ps.Warnings = append(ps.Warnings, "tunnel address is not valid CIDR; please fix before importing")
+		}
+
+		if rs.Port != "" {
+			if port, err := strconv.Atoi(strings.TrimSpace(rs.Port)); err == nil && port > 0 && port <= 65535 {
+				ps.ListenPort = port
+			} else {
+				ps.Warnings = append(ps.Warnings, fmt.Sprintf("invalid listen port %q; please set one manually", rs.Port))
+			}
+		} else {
+			ps.Warnings = append(ps.Warnings, "no listen port found; please set one manually")
+		}
+
+		if rs.MTU != "" {
+			if mtu, err := strconv.Atoi(strings.TrimSpace(rs.MTU)); err == nil && mtu > 0 {
+				ps.MTU = mtu
+			}
+		}
+
+		if rs.PrivKey == "" {
+			ps.Warnings = append(ps.Warnings, "no private key found; a server cannot be imported without one")
+		} else {
+			ps.PrivateKey = rs.PrivKey
+			key, err := wgtypes.ParseKey(rs.PrivKey)
+			if err != nil {
+				ps.Warnings = append(ps.Warnings, fmt.Sprintf("private key does not parse as a valid WireGuard key: %v", err))
+				ps.PrivateKey = ""
+			} else {
+				// Always derive the public key from the private key rather
+				// than trusting the exported <pubkey> blindly.
+				ps.PublicKey = key.PublicKey().String()
+			}
+		}
+
+		peerUUIDs := splitList(rs.Peers)
+		ps.EndpointAddress = firstServerAddress(cfg.Clients, peerUUIDs)
+
+		for _, peerUUID := range peerUUIDs {
+			rc, ok := clientsByUUID[peerUUID]
+			if !ok {
+				ps.Warnings = append(ps.Warnings, fmt.Sprintf("referenced peer %q was not found among the parsed clients", peerUUID))
+				continue
+			}
+			ps.Clients = append(ps.Clients, mapClient(rc))
+		}
+
+		result.Servers = append(result.Servers, ps)
+	}
+
+	return result
+}
+
+func mapClient(rc rawClient) PreviewClient {
+	pc := PreviewClient{
+		SourceUUID:      rc.UUID,
+		Name:            rc.Name,
+		Enabled:         isTruthy(rc.Enabled),
+		PublicKey:       rc.PubKey,
+		PresharedKey:    rc.PSK,
+		AdditionalNotes: "Imported from OPNsense",
+	}
+	if pc.Name == "" {
+		pc.Name = rc.UUID
+	}
+	if rc.PubKey == "" {
+		pc.Warnings = append(pc.Warnings, "no public key found; this client cannot be imported without one")
+	} else if _, err := wgtypes.ParseKey(rc.PubKey); err != nil {
+		pc.Warnings = append(pc.Warnings, fmt.Sprintf("public key does not parse as a valid WireGuard key: %v", err))
+	}
+
+	pc.AllocatedIPs = splitList(rc.TunnelAddress)
+	if len(pc.AllocatedIPs) == 0 {
+		pc.Warnings = append(pc.Warnings, "no tunnel address found; please set the client's allocated address manually")
+	} else if !util.ValidateAllowedIPs(pc.AllocatedIPs) {
+		pc.Warnings = append(pc.Warnings, "tunnel address is not valid CIDR; please fix before importing")
+	}
+	// OPNsense has no separate "allowed IPs for this peer" field for
+	// clients; default to the client's own allocated address(es), same as
+	// wireguard-ui-multi's own "new client" default.
+	pc.AllowedIPs = append([]string(nil), pc.AllocatedIPs...)
+
+	if rc.Keepalive != "" {
+		if ka, err := strconv.Atoi(strings.TrimSpace(rc.Keepalive)); err == nil && ka > 0 {
+			pc.PersistentKeepalive = ka
+		}
+	}
+
+	if rc.ServerAddress != "" {
+		// stashed for the caller to optionally fold into the server's
+		// EndpointAddress override; not part of the per-client model.
+		pc.AdditionalNotes += fmt.Sprintf(" (OPNsense serveraddress: %s)", rc.ServerAddress)
+	}
+
+	return pc
+}
+
+// firstServerAddress returns the first non-empty serveraddress[:serverport]
+// combination found among rc's clients, used to prefill a server's
+// EndpointAddress override in the preview.
+func firstServerAddress(clients []rawClient, peerUUIDs []string) string {
+	wanted := make(map[string]bool, len(peerUUIDs))
+	for _, u := range peerUUIDs {
+		wanted[u] = true
+	}
+	for _, c := range clients {
+		if !wanted[c.UUID] || c.ServerAddress == "" {
+			continue
+		}
+		if c.ServerPort != "" {
+			return fmt.Sprintf("%s:%s", c.ServerAddress, strings.TrimSpace(c.ServerPort))
+		}
+		return c.ServerAddress
+	}
+	return ""
+}
+
+// ToModel converts an admin-reviewed PreviewServer into the model records
+// wireguard-ui-multi actually stores: a Server, its ServerSetting, and the
+// Clients that belong to it. It does not touch the store itself - callers
+// (the commit handler) are responsible for the actual SaveXxx calls, and
+// for checking public-key collisions against clients already in the store.
+func ToModel(ps PreviewServer) (model.Server, model.ServerSetting, []model.Client, error) {
+	now := time.Now().UTC()
+
+	if !util.ValidateRecordID(ps.ID) {
+		return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("invalid server id %q", ps.ID)
+	}
+	if !util.ValidateInterfaceName(ps.Interface) {
+		return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("invalid interface name %q", ps.Interface)
+	}
+	if !util.ValidateServerAddresses(ps.Addresses) {
+		return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("invalid server addresses %v", ps.Addresses)
+	}
+	if ps.ListenPort <= 0 || ps.ListenPort > 65535 {
+		return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("invalid listen port %d", ps.ListenPort)
+	}
+	if ps.PrivateKey == "" {
+		return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("server %q has no private key", ps.ID)
+	}
+	key, err := wgtypes.ParseKey(ps.PrivateKey)
+	if err != nil {
+		return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("server %q private key invalid: %w", ps.ID, err)
+	}
+
+	server := model.Server{
+		ID:   ps.ID,
+		Name: ps.Name,
+		KeyPair: &model.ServerKeypair{
+			PrivateKey: key.String(),
+			PublicKey:  key.PublicKey().String(),
+			UpdatedAt:  now,
+		},
+		Interface: &model.ServerInterface{
+			Name:       ps.Interface,
+			Addresses:  ps.Addresses,
+			ListenPort: ps.ListenPort,
+			UpdatedAt:  now,
+		},
+	}
+
+	settings := model.ServerSetting{
+		EndpointAddress: ps.EndpointAddress,
+		ConfigFilePath:  fmt.Sprintf("/etc/wireguard/%s.conf", ps.Interface),
+		FirewallMark:    util.DefaultFirewallMark,
+		Table:           util.DefaultTable,
+		DNSServers:      ps.DNSServers,
+		MTU:             ps.MTU,
+		UpdatedAt:       now,
+	}
+
+	clients := make([]model.Client, 0, len(ps.Clients))
+	for _, pc := range ps.Clients {
+		if pc.PublicKey == "" {
+			return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("client %q has no public key", pc.Name)
+		}
+		if _, err := wgtypes.ParseKey(pc.PublicKey); err != nil {
+			return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("client %q public key invalid: %w", pc.Name, err)
+		}
+		if !util.ValidateAllowedIPs(pc.AllocatedIPs) {
+			return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("client %q has invalid allocated IPs", pc.Name)
+		}
+		if !util.ValidateAllowedIPs(pc.AllowedIPs) {
+			return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("client %q has invalid allowed IPs", pc.Name)
+		}
+
+		presharedKey := pc.PresharedKey
+		if presharedKey != "" {
+			if _, err := wgtypes.ParseKey(presharedKey); err != nil {
+				return model.Server{}, model.ServerSetting{}, nil, fmt.Errorf("client %q preshared key invalid: %w", pc.Name, err)
+			}
+		}
+
+		clients = append(clients, model.Client{
+			ServerID:            ps.ID,
+			PublicKey:           pc.PublicKey,
+			PresharedKey:        presharedKey,
+			Name:                pc.Name,
+			Email:               pc.Email,
+			AllocatedIPs:        pc.AllocatedIPs,
+			AllowedIPs:          pc.AllowedIPs,
+			AdditionalNotes:     pc.AdditionalNotes,
+			PersistentKeepalive: pc.PersistentKeepalive,
+			Enabled:             pc.Enabled,
+			CreatedAt:           now,
+			UpdatedAt:           now,
+		})
+	}
+
+	return server, settings, clients, nil
+}
@@ -0,0 +1,131 @@
+// Package opnsense parses OPNsense's WireGuard config.xml export and maps
+// it into staging structures that an admin can review and edit before
+// wireguard-ui-multi commits them as model.Server/model.ServerSetting/
+// model.Client records. Parsing and mapping never touch the store, and the
+// resulting data is never auto-applied (no wg-quick / systemctl calls
+// happen anywhere in this package) - see handler/routes_opnsense_import.go
+// for the two-step preview/commit flow that does the actual store writes.
+//
+// Schema reference (verified against OPNsense core master,
+// src/opnsense/mvc/app/models/OPNsense/Wireguard/{Server,Client}.xml):
+//
+//	<OPNsense><wireguard>
+//	  <server><servers>
+//	    <server uuid="..."><enabled/><name/><instance/><pubkey/><privkey/>
+//	      <port/><mtu/><dns/><tunneladdress/><disableroutes/><gateway/>
+//	      <peers/><debug/></server>
+//	  </servers></server>
+//	  <client><clients>
+//	    <client uuid="..."><enabled/><name/><pubkey/><psk/><tunneladdress/>
+//	      <serveraddress/><serverport/><keepalive/></client>
+//	  </clients></client>
+//	  <general><enabled/></general>
+//	</wireguard></OPNsense>
+package opnsense
+
+import (
+	"encoding/xml"
+	"fmt"
+	"io"
+	"strings"
+)
+
+// rawConfig mirrors the on-disk config.xml structure.
+type rawConfig struct {
+	XMLName   xml.Name `xml:"OPNsense"`
+	Wireguard struct {
+		Server struct {
+			Servers struct {
+				Server []rawServer `xml:"server"`
+			} `xml:"servers"`
+		} `xml:"server"`
+		Client struct {
+			Clients struct {
+				Client []rawClient `xml:"client"`
+			} `xml:"clients"`
+		} `xml:"client"`
+		General struct {
+			Enabled string `xml:"enabled"`
+		} `xml:"general"`
+	} `xml:"wireguard"`
+}
+
+type rawServer struct {
+	UUID           string `xml:"uuid,attr"`
+	Enabled        string `xml:"enabled"`
+	Name           string `xml:"name"`
+	Instance       string `xml:"instance"`
+	PubKey         string `xml:"pubkey"`
+	PrivKey        string `xml:"privkey"`
+	Port           string `xml:"port"`
+	MTU            string `xml:"mtu"`
+	DNS            string `xml:"dns"`
+	TunnelAddress  string `xml:"tunneladdress"`
+	DisableRoutes  string `xml:"disableroutes"`
+	Gateway        string `xml:"gateway"`
+	Peers          string `xml:"peers"`
+	Debug          string `xml:"debug"`
+}
+
+type rawClient struct {
+	UUID          string `xml:"uuid,attr"`
+	Enabled       string `xml:"enabled"`
+	Name          string `xml:"name"`
+	PubKey        string `xml:"pubkey"`
+	PSK           string `xml:"psk"`
+	TunnelAddress string `xml:"tunneladdress"`
+	ServerAddress string `xml:"serveraddress"`
+	ServerPort    string `xml:"serverport"`
+	Keepalive     string `xml:"keepalive"`
+}
+
+// ParsedConfig is the raw parsed result, before any admin-editable mapping
+// is applied.
+type ParsedConfig struct {
+	Servers []rawServer
+	Clients []rawClient
+}
+
+// Parse reads an OPNsense config.xml document and extracts the WireGuard
+// server/client definitions. It returns a clean error (never panics) if
+// the document isn't valid XML or doesn't have the expected root element.
+// Go's encoding/xml does not resolve external entities, so this is not
+// vulnerable to XXE; no custom entity handling is added.
+func Parse(r io.Reader) (*ParsedConfig, error) {
+	var cfg rawConfig
+	dec := xml.NewDecoder(r)
+	if err := dec.Decode(&cfg); err != nil {
+		return nil, fmt.Errorf("could not parse config.xml: %w", err)
+	}
+	if cfg.XMLName.Local != "OPNsense" {
+		return nil, fmt.Errorf("not an OPNsense config.xml (unexpected root element %q)", cfg.XMLName.Local)
+	}
+
+	return &ParsedConfig{
+		Servers: cfg.Wireguard.Server.Servers.Server,
+		Clients: cfg.Wireguard.Client.Clients.Client,
+	}, nil
+}
+
+// splitList splits an OPNsense comma-separated field (tunneladdress, dns,
+// peers, ...) into trimmed, non-empty parts.
+func splitList(s string) []string {
+	if strings.TrimSpace(s) == "" {
+		return nil
+	}
+	parts := strings.Split(s, ",")
+	out := make([]string, 0, len(parts))
+	for _, p := range parts {
+		p = strings.TrimSpace(p)
+		if p != "" {
+			out = append(out, p)
+		}
+	}
+	return out
+}
+
+// isTruthy interprets OPNsense's boolean-ish "1"/"0" (or empty) fields.
+func isTruthy(s string) bool {
+	s = strings.TrimSpace(s)
+	return s == "1" || strings.EqualFold(s, "true")
+}