-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
OPA debug logs expose X-AMZ-SECURITY-TOKEN header #5848
Comments
As discussed in this thread, one option would be to extend withMaskedAuthorizationHeader to redact other headers in addition to the authorization header. |
@ashutosh-narkar I don't seem to have access to the thread. Can you either provide the access or briefly describe how to extend this method? Thanks! |
@rudrakhp as you see in https://sourcegraph.com/github.com/open-policy-agent/opa@845652e11501045706b62cb929bf3e507e4a52b0/-/blob/plugins/rest/rest.go?L335 currently we only mask the
I am not sure what masking you're referring to here. There is the ability to mask sensitive data from decision logs and this one is to mask data from OPA's debug logs. What are you trying to achieve here? |
@ashutosh-narkar thanks for the reply. To be very specific we have an use case where we want to log some part of the authorization header (only payload of JWT token for example) in the OPA decision logs instead of redacting the header completely. Is this supported currently? The reason we require this capability is to debug prod incidents without having to reproduce/replay the requests. Also ensuring the complete header is not leaked to avoid security risks. |
So if I understand correctly, the headers logged in OPA decision logs is controlled via rego and can be modified as well (similar to the usecase I pointed out above). |
You should be able to do this by specifying an |
@ashutosh-narkar can I take a look at this, if nobody is working on it. ? Shall I put a check specifically against X-AMZ-SECURITY-TOKEN header in withMaskedAuthorizationHeader method to redact it ? we can also maintain a list of headers to make this check if in future we have to hide another headers. we can just add the header name to list in that case. |
That sounds reasonable @yogisinha. |
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue. |
I also have the case where sensitive information is sent to OPA in the POST/request body. And when running with debug log level the input is logged to stdout/terminal. So it would also be great to have support for masking the information that OPA logs to stdout/terminal. |
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue. |
@ashutosh-narkar since this has gone stale, I can jump on it. I've got a flight coming up that would give me time to knock out the code and tests. If I'm reading through this right, the easiest way to address it would be to create a static slice of headers that we'd want to mask, and then look through it in withMaskedAuthorizationHeader. I'd imagine a future expansion would be to allow a user to set a configuration, but that sounds out of scope for this issue. Is that all correct? |
Correct @colinjlacy. If we extend that method to accept other headers and provide a static list to that call it should be fine for now. |
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]>
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]>
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]>
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]>
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: #5848 Signed-off-by: Colin Lacy <[email protected]>
Short description
I'm using OPA with S3 signing to retrieve bundles and with debug logs turned on. With that configuration, I see request logs that OPA makes to S3 infra. These requests have the Authorization header value overwritten to "REDACTED", but the X-AMZ-SECURITY-TOKEN header is still provided. My understanding is that this is a secret value as well and it would be preferable to have this redacted as well.
We don't normally run with debug logs turned on, but some teams do from time to time, and we don't want AWS security tokens making it into our log aggregator.
Steps To Reproduce
Expected behavior
Any request headers containing secrets should be redacted. This would at least include the Authorization header and the X-AMZ-SECURITY-TOKEN headers. Perhaps auth plugins could contribute to a list of headers that should be filtered.
The text was updated successfully, but these errors were encountered: