formats icon indicating copy to clipboard operation
formats copied to clipboard

der: `GeneralizedTime` can have fractional seconds

Open indygreg opened this issue 3 years ago • 6 comments
trafficstars

As part of investigating https://github.com/indygreg/PyOxidizer/issues/482 I was looking at what other ASN.1 implementations do for GeneralizedTime.

This project's implementation - like mine - assumes the no fractional seconds allowed definition of GeneralizedTime per RFCs like 5280. However, the ASN.1 definition of GeneralizedTime (I think X.690 is the current specification) allows multiple representations, including with fractional seconds and/or with a time zone differentiator. At least 1 crypto related RFC (3161 - Time-Stamp Protocol) allows use of the form with fractional seconds.

I know less about crypto than this project's maintainers. But I think there is a compelling case to allow GeneralizedTime to accept fractional seconds or to define a variant of this type that allows fractional seconds since there is at least 1 crypto RFC needing it - and one used as part of many PKI systems at that. I'm leaning towards making GeneralizedTime generic over an enum that controls which forms are allowed but I haven't decided yet. I'll be very curious to see what this project decides!

indygreg avatar Jan 30 '22 01:01 indygreg

This project's implementation - like mine - assumes the no fractional seconds allowed definition of GeneralizedTime per RFCs like 5280.

It’s a bit more than that… it’s a MUST NOT:

https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.5.2

GeneralizedTime values MUST NOT include fractional seconds.

tarcieri avatar Jan 30 '22 01:01 tarcieri

One thing that could work without compromising RFC5280 use cases is having a second type like GeneralizedTimeFrac which supports fractional seconds.

It could have GeneralizedTime and Option<u16> fields and reuse the parsing logic from the former.

tarcieri avatar Jan 30 '22 01:01 tarcieri

Correct, RFCs like 5280 and 5652 are explicit about banning fractional seconds and non-Z timezone form via MUST [NOT] clauses.

A potential drawback with a separate type like GeneralizedTimeFrac is the potential for type explosion. RFCs like 5280 define choice types like:

Time ::= CHOICE {
        utcTime        UTCTime,
        generalTime    GeneralizedTime }

If there are any RFCs allowing similar types on GeneralizedTime with fractional seconds or timezone offsets, you would need a new Rust enum type to represents that choice ASN.1 type. I have no clue if any such RFCs exist: RFC 3161 expressly requires GeneralizedTime directly.

indygreg avatar Jan 30 '22 01:01 indygreg

We already define Time here:

https://github.com/RustCrypto/formats/blob/master/x509/src/time.rs#L25

And again, that definition needs to reflect the MUST NOT from RFC5280.

I'm much more worried about trying to shoehorn fractional seconds into the current GeneralizedTime violating RFC5280 requirements (e.g. through bugs manifesting at runtime) than I am a hypothetical "type explosion", especially since you can only cite one potential use case for fractional seconds.

Instead, I'd consider that using the type system effectively to make undesired states unrepresentable.

tarcieri avatar Jan 30 '22 13:01 tarcieri

In the absence of evidence that the potential for type explosion exists (which anyone has yet to find), I agree that it is best to avoid complexity and footguns by only carving out a very limited exception for allowing fractional seconds [and timezone offsets, if even needed].

indygreg avatar Jan 30 '22 21:01 indygreg

I think GeneralizedTime shouldn't generally support fractional seconds. When I went and tried to verify if there are other RFCs that allow fractional seconds. In my non-exhaustive search I found exactly one, which indygreg noted above, RFC 3161: the Time Stamp Protocol.

seanturner avatar Mar 16 '22 04:03 seanturner

Closing this as not planned

tarcieri avatar Aug 18 '24 14:08 tarcieri