Redact HTTP query values from logs

HTTP client logs are mostly disabled with the exception of a request retry log. The issue observed is that error messages may contain the full HTTP request including the query component which can contain sensitive information like credentials or session tokens. To prevent leaking sensitive information, this change will redact HTTP query values from log messages. Signed-off-by: Austin Vazquez <[email protected]>
awslabs · Nov 13, 2023 · 3874c72 · 3874c72
1 parent c68b5c4
commit 3874c72
Show file tree

Hide file tree

Showing 3 changed files with 212 additions and 1 deletion.
diff --git a/util/http/log/redact_http_query_values.go b/util/http/log/redact_http_query_values.go
@@ -0,0 +1,45 @@
+/*
+   Copyright The Soci Snapshotter Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package log
+
+import (
+	"errors"
+	"net/url"
+)
+
+// RedactHTTPQueryValues parses an error and replaces any occurrences of an HTTP Query
+// value with 'redacted' to prevent leakage of sensitive information like credentials.
+func RedactHTTPQueryValues(err error) error {
+	var urlErr *url.Error
+
+	if err != nil && errors.As(err, &urlErr) {
+		url, urlParseErr := url.Parse(urlErr.URL)
+		if urlParseErr != nil {
+			urlErr.URL = ""
+		} else {
+			query := url.Query()
+			for k := range query {
+				query.Set(k, "redacted")
+			}
+			url.RawQuery = query.Encode()
+			urlErr.URL = url.String()
+		}
+		return urlErr
+	}
+
+	return err
+}
diff --git a/util/http/log/redact_http_query_values_test.go b/util/http/log/redact_http_query_values_test.go
@@ -0,0 +1,165 @@
+/*
+   Copyright The Soci Snapshotter Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package log
+
+import (
+	"bytes"
+	"errors"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+)
+
+func TestRedactHTTPQueryValues(t *testing.T) {
+	testcases := []struct {
+		Name        string
+		Description string
+		Err         error
+		Assert      func(*testing.T, error)
+	}{
+		{
+			Name:        "NilError",
+			Description: "Utility should not modify an error if the value is nil.",
+			Err:         nil,
+			Assert: func(t *testing.T, actual error) {
+				if actual != nil {
+					t.Fatalf("Expected nil error, got %v", actual)
+				}
+			},
+		},
+		{
+			Name:        "NonURLError",
+			Description: "Utility should not modify an error if error is not a URL error",
+			Err:         errors.New("this error is not a URL error"),
+			Assert: func(t *testing.T, actual error) {
+				const expected = "this error is not a URL error"
+				if strings.Compare(expected, actual.Error()) != 0 {
+					t.Fatalf("Expected '%s', got %v", expected, actual)
+				}
+			},
+		},
+		{
+			Name:        "ErrorWithNoHTTPQuery",
+			Description: "Utility should not modify an error if no HTTP queries are present.",
+			Err: &url.Error{
+				Op:  "GET",
+				URL: "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb",
+				Err: errors.New("connect: connection refused"),
+			},
+			Assert: func(t *testing.T, actual error) {
+				const expected = "GET \"https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb\": connect: connection refused"
+				if strings.Compare(expected, actual.Error()) != 0 {
+					t.Fatalf("Expected '%s', got %v", expected, actual)
+				}
+			},
+		},
+		{
+			Name:        "ErrorWithHTTPQuery",
+			Description: "Utility should redact HTTP query values in errors to prevent logging sensitive information.",
+			Err: &url.Error{
+				Op:  "GET",
+				URL: "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb?username=admin&password=admin",
+				Err: errors.New("connect: connection refused"),
+			},
+			Assert: func(t *testing.T, actual error) {
+				const expected = "GET \"https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb?password=redacted&username=redacted\": connect: connection refused"
+				if strings.Compare(expected, actual.Error()) != 0 {
+					t.Fatalf("Expected '%s', got '%v'", expected, actual)
+				}
+			},
+		},
+	}
+
+	// GET "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb?username=redacted&password=redacted": connect: connection refused
+	// GET "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb?password=redacted&username=redacted": connect: connection refused
+
+	for _, testcase := range testcases {
+		t.Run(testcase.Name, func(t *testing.T) {
+			actual := RedactHTTPQueryValues(testcase.Err)
+			testcase.Assert(t, actual)
+		})
+	}
+}
+
+func BenchmarkRedactHTTPQueryValuesOverhead(b *testing.B) {
+	benchmarks := []struct {
+		Name        string
+		Description string
+		Err         error
+		Log         func(*logrus.Entry, error)
+	}{
+		{
+			Name:        "BaselineLogging",
+			Description: "Log a message to memory without redaction to measure baseline.",
+			Err: &url.Error{
+				Op:  "GET",
+				URL: "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb?username=admin&password=admin",
+				Err: errors.New("connect: connection refused"),
+			},
+			Log: func(logger *logrus.Entry, err error) {
+				logger.WithError(err).Info("Error on HTTP Get")
+			},
+		},
+		{
+			Name:        "WithoutReplacement",
+			Description: "Log a message with no HTTP query values to memory with redaction utility to measure the flat overhead.",
+			Err: &url.Error{
+				Op:  "GET",
+				URL: "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb",
+				Err: errors.New("connect: connection refused"),
+			},
+			Log: func(logger *logrus.Entry, err error) {
+				logger.WithError(RedactHTTPQueryValues(err)).Info("Error on HTTP Get")
+			},
+		},
+		{
+			Name:        "WithErrorReplacement",
+			Description: "Log a message with HTTP query values to memory with redaction utility to measure replacement overhead.",
+			Err: &url.Error{
+				Op:  "GET",
+				URL: "https://s3.us-east-1.amazonaws.com/981ebdad55863b3631dce86a228a3ea230dc87673a06a7d216b1275d4dd707c9/12d7153d7eee2fd595a25e5378384f1ae4b6a1658298a54c5bd3f951ec50b7cb?username=admin&password=admin",
+				Err: errors.New("connect: connection refused"),
+			},
+			Log: func(logger *logrus.Entry, err error) {
+				logger.WithError(RedactHTTPQueryValues(err)).Info("Error on HTTP Get")
+			},
+		},
+	}
+
+	setupUUT := func() *logrus.Entry {
+		entry := &logrus.Entry{
+			Logger: logrus.New(),
+		}
+
+		entry.Logger.Out = bytes.NewBuffer([]byte{})
+
+		return entry
+	}
+
+	for _, benchmark := range benchmarks {
+		b.Run(benchmark.Name, func(b *testing.B) {
+			uut := setupUUT()
+			b.ResetTimer()
+
+			for i := 0; i < b.N; i++ {
+				benchmark.Log(uut, benchmark.Err)
+			}
+		})
+	}
+}
diff --git a/util/http/retry.go b/util/http/retry.go
@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"github.com/awslabs/soci-snapshotter/config"
+	logutil "github.com/awslabs/soci-snapshotter/util/http/log"
 	"github.com/awslabs/soci-snapshotter/version"
 	"github.com/containerd/log"
 	rhttp "github.com/hashicorp/go-retryablehttp"
@@ -83,7 +84,7 @@ func RetryStrategy(ctx context.Context, resp *http.Response, err error) (bool, e
 	retry, err2 := rhttp.DefaultRetryPolicy(ctx, resp, err)
 	if retry {
 		log.G(ctx).WithFields(logrus.Fields{
-			"error":    err,
+			"error":    logutil.RedactHTTPQueryValues(err),
 			"response": resp,
 		}).Debugf("retrying request")
 	}