Skip to content
This repository was archived by the owner on May 18, 2021. It is now read-only.

Fix cred process expiration #303

Merged
merged 1 commit into from
Oct 22, 2020
Merged

Fix cred process expiration #303

merged 1 commit into from
Oct 22, 2020

Conversation

reegnz
Copy link
Contributor

@reegnz reegnz commented Oct 22, 2020

The Expiration field conveyed the wrong value. It's changed to be the
same value that's used in aws-okta exec.

The Expiration field conveyed the wrong value. It's changed to be the
same value that's used in aws-okta exec.
@nickatsegment
Copy link
Contributor

@reegnz what are the practical side effects you've been seeing?

@nickatsegment nickatsegment merged commit 7f92fc8 into segmentio:master Oct 22, 2020
@reegnz
Copy link
Contributor Author

reegnz commented Oct 26, 2020

What I've been noticing is that the timestamp returned by the cred process was not correctly represent the expiration of the credentials. I assume this broke the SDK credential refresh mechanism that actually runs cred-process before the expiration timestamp is reached.
For some reason this change is still not perfect (the timestamp still does not match the one in the token saved into the keychain (I'm on MacOS), so I think this fix is not all there is to it.

@reegnz
Copy link
Contributor Author

reegnz commented Oct 26, 2020

How I built around the faulty cred-process expiration up until now is to build my own cred-process script with bash utilizing aws-okta exec, and using the environment variables with jq to construct the correct json response.

@reegnz
Copy link
Contributor Author

reegnz commented Oct 26, 2020

One of the remaining issues that still needs to be fixed: When the SDK recognizes that the Expiration is approaching, it tries to refresh the token. aws-okta however sees that the token has not expired yet, returning the old token that's about to be expired.
I think the best fix for that would be to adjust the caching to be compatible with the refresh-mechanism in the AWS SDK.

@reegnz
Copy link
Contributor Author

reegnz commented Oct 26, 2020

Did dig a bit in aws-okta code further, just to make sure I was right with the SDK expiration.

Turns out this fix will be good enough. The Expiration comes from the AssumeRole that is performed when there's a source_profile in the config, so the expiry is now handled by the SDK correctly.
The AssumeRole is performed on each new exec or cred-process call, so those tokens are not cached. This allows the SDK to properly handle the rotation.

My initial assumption for looking into this came from how aws-vault does it: there it caches the AssumeRole credentials as well, so it makes the SDK credential refresh more difficult.

Here we don't have that issue present. :)

@reegnz reegnz deleted the fix_cred_process_expiry branch October 26, 2020 13:01
@nickatsegment
Copy link
Contributor

Super, thanks for the digging. FWIW, given #278, there's a certain point where a fix becomes complex enough that I couldn't justify accepting it. It might be worth it for you to invest your time in the fork at https://github.com/aws-okta/aws-okta, though I'm not seeing a lot of movement there.

arohter pushed a commit to TiVo/aws-okta that referenced this pull request Nov 21, 2020
arohter added a commit to TiVo/aws-okta that referenced this pull request Nov 21, 2020
arohter added a commit to TiVo/aws-okta that referenced this pull request Nov 21, 2020
* Revert "disable github releases (currently broken) (segmentio#305)"

This reverts commit b5cad3b.

* Revert "Added Ubuntu 2020 (Focal) to Makefile.release (segmentio#304)"

This reverts commit ac21803.

* Revert "Fix cred process expiration (segmentio#303)"

This reverts commit 90c0192.

* Revert "Update issue templates"

This reverts commit 9e17974.

* Revert "Calculate OktaClient Content-Length correctly (segmentio#300)"

This reverts commit e93f247.
arohter added a commit to TiVo/aws-okta that referenced this pull request Nov 21, 2020
* Calculate OktaClient Content-Length correctly (segmentio#300)

Fixes: segmentio#298

* Update issue templates

* Fix cred process expiration (segmentio#303)

* Added Ubuntu 2020 (Focal) to Makefile.release (segmentio#304)

* disable github releases (currently broken) (segmentio#305)

Co-authored-by: Will Gardner <[email protected]>
Co-authored-by: Nick Irvine <[email protected]>
Co-authored-by: Zoltán Reegn <[email protected]>
Co-authored-by: Yossi Eliaz <[email protected]>
arohter added a commit to TiVo/aws-okta that referenced this pull request Feb 19, 2021
* Calculate OktaClient Content-Length correctly (segmentio#300)

Fixes: segmentio#298

* Update issue templates

* Fix cred process expiration (segmentio#303)

* Added Ubuntu 2020 (Focal) to Makefile.release (segmentio#304)

* disable github releases (currently broken) (segmentio#305)

* Update AWS Go SDK To v1.25.35 (segmentio#307)

Fixes STS regional endpoint support.

* Add STS Regional Endpoint Support To Other STS Clients (segmentio#308)

* Update keyring to v1.1.6 (segmentio#309)

Recent versions of kwallet have removed the old support for the kde4
compatible kwallet dbus interface. This means newer kde5 based
OS installs (e.g. kubuntu 20.04) can no longer use the kwallet backend
with aws-okta.

This was fixed upstream in the keyring lib back in 2019 but the
dependency hasn't been bumped since then.

Co-authored-by: Will Gardner <[email protected]>
Co-authored-by: Nick Irvine <[email protected]>
Co-authored-by: Zoltán Reegn <[email protected]>
Co-authored-by: Yossi Eliaz <[email protected]>
Co-authored-by: Andrew Babichev <[email protected]>
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