Hi Hilko and Kurt,

some progress on this: I have modified Hilko's patch to use new API
functions to access the OCSP response info, see attachment. This seems
to have been the last issue, Bro builds fine with this patch for me with
no additional API breaks.

Unfortunately my additions require the changes from OpenSSL pull request
#1876 [1]. Let's hope that there will be another release with that fix
before the stretch freeze... Kurt, do you think that would be realistic?
Otherwise we'll have to look into other options, up to removing OCSP
functionality from Bro.

I'd be happy to receive some comments. Thanks!

Cheers
Sascha

[1] https://github.com/openssl/openssl/pull/1876
--- a/src/File.cc
+++ b/src/File.cc
@@ -688,7 +688,7 @@
 	// Depending on the OpenSSL version, EVP_*_cbc()
 	// returns a const or a non-const.
 	EVP_CIPHER* cipher_type = (EVP_CIPHER*) EVP_bf_cbc();
-	cipher_ctx = new EVP_CIPHER_CTX;
+	cipher_ctx = EVP_CIPHER_CTX_new();
 
 	unsigned char secret[EVP_PKEY_size(pub_key)];
 	unsigned char* psecret = secret;
@@ -743,7 +743,7 @@
 			return;
 			}
 
-		delete cipher_ctx;
+		EVP_CIPHER_CTX_free(cipher_ctx);
 		cipher_ctx = 0;
 		}
 	}
--- a/src/file_analysis/analyzer/x509/X509.cc
+++ b/src/file_analysis/analyzer/x509/X509.cc
@@ -138,7 +138,9 @@
 	// we only read 255 bytes because byte 256 is always 0.
 	// if the string is longer than 255, that will be our null-termination,
 	// otherwhise i2t does null-terminate.
-	if ( ! i2t_ASN1_OBJECT(buf, 255, ssl_cert->cert_info->key->algor->algorithm) )
+	ASN1_OBJECT *algorithm;
+	X509_PUBKEY_get0_param(&algorithm, NULL, NULL, NULL, X509_get_X509_PUBKEY(ssl_cert));
+	if ( ! i2t_ASN1_OBJECT(buf, 255, algorithm) )
 		buf[0] = 0;
 
 	pX509Cert->Assign(7, new StringVal(buf));
@@ -149,14 +151,12 @@
 	// actually should be (namely - rsaEncryption), so that OpenSSL will parse out the
 	// key later. Otherwise it will just fail to parse the certificate key.
 
-	ASN1_OBJECT* old_algorithm = 0;
-	if ( OBJ_obj2nid(ssl_cert->cert_info->key->algor->algorithm) == NID_md5WithRSAEncryption )
-		{
-		old_algorithm = ssl_cert->cert_info->key->algor->algorithm;
-		ssl_cert->cert_info->key->algor->algorithm = OBJ_nid2obj(NID_rsaEncryption);
-		}
+	if ( X509_get_signature_nid(ssl_cert) == NID_md5WithRSAEncryption )
+		X509_PUBKEY_set0_param(X509_get_X509_PUBKEY(ssl_cert), OBJ_nid2obj(NID_rsaEncryption), 0, NULL, NULL, 0);
+        else
+		algorithm = 0;
 
-	if ( ! i2t_ASN1_OBJECT(buf, 255, ssl_cert->sig_alg->algorithm) )
+	if ( ! i2t_ASN1_OBJECT(buf, 255, OBJ_nid2obj(X509_get_signature_nid(ssl_cert))) )
 		buf[0] = 0;
 
 	pX509Cert->Assign(8, new StringVal(buf));
