Merge remote-tracking branch 'origin/topic/johanna/ticket-1212'

* origin/topic/johanna/ticket-1212:
  Fix ocsp reply validation - there were a few things that definitely were wrong.
  fix null pointer dereference in ocsp verification code in case no certificate is sent as part as the ocsp reply.

CHANGES

@@ -1,4 +1,13 @@
 
+2.3-134 | 2014-09-04 16:16:36 -0700
+
+  * Fixed a number of issues with OCSP reply validation. Addresses
+    BIT-1212. (Johanna Amann)
+
+  * Fix null pointer dereference in OCSP verification code in case no
+    certificate is sent as part as the ocsp reply. Addresses BIT-1212.
+    (Johanna Amann)
+
 2.3-131 | 2014-09-04 16:10:32 -0700
 
   * Make links in documentation templates protocol relative. (Johanna

VERSION

@@ -1 +1 @@
-2.3-131
+2.3-134

src/file_analysis/analyzer/x509/functions.bif

@@ -104,6 +104,39 @@ STACK_OF(X509)* x509_get_untrusted_stack(VectorVal* certs_vec)
 	return untrusted_certs;
 	}
 
+// 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)
+	{
+	// 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);
+
+	// There only should be name and type - but let's be sure...
+	if ( rid->type != V_OCSP_RESPID_KEY )
+		return 0;
+
+	// Just like OpenSSL, we just support SHA-1 lookups and bail out otherwhise.
+	if ( rid->value.byKey->length != SHA_DIGEST_LENGTH )
+		return 0;
+
+	unsigned char* key_hash = rid->value.byKey->data;
+	for ( int i = 0; i < sk_X509_num(certs); ++i )
+		{
+		unsigned char digest[SHA_DIGEST_LENGTH];
+		X509* cert = sk_X509_value(certs, i);
+		if ( ! X509_pubkey_digest(cert, EVP_sha1(), digest, NULL) )
+			// digest failed for this certificate, try with next
+			continue;
+
+		if ( memcmp(digest, key_hash, SHA_DIGEST_LENGTH) == 0 )
+			// keys match, return certificate
+			return cert;
+		}
+
+	return 0;
+	}
+
 %%}
 
 ## Parses a certificate into an X509::Certificate structure.
@@ -221,6 +254,7 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
 	int out = -1;
 	int result = -1;
 	X509* issuer_certificate = 0;
+	X509* signer = 0;
 	OCSP_RESPONSE *resp = d2i_OCSP_RESPONSE(NULL, &start, ocsp_reply->Len());
 	if ( ! resp )
 		{
@@ -250,19 +284,47 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
 	// inject the certificates in the certificate list of the OCSP reply, they actually are used during
 	// the lookup.
 	// Yay.
+
+	if ( ! basic->certs )
+		{
+		basic->certs = sk_X509_new_null();
+		if ( ! basic->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(basic->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))) )
+		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);
 		}
 
 	// 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);
+
+	/*
+	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 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);
+	*/
+
+	if ( ! signer )
+		{
+		rval = x509_result_record(-1, "Could not find OCSP responder certificate");
+		goto x509_ocsp_cleanup;
+		}
+
 	csc = X509_STORE_CTX_new();
-	X509_STORE_CTX_init(csc, ctx, sk_X509_value(basic->certs, 0), basic->certs);
+	X509_STORE_CTX_init(csc, ctx, signer, basic->certs);
 	X509_STORE_CTX_set_time(csc, 0, (time_t) verify_time);
 	X509_STORE_CTX_set_purpose(csc, X509_PURPOSE_OCSP_HELPER);
 
@@ -281,7 +343,6 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
 		goto x509_ocsp_cleanup;
 		}
 
-
 	// 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
@@ -322,7 +383,7 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
 		goto x509_ocsp_cleanup;
 		}
 
-	if ( ! OCSP_id_cmp(certid, single->certId) )
+	if ( OCSP_id_cmp(certid, single->certId) != 0 )
 		return x509_result_record(-1, "OCSP reply is not for host certificate");
 
 	// next - check freshness of proof...

testing/btest/Baseline/scripts.policy.protocols.ssl.validate-ocsp/ssl-digicert.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#open	2014-09-04-19-17-18
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	cipher	curve	server_name	session_id	last_alert	established	cert_chain_fuids	client_cert_chain_fuids	subject	issuer	client_subject	client_issuer	ocsp_status
+#types	time	string	addr	port	addr	port	string	string	string	string	string	string	bool	vector[string]	vector[string]	string	string	string	string	string
+1404148886.994021	CXWv6p3arKYeMETxOg	192.168.4.149	51293	72.21.91.29	443	TLSv10	TLS_ECDHE_RSA_WITH_RC4_128_SHA	secp256r1	-	-	-	T	FhwjYM0FkbvVCvMf2,Fajs2d2lipsadwoK1h	(empty)	CN=www.digicert.com,O=DigiCert\, Inc.,L=Lehi,ST=Utah,C=US,postalCode=84043,street=2600 West Executive Parkway,street=Suite 500,serialNumber=5299537-0142,1.3.6.1.4.1.311.60.2.1.2=#130455746168,1.3.6.1.4.1.311.60.2.1.3=#13025553,businessCategory=Private Organization	CN=DigiCert SHA2 Extended Validation Server CA,OU=www.digicert.com,O=DigiCert Inc,C=US	-	-	good
+#close	2014-09-04-19-17-18

testing/btest/Baseline/scripts.policy.protocols.ssl.validate-ocsp/ssl-twimg.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#open	2014-09-04-19-17-14
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	cipher	curve	server_name	session_id	last_alert	established	cert_chain_fuids	client_cert_chain_fuids	subject	issuer	client_subject	client_issuer	ocsp_status
+#types	time	string	addr	port	addr	port	string	string	string	string	string	string	bool	vector[string]	vector[string]	string	string	string	string	string
+1409786981.016881	CXWv6p3arKYeMETxOg	192.168.4.149	53106	93.184.216.146	443	TLSv10	TLS_ECDHE_RSA_WITH_RC4_128_SHA	secp256r1	-	-	-	T	FtaZVlJfywdNmVFr1,FoILekwkdtTuZtlVa	(empty)	CN=si0.twimg.com,O=Twitter\, Inc.,L=San Francisco,ST=California,C=US	CN=DigiCert High Assurance CA-3,OU=www.digicert.com,O=DigiCert Inc,C=US	-	-	good
+#close	2014-09-04-19-17-14

testing/btest/scripts/policy/protocols/ssl/validate-ocsp.bro

@@ -1,4 +1,10 @@
 # @TEST-EXEC: bro -C -r $TRACES/tls/ocsp-stapling.trace %INPUT
 # @TEST-EXEC: btest-diff ssl.log
+# @TEST-EXEC: bro -C -r $TRACES/tls/ocsp-stapling-twimg.trace %INPUT
+# @TEST-EXEC: mv ssl.log ssl-twimg.log
+# @TEST-EXEC: btest-diff ssl-twimg.log
+# @TEST-EXEC: bro -C -r $TRACES/tls/ocsp-stapling-digicert.trace %INPUT
+# @TEST-EXEC: mv ssl.log ssl-digicert.log
+# @TEST-EXEC: btest-diff ssl-digicert.log
 
 @load protocols/ssl/validate-ocsp