From 708ede22c6781e854739c67332ac18a391f4782f Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 18 Sep 2015 12:32:23 -0700 Subject: [PATCH] Refactor X509 generalizedtime support and test. The generalizedtime support in for certificates now fits more seamlessly to how the rest of the code was structured and does the different processing for UTC and generalized times at the beginning, when checking for them. The test does not output the common name anymore, since the output format might change accross openssl versions (inserted the serial instead). I also added a bit more error checking for the UTC time case. --- src/file_analysis/analyzer/x509/X509.cc | 64 ++++++++++--------- .../Baseline/core.x509-generalizedtime/output | 8 +-- testing/btest/core/x509-generalizedtime.bro | 2 +- 3 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/file_analysis/analyzer/x509/X509.cc b/src/file_analysis/analyzer/x509/X509.cc index 9059a3e250..534676d41f 100644 --- a/src/file_analysis/analyzer/x509/X509.cc +++ b/src/file_analysis/analyzer/x509/X509.cc @@ -521,7 +521,7 @@ double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime, const char* const char *fid = arg_fid ? arg_fid : ""; time_t lResult = 0; - char lBuffer[24]; + char lBuffer[26]; char* pBuffer = lBuffer; const char *pString = (const char *) atime->data; @@ -535,16 +535,35 @@ double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime, const char* return 0; } + if ( pString[atime->length-1] != 'Z' ) + { + // not valid according to RFC 2459 4.1.2.5.1 + reporter->Weird(fmt("Could not parse UTC time in non-YY-format in X509 certificate (x509 %s)", fid)); + return 0; + } + + // year is first two digits in YY format. Buffer expects YYYY format. + if ( pString[0] - '0' < 50 ) // RFC 2459 4.1.2.5.1 + { + *(pBuffer++) = '2'; + *(pBuffer++) = '0'; + } + else + { + *(pBuffer++) = '1'; + *(pBuffer++) = '9'; + } + memcpy(pBuffer, pString, 10); pBuffer += 10; pString += 10; remaining -= 10; } - - else + else if ( atime->type == V_ASN1_GENERALIZEDTIME ) { // generalized time. We apparently ignore the YYYYMMDDHH case // for now and assume we always have minutes and seconds. + // This should be ok because it is specified as a requirement in RFC 2459 4.1.2.5.2 if ( remaining < 12 || remaining > 23 ) { @@ -557,6 +576,11 @@ double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime, const char* pString += 12; remaining -= 12; } + else + { + reporter->Weird(fmt("Invalid time type in X509 certificate (fuid %s)", fid)); + return 0; + } if ( (remaining == 0) || (*pString == 'Z') || (*pString == '-') || (*pString == '+') ) { @@ -620,33 +644,15 @@ double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime, const char* } tm lTime; - size_t i; - if ( atime->type == V_ASN1_GENERALIZEDTIME ) - { - // YYYY format - lTime.tm_year = (lBuffer[0] - '0') * 1000; - lTime.tm_year += (lBuffer[1] - '0') * 100; - lTime.tm_year += (lBuffer[2] - '0') * 10; - lTime.tm_year += (lBuffer[3] - '0'); - if ( lTime.tm_year > 1900) - lTime.tm_year -= 1900; - i = 4; - } - else - { - // YY format - lTime.tm_year = (lBuffer[0] - '0') * 10; - lTime.tm_year += (lBuffer[1] - '0'); - if ( lTime.tm_year < 50 ) - lTime.tm_year += 100; // RFC 2459 - i = 2; - } + lTime.tm_sec = ((lBuffer[12] - '0') * 10) + (lBuffer[13] - '0'); + lTime.tm_min = ((lBuffer[10] - '0') * 10) + (lBuffer[11] - '0'); + lTime.tm_hour = ((lBuffer[8] - '0') * 10) + (lBuffer[9] - '0'); + lTime.tm_mday = ((lBuffer[6] - '0') * 10) + (lBuffer[7] - '0'); + lTime.tm_mon = (((lBuffer[4] - '0') * 10) + (lBuffer[5] - '0')) - 1; + lTime.tm_year = (lBuffer[0] - '0') * 1000 + (lBuffer[1] - '0') * 100 + ((lBuffer[2] - '0') * 10) + (lBuffer[3] - '0'); - lTime.tm_mon = ((lBuffer[i+0] - '0') * 10) + (lBuffer[i+1] - '0') - 1; // MM - lTime.tm_mday = ((lBuffer[i+2] - '0') * 10) + (lBuffer[i+3] - '0'); // DD - lTime.tm_hour = ((lBuffer[i+4] - '0') * 10) + (lBuffer[i+5] - '0'); // hh - lTime.tm_min = ((lBuffer[i+6] - '0') * 10) + (lBuffer[i+7] - '0'); // mm - lTime.tm_sec = ((lBuffer[i+8] - '0') * 10) + (lBuffer[i+9] - '0'); // ss + if ( lTime.tm_year > 1900) + lTime.tm_year -= 1900; lTime.tm_wday = 0; lTime.tm_yday = 0; diff --git a/testing/btest/Baseline/core.x509-generalizedtime/output b/testing/btest/Baseline/core.x509-generalizedtime/output index 75605f5668..349551efe5 100644 --- a/testing/btest/Baseline/core.x509-generalizedtime/output +++ b/testing/btest/Baseline/core.x509-generalizedtime/output @@ -1,16 +1,16 @@ ----- x509_certificate ---- -subject: CN=bro-generalizedtime-test,O=Bro,C=NL +serial: 03E8 not_valid_before: 2015-09-01-13:33:37.000000000 (epoch: 1441114417.0) not_valid_after : 2025-09-01-13:33:37.000000000 (epoch: 1756733617.0) ----- x509_certificate ---- -subject: CN=*.taleo.net,OU=Comodo PremiumSSL Wildcard,OU=Web,O=Taleo Inc.,street=4140 Dublin Boulevard,street=Suite 400,L=Dublin,ST=CA,postalCode=94568,C=US +serial: 99FAA8037A4EB2FAEF84EB5E55D5B8C8 not_valid_before: 2011-05-04-00:00:00.000000000 (epoch: 1304467200.0) not_valid_after : 2016-07-04-23:59:59.000000000 (epoch: 1467676799.0) ----- x509_certificate ---- -subject: CN=COMODO High-Assurance Secure Server CA,O=COMODO CA Limited,L=Salford,ST=Greater Manchester,C=GB +serial: 1690C329B6780607511F05B0344846CB not_valid_before: 2010-04-16-00:00:00.000000000 (epoch: 1271376000.0) not_valid_after : 2020-05-30-10:48:38.000000000 (epoch: 1590835718.0) ----- x509_certificate ---- -subject: CN=AddTrust External CA Root,OU=AddTrust External TTP Network,O=AddTrust AB,C=SE +serial: 01 not_valid_before: 2000-05-30-10:48:38.000000000 (epoch: 959683718.0) not_valid_after : 2020-05-30-10:48:38.000000000 (epoch: 1590835718.0) diff --git a/testing/btest/core/x509-generalizedtime.bro b/testing/btest/core/x509-generalizedtime.bro index 5d82b28ca8..b69ab31743 100644 --- a/testing/btest/core/x509-generalizedtime.bro +++ b/testing/btest/core/x509-generalizedtime.bro @@ -4,7 +4,7 @@ event x509_certificate(f: fa_file, cert_ref: opaque of x509, cert: X509::Certificate) { print "----- x509_certificate ----"; - print fmt("subject: %s", cert$subject); + print fmt("serial: %s", cert$serial); print fmt("not_valid_before: %T (epoch: %s)", cert$not_valid_before, cert$not_valid_before); print fmt("not_valid_after : %T (epoch: %s)", cert$not_valid_after, cert$not_valid_after); }