From 081ae2eb61b1c7949d71ed4ddf0ce1efe416e218 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 19 Mar 2008 09:44:47 +0000 Subject: [PATCH] fixed CRL check return value on revoked certificates fixed possible refcounting bugs generic return_null() implementation --- src/charon/credentials/credential_manager.c | 48 ++++++++------------- src/charon/plugins/med_db/med_db_creds.c | 8 ---- src/charon/plugins/sql/sql_cred.c | 8 ---- src/charon/plugins/stroke/stroke.c | 8 ---- src/libstrongswan/utils.c | 8 ++++ src/libstrongswan/utils.h | 5 +++ 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 @@ -331,14 +331,6 @@ static enumerator_t* create_shared_enumerator(private_sql_cred_t *this, return &e->public; } -/** - * return null - */ -static void *return_null() -{ - return NULL; -} - /** * Implementation of sql_cred_t.destroy. */ 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 @@ -281,14 +281,6 @@ static void ca_section_destroy(ca_section_t *this) free(this); } -/** - * another return NULL - */ -static void* return_null() -{ - return NULL; -} - /** * data to pass to create_inner_cdp */ 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 @@ -63,6 +63,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 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 @@ -209,6 +209,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 */ -- 2.39.5