From: Harlan Stenn Date: Sat, 15 Nov 2014 04:42:01 +0000 (+0000) Subject: [Bug 2651] Certificates with ASN timestamps w/ 4-digit years mis-parsed X-Git-Tag: NTP_4_2_7P479~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d839df345093f080ff9624c7f541a816c6fae869;p=thirdparty%2Fntp.git [Bug 2651] Certificates with ASN timestamps w/ 4-digit years mis-parsed bk: 5466d999iwytuuZxXdC0E0xqFgG7sA --- diff --git a/ChangeLog b/ChangeLog index 2cc6b81e1..12a086cf0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ +* [Bug 2651] Certificates with ASN timestamps w/ 4-digit years mis-parsed. (4.2.7p478) 2014/11/14 Released by Harlan Stenn * [Sec 2630] buffer overrun in ntpq tokenize(). * [Bug 2639] Check return value of ntp_adjtime(). diff --git a/include/ntp_crypto.h b/include/ntp_crypto.h index bba39e1c1..b80100614 100644 --- a/include/ntp_crypto.h +++ b/include/ntp_crypto.h @@ -25,6 +25,7 @@ invalidsyntax: AUTOKEY should be defined only if OPENSSL is. #endif #include "openssl/evp.h" +#include "ntp_calendar.h" /* for fields in the cert_info structure */ /* @@ -147,6 +148,7 @@ struct exten { u_int32 pkt[1]; /* start of value field */ }; + /* * The certificate info/value structure */ @@ -158,8 +160,8 @@ struct cert_info { int nid; /* signature/digest ID */ const EVP_MD *digest; /* message digest algorithm */ u_long serial; /* serial number */ - tstamp_t first; /* not valid before */ - tstamp_t last; /* not valid after */ + struct calendar first; /* not valid before */ + struct calendar last; /* not valid after */ char *subject; /* subject common name */ char *issuer; /* issuer common name */ BIGNUM *grpkey; /* GQ group key */ diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c index dde46318b..266978e4a 100644 --- a/ntpd/ntp_control.c +++ b/ntpd/ntp_control.c @@ -2319,11 +2319,14 @@ ctl_putsys( case CS_CERTIF: for (cp = cinfo; cp != NULL; cp = cp->link) { + tstamp_t tstamp; + snprintf(str, sizeof(str), "%s %s 0x%x", cp->subject, cp->issuer, cp->flags); ctl_putstr(sys_var[CS_CERTIF].text, str, strlen(str)); - ctl_putfs(sys_var[CS_REVTIME].text, cp->last); + tstamp = caltontp(&(cp->last)); /* XXX too small to hold some values, but that's what ctl_putfs requires */ + ctl_putfs(sys_var[CS_REVTIME].text, tstamp); } break; diff --git a/ntpd/ntp_crypto.c b/ntpd/ntp_crypto.c index 010bf2daf..e66d5c782 100644 --- a/ntpd/ntp_crypto.c +++ b/ntpd/ntp_crypto.c @@ -7,6 +7,7 @@ #ifdef AUTOKEY #include +#include /* strtoul */ #include #include #include @@ -33,6 +34,33 @@ #include "ntp_syscall.h" #endif /* KERNEL_PLL */ +/* + * calcomp - compare two calendar structures, ignoring yearday and weekday; like strcmp + * No, it's not a plotter. If you don't understand that, you're too young. + */ +static int calcomp(struct calendar *pjd1, struct calendar *pjd2) +{ + int32_t diff; /* large enough to hold the signed difference between two uint16_t values */ + + diff = pjd1->year - pjd2->year; + if (diff < 0) return -1; else if (diff > 0) return 1; + /* same year; compare months */ + diff = pjd1->month - pjd2->month; + if (diff < 0) return -1; else if (diff > 0) return 1; + /* same year and month; compare monthday */ + diff = pjd1->monthday - pjd2->monthday; + if (diff < 0) return -1; else if (diff > 0) return 1; + /* same year and month and monthday; compare time */ + diff = pjd1->hour - pjd2->hour; + if (diff < 0) return -1; else if (diff > 0) return 1; + diff = pjd1->minute - pjd2->minute; + if (diff < 0) return -1; else if (diff > 0) return 1; + diff = pjd1->second - pjd2->second; + if (diff < 0) return -1; else if (diff > 0) return 1; + /* identical */ + return 0; +} + /* * Extension field message format * @@ -164,7 +192,7 @@ static int crypto_gq (struct exten *, struct peer *); static int crypto_mv (struct exten *, struct peer *); static int crypto_send (struct exten *, struct value *, int); static tstamp_t crypto_time (void); -static u_long asn2ntp (ASN1_TIME *); +static void asn_to_calendar (ASN1_TIME *, struct calendar*); static struct cert_info *cert_parse (const u_char *, long, tstamp_t); static int cert_sign (struct exten *, struct value *); static struct cert_info *cert_install (struct exten *, struct peer *); @@ -1076,6 +1104,7 @@ crypto_xmit( char certname[MAXHOSTNAME + 1]; /* subject name buffer */ char statstr[NTP_MAXSTRLEN]; /* statistics for filegen */ tstamp_t tstamp; + struct calendar tscal; u_int vallen; struct value vtemp; associd_t associd; @@ -1135,8 +1164,9 @@ crypto_xmit( * signed and may or may not be trusted. */ case CRYPTO_SIGN: - if (tstamp < cert_host->first || tstamp > - cert_host->last) + (void)ntpcal_ntp_to_date(&tscal, tstamp, NULL); + if ((calcomp(&tscal, &(cert_host->first)) < 0) + || (calcomp(&tscal, &(cert_host->last)) > 0)) rval = XEVNT_PER; else len = crypto_send(fp, &cert_host->cert, start); @@ -1906,39 +1936,62 @@ crypto_time() /* - * asn2ntp - convert ASN1_TIME time structure to NTP time. + * asn_to_calendar - convert ASN1_TIME time structure to struct calendar. * - * Returns NTP seconds (no errors) */ -u_long -asn2ntp ( - ASN1_TIME *asn1time /* pointer to ASN1_TIME structure */ +static +void +asn_to_calendar ( + ASN1_TIME *asn1time, /* pointer to ASN1_TIME structure */ + struct calendar *pjd /* pointer to result */ ) { - char *v; /* pointer to ASN1_TIME string */ - struct calendar jd; /* used to convert to NTP time */ + int len; /* length of ASN1_TIME string */ + char v[24]; /* writable copy of ASN1_TIME string */ + unsigned long temp; /* result from strtoul */ /* * Extract time string YYMMDDHHMMSSZ from ASN1 time structure. + * Or YYYYMMDDHHMMSSZ. * Note that the YY, MM, DD fields start with one, the HH, MM, - * SS fiels start with zero and the Z character is ignored. - * Also note that years less than 50 map to years greater than + * SS fields start with zero and the Z character is ignored. + * Also note that two-digit years less than 50 map to years greater than * 100. Dontcha love ASN.1? Better than MIL-188. */ - v = (char *)asn1time->data; - jd.year = (v[0] - '0') * 10 + v[1] - '0'; - if (jd.year < 50) - jd.year += 100; - jd.year += 1900; /* should we do century unfolding here? */ - jd.month = (v[2] - '0') * 10 + v[3] - '0'; - jd.monthday = (v[4] - '0') * 10 + v[5] - '0'; - jd.hour = (v[6] - '0') * 10 + v[7] - '0'; - jd.minute = (v[8] - '0') * 10 + v[9] - '0'; - jd.second = (v[10] - '0') * 10 + v[11] - '0'; - jd.yearday = 0; - jd.weekday = 0; - - return caltontp(&jd); + len = asn1time->length; + NTP_REQUIRE(len < sizeof(v)); + (void)strncpy(v, (char *)(asn1time->data), len); + NTP_REQUIRE(len >= 13); + temp = strtoul(v+len-3, NULL, 10); + pjd->second = temp; + v[len-3] = '\0'; + + temp = strtoul(v+len-5, NULL, 10); + pjd->minute = temp; + v[len-5] = '\0'; + + temp = strtoul(v+len-7, NULL, 10); + pjd->hour = temp; + v[len-7] = '\0'; + + temp = strtoul(v+len-9, NULL, 10); + pjd->monthday = temp; + v[len-9] = '\0'; + + temp = strtoul(v+len-11, NULL, 10); + pjd->month = temp; + v[len-11] = '\0'; + + temp = strtoul(v, NULL, 10); + /* handle two-digit years */ + if (temp < 50UL) + temp += 100UL; + if (temp < 150UL) + temp += 1900UL; + pjd->year = temp; + + pjd->yearday = pjd->weekday = 0; + return; } @@ -2949,6 +3002,7 @@ cert_sign( EVP_PKEY *pkey; /* public key */ EVP_MD_CTX ctx; /* message digest context */ tstamp_t tstamp; /* NTP timestamp */ + struct calendar tscal; u_int len; const u_char *cptr; u_char *ptr; @@ -3009,7 +3063,9 @@ cert_sign( * Sign and verify the client certificate, but only if the host * certificate has not expired. */ - if (tstamp < cert_host->first || tstamp > cert_host->last) { + (void)ntpcal_ntp_to_date(&tscal, tstamp, NULL); + if ((calcomp(&tscal, &(cert_host->first)) < 0) + || (calcomp(&tscal, &(cert_host->last)) > 0)) { X509_free(cert); return (XEVNT_PER); } @@ -3196,7 +3252,8 @@ cert_hike( * Signature X is valid only if it begins during the * lifetime of Y. */ - if (xp->first < yp->first || xp->first > yp->last) { + if ((calcomp(&(xp->first), &(yp->first)) < 0) + || (calcomp(&(xp->first), &(yp->last)) > 0)) { xp->flags |= CERT_ERROR; return (XEVNT_PER); } @@ -3233,6 +3290,7 @@ cert_parse( const u_char *ptr; char *pch; int temp, cnt, i; + struct calendar fscal; /* * Decode ASN.1 objects and construct certificate structure. @@ -3293,8 +3351,8 @@ cert_parse( return (NULL); } ret->issuer = estrdup(pch + 3); - ret->first = asn2ntp(X509_get_notBefore(cert)); - ret->last = asn2ntp(X509_get_notAfter(cert)); + asn_to_calendar(X509_get_notBefore(cert), &(ret->first)); + asn_to_calendar(X509_get_notAfter(cert), &(ret->last)); /* * Extract extension fields. These are ad hoc ripoffs of @@ -3377,10 +3435,18 @@ cert_parse( * Verify certificate valid times. Note that certificates cannot * be retroactive. */ - if (ret->first > ret->last || ret->first < fstamp) { + (void)ntpcal_ntp_to_date(&fscal, fstamp, NULL); + if ((calcomp(&(ret->first), &(ret->last)) > 0) + || (calcomp(&(ret->first), &fscal) < 0)) { msyslog(LOG_NOTICE, - "cert_parse: invalid times %s first %u last %u fstamp %u", - ret->subject, ret->first, ret->last, fstamp); + "cert_parse: invalid times %s first %u-%02u-%02uT%02u:%02u:%02u last %u-%02u-%02uT%02u:%02u:%02u fstamp %u-%02u-%02uT%02u:%02u:%02u", + ret->subject, + ret->first.year, ret->first.month, ret->first.monthday, + ret->first.hour, ret->first.minute, ret->first.second, + ret->last.year, ret->last.month, ret->last.monthday, + ret->last.hour, ret->last.minute, ret->last.second, + fscal.year, fscal.month, fscal.monthday, + fscal.hour, fscal.minute, fscal.second); cert_free(ret); X509_free(cert); return (NULL);