diff --git a/api/v1alpha1/wireguardpeer_types.go b/api/v1alpha1/wireguardpeer_types.go index b95c629..80e3b30 100644 --- a/api/v1alpha1/wireguardpeer_types.go +++ b/api/v1alpha1/wireguardpeer_types.go @@ -51,6 +51,12 @@ type WireguardPeerSpec struct { PrivateKey PrivateKey `json:"privateKeyRef,omitempty"` // The key used by the peer to authenticate with the wg server. PublicKey string `json:"publicKey,omitempty"` + // Resolved preshared-key value, carried into the agent state (state.json) + // so the server-side [Peer] block can emit PresharedKey. Populated by the + // controller in-memory from the per-peer `-peer` Secret's + // `presharedKey` key; NOT meant to be set directly on the CR (it is never + // patched back, so it does not persist). + PresharedKey string `json:"presharedKey,omitempty"` // The name of the Wireguard instance in k8s that the peer belongs to. The wg instance should be in the same namespace as the peer. //+kubebuilder:validation:Required //+kubebuilder:validation:MinLength=1 diff --git a/config/crd/bases/vpn.wireguard-operator.io_wireguardpeers.yaml b/config/crd/bases/vpn.wireguard-operator.io_wireguardpeers.yaml index 36e97bf..b1c6b84 100644 --- a/config/crd/bases/vpn.wireguard-operator.io_wireguardpeers.yaml +++ b/config/crd/bases/vpn.wireguard-operator.io_wireguardpeers.yaml @@ -128,6 +128,14 @@ spec: maximum: 65535 minimum: 1 type: integer + presharedKey: + description: |- + Resolved preshared-key value, carried into the agent state (state.json) + so the server-side [Peer] block can emit PresharedKey. Populated by the + controller in-memory from the per-peer `-peer` Secret's + `presharedKey` key; NOT meant to be set directly on the CR (it is never + patched back, so it does not persist). + type: string privateKeyRef: description: The private key of the peer properties: diff --git a/internal/controller/wireguard_controller.go b/internal/controller/wireguard_controller.go index 41f8416..7cbc7bf 100644 --- a/internal/controller/wireguard_controller.go +++ b/internal/controller/wireguard_controller.go @@ -285,22 +285,28 @@ func (r *WireguardReconciler) updateWireguardPeers(ctx context.Context, req ctrl dnsConfiguration = dns + ", " + dnsSearchDomain } - allowIps := peer.Spec.AllowedIPs - - if allowIps == "" { - hasIPv4 := peer.Spec.Address != "" - hasIPv6 := peer.Spec.AddressV6 != "" - - switch { - case ipv6Only && hasIPv6: - allowIps = "::/0" - case v6Enabled && hasIPv4 && hasIPv6: - allowIps = "0.0.0.0/0, ::/0" - case v6Enabled && !hasIPv4 && hasIPv6: - allowIps = "::/0" - default: - allowIps = "0.0.0.0/0" - } + // Client-side AllowedIPs describes what the customer's device routes + // INTO the tunnel — a distinct concept from the server-side + // peer.Spec.AllowedIPs (the peer's identity / return-route CIDR the + // server enforces, rendered separately by the agent). Seeding the + // client config from peer.Spec.AllowedIPs produced a useless config: + // the RGD sets that to the peer's own /32, so the client routed + // nothing into the tunnel. We instead always hand out a full-tunnel + // default route; split-tunnel clients can trim it locally. Route + // family follows the peer's assigned address(es). + hasIPv4 := peer.Spec.Address != "" + hasIPv6 := peer.Spec.AddressV6 != "" + + var allowIps string + switch { + case ipv6Only && hasIPv6: + allowIps = "::/0" + case v6Enabled && hasIPv4 && hasIPv6: + allowIps = "0.0.0.0/0, ::/0" + case v6Enabled && !hasIPv4 && hasIPv6: + allowIps = "::/0" + default: + allowIps = "0.0.0.0/0" } // Do not store shell-wrapped config in status anymore per upstream PR 212 @@ -340,6 +346,17 @@ DNS = %s`, strings.TrimSpace(string(v)), addressLine, dnsConfiguration) persistentKeepaliveLine = fmt.Sprintf("\nPersistentKeepalive = %d", *peer.Spec.PersistentKeepalive) } + // Per-peer preshared key in the client [Peer] block, read from + // the same `-peer` Secret that holds the private key (the + // convention Secret; field `presharedKey`). The server enforces + // this PSK (agent renders it from the same Secret), so a client + // config without it gets its handshake silently dropped. Absent + // => no line (config byte-identical for non-PSK peers). + presharedKeyLine := "" + if psk := strings.TrimSpace(string(peerPrivSecret.Data["presharedKey"])); psk != "" { + presharedKeyLine = "\nPresharedKey = " + psk + } + if wireguard.Spec.Tunnel.Enabled { tunnelPort := wireguard.Spec.Tunnel.Port if tunnelPort == 0 { @@ -352,10 +369,10 @@ PreUp = wstunnel client -L udp://127.0.0.1:%d:127.0.0.1:%d wss://%s:%d & PostDown = killall wstunnel || true [Peer] -PublicKey = %s +PublicKey = %s%s AllowedIPs = %s Endpoint = 127.0.0.1:%d%s -`, port, port, serverAddress, tunnelPort, serverPublicKey, allowIps, port, persistentKeepaliveLine) +`, port, port, serverAddress, tunnelPort, serverPublicKey, presharedKeyLine, allowIps, port, persistentKeepaliveLine) if wireguard.Spec.Tunnel.DualMode { // In dual mode, store both configs: @@ -364,10 +381,10 @@ Endpoint = 127.0.0.1:%d%s directCfg := pureCfg + fmt.Sprintf(` [Peer] -PublicKey = %s +PublicKey = %s%s AllowedIPs = %s Endpoint = %s:%s%s -`, serverPublicKey, allowIps, serverAddress, resources.PeerEndpointPort(wireguard), persistentKeepaliveLine) +`, serverPublicKey, presharedKeyLine, allowIps, serverAddress, resources.PeerEndpointPort(wireguard), persistentKeepaliveLine) newPeerCfgData[peer.Name] = []byte(directCfg) newPeerCfgData[peer.Name+".tunnel"] = []byte(tunnelCfg) } else { @@ -378,10 +395,10 @@ Endpoint = %s:%s%s pureCfg = pureCfg + fmt.Sprintf(` [Peer] -PublicKey = %s +PublicKey = %s%s AllowedIPs = %s Endpoint = %s:%s%s -`, serverPublicKey, allowIps, serverAddress, resources.PeerEndpointPort(wireguard), persistentKeepaliveLine) +`, serverPublicKey, presharedKeyLine, allowIps, serverAddress, resources.PeerEndpointPort(wireguard), persistentKeepaliveLine) newPeerCfgData[peer.Name] = []byte(pureCfg) } } @@ -499,6 +516,18 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( continue } + // Carry an optional preshared key from the per-peer `-peer` Secret + // (the same convention Secret the peer reconciler adopts/creates) into + // the agent state, so the server-side [Peer] block emits PresharedKey. + // For migrated external peers the PSK is supplied from 1Password via an + // ExternalSecret targeting `-peer`. Absent key => no PSK (default). + pskSecret := &corev1.Secret{} + if err := r.Get(ctx, types.NamespacedName{Name: peer.Name + "-peer", Namespace: peer.Namespace}, pskSecret); err == nil { + if v, ok := pskSecret.Data["presharedKey"]; ok { + peer.Spec.PresharedKey = strings.TrimSpace(string(v)) + } + } + filteredPeers = append(filteredPeers, peer) } @@ -708,6 +737,28 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err == nil { privateKey := string(secret.Data["privateKey"]) + // FAIL CLOSED on a corrupt/mismatched server keypair: if the stored + // public key is not the curve25519 derivation of the stored private + // key, refuse to reconcile. A wrong server public key would be handed + // to every peer's wg-quick config and silently break all handshakes. + // (The peer-side provenance/fail-closed logic lives in the + // WireguardPeer reconciler; this guards the server's own keypair.) + if storedPub := strings.TrimSpace(string(secret.Data["publicKey"])); storedPub != "" { + k, perr := wgtypes.ParseKey(strings.TrimSpace(privateKey)) + if perr != nil { + msg := fmt.Sprintf("stored server private key is not a valid WireGuard key: %v", perr) + log.Error(perr, msg) + _ = r.updateStatus(ctx, req, metav1.Condition{Type: ConditionDegraded, Status: metav1.ConditionTrue, Reason: "ServerKeyInvalid", Message: msg}) + return ctrl.Result{}, fmt.Errorf("%s", msg) + } + if derived := k.PublicKey().String(); derived != storedPub { + msg := fmt.Sprintf("server public key %s does not match the key %s derived from the stored private key; refusing to reconcile", storedPub, derived) + log.Error(fmt.Errorf("server public/private key mismatch"), msg) + _ = r.updateStatus(ctx, req, metav1.Condition{Type: ConditionDegraded, Status: metav1.ConditionTrue, Reason: "ServerKeyMismatch", Message: msg}) + return ctrl.Result{}, fmt.Errorf("%s", msg) + } + } + state := agent.State{ Server: *wireguard.DeepCopy(), ServerPrivateKey: privateKey, diff --git a/internal/wireguard/wireguard.go b/internal/wireguard/wireguard.go index ae8cf58..dc05113 100644 --- a/internal/wireguard/wireguard.go +++ b/internal/wireguard/wireguard.go @@ -603,6 +603,16 @@ func BuildWgQuickConfig(state agent.State, listenPort int) (string, error) { fmt.Fprintf(&b, "\n[Peer]\nPublicKey = %s\nAllowedIPs = %s\n", peer.Spec.PublicKey, allowed) + // Per-peer preshared key (PSK). The controller populates + // peer.Spec.PresharedKey in-memory from the `-peer` Secret's + // `presharedKey` key; absent => no line (peers without a PSK keep their + // config byte-identical). The server enforces the PSK here, so the + // client's wg-quick blob MUST carry the same PresharedKey or the + // handshake is silently dropped. + if peer.Spec.PresharedKey != "" { + fmt.Fprintf(&b, "PresharedKey = %s\n", peer.Spec.PresharedKey) + } + // Intentionally NO PersistentKeepalive on the server-side [Peer] block: // the server sits behind a stable LoadBalancer IP (not behind NAT), so // it has no mapping to keep alive. The whole design is responder-only — diff --git a/internal/wireguard/wireguard_test.go b/internal/wireguard/wireguard_test.go index 93e551f..c93808c 100644 --- a/internal/wireguard/wireguard_test.go +++ b/internal/wireguard/wireguard_test.go @@ -207,6 +207,65 @@ func TestBuildWgQuickConfig_OmitsPersistentKeepaliveWhenUnset(t *testing.T) { } } +// TestBuildWgQuickConfig_EmitsPresharedKeyWhenSet locks down the server-side +// PSK render: when the controller has resolved peer.Spec.PresharedKey (from the +// `-peer` Secret's `presharedKey` key), the wg0 [Peer] block MUST emit a +// `PresharedKey = ` line. The server enforces the PSK, so without this +// the symmetric layer is missing and the migrated peers (which carry a legacy +// PSK in their client config) get their handshake silently dropped. +func TestBuildWgQuickConfig_EmitsPresharedKeyWhenSet(t *testing.T) { + const psk = "+5Sfa0dhfiGJdqzB+gcFirhyacqt2GjcLJEpoOSfCy0=" + state := agent.State{ + ServerPrivateKey: validServerPrivateKey, + Server: v1alpha1.Wireguard{}, + Peers: []v1alpha1.WireguardPeer{ + { + Spec: v1alpha1.WireguardPeerSpec{ + PublicKey: validPeerPublicKey, + Address: "172.31.255.2", + PresharedKey: psk, + }, + }, + }, + } + + cfg, err := BuildWgQuickConfig(state, 51820) + if err != nil { + t.Fatalf("BuildWgQuickConfig: %v", err) + } + + if !strings.Contains(cfg, "PresharedKey = "+psk) { + t.Errorf("server-side [Peer] block must include the resolved PresharedKey line:\n%s", cfg) + } +} + +// TestBuildWgQuickConfig_OmitsPresharedKeyWhenUnset is the backwards-compat +// guard: peers without a PSK (the bulk of the fleet) must produce a [Peer] +// block with no PresharedKey line, byte-identical to the pre-feature output. +func TestBuildWgQuickConfig_OmitsPresharedKeyWhenUnset(t *testing.T) { + state := agent.State{ + ServerPrivateKey: validServerPrivateKey, + Server: v1alpha1.Wireguard{}, + Peers: []v1alpha1.WireguardPeer{ + { + Spec: v1alpha1.WireguardPeerSpec{ + PublicKey: validPeerPublicKey, + Address: "172.31.255.2", + }, + }, + }, + } + + cfg, err := BuildWgQuickConfig(state, 51820) + if err != nil { + t.Fatalf("BuildWgQuickConfig: %v", err) + } + + if strings.Contains(cfg, "PresharedKey") { + t.Errorf("PresharedKey must NOT appear when the peer has no resolved PSK:\n%s", cfg) + } +} + // TestBuildWgQuickConfig_RoutesAppendedToAllowedIPs locks down Phase G: when // WireguardPeer.spec.routes is set, the rendered server-side [Peer] AllowedIPs // CSV must include the peer's own /32 PLUS every CIDR in Routes. This is what