X509_time_adj_ex normalizes the object type or not depending on how it was created

The X509_time_adj_ex() function overwrites an existing ASN1_TIME value with a new time depending on the arguments passed. It has this signature:

ASN1_TIME *X509_time_adj_ex(ASN1_TIME *asn1_time, int offset_day, long
                            offset_sec, time_t *in_tm);

The given asn1_time value (if not NULL) can have a type of either ASN1_UTCTIME (tag 0x17) or ASN1_GENERALIZEDTIME (tag 0x18). If the object was first created using ASN1_UTCTIME_new or ASN1_GENERALIZEDTIME_new or d2i_ASN1_TIME, the object's existing type will be preserved, regardless of what new time is specified.

However, if the asn1_time object was previously created with ASN1_TIME_new(), then the call to X509_time_adj_ex() will change the type of the object to UTC or Generalized depending on the year of the specified time.

The fact that X509_time_adj_ex() can change the object type is not documented, in contrast with ASN1_TIME_adj().

Furthermore, the fact that whether it does so is dependent on the history of object reuse is troubling, as one would normally expect that history to be irrelevant; a reused object should have the same behavior as a newly-allocated object.

Example (excerpt of test program):

void test_x509_time_adj(int bug)
{
  ASN1_TIME *t = NULL;
  if (!bug) {
    t = ASN1_GENERALIZEDTIME_new();     // Works.
  }
  else {
    t = ASN1_TIME_new();                // Leads to the bug.
  }

  // Use 'd2i' to populate 't' with a GeneralizedTime.
  d2i_ASN1_TIME(&t, &p, sizeof(derGen));
  assert(t->type == 0x18);

  // Use 'adj' to change the time.
  X509_time_adj_ex(t, 0, 0, NULL /*in_tm*/);
  if (t->type != 0x18) {
    printf("bug detected!  type has changed to 0x%X\n", t->type);
  }
}

x509_time_adj.c.txt is a complete program demonstrating this behavior. Its output is:

---- test_x509_time_adj, bug=0 ----
ASN1_GENERALIZEDTIME_new:
  len=0 type=0x18 flags=0x0 data: ""
d2i_ASN1_TIME(derGen):
  len=15 type=0x18 flags=0x0 data: "20101108181047Z"
  Nov  8 18:10:47 2010 GMT
X509_time_adj_ex:
  len=15 type=0x18 flags=0x0 data: "20210326024544Z"
  Mar 26 02:45:44 2021 GMT
---- test_x509_time_adj, bug=1 ----
ASN1_TIME_new:
  len=0 type=0xFFFFFFFF flags=0x40 data: ""
d2i_ASN1_TIME(derGen):
  len=15 type=0x18 flags=0x40 data: "20101108181047Z"
  Nov  8 18:10:47 2010 GMT
X509_time_adj_ex:
  len=13 type=0x17 flags=0x40 data: "210326024544Z"
  Mar 26 02:45:44 2021 GMT
bug detected!  type has changed to 0x17

I have observed this behavior with OpenSSL 1.1.1b and 1.1.1j running on Windows 10, built from source.

Recommended fix: This bug occurs because asn1_ex_c2i() does not set or clear the ASN1_STRING_FLAG_MSTRING flag when an object is reused. The only place this flag is read is in X509_time_adj_ex(), where its effect is to normalize the object type rather than preserving it (i.e., to cause this bug). I suggest simply removing the ASN1_STRING_FLAG_MSTRING flag altogether to fix this bug. With that change, X509_time_adj_ex would always preserve the object type when it is ASN1_UTCTIME or ASN1_GENERALIZEDTIME.

Discussion of severity: The d2i_ASN1_TIME() man page explains that object reuse is discouraged, and I agree with that advice, but so long as the functionality is made available, bugs in it ought to be fixed since, among other things, they could contribute to security vulnerabilities. (I'm filing this publicly because I think it is very unlikely that this alone would be such an issue, although it is impossible to completely rule it out.)

Start with the ASN1_TIME reuse path through asn1_ex_c2i() and X509_time_adj_ex(), using the x509_time_adj.c.txt reproducer to observe the type change. Check how ASN1_STRING_FLAG_MSTRING is set and read. Done means reused ASN1_UTCTIME or ASN1_GENERALIZEDTIME objects preserve their existing type after adjustment, including the demonstrated case.