Skip to content

fix: prevent sensitive data exposure in KMS error logs - #1217

Open
hsinn0 wants to merge 2 commits into
mainfrom
fix/tnzgov-447-kms-error-logging
Open

hsinn0 wants to merge 2 commits into
mainfrom
fix/tnzgov-447-kms-error-logging

Conversation

@hsinn0

@hsinn0 hsinn0 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Log only the operation and gRPC status code on encryption and decryption failures. Add regression coverage for request payloads and secret-bearing backend errors.

ai-assisted=yes
TNZGOV-447

Log only the operation and gRPC status code on encryption and decryption failures. Add regression coverage for request payloads and secret-bearing backend errors.

ai-assisted=yes
TNZGOV-447
LOGGER.error("Error for request: " + request.getPlain(), e);
// KMS error descriptions and causes can also contain credential data.
LOGGER.error("KMS encryption failed with status {}", e.getStatus().getCode());
throw e;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The credential data is most likely still in this exception that is getting thrown back to callers. Can you confirm how the callers handle / log these exceptions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out.
Analyzed the call stacks with Codex. The exception ends up in DefaultExceptionHandler (components/errors/src/main/kotlin/org/cloudfoundry/credhub/generate/DefaultExceptionHandler.kt:18), which logs the complete exception, including causes. If the KMS includes a secret in its error description, that secret can reach application logs.
Made new commit to address that.

}
LOGGER.error("Error for request: " + request.getCipher(), e);
LOGGER.error("KMS decryption failed with status {}", e.getStatus().getCode());
throw e;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in the new commit.

Preserve gRPC status codes while removing backend descriptions, causes, and trailers. Use a fixed invalid-ciphertext message and verify that wrapped exceptions cannot expose backend secrets.

ai-assisted=yes
TNZGOV-447
@hsinn0
hsinn0 requested a review from dlinsley October 22, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants