From 04e62715db684d83bffac53793ff4cfac51e047a Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Sun, 11 Jun 2017 16:36:07 -0400 Subject: Introduce ASN1_TIME_set_string_X509 API Make funcs to deal with non-null-term'd string in both asn1_generalizedtime_to_tm() and asn1_utctime_to_tm(). Fixes issue #3444. This one is used to enforce strict format (RFC 5280) check and to convert GeneralizedTime to UTCTime. apps/ca has been changed to use the new API. Test cases and documentation are updated/added Signed-off-by: Paul Yang Reviewed-by: Kurt Roeckx Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3566) --- crypto/asn1/a_gentm.c | 57 +++++++++++++++++++++++++++++++++++++++--------- crypto/asn1/a_time.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++- crypto/asn1/a_utctm.c | 43 +++++++++++++++++++++++++++++------- 3 files changed, 141 insertions(+), 19 deletions(-) (limited to 'crypto/asn1') diff --git a/crypto/asn1/a_gentm.c b/crypto/asn1/a_gentm.c index ff1b695475..bd90d05114 100644 --- a/crypto/asn1/a_gentm.c +++ b/crypto/asn1/a_gentm.c @@ -22,7 +22,7 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d) static const int min[9] = { 0, 0, 1, 1, 0, 0, 0, 0, 0 }; static const int max[9] = { 99, 99, 12, 31, 23, 59, 59, 12, 59 }; char *a; - int n, i, l, o; + int n, i, l, o, min_l = 13, strict = 0; if (d->type != V_ASN1_GENERALIZEDTIME) return (0); @@ -34,10 +34,26 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d) * as YYYY. This stuff treats everything as a two digit field so make * first two fields 00 to 99 */ - if (l < 13) + + /* + * ASN1_STRING_FLAG_X509_TIME is used to enforce RFC 5280 + * time string format, in which: + * + * 1. "seconds" is a 'MUST' + * 2. "Zulu" timezone is a 'MUST' + * 3. "+|-" is not allowed to indicate a time zone + * 4. fractional seconds are not allowed in GeneralizedTime + */ + + if (d->flags & ASN1_STRING_FLAG_X509_TIME) { + min_l = 15; + strict = 1; + } + + if (l < min_l) goto err; for (i = 0; i < 7; i++) { - if ((i == 6) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) { + if (!strict && (i == 6) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) { i++; if (tm) tm->tm_sec = 0; @@ -46,13 +62,15 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d) if ((a[o] < '0') || (a[o] > '9')) goto err; n = a[o] - '0'; - if (++o > l) + /* incomplete 2-digital number */ + if (++o == l) goto err; if ((a[o] < '0') || (a[o] > '9')) goto err; n = (n * 10) + a[o] - '0'; - if (++o > l) + /* no more bytes to read, but we haven't seen time-zone yet */ + if (++o == l) goto err; if ((n < min[i]) || (n > max[i])) @@ -88,22 +106,39 @@ int asn1_generalizedtime_to_tm(struct tm *tm, const ASN1_GENERALIZEDTIME *d) * digits. */ if (a[o] == '.') { - if (++o > l) + if (strict) + /* RFC 5280 forbids fractional seconds */ + goto err; + if (++o == l) goto err; i = o; - while ((a[o] >= '0') && (a[o] <= '9') && (o <= l)) + while ((o < l) && (a[o] >= '0') && (a[o] <= '9')) o++; /* Must have at least one digit after decimal point */ if (i == o) goto err; + /* no more bytes to read, but we haven't seen time-zone yet */ + if (o == l) + goto err; } - if (a[o] == 'Z') + /* + * 'o' will never point to '\0' at this point, the only chance + * 'o' can point th '\0' is either the subsequent if or the first + * else if is true. + */ + if (a[o] == 'Z') { o++; - else if ((a[o] == '+') || (a[o] == '-')) { + } else if (!strict && ((a[o] == '+') || (a[o] == '-'))) { int offsign = a[o] == '-' ? 1 : -1, offset = 0; o++; - if (o + 4 > l) + /* + * if not equal, no need to do subsequent checks + * since the following for-loop will add 'o' by 4 + * and the final return statement will check if 'l' + * and 'o' are equal. + */ + if (o + 4 != l) goto err; for (i = 7; i < 9; i++) { if ((a[o] < '0') || (a[o] > '9')) @@ -146,6 +181,8 @@ int ASN1_GENERALIZEDTIME_set_string(ASN1_GENERALIZEDTIME *s, const char *str) t.type = V_ASN1_GENERALIZEDTIME; t.length = strlen(str); t.data = (unsigned char *)str; + t.flags = 0; + if (ASN1_GENERALIZEDTIME_check(&t)) { if (s != NULL) { if (!ASN1_STRING_set((ASN1_STRING *)s, str, t.length)) diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c index 27f9bc6808..12b1ff5890 100644 --- a/crypto/asn1/a_time.c +++ b/crypto/asn1/a_time.c @@ -107,7 +107,6 @@ ASN1_GENERALIZEDTIME *ASN1_TIME_to_generalizedtime(const ASN1_TIME *t, return NULL; } - int ASN1_TIME_set_string(ASN1_TIME *s, const char *str) { ASN1_TIME t; @@ -130,6 +129,65 @@ int ASN1_TIME_set_string(ASN1_TIME *s, const char *str) return 1; } +int ASN1_TIME_set_string_X509(ASN1_TIME *s, const char *str) +{ + ASN1_TIME t; + struct tm tm; + int rv = 0; + + t.length = strlen(str); + t.data = (unsigned char *)str; + t.flags = ASN1_STRING_FLAG_X509_TIME; + + t.type = V_ASN1_UTCTIME; + + if (!ASN1_TIME_check(&t)) { + t.type = V_ASN1_GENERALIZEDTIME; + if (!ASN1_TIME_check(&t)) + goto out; + } + + /* + * Per RFC 5280 (section 4.1.2.5.), the valid input time + * strings should be encoded with the following rules: + * + * 1. UTC: YYMMDDHHMMSSZ, if YY < 50 (20YY) --> UTC: YYMMDDHHMMSSZ + * 2. UTC: YYMMDDHHMMSSZ, if YY >= 50 (19YY) --> UTC: YYMMDDHHMMSSZ + * 3. G'd: YYYYMMDDHHMMSSZ, if YYYY >= 2050 --> G'd: YYYYMMDDHHMMSSZ + * 4. G'd: YYYYMMDDHHMMSSZ, if YYYY < 2050 --> UTC: YYMMDDHHMMSSZ + * + * Only strings of the 4th rule should be reformatted, but since a + * UTC can only present [1950, 2050), so if the given time string + * is less than 1950 (e.g. 19230419000000Z), we do nothing... + */ + + if (s != NULL && t.type == V_ASN1_GENERALIZEDTIME) { + if (!asn1_generalizedtime_to_tm(&tm, &t)) + goto out; + if (tm.tm_year >= 50 && tm.tm_year < 150) { + t.length -= 2; + /* + * it's OK to let original t.data go since that's assigned + * to a piece of memory allocated outside of this function. + * new t.data would be freed after ASN1_STRING_copy is done. + */ + t.data = OPENSSL_zalloc(t.length + 1); + if (t.data == NULL) + goto out; + memcpy(t.data, str + 2, t.length); + t.type = V_ASN1_UTCTIME; + } + } + + if (s == NULL || ASN1_STRING_copy((ASN1_STRING *)s, (ASN1_STRING *)&t)) + rv = 1; + + if (t.data != (unsigned char *)str) + OPENSSL_free(t.data); +out: + return rv; +} + int ASN1_TIME_to_tm(const ASN1_TIME *s, struct tm *tm) { if (s == NULL) { diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c index 9797aa8a1e..ee98e4b489 100644 --- a/crypto/asn1/a_utctm.c +++ b/crypto/asn1/a_utctm.c @@ -18,7 +18,7 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) static const int min[8] = { 0, 1, 1, 0, 0, 0, 0, 0 }; static const int max[8] = { 99, 12, 31, 23, 59, 59, 12, 59 }; char *a; - int n, i, l, o; + int n, i, l, o, min_l = 11, strict = 0; if (d->type != V_ASN1_UTCTIME) return (0); @@ -26,10 +26,24 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) a = (char *)d->data; o = 0; - if (l < 11) + /* + * ASN1_STRING_FLAG_X509_TIME is used to enforce RFC 5280 + * time string format, in which: + * + * 1. "seconds" is a 'MUST' + * 2. "Zulu" timezone is a 'MUST' + * 3. "+|-" is not allowed to indicate a time zone + */ + + if (d->flags & ASN1_STRING_FLAG_X509_TIME) { + min_l = 13; + strict = 1; + } + + if (l < min_l) goto err; for (i = 0; i < 6; i++) { - if ((i == 5) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) { + if (!strict && (i == 5) && ((a[o] == 'Z') || (a[o] == '+') || (a[o] == '-'))) { i++; if (tm) tm->tm_sec = 0; @@ -38,13 +52,15 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) if ((a[o] < '0') || (a[o] > '9')) goto err; n = a[o] - '0'; - if (++o > l) + /* incomplete 2-digital number */ + if (++o == l) goto err; if ((a[o] < '0') || (a[o] > '9')) goto err; n = (n * 10) + a[o] - '0'; - if (++o > l) + /* no more bytes to read, but we haven't seen time-zone yet */ + if (++o == l) goto err; if ((n < min[i]) || (n > max[i])) @@ -72,12 +88,18 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) } } } - if (a[o] == 'Z') + + /* + * 'o' will never point to '\0' at this point, the only chance + * 'o' can point th '\0' is either the subsequent if or the first + * else if is true. + */ + if (a[o] == 'Z') { o++; - else if ((a[o] == '+') || (a[o] == '-')) { + } else if (!strict && ((a[o] == '+') || (a[o] == '-'))) { int offsign = a[o] == '-' ? 1 : -1, offset = 0; o++; - if (o + 4 > l) + if (o + 4 != l) goto err; for (i = 6; i < 8; i++) { if ((a[o] < '0') || (a[o] > '9')) @@ -99,6 +121,9 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) } if (offset && !OPENSSL_gmtime_adj(tm, 0, offset * offsign)) return 0; + } else { + /* not Z, or not +/- in non-strict mode */ + return 0; } return o == l; err: @@ -117,6 +142,8 @@ int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str) t.type = V_ASN1_UTCTIME; t.length = strlen(str); t.data = (unsigned char *)str; + t.flags = 0; + if (ASN1_UTCTIME_check(&t)) { if (s != NULL) { if (!ASN1_STRING_set((ASN1_STRING *)s, str, t.length)) -- cgit v1.2.3