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

Suspicious overly permissive KMS key policy created - Rule Tuning #11296

Open
r4nd0mlyCh0sen opened this issue Oct 17, 2024 · 2 comments
Open
Assignees

Comments

@r4nd0mlyCh0sen
Copy link

Describe the bug
In the analytics rule named Suspicious overly permissive KMS key policy created, it doesn't appear to alert on any of the activity. It appears that this is due to attempting to match (using ==) kms:Encrypt or kms:* while these values are contained within an array.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Sentinel -> Analytics Rules
  2. Click on Suspicious overly permissive KMS key policy created
  3. Navigate to the Query logic
  4. Attempt to run the query while the events are within your logs.

Expected behavior
I expect this rule to fire when the logic is met.

Additional context
This appears to be a better version of the query:

//A variable to contain the actions we want to monitor
let kmsActions = dynamic(["kms:Encrypt", "kms:"]);
AWSCloudTrail
| where EventName in ("CreateKey", "PutKeyPolicy")
and isempty(ErrorCode)
and isempty(ErrorMessage)
| extend Statement = parse_json(tostring((parse_json(RequestParameters).policy))).Statement
| mvexpand Statement
| extend
Action = tostring(parse_json(Statement).Action),
Effect = tostring(parse_json(Statement).Effect),
Principal = iff(isnotempty(tostring(parse_json(Statement).Principal.AWS)), tostring(parse_json(Statement).Principal.AWS), tostring(parse_json(Statement).Principal))
| where Effect =~ "Allow"
//This is where it is attempting to match the exact values contained within the array:
// and (Action == "kms:Encrypt" or Action == "kms:
")
//This is the change I have made.
and Action has_any (kmsActions)
and Principal == "*"
| extend UserIdentityArn = iif(isempty(UserIdentityArn), tostring(parse_json(Resources)[0].ARN), UserIdentityArn)
| extend UserName = tostring(split(UserIdentityArn, '/')[-1])
| extend AccountName = case(UserIdentityPrincipalid == "Anonymous", "Anonymous", isempty(UserIdentityUserName), UserName, UserIdentityUserName)
| extend
AccountName = iif(AccountName contains "@", tostring(split(AccountName, '@', 0)[0]), AccountName),
AccountUPNSuffix = iif(AccountName contains "@", tostring(split(AccountName, '@', 1)[0]), "")
| extend timestamp = TimeGenerated

@v-rusraut
Copy link
Contributor

Hi @r4nd0mlyCh0sen, thanks for flagging this issue, we will investigate this issue and get back to you with some updates. Thanks!

@v-rusraut
Copy link
Contributor

Hi @r4nd0mlyCh0sen , please provide sample logs to reproduce this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants