fixed CRL check return value on revoked certificates

fixed possible refcounting bugs
generic return_null() implementation

6 files changed, 32 insertions, 53 deletions

diff --git a/src/charon/credentials/credential_manager.c b/src/charon/credentials/credential_manager.c
index 71a06e9f7..6642c193f 100644
--- a/src/charon/credentials/credential_manager.c
+++ b/src/charon/credentials/credential_manager.c
@@ -298,13 +298,6 @@ static shared_key_t *get_shared(private_credential_manager_t *this,
 static certificate_t *get_trusted_cert(private_credential_manager_t *this,
 									   key_type_t type, identification_t *id,
 									   auth_info_t *auth, bool crl, bool ocsp);
-/**
- * return null ;-)
- */
-static void *return_null()
-{
-	return NULL;
-}
 
 /**
  * credential_set_t implementation around an OCSP response
@@ -492,22 +485,20 @@ static cert_validation_t check_ocsp(private_credential_manager_t *this,
 {
 	certificate_t *sub = (certificate_t*)subject;
 	certificate_t *best_cert = NULL;
+	certificate_t *cert;
+	public_key_t *public;
 	cert_validation_t valid = VALIDATION_SKIPPED;
 	identification_t *keyid = NULL;
 	bool stale = TRUE;
 	
 	/* derive the authorityKeyIdentifier from the issuer's public key */
+	cert = &issuer->interface;
+	public = cert->get_public_key(cert);
+	if (public)
 	{
-		certificate_t *cert = &issuer->interface;
-		public_key_t *public = cert->get_public_key(cert);
-
-		if (public)
-		{
-			keyid = public->get_id(public, ID_PUBKEY_SHA1);
-			public->destroy(public);
-		}
+		keyid = public->get_id(public, ID_PUBKEY_SHA1);
 	}
-
+	
 	/* find a cached ocsp response by authorityKeyIdentifier */	
 	if (keyid)
 	{
@@ -612,6 +603,7 @@ static cert_validation_t check_ocsp(private_credential_manager_t *this,
 		}
 		enumerator->destroy(enumerator);
 	}
+	DESTROY_IF(public);
 
 	/* if we have an ocsp response, check the revocation status */
 	if (best_cert)
@@ -638,7 +630,7 @@ static cert_validation_t check_ocsp(private_credential_manager_t *this,
 		}
 		best_cert->destroy(best_cert);
 	}
-
+	
 	if (auth)
 	{
 		auth->add_item(auth, AUTHZ_OCSP_VALIDATION, &valid);
@@ -707,19 +699,17 @@ static cert_validation_t check_crl(private_credential_manager_t *this,
 {
 	identification_t *keyid = NULL;
 	certificate_t *best_cert = NULL;
+	certificate_t *cert;
+	public_key_t *public;
 	cert_validation_t valid = VALIDATION_SKIPPED;
 	bool stale = TRUE;
 	
 	/* derive the authorityKeyIdentifier from the issuer's public key */
+	cert = &issuer->interface;
+	public = cert->get_public_key(cert);
+	if (public)
 	{
-		certificate_t *cert = &issuer->interface;
-		public_key_t *public = cert->get_public_key(cert);
-
-		if (public)
-		{
-			keyid = public->get_id(public, ID_PUBKEY_SHA1);
-			public->destroy(public);
-		}
+		keyid = public->get_id(public, ID_PUBKEY_SHA1);
 	}
 	
 	/* find a cached crl by authorityKeyIdentifier */
@@ -820,6 +810,7 @@ static cert_validation_t check_crl(private_credential_manager_t *this,
 		}
 		enumerator->destroy(enumerator);
 	}
+	DESTROY_IF(public);
 
 	/* if we have a crl, check the revocation status */
 	if (best_cert)
@@ -884,8 +875,7 @@ static bool check_certificate(private_credential_manager_t *this,
 			switch (check_ocsp(this, (x509_t*)subject, (x509_t*)issuer, auth))
 			{
 				case VALIDATION_GOOD:
-					DBG1(DBG_CFG, "certificate status is good",
-						 subject->get_subject(subject));
+					DBG1(DBG_CFG, "certificate status is good");
 					return TRUE;
 				case VALIDATION_REVOKED:
 					/* has already been logged */			
@@ -905,10 +895,10 @@ static bool check_certificate(private_credential_manager_t *this,
 			{
 				case VALIDATION_GOOD:
 					DBG1(DBG_CFG, "certificate status is good");
-					break;
+					return TRUE;
 				case VALIDATION_REVOKED:		
 					/* has already been logged */			
-					break;
+					return FALSE;
 				case VALIDATION_UNKNOWN:
 					DBG1(DBG_CFG, "certificate status is unknown");
 					break;
diff --git a/src/charon/plugins/med_db/med_db_creds.c b/src/charon/plugins/med_db/med_db_creds.c
index f8aed2597..cd1058aa5 100644
--- a/src/charon/plugins/med_db/med_db_creds.c
+++ b/src/charon/plugins/med_db/med_db_creds.c
@@ -173,14 +173,6 @@ static enumerator_t* create_shared_enumerator(private_med_db_creds_t *this,
 	}
 	return NULL;
 }
-
-/**
- * returns null
- */
-static void *return_null()
-{
-	return NULL;
-}
 	
 /**
  * Implementation of backend_t.destroy.
diff --git a/src/charon/plugins/sql/sql_cred.c b/src/charon/plugins/sql/sql_cred.c
index 73a6a9c66..a504b23af 100644
--- a/src/charon/plugins/sql/sql_cred.c
+++ b/src/charon/plugins/sql/sql_cred.c
@@ -332,14 +332,6 @@ static enumerator_t* create_shared_enumerator(private_sql_cred_t *this,
 }
 
 /**
- * return null
- */
-static void *return_null()
-{
-	return NULL;
-}
-
-/**
  * Implementation of sql_cred_t.destroy.
  */
 static void destroy(private_sql_cred_t *this)
diff --git a/src/charon/plugins/stroke/stroke.c b/src/charon/plugins/stroke/stroke.c
index 6af5ebefc..401cb2228 100755
--- a/src/charon/plugins/stroke/stroke.c
+++ b/src/charon/plugins/stroke/stroke.c
@@ -282,14 +282,6 @@ static void ca_section_destroy(ca_section_t *this)
 }
 
 /**
- * another return NULL 
- */
-static void* return_null()
-{
-	return NULL;
-}
-
-/**
  * data to pass to create_inner_cdp
  */
 typedef struct {
diff --git a/src/libstrongswan/utils.c b/src/libstrongswan/utils.c
index d6920cf28..953ef505e 100644
--- a/src/libstrongswan/utils.c
+++ b/src/libstrongswan/utils.c
@@ -64,6 +64,14 @@ void memxor(u_int8_t dest[], u_int8_t src[], size_t n)
 }
 
 /**
+ * return null
+ */
+void *return_null()
+{
+	return NULL;
+}
+
+/**
  * We use a single mutex for all refcount variables. This
  * is not optimal for performance, but the critical section
  * is not that long...
diff --git a/src/libstrongswan/utils.h b/src/libstrongswan/utils.h
index 2624c31d1..aae368af1 100644
--- a/src/libstrongswan/utils.h
+++ b/src/libstrongswan/utils.h
@@ -210,6 +210,11 @@ void *clalloc(void *pointer, size_t size);
 void memxor(u_int8_t dest[], u_int8_t src[], size_t n);
 
 /**
+ * returns null
+ */
+void *return_null();
+
+/**
  * Special type to count references
  */
 typedef volatile u_int refcount_t;