[tor-commits] [snowflake/master] Sanitize IP addresses from server log output

[cohosh at torproject.org]

commit f586a4bab8fc428fa93ed7523da150fc45a13533
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Wed Mar 20 15:50:55 2019 -0400

    Sanitize IP addresses from server log output
    
    Added a scrubber that takes all logging output to the standard logger
    and passes through a series of regular expressions to replace IP
    addresses with safe strings (e.g., X.X.X.X:443).
    
    Ensure server logs to stdout are also scrubbed
---
 server/server.go      | 28 ++++++++++++++++++++++++++--
 server/server_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/server/server.go b/server/server.go
index 03095af..ad1cd2a 100644
--- a/server/server.go
+++ b/server/server.go
@@ -15,6 +15,7 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 	"syscall"
@@ -54,6 +55,22 @@ additional HTTP listener on port 80 to work with ACME.
 	flag.PrintDefaults()
 }
 
+// An io.Writer that can be used as the output for a logger that first
+// sanitizes logs and then writes to the provided io.Writer
+type logScrubber struct {
+	output io.Writer
+}
+
+func (ls *logScrubber) Write(b []byte) (n int, err error) {
+	//First scrub the input of IP addresses
+	reIPv4 := regexp.MustCompile(`\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b`)
+	reIPv6 := regexp.MustCompile(`(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))`)
+	scrubbedBytes := reIPv4.ReplaceAll(b, []byte("X.X.X.X"))
+	scrubbedBytes = reIPv6.ReplaceAll(scrubbedBytes,
+		[]byte("X:X:X:X:X:X:X:X"))
+	return ls.output.Write(scrubbedBytes)
+}
+
 // An abstraction that makes an underlying WebSocket connection look like an
 // io.ReadWriteCloser.
 type webSocketConn struct {
@@ -280,8 +297,15 @@ func main() {
 			log.Fatalf("can't open log file: %s", err)
 		}
 		defer f.Close()
-		log.SetOutput(f)
-	}
+		//We want to send the log output through our scrubber first
+		scrubber := &logScrubber{f}
+		log.SetOutput(scrubber)
+	} else {
+            // we still want to send log output through our scrubber, even
+            // if no log file was specified
+            scrubber := &logScrubber{os.Stdout}
+            log.SetOutput(scrubber)
+        }
 
 	if !disableTLS && acmeHostnamesCommas == "" {
 		log.Fatal("the --acme-hostnames option is required")
diff --git a/server/server_test.go b/server/server_test.go
index 84ac7ba..c3514ed 100644
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -1,6 +1,8 @@
 package main
 
 import (
+	"bytes"
+	"log"
 	"net"
 	"strconv"
 	"testing"
@@ -47,3 +49,26 @@ func TestClientAddr(t *testing.T) {
 		}
 	}
 }
+
+func TestLogScrubber(t *testing.T) {
+
+	var buff bytes.Buffer
+	scrubber := &logScrubber{&buff}
+	log.SetFlags(0) //remove all extra log output for test comparisons
+	log.SetOutput(scrubber)
+
+	log.Printf("%s", "http: TLS handshake error from 129.97.208.23:38310:")
+
+	if bytes.Compare(buff.Bytes(), []byte("http: TLS handshake error from X.X.X.X:38310:\n")) != 0 {
+		t.Errorf("log scrubber didn't scrub IPv4 address. Output: %s", string(buff.Bytes()))
+	}
+	buff.Reset()
+
+	log.Printf("%s", "http2: panic serving [2620:101:f000:780:9097:75b1:519f:dbb8]:58344: interface conversion: *http2.responseWriter is not http.Hijacker: missing method Hijack")
+
+	if bytes.Compare(buff.Bytes(), []byte("http2: panic serving [X:X:X:X:X:X:X:X]:58344: interface conversion: *http2.responseWriter is not http.Hijacker: missing method Hijack\n")) != 0 {
+		t.Errorf("log scrubber didn't scrub IPv6 address. Output: %s", string(buff.Bytes()))
+	}
+	buff.Reset()
+
+}