You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2014/09/02 00:30:48 UTC
git commit: TS-3039: plug BIO leak in OCSP stapling support
Repository: trafficserver
Updated Branches:
refs/heads/5.1.x a32798771 -> ea4fec414
TS-3039: plug BIO leak in OCSP stapling support
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ea4fec41
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ea4fec41
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ea4fec41
Branch: refs/heads/5.1.x
Commit: ea4fec41496172815a8e1c364178905a14e68651
Parents: a327987
Author: James Peach <jp...@apache.org>
Authored: Mon Aug 25 15:22:18 2014 -0700
Committer: James Peach <jp...@apache.org>
Committed: Mon Sep 1 15:30:32 2014 -0700
----------------------------------------------------------------------
CHANGES | 2 ++
iocore/net/OCSPStapling.cc | 10 +++++-----
iocore/net/P_SSLUtils.h | 19 +++++++++++++++++++
iocore/net/SSLUtils.cc | 34 ++++------------------------------
4 files changed, 30 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ea4fec41/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index fc1e355..b73ca34 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 5.1.0
+ *) [TS-3039] Plug SSL memory leaks in OCSP stapling support.
+
*) [TS-3022] Fix double free in OCSP.
*) [TS-2993] Update ats_pagespeed towards PSOL 1.8.31.4-stable.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ea4fec41/iocore/net/OCSPStapling.cc
----------------------------------------------------------------------
diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc
index 88c6b71..136c623 100644
--- a/iocore/net/OCSPStapling.cc
+++ b/iocore/net/OCSPStapling.cc
@@ -23,6 +23,7 @@
#include "P_OCSPStapling.h"
#include "P_Net.h"
#include "P_SSLConfig.h"
+#include "P_SSLUtils.h"
#ifdef HAVE_OPENSSL_OCSP_STAPLING
@@ -105,12 +106,12 @@ bool
ssl_stapling_init_cert(SSL_CTX *ctx, const char *certfile)
{
certinfo *cinf;
- X509 *cert = NULL;
- X509 *issuer = NULL;
+ scoped_X509 cert;
+ scoped_X509 issuer;
STACK_OF(OPENSSL_STRING) *aia = NULL;
- BIO *bio = BIO_new_file(certfile, "r");
+ scoped_BIO bio(BIO_new_file(certfile, "r"));
- cert = PEM_read_bio_X509_AUX(bio, NULL, NULL, NULL);
+ cert = PEM_read_bio_X509_AUX(bio.get(), NULL, NULL, NULL);
if (!cert) {
Debug("ssl", "can not read cert from certfile %s!", certfile);
return false;
@@ -145,7 +146,6 @@ ssl_stapling_init_cert(SSL_CTX *ctx, const char *certfile)
}
cinf->cid = OCSP_cert_to_id(NULL, cert, issuer);
- X509_free(issuer);
if (!cinf->cid)
return false;
X509_digest(cert, EVP_sha1(), cinf->idx, NULL);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ea4fec41/iocore/net/P_SSLUtils.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h
index 6e44be3..82c41b4 100644
--- a/iocore/net/P_SSLUtils.h
+++ b/iocore/net/P_SSLUtils.h
@@ -131,4 +131,23 @@ void SSLDebugBufferPrint(const char * tag, const char * buffer, unsigned buflen,
// Load the SSL certificate configuration.
bool SSLParseCertificateConfiguration(const SSLConfigParams * params, SSLCertLookup * lookup);
+namespace ssl { namespace detail {
+ struct SCOPED_X509_TRAITS {
+ typedef X509* value_type;
+ static value_type initValue() { return NULL; }
+ static bool isValid(value_type x) { return x != NULL; }
+ static void destroy(value_type x) { X509_free(x); }
+ };
+
+ struct SCOPED_BIO_TRAITS {
+ typedef BIO* value_type;
+ static value_type initValue() { return NULL; }
+ static bool isValid(value_type x) { return x != NULL; }
+ static void destroy(value_type x) { BIO_free(x); }
+ };
+/* namespace ssl */ } /* namespace detail */ }
+
+typedef ats_scoped_resource<ssl::detail::SCOPED_X509_TRAITS> scoped_X509;
+typedef ats_scoped_resource<ssl::detail::SCOPED_BIO_TRAITS> scoped_BIO;
+
#endif /* __P_SSLUTILS_H__ */
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ea4fec41/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 686eac4..9ebdf2e 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -120,28 +120,6 @@ static bool open_ssl_initialized = false;
RecRawStatBlock *ssl_rsb = NULL;
static InkHashTable *ssl_cipher_name_table = NULL;
-struct ats_file_bio
-{
- ats_file_bio(const char * path, const char * mode)
- : bio(BIO_new_file(path, mode)) {
- }
-
- ~ats_file_bio() {
- (void)BIO_set_close(bio, BIO_CLOSE);
- BIO_free(bio);
- }
-
- operator bool() const {
- return bio != NULL;
- }
-
- BIO * bio;
-
-private:
- ats_file_bio(const ats_file_bio&);
- ats_file_bio& operator=(const ats_file_bio&);
-};
-
/* Using pthread thread ID and mutex functions directly, instead of
* ATS this_ethread / ProxyMutex, so that other linked libraries
* may use pthreads and openssl without confusing us here. (TS-2271).
@@ -172,14 +150,10 @@ static bool
SSL_CTX_add_extra_chain_cert_file(SSL_CTX * ctx, const char * chainfile)
{
X509 *cert;
- ats_file_bio bio(chainfile, "r");
-
- if (!bio) {
- return false;
- }
+ scoped_BIO bio(BIO_new_file(chainfile, "r"));
for (;;) {
- cert = PEM_read_bio_X509_AUX(bio.bio, NULL, NULL, NULL);
+ cert = PEM_read_bio_X509_AUX(bio.get(), NULL, NULL, NULL);
if (!cert) {
// No more the certificates in this file.
@@ -1254,9 +1228,9 @@ ssl_index_certificate(SSLCertLookup * lookup, SSL_CTX * ctx, const char * certfi
{
X509_NAME * subject = NULL;
X509 * cert;
- ats_file_bio bio(certfile, "r");
+ scoped_BIO bio(BIO_new_file(certfile, "r"));
- cert = PEM_read_bio_X509_AUX(bio.bio, NULL, NULL, NULL);
+ cert = PEM_read_bio_X509_AUX(bio.get(), NULL, NULL, NULL);
if (NULL == cert) {
return;
}