From c786fca372078ed5dc467f3c02abb3071d26c388 Mon Sep 17 00:00:00 2001
From: Julien Viard de Galbert <jviarddegalbert@scaleway.com>
Date: Tue, 6 Jul 2021 11:10:20 +0200
Subject: [PATCH] Add error handling to ParseDump

---
 pkg/mesh/mesh.go           |  7 ++++++-
 pkg/wireguard/conf.go      | 26 +++++++++++++++++---------
 pkg/wireguard/conf_test.go |  2 +-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/pkg/mesh/mesh.go b/pkg/mesh/mesh.go
index ecbc762..a805c1e 100644
--- a/pkg/mesh/mesh.go
+++ b/pkg/mesh/mesh.go
@@ -460,7 +460,12 @@ func (m *Mesh) applyTopology() {
 		m.errorCounter.WithLabelValues("apply").Inc()
 		return
 	}
-	oldConf := wireguard.ParseDump(oldConfDump)
+	oldConf, err := wireguard.ParseDump(oldConfDump)
+	if err != nil {
+		level.Error(m.logger).Log("error", err)
+		m.errorCounter.WithLabelValues("apply").Inc()
+		return
+	}
 	natEndpoints := discoverNATEndpoints(nodes, peers, oldConf, m.logger)
 	nodes[m.hostname].DiscoveredEndpoints = natEndpoints
 	t, err := NewTopology(nodes, peers, m.granularity, m.hostname, nodes[m.hostname].Endpoint.Port, m.priv, m.subnet, nodes[m.hostname].PersistentKeepalive, m.logger)
diff --git a/pkg/wireguard/conf.go b/pkg/wireguard/conf.go
index bd54e03..7cef64d 100644
--- a/pkg/wireguard/conf.go
+++ b/pkg/wireguard/conf.go
@@ -512,7 +512,7 @@ func (p *Peer) parseAllowedIPs(v string) error {
 }
 
 // ParseDump parses a given WireGuard dump and produces a Conf struct.
-func ParseDump(buf []byte) *Conf {
+func ParseDump(buf []byte) (*Conf, error) {
 	// from man wg, show section:
 	// If dump is specified, then several lines are printed;
 	// the first contains in order separated by tab: private-key, public-key, listen-port, fw‐mark.
@@ -528,6 +528,7 @@ func ParseDump(buf []byte) *Conf {
 		port   uint64
 		sec    int64
 		pka    int
+		line   int
 	)
 	// First line is Interface
 	active = interfaceSection
@@ -538,7 +539,7 @@ func ParseDump(buf []byte) *Conf {
 		switch active {
 		case interfaceSection:
 			if len(values) < dumpInterfaceLen {
-				break
+				return nil, fmt.Errorf("invalid interface line: missing fields (%d < %d)", len(values), dumpInterfaceLen)
 			}
 			iface = new(Interface)
 			for i := range values {
@@ -548,7 +549,7 @@ func ParseDump(buf []byte) *Conf {
 				case dumpInterfaceListenPortIndex:
 					port, err = strconv.ParseUint(values[i], 10, 32)
 					if err != nil {
-						continue
+						return nil, fmt.Errorf("invalid interface line: error parsing listen-port: %w", err)
 					}
 					iface.ListenPort = uint32(port)
 				}
@@ -558,7 +559,7 @@ func ParseDump(buf []byte) *Conf {
 			active = peerSection
 		case peerSection:
 			if len(values) < dumpPeerLen {
-				break
+				return nil, fmt.Errorf("invalid peer line %d: missing fields (%d < %d)", line, len(values), dumpPeerLen)
 			}
 			peer = new(Peer)
 
@@ -575,12 +576,18 @@ func ParseDump(buf []byte) *Conf {
 					if values[i] == dumpNone {
 						continue
 					}
-					peer.parseEndpoint(values[i])
+					err = peer.parseEndpoint(values[i])
+					if err != nil {
+						return nil, fmt.Errorf("invalid peer line %d: error parsing endpoint: %w", line, err)
+					}
 				case dumpPeerAllowedIPsIndex:
 					if values[i] == dumpNone {
 						continue
 					}
-					peer.parseAllowedIPs(values[i])
+					err = peer.parseAllowedIPs(values[i])
+					if err != nil {
+						return nil, fmt.Errorf("invalid peer line %d: error parsing allowed-ips: %w", line, err)
+					}
 				case dumpPeerLatestHandshakeIndex:
 					if values[i] == "0" {
 						// Use go zero value, not unix 0 timestamp.
@@ -589,7 +596,7 @@ func ParseDump(buf []byte) *Conf {
 					}
 					sec, err = strconv.ParseInt(values[i], 10, 64)
 					if err != nil {
-						continue
+						return nil, fmt.Errorf("invalid peer line %d: error parsing latest-handshake: %w", line, err)
 					}
 					peer.LatestHandshake = time.Unix(sec, 0)
 				case dumpPeerPersistentKeepaliveIndex:
@@ -598,7 +605,7 @@ func ParseDump(buf []byte) *Conf {
 					}
 					pka, err = strconv.Atoi(values[i])
 					if err != nil {
-						continue
+						return nil, fmt.Errorf("invalid peer line %d: error parsing persistent-keepalive: %w", line, err)
 					}
 					peer.PersistentKeepalive = pka
 				}
@@ -606,6 +613,7 @@ func ParseDump(buf []byte) *Conf {
 			c.Peers = append(c.Peers, peer)
 			peer = nil
 		}
+		line++
 	}
-	return &c
+	return &c, nil
 }
diff --git a/pkg/wireguard/conf_test.go b/pkg/wireguard/conf_test.go
index 229be26..aa3e782 100644
--- a/pkg/wireguard/conf_test.go
+++ b/pkg/wireguard/conf_test.go
@@ -345,7 +345,7 @@ key2	(none)	10.254.2.1:51820	100.64.4.0/24,10.69.76.55/32,100.64.3.0/24,10.66.25
 		},
 	} {
 
-		dumpConf := ParseDump(tc.d)
+		dumpConf, _ := ParseDump(tc.d)
 		conf := Parse(tc.c)
 		// Equal will ignore runtime fields and only compare configuration fields.
 		if !dumpConf.Equal(conf) {