aws.cloudtrail: improve CloudTrail user identity processing

[chemamartinez]

Proposed commit message

For CloudTrail events, it has been updated how IAM users are handled.

In particular, for the user identity IAMUser type, the user.name and user.id are
populated with the user fields that made the action/request.

For the user identity AssumedRole type, AWS SIEM rules need roles to be treated as IAMUsers in order to work fine. So the role identifies inside sessionIssuer populate user.* fields. Then, the session name is being mapped as user.changes.name as it can be interpreted as the name the user is taking for that particular session, and it's the closest approach in ECS.

References:

• https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-event-reference-user-identity.html
• https://www.elastic.co/docs/reference/ecs/ecs-user-usage#ecs-user-usage-combining

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.
• I have verified that any added dashboard complies with Kibana's Dashboard good practices

Related issues

• Relates https://github.com/elastic/enhancements/issues/25241

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[elastic-vault-github-plugin-prod]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[efd6]

This seems more odd, since this is a session now.

[chemamartinez]

From this same conversation, the goal is to consider the session name as temporary user name, that's why it is being mapped as user.changes.name and therefore included into related.users.

[imays11]

Again, I'd add the "identityStoreArn\":\"arn:aws:identitystore::redacted:identitystore/redacted\" value here

[imays11]

This is going to prevent proper mapping for FederatedUser type, which also provides the userIdentity.sessionContext.sessionIssuer.userName field that needs to continue to be mapped to user.name field

[imays11]

Same here, this change should apply to FederateUser type as well

[imays11]

We should use this to also map the user.changes.name value for FederatedUser identity type. Example: aws.cloudtrail.user_identity.arn : "arn:aws:sts::123445667789:federated-user/fed_consoler" . The arn is created a little differently from Assumed Role since it doesn't keep the calling user's name in the arn but fed_consoler is the name change that should be extracted and treated just like we are treated the session.name for AssumedRole identity type.

[chemamartinez]

@imays11 @Mikaayenson I have a question for you, the latest changes to the change request respond to Isai's suggestions to avoid breaking the existing rules, so role names are still mapped as user.name and the only change would be to assign the session name to user.changes.name, which I don't think should be a problem for the rules. Correct me if I'm wrong.

Do you still agree with these changes? I understand that the most painful change for you would be to separate roles and user names into different fields. If so, assigning some-role to user.roles would break the rules, right?

[imays11]

@chemamartinez

@imays11 @Mikaayenson I have a question for you, the latest changes to the change request respond to Isai's suggestions to avoid breaking the existing rules, so role names are still mapped as user.name and the only change would be to assign the session name to user.changes.name, which I don't think should be a problem for the rules. Correct me if I'm wrong.

Yes I still agree with those changes

Do you still agree with these changes? I understand that the most painful change for you would be to separate roles and user names into different fields. If so, assigning some-role to user.roles would break the rules, right?

Yes that is correct, if the some-role > user.name mapping is removed that would require us to duplicate a set of new terms rules to utilize the new field mapping for roles.

[imays11]

@chemamartinez
Have you considered using cloud.entity.name as a field that can be used for both IAM Users and Roles?
https://www.elastic.co/docs/reference/ecs/ecs-cloud#ecs-cloud-nestings
https://www.elastic.co/docs/reference/ecs/ecs-entity#field-entity-name

[chemamartinez]

Yes I still agree with those changes

In that case, if no more changes are required the PR is ready for review I think.

Yes that is correct, if the some-role > user.name mapping is removed that would require us to duplicate a set of new terms rules to utilize the new field mapping for roles.
Have you considered using cloud.entity.name as a field that can be used for both IAM Users and Roles?

@imays11 aren't these two comments contradictory? If we now map the fields to cloud.entity.name instead of keeping using user.name all rules should be rewritten, isn't it?

[imays11]

Yes that is correct, if the some-role > user.name mapping is removed that would require us to duplicate a set of new terms rules to utilize the new field mapping for roles.
Have you considered using cloud.entity.name as a field that can be used for both IAM Users and Roles?

@imays11 aren't these two comments contradictory? If we now map the fields to cloud.entity.name instead of keeping using user.name all rules should be rewritten, isn't it?

@chemamartinez
No, as long as both user identity types are under the same field (user.name OR cloud.entity.name), then we can simply do a rule tuning of the existing rules and replace the user.name field to use cloud.entity.name field instead.

The problem comes with keeping IAM Users under user.name but moving Roles to a different field like user.roles. That would require us to create a duplicate set of rules in order to utilize 2 different fields.

I'm fine keeping both under user.name, I only gave the suggestion because I know there was some concern of "long-term semantic weakening of an ECS field definition" by keeping Roles under user.name field.

[chemamartinez]

@imays11 thanks for the clarification!

[efd6]

Am I correct in understanding that this change is still going to conflate roles with users? If that's the case, I still do not agree, but I am not going to block this.

[kcreddy]

@chemamartinez Have you considered using cloud.entity.name as a field that can be used for both IAM Users and Roles? https://www.elastic.co/docs/reference/ecs/ecs-cloud#ecs-cloud-nestings https://www.elastic.co/docs/reference/ecs/ecs-entity#field-entity-name

One problem is that the field entity.name is in Beta and may not be ideal for GA integration. Also, the field is introduced in ECS version 9.1 while aws integration uses 8.11. Changing it to 9.1 might require approvals from all owners of aws integration.

[chemamartinez]

Am I correct in understanding that this change is still going to conflate roles with users? If that's the case, I still do not agree, but I am not going to block this.

@efd6 yes, but this is not something in this PR. It is something it was happening before so the SIEM rules work.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 058af15

History

• 💚 Build #34312 succeeded 352a6de
• 💚 Build #33251 succeeded 3e7f4c9
• 💚 Build #32950 succeeded 6f4b104
• 💚 Build #32490 succeeded 6b2aa19

cc @chemamartinez

[agithomas]

LGTM, considering the design approach taken! Code owner approval.

[elastic-vault-github-plugin-prod]

Package aws - 5.2.0 containing this change is available at https://epr.elastic.co/package/aws/5.2.0/