From 8f990036f6ad1b9f5cb935a75c260a07d17b94c6 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 29 Jun 2018 15:58:53 -0500 Subject: [PATCH 1/4] Fixes for OpenSSL 1.1 support The following tests currently fail due to what seems like different behavior in OpenSSL 1.1 vs 1.0: scripts/base/protocols/rdp/rdp-x509.bro bifs/x509_verify.bro --- cmake | 2 +- src/file_analysis/analyzer/x509/OCSP.cc | 433 ++++++++++++++++-- src/file_analysis/analyzer/x509/OCSP.h | 4 +- src/file_analysis/analyzer/x509/X509.cc | 4 + src/file_analysis/analyzer/x509/functions.bif | 41 +- 5 files changed, 425 insertions(+), 59 deletions(-) diff --git a/cmake b/cmake index 1600554d1d..064bf442f2 160000 --- a/cmake +++ b/cmake @@ -1 +1 @@ -Subproject commit 1600554d1d907f4f252f19cf1f55e13d368a936f +Subproject commit 064bf442f22e72a2c15b3db6b7995189efaee982 diff --git a/src/file_analysis/analyzer/x509/OCSP.cc b/src/file_analysis/analyzer/x509/OCSP.cc index 02c9274999..e22440aab8 100644 --- a/src/file_analysis/analyzer/x509/OCSP.cc +++ b/src/file_analysis/analyzer/x509/OCSP.cc @@ -21,7 +21,7 @@ // helper function of sk_X509_value to avoid namespace problem // sk_X509_value(X,Y) = > SKM_sk_value(X509,X,Y) // X509 => file_analysis::X509 -X509 *helper_sk_X509_value(STACK_OF(X509) *certs, int i) +X509* helper_sk_X509_value(const STACK_OF(X509)* certs, int i) { return sk_X509_value(certs, i); } @@ -42,38 +42,84 @@ static Val* get_ocsp_type(RecordVal* args, const char* name) return rval; } -static void OCSP_RESPID_bio(OCSP_RESPID *resp_id, BIO* bio) +static bool OCSP_RESPID_bio(OCSP_BASICRESP* basic_resp, BIO* bio) { - if (resp_id->type == V_OCSP_RESPID_NAME) - X509_NAME_print_ex(bio, resp_id->value.byName, 0, XN_FLAG_ONELINE); - else if (resp_id->type == V_OCSP_RESPID_KEY) - i2a_ASN1_STRING(bio, resp_id->value.byKey, V_ASN1_OCTET_STRING); +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + ASN1_OCTET_STRING* key = nullptr; + X509_NAME* name = nullptr; + + if ( ! basic_resp->tbsResponseData ) + return false; + + auto resp_id = basic_resp->tbsResponseData->responderId; + + if ( resp_id->type == V_OCSP_RESPID_NAME ) + name = resp_id->value.byName; + else if ( resp_id->type == V_OCSP_RESPID_KEY ) + key = resp_id->value.byKey; + else + return false; +#else + const ASN1_OCTET_STRING* key = nullptr; + const X509_NAME* name = nullptr; + + if ( ! OCSP_resp_get0_id(basic_resp, &key, &name) ) + return false; +#endif + + if ( name ) + X509_NAME_print_ex(bio, name, 0, XN_FLAG_ONELINE); + else + i2a_ASN1_STRING(bio, key, V_ASN1_OCTET_STRING); + + return true; } -void ocsp_add_cert_id(OCSP_CERTID *cert_id, val_list* vl, BIO* bio) +bool ocsp_add_cert_id(const OCSP_CERTID* cert_id, val_list* vl, BIO* bio) { + ASN1_OBJECT* hash_alg = nullptr; + ASN1_OCTET_STRING* issuer_name_hash = nullptr; + ASN1_OCTET_STRING* issuer_key_hash = nullptr; + ASN1_INTEGER* serial_number = nullptr; + + auto res = OCSP_id_get0_info(&issuer_name_hash, &hash_alg, + &issuer_key_hash, &serial_number, + const_cast(cert_id)); + + if ( ! res ) + { + reporter->Weird("OpenSSL failed to get OCSP_CERTID info"); + vl->append(new StringVal("")); + vl->append(new StringVal("")); + vl->append(new StringVal("")); + vl->append(new StringVal("")); + return false; + } + char buf[OCSP_STRING_BUF_SIZE]; memset(buf, 0, sizeof(buf)); - i2a_ASN1_OBJECT(bio, cert_id->hashAlgorithm->algorithm); + i2a_ASN1_OBJECT(bio, hash_alg); int len = BIO_read(bio, buf, sizeof(buf)); vl->append(new StringVal(len, buf)); BIO_reset(bio); - i2a_ASN1_STRING(bio, cert_id->issuerNameHash, V_ASN1_OCTET_STRING); + i2a_ASN1_STRING(bio, issuer_name_hash, V_ASN1_OCTET_STRING); len = BIO_read(bio, buf, sizeof(buf)); vl->append(new StringVal(len, buf)); BIO_reset(bio); - i2a_ASN1_STRING(bio, cert_id->issuerKeyHash, V_ASN1_OCTET_STRING); + i2a_ASN1_STRING(bio, issuer_key_hash, V_ASN1_OCTET_STRING); len = BIO_read(bio, buf, sizeof(buf)); vl->append(new StringVal(len, buf)); BIO_reset(bio); - i2a_ASN1_INTEGER(bio, cert_id->serialNumber); + i2a_ASN1_INTEGER(bio, serial_number); len = BIO_read(bio, buf, sizeof(buf)); vl->append(new StringVal(len, buf)); BIO_reset(bio); + + return true; } file_analysis::Analyzer* OCSP::InstantiateRequest(RecordVal* args, File* file) @@ -124,6 +170,7 @@ bool file_analysis::OCSP::EndOfFile() else { OCSP_RESPONSE *resp = d2i_OCSP_RESPONSE(NULL, &ocsp_char, ocsp_data.size()); + if (!resp) { reporter->Weird(fmt("OPENSSL Could not parse OCSP response (fuid %s)", GetFile()->GetID().c_str())); @@ -138,22 +185,262 @@ bool file_analysis::OCSP::EndOfFile() return true; } -void file_analysis::OCSP::ParseRequest(OCSP_REQUEST *req, const char* fid) +#if ( OPENSSL_VERSION_NUMBER >= 0x10100000L ) +// Re-encode and then parse out ASN1 structures to get at what we need... +/*- BasicOCSPResponse ::= SEQUENCE { + * tbsResponseData ResponseData, + * signatureAlgorithm AlgorithmIdentifier, + * signature BIT STRING, + * certs [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL } +typedef struct ocsp_basic_response_st { + OCSP_RESPDATA *tbsResponseData; + X509_ALGOR *signatureAlgorithm; + ASN1_BIT_STRING *signature; + STACK_OF(X509) *certs; +} OCSP_BASICRESP; +*/ +static StringVal* parse_basic_resp_sig_alg(OCSP_BASICRESP* basic_resp, + BIO* bio, char* buf, size_t buf_len) { - OCSP_REQINFO *inf = req->tbsRequest; + int der_basic_resp_len = 0; + unsigned char* der_basic_resp_dat = nullptr; + der_basic_resp_len = i2d_OCSP_BASICRESP(basic_resp, &der_basic_resp_dat); + + if ( der_basic_resp_len <= 0 ) + return new StringVal(""); + + const unsigned char* const_der_basic_resp_dat = der_basic_resp_dat; + + auto bseq = d2i_ASN1_SEQUENCE_ANY(nullptr, &const_der_basic_resp_dat, + der_basic_resp_len); + + if ( ! bseq ) + { + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + if ( sk_ASN1_TYPE_num(bseq) < 3 ) + { + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + auto constexpr sig_alg_idx = 1u; + auto aseq_type = sk_ASN1_TYPE_value(bseq, sig_alg_idx); + + if ( ASN1_TYPE_get(aseq_type) != V_ASN1_SEQUENCE ) + { + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + auto aseq_str = aseq_type->value.asn1_string; + auto aseq_len = ASN1_STRING_length(aseq_str); + auto aseq_dat = ASN1_STRING_get0_data(aseq_str); + + auto aseq = d2i_ASN1_SEQUENCE_ANY(nullptr, &aseq_dat, aseq_len); + + if ( ! aseq ) + { + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + if ( sk_ASN1_TYPE_num(aseq) < 1 ) + { + sk_ASN1_TYPE_free(aseq); + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + auto constexpr alg_obj_idx = 0u; + auto alg_obj_type = sk_ASN1_TYPE_value(aseq, alg_obj_idx); + + if ( ASN1_TYPE_get(alg_obj_type) != V_ASN1_OBJECT ) + { + sk_ASN1_TYPE_free(aseq); + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + auto alg_obj = alg_obj_type->value.object; + i2a_ASN1_OBJECT(bio, alg_obj); + auto alg_len = BIO_read(bio, buf, buf_len); + auto rval = new StringVal(alg_len, buf); + BIO_reset(bio); + + sk_ASN1_TYPE_free(aseq); + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return rval; + } + +static Val* parse_basic_resp_data_version(OCSP_BASICRESP* basic_resp) + { + int der_basic_resp_len = 0; + unsigned char* der_basic_resp_dat = nullptr; + + der_basic_resp_len = i2d_OCSP_BASICRESP(basic_resp, &der_basic_resp_dat); + + if ( der_basic_resp_len <= 0 ) + return new Val(-1, TYPE_COUNT); + + const unsigned char* const_der_basic_resp_dat = der_basic_resp_dat; + + auto bseq = d2i_ASN1_SEQUENCE_ANY(nullptr, &const_der_basic_resp_dat, + der_basic_resp_len); + + if ( ! bseq ) + { + OPENSSL_free(der_basic_resp_dat); + return new Val(-1, TYPE_COUNT); + } + + if ( sk_ASN1_TYPE_num(bseq) < 3 ) + { + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new Val(-1, TYPE_COUNT); + } + + auto constexpr resp_data_idx = 0u; + auto dseq_type = sk_ASN1_TYPE_value(bseq, resp_data_idx); + + if ( ASN1_TYPE_get(dseq_type) != V_ASN1_SEQUENCE ) + { + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new Val(-1, TYPE_COUNT); + } + + auto dseq_str = dseq_type->value.asn1_string; + auto dseq_len = ASN1_STRING_length(dseq_str); + auto dseq_dat = ASN1_STRING_get0_data(dseq_str); + + auto dseq = d2i_ASN1_SEQUENCE_ANY(nullptr, &dseq_dat, dseq_len); + + if ( ! dseq ) + { + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + + if ( sk_ASN1_TYPE_num(dseq) < 1 ) + { + sk_ASN1_TYPE_free(dseq); + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new StringVal(""); + } + +/*- ResponseData ::= SEQUENCE { + * version [0] EXPLICIT Version DEFAULT v1, + * responderID ResponderID, + * producedAt GeneralizedTime, + * responses SEQUENCE OF SingleResponse, + * responseExtensions [1] EXPLICIT Extensions OPTIONAL } + */ + + auto constexpr version_idx = 0u; + auto version_type = sk_ASN1_TYPE_value(dseq, version_idx); + + if ( ASN1_TYPE_get(version_type) != V_ASN1_INTEGER ) + { + sk_ASN1_TYPE_free(dseq); + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + // Not present, use default value. + return new Val(0, TYPE_COUNT); + } + + uint64_t asn1_int = ASN1_INTEGER_get(version_type->value.integer); + sk_ASN1_TYPE_free(dseq); + sk_ASN1_TYPE_free(bseq); + OPENSSL_free(der_basic_resp_dat); + return new Val(asn1_int, TYPE_COUNT); + } + +static uint64 parse_request_version(OCSP_REQUEST* req) + { + int der_req_len = 0; + unsigned char* der_req_dat = nullptr; + der_req_len = i2d_OCSP_REQUEST(req, &der_req_dat); + const unsigned char* const_der_req_dat = der_req_dat; + + if ( ! der_req_dat ) + return -1; + + auto rseq = d2i_ASN1_SEQUENCE_ANY(nullptr, &const_der_req_dat, + der_req_len); + + if ( ! rseq ) + { + OPENSSL_free(der_req_dat); + return -1; + } + + if ( sk_ASN1_TYPE_num(rseq) < 1 ) + { + sk_ASN1_TYPE_free(rseq); + OPENSSL_free(der_req_dat); + return -1; + } + + auto constexpr version_idx = 0u; + auto version_type = sk_ASN1_TYPE_value(rseq, version_idx); + + if ( ASN1_TYPE_get(version_type) != V_ASN1_INTEGER ) + { + sk_ASN1_TYPE_free(rseq); + OPENSSL_free(der_req_dat); + // Not present, use default value. + return 0; + } + + uint64_t asn1_int = ASN1_INTEGER_get(version_type->value.integer); + sk_ASN1_TYPE_free(rseq); + OPENSSL_free(der_req_dat); + return asn1_int; + } +#endif + +void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req, const char* fid) + { char buf[OCSP_STRING_BUF_SIZE]; // we need a buffer for some of the openssl functions memset(buf, 0, sizeof(buf)); // build up our response as we go along... val_list* vl = new val_list(); vl->append(GetFile()->GetVal()->Ref()); - vl->append(new Val((uint64)ASN1_INTEGER_get(inf->version), TYPE_COUNT)); + + uint64 version = 0; + GENERAL_NAME* general_name = nullptr; + +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + if ( req->tbsRequest->version ) + version = (uint64)ASN1_INTEGER_get(req->tbsRequest->version); + + general_name = req->tbsRequest->requestorName; +#else + version = parse_request_version(req); + // TODO: try to parse out general name ? +#endif + + vl->append(new Val(version, TYPE_COUNT)); + BIO *bio = BIO_new(BIO_s_mem()); - if (inf->requestorName != NULL) + if ( general_name ) { - GENERAL_NAME_print(bio, inf->requestorName); + GENERAL_NAME_print(bio, general_name); int len = BIO_read(bio, buf, sizeof(buf)); vl->append(new StringVal(len, buf)); BIO_reset(bio); @@ -182,10 +469,12 @@ void file_analysis::OCSP::ParseRequest(OCSP_REQUEST *req, const char* fid) void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) { OCSP_RESPONSE *resp = resp_val->GetResp(); - OCSP_RESPBYTES *resp_bytes = resp->responseBytes; + //OCSP_RESPBYTES *resp_bytes = resp->responseBytes; OCSP_BASICRESP *basic_resp = nullptr; OCSP_RESPDATA *resp_data = nullptr; OCSP_RESPID *resp_id = nullptr; + const ASN1_GENERALIZEDTIME* produced_at = nullptr; + const STACK_OF(X509)* certs = nullptr; int resp_count, num_ext = 0; VectorVal *certs_vector = nullptr; @@ -203,11 +492,11 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) mgr.QueueEvent(ocsp_response_status, vl); vl = nullptr; - if (!resp_bytes) - { - Unref(status_val); - return; - } + //if (!resp_bytes) + // { + // Unref(status_val); + // return; + // } BIO *bio = BIO_new(BIO_s_mem()); //i2a_ASN1_OBJECT(bio, resp_bytes->responseType); @@ -219,31 +508,53 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) if ( !basic_resp ) goto clean_up; +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) resp_data = basic_resp->tbsResponseData; if ( !resp_data ) goto clean_up; +#endif vl = new val_list(); vl->append(GetFile()->GetVal()->Ref()); vl->append(resp_val->Ref()); vl->append(status_val); + +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) vl->append(new Val((uint64)ASN1_INTEGER_get(resp_data->version), TYPE_COUNT)); +#else + vl->append(parse_basic_resp_data_version(basic_resp)); +#endif // responderID - resp_id = resp_data->responderId; - OCSP_RESPID_bio(resp_id, bio); - len = BIO_read(bio, buf, sizeof(buf)); - vl->append(new StringVal(len, buf)); - BIO_reset(bio); + if ( OCSP_RESPID_bio(basic_resp, bio) ) + { + len = BIO_read(bio, buf, sizeof(buf)); + vl->append(new StringVal(len, buf)); + BIO_reset(bio); + } + else + { + reporter->Weird("OpenSSL failed to get OCSP responder id"); + vl->append(new StringVal("")); + } // producedAt - vl->append(new Val(GetTimeFromAsn1(resp_data->producedAt, fid, reporter), TYPE_TIME)); +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + produced_at = resp_data->producedAt; +#else + produced_at = OCSP_resp_get0_produced_at(basic_resp); +#endif + + vl->append(new Val(GetTimeFromAsn1(produced_at, fid, reporter), TYPE_TIME)); // responses - resp_count = sk_OCSP_SINGLERESP_num(resp_data->responses); + + resp_count = OCSP_resp_count(basic_resp); + for ( int i=0; iresponses, i); + OCSP_SINGLERESP* single_resp = OCSP_resp_get0(basic_resp, i); + if ( !single_resp ) continue; @@ -251,24 +562,41 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) rvl->append(GetFile()->GetVal()->Ref()); // cert id - OCSP_CERTID *cert_id = single_resp->certId; + const OCSP_CERTID* cert_id = nullptr; + +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + cert_id = single_resp->certId; +#else + cert_id = OCSP_SINGLERESP_get0_id(single_resp); +#endif + ocsp_add_cert_id(cert_id, rvl, bio); BIO_reset(bio); // certStatus - OCSP_CERTSTATUS *cert_status = single_resp->certStatus; - const char* cert_status_str = OCSP_cert_status_str(cert_status->type); + int status = V_OCSP_CERTSTATUS_UNKNOWN; + int reason = OCSP_REVOKED_STATUS_NOSTATUS; + ASN1_GENERALIZEDTIME* revoke_time = nullptr; + ASN1_GENERALIZEDTIME* this_update = nullptr; + ASN1_GENERALIZEDTIME* next_update = nullptr; + + if ( ! OCSP_resp_find_status(basic_resp, + const_cast(cert_id), + &status, &reason, &revoke_time, + &this_update, &next_update) ) + reporter->Weird("OpenSSL failed to find status of OCSP response"); + + const char* cert_status_str = OCSP_cert_status_str(status); rvl->append(new StringVal(strlen(cert_status_str), cert_status_str)); // revocation time and reason if revoked - if ( cert_status->type == V_OCSP_CERTSTATUS_REVOKED ) + if ( status == V_OCSP_CERTSTATUS_REVOKED ) { - OCSP_REVOKEDINFO *revoked_info = cert_status->value.revoked; - rvl->append(new Val(GetTimeFromAsn1(revoked_info->revocationTime, fid, reporter), TYPE_TIME)); + rvl->append(new Val(GetTimeFromAsn1(revoke_time, fid, reporter), TYPE_TIME)); - if ( revoked_info->revocationReason ) + if ( reason != OCSP_REVOKED_STATUS_NOSTATUS ) { - const char* revoke_reason = OCSP_crl_reason_str(ASN1_ENUMERATED_get(revoked_info->revocationReason)); + const char* revoke_reason = OCSP_crl_reason_str(reason); rvl->append(new StringVal(strlen(revoke_reason), revoke_reason)); } else @@ -280,9 +608,13 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) rvl->append(new StringVal(0, "")); } - rvl->append(new Val(GetTimeFromAsn1(single_resp->thisUpdate, fid, reporter), TYPE_TIME)); - if ( single_resp->nextUpdate ) - rvl->append(new Val(GetTimeFromAsn1(single_resp->nextUpdate, fid, reporter), TYPE_TIME)); + if ( this_update ) + rvl->append(new Val(GetTimeFromAsn1(this_update, fid, reporter), TYPE_TIME)); + else + rvl->append(new Val(0, TYPE_TIME)); + + if ( next_update ) + rvl->append(new Val(GetTimeFromAsn1(next_update, fid, reporter), TYPE_TIME)); else rvl->append(new Val(0, TYPE_TIME)); @@ -299,10 +631,14 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) } } +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) i2a_ASN1_OBJECT(bio, basic_resp->signatureAlgorithm->algorithm); len = BIO_read(bio, buf, sizeof(buf)); vl->append(new StringVal(len, buf)); BIO_reset(bio); +#else + vl->append(parse_basic_resp_sig_alg(basic_resp, bio, buf, sizeof(buf))); +#endif //i2a_ASN1_OBJECT(bio, basic_resp->signature); //len = BIO_read(bio, buf, sizeof(buf)); @@ -311,13 +647,20 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) certs_vector = new VectorVal(internal_type("x509_opaque_vector")->AsVectorType()); vl->append(certs_vector); - if ( basic_resp->certs ) + +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + certs = basic_resp->certs; +#else + certs = OCSP_resp_get0_certs(basic_resp); +#endif + + if ( certs ) { - int num_certs = sk_X509_num(basic_resp->certs); + int num_certs = sk_X509_num(certs); for ( int i=0; icerts, i)); - //::X509 *this_cert = X509_dup(sk_X509_value(basic_resp->certs, i)); + ::X509 *this_cert = X509_dup(helper_sk_X509_value(certs, i)); + //::X509 *this_cert = X509_dup(sk_X509_value(certs, i)); if (this_cert) certs_vector->Assign(i, new file_analysis::X509Val(this_cert)); else diff --git a/src/file_analysis/analyzer/x509/OCSP.h b/src/file_analysis/analyzer/x509/OCSP.h index ca8c2c563e..75caf3120a 100644 --- a/src/file_analysis/analyzer/x509/OCSP.h +++ b/src/file_analysis/analyzer/x509/OCSP.h @@ -29,8 +29,8 @@ protected: OCSP(RecordVal* args, File* file, bool request); private: - void ParseResponse(OCSP_RESPVal *, const char* fid = 0); - void ParseRequest(OCSP_REQUEST *, const char* fid = 0); + void ParseResponse(OCSP_RESPVal*, const char* fid = 0); + void ParseRequest(OCSP_REQUEST*, const char* fid = 0); void ParseExtensionsSpecific(X509_EXTENSION* ex, bool, ASN1_OBJECT*, const char*) override; std::string ocsp_data; diff --git a/src/file_analysis/analyzer/x509/X509.cc b/src/file_analysis/analyzer/x509/X509.cc index 0426b59923..7571915207 100644 --- a/src/file_analysis/analyzer/x509/X509.cc +++ b/src/file_analysis/analyzer/x509/X509.cc @@ -290,7 +290,11 @@ void file_analysis::X509::ParseSAN(X509_EXTENSION* ext) continue; } +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) const char* name = (const char*) ASN1_STRING_data(gen->d.ia5); +#else + const char* name = (const char*) ASN1_STRING_get0_data(gen->d.ia5); +#endif StringVal* bs = new StringVal(name); switch ( gen->type ) diff --git a/src/file_analysis/analyzer/x509/functions.bif b/src/file_analysis/analyzer/x509/functions.bif index 18f56758f5..3622e0d13a 100644 --- a/src/file_analysis/analyzer/x509/functions.bif +++ b/src/file_analysis/analyzer/x509/functions.bif @@ -109,21 +109,36 @@ STACK_OF(X509)* x509_get_untrusted_stack(VectorVal* certs_vec) // We need this function to be able to identify the signer certificate of an // OCSP request out of a list of possible certificates. -X509* x509_get_ocsp_signer(STACK_OF(X509) *certs, OCSP_RESPID *rid) +X509* x509_get_ocsp_signer(const STACK_OF(X509)* certs, + OCSP_BASICRESP* basic_resp) { - // We support two lookup types - either by response id or by key. - if ( rid->type == V_OCSP_RESPID_NAME ) - return X509_find_by_subject(certs, rid->value.byName); + const ASN1_OCTET_STRING* key = nullptr; + const X509_NAME* name = nullptr; - // There only should be name and type - but let's be sure... - if ( rid->type != V_OCSP_RESPID_KEY ) +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + OCSP_RESPID* resp_id = basic_resp->tbsResponseData->responderId; + + if ( resp_id->type == V_OCSP_RESPID_NAME ) + name = resp_id->value.byName; + else if ( resp_id->type == V_OCSP_RESPID_KEY ) + key = resp_id->value.byKey; + else return 0; +#else + if ( ! OCSP_resp_get0_id(basic_resp, &key, &name) ) + return 0; +#endif + + if ( name ) + return X509_find_by_subject(const_cast(certs), + const_cast(name)); // Just like OpenSSL, we just support SHA-1 lookups and bail out otherwhise. - if ( rid->value.byKey->length != SHA_DIGEST_LENGTH ) + if ( key->length != SHA_DIGEST_LENGTH ) return 0; - unsigned char* key_hash = rid->value.byKey->data; + unsigned char* key_hash = key->data; + for ( int i = 0; i < sk_X509_num(certs); ++i ) { unsigned char digest[SHA_DIGEST_LENGTH]; @@ -328,15 +343,19 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c // Because we actually want to be able to give nice error messages that show why we were // not able to verify the OCSP response - do our own verification logic first. - signer = x509_get_ocsp_signer(basic->certs, basic->tbsResponseData->responderId); +#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) + signer = x509_get_ocsp_signer(basic->certs, basic); +#else + signer = x509_get_ocsp_signer(OCSP_resp_get0_certs(basic), basic); +#endif /* Do this perhaps - OpenSSL also cannot do it, so I do not really feel bad about it. Needs a different lookup because the root store is no stack of X509 certs - if ( !s igner ) + if ( ! signer ) // if we did not find it in the certificates that were sent, search in the root store - signer = x509_get_ocsp_signer(ocsp_certs, basic->tbsResponseData->responderId); + signer = x509_get_ocsp_signer(ocsp_certs, basic); */ if ( ! signer ) From 2e0edd741608ba73c5f0e6aca9b2bc4a2277ad17 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 29 Jun 2018 16:01:23 -0500 Subject: [PATCH 2/4] Adjust x509 unit tests to work around OpenSSL 1.0 vs. 1.1 differences --- CMakeLists.txt | 4 ++ .../{.stdout => stdout-openssl-1.0} | 0 .../bifs.x509_verify/stdout-openssl-1.1 | 6 ++ testing/btest/bifs/x509_verify.bro | 11 +++- .../scripts/base/protocols/rdp/rdp-x509.bro | 2 +- testing/scripts/diff-remove-x509-key-info | 55 +++++++++++++++++++ 6 files changed, 76 insertions(+), 2 deletions(-) rename testing/btest/Baseline/bifs.x509_verify/{.stdout => stdout-openssl-1.0} (100%) create mode 100644 testing/btest/Baseline/bifs.x509_verify/stdout-openssl-1.1 create mode 100755 testing/scripts/diff-remove-x509-key-info diff --git a/CMakeLists.txt b/CMakeLists.txt index d0ea236330..aad1163ed5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -220,6 +220,10 @@ include(CheckNameserCompat) include(GetArchitecture) include(RequireCXX11) +if ( (OPENSSL_VERSION VERSION_EQUAL "1.1.0") OR (OPENSSL_VERSION VERSION_GREATER "1.1.0") ) + set(BRO_HAVE_OPENSSL_1_1 true CACHE INTERNAL "" FORCE) +endif() + # Tell the plugin code that we're building as part of the main tree. set(BRO_PLUGIN_INTERNAL_BUILD true CACHE INTERNAL "" FORCE) diff --git a/testing/btest/Baseline/bifs.x509_verify/.stdout b/testing/btest/Baseline/bifs.x509_verify/stdout-openssl-1.0 similarity index 100% rename from testing/btest/Baseline/bifs.x509_verify/.stdout rename to testing/btest/Baseline/bifs.x509_verify/stdout-openssl-1.0 diff --git a/testing/btest/Baseline/bifs.x509_verify/stdout-openssl-1.1 b/testing/btest/Baseline/bifs.x509_verify/stdout-openssl-1.1 new file mode 100644 index 0000000000..29660eade5 --- /dev/null +++ b/testing/btest/Baseline/bifs.x509_verify/stdout-openssl-1.1 @@ -0,0 +1,6 @@ +Validation result: certificate has expired +Validation result: ok +Resulting chain: +Fingerprint: 70829f77ff4b6e908324a3f4e1940fce6c489098, Subject: CN=www.tobu-estate.com,OU=Terms of use at www.verisign.com/rpa (c)05,O=TOBU RAILWAY Co.\,Ltd.,L=Sumida-ku,ST=Tokyo,C=JP +Fingerprint: 5deb8f339e264c19f6686f5f8f32b54a4c46b476, Subject: CN=VeriSign Class 3 Secure Server CA - G3,OU=Terms of use at https://www.verisign.com/rpa (c)10,OU=VeriSign Trust Network,O=VeriSign\, Inc.,C=US +Fingerprint: 4eb6d578499b1ccf5f581ead56be3d9b6744a5e5, Subject: CN=VeriSign Class 3 Public Primary Certification Authority - G5,OU=(c) 2006 VeriSign\, Inc. - For authorized use only,OU=VeriSign Trust Network,O=VeriSign\, Inc.,C=US diff --git a/testing/btest/bifs/x509_verify.bro b/testing/btest/bifs/x509_verify.bro index 59939180c8..2afc735172 100644 --- a/testing/btest/bifs/x509_verify.bro +++ b/testing/btest/bifs/x509_verify.bro @@ -1,5 +1,14 @@ # @TEST-EXEC: bro -r $TRACES/tls/tls-expired-cert.trace %INPUT -# @TEST-EXEC: btest-diff .stdout + +# This is a hack: the results of OpenSSL 1.1's vs 1.0's +# X509_verify_cert() -> X509_STORE_CTX_get1_chain() calls +# differ. Word seems to be that OpenSSL 1.1's cert-chain-building +# code is significantly different/rewritten so may be the reason... + +# @TEST-EXEC: cp .stdout stdout-openssl-1.0 +# @TEST-EXEC: cp .stdout stdout-openssl-1.1 + +# @TEST-EXEC: grep -q "BRO_HAVE_OPENSSL_1_1" $BUILD/CMakeCache.txt && btest-diff stdout-openssl-1.1 || btest-diff stdout-openssl-1.0 redef SSL::root_certs += { ["OU=Class 3 Public Primary Certification Authority,O=VeriSign\, Inc.,C=US"] = "\x30\x82\x02\x3C\x30\x82\x01\xA5\x02\x10\x70\xBA\xE4\x1D\x10\xD9\x29\x34\xB6\x38\xCA\x7B\x03\xCC\xBA\xBF\x30\x0D\x06\x09\x2A\x86\x48\x86\xF7\x0D\x01\x01\x02\x05\x00\x30\x5F\x31\x0B\x30\x09\x06\x03\x55\x04\x06\x13\x02\x55\x53\x31\x17\x30\x15\x06\x03\x55\x04\x0A\x13\x0E\x56\x65\x72\x69\x53\x69\x67\x6E\x2C\x20\x49\x6E\x63\x2E\x31\x37\x30\x35\x06\x03\x55\x04\x0B\x13\x2E\x43\x6C\x61\x73\x73\x20\x33\x20\x50\x75\x62\x6C\x69\x63\x20\x50\x72\x69\x6D\x61\x72\x79\x20\x43\x65\x72\x74\x69\x66\x69\x63\x61\x74\x69\x6F\x6E\x20\x41\x75\x74\x68\x6F\x72\x69\x74\x79\x30\x1E\x17\x0D\x39\x36\x30\x31\x32\x39\x30\x30\x30\x30\x30\x30\x5A\x17\x0D\x32\x38\x30\x38\x30\x31\x32\x33\x35\x39\x35\x39\x5A\x30\x5F\x31\x0B\x30\x09\x06\x03\x55\x04\x06\x13\x02\x55\x53\x31\x17\x30\x15\x06\x03\x55\x04\x0A\x13\x0E\x56\x65\x72\x69\x53\x69\x67\x6E\x2C\x20\x49\x6E\x63\x2E\x31\x37\x30\x35\x06\x03\x55\x04\x0B\x13\x2E\x43\x6C\x61\x73\x73\x20\x33\x20\x50\x75\x62\x6C\x69\x63\x20\x50\x72\x69\x6D\x61\x72\x79\x20\x43\x65\x72\x74\x69\x66\x69\x63\x61\x74\x69\x6F\x6E\x20\x41\x75\x74\x68\x6F\x72\x69\x74\x79\x30\x81\x9F\x30\x0D\x06\x09\x2A\x86\x48\x86\xF7\x0D\x01\x01\x01\x05\x00\x03\x81\x8D\x00\x30\x81\x89\x02\x81\x81\x00\xC9\x5C\x59\x9E\xF2\x1B\x8A\x01\x14\xB4\x10\xDF\x04\x40\xDB\xE3\x57\xAF\x6A\x45\x40\x8F\x84\x0C\x0B\xD1\x33\xD9\xD9\x11\xCF\xEE\x02\x58\x1F\x25\xF7\x2A\xA8\x44\x05\xAA\xEC\x03\x1F\x78\x7F\x9E\x93\xB9\x9A\x00\xAA\x23\x7D\xD6\xAC\x85\xA2\x63\x45\xC7\x72\x27\xCC\xF4\x4C\xC6\x75\x71\xD2\x39\xEF\x4F\x42\xF0\x75\xDF\x0A\x90\xC6\x8E\x20\x6F\x98\x0F\xF8\xAC\x23\x5F\x70\x29\x36\xA4\xC9\x86\xE7\xB1\x9A\x20\xCB\x53\xA5\x85\xE7\x3D\xBE\x7D\x9A\xFE\x24\x45\x33\xDC\x76\x15\xED\x0F\xA2\x71\x64\x4C\x65\x2E\x81\x68\x45\xA7\x02\x03\x01\x00\x01\x30\x0D\x06\x09\x2A\x86\x48\x86\xF7\x0D\x01\x01\x02\x05\x00\x03\x81\x81\x00\xBB\x4C\x12\x2B\xCF\x2C\x26\x00\x4F\x14\x13\xDD\xA6\xFB\xFC\x0A\x11\x84\x8C\xF3\x28\x1C\x67\x92\x2F\x7C\xB6\xC5\xFA\xDF\xF0\xE8\x95\xBC\x1D\x8F\x6C\x2C\xA8\x51\xCC\x73\xD8\xA4\xC0\x53\xF0\x4E\xD6\x26\xC0\x76\x01\x57\x81\x92\x5E\x21\xF1\xD1\xB1\xFF\xE7\xD0\x21\x58\xCD\x69\x17\xE3\x44\x1C\x9C\x19\x44\x39\x89\x5C\xDC\x9C\x00\x0F\x56\x8D\x02\x99\xED\xA2\x90\x45\x4C\xE4\xBB\x10\xA4\x3D\xF0\x32\x03\x0E\xF1\xCE\xF8\xE8\xC9\x51\x8C\xE6\x62\x9F\xE6\x9F\xC0\x7D\xB7\x72\x9C\xC9\x36\x3A\x6B\x9F\x4E\xA8\xFF\x64\x0D\x64" diff --git a/testing/btest/scripts/base/protocols/rdp/rdp-x509.bro b/testing/btest/scripts/base/protocols/rdp/rdp-x509.bro index ae1eb8b542..2fed0d7d19 100644 --- a/testing/btest/scripts/base/protocols/rdp/rdp-x509.bro +++ b/testing/btest/scripts/base/protocols/rdp/rdp-x509.bro @@ -1,5 +1,5 @@ # @TEST-EXEC: bro -r $TRACES/rdp/rdp-x509.pcap %INPUT # @TEST-EXEC: btest-diff rdp.log -# @TEST-EXEC: btest-diff x509.log +# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-timestamps | $SCRIPTS/diff-remove-x509-key-info" btest-diff x509.log @load base/protocols/rdp diff --git a/testing/scripts/diff-remove-x509-key-info b/testing/scripts/diff-remove-x509-key-info new file mode 100755 index 0000000000..85404fb30d --- /dev/null +++ b/testing/scripts/diff-remove-x509-key-info @@ -0,0 +1,55 @@ +#! /usr/bin/env bash +# +# A diff canonifier that removes all X.509 public key information +# which, in the specific case of the RDP protocol's misuse of +# md5WithRSAEncryption, seems that OpenSSL 1.0 is able to manually +# workaround by setting to rsaEncryption, but OpenSSL 1.1 still fails +# to extract the key, so the corresponding fields are always removed here. + +awk ' +BEGIN { FS="\t"; OFS="\t"; key_type_col = -1; key_length_col = -1; exponent_col = -1; curve_col = -1 } + +/^#/ { + if ( $1 == "#fields" ) + { + for ( i = 2; i <= NF; ++i ) + { + if ( $i == "certificate.key_type" ) + key_type_col = i-1; + if ( $i == "certificate.key_length" ) + key_length_col = i-1; + if ( $i == "certificate.exponent" ) + exponent_col = i-1; + if ( $i == "certificate.curve" ) + curve_col = i-1; + } + } + + print; + next; +} + +key_type_col > 0 { + # Mark it regardless of whether it is set. + $key_type_col = "x"; +} + +key_length_col > 0 { + # Mark it regardless of whether it is set. + $key_length_col = "x"; +} + +exponent_col > 0 { + # Mark it regardless of whether it is set. + $exponent_col = "x"; +} + +curve_col > 0 { + # Mark it regardless of whether it is set. + $curve_col = "x"; +} + +{ + print; +} +' From bb55f828096e5690e0b97079a53f0e77b171d480 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 29 Jun 2018 16:15:34 -0500 Subject: [PATCH 3/4] Remove requestorName parameter of ocsp_request event This field isn't publicly available via the OpenSSL 1.1 API, not used in the base scripts, and has no example in the test suit, so removing it is simpler than trying to support manually parsing it out of the raw data. --- NEWS | 2 ++ src/file_analysis/analyzer/x509/OCSP.cc | 13 ------------- src/file_analysis/analyzer/x509/ocsp_events.bif | 5 +---- .../scripts/base/protocols/ssl/ocsp-http-get.test | 4 ++-- .../base/protocols/ssl/ocsp-request-only.test | 4 ++-- .../base/protocols/ssl/ocsp-request-response.test | 4 ++-- .../base/protocols/ssl/ocsp-response-only.test | 4 ++-- .../scripts/base/protocols/ssl/ocsp-revoked.test | 4 ++-- 8 files changed, 13 insertions(+), 27 deletions(-) diff --git a/NEWS b/NEWS index a4f233a116..bbfb567e91 100644 --- a/NEWS +++ b/NEWS @@ -344,6 +344,8 @@ Removed Functionality available (though Broker should be able to handle IPv6 automatically). +- The "ocsp_request" event no longer has "requestorName" parameter. + Deprecated Functionality ------------------------ diff --git a/src/file_analysis/analyzer/x509/OCSP.cc b/src/file_analysis/analyzer/x509/OCSP.cc index e22440aab8..ccb6762739 100644 --- a/src/file_analysis/analyzer/x509/OCSP.cc +++ b/src/file_analysis/analyzer/x509/OCSP.cc @@ -422,13 +422,10 @@ void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req, const char* fid) vl->append(GetFile()->GetVal()->Ref()); uint64 version = 0; - GENERAL_NAME* general_name = nullptr; #if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) if ( req->tbsRequest->version ) version = (uint64)ASN1_INTEGER_get(req->tbsRequest->version); - - general_name = req->tbsRequest->requestorName; #else version = parse_request_version(req); // TODO: try to parse out general name ? @@ -438,16 +435,6 @@ void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req, const char* fid) BIO *bio = BIO_new(BIO_s_mem()); - if ( general_name ) - { - GENERAL_NAME_print(bio, general_name); - int len = BIO_read(bio, buf, sizeof(buf)); - vl->append(new StringVal(len, buf)); - BIO_reset(bio); - } - else - vl->append(new StringVal(0, "")); - mgr.QueueEvent(ocsp_request, vl); int req_count = OCSP_request_onereq_count(req); diff --git a/src/file_analysis/analyzer/x509/ocsp_events.bif b/src/file_analysis/analyzer/x509/ocsp_events.bif index 4e0ed6e9dc..f49208d238 100644 --- a/src/file_analysis/analyzer/x509/ocsp_events.bif +++ b/src/file_analysis/analyzer/x509/ocsp_events.bif @@ -7,13 +7,10 @@ ## ## req: version: the version of the OCSP request. Typically 0 (Version 1). ## -## requestorName: name of the OCSP requestor. This attribute is optional; if -## it is not set, an empty string is returned here. -## ## .. bro:see:: ocsp_request_certificate ocsp_response_status ## ocsp_response_bytes ocsp_response_certificate ocsp_extension ## x509_ocsp_ext_signed_certificate_timestamp -event ocsp_request%(f: fa_file, version: count, requestorName: string%); +event ocsp_request%(f: fa_file, version: count%); ## Event that is raised when encountering an OCSP request for a certificate, ## e.g. in an HTTP connection. See :rfc:`6960` for more details. diff --git a/testing/btest/scripts/base/protocols/ssl/ocsp-http-get.test b/testing/btest/scripts/base/protocols/ssl/ocsp-http-get.test index ff48772b6a..c8c8acc589 100644 --- a/testing/btest/scripts/base/protocols/ssl/ocsp-http-get.test +++ b/testing/btest/scripts/base/protocols/ssl/ocsp-http-get.test @@ -17,9 +17,9 @@ event ocsp_extension(f: fa_file, ext: X509::Extension, global_resp: bool) print "extension: ", ext, global_resp; } -event ocsp_request(f: fa_file, version: count, requestorName: string) +event ocsp_request(f: fa_file, version: count) { - print "request", version, requestorName; + print "request", version, ""; } event ocsp_request_certificate(f: fa_file, hashAlgorithm: string, issuerNameHash: string, issuerKeyHash: string, serialNumber: string) diff --git a/testing/btest/scripts/base/protocols/ssl/ocsp-request-only.test b/testing/btest/scripts/base/protocols/ssl/ocsp-request-only.test index 0176716553..05483717b0 100644 --- a/testing/btest/scripts/base/protocols/ssl/ocsp-request-only.test +++ b/testing/btest/scripts/base/protocols/ssl/ocsp-request-only.test @@ -16,9 +16,9 @@ event ocsp_extension(f: fa_file, ext: X509::Extension, global_resp: bool) print "extension: ", ext, global_resp; } -event ocsp_request(f: fa_file, version: count, requestorName: string) +event ocsp_request(f: fa_file, version: count) { - print "request", version, requestorName; + print "request", version, ""; } event ocsp_request_certificate(f: fa_file, hashAlgorithm: string, issuerNameHash: string, issuerKeyHash: string, serialNumber: string) diff --git a/testing/btest/scripts/base/protocols/ssl/ocsp-request-response.test b/testing/btest/scripts/base/protocols/ssl/ocsp-request-response.test index 3adfab9aa2..b95203dfd8 100644 --- a/testing/btest/scripts/base/protocols/ssl/ocsp-request-response.test +++ b/testing/btest/scripts/base/protocols/ssl/ocsp-request-response.test @@ -17,9 +17,9 @@ event ocsp_extension(f: fa_file, ext: X509::Extension, global_resp: bool) print "extension: ", ext, global_resp; } -event ocsp_request(f: fa_file, version: count, requestorName: string) +event ocsp_request(f: fa_file, version: count) { - print "request", version, requestorName; + print "request", version, ""; } event ocsp_request_certificate(f: fa_file, hashAlgorithm: string, issuerNameHash: string, issuerKeyHash: string, serialNumber: string) diff --git a/testing/btest/scripts/base/protocols/ssl/ocsp-response-only.test b/testing/btest/scripts/base/protocols/ssl/ocsp-response-only.test index f99a71802c..43dbf82583 100644 --- a/testing/btest/scripts/base/protocols/ssl/ocsp-response-only.test +++ b/testing/btest/scripts/base/protocols/ssl/ocsp-response-only.test @@ -17,9 +17,9 @@ event ocsp_extension(f: fa_file, ext: X509::Extension, global_resp: bool) print "extension: ", ext, global_resp; } -event ocsp_request(f: fa_file, version: count, requestorName: string) +event ocsp_request(f: fa_file, version: count) { - print "request", version, requestorName; + print "request", version, ""; } event ocsp_request_certificate(f: fa_file, hashAlgorithm: string, issuerNameHash: string, issuerKeyHash: string, serialNumber: string) diff --git a/testing/btest/scripts/base/protocols/ssl/ocsp-revoked.test b/testing/btest/scripts/base/protocols/ssl/ocsp-revoked.test index ae39640f3f..e4378135ad 100644 --- a/testing/btest/scripts/base/protocols/ssl/ocsp-revoked.test +++ b/testing/btest/scripts/base/protocols/ssl/ocsp-revoked.test @@ -17,9 +17,9 @@ event ocsp_extension(f: fa_file, ext: X509::Extension, global_resp: bool) print "extension: ", ext, global_resp; } -event ocsp_request(f: fa_file, version: count, requestorName: string) +event ocsp_request(f: fa_file, version: count) { - print "request", version, requestorName; + print "request", version, ""; } event ocsp_request_certificate(f: fa_file, hashAlgorithm: string, issuerNameHash: string, issuerKeyHash: string, serialNumber: string) From a66364fee0c74018da638eb3a580a92b1e11dfc4 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Mon, 2 Jul 2018 14:04:55 -0500 Subject: [PATCH 4/4] Update install instructions for OpenSSL 1.1 compat --- doc/install/install.rst | 6 ------ 1 file changed, 6 deletions(-) diff --git a/doc/install/install.rst b/doc/install/install.rst index fd05ac9d69..cc8d81b14f 100644 --- a/doc/install/install.rst +++ b/doc/install/install.rst @@ -54,18 +54,12 @@ To install the required dependencies, you can use: sudo yum install cmake make gcc gcc-c++ flex bison libpcap-devel openssl-devel python-devel swig zlib-devel - In order to build Bro on Fedora 26, install ``compat-openssl10-devel`` instead - of ``openssl-devel``. - * DEB/Debian-based Linux: .. console:: sudo apt-get install cmake make gcc g++ flex bison libpcap-dev libssl-dev python-dev swig zlib1g-dev - In order to build Bro on Debian 9 or Ubuntu 18.04, install ``libssl1.0-dev`` - instead of ``libssl-dev``. - * FreeBSD: Most required dependencies should come with a minimal FreeBSD install