-
Notifications
You must be signed in to change notification settings - Fork 149
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
Possible fix for the time parsing related issue (#1969) #1970
Conversation
Allow for GeneralizedTime (> year 2049) in certificates. Fix also a bug in renormalizing the Year from UTCTime parsing
src/XrdCrypto/XrdCryptosslAux.cc
Outdated
ltm.tm_year += 100; | ||
// In case GeneralizedTime is used | ||
if (ltm.tm_year > 2000) | ||
ltm.tm_year -= 1900; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need ltm.tm_year > 1900
here instead of ltm.tm_year > 2000
, otherwise I see wrong dates (i.e. with year >3600) being generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last thing, maybe we actually need tm.tm_year >= 1900
such that 19000321150000Z
belongs to year 1900 and not 3800, but that's probably mostly irrelevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end, I think this is what would give the most reasonable answers:
if (ltm.tm_year < 50)
ltm.tm_year += 2000;
else if (ltm.tm_year < 100)
ltm.tm_year += 1900;
ltm.tm_year -= 1900;
This works for years 00(2000) 01(2001) 49(2049) 51(1951) 1800 1900 1949 1951 2000, and 2100 without wrapping into 3000+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed and pushed. Thanks!
Thank you very much, @gganis! |
Allow for GeneralizedTime (> year 2049) in certificates.
Fix also a bug in renormalizing the Year from UTCTime parsing.