Tuf: Remove iso8601 dependency

[jku]

I've been going through the tuf dependency chain with an eye on integrating tuf with pip: The issue with pip is that it's a package manager so needs to vendor everything it needs -- so being conservative with dependencies is a good idea. The good news is that tuf does not have many direct or indirect dependencies that would be a problem (good work!).

The one that possibly sticks out is "iso8601". The module is currently used in two places:

• repository_tool: Metadata.expiration property getter uses it to return a datetime object
• updater.py: Updater::_ensure_not_expired()

I'm mostly interested in that last one. it's used to compare the expiration stamp to current time and to format the error message:

    expires_datetime = iso8601.parse_date(expires)
    expires_timestamp = tuf.formats.datetime_to_unix_timestamp(expires_datetime)

    if expires_timestamp < current_time:
      message = 'Metadata '+repr(metadata_rolename)+' expired on ' + \
        expires_datetime.ctime() + ' (UTC).'
      logger.error(message)

      raise tuf.exceptions.ExpiredMetadataError(message)

I'm not familiar with date handling in python so my question is: Is this dependency valid or could this code be replaced with something that did not depend on iso8601?

[jku]

The spec says

Metadata date-time data follows the ISO 8601 standard. The expected format of the combined date and time string is "YYYY-MM-DDTHH:MM:SSZ". Time is always in UTC, and the "Z" time zone designator is attached to indicate a zero UTC offset.

My naive understanding is that because of the zero offset requirement this might be easy to parse with standard library functions.

[jku]

That said, now that I've looked at iso8601 module itself... It's tiny, literally 215 lines of code. So vendoring it might not be a problem.

[joshuagl]

1060 makes use of dateutil. A quick browse of their docs made me think we could probably replace iso8601 with functionality in dateutil.

[joshuagl]

The spec says

Metadata date-time data follows the ISO 8601 standard. The expected format of the combined date and time string is "YYYY-MM-DDTHH:MM:SSZ". Time is always in UTC, and the "Z" time zone designator is attached to indicate a zero UTC offset.

My naive understanding is that because of the zero offset requirement this might be easy to parse with standard library functions.

oh, that would be even better!

[jku]

1060 makes use of dateutil. A quick browse of their docs made me think we could probably replace iso8601 with functionality in dateutil.

For the pip use case dateutil would be worse than iso8601 as it's much larger -- but I guess #1060 wouldn't necessarily mean updater changes?

Oh and for reference: securesystemslib does use dateutil but AFAICS only in one place in gpg code

[joshuagl]

1060 makes use of dateutil. A quick browse of their docs made me think we could probably replace iso8601 with functionality in dateutil.

For the pip use case dateutil would be worse than iso8601 as it's much larger -- but I guess #1060 wouldn't necessarily mean updater changes?

In the short term, no. However, my current thought process is that the work in #1060 could become a low-level abstraction around TUF metadata that we build client/updater and repository APIs on top of. Therefore I'd like to make/keep those APIs as minimal (with as minimal dependencies) as possible so that they can be comfortably vendored into pip.

Note to self (or whoever picks up this issue): look at the use of dateutil in tuf/api and figure out if we can achieve the same with the standard library. Specifically in tuf/api/metadata we're currently using dateutil.relativedelta to cleanly specify a number of days/months/years by which to bump metadata expiration.

Oh and for reference: securesystemslib does use dateutil but AFAICS only in one place in gpg code

💯 Thanks for looking into this.

[joshuagl]

The dateutil usage in securesystemslib does not appear to be necessary, so I created a PR to remove it secure-systems-lab/securesystemslib#268

[joshuagl]

Our new metadata model (#1112) avoids using iso8601, which we'll build upon for both the client and repository code during the course of our refactor

[lukpueh]

@joshuagl suggests to create a helper for the iso8601 replacing call

datetime.strptime(exires, "%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=None)

[jku]

I've just reviewed the iso8601 test suite and have come to the conclusion that using the iso8601 module is in fact not compliant with the (spirit of the) tuf spec: current implementation allows for timestamps that I would not expect to work after reading the spec, such as "2006-10-20T15:34:56.123+02:30".

The spec currently says:

Metadata date-time data follows the ISO 8601 standard. The expected format of the combined date and time string is "YYYY-MM-DDTHH:MM:SSZ". Time is always in UTC, and the "Z" time zone designator is attached to indicate a zero UTC offset. An example date-time string is "1985-10-21T01:21:00Z".

I think the paragraph is fairly clear but first sentence could be better: we want to support _only this very specific format_ of the ISO8601 not the whole iso8601 spec.

As for the implementation, I think the helper idea is good. I don't think the replace() call in the example is needed: the strptime format used does not allow for a timezone at all.

[jku]

Adding one more thing: the Securesystemslib schema for "iso8601 datetime string" is very strict: datetime can definitely parse that.

I think the review is over... but since there's good discussion here I'm not closing: I'll just rename this to "Remove iso8601 dependency"

[trishankatdatadog]

I think the paragraph is fairly clear but first sentence could be better: we want to support _only this very specific format_ of the ISO8601 not the whole iso8601 spec.

Great observation. Please send a PR to the spec 🙂