Skip to content

Conversation

@ChinmayHegde24
Copy link
Contributor

Currently, when the handleEncryptedKeyOp() method in KMS.java is invoked via the API, there is no visibility into whether the request is performing a decryptKey or reEncryptKey operation as same path gets logged for both operations as it hits the same method.
So after this patch opCode will be included in AccessLog pattern.

e.g In access logs we can see opcode appended
- - [] "GET /kms/v1/keys/names" 200 3 50 -
- - [] "GET /kms/v1/key/key1/_eek" 500 288 29 generate
- - [] "POST /kms/v1/keyversion/test-2@1/_eek" 400 176 16 reencrypt
- - [] "POST /kms/v1/keyversion/test-2@1/_eek" 400 176 14 decrypt

How was this patch tested?

Checked Access logs after hitting endpoints through Docker setup
Mvn local build

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances KMS access logging by capturing operation codes (opCode) for EEK (Encrypted Encryption Key) operations to distinguish between decrypt and reencrypt operations in access logs.

  • Modifies the KMSMDCFilter to extract and store the eek_op parameter from query strings for /_eek endpoints
  • Updates the Tomcat access log pattern to include the operation code in KMS server logs
  • Adds comprehensive test coverage for the new filtering functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
KMSMDCFilter.java Adds logic to parse and store eek_op query parameter as request attribute
TestKMSMDCFilter.java Adds test cases for new eek_op parameter handling functionality
EmbeddedServer.java Updates KMS access log pattern to include eek_op request attribute

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants