18

How can I convert ASN1_TIME to time_t format? I wanted to convert the return value of X509_get_notAfter() to seconds.

jww
  • 97,681
  • 90
  • 411
  • 885
user1345697
  • 405
  • 2
  • 5
  • 15

7 Answers7

12

From the openssl code, it seems to be a bad idea:

/*
 * FIXME: mktime assumes the current timezone
 * instead of UTC, and unless we rewrite OpenSSL
 * in Lisp we cannot locally change the timezone
 * without possibly interfering with other parts
 * of the program. timegm, which uses UTC, is
 * non-standard.
 * Also time_t is inappropriate for general
 * UTC times because it may a 32 bit type.
 */

Note that you can use ASN1_TIME_diff() to get the number of days / seconds between two ASN1_TIME*. If you pass NULL as ASN1_TIME *from, you can get the difference from current time.

r0ro
  • 196
  • 2
  • 8
11

Times are stored as a string internally, on the format YYmmddHHMMSS or YYYYmmddHHMMSS.

At the end of the string there is room for fractions of seconds and timezone, but let's ignore that for now, and have some (untested) code.

Note: also see Bryan Olson's answer below, which discusses the undefined behavior due to the i++'s. Also see Seak's answer which removes the undefined behavior.

static time_t ASN1_GetTimeT(ASN1_TIME* time)
{
    struct tm t;
    const char* str = (const char*) time->data;
    size_t i = 0;

    memset(&t, 0, sizeof(t));

    if (time->type == V_ASN1_UTCTIME) /* two digit year */
    {
        t.tm_year = (str[i++] - '0') * 10 + (str[++i] - '0');
        if (t.tm_year < 70)
        t.tm_year += 100;
    }
    else if (time->type == V_ASN1_GENERALIZEDTIME) /* four digit year */
    {
        t.tm_year = (str[i++] - '0') * 1000 + (str[++i] - '0') * 100 + (str[++i] - '0') * 10 + (str[++i] - '0');
        t.tm_year -= 1900;
    }
    t.tm_mon = ((str[i++] - '0') * 10 + (str[++i] - '0')) - 1; // -1 since January is 0 not 1.
    t.tm_mday = (str[i++] - '0') * 10 + (str[++i] - '0');
    t.tm_hour = (str[i++] - '0') * 10 + (str[++i] - '0');
    t.tm_min  = (str[i++] - '0') * 10 + (str[++i] - '0');
    t.tm_sec  = (str[i++] - '0') * 10 + (str[++i] - '0');

    /* Note: we did not adjust the time based on time zone information */
    return mktime(&t);
}
jww
  • 97,681
  • 90
  • 411
  • 885
Jan Vidar Krey
  • 158
  • 1
  • 4
  • 13
    This code is wrong since it relies on undefined behavior of having `i++` (or `++i`) in the same expression: order of evaluation is _not_ guaranteed. – Paul J. Lucas May 08 '15 at 16:35
9

Well, I don't know about the rest, but that code is just wrong for the cases the ASN1_TIME is in UTCTime format : YYMMDDHHMMSSZ.

I tried and returns the value wrong, even with the correction from ++i to i++, nevertheless ... the code is not an example of good coding.

I manage to fix it, it was the sums of the char types:

static time_t ASN1_GetTimeT(ASN1_TIME* time){
    struct tm t;
    const char* str = (const char*) time->data;
    size_t i = 0;

    memset(&t, 0, sizeof(t));

    if (time->type == V_ASN1_UTCTIME) {/* two digit year */
        t.tm_year = (str[i++] - '0') * 10;
        t.tm_year += (str[i++] - '0');
        if (t.tm_year < 70)
            t.tm_year += 100;
    } else if (time->type == V_ASN1_GENERALIZEDTIME) {/* four digit year */
        t.tm_year = (str[i++] - '0') * 1000;
        t.tm_year+= (str[i++] - '0') * 100;
        t.tm_year+= (str[i++] - '0') * 10;
        t.tm_year+= (str[i++] - '0');
        t.tm_year -= 1900;
    }
    t.tm_mon  = (str[i++] - '0') * 10;
    t.tm_mon += (str[i++] - '0') - 1; // -1 since January is 0 not 1.
    t.tm_mday = (str[i++] - '0') * 10;
    t.tm_mday+= (str[i++] - '0');
    t.tm_hour = (str[i++] - '0') * 10;
    t.tm_hour+= (str[i++] - '0');
    t.tm_min  = (str[i++] - '0') * 10;
    t.tm_min += (str[i++] - '0');
    t.tm_sec  = (str[i++] - '0') * 10;
    t.tm_sec += (str[i++] - '0');

    /* Note: we did not adjust the time based on time zone information */
    return mktime(&t);
}
Paul J. Lucas
  • 6,895
  • 6
  • 44
  • 88
