Adjust coding style & fix test failures.

I am still not 100% convinced that there is no memory leak hidden
somwehere...

This also makes everything compile with OpenSSL 1.0.2 for me.

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;
 

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 ( p != nullptr )
-	if (q != NULL) *q = d->q;
+		*p = d->p;
-	if (g != NULL) *g = d->g;
+	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 ( n != nullptr )
-	if (e != NULL) *e = r->e;
+		*n = r->n;
-	if (d != NULL) *d = r->d;
+	if ( e != nullptr )
+		*e = r->e;
+	if ( d != nullptr )
+		*d = r->d;
 }
+
 #endif
 
 namespace file_analysis {

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;
+		unsigned char *buf = nullptr;
-		int len = i2d_ASN1_OCTET_STRING(X509_EXTENSION_get_data(ex), buf);
+		int len = i2d_ASN1_OCTET_STRING(X509_EXTENSION_get_data(ex), &buf);
-		if (len >=0 )
+		if ( len >=0 )
-			BIO_write(bio, *buf, len);
+			{
+			BIO_write(bio, &buf, len);
+			OPENSSL_free(buf);
+			}
 		}
 
 	StringVal* ext_val = GetExtensionFromBIO(bio);

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<const char*>(issuer_key_hash->Bytes()), issuer_key_hash->Len());
 		}