Tuf: Expiration check does not follow the spec

[trishankatdatadog]

Description of issue or feature request:

The metadata expiry check code does not follow the specification:

Check for a freeze attack. The expiration timestamp in the trusted $ROLE metadata file MUST be higher than the fixed update expiration time.

Current behavior:

expires < now

Expected behavior:

expires <= now

[trishankatdatadog]

Cc the in-toto team @lukpueh @adityasaky @SantiagoTorres, where the code is expires < now

The spec seems underspecified:

EXPIRES determines when layout metadata should be considered expired and no longer trusted by clients. Clients MUST NOT trust an expired file.

We should discuss whether both projects should consider whether a metadata file has expired in the same manner.

[mnm678]

I think this is actually a problem with the specification. I understand an expiration time as metadata that expires at a certain time, rather than after a certain time.

[SantiagoTorres]

I think this is actually a problem with the specification. I understand an expiration time as metadata that expires at a certain time, rather than after a certain time.

+1. Although I think just having agreement is more important than either option winning imvho

[trishankatdatadog]

Yes, this was a recent change to the spec, and a second of difference is not going to break any significant security guarantee.

Personally, I think <= makes (slightly) more sense than <.

[trishankatdatadog]

Now that I look at other implementations, such as ProtonMail/Crypto, they use <. Can someone double-check me here? It's getting late (4am) and I'm tired.

Also, does anyone have other implementations/interpretations elsewhere?

[joshuagl]

I think this is actually a problem with the specification. I understand an expiration time as metadata that expires at a certain time, rather than after a certain time.

I think this sounds sensible. However,

does anyone have other implementations/interpretations elsewhere?

x509 certificates are not a perfect comparison, but they are valid for a date range (notBefore through notAfter), with the certificate only classing as expired, with:

validity period for a certificate is the period of time from notBefore through notAfter, inclusive.

Which results in comparisons as <= such as in openssl.

GnuPG has expiration checks that look like sig->expiredate <= make_timestamp () i.e. in parse_signature and build_sig_subpkt.

I agree with Santiago. The important thing is that we agree on an interpretation, and are consistent.

[trishankatdatadog]

Thanks for checking against independent implementations, Joshua, great work!

Agreed with consensus (luckily that doesn't need meta-voting). By sheer precedent, as in legal cases, I'm personally voting for <=. Are we using threshold signatures for voting here; how does this work?

[trishankatdatadog]

OTOH, to play devil's advocate, reference and independent implementations of TUF and in-toto are probably all using < for some time already, and using <= requires all of them to update. Probably something that requires a community meeting in (of?) both.

[joshuagl]

Looking at known open source tuf implementations:

• rust-tuf: <= i.e. https://github.com/heartsucker/rust-tuf/blob/373cf2216c3ec38b64629675ccbd96979e4bede8/src/client.rs#L543
• go-tuf: <= i.e. https://github.com/theupdateframework/go-tuf/blob/1353a38b9741511d20a627df516f0737f657007a/verify/verify.go#L40
• tough: < i.e. https://github.com/awslabs/tough/blob/e63ee71f4fdb08c3c877686061e16ae9dd4a555a/tough/src/lib.rs#L395

(I would appreciate it if people who are fluent in Go and Rust would double-check my interpretations)

[trishankatdatadog]

(I would appreciate it if people who are fluent in Go and Rust would double-check my interpretations)

Cc @titanous @erickt @iliana

[lukpueh]

The spec seems underspecified:
...

I beg to differ, the final product verification workflow section clearly mandates expires < now (as is implemented in python-in-toto):

If the system's date is newer than the expiration date on the layout's expiration field, verification should fail.

Although, we might want to s/newer/higher and s/date/datetime.

[erickt]

@joshuagl - you are correct, we are using <= in rust-tuf and go-tuf.

[joshuagl]

Thank you for confirming @erickt

[mnm678]

Thanks for doing some research @trishankatdatadog and @joshuagl. It looks like there is a general consensus for <=, so I agree that it makes sense to change our implementation.

[lukpueh]

It looks like there is a general consensus for <=

Is there though? Here are some more data points:

python-tuf: <
tough: <
in-toto spec: <
python-in-toto: <
in-toto-golang: <=
tuf spec: <=
rust-tuf: <=
go-tuf: <=

[trishankatdatadog]

Is there though? Here are some more data points:

Don't forget GPG and OpenSSL...

[lukpueh]

Maybe not a consensus but at least a majority for <=?

I'm happy to review PRs that update python-tuf, in-toto spec and python-in-toto. :)

[iliana]

tough: <

Hard to remember but I imagine this was a literal reading of the detailed workflows, which were followed very literally to develop tough initially.

We haven't gone through spec changes in a while. Is the current revision internally inconsistent on this right now, and does it need clarified?

[lukpueh]

Thanks for chiming in, @iliana. The spec is not inconsistent in itself. There are only inconsistencies between tuf and in-toto (i.e. tuf's sister project) specs and the corresponding implementations. And there seems an agreement to consolidate all of these, which will require either some implementations and/or specifications to change.

[joshuagl]

I've created a PR against python-tuf (#1235) and filed issues against the in-toto specification (in-toto/docs#42) and reference implementation (in-toto/in-toto#417). That just leaves Tough that doesn't follow the rough consensus here, @iliana would you like me to file an issue there?

[iliana]

That just leaves Tough that doesn't follow the rough consensus here, @iliana would you like me to file an issue there?

Sure, thanks!

[joshuagl]

That just leaves Tough that doesn't follow the rough consensus here, @iliana would you like me to file an issue there?

Sure, thanks!

Done