Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client accepts expires values in non-UTC timezones #136

Open
joshuagl opened this issue Jun 28, 2021 · 3 comments
Open

Client accepts expires values in non-UTC timezones #136

joshuagl opened this issue Jun 28, 2021 · 3 comments

Comments

@joshuagl
Copy link
Member

joshuagl commented Jun 28, 2021

Note: description edited to clarify that this is not a go-tuf metadata generation issue.

While looking at the metadata generated for the sigstore root of trust I noticed that the expires entries in the metadata encodes a non-UTC timezone:

"expires": "2021-12-18T13:28:12.99008-06:00"

(from 1.root.json)

Whereas the the specification suggests time should always be in UTC:

Metadata date-time 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".

@asraa pointed out below that the expires field is not set by go-tuf, so the issue here (if any) is that the client does not verify that the date-time in the expires field is in UTC.

@asraa
Copy link
Contributor

asraa commented Jun 28, 2021

First thanks so much for taking a deeper look! Two comments
(1) The generation of UTC if my mistake, since I manually set the Expiration and didn't convert to UTC as the go-tuf impl does. (go-tuf supports changing root expiration by genkey or adding private key, neither of which we could use since we did not want to genkey or add encrypted private keys). If you could file an issue in root-signing, I'll make sure that closes before the next root signing. My plan was to expose a method "AddVerificationKey" like the python TUF implementation does so I don't have to manually change the JSON in our root-signing repository (To that end, how much of an issue is this for now?)

(2) The go client does not verify this, if this is a specification that requires this, I think it should be enforced in the client validating the metadata (validating the metadata and retrieving targets via the client never caught this!)

@joshuagl joshuagl changed the title Expiration date-times are not normalised to UTC Client accepts expires in non-UTC timezones Jun 28, 2021
@joshuagl
Copy link
Member Author

First thanks so much for taking a deeper look! Two comments
(1) The generation of UTC if my mistake, since I manually set the Expiration and didn't convert to UTC as the go-tuf impl does. (go-tuf supports changing root expiration by genkey or adding private key, neither of which we could use since we did not want to genkey or add encrypted private keys). If you could file an issue in root-signing, I'll make sure that closes before the next root signing. My plan was to expose a method "AddVerificationKey" like the python TUF implementation does so I don't have to manually change the JSON in our root-signing repository (To that end, how much of an issue is this for now?)

Ah, my mistake – I didn't dig deep enough and assumed it was a go-tuf issue. Thanks for the swift comment. I've filed an issue against sigstore/root-signing here sigstore/root-signing#27 and updated the description here to highlight that this is not a bug in go-tuf metadata creation.

(2) The go client does not verify this, if this is a specification that requires this, I think it should be enforced in the client validating the metadata (validating the metadata and retrieving targets via the client never caught this!)

I've updated description above to suggest this too.

go-tuf maintainers, feel free to close this WONTFIX if you are not interested in validating the date-time format in the client.

@joshuagl joshuagl changed the title Client accepts expires in non-UTC timezones Client accepts expires values in non-UTC timezones Jun 28, 2021
@jku
Copy link
Member

jku commented Feb 14, 2022

another related issue is including microseconds -- my reading is that they are not allowed as the format string is specified:

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 don't think there is a security issue in supporting/accepting msecs in a client, but it should be difficult to create metadata with msecs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants