Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

cleanup: gc usage

Cleanup of "Use the garbage collector when retrieving x509 fields"
patch series.

Discussed at [1].

There should be an effort to produce common function prologue
and epilogue, so that cleanups will be done at single point.

[1] http://comments.gmane.org/gmane.network.openvpn.devel/5401

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
Acked-by: Adriaan de Jong <dejong@fox-it.com>
Signed-off-by: David Sommerseth <davids@redhat.com>
  • Loading branch information...
commit 75b49e406430299b187964744f82e50a9035a0d3 1 parent 12e4609
@alonbl alonbl authored dsommers committed
View
5 src/openvpn/pkcs11.c
@@ -769,6 +769,7 @@ show_pkcs11_ids (
const char * const provider,
bool cert_private
) {
+ struct gc_arena gc = gc_new();
pkcs11h_certificate_id_list_t user_certificates = NULL;
pkcs11h_certificate_id_list_t current = NULL;
CK_RV rv = CKR_FUNCTION_FAILED;
@@ -834,7 +835,6 @@ show_pkcs11_ids (
);
for (current = user_certificates;current != NULL; current = current->next) {
pkcs11h_certificate_t certificate = NULL;
- struct gc_arena gc = gc_new();
char *dn = NULL;
char serial[1024] = {0};
char *ser = NULL;
@@ -927,8 +927,6 @@ show_pkcs11_ids (
free (ser);
ser = NULL;
}
-
- gc_free (&gc);
}
cleanup:
@@ -938,6 +936,7 @@ show_pkcs11_ids (
}
pkcs11h_terminate ();
+ gc_free (&gc);
}
#else
View
8 src/openvpn/pkcs11_polarssl.c
@@ -75,7 +75,7 @@ pkcs11_init_tls_session(pkcs11h_certificate_t certificate,
char *
pkcs11_certificate_dn (pkcs11h_certificate_t cert, struct gc_arena *gc)
{
- int ret = 1;
+ char *ret = NULL;
char dn[1024] = {0};
x509_cert polar_cert = {0};
@@ -90,14 +90,12 @@ pkcs11_certificate_dn (pkcs11h_certificate_t cert, struct gc_arena *gc)
goto cleanup;
}
- ret = 0;
+ ret = string_alloc(dn, gc);
cleanup:
x509_free(&polar_cert);
- if (ret == 0)
- return string_alloc(dn, gc);
- return NULL;
+ return ret;
}
int
View
53 src/openvpn/ssl_verify.c
@@ -538,8 +538,9 @@ verify_cert_call_command(const char *verify_command, struct env_set *es,
static result_t
verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert)
{
+ result_t ret = FAILURE;
char fn[256];
- int fd;
+ int fd = -1;
struct gc_arena gc = gc_new();
char *serial = x509_get_serial(cert, &gc);
@@ -547,26 +548,29 @@ verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert)
if (!openvpn_snprintf(fn, sizeof(fn), "%s%c%s", crl_dir, OS_SPECIFIC_DIRSEP, serial))
{
msg (D_HANDSHAKE, "VERIFY CRL: filename overflow");
- gc_free(&gc);
- return FAILURE;
+ goto cleanup;
}
fd = platform_open (fn, O_RDONLY, 0);
if (fd >= 0)
{
msg (D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is revoked", serial);
- close(fd);
- gc_free(&gc);
- return FAILURE;
+ goto cleanup;
}
- gc_free(&gc);
+ ret = SUCCESS;
- return SUCCESS;
+cleanup:
+
+ if (fd != -1)
+ close(fd);
+ gc_free(&gc);
+ return ret;
}
result_t
verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_depth)
{
+ result_t ret = FAILURE;
char *subject = NULL;
char common_name[TLS_USERNAME_LEN] = {0};
const struct tls_options *opt;
@@ -583,7 +587,7 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
{
msg (D_TLS_ERRORS, "VERIFY ERROR: depth=%d, could not extract X509 "
"subject string from certificate", cert_depth);
- goto err;
+ goto cleanup;
}
/* enforce character class restrictions in X509 name */
@@ -602,7 +606,7 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
opt->x509_username_field,
subject,
TLS_USERNAME_LEN);
- goto err;
+ goto cleanup;
}
}
@@ -613,7 +617,7 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
if (cert_depth >= MAX_CERT_DEPTH)
{
msg (D_TLS_ERRORS, "TLS Error: Convoluted certificate chain detected with depth [%d] greater than %d", cert_depth, MAX_CERT_DEPTH);
- goto err; /* Reject connection */
+ goto cleanup; /* Reject connection */
}
/* verify level 1 cert, i.e. the CA that signed our leaf cert */
@@ -623,7 +627,7 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
if (memcmp (sha1_hash, opt->verify_hash, SHA_DIGEST_LENGTH))
{
msg (D_TLS_ERRORS, "TLS Error: level-1 certificate hash verification failed");
- goto err;
+ goto cleanup;
}
}
@@ -645,16 +649,16 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
/* If this is the peer's own certificate, verify it */
if (cert_depth == 0 && SUCCESS != verify_peer_cert(opt, cert, subject, common_name))
- goto err;
+ goto cleanup;
/* call --tls-verify plug-in(s), if registered */
if (SUCCESS != verify_cert_call_plugin(opt->plugins, opt->es, cert_depth, cert, subject))
- goto err;
+ goto cleanup;
/* run --tls-verify script */
if (opt->verify_command && SUCCESS != verify_cert_call_command(opt->verify_command,
opt->es, cert_depth, cert, subject, opt->verify_export_cert))
- goto err;
+ goto cleanup;
/* check peer cert against CRL */
if (opt->crl_file)
@@ -662,26 +666,29 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
if (opt->ssl_flags & SSLF_CRL_VERIFY_DIR)
{
if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert))
- goto err;
+ goto cleanup;
}
else
{
if (SUCCESS != x509_verify_crl(opt->crl_file, cert, subject))
- goto err;
+ goto cleanup;
}
}
msg (D_HANDSHAKE, "VERIFY OK: depth=%d, %s", cert_depth, subject);
session->verified = true;
+ ret = SUCCESS;
- gc_free(&gc);
- return SUCCESS;
+cleanup:
- err:
- tls_clear_error();
- session->verified = false;
+ if (ret != SUCCESS)
+ {
+ tls_clear_error(); /* always? */
+ session->verified = false; /* double sure? */
+ }
gc_free(&gc);
- return FAILURE;
+
+ return ret;
}
/* ***************************************************************************
View
15 src/openvpn/ssl_verify_openssl.c
@@ -46,6 +46,7 @@
int
verify_callback (int preverify_ok, X509_STORE_CTX * ctx)
{
+ int ret = 0;
struct tls_session *session;
SSL *ssl;
struct gc_arena gc = gc_new();
@@ -76,17 +77,19 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx)
ERR_clear_error();
- gc_free(&gc);
session->verified = false;
-
- return 0;
+ goto cleanup;
}
+ if (SUCCESS != verify_cert(session, ctx->current_cert, ctx->error_depth))
+ goto cleanup;
+
+ ret = 1;
+
+cleanup:
gc_free(&gc);
- if (SUCCESS == verify_cert(session, ctx->current_cert, ctx->error_depth))
- return 1;
- return 0;
+ return ret;
}
#ifdef ENABLE_X509ALTUSERNAME
View
18 src/openvpn/ssl_verify_polarssl.c
@@ -48,6 +48,7 @@ verify_callback (void *session_obj, x509_cert *cert, int cert_depth,
{
struct tls_session *session = (struct tls_session *) session_obj;
struct gc_arena gc = gc_new();
+ int ret = 1;
ASSERT (cert);
ASSERT (session);
@@ -68,18 +69,21 @@ verify_callback (void *session_obj, x509_cert *cert, int cert_depth,
msg (D_TLS_ERRORS, "VERIFY ERROR: depth=%d, could not extract X509 "
"subject string from certificate", cert_depth);
- gc_free(&gc);
- return 1;
+ goto cleanup;
}
+ if (SUCCESS != verify_cert(session, cert, cert_depth))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ gc_free(&gc);
+
/*
* PolarSSL expects 1 on failure, 0 on success
*/
- gc_free(&gc);
-
- if (SUCCESS == verify_cert(session, cert, cert_depth))
- return 0;
- return 1;
+ return ret;
}
#ifdef ENABLE_X509ALTUSERNAME
Please sign in to comment.
Something went wrong with that request. Please try again.