diff --git a/src/file_analysis/analyzer/x509/X509.cc b/src/file_analysis/analyzer/x509/X509.cc index c9e78bb0b9..6f3cce23f4 100644 --- a/src/file_analysis/analyzer/x509/X509.cc +++ b/src/file_analysis/analyzer/x509/X509.cc @@ -152,7 +152,7 @@ RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val, const char* // actually should be (namely - rsaEncryption), so that OpenSSL will parse out the // key later. Otherwise it will just fail to parse the certificate key. - if ( X509_get_signature_nid(ssl_cert) == NID_md5WithRSAEncryption ) + if ( OBJ_obj2nid(algorithm) == NID_md5WithRSAEncryption ) X509_PUBKEY_set0_param(X509_get_X509_PUBKEY(ssl_cert), OBJ_nid2obj(NID_rsaEncryption), 0, NULL, NULL, 0); else algorithm = 0; @@ -380,7 +380,7 @@ StringVal* file_analysis::X509::KeyCurve(EVP_PKEY *key) const EC_GROUP *group; int nid; - if ( (group = EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(key))) == NULL) + if ( (group = EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(key))) == NULL ) // I guess we could not parse this return NULL; diff --git a/src/file_analysis/analyzer/x509/X509.h b/src/file_analysis/analyzer/x509/X509.h index 60f67a9fd1..1e7cd05a62 100644 --- a/src/file_analysis/analyzer/x509/X509.h +++ b/src/file_analysis/analyzer/x509/X509.h @@ -12,42 +12,49 @@ #define X509_get_signature_nid(x) OBJ_obj2nid((x)->sig_alg->algorithm) -#define X509_OBJECT_new() (X509_OBJECT*)malloc(sizeof(X509_OBJECT)) -#define X509_OBJECT_free(a) free(a) - -static X509 *X509_OBJECT_get0_X509(const X509_OBJECT *a) -{ - if (a == NULL || a->type != X509_LU_X509) - return NULL; - return a->data.x509; -} - -#define OCSP_SINGLERESP_get0_id(s) (s)->certId -#define OCSP_resp_get0_certs(x) (x)->certs - #endif #if (OPENSSL_VERSION_NUMBER < 0x1010000fL) +#define X509_OBJECT_new() (X509_OBJECT*)malloc(sizeof(X509_OBJECT)) +#define X509_OBJECT_free(a) free(a) + +#define OCSP_SINGLERESP_get0_id(s) (s)->certId +#define OCSP_resp_get0_certs(x) (x)->certs + #define EVP_PKEY_get0_DSA(p) ((p)->pkey.dsa) #define EVP_PKEY_get0_EC_KEY(p) ((p)->pkey.ec) #define EVP_PKEY_get0_RSA(p) ((p)->pkey.rsa) +static X509 *X509_OBJECT_get0_X509(const X509_OBJECT *a) +{ + if ( a == nullptr || a->type != X509_LU_X509 ) + return nullptr; + return a->data.x509; +} + static void DSA_get0_pqg(const DSA *d, const BIGNUM **p, const BIGNUM **q, const BIGNUM **g) { - if (p != NULL) *p = d->p; - if (q != NULL) *q = d->q; - if (g != NULL) *g = d->g; + if ( p != nullptr ) + *p = d->p; + if ( q != nullptr ) + *q = d->q; + if ( g != nullptr ) + *g = d->g; } static void RSA_get0_key(const RSA *r, const BIGNUM **n, const BIGNUM **e, const BIGNUM **d) { - if (n != NULL) *n = r->n; - if (e != NULL) *e = r->e; - if (d != NULL) *d = r->d; + if ( n != nullptr ) + *n = r->n; + if ( e != nullptr ) + *e = r->e; + if ( d != nullptr ) + *d = r->d; } + #endif namespace file_analysis { diff --git a/src/file_analysis/analyzer/x509/X509Common.cc b/src/file_analysis/analyzer/x509/X509Common.cc index 2e0a03ac8f..b101f502ff 100644 --- a/src/file_analysis/analyzer/x509/X509Common.cc +++ b/src/file_analysis/analyzer/x509/X509Common.cc @@ -240,10 +240,13 @@ void file_analysis::X509Common::ParseExtension(X509_EXTENSION* ex, EventHandlerP BIO *bio = BIO_new(BIO_s_mem()); if( ! X509V3_EXT_print(bio, ex, 0, 0)) { - unsigned char **buf = NULL; - int len = i2d_ASN1_OCTET_STRING(X509_EXTENSION_get_data(ex), buf); - if (len >=0 ) - BIO_write(bio, *buf, len); + unsigned char *buf = nullptr; + int len = i2d_ASN1_OCTET_STRING(X509_EXTENSION_get_data(ex), &buf); + if ( len >=0 ) + { + BIO_write(bio, &buf, len); + OPENSSL_free(buf); + } } StringVal* ext_val = GetExtensionFromBIO(bio); diff --git a/src/file_analysis/analyzer/x509/functions.bif b/src/file_analysis/analyzer/x509/functions.bif index 217a845aa3..6fdb695ce8 100644 --- a/src/file_analysis/analyzer/x509/functions.bif +++ b/src/file_analysis/analyzer/x509/functions.bif @@ -239,8 +239,6 @@ function x509_get_certificate_string%(cert: opaque of x509, pem: bool &default=F ## x509_get_certificate_string x509_verify function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_certs: table_string_of_string, verify_time: time &default=network_time()%): X509::Result %{ - stack_st_X509* ocsp_certs; - RecordVal* rval = 0; X509_STORE* ctx = x509_get_root_store(root_certs->AsTableVal()); if ( ! ctx ) @@ -284,6 +282,7 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c OCSP_SINGLERESP *single = 0; X509_STORE_CTX *csc = 0; OCSP_CERTID *certid = 0; + stack_st_X509* ocsp_certs = nullptr; int status = -1; int out = -1; int result = -1; @@ -310,7 +309,6 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c goto x509_ocsp_cleanup; } - // the following code took me _forever_ to get right. // The OCSP_basic_verify command takes a list of certificates. However (which is not immediately // visible or understandable), those are only used to find the signer certificate. They are _not_ @@ -319,21 +317,10 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c // the lookup. // Yay. - ocsp_certs = sk_X509_dup(OCSP_resp_get0_certs(basic)); - if ( !ocsp_certs ) - { - ocsp_certs = sk_X509_new_null(); - if ( !ocsp_certs ) - { - rval = x509_result_record(-1, "Could not allocate basic x509 stack"); - goto x509_ocsp_cleanup; - } - } - issuer_certificate = 0; for ( int i = 0; i < sk_X509_num(untrusted_certs); i++) { - sk_X509_push(ocsp_certs, X509_dup(sk_X509_value(untrusted_certs, i))); + OCSP_basic_add1_cert(basic, sk_X509_value(untrusted_certs, i)); if ( X509_NAME_cmp(X509_get_issuer_name(cert), X509_get_subject_name(sk_X509_value(untrusted_certs, i))) == 0 ) issuer_certificate = sk_X509_value(untrusted_certs, i); @@ -358,6 +345,14 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c goto x509_ocsp_cleanup; } + { + auto basic_certs = OCSP_resp_get0_certs(basic); + if ( basic_certs ) + ocsp_certs = sk_X509_dup(basic_certs); + + assert(ocsp_certs); + } + csc = X509_STORE_CTX_new(); X509_STORE_CTX_init(csc, ctx, signer, ocsp_certs); X509_STORE_CTX_set_time(csc, 0, (time_t) verify_time); @@ -383,7 +378,7 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c // ok, now we verified the OCSP response. This means that we have a valid chain tying it // to a root that we trust and that the signature also hopefully is valid. This does not yet - // mean that the ocsp response actually matches the certificate the server send us or that + // mean that the ocsp response actually matches the certificate the server sent us or that // the OCSP response even says that the certificate is valid. // let's start this out by checking that the response is actually for the certificate we want @@ -457,6 +452,9 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c x509_ocsp_cleanup: + if ( ocsp_certs ) + sk_X509_free(ocsp_certs); + if ( untrusted_certs ) sk_X509_free(untrusted_certs); @@ -573,12 +571,13 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str x509_verify_chainerror: + RecordVal* rrecord = x509_result_record(X509_STORE_CTX_get_error(csc), X509_verify_cert_error_string(X509_STORE_CTX_get_error(csc)), chainVector); + X509_STORE_CTX_cleanup(csc); + X509_STORE_CTX_free(csc); sk_X509_free(untrusted_certs); - RecordVal* rrecord = x509_result_record(X509_STORE_CTX_get_error(csc), X509_verify_cert_error_string(X509_STORE_CTX_get_error(csc)), chainVector); - return rrecord; %} @@ -673,7 +672,7 @@ function sct_verify%(cert: opaque of x509, logid: string, log_key: string, signa x->cert_info->enc.modified = 1; cert_length = i2d_X509_CINF(x->cert_info, &cert_out); #else - i2d_re_X509_tbs(x, &cert_out); + cert_length = i2d_re_X509_tbs(x, &cert_out); #endif data.append(reinterpret_cast(issuer_key_hash->Bytes()), issuer_key_hash->Len()); }