This fixes a null-pointer dereference in tls_ctx_cert_time(), which will
occur on clients that do not use a client certificate (ie that only have
auth-user-pass in the config, but no key and cert). This bug was
introduced by commit 091edd8e on the master branch, and commit dfd940bb
on the release/2.3 branch.
This bug was found by chipitsine and reported in trac ticket #644.
While touching this function, I also made this function conform to the
openvpn coding style.
v2 - fix memory leak in builds using pre-1.0.2 openssl
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1451814476-32574-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10921
Signed-off-by: Gert Doering <gert@greenie.muc.de>
... | ... |
@@ -356,15 +356,22 @@ tls_ctx_check_cert_time (const struct tls_root_ctx *ctx) |
356 | 356 |
int ret; |
357 | 357 |
const X509 *cert; |
358 | 358 |
|
359 |
+ ASSERT (ctx); |
|
360 |
+ |
|
359 | 361 |
#if OPENSSL_VERSION_NUMBER >= 0x10002000L |
360 | 362 |
/* OpenSSL 1.0.2 and up */ |
361 |
- cert = SSL_CTX_get0_certificate(ctx->ctx); |
|
363 |
+ cert = SSL_CTX_get0_certificate (ctx->ctx); |
|
362 | 364 |
#else |
363 | 365 |
/* OpenSSL 1.0.1 and earlier need an SSL object to get at the certificate */ |
364 |
- SSL *ssl = SSL_new(ctx->ctx); |
|
365 |
- cert = SSL_get_certificate(ssl); |
|
366 |
+ SSL *ssl = SSL_new (ctx->ctx); |
|
367 |
+ cert = SSL_get_certificate (ssl); |
|
366 | 368 |
#endif |
367 | 369 |
|
370 |
+ if (cert == NULL) |
|
371 |
+ { |
|
372 |
+ goto cleanup; /* Nothing to check if there is no certificate */ |
|
373 |
+ } |
|
374 |
+ |
|
368 | 375 |
ret = X509_cmp_time (X509_get_notBefore (cert), NULL); |
369 | 376 |
if (ret == 0) |
370 | 377 |
{ |
... | ... |
@@ -384,9 +391,12 @@ tls_ctx_check_cert_time (const struct tls_root_ctx *ctx) |
384 | 384 |
{ |
385 | 385 |
msg (M_WARN, "WARNING: Your certificate has expired!"); |
386 | 386 |
} |
387 |
+ |
|
388 |
+cleanup: |
|
387 | 389 |
#if OPENSSL_VERSION_NUMBER < 0x10002000L |
388 |
- SSL_free(ssl); |
|
390 |
+ SSL_free (ssl); |
|
389 | 391 |
#endif |
392 |
+ return; |
|
390 | 393 |
} |
391 | 394 |
|
392 | 395 |
void |
... | ... |
@@ -218,6 +218,12 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers) |
218 | 218 |
void |
219 | 219 |
tls_ctx_check_cert_time (const struct tls_root_ctx *ctx) |
220 | 220 |
{ |
221 |
+ ASSERT (ctx); |
|
222 |
+ if (ctx->crt_chain == NULL) |
|
223 |
+ { |
|
224 |
+ return; /* Nothing to check if there is no certificate */ |
|
225 |
+ } |
|
226 |
+ |
|
221 | 227 |
if (x509_time_future (&ctx->crt_chain->valid_from)) |
222 | 228 |
{ |
223 | 229 |
msg (M_WARN, "WARNING: Your certificate is not yet valid!"); |