seak
  • 125
  • 2
  • 6
  • rfc 5280 says that 1- the input time is in UTC and therefore `mktime()` can return a wrong result here (`mktime()` expects the input time in the local timezone). 2- `YY >= 50` shall be interpreted as `19YY` 3- `99991231235959Z` is a special value. Here's a [code example how these issues can be fixed](https://stackoverflow.com/a/47015958/4279). – jfs Oct 30 '17 at 13:11
6

I have to disagree with Jan and Jack. Someone actually copied and used the given code where I work, and it fails. Here's why, from the C99 standard:

Between the previous and next sequence point an object shall have its stored value modified at most once by the evaluation of an expression." -- ISO/IEC 9899:1999, "Programming Languages - C", Section 6.5, Clause 1.

When compiling the given code, gcc (version 4.1.2) says, nine times,

warning: operation on ‘i’ may be undefined.

The code has undefined behavior. The bug I actually saw was year "13" being read as 11. That's because:

The result of the postfix ++ operator is the value of the operand. After the result is obtained, the value of the operand is incremented. [...] The side effect of updating the stored value of the operand shall occur between the previous and the next sequence point. -- Ibid, Section 6.5.2.4, Clause 2.

Both instances of str[i++] in:

t.tm_year = (str[i++] - '0') * 10 + (str[i++] - '0');

read the '1' in "13", because they both happened before the update of i. All the lines that update i multiple times have the same problems.

The easy fix is to get rid of 'i' and replace all those lines with a single call to sscanf().

Even with that fix, I wouldn't like the code. In addition to ignoring a timezone suffix, it doesn't check for errors or unexpected values. Certificates are a security mechanism, and security code has strict requirements for robustness. The corner cases that your program does not handled correctly are the ones your attackers fill feed it.

Bryan Olson
  • 71
  • 1
  • 2
  • *"The easy fix is to get rid of 'i' and replace all those lines with a single call to sscanf()"* - you should probably provide an example since its easy to use `sscanf` incorrectly. There's no sense it trading one bug for another. – jww May 08 '15 at 19:23
2

Jan's answer mostly works in this situation, however, the accumulator i should consistently use i++:

static time_t ASN1_GetTimeT(ASN1_TIME* time)
{
    struct tm t;
    const char* str = (const char*) time->data;
    size_t i = 0;

    memset(&t, 0, sizeof(t));

    if (time->type == V_ASN1_UTCTIME) /* two digit year */
    {
        t.tm_year = (str[i++] - '0') * 10 + (str[i++] - '0');
        if (t.tm_year < 70)
        t.tm_year += 100;
    }
    else if (time->type == V_ASN1_GENERALIZEDTIME) /* four digit year */
    {
        t.tm_year = (str[i++] - '0') * 1000 + (str[i++] - '0') * 100 + (str[i++] - '0') * 10 + (str[i++] - '0');
        t.tm_year -= 1900;
    }
    t.tm_mon = ((str[i++] - '0') * 10 + (str[i++] - '0')) - 1; // -1 since January is 0 not 1.
    t.tm_mday = (str[i++] - '0') * 10 + (str[i++] - '0');
    t.tm_hour = (str[i++] - '0') * 10 + (str[i++] - '0');
    t.tm_min  = (str[i++] - '0') * 10 + (str[i++] - '0');
    t.tm_sec  = (str[i++] - '0') * 10 + (str[i++] - '0');

    /* Note: we did not adjust the time based on time zone information */
    return mktime(&t);
}
  • i++ implies that the i is incremented AFTER the statement is complete. This means say for the year, say it is 2014, this ends up being 322 (2222) inside the tm struct, so the correct answer is i++ for the first, and ++i for each subsequent. – Brad Mitchell Jul 30 '14 at 00:45
  • 6
    No, your code is _still_ wrong. You simply can _not_ have more than one `i++` in the same expression since order of evaluation is not guaranteed (left-to-right vs right-to-left). – Paul J. Lucas May 08 '15 at 16:36
  • Great! I have added my answer as well. I may be a workaround but it would be great if u could check and comment: https://stackoverflow.com/a/59721373/5735010 – abhiarora Feb 04 '20 at 16:33
2

time_t may have a narrower range than ASN1_TIME and therefore ASN1_TIME_* functions might be more robust alternative. For example, to compare times, you could use ASN1_TIME_diff() (this avoids possible security issues with overflow if time_t is used). To print in a human readable format, call ASN1_TIME_print(), etc.

So far none of the answers follow rfc 5280 which specifies that the input times are in UTC (mktime() expects time in the local timezone i.e., the answers are incorrect if the local time zone is not UTC). Also:

Conforming systems MUST interpret the year field (YY) as follows: Where YY is greater than or equal to 50, the year SHALL be interpreted as 19YY; and Where YY is less than 50, the year SHALL be interpreted as 20YY.

i.e., if (tm_year < 70) tm_year += 100; violates the rfc. This answer uses year += year < 50 ? 2000 : 1900.

Additionally, 99991231235959Z in the input means that the certificate has no well-defined expiration date (the function should return (time_t)-1 -- an error).

