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

make MinTokenExpiration configurable by cli flag #156

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

everpeace
Copy link

Issue #, if available: #155

Description of changes:

  • Changed MinTokenExpiration from const to var
  • Introduced a cli flag --min-token-expiration to configure MinTokenExpiration with validation(it must be at least 10min).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@everpeace everpeace requested a review from a team as a code owner June 10, 2022 04:46
@jaypipes
Copy link
Contributor

@micahhausler any reason why MinimumTokenExpiration was set to a constant?

@everpeace can you elaborate on the use case that a configurable minimum token expiration value is providing?

@everpeace
Copy link
Author

everpeace commented Jun 10, 2022

I want to make k8s SA tokens refresh in roughly the same frequency as the default AWS session expiration.

When we assume some security breach scenario, a longer expiration k8s token allows attackers more chances to impersonate. AWS sessions can revoke timely on the AWS management console, but revoking k8s SA tokens timely is relatively difficult because the cluster operator must rotate the signing key of SA tokens, especially in the on-premise cluster cases.

Thus, I think making the value configurable allows users to be able to control their security risk by themselves.

What do you think?

@everpeace
Copy link
Author

@jaypipes @micahhausler could you check my PR again??

@micahhausler
Copy link
Member

Hey sorry, I have no objection to adding this flag. I'd rather see a value plumbed down than a global variable.

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.

None yet

3 participants