SSL: Fix memleak related to local certificate

Task-number: QTBUG-6504
Task-number: QTBUG-8924
Task-number: QTBUG-5645
Reviewed-by: andreas
Reviewed-by: Peter Hartmann

5 files changed, 36 insertions, 3 deletions

diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp
index ce2aee1..4010710 100644
--- a/src/network/ssl/qsslsocket_openssl.cpp
+++ b/src/network/ssl/qsslsocket_openssl.cpp
@@ -153,6 +153,7 @@ static unsigned long id_function()
 QSslSocketBackendPrivate::QSslSocketBackendPrivate()
     : ssl(0),
       ctx(0),
+      pkey(0),
       readBio(0),
       writeBio(0),
       session(0)
@@ -311,11 +312,14 @@ init_context:
         }
 
         // Load private key
-        EVP_PKEY *pkey = q_EVP_PKEY_new();
+        pkey = q_EVP_PKEY_new();
+        // before we were using EVP_PKEY_assign_R* functions and did not use EVP_PKEY_free.
+        // this lead to a memory leak. Now we use the *_set1_* functions which do not
+        // take ownership of the RSA/DSA key instance because the QSslKey already has ownership.
         if (configuration.privateKey.algorithm() == QSsl::Rsa)
-            q_EVP_PKEY_assign_RSA(pkey, (RSA *)configuration.privateKey.handle());
+            q_EVP_PKEY_set1_RSA(pkey, (RSA *)configuration.privateKey.handle());
         else
-            q_EVP_PKEY_assign_DSA(pkey, (DSA *)configuration.privateKey.handle());
+            q_EVP_PKEY_set1_DSA(pkey, (DSA *)configuration.privateKey.handle());
         if (!q_SSL_CTX_use_PrivateKey(ctx, pkey)) {
             q->setErrorString(QSslSocket::tr("Error loading private key, %1").arg(SSL_ERRORSTR()));
             emit q->error(QAbstractSocket::UnknownSocketError);
@@ -922,6 +926,11 @@ void QSslSocketBackendPrivate::disconnected()
         q_SSL_CTX_free(ctx);
         ctx = 0;
     }
+    if (pkey) {
+        q_EVP_PKEY_free(pkey);
+        pkey = 0;
+    }
+
 }
 
 QSslCipher QSslSocketBackendPrivate::sessionCipher() const
diff --git a/src/network/ssl/qsslsocket_openssl_p.h b/src/network/ssl/qsslsocket_openssl_p.h
index 836f064..3c08757 100644
--- a/src/network/ssl/qsslsocket_openssl_p.h
+++ b/src/network/ssl/qsslsocket_openssl_p.h
@@ -97,6 +97,7 @@ public:
     bool initSslContext();
     SSL *ssl;
     SSL_CTX *ctx;
+    EVP_PKEY *pkey;
     BIO *readBio;
     BIO *writeBio;
     SSL_SESSION *session;
