Metadata Attribute research: expires

[MVrachev]

This issue aims to document thoughts about the expires attribute from the Signed class in order
to understand how we use that attribute, what might go wrong with it and how we can validate it.

We want to answer/address the following 6 questions/points based on my comment here:

1. How do we use it?
2. What might go wrong?
3. Rethink how we store it?
4. If, after the changes in step 3 there are concerns, then add a validation function
5. Make the necessary changes for points 1 - 4, think and propose a way to integrate it into the class/classes (decorators, descriptors, etc.)
6. create a pr for all additional changes on top of the validation functions

PS: The 7-th point is covered by documenting this issue.

[MVrachev]

My initial comments on those 6 steps:

1. How do we use it?

• Initialize it through init.
• Bump it through bump_expiration function.

2. What might go wrong?

• Wrong or outdated expires passed during initialization.
• Modification or deletion of the expires attribute directly from the metadata API, like this:
  metadata.signed.expires = “BAD

3. Rethink how we store it?

• expires is mutable, but at the same time we want to help the user to not change expires directly like metadata.signed.expires = -10.
  I didn’t found a way to forbid access to expires outside the class, but I found a way to enforce validation.
  We can achieve this by making expires a property that sets a private _expires and makes sure it validates it before setting it. Sadly, I didn’t find a way to block the user from changing _expires directly like: “metadata.signed._expires = “2.10.10”.
  That way if the user is accessing the self._expires he would know he is accessing a private field and it’s his responsibility.

4. If, after the changes in step 3 there are concerns, then add a validation function

• We need a validation function to check that expires is actually a DateTime object.

5. Make the necessary changes for points 1 - 4, think and propose a way to integrate it into the class/classes (decorators, descriptors, etc.)

• The validation function could be called into the expires.setter (expires is defined as property)
  just before assigning the value to _expires.
  That way we make sure we won’t forget to validate the value when setting _expires.

6. create a pr for all additional changes on top of the validation functions

• I made a simple commit where small prototypes for the different attributes can be seen https://github.com/MVrachev/tuf/commits/checks

[MVrachev]

Wrong or outdated expires passed during initialization.

@jku commented the above:

it's not so much generic init we're interested in but deserialization: we're getting something from remote,
how do we ensure we either raise or end up with a datetime?

Currently we call
expires = formats.expiry_string_to_datetime(expires_str)

we should not be calling formats.py in metadata.py (but using the same implementation is probably fine
That's issue #1384.

Modification or deletion of the expires attribute directly from the metadata API, like this “metadata.signed.expires = “BAD”

@jku commented on the above:

so we've done quite a bit to make it easy to use (docs, type hints, provide a function to safely bump it (it's probably not a function they'd use but still...)

The worst that can happen if the user still uses something else is that serialization then fails --
so the faulty metadata does not get written to storage
so I'm not sure if we should be adding a property, a setter, and a validation function just so we can call isinstance()...

[MVrachev]

All of the participants in this discussion discussed this again and decided we won't make additional changes for expires considering that there is a public API function to change it - bump_expiration.
Also, if a value that can't be converted to a datetime object is passed, then in _common_fields_from_dict we would fail when we call formats.expiry_string_to_datetime() (or a substitute when we fix #1384).