To convert UTCTime or GeneralizedTime strings (ASN1_TIME*) to seconds since Epoch (time_t):

typedef unsigned U;

time_t ASN1_TIME_to_posix_time(const ASN1_TIME* time) {
  if(!time) return -1;
  const char *s = (const char*)time->data;
  if (!s) return -1;

  U two_digits_to_uint() // nested function: gcc extension
  {
    U n = 10 * (*s++ - '0');
    return n + (*s++ - '0');
  }
  U year, month, day, hour, min, sec;
  switch(time->type) {
    // https://www.rfc-editor.org/rfc/rfc5280#section-4.1.2.5.1
  case V_ASN1_UTCTIME: // YYMMDDHHMMSSZ
    year = two_digits_to_uint();
    year += year < 50 ? 2000 : 1900;
    break;
  case V_ASN1_GENERALIZEDTIME: // YYYYMMDDHHMMSSZ
    year = 100 * two_digits_to_uint();
    year += two_digits_to_uint();
    break;
  default:
    return -1; // error
  }
  month = two_digits_to_uint();
  day   = two_digits_to_uint();
  hour  = two_digits_to_uint();
  min   = two_digits_to_uint();
  sec   = two_digits_to_uint();
  if (*s != 'Z') return -1;
  if (year == 9999 && month == 12 && day == 31 && hour == 23 && min == 59
      && sec == 59) // 99991231235959Z rfc 5280
    return -1;
  return posix_time(year, month, day, hour, min, sec);
}

where posix_time() is used to convert broken-down UTC time to the calendar time. Seconds Since the Epoch:

time_t posix_time(U year, U month, U day, U hour, U min, U sec)
{
  if (year < 1970 || month < 1 || month > 12 || day < 1 || day > 31
      || hour > 23 || min > 59 || sec > 60)
    return -1;

  // days upto months for non-leap years
  static const U month_day[13] =
    {-1, 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334};
  year -= 1900;
  // number of Februaries since 1900
  const U year_for_leap = (month > 2) ? year + 1 : year;
  // XXX may overflow
  return sec + min*60 + hour*3600 + (month_day[month] + day - 1)*86400 +
    (year-70)*31536000 + ((year_for_leap-69)/4)*86400 -
    ((year_for_leap-1)/100)*86400 + ((year_for_leap+299)/400)*86400;
}

month_day and year_for_leap are from @DTiedy's answer.

Community
  • 1
  • 1
jfs
  • 399,953
  • 195
  • 994
  • 1,670
  • Great! I have added my answer as well. I may be a workaround but it would be great if u could check and comment – abhiarora Jan 20 '20 at 07:13
2

I know it's too late and openssl has a introduced a function ASN1_TIME_to_tm but I had to use older version of openssl which doesn't have this method.

I saw various possible answers to this question and they were parsing the time string in their code but I was not comfortable with that approach as I thought I might somehow miss something in parsing logic and my code could break or may not handle all corner cases. So, I implemented the function for C++ which uses openssl functions only to achieve the conversion.

It uses ASN1_TIME_diff to calculate seconds from epoch. To get ASN1_TIME for epoch, I used ASN1_TIME_SET with time_t argument passed as 0.

Feel free to comment and test.

bool _ASN1_TIME_to_tm(const ASN1_TIME *pTime, struct tm *pTm)
{
    int days = 0, seconds = 0;
    ASN1_TIME *epochTime = ASN1_TIME_new();
    ASN1_TIME_set(epochTime, time_t(0));

    if (!ASN1_TIME_diff(&days, &seconds, epochTime, pTime))
        return false;
    time_t sinceEpoch = time_t(86400LL * days + seconds); // No of seconds in a day = 86400
    gmtime_r(&sinceEpoch, pTm);
    std::cout << "DateTime: " << TOS::convertTmToStr(*pTm) << std::endl;
    ASN1_TIME_free(epochTime);
    return true;
}

Or Code with more checking:

bool _ASN1_TIME_to_tm(const ASN1_TIME *pTime, struct tm *pTm)
{
    bool result = false;
    time_t sinceEpoch = 0;
    int days = 0, seconds = 0;
    if (!pTime)
        return false;

    ASN1_TIME *epochTime = ASN1_TIME_new();
    if (!epochTime)
        return false;
    do {
        if (!ASN1_TIME_set(epochTime, time_t(0)))
            break;
        if (!ASN1_TIME_diff(&days, &seconds, epochTime, pTime))
            break;
        // No of seconds in a day = 86400
        sinceEpoch = time_t(86400LL * days + seconds);
        gmtime_r(&sinceEpoch, pTm);
        std::cout << "DateTime: " << TOS::convertTmToStr(*pTm) << std::endl;
        result = true;
    } while (0);

    ASN1_TIME_free(epochTime);
    return result;
}
abhiarora
  • 9,743
  • 5
  • 32
  • 57
  • I like your approach although your implementation has potential memory leak. In case of error, you don't free `epochTime` – Gils Jan 20 '22 at 19:06