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.

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

Redact sensitive information in redirected URLs #1408

Merged
merged 2 commits into from
Jul 18, 2022

Conversation

imjasonh
Copy link
Collaborator

@imjasonh imjasonh commented Jul 18, 2022

This adds internal methods to redact potentially sensitive information
from URLs in error messages, especially when those URLs are the result
of server-side redirects.

We already redact potentially sensitive information like this as part of
transport.CheckError, but this isn't called when the error is the result
of an http.Client.Do (e.g., tcp dial error).

The specific use case where this can happen is a registry like GCR which
redirects blob requests to GCS with a sensitive access_token in the
query parameter. If the request to GCS fails due to tcp error, the error
message will include the sensitive access token.

This method of redaction relies on the original error being a
*url.Error, and redaction is accomplished by simply updating the error's
URL with a redacted equivalent.

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2022

Codecov Report

Merging #1408 (38c0518) into main (d187a71) will increase coverage by 0.00%.
The diff coverage is 50.00%.

@@           Coverage Diff           @@
##             main    #1408   +/-   ##
=======================================
  Coverage   73.40%   73.40%           
=======================================
  Files         115      115           
  Lines        8758     8747   -11     
=======================================
- Hits         6429     6421    -8     
+ Misses       1687     1685    -2     
+ Partials      642      641    -1     
Impacted Files Coverage Δ
pkg/v1/remote/descriptor.go 73.50% <33.33%> (+1.11%) ⬆️
pkg/v1/remote/transport/error.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d187a71...38c0518. Read the comment docs.

Copy link
Collaborator

@jonjohnsonjr jonjohnsonjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just update the commit message, PR description, and godoc to reflect the new reality.

This adds internal methods to redact potentially sensitive information
from URLs in error messages, especially when those URLs are the result
of server-side redirects.

We already redact potentially sensitive information like this as part of
transport.CheckError, but this isn't called when the error is the result
of an http.Client.Do (e.g., tcp dial error).

The specific use case where this can happen is a registry like GCR which
redirects blob requests to GCS with a sensitive access_token in the
query parameter. If the request to GCS fails due to tcp error, the error
message will include the sensitive access token.

This method of redaction relies on the original error being a
*url.Error, and redaction is accomplished by simply updating the error's
URL with a redacted equivalent.
@imjasonh imjasonh merged commit 53e6bea into google:main Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants