Don't log invalid certificates (#1116)

slackhq · Apr 29, 2024 · a99618e · a99618e
1 parent 8e94eb9
commit a99618e
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 8 deletions.
diff --git a/LOGGING.md b/LOGGING.md
@@ -33,6 +33,5 @@ l.WithError(err).
     WithField("vpnIp", IntIp(hostinfo.hostId)).
     WithField("udpAddr", addr).
     WithField("handshake", m{"stage": 1, "style": "ix"}).
-    WithField("cert", remoteCert).
     Info("Invalid certificate from host")
 ```
diff --git a/examples/config.yml b/examples/config.yml
@@ -244,7 +244,10 @@ tun:
 # TODO
 # Configure logging level
 logging:
-  # panic, fatal, error, warning, info, or debug. Default is info
+  # panic, fatal, error, warning, info, or debug. Default is info and is reloadable.
+  #NOTE: Debug mode can log remotely controlled/untrusted data which can quickly fill a disk in some
+  # scenarios. Debug logging is also CPU intensive and will decrease performance overall.
+  # Only enable debug logging while actively investigating an issue.
   level: info
   # json or text formats currently available. Default is text
   format: text

diff --git a/handshake_ix.go b/handshake_ix.go
@@ -90,9 +90,14 @@ func ixHandshakeStage1(f *Interface, addr *udp.Addr, via *ViaSender, packet []by
 
 	remoteCert, err := RecombineCertAndValidate(ci.H, hs.Details.Cert, f.pki.GetCAPool())
 	if err != nil {
-		f.l.WithError(err).WithField("udpAddr", addr).
-			WithField("handshake", m{"stage": 1, "style": "ix_psk0"}).WithField("cert", remoteCert).
-			Info("Invalid certificate from host")
+		e := f.l.WithError(err).WithField("udpAddr", addr).
+			WithField("handshake", m{"stage": 1, "style": "ix_psk0"})
+
+		if f.l.Level > logrus.DebugLevel {
+			e = e.WithField("cert", remoteCert)
+		}
+
+		e.Info("Invalid certificate from host")
 		return
 	}
 	vpnIp := iputil.Ip2VpnIp(remoteCert.Details.Ips[0].IP)
@@ -372,9 +377,14 @@ func ixHandshakeStage2(f *Interface, addr *udp.Addr, via *ViaSender, hh *Handsha
 
 	remoteCert, err := RecombineCertAndValidate(ci.H, hs.Details.Cert, f.pki.GetCAPool())
 	if err != nil {
-		f.l.WithError(err).WithField("vpnIp", hostinfo.vpnIp).WithField("udpAddr", addr).
-			WithField("cert", remoteCert).WithField("handshake", m{"stage": 2, "style": "ix_psk0"}).
-			Error("Invalid certificate from host")
+		e := f.l.WithError(err).WithField("vpnIp", hostinfo.vpnIp).WithField("udpAddr", addr).
+			WithField("handshake", m{"stage": 2, "style": "ix_psk0"})
+
+		if f.l.Level > logrus.DebugLevel {
+			e = e.WithField("cert", remoteCert)
+		}
+
+		e.Error("Invalid certificate from host")
 
 		// The handshake state machine is complete, if things break now there is no chance to recover. Tear down and start again
 		return true