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

Adding support for determining the last used date of an access keys #1101

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cartography/data/indexes.cypher
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ CREATE INDEX IF NOT EXISTS FOR (n:AWSUser) ON (n.name);
CREATE INDEX IF NOT EXISTS FOR (n:AWSUser) ON (n.lastupdated);
CREATE INDEX IF NOT EXISTS FOR (n:AWSVpc) ON (n.id);
CREATE INDEX IF NOT EXISTS FOR (n:AWSVpc) ON (n.lastupdated);
CREATE INDEX IF NOT EXISTS FOR (n:AccountAccessKey) ON (n.accesskeyid);
CREATE INDEX IF NOT EXISTS FOR (n:AccountAccessKey) ON (n.lastupdated);
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add an additional property id, which is will be equivalent to accesskeyid

Copy link
Author

Choose a reason for hiding this comment

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

I think this might be on a previous version of the code - as it currently stands the following is the code

CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid);
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.lastupdated);

Copy link
Collaborator

Choose a reason for hiding this comment

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

this is what I mean, along with another suggestion below upon MERGE

Suggested change
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid);
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.id);
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid);

CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.lastupdated);
CREATE INDEX IF NOT EXISTS FOR (n:AutoScalingGroup) ON (n.arn);
CREATE INDEX IF NOT EXISTS FOR (n:AutoScalingGroup) ON (n.lastupdated);
CREATE INDEX IF NOT EXISTS FOR (n:ChromeExtension) ON (n.id);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{
"statements": [{
"query": "MATCH (n:AccountAccessKey)<-[:AWS_ACCESS_KEY]-(:AWSUser)<-[:RESOURCE]-(:AWSAccount{id: $AWS_ID}) WHERE n.lastupdated <> $UPDATE_TAG WITH n LIMIT $LIMIT_SIZE DETACH DELETE (n)",
"iterative": true,
"iterationsize": 100
}],
"name": "cleanup AccountAccessKey"
"statements": [
{
"query": "MATCH (n:UserAccessKey)<-[:AWS_ACCESS_KEY]-(:AWSUser)<-[:RESOURCE]-(:AWSAccount{id: $AWS_ID}) WHERE n.lastupdated <> $UPDATE_TAG WITH n LIMIT $LIMIT_SIZE DETACH DELETE (n)",
"iterative": true,
"iterationsize": 100
}
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

cleanup the relationship as well

Suggested change
],
{
"query": "MATCH (UserAccessKey)<-[r:AWS_ACCESS_KEY]-(:AWSUser)<-[:RESOURCE]-(:AWSAccount{id: $AWS_ID}) WHERE r.lastupdated <> $UPDATE_TAG WITH n LIMIT $LIMIT_SIZE DELETE (r)",
"iterative": true,
"iterationsize": 100
}
],

Copy link
Author

Choose a reason for hiding this comment

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

not really sure what the changes being outlined here are

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is just the standard for cartography.

  1. cleanup stale nodes
  2. cleanup stale relationships that are still attached to fresh nodes

This shouldn't really be needed for access keys, since a given access key should only ever relate to one user in its lifetime. But you never know.

