googleapis / google-auth-library-python

Google Auth Python Library
https://googleapis.dev/python/google-auth/latest/
Apache License 2.0
773 stars 305 forks source link

Wrong timezones in compute_engine.IDTokenCredentials expiry #1323

Closed juzna closed 1 year ago

juzna commented 1 year ago

The expiry of compute_engine.IDTokenCredentials is in the local timezone, but it's then compared to utc. This means that an expired token may be used. Expiry of all other credential types are correctly in UTC.

Environment details

Steps to reproduce

Run on a GCE VM (or a GKE pod).

Configure Python to use some timezone far from UTC, eg export TZ=America/New_York.

import google.auth.compute_engine.credentials
import google.auth.transport.requests

r = google.auth.transport.requests.Request()
creds = google.auth.compute_engine.credentials.IDTokenCredentials(r, target_audience="foo", use_metadata_identity_endpoint=True)
creds.refresh(r)

print(f"expiry: {creds.expiry}")
print(f"expired: {creds.expired}")

Here, expired incorrectly reports false, because it compares the local expiry with utcnow.

Another failure mode is in timezones with a positive offset (eg Europe/Prague), where the token will be treated as not-expired even after it actually expired.

All other credential types use utc for everything, so they don't have the problem. Even the compute engine OAuth2 credentials in the same file (ie just Credentials, not IDTokenCredentials).

Should be a very simple fix, to use UTC datetime everywhere.

clundin25 commented 1 year ago

Quick scan: Issue seems similar to https://github.com/googleapis/google-auth-library-python/issues/1264. Will take a deeper look later today.

juzna commented 1 year ago

Btw the fix is really simple, just changing fromtimestamp to utcfromtimestamp in https://github.com/googleapis/google-auth-library-python/blob/bcb85bf1f5b98d98c1b1af47487acb52053c4b17/google/auth/compute_engine/credentials.py#L392. We have deployed this as a hotfix to our app and works fine.

clundin25 commented 1 year ago

Looks like impersonated_credentials.py has the same bug. I will cut a new issue to track it.

➜ rg "\.fromtimestamp" -g '!*test*'
google/auth/compute_engine/credentials.py
392:        return id_token, datetime.datetime.fromtimestamp(payload["exp"])

google/auth/impersonated_credentials.py
457:        self.expiry = datetime.fromtimestamp(jwt.decode(id_token, verify=False)["exp"])
juzna commented 1 year ago

Hey, thx for the quick feedback. And sorry, I was too busy to properly finish the PR including tests. I can double-check the impesonated credentials too. I was about to use those in our project too :)

clundin25 commented 1 year ago

@juzna, No worries! Let's leave the impersonated credentials for a separate PR. We can own that update