Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Redact HTTP query values from logs #930

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Redact HTTP query values from logs #930

merged 1 commit into from
Nov 17, 2023

Conversation

austinvazquez
Copy link
Contributor

@austinvazquez austinvazquez commented Nov 9, 2023 •
edited
Loading

Issue #, if available:
N/A

Description of changes:
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.

Testing performed:
Added unit tests and benchmarks to measure overhead for redacting HTTP query values from errors in logs.

=== RUN   TestRedactHTTPQueryValuesFromError
=== RUN   TestRedactHTTPQueryValuesFromError/NilError
=== RUN   TestRedactHTTPQueryValuesFromError/NonURLError
=== RUN   TestRedactHTTPQueryValuesFromError/ErrorWithNoHTTPQuery
=== RUN   TestRedactHTTPQueryValuesFromError/ErrorWithHTTPQuery
--- PASS: TestRedactHTTPQueryValuesFromError (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromError/NilError (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromError/NonURLError (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromError/ErrorWithNoHTTPQuery (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromError/ErrorWithHTTPQuery (0.00s)
=== RUN   TestRedactHTTPQueryValuesFromURL
=== RUN   TestRedactHTTPQueryValuesFromURL/NilURL
=== RUN   TestRedactHTTPQueryValuesFromURL/URLWithNoQueries
=== RUN   TestRedactHTTPQueryValuesFromURL/URLWithQueries
--- PASS: TestRedactHTTPQueryValuesFromURL (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromURL/NilURL (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromURL/URLWithNoQueries (0.00s)
    --- PASS: TestRedactHTTPQueryValuesFromURL/URLWithQueries (0.00s)
PASS
coverage: 92.9% of statements
ok      github.com/awslabs/soci-snapshotter/util/http/log       1.017s  coverage: 92.9% of statements

goos: linux
goarch: amd64
pkg: github.com/awslabs/soci-snapshotter/util/http/log
cpu: Intel(R) Xeon(R) CPU E5-2686 v4 @ 2.30GHz
BenchmarkRedactHTTPQueryValuesOverhead
BenchmarkRedactHTTPQueryValuesOverhead/BaselineLogging
BenchmarkRedactHTTPQueryValuesOverhead/BaselineLogging-2         	   12108	    104393 ns/op	    6220 B/op	      35 allocs/op
BenchmarkRedactHTTPQueryValuesOverhead/WithoutReplacement
BenchmarkRedactHTTPQueryValuesOverhead/WithoutReplacement-2      	   10000	    108078 ns/op	    6374 B/op	      42 allocs/op
BenchmarkRedactHTTPQueryValuesOverhead/WithErrorReplacement
BenchmarkRedactHTTPQueryValuesOverhead/WithErrorReplacement-2    	   10000	    114341 ns/op	    6799 B/op	      53 allocs/op
PASS
ok  	github.com/awslabs/soci-snapshotter/util/http/log	5.549s

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@austinvazquez austinvazquez force-pushed the redact-http-query-values branch 3 times, most recently from 2d7b5ef to ee77932 Compare November 9, 2023 23:09
@austinvazquez austinvazquez marked this pull request as ready for review November 9, 2023 23:10
@austinvazquez austinvazquez requested a review from a team as a code owner November 9, 2023 23:10
turan18
turan18 previously approved these changes Nov 10, 2023
View reviewed changes
Kern--
Kern-- requested changes Nov 10, 2023
View reviewed changes
util/http/log/redact_http_query_values.go Outdated Show resolved Hide resolved
util/http/retry.go Outdated Show resolved Hide resolved
@austinvazquez austinvazquez force-pushed the redact-http-query-values branch 2 times, most recently from 3874c72 to a749a09 Compare November 13, 2023 16:53
Kern--
Kern-- requested changes Nov 13, 2023
View reviewed changes
util/http/log/redact_http_query_values_test.go Outdated Show resolved Hide resolved
util/http/log/redact_http_query_values.go Outdated Show resolved Hide resolved
util/http/retry.go Outdated Show resolved Hide resolved
@austinvazquez austinvazquez force-pushed the redact-http-query-values branch 6 times, most recently from d9b4cae to 919f5e0 Compare November 14, 2023 19:40
@austinvazquez austinvazquez force-pushed the redact-http-query-values branch from 919f5e0 to 6130136 Compare November 15, 2023 17:58
@austinvazquez
Redact HTTP query values from logs
9261f46 
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]>
@austinvazquez austinvazquez force-pushed the redact-http-query-values branch from 6130136 to 9261f46 Compare November 15, 2023 18:00
@turan18 turan18 merged commit e1c66c0 into awslabs:main Nov 17, 2023
4 checks passed
@austinvazquez austinvazquez deleted the redact-http-query-values branch November 17, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turan18 turan18 turan18 approved these changes

@Kern-- Kern-- Kern-- approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@austinvazquez @Kern-- @turan18