expiration check is using different timezone other than UTC

Bug #1195924 reported by yong sheng gong
28
This bug affects 4 people
Affects Status Importance Assigned to Milestone
python-keystoneclient
Fix Released
Medium
Bryan Davidson

Bug Description

It seems we are using local time to compare the UTC token expiration time for expiration check.
I am trying using the patch following:
diff --git a/keystoneclient/middleware/auth_token.py b/keystoneclient/middleware/auth_token.py
index 1b82cfa..a9fa54c 100644
--- a/keystoneclient/middleware/auth_token.py
+++ b/keystoneclient/middleware/auth_token.py
@@ -937,7 +937,8 @@
         else:
             raise InvalidUserToken('Token authorization failed')
         expires = timeutils.parse_isotime(timestamp).strftime('%s')
- if time.time() >= float(expires):
+ utcnow = timeutils.utcnow().strftime('%s')
+ if float(utcnow) >= float(expires):
             self.LOG.debug('Token expired a %s', timestamp)
             raise InvalidUserToken('Token authorization failed')
         return expires

but I am not sure if it is a problem and cannot assess the scope of impact.

Revision history for this message
yong sheng gong (gongysh) wrote :

My time zone is GMT+8, I am running following in python console to verify it:
>>> from keystoneclient.openstack.common import timeutils
>>> import time
>>> print time.time()
1372461272.58
>>> print timeutils.utcnow().strftime('%s')
1372432495
>>> print time.time(), timeutils.utcnow().strftime('%s')
1372461312.73 1372432512
>>> print (1372461312 - 1372432512)/3600
8

Changed in keystone:
assignee: nobody → yong sheng gong (gongysh)
Revision history for this message
Dolph Mathews (dolph) wrote :

This raised a red flag for me recently as well. I think that's just comparing the cache expiration, specified in local time, against local time. As long as every node sharing the cache is in the same time zone, it shouldn't present an issue. That said, there's no reason why we shouldn't be using UTC here.

Changed in keystone:
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Dolph Mathews (dolph) wrote :

If the above is not true (i.e. we're comparing local time against UTC somewhere), this should be elevated to High.

Revision history for this message
Alex Meade (alex-meade) wrote :

Hi yong, I will have a patch soon that should address this bug. Mind if I assign myself or are you working on it?

Alex Meade (alex-meade)
Changed in keystone:
assignee: yong sheng gong (gongysh) → Alex Meade (alex-meade)
status: Confirmed → In Progress
Revision history for this message
yong sheng gong (gongysh) wrote :

go ahead.

Revision history for this message
Dolph Mathews (dolph) wrote :

Looking at keystoneclient's source code, my above statement doesn't apply to the source code cited in the bug description.

Revision history for this message
Dolph Mathews (dolph) wrote :

More detailed description for this issue is in bug 1197120.

affects: keystone → python-keystoneclient
Changed in python-keystoneclient:
importance: Low → Medium
Revision history for this message
Kieran Spear (kspear) wrote :

Current local time is used for the check when a token is first encountered and when it is pulled out of the cache. In both cases the token expiry value is in UTC.

The token expiry itself is always UTC. If I change the tz into the future on a devstack node, token expire time is cut short by auth_token middleware due to the tz offset. I found this when testing a short (1 hour) expiry time on tokens - tokens are considered expired by the auth_token middleware as soon as they're created in that case, because the local time is in the future.

Fix review is happening here:
https://review.openstack.org/#/c/35403/

Changed in python-keystoneclient:
assignee: Alex Meade (alex-meade) → nikhil komawar (nikhil-komawar)
Changed in python-keystoneclient:
assignee: nikhil komawar (nikhil-komawar) → Bryan Davidson (bryan-davidson)
Changed in python-keystoneclient:
assignee: Bryan Davidson (bryan-davidson) → nikhil komawar (nikhil-komawar)
Changed in python-keystoneclient:
assignee: nikhil komawar (nikhil-komawar) → Bryan Davidson (bryan-davidson)
Dolph Mathews (dolph)
Changed in python-keystoneclient:
milestone: none → 0.3.3
Changed in python-keystoneclient:
assignee: Bryan Davidson (bryan-davidson) → Alex Meade (alex-meade)
Changed in python-keystoneclient:
assignee: Alex Meade (alex-meade) → Bryan Davidson (bryan-davidson)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-keystoneclient (master)

Reviewed: https://review.openstack.org/35403
Committed: http://github.com/openstack/python-keystoneclient/commit/93793cb3963d5d001cb51e3452d2226230a72986
Submitter: Jenkins
Branch: master

commit 93793cb3963d5d001cb51e3452d2226230a72986
Author: Bryan Davidson <email address hidden>
Date: Fri Aug 30 13:38:37 2013 -0400

    Normalize datetimes to account for tz

    This patch makes sure that datetimes in the auth_token middleware
    are normalized to account for timezone offsets.

    Some of the old tests were changed to ensure that the expires string
    stored in the cache is in ISO 8601 format and not a random float.

    Fixes bug 1195924

    Change-Id: I5917ab728193cd2aa8784c4860a96cdc17f3d43f

Changed in python-keystoneclient:
status: In Progress → Fix Committed
Dolph Mathews (dolph)
Changed in python-keystoneclient:
milestone: 0.3.3 → 0.4.0
Dolph Mathews (dolph)
Changed in python-keystoneclient:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.