Project

General

Profile

Bug #3915

time::toIsoString and time::toString overflow with large dates

Added by Alex Afanasyev over 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Utils
Target version:
Start date:
01/11/2017
Due date:
% Done:

100%

Estimated time:

Description

During conversion system_clock::TimePoint to boost::posix_time (for formatting purposes), there is a silent variable overflow when TimePoint is large. It doesn't even work 100 years from now:

Here is the test snippet

  system_clock::TimePoint value = fromUnixTimestamp(milliseconds(1390966967032LL));
  BOOST_CHECK_EQUAL(toIsoString(value), "20140129T034247.032000");

  value += days(365 * 100 + 25 - 1); // 36524 days
  BOOST_CHECK_EQUAL(toIsoString(value), "21140129T034247.032000");

Expected:

../tests/unit-tests/util/time.t.cpp:88: info: check toIsoString(value) == "20140129T034247.032000" has passed
../tests/unit-tests/util/time.t.cpp:91: info: check toIsoString(value) == "21140129T034247.032000" has passed

Actual:

../tests/unit-tests/util/time.t.cpp:88: info: check toIsoString(value) == "20140129T034247.032000" has passed
../tests/unit-tests/util/time.t.cpp:91: error: in "Util/TestTime/LargeDates": check toIsoString(value) == "21140129T034247.032000" has failed [19771222T211431.032000 != 21140129T034247.032000]

As a result of this, there is an issue with v2::KeyChain self-signing certificate generation: it's validity period was expected to be set to 1000 years from now(), but was set to a time point in the past.


Related issues

Related to ndn-cxx - Bug #4315: ndnsec cert-gen: not-before cannot be later than not-afterClosed2017-09-30

Related to ndn-cxx - Bug #4478: time::fromIsoString doesn't decode large datesClosed2018-01-25

History

#1 Updated by Alex Afanasyev over 2 years ago

  • Status changed from New to Code review
  • Assignee set to Alex Afanasyev
  • Start date set to 01/11/2017
  • % Done changed from 0 to 50

I plan to fix only issue in v2::KeyChain, keeping this issue as a reminder of the problem. We probably should fix implementation of toIsoString by avoiding conversion to posix_time, but it is not critical.

#2 Updated by Junxiao Shi over 2 years ago

  • Status changed from Code review to New
  • Assignee deleted (Alex Afanasyev)

https://gerrit.named-data.net/3570 is merely a workaround in v2::KeyChain, not a solution of this bug. Thus returning to New state.

#3 Updated by Junxiao Shi over 2 years ago

  • Status changed from New to Code review
  • Assignee set to Junxiao Shi

The overflow occurs when boost::posix_time::from_time_t constructs seconds.
https://gerrit.named-data.net/3820 emulates boost::posix_time::from_time_t but constructs boost::posix_time::time_duration with separate hour and microsecond/nanosecond arguments, and neither would overflow in 489967 years.

#4 Updated by Junxiao Shi over 2 years ago

  • Status changed from Code review to Closed

#5 Updated by Junxiao Shi over 2 years ago

  • Status changed from Closed to In Progress

The change breaks ndns ManagementTool/ListZone test case.
Expected: NotAfter: 20380119T031408.
Actual: NotAfter: 19011213T204552.

#6 Updated by Junxiao Shi over 2 years ago

  • Status changed from In Progress to Feedback

The ndns test failure is actually a ndn-cxx problem. ndnsec cert-dump -p is showing the same, and cert.getNotAfter() returns -2147483648000000000 nanoseconds since Jan 1, 1970.

vagrant@m0212:~/ndns$ HOME=tests/unit/mgmt ndnsec cert-dump -p /ndns-test/KEY/dsk-1416974006659/ID-CERT/%FD%00%00%01I%EA%3Bz%0E
Certificate name:
  /ndns-test/KEY/dsk-1416974006659/ID-CERT/%FD%00%00%01I%EA%3Bz%0E
Validity:
  NotBefore: 19700101T000000
  NotAfter: 19011213T204552
Subject Description:
  2.5.4.41: /ndns-test
Public key bits: (RSA)
  MIIBIDANBgkqhkiG9w0BAQEFAAOCAQ0AMIIBCAKCAQEAyBVC+xc/JpscSE/JdxbV
  pvgrh/fokNFI/2t9D5inuIFr7cc4W+LyJ4GG1xr9olMx7MHamJU1Xg3VunjhSjL8
  mOaeXlbS6gxWexBCtNK6U4euPB4wks/gMIKdp24mAAFb4T+mBfjcRgR+NdrjyO5C
  2OqM8qbDZmD/iuEmE6GPXnuMS0o6s13yzMj9YfDh3Df2jZnHESZcmG5Qpgg22T58
  7t7bRx8Ha2EC3hb29AeYKwgEKDx8JH8ZBJ80AQP321HbyjXWshJLomzy5SJZo9nA
  bZOYlZPCQkomz92Zc9+kpLNQwDvtRLwkZ46B+b2JpKTFARbnvugONCEBuG6zNgoi
  EQIB
Signature Information:
  Signature Type: SignatureSha256WithRsa
  Key Locator: (Name) /ndns-test/KEY/ksk-1416974006577/ID-CERT

Regenerating tests/unit/mgmt/.ndn does not help.
Given this problem is within ndn::security::v1::IdentityCertificate which is deprecated, I won't further investigate.

#7 Updated by Junxiao Shi almost 2 years ago

  • Status changed from Feedback to Closed
  • % Done changed from 50 to 100

ndn::security::v1::IdentityCertificate is gone.

#8 Updated by Alex Afanasyev almost 2 years ago

  • Status changed from Closed to Feedback

Even though v1 security is gone, the same issue applies to v2 as well. We use exactly the same mechanism in ValidityPeriod.

#9 Updated by Junxiao Shi almost 2 years ago

  • Related to Bug #4315: ndnsec cert-gen: not-before cannot be later than not-after added

#10 Updated by Junxiao Shi almost 2 years ago

  • Status changed from Feedback to Closed

Problem with ValidityPeriod is split to #4315. It is not caused by time::toIsoString that is already covered by test cases, so this issue is closed.

#11 Updated by Alex Afanasyev over 1 year ago

  • Status changed from Closed to Feedback
  • Target version changed from v0.6 to v0.7

Older version of boost still have this issue (at least combination of boost 1.58/Ubuntu 16.04).

#12 Updated by Alex Afanasyev over 1 year ago

  • Related to Bug #4478: time::fromIsoString doesn't decode large dates added

#13 Updated by Davide Pesavento over 1 year ago

  • Category changed from Base to Utils

#14 Updated by Davide Pesavento over 1 year ago

  • Status changed from Feedback to Code review

time::toString() has the same problem. Fixed in https://gerrit.named-data.net/4482

#15 Updated by Davide Pesavento over 1 year ago

  • Subject changed from Unexpected result of time::toIsoString for large dates to time::toIsoString and time::toString overflow with large dates

#16 Updated by Davide Pesavento over 1 year ago

  • Status changed from Code review to Closed

Also available in: Atom PDF