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

plugins/rest: masks more headers in decision logs #6421

Conversation

colinjlacy
Copy link
Contributor

Why the changes in this PR are needed?

Decision logs had previously been configured to hide the value of the Authorization header, as that is considered sensitive information. However, there are cases when additional headers are provided that contain sensitive information, such as the X-AMZ-SECURITY-TOKEN header. These values were being logged in plaintext, despite being equally sensitive.

What are the changes in this PR?

This PR creates an internal map of headers that should be masked, which can be expanded if additional headers are required. It then loops over the headers in a request, and performs a lookup on the internal map to see if any of them match those that should be masked. If so, it replaces their values with "REDACTED". An existing test was added to check both the header keys that should be masked, as well as a key that should not.

An existing test was modified to check both the header keys that should be masked, as well as a test key that should not.

Notes to assist PR review:

Implementation for this change was discussed in this comment.

Nothing else comes to mind. I've got the basics - tests pass, make check went well, etc. If I'm missing something, please let me know.

Further comments:

Additional work, out of scope for this PR, would be to open a config setting that would allow users to pass in a list of headers that should be masked.

Fixes: #5848

Copy link

netlify bot commented Nov 19, 2023

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit d002717
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/655a307aee9fe400084d52c1
😎 Deploy Preview https://deploy-preview-6421--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

colinjlacy and others added 6 commits November 20, 2023 06:30
Decision logs had previously been configured to hide the value of the
Authorization header, as that is considered sensitive information.
However, there are cases when additional headers are provided that
contain sensitive information, such as the X-AMZ-SECURITY-TOKEN header.
This PR creates an internal map of headers that should be masked, which
can be expanded if additional headers are required. It then loops over
the headers in a request, and performs a lookup on the internal map
to see if any of them match those that should be masked. If so, it
replaces their values with "REDACTED". An existing test was added to
check both the header keys that should be masked, as well as a key
that should not.

Additional work, out of scope for this PR, would be to open a config
setting that would allow users to pass in a list of headers that should
be masked.

Fixes: open-policy-agent#5848

Signed-off-by: Colin Lacy <[email protected]>
Add two environment variable tests which illustrate how an environment variable can be used in OPA to verify a JWT.

Signed-off-by: Robert Hafner <[email protected]>
Signed-off-by: Colin Lacy <[email protected]>
Bumps the go-opentelemetry-io group with 3 updates: [go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp](https://github.com/open-telemetry/opentelemetry-go-contrib), [go.opentelemetry.io/otel/exporters/otlp/otlptrace](https://github.com/open-telemetry/opentelemetry-go) and [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc](https://github.com/open-telemetry/opentelemetry-go).

Updates `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` from 0.46.0 to 0.46.1
- [Release notes](https://github.com/open-telemetry/opentelemetry-go-contrib/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go-contrib@zpages/v0.46.0...zpages/v0.46.1)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlptrace` from 1.20.0 to 1.21.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.20.0...v1.21.0)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc` from 1.20.0 to 1.21.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.20.0...v1.21.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go-opentelemetry-io
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlptrace
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-opentelemetry-io
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-opentelemetry-io
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Colin Lacy <[email protected]>
… render templated strings

This adds support for rendering of templated strings utilizing Golang's text/template library.
For a given templated string and key/value mapping of template var inputs, this builtin will
inject the values into the template where they are referenced by key.

Fixes open-policy-agent#6371
Signed-off-by: Rohan Vasavada <[email protected]>
Signed-off-by: Colin Lacy <[email protected]>
Signed-off-by: Colin Lacy <[email protected]>
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.

OPA debug logs expose X-AMZ-SECURITY-TOKEN header
3 participants