Incorrect comparison of metadata expiry time? about python-tuf HOT 9 CLOSED

This is a good one for Kon if he's up to it. We should create unit tests
for expiration / time regardless.

Justin

On Fri, Mar 15, 2013 at 1:02 AM, TKK [email protected] wrote:

I fast-forwarded my system clock beyond the root metadata expiry time, and
found that the current implementation did not considered it expired.

I think that, presently, the time comparison is wronghttps://github.com/akonst/tuf/blob/112889787a662d4b3982825800440b1edbfba0bf/tuf/client/updater.py#L1138.
The comparison is happening between two time values of different units.

Should it not be like this instead?

• if expires < time.time():- expires_formatted = tuf.formats.format_time(expires)- message = 'Metadata '+repr(rolepath)+' expired on '+expires_formatted+'.'+ if tuf.formats.parse_time( expires ) < time.time():+ message = 'Metadata '+repr(rolepath)+' expired on '+ expires +'.'

—
Reply to this email directly or view it on GitHubhttps://github.com/akonst/tuf/issues/37
.

from python-tuf.

Expired metadata should now be properly detected.

$ python -B ../../examples/example_client.py
No handlers could be found for logger "tuf.hash"
[2015-01-23 22:31:01,945] [tuf.download] [INFO] Downloading: http://localhost:8001/metadata/timestamp.txt
Traceback (most recent call last):
File "../../examples/example_client.py", line 56, in
updater.refresh()
File "/home/vlad/test/virtualenv-1.9/test/local/lib/python2.7/site-packages/tuf/client/updater.py", line 578, in refresh
self._ensure_not_expired(metadata_role)
File "/home/vlad/test/virtualenv-1.9/test/local/lib/python2.7/site-packages/tuf/client/updater.py", line 1142, in _ensure_not_expired
raise tuf.ExpiredMetadataError(message)
tuf.ExpiredMetadataError: Metadata './metadata/current/timestamp.txt' expired on 2014-03-16 03:28:39.

TKK: Confirm, then close.

from python-tuf.

Why did a unit test not catch this before?

from python-tuf.

Carefully examine the two commits referenced above. The old test condition set the metadata 'expires' time in an invalid format and _ensure_not_expired() incorrectly assumed the expires time was also in this invalid format. _ensure_not_expired() has been comparing a string and floating point number, resulting in a False result (an exception was never raised). We are careful to always validate arguments ('expires' is validated prior to calling _ensure_not_expired()), however, this will not protect against code using them in unintended ways. "1234" < 1234.2 is okay to compare, just not what we intended.

from python-tuf.

Also, the old code base did not provide a way to set custom expiration dates, so the opportunity was not there to thoroughly test them -- we were still bogged down with other issues and unable to implement remaining TODOs.

from python-tuf.

Got it. I understand that we have time constraints, and I am certainly not a perfect programmer myself! :)

from python-tuf.

In fact, let me help you improve the unit test for this. I will close this issue once I complete that.

from python-tuf.

The metadata branch (https://github.com/akonst/tuf/tree/metadata) will include changes that will affect this unit test. The format of the expires field may be changing. Hold off on adding major changes; they might not apply in the future.

I made this small fix to be ready in time for the pycon demo, which I mentioned it in a previous email. The other changes are coming soon.

from python-tuf.

Works for me too:

ExpiredMetadataError: Metadata u'/tmp/virtualenv/virtualtuf/local/lib/python2.7/site-packages/pip-1.4.dev1-py2.7.egg/pip/tuf-metadata/metadata/current/root.txt' expired on 2013-03-31 06:33:56.

from python-tuf.