diff --git a/src/network/ssl/qsslsocket_openssl_symbols.cpp b/src/network/ssl/qsslsocket_openssl_symbols.cpp
index 94cc9d2..d2eb6f1 100644
--- a/src/network/ssl/qsslsocket_openssl_symbols.cpp
+++ b/src/network/ssl/qsslsocket_openssl_symbols.cpp
@@ -119,6 +119,8 @@ DEFINEFUNC2(char *, ERR_error_string, unsigned long a, a, char *b, b, return 0,
 DEFINEFUNC(unsigned long, ERR_get_error, DUMMYARG, DUMMYARG, return 0, return)
 DEFINEFUNC(const EVP_CIPHER *, EVP_des_ede3_cbc, DUMMYARG, DUMMYARG, return 0, return)
 DEFINEFUNC3(int, EVP_PKEY_assign, EVP_PKEY *a, a, int b, b, char *c, c, return -1, return)
+DEFINEFUNC2(int, EVP_PKEY_set1_RSA, EVP_PKEY *a, a, RSA *b, b, return -1, return)
+DEFINEFUNC2(int, EVP_PKEY_set1_DSA, EVP_PKEY *a, a, DSA *b, b, return -1, return)
 DEFINEFUNC(void, EVP_PKEY_free, EVP_PKEY *a, a, return, DUMMYARG)
 DEFINEFUNC(DSA *, EVP_PKEY_get1_DSA, EVP_PKEY *a, a, return 0, return)
 DEFINEFUNC(RSA *, EVP_PKEY_get1_RSA, EVP_PKEY *a, a, return 0, return)
@@ -510,6 +512,8 @@ bool q_resolveOpenSslSymbols()
     RESOLVEFUNC(ERR_get_error, 749, libs.second )
     RESOLVEFUNC(EVP_des_ede3_cbc, 919, libs.second )
     RESOLVEFUNC(EVP_PKEY_assign, 859, libs.second )
+    RESOLVEFUNC(EVP_PKEY_set1_RSA, 880, libs.second )
+    RESOLVEFUNC(EVP_PKEY_set1_DSA, 879, libs.second )
     RESOLVEFUNC(EVP_PKEY_free, 867, libs.second )
     RESOLVEFUNC(EVP_PKEY_get1_DSA, 869, libs.second )
     RESOLVEFUNC(EVP_PKEY_get1_RSA, 870, libs.second )
@@ -632,6 +636,8 @@ bool q_resolveOpenSslSymbols()
     RESOLVEFUNC(ERR_get_error)
     RESOLVEFUNC(EVP_des_ede3_cbc)
     RESOLVEFUNC(EVP_PKEY_assign)
+    RESOLVEFUNC(EVP_PKEY_set1_RSA)
+    RESOLVEFUNC(EVP_PKEY_set1_DSA)
     RESOLVEFUNC(EVP_PKEY_free)
     RESOLVEFUNC(EVP_PKEY_get1_DSA)
     RESOLVEFUNC(EVP_PKEY_get1_RSA)
diff --git a/src/network/ssl/qsslsocket_openssl_symbols_p.h b/src/network/ssl/qsslsocket_openssl_symbols_p.h
index c93d547..ef61dbf 100644
--- a/src/network/ssl/qsslsocket_openssl_symbols_p.h
+++ b/src/network/ssl/qsslsocket_openssl_symbols_p.h
@@ -227,6 +227,8 @@ char *q_ERR_error_string(unsigned long a, char *b);
 unsigned long q_ERR_get_error();
 const EVP_CIPHER *q_EVP_des_ede3_cbc();
 int q_EVP_PKEY_assign(EVP_PKEY *a, int b, char *c);
+int q_EVP_PKEY_set1_RSA(EVP_PKEY *a, RSA *b);
+int q_EVP_PKEY_set1_DSA(EVP_PKEY *a, DSA *b);
 void q_EVP_PKEY_free(EVP_PKEY *a);
 RSA *q_EVP_PKEY_get1_RSA(EVP_PKEY *a);
 DSA *q_EVP_PKEY_get1_DSA(EVP_PKEY *a);
diff --git a/tests/auto/qsslsocket/tst_qsslsocket.cpp b/tests/auto/qsslsocket/tst_qsslsocket.cpp
index ad2b50d..4397ab1 100644
--- a/tests/auto/qsslsocket/tst_qsslsocket.cpp
+++ b/tests/auto/qsslsocket/tst_qsslsocket.cpp
@@ -666,6 +666,21 @@ void tst_QSslSocket::isEncrypted()
 
 void tst_QSslSocket::localCertificate()
 {
+    if (!QSslSocket::supportsSsl())
+        return;
+
+    // This test does not make 100% sense yet. We just set some local CA/cert/key and use it
+    // to authenticate ourselves against the server. The server does not actually check this
+    // values. This test should just run the codepath inside qsslsocket_openssl.cpp
+
+    QSslSocketPtr socket = newSocket();
+    QList<QSslCertificate> localCert = QSslCertificate::fromPath(SRCDIR "certs/qt-test-server-cacert.pem");
+    socket->setCaCertificates(localCert);
+    socket->setLocalCertificate(QLatin1String(SRCDIR "certs/fluke.cert"));
+    socket->setPrivateKey(QLatin1String(SRCDIR "certs/fluke.key"));
+
+    socket->connectToHostEncrypted(QtNetworkSettings::serverName(), 443);
+    QVERIFY(socket->waitForEncrypted(5000));
 }
 
 void tst_QSslSocket::mode()