NFD » ndn-cxx

I did some testing on this.
I found the root cause is: ValidityPeriod type stores timestamps as boost::chrono::time_point<system_clock> type, which represents the timestamp as int64_t nanoseconds since Unix epoch.
For any date after 22620411T234716, the int64 overflows.

I don't think so. IIRC ValidityPeriod uses a time point with seconds granularity, instead of nanoseconds, precisely to avoid this issue. My guess (I haven't looked at the code) is that the problem lies in the conversion to/from this internal format to whatever chrono format is used externally, which does suffer from the limitation you described.

Davide Pesavento wrote in #note-2:

ValidityPeriod uses a time point with seconds granularity, instead of nanoseconds, precisely to avoid this issue. My guess is that the problem lies in the conversion to/from this internal format to whatever chrono format is used externally

I inserted some printing in ValidityPeriod and Certificate types, to observe the values during the cert-dump command.

This suggested that the overflow occurred during decoding.
This time::fromIsoString looks suspicious: it returns system_clock::time_point that cannot represent large dates.

    m_notAfter = time_point_cast<TimePoint::duration>(
                   time::fromIsoString(readString(m_wire.elements()[NOT_AFTER_OFFSET])));

Going further into time::fromIsoString and convertToTimePoint:

The point variable has type system_clock::time_point where the large date overflows.

I can propose two potential solutions to this bug.
Both solution would allow ValidityPeriod::isValid to work correctly, at least within the next few decades.
They may not allow ndn-cxx to generate certificates with large dates, but they would ensure ndn-cxx is interoperable with other software that publishes such certificates.

Simpler solution is interpreting NotBefore and NotAfter dates that would under/overflow system_clock::time_point type to the limit values.
This solution can be implemented by performing a string comparison in the decoding function.
Under this solution, the sample certificate would be decoded as having a NotAfter date of 22620411T234716.

More complex solution is introducing templated date conversion functions, such as time::pointFromIsoString<ValidityPeriod::TimePoint>(str).
Additionally, ValidityPeriod::getPeriod return type must be changed from system_clock::time_point to ValidityPeriod::TimePoint, which would be a breaking change.

Davide Pesavento wrote in #note-7:

The proper solution would be to use boost::posix_time::ptime directly and avoid roundtrips to/from system_clock::time_point, right?

Yes, but it requires a breaking change in getPeriod return type.
This could be considered in a later commit.

Junxiao Shi wrote in #note-8:

Yes, but it requires a breaking change in getPeriod return type.

Not necessarily. We could deprecate getPeriod (keeping its current broken behavior) and add a pair of getters for NotBefore and NotAfter with the proper behavior and return type.

This could be considered in a later commit.

Agreed.