commit 1ea467c4cf950b571ccb25b434b6cb29682fef07 Author: Cecylia Bocovich cohosh@torproject.org Date: Mon Mar 25 09:59:20 2019 -0400

Restructured scrubbing code and tests

It is now more readable, and the regexp's are only compiled once --- server/server.go | 34 +++++++++++---------- server/server_test.go | 85 +++++++++++++++++++++++++++++++-------------------- 2 files changed, 70 insertions(+), 49 deletions(-)

diff --git a/server/server.go b/server/server.go index 74cffd6..decfed4 100644 --- a/server/server.go +++ b/server/server.go @@ -55,6 +55,13 @@ additional HTTP listener on port 80 to work with ACME. flag.PrintDefaults() }

+var scrubberPatterns = []*regexp.Regexp{ + /* IPv6 */ + regexp.MustCompile(`[([0-9a-fA-F]{0,4}:){2,7}([0-9a-fA-F]{0,4})?(\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3})?]`), + /* IPv4 */ + regexp.MustCompile(`\b\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3}\b`), +} + // 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 { @@ -62,13 +69,11 @@ type logScrubber struct { }

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`) - //Note that for embedded IPv4 address, the previous regex will scrub it - reIPv6 := regexp.MustCompile(`([0-9a-fA-F]{0,4}:){2,7}([0-9a-fA-F]{0,4})?`) - scrubbedBytes := reIPv4.ReplaceAll(b, []byte("X.X.X.X")) - scrubbedBytes = reIPv6.ReplaceAll(scrubbedBytes, - []byte("X:X:X:X:X:X:X:X")) + scrubbedBytes := b + for _, pattern := range scrubberPatterns { + scrubbedBytes = pattern.ReplaceAll(scrubbedBytes, []byte("[scrubbed]")) + } + return ls.output.Write(scrubbedBytes) }

@@ -292,21 +297,18 @@ func main() { flag.Parse()

log.SetFlags(log.LstdFlags | log.LUTC) + + var logOutput io.Writer = os.Stderr if logFilename != "" { f, err := os.OpenFile(logFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) if err != nil { log.Fatalf("can't open log file: %s", err) } defer f.Close() - //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) - } + logOutput = f + } + //We want to send the log output through our scrubber first + log.SetOutput(&logScrubber{logOutput})

if !disableTLS && acmeHostnamesCommas == "" { log.Fatal("the --acme-hostnames option is required") diff --git a/server/server_test.go b/server/server_test.go index 537360a..cfe4653 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -51,39 +51,58 @@ 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:") - - //Example IPv4 address that ended up in log - 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") - - //Example IPv6 address that ended up in log - 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())) + for _, test := range []struct { + input, expected string + }{ + { + "http: TLS handshake error from 129.97.208.23:38310:", + "http: TLS handshake error from [scrubbed]:38310:\n", + }, + { + "http2: panic serving [2620:101:f000:780:9097:75b1:519f:dbb8]:58344: interface conversion: *http2.responseWriter is not http.Hijacker: missing method Hijack", + "http2: panic serving [scrubbed]:58344: interface conversion: *http2.responseWriter is not http.Hijacker: missing method Hijack\n", + }, + { + //Make sure it doesn't scrub fingerprint + "a=fingerprint:sha-256 33:B6:FA:F6:94:CA:74:61:45:4A:D2:1F:2C:2F:75:8A:D9:EB:23:34:B2:30:E9:1B:2A:A6:A9:E0:44:72:CC:74", + "a=fingerprint:sha-256 33:B6:FA:F6:94:CA:74:61:45:4A:D2:1F:2C:2F:75:8A:D9:EB:23:34:B2:30:E9:1B:2A:A6:A9:E0:44:72:CC:74\n", + }, + { + "[1::]:58344", + "[scrubbed]:58344\n", + }, + { + "[1:2:3:4:5:6::8]", + "[scrubbed]\n", + }, + { + "[1::7:8]", + "[scrubbed]\n", + }, + { + "[::4:5:6:7:8]", + "[scrubbed]\n", + }, + { + "[::255.255.255.255]", + "[scrubbed]\n", + }, + { + "[::ffff:0:255.255.255.255]", + "[scrubbed]\n", + }, + { + "[2001:db8:3:4::192.0.2.33]", + "[scrubbed]\n", + }, + } { + var buff bytes.Buffer + log.SetFlags(0) //remove all extra log output for test comparisons + log.SetOutput(&logScrubber{&buff}) + log.Print(test.input) + if buff.String() != test.expected { + t.Errorf("%q: got %q, expected %q", test.input, buff.String(), test.expected) + } } - buff.Reset() - - //Testing IPv6 edge cases - log.Printf("%s", "[1::]:58344") - log.Printf("%s", "[1:2:3:4:5:6::8]:58344") - log.Printf("%s", "[1::7:8]:58344") - log.Printf("%s", "[::4:5:6:7:8]:58344") - log.Printf("%s", "[::255.255.255.255]:58344") - log.Printf("%s", "[::ffff:0:255.255.255.255]:58344") - log.Printf("%s", "[2001:db8:3:4::192.0.2.33]:58344")

- if bytes.Compare(buff.Bytes(), []byte("[X:X:X:X:X:X:X:X]:58344\n[X:X:X:X:X:X:X:X]:58344\n[X:X:X:X:X:X:X:X]:58344\n[X:X:X:X:X:X:X:X]:58344\n[X:X:X:X:X:X:X:XX.X.X.X]:58344\n[X:X:X:X:X:X:X:XX.X.X.X]:58344\n[X:X:X:X:X:X:X:XX.X.X.X]:58344\n")) != 0 { - t.Errorf("log scrubber didn't scrub IPv6 address. Output: %s", string(buff.Bytes())) - } - buff.Reset() }