@@ -165,14 +165,16 @@
 	EVP_PKEY *pkey = X509_extract_key(ssl_cert);
 	if ( pkey != NULL )
 		{
-		if ( pkey->type == EVP_PKEY_DSA )
+		if ( EVP_PKEY_base_id(pkey) == EVP_PKEY_DSA )
 			pX509Cert->Assign(9, new StringVal("dsa"));
 
-		else if ( pkey->type == EVP_PKEY_RSA )
+		else if ( EVP_PKEY_base_id(pkey) == EVP_PKEY_RSA )
 			{
 			pX509Cert->Assign(9, new StringVal("rsa"));
 
-			char *exponent = BN_bn2dec(pkey->pkey.rsa->e);
+			const BIGNUM *e;
+			RSA_get0_key(EVP_PKEY_get0_RSA(pkey), NULL, &e, NULL);
+			char *exponent = BN_bn2dec(e);
 			if ( exponent != NULL )
 				{
 				pX509Cert->Assign(11, new StringVal(exponent));
@@ -181,7 +183,7 @@
 				}
 			}
 #ifndef OPENSSL_NO_EC
-		else if ( pkey->type == EVP_PKEY_EC )
+		else if ( EVP_PKEY_base_id(pkey) == EVP_PKEY_EC )
 			{
 			pX509Cert->Assign(9, new StringVal("ecdsa"));
 			pX509Cert->Assign(12, KeyCurve(pkey));
@@ -190,8 +192,8 @@
 
 		// set key algorithm back. We do not have to free the value that we created because (I think) it
 		// comes out of a static array from OpenSSL memory.
-		if ( old_algorithm )
-			ssl_cert->cert_info->key->algor->algorithm = old_algorithm;
+		if ( algorithm )
+			X509_PUBKEY_set0_param(X509_get_X509_PUBKEY(ssl_cert), algorithm, 0, NULL, NULL, 0);
 
 		unsigned int length = KeyLength(pkey);
 		if ( length > 0 )
@@ -262,7 +264,7 @@
 
 	BIO *bio = BIO_new(BIO_s_mem());
 	if( ! X509V3_EXT_print(bio, ex, 0, 0))
-		M_ASN1_OCTET_STRING_print(bio,ex->value);
+		ASN1_STRING_print(bio,(ASN1_STRING *)X509_EXTENSION_get_data(ex));
 
 	StringVal* ext_val = GetExtensionFromBIO(bio);
 
@@ -444,7 +446,7 @@
 	// well, we do not have EC-Support...
 	return NULL;
 #else
-	if ( key->type != EVP_PKEY_EC )
+	if ( EVP_PKEY_base_id(key) != EVP_PKEY_EC )
 		{
 		// no EC-key - no curve name
 		return NULL;
@@ -452,7 +454,7 @@
 
 	const EC_GROUP *group;
 	int nid;
-	if ( (group = EC_KEY_get0_group(key->pkey.ec)) == NULL)
+	if ( (group = EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(key))) == NULL)
 		// I guess we could not parse this
 		return NULL;
 
@@ -473,12 +475,16 @@
 	{
 	assert(key != NULL);
 
-	switch(key->type) {
+	switch(EVP_PKEY_base_id(key)) {
 	case EVP_PKEY_RSA:
-		return BN_num_bits(key->pkey.rsa->n);
+		const BIGNUM *n;
+		RSA_get0_key(EVP_PKEY_get0_RSA(key), &n, NULL, NULL);
+		return BN_num_bits(n);
 
 	case EVP_PKEY_DSA:
-		return BN_num_bits(key->pkey.dsa->p);
+		const BIGNUM *p;
+		DSA_get0_pqg(EVP_PKEY_get0_DSA(key), &p, NULL, NULL);
+		return BN_num_bits(p);
 
 #ifndef OPENSSL_NO_EC
 	case EVP_PKEY_EC:
@@ -488,7 +494,7 @@
 			// could not malloc bignum?
 			return 0;
 
-		const EC_GROUP *group = EC_KEY_get0_group(key->pkey.ec);
+		const EC_GROUP *group = EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(key));
 
 		if ( ! group )
 			{
--- a/src/file_analysis/analyzer/x509/functions.bif
+++ b/src/file_analysis/analyzer/x509/functions.bif
@@ -106,19 +106,21 @@
 
 // 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(OCSP_BASICRESP *bs)
+X509* x509_get_ocsp_signer(STACK_OF(X509) *certs, OCSP_BASICRESP *bs)
 	{
-    const ASN1_OCTET_STRING *pid = NULL;
-    const X509_NAME *pname = NULL;
-    int rval = OCSP_resp_get0_id(bs, &pid, &pname);
-    if (!rval)
-    	return 0;
-    STACK_OF(X509) *certs = OCSP_resp_get0_certs(bs);
+    ASN1_OCTET_STRING *pid = NULL;
+    X509_NAME *pname = NULL;
+    if ( ! OCSP_resp_get1_id(bs, &pid, &pname) )
+        return 0;
 
-    assert(pid || pname);
+    assert( pid || pname );
 	// We support two lookup types - either by response id or by key.
 	if ( pname && (!pid) ) // by name
-		return X509_find_by_subject(certs, pname);
+        {
+        X509 *ret = X509_find_by_subject(certs, pname);
+        X509_NAME_free(pname);
+        return ret;
+        }
 
 	// There only should be name and type - but let's be sure...
 	if ( !pname && !pid )
@@ -139,10 +141,14 @@
 			continue;
 
 		if ( memcmp(digest, key_hash, SHA_DIGEST_LENGTH) == 0 )
+		    {
 			// keys match, return certificate
+			ASN1_OCTET_STRING_free(pid);
 			return cert;
+		    }
 		}
 
+	ASN1_OCTET_STRING_free(pid);
 	return 0;
 	}
 
@@ -216,6 +222,8 @@
 ##              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 )
@@ -294,10 +302,11 @@
 	// the lookup.
 	// Yay.
 
-	if ( ! basic->certs )
+	ocsp_certs = sk_X509_dup(OCSP_resp_get0_certs(basic));
+	if ( !ocsp_certs )
 		{
-		basic->certs = sk_X509_new_null();
-		if ( ! basic->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;
@@ -307,7 +316,7 @@
 	issuer_certificate = 0;
 	for ( int i = 0; i < sk_X509_num(untrusted_certs); i++)
 		{
-		sk_X509_push(basic->certs, X509_dup(sk_X509_value(untrusted_certs, i)));
+		sk_X509_push(ocsp_certs, X509_dup(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);
@@ -315,7 +324,7 @@
 
 	// 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);
+	signer = x509_get_ocsp_signer(ocsp_certs, basic);
 
 	/*
 	Do this perhaps - OpenSSL also cannot do it, so I do not really feel bad about it.
@@ -323,7 +332,7 @@
 
 	if ( !s igner )
 		// if we did not find it in the certificates that were sent, search in the root store
-		signer = x509_get_ocsp_signer(basic->certs, basic->tbsResponseData->responderId);
+		signer = x509_get_ocsp_signer(ocsp_certs, basic->tbsResponseData->responderId);
 	*/
 
 	if ( ! signer )
@@ -333,15 +342,15 @@
 		}
 
 	csc = X509_STORE_CTX_new();
-	X509_STORE_CTX_init(csc, ctx, signer, basic->certs);
+	X509_STORE_CTX_init(csc, ctx, signer, ocsp_certs);
 	X509_STORE_CTX_set_time(csc, 0, (time_t) verify_time);
 	X509_STORE_CTX_set_purpose(csc, X509_PURPOSE_OCSP_HELPER);
 
 	result = X509_verify_cert(csc);
 	if ( result != 1 )
 		{
-		const char *reason = X509_verify_cert_error_string((*csc).error);
-		rval = x509_result_record(result, X509_verify_cert_error_string((*csc).error));
+		const char *reason = X509_verify_cert_error_string(X509_STORE_CTX_get_error(csc));
+		rval = x509_result_record(result, X509_verify_cert_error_string(X509_STORE_CTX_get_error(csc)));
 		goto x509_ocsp_cleanup;
 		}
 
@@ -366,15 +375,17 @@
 	else
 		{
 		// issuer not in list sent by server, check store
-		X509_OBJECT obj;
-		int lookup = X509_STORE_get_by_subject(csc, X509_LU_X509, X509_get_subject_name(cert), &obj);
+		X509_OBJECT *obj = X509_OBJECT_new();
+		int lookup = X509_STORE_get_by_subject(csc, X509_LU_X509, X509_get_subject_name(cert), obj);
 		if ( lookup <= 0)
 			{
 			rval = x509_result_record(lookup, "Could not find issuer of host certificate");
+			X509_OBJECT_free(obj);
 			goto x509_ocsp_cleanup;
 			}
 
-		certid = OCSP_cert_to_id(NULL, cert, obj.data.x509);
+		certid = OCSP_cert_to_id(NULL, cert,X509_OBJECT_get0_X509( obj));
+		X509_OBJECT_free(obj);
 		}
 
 
@@ -385,18 +396,22 @@
 		}
 
 	// for now, assume we have one reply...
-	single = sk_OCSP_SINGLERESP_value(basic->tbsResponseData->responses, 0);
+	single = OCSP_resp_get0(basic, 0);
 	if ( ! single )
 		{
 		rval = x509_result_record(-1, "Could not lookup OCSP response information");
 		goto x509_ocsp_cleanup;
 		}
 
-	if ( OCSP_id_cmp(certid, single->certId) != 0 )
+	if ( OCSP_id_cmp(certid, (OCSP_CERTID*)OCSP_SINGLERESP_get0_id(single)) != 0 )
 		return x509_result_record(-1, "OCSP reply is not for host certificate");
 
 	// next - check freshness of proof...
-	if ( ! ASN1_GENERALIZEDTIME_check(single->thisUpdate) || ! ASN1_GENERALIZEDTIME_check(single->nextUpdate) )
+	ASN1_GENERALIZEDTIME *thisUpdate;
+	ASN1_GENERALIZEDTIME *nextUpdate;
+	int type;
+	type = OCSP_single_get0_status(single, NULL, NULL, &thisUpdate, &nextUpdate);
+	if ( ! ASN1_GENERALIZEDTIME_check(thisUpdate) || ! ASN1_GENERALIZEDTIME_check(nextUpdate) )
 		{
 		rval = x509_result_record(-1, "OCSP reply contains invalid dates");
 		goto x509_ocsp_cleanup;
@@ -409,16 +424,16 @@
 	// Well, we will do it manually.
 
 
-	if ( X509_cmp_time(single->thisUpdate, &vtime) > 0 )
+	if ( X509_cmp_time(thisUpdate, &vtime) > 0 )
 		rval = x509_result_record(-1, "OCSP reply specifies time in future");
-	else if ( X509_cmp_time(single->nextUpdate, &vtime) < 0 )
+	else if ( X509_cmp_time(nextUpdate, &vtime) < 0 )
 		rval = x509_result_record(-1, "OCSP reply expired");
-	else if ( single->certStatus->type != V_OCSP_CERTSTATUS_GOOD )
-		rval = x509_result_record(-1, OCSP_cert_status_str(single->certStatus->type));
+	else if ( type != V_OCSP_CERTSTATUS_GOOD )
+		rval = x509_result_record(-1, OCSP_cert_status_str(type));
 
 	// if we have no error so far, we are done.
 	if ( !rval )
-		rval = x509_result_record(1, OCSP_cert_status_str(single->certStatus->type));
+		rval = x509_result_record(1, OCSP_cert_status_str(type));
 
 x509_ocsp_cleanup:
 
@@ -495,18 +510,18 @@
 	if ( ! untrusted_certs )
 		return x509_result_record(-1, "Problem initializing list of untrusted certificates");
 
-	X509_STORE_CTX csc;
-	X509_STORE_CTX_init(&csc, ctx, cert, untrusted_certs);
-	X509_STORE_CTX_set_time(&csc, 0, (time_t) verify_time);
-	X509_STORE_CTX_set_flags(&csc, X509_V_FLAG_USE_CHECK_TIME);
+	X509_STORE_CTX *csc = X509_STORE_CTX_new();
+	X509_STORE_CTX_init(csc, ctx, cert, untrusted_certs);
+	X509_STORE_CTX_set_time(csc, 0, (time_t) verify_time);
+	X509_STORE_CTX_set_flags(csc, X509_V_FLAG_USE_CHECK_TIME);
 
-	int result = X509_verify_cert(&csc);
+	int result = X509_verify_cert(csc);
 
 	VectorVal* chainVector = 0;
 
 	if ( result == 1 ) // we have a valid chain. try to get it...
 		{
-		STACK_OF(X509)* chain = X509_STORE_CTX_get1_chain(&csc); // get1 = deep copy
+		STACK_OF(X509)* chain = X509_STORE_CTX_get1_chain(csc); // get1 = deep copy
 
 		if ( ! chain )
 			{
@@ -538,11 +553,11 @@
 
 x509_verify_chainerror:
 
-	X509_STORE_CTX_cleanup(&csc);
+	X509_STORE_CTX_cleanup(csc);
 
 	sk_X509_free(untrusted_certs);
 
-	RecordVal* rrecord = x509_result_record(csc.error, X509_verify_cert_error_string(csc.error), chainVector);
+	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;
 	%}

Reply via email to