From 657a1c6a96d78f18b92b72aba3e8d1621114d39b Mon Sep 17 00:00:00 2001 From: Juha Turunen Date: Thu, 9 Sep 2010 09:59:49 +0200 Subject: QSslSocketPrivate::systemCaCertificates() hangs sometimes on Symbian The patch fixes the hanging issues on some Symbian devices that occurs while retrieving certificates from the Symbian certificate store. The hanging was caused by the certificate info array not being closed before exiting the thread. This alone wouldn't make the existing implementation work, so the patch replaces it with a pure Symbian style implementation which doesn't seem to be affected (probably some OpenC threads issue). Merge-request: 808 Reviewed-by: Shane Kearns Reviewed-by: Simon Hausmann Task: QTBUG-13033 (cherry picked from commit 5342be5ceffc84b56476fff57dd7d5e1bbfeb471) --- src/network/ssl/qsslsocket_openssl.cpp | 193 ++++++++++++++++++--------------- src/network/ssl/qsslsocket_openssl_p.h | 77 ++++++------- 2 files changed, 136 insertions(+), 134 deletions(-) diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp index 3174501..5182f11 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -71,6 +71,9 @@ QT_BEGIN_NAMESPACE PtrCertFindCertificateInStore QSslSocketPrivate::ptrCertFindCertificateInStore = 0; PtrCertCloseStore QSslSocketPrivate::ptrCertCloseStore = 0; #elif defined(Q_OS_SYMBIAN) +#include +#include +#include #include #endif @@ -569,120 +572,138 @@ void QSslSocketPrivate::resetDefaultCiphers() #if defined(Q_OS_SYMBIAN) -QCertificateRetriever::QCertificateRetriever(QCertificateConsumer* parent) - : CActive(EPriorityStandard) - , certStore(0) - , certFilter(0) - , consumer(parent) - , currentCertificateIndex(0) - , certDescriptor(0, 0) +CSymbianCertificateRetriever::CSymbianCertificateRetriever() : CActive(CActive::EPriorityStandard), + iSequenceError(KErrNone) { - CActiveScheduler::Add(this); - QT_TRAP_THROWING(certStore = CUnifiedCertStore::NewL(qt_s60GetRFs(), EFalse)); - QT_TRAP_THROWING(certFilter = CCertAttributeFilter::NewL()); - certFilter->SetFormat(EX509Certificate); } -QCertificateRetriever::~QCertificateRetriever() +CSymbianCertificateRetriever::~CSymbianCertificateRetriever() { - delete certFilter; - delete certStore; - Cancel(); + iThread.Close(); } -void QCertificateRetriever::fetch() +CSymbianCertificateRetriever* CSymbianCertificateRetriever::NewL() { - certStore->Initialize(iStatus); - state = Initializing; - SetActive(); + CSymbianCertificateRetriever* self = new (ELeave) CSymbianCertificateRetriever(); + CleanupStack::PushL(self); + self->ConstructL(); + CleanupStack::Pop(); + return self; } -void QCertificateRetriever::list() +int CSymbianCertificateRetriever::GetCertificates(QList &certificates) { - certStore->List(certs, *certFilter, iStatus); - state = Listing; - SetActive(); + iCertificates = &certificates; + + TRequestStatus status; + iThread.Logon(status); + iThread.Resume(); + User::WaitForRequest(status); + if (iThread.ExitType() == EExitKill) + return KErrDied; + else + return status.Int(); // Logon() completes with the thread's exit value } -void QCertificateRetriever::retrieveNextCertificate() +void CSymbianCertificateRetriever::doThreadEntryL() { - CCTCertInfo* cert = certs[currentCertificateIndex]; - currentCertificate.resize(cert->Size()); - certDescriptor.Set((TUint8*)currentCertificate.data(), 0, currentCertificate.size()); - certStore->Retrieve(*cert, certDescriptor, iStatus); - state = RetrievingCertificates; + CActiveScheduler* activeScheduler = new (ELeave) CActiveScheduler; + CleanupStack::PushL(activeScheduler); + CActiveScheduler::Install(activeScheduler); + + CActiveScheduler::Add(this); + + // These aren't deleted in the destructor so leaving the to CS is ok + iCertStore = CUnifiedCertStore::NewLC(qt_s60GetRFs(), EFalse); + iCertFilter = CCertAttributeFilter::NewLC(); + + // Kick off the sequence by initializing the cert store + iState = Initializing; + iCertStore->Initialize(iStatus); SetActive(); -} -void QCertificateRetriever::RunL() -{ - QT_TRYCATCH_LEAVING(run()); + CActiveScheduler::Start(); + + // Sequence complete, clean up + + // These MUST be cleaned up before the installed CActiveScheduler is destroyed and can't be left to the + // destructor of CSymbianCertificateRetriever. Otherwise the destructor of CActiveScheduler will get + // stuck. + iCertInfos.Close(); + CleanupStack::PopAndDestroy(3); // activeScheduler, iCertStore, iCertFilter } -void QCertificateRetriever::run() + +TInt CSymbianCertificateRetriever::ThreadEntryPoint(TAny* aParams) { - switch (state) { - case Initializing: - list(); - break; - case Listing: - currentCertificateIndex = 0; - retrieveNextCertificate(); - break; - case RetrievingCertificates: - consumer->addEncodedCertificate(currentCertificate); - currentCertificate = QByteArray(); + CTrapCleanup* cleanupStack = CTrapCleanup::New(); - currentCertificateIndex++; + CSymbianCertificateRetriever* self = (CSymbianCertificateRetriever*) aParams; + TRAPD(err, self->doThreadEntryL()); + delete cleanupStack; - if (currentCertificateIndex < certs.Count()) - retrieveNextCertificate(); - else - consumer->finish(); - break; - } + // doThreadEntryL() can leave only before the retrieval sequence is started + if (err) + return err; + else + return self->iSequenceError; // return any error that occured during the retrieval } -void QCertificateRetriever::DoCancel() +void CSymbianCertificateRetriever::ConstructL() { - switch (state) { - case Initializing: - certStore->CancelInitialize(); - break; - case Listing: - certStore->CancelList(); - break; - case RetrievingCertificates: - certStore->CancelRetrieve(); - break; - } + User::LeaveIfError(iThread.Create(_L("CertWorkerThread"), + CSymbianCertificateRetriever::ThreadEntryPoint, 16384, NULL, this)); } -QCertificateConsumer::QCertificateConsumer(QObject* parent) - : QObject(parent) - , retriever(0) +void CSymbianCertificateRetriever::DoCancel() { + // We never cancel the sequence } -QCertificateConsumer::~QCertificateConsumer() +TInt CSymbianCertificateRetriever::RunError(TInt aError) { - delete retriever; + // If something goes wrong in the sequence, abort the sequence + iSequenceError = aError; // this gets reported to the client in the TRequestStatus + CActiveScheduler::Stop(); + return KErrNone; } -void QCertificateConsumer::finish() +void CSymbianCertificateRetriever::GetCertificateL() { - delete retriever; - retriever = 0; - emit finished(); + CCTCertInfo* certInfo = iCertInfos[iCurrentCertIndex]; + iCertificateData.resize(certInfo->Size()); + TPtr8 des((TUint8*)iCertificateData.data(), 0, iCertificateData.size()); + iCertStore->Retrieve(*certInfo, des, iStatus); + iState = RetrievingCertificates; + SetActive(); } -void QCertificateConsumer::start() +void CSymbianCertificateRetriever::RunL() { - retriever = new QCertificateRetriever(this); - Q_CHECK_PTR(retriever); - retriever->fetch(); -} + switch (iState) { + case Initializing: + iState = Listing; + iCertStore->List(iCertInfos, *iCertFilter, iStatus); + SetActive(); + break; + case Listing: + iCurrentCertIndex = 0; + GetCertificateL(); + break; + + case RetrievingCertificates: + iCertificates->append(iCertificateData); + iCertificateData = QByteArray(); + iCurrentCertIndex++; + if (iCurrentCertIndex < iCertInfos.Count()) + GetCertificateL(); + else + // Stop the scheduler to return to the thread entry function + CActiveScheduler::Stop(); + break; + } +} #endif // defined(Q_OS_SYMBIAN) QList QSslSocketPrivate::systemCaCertificates() @@ -759,23 +780,15 @@ QList QSslSocketPrivate::systemCaCertificates() systemCerts.append(QSslCertificate::fromPath(QLatin1String("/usr/share/ssl/*.pem"), QSsl::Pem, QRegExp::Wildcard)); // Centos, Redhat, SuSE systemCerts.append(QSslCertificate::fromPath(QLatin1String("/usr/local/ssl/*.pem"), QSsl::Pem, QRegExp::Wildcard)); // Normal OpenSSL Tarball #elif defined(Q_OS_SYMBIAN) - QThread* certThread = new QThread; + QList certs; + QScopedPointer retriever(CSymbianCertificateRetriever::NewL()); - QCertificateConsumer *consumer = new QCertificateConsumer(); - consumer->moveToThread(certThread); - QObject::connect(certThread, SIGNAL(started()), consumer, SLOT(start())); - QObject::connect(consumer, SIGNAL(finished()), certThread, SLOT(quit()), Qt::DirectConnection); - - certThread->start(); - certThread->wait(); - foreach (const QByteArray &encodedCert, consumer->encodedCertificates()) { + retriever->GetCertificates(certs); + foreach (const QByteArray &encodedCert, certs) { QSslCertificate cert(encodedCert, QSsl::Der); if (!cert.isNull()) systemCerts.append(cert); } - - delete consumer; - delete certThread; #endif return systemCerts; diff --git a/src/network/ssl/qsslsocket_openssl_p.h b/src/network/ssl/qsslsocket_openssl_p.h index ca377d9..7716c95 100644 --- a/src/network/ssl/qsslsocket_openssl_p.h +++ b/src/network/ssl/qsslsocket_openssl_p.h @@ -120,67 +120,56 @@ public: }; #if defined(Q_OS_SYMBIAN) + +#include +#include +#include #include // link against certstore.lib #include // link against ctframework.lib -class QCertificateRetriever; +// The purpose of this class is to wrap the asynchronous API of Symbian certificate store to one +// synchronizable call. The user of this class needs to provide a TRequestStatus object which can +// be used with User::WaitForRequest() unlike with the calls of the certificate store API. +// A thread is used instead of a CActiveSchedulerWait scheme, because that would make the call +// asynchronous (other events might be processed during the call even though the call would be seemingly +// synchronous). -class QCertificateConsumer : public QObject +class CSymbianCertificateRetriever : public CActive { - Q_OBJECT public: - QCertificateConsumer(QObject* parent = 0); - ~QCertificateConsumer(); - - void finish(); - - void addEncodedCertificate(const QByteArray& certificate) - { certificates.append(certificate); } - QList encodedCertificates() const { return certificates; } - -public slots: - void start(); + static CSymbianCertificateRetriever* NewL(); + ~CSymbianCertificateRetriever(); -signals: - void finished(); + int GetCertificates(QList &aCertificates); private: - QList certificates; - QCertificateRetriever *retriever; -}; - - -class QCertificateRetriever : public CActive -{ -public: - QCertificateRetriever(QCertificateConsumer* consumer); - ~QCertificateRetriever(); - - void fetch(); + void ConstructL(); + CSymbianCertificateRetriever(); + static TInt ThreadEntryPoint(TAny* aParams); + void doThreadEntryL(); + void GetCertificateL(); + void DoCancel(); + void RunL(); + TInt RunError(TInt aError); private: - virtual void RunL(); - virtual void DoCancel(); - - void run(); - void list(); - void retrieveNextCertificate(); - enum { Initializing, Listing, RetrievingCertificates - } state; - - CUnifiedCertStore* certStore; - RMPointerArray certs; - CCertAttributeFilter* certFilter; - QCertificateConsumer* consumer; - int currentCertificateIndex; - QByteArray currentCertificate; - TPtr8 certDescriptor; + } iState; + + RThread iThread; + CUnifiedCertStore* iCertStore; + RMPointerArray iCertInfos; + CCertAttributeFilter* iCertFilter; + TInt iCurrentCertIndex; + QByteArray iCertificateData; + QList* iCertificates; + TInt iSequenceError; }; + #endif -- cgit v0.12