"name": "cleanup UserAccessKey"
}
37 changes: 28 additions & 9 deletions cartography/intel/aws/iam.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,18 @@ def get_account_access_key_data(boto3_session: boto3.session.Session, username:
logger.warning(
f"Could not get access key for user {username} due to NoSuchEntityException; skipping.",
)
# We add Last Used data to each access key
access_keys_metadata = access_keys['AccessKeyMetadata']
for access_key in access_keys_metadata:
access_key_id = access_key['AccessKeyId']
last_used_result = client.get_access_key_last_used(AccessKeyId=access_key_id)
last_used_info = last_used_result.get(
'AccessKeyLastUsed',
{"LastUsedDate": None, "LastUsedService": None, "LastUsedRegion": None}
)
access_key['LastUsedDate'] = last_used_info.get('LastUsedDate', None)
access_key['LastUsedService'] = last_used_info.get('ServiceName', None)
access_key['LastUsedRegion'] = last_used_info.get('Region', None)
return access_keys


Expand All @@ -252,7 +264,7 @@ def load_users(
ingest_user,
ARN=user["Arn"],
USERID=user["UserId"],
CREATE_DATE=str(user["CreateDate"]),
CREATE_DATE=user["CreateDate"],
USERNAME=user["UserName"],
PATH=user["Path"],
PASSWORD_LASTUSED=str(user.get("PasswordLastUsed", "")),
Expand Down Expand Up @@ -281,7 +293,7 @@ def load_groups(
ingest_group,
ARN=group["Arn"],
GROUP_ID=group["GroupId"],
CREATE_DATE=str(group["CreateDate"]),
CREATE_DATE=group["CreateDate"],
GROUP_NAME=group["GroupName"],
PATH=group["Path"],
AWS_ACCOUNT_ID=current_aws_account_id,
Expand Down Expand Up @@ -358,7 +370,7 @@ def load_roles(
ingest_role,
Arn=role["Arn"],
RoleId=role["RoleId"],
CreateDate=str(role["CreateDate"]),
CreateDate=role["CreateDate"],
RoleName=role["RoleName"],
Path=role["Path"],
AWS_ACCOUNT_ID=current_aws_account_id,
Expand Down Expand Up @@ -480,13 +492,17 @@ def sync_assumerole_relationships(

@timeit
def load_user_access_keys(neo4j_session: neo4j.Session, user_access_keys: Dict, aws_update_tag: int) -> None:
# TODO change the node label to reflect that this is a user access key, not an account access key
ingest_account_key = """
ingest_user_key = """
MATCH (user:AWSUser{arn: $UserARN})
WITH user
MERGE (key:AccountAccessKey{accesskeyid: $AccessKeyId})
MERGE (key:UserAccessKey{accesskeyid: $AccessKeyId})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
MERGE (key:UserAccessKey{accesskeyid: $AccessKeyId})
MERGE (key:UserAccessKey{accesskeyid: $AccessKeyId, id:$AccessKeyId})

ON CREATE SET key.firstseen = timestamp(), key.createdate = $CreateDate
SET key.status = $Status, key.lastupdated = $aws_update_tag
SET
key.status = $Status,
key.lastuseddate = $LastUsedDate,
key.lastusedservice = $LastUsedService,
key.lastusedregion = $LastUsedRegion,
key.lastupdated = $aws_update_tag
WITH user,key
MERGE (user)-[r:AWS_ACCESS_KEY]->(key)
ON CREATE SET r.firstseen = timestamp()
Expand All @@ -497,11 +513,14 @@ def load_user_access_keys(neo4j_session: neo4j.Session, user_access_keys: Dict,
for key in access_keys["AccessKeyMetadata"]:
if key.get('AccessKeyId'):
neo4j_session.run(
ingest_account_key,
ingest_user_key,
UserARN=arn,
AccessKeyId=key['AccessKeyId'],
CreateDate=str(key['CreateDate']),
CreateDate=key['CreateDate'],
Status=key['Status'],
LastUsedDate=key['LastUsedDate'],
LastUsedService=key['LastUsedService'],
LastUsedRegion=key['LastUsedRegion'],
aws_update_tag=aws_update_tag,
)

Expand Down
13 changes: 7 additions & 6 deletions docs/root/modules/aws/schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,10 @@ Representation of an [AWSPrincipal](https://docs.aws.amazon.com/IAM/latest/APIRe
(AWSPrincipal)-[MEMBER_AWS_GROUP]->(AWSGroup)
```

- This AccountAccessKey is used to authenticate to this AWSPrincipal.
- This UserAccessKey is used to authenticate to this AWSPrincipal.

```
(AWSPrincipal)-[AWS_ACCESS_KEY]->(AccountAccessKey)
(AWSPrincipal)-[AWS_ACCESS_KEY]->(UserAccessKey)
```

- AWS Roles can trust AWS Principals.
Expand Down Expand Up @@ -508,10 +508,10 @@ Representation of an [AWSUser](https://docs.aws.amazon.com/IAM/latest/APIReferen
(AWSUser)-[STS_ASSUMEROLE_ALLOW]->(AWSRole)
```

- This AccountAccessKey is used to authenticate to this AWSUser
- This UserAccessKey is used to authenticate to this AWSUser

```
(AWSUser)-[AWS_ACCESS_KEY]->(AccountAccessKey)
(AWSUser)-[AWS_ACCESS_KEY]->(UserAccessKey)
```

- AWS Accounts contain AWS Users.
Expand Down Expand Up @@ -681,7 +681,7 @@ Representation of an AWS [Tag](https://docs.aws.amazon.com/resourcegroupstagging
(AWSVpc, DBSubnetGroup, EC2Instance, EC2SecurityGroup, EC2Subnet, NetworkInterface, RDSInstance, S3Bucket)-[TAGGED]->(AWSTag)
```

### AccountAccessKey
### UserAccessKey

Representation of an AWS [Access Key](https://docs.aws.amazon.com/IAM/latest/APIReference/API_AccessKey.html).

Expand All @@ -691,13 +691,14 @@ Representation of an AWS [Access Key](https://docs.aws.amazon.com/IAM/latest/API
| lastupdated | Timestamp of the last time the node was updated
| createdate | Date when access key was created |
| status | Active: valid for API calls. Inactive: not valid for API calls|
| lastused | Date when the access key was last used |
| **accesskeyid** | The ID for this access key|

#### Relationships
- Account Access Keys may authenticate AWS Users and AWS Principal objects.

```
(AWSUser, AWSPrincipal)-[AWS_ACCESS_KEY]->(AccountAccessKey)
(AWSUser, AWSPrincipal)-[AWS_ACCESS_KEY]->(UserAccessKey)
```


Expand Down