From 82a6d47bc912076df9c751196f9492c133888f6a Mon Sep 17 00:00:00 2001 From: jdl Date: Thu, 11 Jun 2026 16:46:38 +0200 Subject: [PATCH] Audit changes. --- hub/api/api.go | 13 ++++---- hub/api/db/sanitize-validate.go | 13 ++++++-- hub/api/errors.go | 1 - hub/api/migrations/2024-11-30-init.sql | 3 +- hub/cookie.go | 12 ++++---- hub/handlers.go | 24 +++++++++------ m/models.go | 2 +- peer/app.go | 1 - peer/control_conn.go | 4 +++ peer/device.go | 3 +- peer/hosts.go | 4 +-- peer/hub_poller.go | 7 +++-- peer/hub_poller_test.go | 6 ++-- peer/init.go | 35 +++++++++++++++++----- peer/multicast.go | 10 +++++++ peer/network_state_test.go | 41 +++++++++++++------------- peer/new.go | 25 ++++++++++++---- 17 files changed, 133 insertions(+), 71 deletions(-) diff --git a/hub/api/api.go b/hub/api/api.go index e103b78..6aaffce 100644 --- a/hub/api/api.go +++ b/hub/api/api.go @@ -65,12 +65,8 @@ func (a *API) ensurePassword() error { return db.Config_Insert(a.db, conf) } -func (a *API) Config_Get() *Config { - conf, err := db.Config_Get(a.db, 1) - if err != nil { - panic(err) - } - return conf +func (a *API) Config_Get() (*Config, error) { + return db.Config_Get(a.db, 1) } func (a *API) Config_Update(conf *Config) error { @@ -116,7 +112,10 @@ func (a *API) session_Create() *Session { } func (a *API) Session_SignIn(s *Session, pwd string) error { - conf := a.Config_Get() + conf, err := a.Config_Get() + if err != nil { + return err + } if err := bcrypt.CompareHashAndPassword(conf.Password, []byte(pwd)); err != nil { return ErrNotAuthorized } diff --git a/hub/api/db/sanitize-validate.go b/hub/api/db/sanitize-validate.go index 2782bf5..ffe1f7d 100644 --- a/hub/api/db/sanitize-validate.go +++ b/hub/api/db/sanitize-validate.go @@ -8,6 +8,7 @@ import ( var ( ErrInvalidIP = errors.New("invalid IP") + ErrInvalidPeerIP = errors.New("invalid peer IP") ErrNonPrivateIP = errors.New("non-private IP") ErrInvalidPort = errors.New("invalid port") ErrInvalidNetName = errors.New("invalid network name") @@ -67,7 +68,8 @@ func Peer_Sanitize(p *Peer) { p.Name = strings.TrimSpace(p.Name) if len(p.Addr4) != 0 { if addr, ok := netip.AddrFromSlice(p.Addr4); ok { - p.Addr4 = addr.AsSlice() + // Unmap so an IPv4-mapped form is stored canonically as 4 bytes. + p.Addr4 = addr.Unmap().AsSlice() } } if len(p.Addr6) != 0 { @@ -81,13 +83,18 @@ func Peer_Sanitize(p *Peer) { } func Peer_Validate(p *Peer) error { + if p.PeerIP < 1 || p.PeerIP > 254 { + return ErrInvalidPeerIP + } if len(p.Addr4) > 0 { - if _, ok := netip.AddrFromSlice(p.Addr4); !ok { + // Must be a genuine IPv4 address (reject an IPv6 in the v4 field). + if addr, ok := netip.AddrFromSlice(p.Addr4); !ok || !addr.Is4() { return ErrInvalidIP } } if len(p.Addr6) > 0 { - if _, ok := netip.AddrFromSlice(p.Addr6); !ok { + // Must be a genuine IPv6 address (reject IPv4 / IPv4-mapped in the v6 field). + if addr, ok := netip.AddrFromSlice(p.Addr6); !ok || !addr.Is6() || addr.Is4In6() { return ErrInvalidIP } } diff --git a/hub/api/errors.go b/hub/api/errors.go index a7da6f6..27d241f 100644 --- a/hub/api/errors.go +++ b/hub/api/errors.go @@ -7,7 +7,6 @@ import ( var ( ErrNotAuthorized = errors.New("not authorized") - ErrNoIPAvailable = errors.New("no IP address available") ErrInvalidIP = db.ErrInvalidIP ErrInvalidPort = db.ErrInvalidPort ) diff --git a/hub/api/migrations/2024-11-30-init.sql b/hub/api/migrations/2024-11-30-init.sql index c62b28a..b443841 100644 --- a/hub/api/migrations/2024-11-30-init.sql +++ b/hub/api/migrations/2024-11-30-init.sql @@ -14,12 +14,13 @@ CREATE TABLE peers ( PeerIP INTEGER NOT NULL, -- Final byte of IP. Version INTEGER NOT NULL, -- Changes when updated. APIKey TEXT NOT NULL UNIQUE, -- Peer's secret API key. - Name TEXT NOT NULL UNIQUE, -- For humans. + Name TEXT NOT NULL, -- For humans. Addr4 BLOB NOT NULL, Addr6 BLOB NOT NULL, Port INTEGER NOT NULL, Relay INTEGER NOT NULL DEFAULT 0, -- Boolean if peer will forward packets. WGPubKey BLOB NOT NULL, SignPubKey BLOB NOT NULL, + UNIQUE(NetworkID, Name), PRIMARY KEY(NetworkID, PeerIP) ) WITHOUT ROWID; diff --git a/hub/cookie.go b/hub/cookie.go index 2048d6b..c2bacc9 100644 --- a/hub/cookie.go +++ b/hub/cookie.go @@ -2,7 +2,6 @@ package hub import ( "net/http" - "time" ) func (a *App) getCookie(r *http.Request, name string) string { @@ -26,9 +25,12 @@ func (a *App) setCookie(w http.ResponseWriter, name, value string) { func (a *App) deleteCookie(w http.ResponseWriter, name string) { http.SetCookie(w, &http.Cookie{ - Name: name, - Value: "", - Path: "/", - Expires: time.Unix(0, 0), + Name: name, + Value: "", + Path: "/", + Secure: !a.insecure, + SameSite: http.SameSiteStrictMode, + HttpOnly: true, + MaxAge: -1, // delete now }) } diff --git a/hub/handlers.go b/hub/handlers.go index ffa4095..1400f47 100644 --- a/hub/handlers.go +++ b/hub/handlers.go @@ -260,13 +260,17 @@ func (a *App) _adminPasswordEdit(s *api.Session, w http.ResponseWriter, r *http. func (a *App) _adminPasswordSubmit(s *api.Session, w http.ResponseWriter, r *http.Request) error { var ( - conf = a.api.Config_Get() curPwd string newPwd string newPwd2 string ) - err := webutil.NewFormScanner(r.Form). + conf, err := a.api.Config_Get() + if err != nil { + return err + } + + err = webutil.NewFormScanner(r.Form). Scan("CurrentPassword", &curPwd). Scan("NewPassword", &newPwd). Scan("NewPassword2", &newPwd2). @@ -337,7 +341,7 @@ func (a *App) _peerInit(peer *api.Peer, w http.ResponseWriter, r *http.Request) LocalDomain: net.LocalDomain, } - resp.NetworkState.Peers, err = a.peersArray(net.NetworkID) + resp.NetworkState.Peers, err = a.peersList(net.NetworkID) if err != nil { return err } @@ -346,22 +350,24 @@ func (a *App) _peerInit(peer *api.Peer, w http.ResponseWriter, r *http.Request) } func (a *App) _peerFetchState(peer *api.Peer, w http.ResponseWriter, r *http.Request) error { - peers, err := a.peersArray(peer.NetworkID) + peers, err := a.peersList(peer.NetworkID) if err != nil { return err } return a.sendJSON(w, m.NetworkState{Peers: peers}) } -func (a *App) peersArray(networkID int64) (peers [256]*m.Peer, err error) { +func (a *App) peersList(networkID int64) (peers []m.Peer, err error) { l, err := a.api.Peer_List(networkID) if err != nil { - return peers, err + return nil, err } + peers = make([]m.Peer, 0, len(l)) + for _, p := range l { if len(p.WGPubKey) != 0 { - peers[p.PeerIP] = &m.Peer{ + peers = append(peers, m.Peer{ PeerIP: p.PeerIP, Version: p.Version, Name: p.Name, @@ -371,9 +377,9 @@ func (a *App) peersArray(networkID int64) (peers [256]*m.Peer, err error) { Relay: p.Relay, WGPubKey: p.WGPubKey, SignPubKey: p.SignPubKey, - } + }) } } - return + return peers, nil } diff --git a/m/models.go b/m/models.go index 232c87e..a49724f 100644 --- a/m/models.go +++ b/m/models.go @@ -26,5 +26,5 @@ type Peer struct { } type NetworkState struct { - Peers [256]*Peer + Peers []Peer } diff --git a/peer/app.go b/peer/app.go index 0042ec1..63affe6 100644 --- a/peer/app.go +++ b/peer/app.go @@ -109,6 +109,5 @@ func (a *App) Run() error { } func (a *App) onShutdown() error { - // TODO: removeHosts() ? return wginterface.Delete(a.dev.Name()) } diff --git a/peer/control_conn.go b/peer/control_conn.go index 50f762a..52a2a15 100644 --- a/peer/control_conn.go +++ b/peer/control_conn.go @@ -59,3 +59,7 @@ func (c *udpControlConn) run(ch chan<- PingEvent) { ch <- PingEvent{srcVPNIP: srcIP.Unmap(), ping: ping} } } + +func (c *udpControlConn) Close() error { + return c.conn.Close() +} diff --git a/peer/device.go b/peer/device.go index 61765c5..71bfaaf 100644 --- a/peer/device.go +++ b/peer/device.go @@ -11,7 +11,8 @@ import ( ) // devRetry calls fn up to 6 times with exponential backoff, retrying on EBUSY -// (transient netlink contention during WireGuard handshake/rekey). Fatal on any other error. +// (transient netlink contention during WireGuard handshake/rekey). Fatal on +// any other error. func devRetry(vpnIP netip.Addr, op string, fn func() error) { const attempts = 6 timeout := 10 * time.Millisecond diff --git a/peer/hosts.go b/peer/hosts.go index 2423d7f..3dca3b1 100644 --- a/peer/hosts.go +++ b/peer/hosts.go @@ -55,9 +55,9 @@ func updateHosts(hostsPath, localDomain string, peers map[netip.Addr]*Peer) erro after := "" if idxBegin := strings.Index(data, begin); idxBegin != -1 { - idxEnd := strings.Index(data, end) + idxEnd := strings.Index(data[idxBegin:], end) if idxEnd != -1 { - after = strings.TrimSpace(data[idxEnd+len(end):]) + after = strings.TrimSpace(data[idxBegin+idxEnd+len(end):]) } before = strings.TrimSpace(data[:idxBegin]) } diff --git a/peer/hub_poller.go b/peer/hub_poller.go index ab0783b..343f5b2 100644 --- a/peer/hub_poller.go +++ b/peer/hub_poller.go @@ -119,7 +119,7 @@ func (hp *HubPoller) apply(state m.NetworkState) (changed bool) { netAddr := hp.vpnNet.Addr().As4() for _, p := range state.Peers { - if p == nil || len(p.WGPubKey) != wgtypes.KeyLen || len(p.SignPubKey) != 32 { + if len(p.WGPubKey) != wgtypes.KeyLen || len(p.SignPubKey) != 32 { continue } @@ -156,16 +156,17 @@ func (hp *HubPoller) apply(state m.NetworkState) (changed bool) { return changed } -func hubPeerFrom(pubKey wgtypes.Key, vpnIP netip.Addr, p *m.Peer) HubPeer { +func hubPeerFrom(pubKey wgtypes.Key, vpnIP netip.Addr, p m.Peer) HubPeer { var ep4, ep6 netip.AddrPort if len(p.Addr4) > 0 { if addr, ok := netip.AddrFromSlice(p.Addr4); ok { ep4 = netip.AddrPortFrom(addr.Unmap(), p.Port) } } + if len(p.Addr6) > 0 { if addr, ok := netip.AddrFromSlice(p.Addr6); ok { - ep6 = netip.AddrPortFrom(addr, p.Port) + ep6 = netip.AddrPortFrom(addr.Unmap(), p.Port) } } var signPubKey [32]byte diff --git a/peer/hub_poller_test.go b/peer/hub_poller_test.go index b19565f..f7f765c 100644 --- a/peer/hub_poller_test.go +++ b/peer/hub_poller_test.go @@ -24,14 +24,12 @@ func testPoller(t *testing.T) (*HubPoller, chan HubPeer, chan wgtypes.Key) { } func stateWith(key wgtypes.Key, peerIP byte, version int64) m.NetworkState { - var s m.NetworkState - s.Peers[peerIP] = &m.Peer{ + return m.NetworkState{Peers: []m.Peer{{ PeerIP: peerIP, Version: version, WGPubKey: key[:], SignPubKey: make([]byte, 32), - } - return s + }}} } func TestApply_EmitsAddsAndReportsChange(t *testing.T) { diff --git a/peer/init.go b/peer/init.go index 45cc788..20658b3 100644 --- a/peer/init.go +++ b/peer/init.go @@ -115,13 +115,34 @@ func initFromHub(hubURL, apiKey string, privKey wgtypes.Key) (LocalState, error) vpnIP := netip.AddrFrom4(octets) vpnNet := netip.PrefixFrom(netAddr, 24) - var isRelay, isPublic bool - var wgPort uint16 - if self := r.NetworkState.Peers[r.PeerIP]; self != nil { - isRelay = self.Relay - isPublic = len(self.Addr4) > 0 || len(self.Addr6) > 0 - wgPort = self.Port + var self *m.Peer + for i := range r.NetworkState.Peers { + if r.NetworkState.Peers[i].PeerIP == r.PeerIP { + self = &r.NetworkState.Peers[i] + break + } } + if self == nil { + return LocalState{}, fmt.Errorf("hub init: no peer for own IP: %d", r.PeerIP) + } + + var isRelay, public bool + var wgPort uint16 + + var ep4, ep6 netip.AddrPort + if len(self.Addr4) > 0 { + if addr, ok := netip.AddrFromSlice(self.Addr4); ok { + ep4 = netip.AddrPortFrom(addr.Unmap(), self.Port) + } + } + if len(self.Addr6) > 0 { + if addr, ok := netip.AddrFromSlice(self.Addr6); ok { + ep6 = netip.AddrPortFrom(addr.Unmap(), self.Port) + } + } + public = ep4.IsValid() || ep6.IsValid() + isRelay = self.Relay && public + wgPort = self.Port return LocalState{ PrivKey: privKey, @@ -130,7 +151,7 @@ func initFromHub(hubURL, apiKey string, privKey wgtypes.Key) (LocalState, error) VPNNet: vpnNet, WGPort: wgPort, IsRelay: isRelay, - IsPublic: isPublic, + IsPublic: public, LocalDomain: r.LocalDomain, }, nil } diff --git a/peer/multicast.go b/peer/multicast.go index 648f9db..f383480 100644 --- a/peer/multicast.go +++ b/peer/multicast.go @@ -26,11 +26,19 @@ var mcAddr = net.UDPAddrFromAddrPort(netip.AddrPortFrom( // RunMCWriter broadcasts a signed beacon on the local multicast group every // mcBroadcastInterval so that LAN peers can discover our WireGuard endpoint. func RunMCWriter(selfVPNIP netip.Addr, pubKey wgtypes.Key, wgPort uint16, signKey *[64]byte) { + for { + runMCWriterInner(selfVPNIP, pubKey, wgPort, signKey) + time.Sleep(mcErrorRetryInterval) + } +} + +func runMCWriterInner(selfVPNIP netip.Addr, pubKey wgtypes.Key, wgPort uint16, signKey *[64]byte) { conn, err := net.ListenMulticastUDP("udp", nil, mcAddr) if err != nil { log.Printf("[MCWriter] bind: %v", err) return } + defer conn.Close() payload := buildBeacon(selfVPNIP, pubKey, wgPort) signed := sign.Sign(nil, payload, signKey) @@ -38,9 +46,11 @@ func RunMCWriter(selfVPNIP netip.Addr, pubKey wgtypes.Key, wgPort uint16, signKe if _, err := conn.WriteToUDP(signed, mcAddr); err != nil { log.Printf("[MCWriter] write: %v", err) } + for range time.Tick(mcBroadcastInterval) { if _, err := conn.WriteToUDP(signed, mcAddr); err != nil { log.Printf("[MCWriter] write: %v", err) + return } } } diff --git a/peer/network_state_test.go b/peer/network_state_test.go index 42e6741..1776bdb 100644 --- a/peer/network_state_test.go +++ b/peer/network_state_test.go @@ -11,26 +11,27 @@ import ( func TestNetworkState_RoundTrip(t *testing.T) { path := filepath.Join(t.TempDir(), "network.json") - var state m.NetworkState - state.Peers[1] = &m.Peer{ - PeerIP: 1, - Version: 7, - Name: "hub", - Addr4: []byte{10, 11, 12, 1}, - Port: 51820, - Relay: true, - WGPubKey: make([]byte, 32), - SignPubKey: make([]byte, 32), - } - state.Peers[10] = &m.Peer{ - PeerIP: 10, - Version: 3, - Name: "laptop", - Addr4: []byte{10, 11, 12, 10}, - Port: 51820, - WGPubKey: []byte("0123456789abcdef0123456789abcdef"), - SignPubKey: []byte("fedcba9876543210fedcba9876543210"), - } + state := m.NetworkState{Peers: []m.Peer{ + { + PeerIP: 1, + Version: 7, + Name: "hub", + Addr4: []byte{10, 11, 12, 1}, + Port: 51820, + Relay: true, + WGPubKey: make([]byte, 32), + SignPubKey: make([]byte, 32), + }, + { + PeerIP: 10, + Version: 3, + Name: "laptop", + Addr4: []byte{10, 11, 12, 10}, + Port: 51820, + WGPubKey: []byte("0123456789abcdef0123456789abcdef"), + SignPubKey: []byte("fedcba9876543210fedcba9876543210"), + }, + }} if err := saveNetworkState(path, state); err != nil { t.Fatal(err) diff --git a/peer/new.go b/peer/new.go index e8396ce..e2145ad 100644 --- a/peer/new.go +++ b/peer/new.go @@ -27,24 +27,35 @@ func New( dev, err := wginterface.Open(ifaceName) if err != nil { + _ = wginterface.Delete(ifaceName) return nil, fmt.Errorf("open WG device: %w", err) } + cc, err := newUDPControlConn(state.VPNIP, ControlPort) + if err != nil { + _ = dev.Close() + _ = wginterface.Delete(ifaceName) + return nil, fmt.Errorf("control conn: %w", err) + } + + cleanup := func() { + _ = cc.Close() + _ = dev.Close() + _ = wginterface.Delete(ifaceName) + } + if err := dev.Configure(state.PrivKey, int(state.WGPort)); err != nil { + cleanup() return nil, fmt.Errorf("configure WG device: %w", err) } if state.IsRelay { if err := dev.EnableForwarding(); err != nil { + cleanup() return nil, fmt.Errorf("enable forwarding: %w", err) } } - cc, err := newUDPControlConn(state.VPNIP, ControlPort) - if err != nil { - return nil, fmt.Errorf("control conn: %w", err) - } - pingCh := make(chan PingEvent) hubAddCh := make(chan HubPeer) hubRemoveCh := make(chan wgtypes.Key) @@ -59,13 +70,15 @@ func New( hubAddCh, hubRemoveCh) if err != nil { + cleanup() return nil, fmt.Errorf("hub poller: %w", err) } go cc.run(pingCh) go poller.Run() - go RunMCWriter(state.VPNIP, state.PrivKey.PublicKey(), state.WGPort, &state.SignKey) + if !state.IsPublic { + go RunMCWriter(state.VPNIP, state.PrivKey.PublicKey(), state.WGPort, &state.SignKey) go RunMCReader(state.VPNNet, state.VPNIP, multicastCh) }