diff --git a/src/file_analysis/analyzer/x509/X509.cc b/src/file_analysis/analyzer/x509/X509.cc index 8c70597dca..d9604740a7 100644 --- a/src/file_analysis/analyzer/x509/X509.cc +++ b/src/file_analysis/analyzer/x509/X509.cc @@ -52,7 +52,8 @@ bool file_analysis::X509::EndOfFile() X509Val* cert_val = new X509Val(ssl_cert); // cert_val takes ownership of ssl_cert - RecordVal* cert_record = ParseCertificate(cert_val); // parse basic information into record + // parse basic information into record. + RecordVal* cert_record = ParseCertificate(cert_val, GetFile()->GetID().c_str()); // and send the record on to scriptland val_list* vl = new val_list(); @@ -84,7 +85,7 @@ bool file_analysis::X509::EndOfFile() return false; } -RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val) +RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val, const char* fid) { ::X509* ssl_cert = cert_val->GetCertificate(); @@ -131,8 +132,8 @@ RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val) pX509Cert->Assign(3, new StringVal(len, buf)); BIO_free(bio); - pX509Cert->Assign(5, new Val(GetTimeFromAsn1(X509_get_notBefore(ssl_cert)), TYPE_TIME)); - pX509Cert->Assign(6, new Val(GetTimeFromAsn1(X509_get_notAfter(ssl_cert)), TYPE_TIME)); + pX509Cert->Assign(5, new Val(GetTimeFromAsn1(X509_get_notBefore(ssl_cert), fid), TYPE_TIME)); + pX509Cert->Assign(6, new Val(GetTimeFromAsn1(X509_get_notAfter(ssl_cert), fid), TYPE_TIME)); // we only read 255 bytes because byte 256 is always 0. // if the string is longer than 255, that will be our null-termination, @@ -515,54 +516,79 @@ unsigned int file_analysis::X509::KeyLength(EVP_PKEY *key) reporter->InternalError("cannot be reached"); } -double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime) +double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid) { + const char *fid = arg_fid ? arg_fid : ""; time_t lResult = 0; char lBuffer[24]; char* pBuffer = lBuffer; - size_t lTimeLength = atime->length; - char * pString = (char *) atime->data; + const char *pString = (const char *) atime->data; + unsigned int remaining = atime->length; if ( atime->type == V_ASN1_UTCTIME ) { - if ( lTimeLength < 11 || lTimeLength > 17 ) + if ( remaining < 11 || remaining > 17 ) + { + reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- UTCTime has wrong length", fid)); return 0; + } memcpy(pBuffer, pString, 10); pBuffer += 10; pString += 10; + remaining -= 10; } else { - if ( lTimeLength < 13 ) + // generalized time. We apparently ignore the YYYYMMDDHH case + // for now and assume we always have minutes and seconds. + + if ( remaining < 12 || remaining > 23 ) + { + reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- Generalized time has wrong length", fid)); return 0; + } memcpy(pBuffer, pString, 12); pBuffer += 12; pString += 12; + remaining -= 12; } - if ((*pString == 'Z') || (*pString == '-') || (*pString == '+')) + if ( (remaining == 0) || (*pString == 'Z') || (*pString == '-') || (*pString == '+') ) { *(pBuffer++) = '0'; *(pBuffer++) = '0'; } + else if ( remaining >= 2 ) + { + *(pBuffer++) = *(pString++); + *(pBuffer++) = *(pString++); + + remaining -= 2; + + // Skip any fractional seconds... + if ( (remaining > 0) && (*pString == '.') ) + { + pString++; + remaining--; + + while ( (remaining > 0) && (*pString >= '0') && (*pString <= '9') ) + { + pString++; + remaining--; + } + } + } + else { - *(pBuffer++) = *(pString++); - *(pBuffer++) = *(pString++); - - // Skip any fractional seconds... - if (*pString == '.') - { - pString++; - while ((*pString >= '0') && (*pString <= '9')) - pString++; - } + reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- additional char after time", fid)); + return 0; } *(pBuffer++) = 'Z'; @@ -570,16 +596,24 @@ double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime) time_t lSecondsFromUTC; - if ( *pString == 'Z' ) + if ( remaining == 0 || *pString == 'Z' ) lSecondsFromUTC = 0; - else { - if ((*pString != '+') && (pString[5] != '-')) + if ( remaining < 5 ) + { + reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- not enough bytes remaining for offset", fid)); return 0; + } - lSecondsFromUTC = ((pString[1]-'0') * 10 + (pString[2]-'0')) * 60; - lSecondsFromUTC += (pString[3]-'0') * 10 + (pString[4]-'0'); + if ((*pString != '+') && (*pString != '-')) + { + reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- unknown offset type", fid)); + return 0; + } + + lSecondsFromUTC = ((pString[1] - '0') * 10 + (pString[2] - '0')) * 60; + lSecondsFromUTC += (pString[3] - '0') * 10 + (pString[4] - '0'); if (*pString == '-') lSecondsFromUTC = -lSecondsFromUTC; @@ -604,7 +638,7 @@ double file_analysis::X509::GetTimeFromAsn1(const ASN1_TIME* atime) if ( lResult ) { - if ( 0 != lTime.tm_isdst ) + if ( lTime.tm_isdst != 0 ) lResult -= 3600; // mktime may adjust for DST (OS dependent) lResult += lSecondsFromUTC; diff --git a/src/file_analysis/analyzer/x509/X509.h b/src/file_analysis/analyzer/x509/X509.h index bd4c8fc7a5..c671c68a99 100644 --- a/src/file_analysis/analyzer/x509/X509.h +++ b/src/file_analysis/analyzer/x509/X509.h @@ -29,10 +29,13 @@ public: * * @param cert_val The certificate to converts. * + * @param fid A file ID associated with the certificate, if any + * (primarily for error reporting). + * * @param Returns the new record value and passes ownership to * caller. */ - static RecordVal* ParseCertificate(X509Val* cert_val); + static RecordVal* ParseCertificate(X509Val* cert_val, const char* fid = 0); static file_analysis::Analyzer* Instantiate(RecordVal* args, File* file) { return new X509(args, file); } @@ -59,7 +62,7 @@ private: std::string cert_data; // Helpers for ParseCertificate. - static double GetTimeFromAsn1(const ASN1_TIME * atime); + static double GetTimeFromAsn1(const ASN1_TIME * atime, const char* fid); static StringVal* KeyCurve(EVP_PKEY *key); static unsigned int KeyLength(EVP_PKEY *key); };