Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

Log session-name and external-id when logging credentials #447

Merged
merged 2 commits into from
Dec 8, 2020

Conversation

stefansedich
Copy link
Contributor

@stefansedich stefansedich commented Dec 7, 2020

Adds the session-name and external-id to the log fields when logging credentials.

Session name is safe and easy but logging external-id worries me a little, at the moment I mask the entire string which really adds no value apart from showing me there is an external-id used and how long it might be.

We could attempt some partial masking but that again makes me nervous, thoughts @pingles ?

"credentials.access.key": creds.AccessKeyId,
"credentials.expiration": creds.Expiration,
"credentials.role": identity.Role,
"credentials.role": identity.Role.ARN,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were logging the entire Role object which isn't nescasary, this will log only the ARN string and not the nested object.

@pingles
Copy link
Contributor

pingles commented Dec 8, 2020

I think logging the masked version is ok.

As you say, it confirms a value is set. As per the AWS docs (https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html), it's not considered a secret as the org is expected to issue them. But, for it not to be a problem I'd expect logs to need to be secured properly and make sure people couldn't see all IDs together.

For now, I'd say let's keep the masked version and then if it's problematic we can address later.

@pingles pingles merged commit 7f7fd09 into uswitch:master Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants