From 38463c905e5a86cd86fa289a9389ad6650abb988 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 19 Apr 2010 21:50:03 +0000 Subject: [PATCH] ported branch commit 2732: fixed deadlock in passwd callback git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@2734 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- libretroshare/src/pqi/authssl.cc | 107 +++++++++++--------- libretroshare/src/rsiface/rsiface.h | 2 +- libretroshare/src/rsserver/rsinit.cc | 2 +- retroshare-gui/src/gui/MainWindow.cpp | 3 + retroshare-gui/src/gui/MessengerWindow.cpp | 3 + retroshare-gui/src/gui/NetworkDialog.cpp | 6 ++ retroshare-gui/src/gui/RsAutoUpdatePage.cpp | 10 +- retroshare-gui/src/gui/RsAutoUpdatePage.h | 7 ++ retroshare-gui/src/gui/StartDialog.cpp | 2 +- retroshare-gui/src/gui/bwgraph/bwgraph.cpp | 4 + retroshare-gui/src/gui/notifyqt.cpp | 15 ++- retroshare-gui/src/gui/notifyqt.h | 2 +- 12 files changed, 102 insertions(+), 61 deletions(-) diff --git a/libretroshare/src/pqi/authssl.cc b/libretroshare/src/pqi/authssl.cc index 3f8cf3221..cf757dbd2 100644 --- a/libretroshare/src/pqi/authssl.cc +++ b/libretroshare/src/pqi/authssl.cc @@ -1894,60 +1894,67 @@ int pem_passwd_cb(char *buf, int size, int rwflag, void *password) return(strlen(buf)); } -bool AuthSSL::LocalStoreCert(X509* x509) { - //store the certificate in the local cert list - std::string peerId; - if(!getX509id(x509, peerId)) - { - #ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSL::LocalStoreCert() Cannot retrieve peer id from certificate." << std::endl; - #endif - return false; - } - if (peerId != mOwnId) { - if (mCerts[peerId]) { - #ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSL::LocalStoreCert() get duplicate for " << mCerts[peerId]->id << std::endl; - #endif - /* have a duplicate */ - /* check that they are exact */ - if (0 != X509_cmp(mCerts[peerId]->certificate, x509)) - { - /* MAJOR ERROR */ - std::cerr << "ERROR : AuthSSL::ValidateCertificate() got two different ssl certificate from the same peer. It could be a security intrusion attempt (man in the middle)."; - std::cerr << std::endl; - return false; - } - } else { - RsStackMutex stack(sslMtx); /******* LOCKED ******/ +bool AuthSSL::LocalStoreCert(X509* x509) +{ + //store the certificate in the local cert list + std::string peerId; + if(!getX509id(x509, peerId)) + { +#ifdef AUTHSSL_DEBUG + std::cerr << "AuthSSL::LocalStoreCert() Cannot retrieve peer id from certificate." << std::endl; +#endif + return false; + } + if (peerId != mOwnId) + { + if (mCerts[peerId]) + { +#ifdef AUTHSSL_DEBUG + std::cerr << "AuthSSL::LocalStoreCert() get duplicate for " << mCerts[peerId]->id << std::endl; +#endif + /* have a duplicate */ + /* check that they are exact */ + if (0 != X509_cmp(mCerts[peerId]->certificate, x509)) + { + /* MAJOR ERROR */ + std::cerr << "ERROR : AuthSSL::ValidateCertificate() got two different ssl certificate from the same peer. It could be a security intrusion attempt (man in the middle)."; + std::cerr << std::endl; + return false; + } + } + else + { + RsStackMutex stack(sslMtx); /******* LOCKED ******/ - #ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSL::LocalStoreCert() storing certificate for " << peerId << std::endl; - #endif - //have a deep copy of the x509 cert - BIO *bp = BIO_new(BIO_s_mem()); - PEM_write_bio_X509(bp, x509); - X509 *certCopy = PEM_read_bio_X509(bp, NULL, 0, NULL);certCopy->cert_info->key->pkey; +#ifdef AUTHSSL_DEBUG + std::cerr << "AuthSSL::LocalStoreCert() storing certificate for " << peerId << std::endl; +#endif + //have a deep copy of the x509 cert + BIO *bp = BIO_new(BIO_s_mem()); + PEM_write_bio_X509(bp, x509); + X509 *certCopy = PEM_read_bio_X509(bp, NULL, 0, NULL); - mCerts[peerId] = new sslcert(certCopy, peerId); - /* cert->cert_info->key->pkey is NULL until we call SSL_CTX_use_certificate(), - * so we do it here then... */ - SSL_CTX *newSslctx = SSL_CTX_new(TLSv1_method()); - SSL_CTX_set_cipher_list(newSslctx, "DEFAULT"); - SSL_CTX_use_certificate(newSslctx, mCerts[peerId]->certificate); + mCerts[peerId] = new sslcert(certCopy, peerId); + /* cert->cert_info->key->pkey is NULL until we call SSL_CTX_use_certificate(), + * so we do it here then... */ + SSL_CTX *newSslctx = SSL_CTX_new(TLSv1_method()); + SSL_CTX_set_cipher_list(newSslctx, "DEFAULT"); + SSL_CTX_use_certificate(newSslctx, mCerts[peerId]->certificate); - #ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSL::LocalStoreCert() storing certificate with public key : " << mCerts[peerId]->certificate->cert_info->key->pkey << std::endl; - #endif +#ifdef AUTHSSL_DEBUG + std::cerr << "AuthSSL::LocalStoreCert() storing certificate with public key : " << mCerts[peerId]->certificate->cert_info->key->pkey << std::endl; +#endif - IndicateConfigChanged(); - } - } else { - #ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSL::LocalStoreCert() not storing certificate because it's our own " << peerId << std::endl; - #endif - } - } + IndicateConfigChanged(); + } + } else { +#ifdef AUTHSSL_DEBUG + std::cerr << "AuthSSL::LocalStoreCert() not storing certificate because it's our own " << peerId << std::endl; +#endif + } + + return true ; +} int AuthSSL::VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx) { diff --git a/libretroshare/src/rsiface/rsiface.h b/libretroshare/src/rsiface/rsiface.h index 504d573c0..052288876 100644 --- a/libretroshare/src/rsiface/rsiface.h +++ b/libretroshare/src/rsiface/rsiface.h @@ -208,7 +208,7 @@ class NotifyBase virtual void notifyOwnAvatarChanged() {} virtual void notifyOwnStatusMessageChanged() {} - virtual std::string askForPassword(const std::string& key_details) { return "" ;} + virtual std::string askForPassword(const std::string& key_details,bool prev_is_bad) { return "" ;} }; const int NOTIFY_LIST_NEIGHBOURS = 1; diff --git a/libretroshare/src/rsserver/rsinit.cc b/libretroshare/src/rsserver/rsinit.cc index b08f70ec1..7dfd4ef26 100644 --- a/libretroshare/src/rsserver/rsinit.cc +++ b/libretroshare/src/rsserver/rsinit.cc @@ -1234,7 +1234,7 @@ int RsInit::LoadCertificates(bool autoLoginNT) RsInitConfig::passwd = ""; create_configinit(RsInitConfig::basedir, RsInitConfig::preferedId); //don't autorise the password callback again because it will lead to deadlock due to QT reentrance - AuthGPG::getAuthGPG()->setAutorisePasswordCallbackNotify(false); +// AuthGPG::getAuthGPG()->setAutorisePasswordCallbackNotify(false); return 1; } diff --git a/retroshare-gui/src/gui/MainWindow.cpp b/retroshare-gui/src/gui/MainWindow.cpp index 4fd71b9dd..2d2e0d49e 100644 --- a/retroshare-gui/src/gui/MainWindow.cpp +++ b/retroshare-gui/src/gui/MainWindow.cpp @@ -329,6 +329,9 @@ void MainWindow::displaySystrayMsg(const QString& title,const QString& msg) void MainWindow::updateStatus() { + // This call is essential to remove locks due to QEventLoop re-entrance while asking gpg passwds. Dont' remove it! + if(RsAutoUpdatePage::eventsLocked()) + return ; if (ratesstatus) ratesstatus->getRatesStatus(); diff --git a/retroshare-gui/src/gui/MessengerWindow.cpp b/retroshare-gui/src/gui/MessengerWindow.cpp index c40936aab..b7e8856cd 100644 --- a/retroshare-gui/src/gui/MessengerWindow.cpp +++ b/retroshare-gui/src/gui/MessengerWindow.cpp @@ -969,6 +969,9 @@ void MessengerWindow::loadOwnStatus() /** Save own status Online,Away,Busy **/ void MessengerWindow::savestatus() { + if(RsAutoUpdatePage::eventsLocked()) + return ; + //rsiface->lockData(); /* Lock Interface */ RsPeerDetails detail; diff --git a/retroshare-gui/src/gui/NetworkDialog.cpp b/retroshare-gui/src/gui/NetworkDialog.cpp index 0efea789f..7d6fa1712 100644 --- a/retroshare-gui/src/gui/NetworkDialog.cpp +++ b/retroshare-gui/src/gui/NetworkDialog.cpp @@ -710,6 +710,9 @@ void NetworkDialog::displayInfoLogMenu(const QPoint& pos) { void NetworkDialog::getNetworkStatus() { + if(RsAutoUpdatePage::eventsLocked()) + return ; + rsiface->lockData(); /* Lock Interface */ /* now the extra bit .... switch on check boxes */ @@ -766,6 +769,9 @@ void NetworkDialog::getNetworkStatus() void NetworkDialog::updateNetworkStatus() { + if(RsAutoUpdatePage::eventsLocked()) + return ; + rsiface->lockData(); /* Lock Interface */ /* now the extra bit .... switch on check boxes */ diff --git a/retroshare-gui/src/gui/RsAutoUpdatePage.cpp b/retroshare-gui/src/gui/RsAutoUpdatePage.cpp index 63413e312..9e8e15216 100644 --- a/retroshare-gui/src/gui/RsAutoUpdatePage.cpp +++ b/retroshare-gui/src/gui/RsAutoUpdatePage.cpp @@ -3,6 +3,8 @@ #include "RsAutoUpdatePage.h" #include "MessengerWindow.h" +bool RsAutoUpdatePage::_locked = false ; + RsAutoUpdatePage::RsAutoUpdatePage(int ms_update_period,QWidget *parent) : MainPage(parent) { @@ -16,14 +18,15 @@ RsAutoUpdatePage::RsAutoUpdatePage(int ms_update_period,QWidget *parent) void RsAutoUpdatePage::showEvent(QShowEvent *event) { //std::cout << "RsAutoUpdatePage::showEvent() In show event !!" << std::endl ; - updateDisplay(); + if(!_locked) + updateDisplay(); } void RsAutoUpdatePage::timerUpdate() { // only update when the widget is visible. // - if(!isVisible()) + if(_locked || !isVisible()) return ; updateDisplay(); @@ -31,3 +34,6 @@ void RsAutoUpdatePage::timerUpdate() update() ; // Qt flush } +void RsAutoUpdatePage::lockAllEvents() { _locked = true ; } +void RsAutoUpdatePage::unlockAllEvents() { _locked = false ; } +bool RsAutoUpdatePage::eventsLocked() { return _locked ; } diff --git a/retroshare-gui/src/gui/RsAutoUpdatePage.h b/retroshare-gui/src/gui/RsAutoUpdatePage.h index 18fa5c8e9..4dc951a54 100644 --- a/retroshare-gui/src/gui/RsAutoUpdatePage.h +++ b/retroshare-gui/src/gui/RsAutoUpdatePage.h @@ -21,6 +21,10 @@ class RsAutoUpdatePage: public MainPage RsAutoUpdatePage(int ms_update_period = 1000,QWidget *parent=NULL) ; virtual void updateDisplay() {} + + static void lockAllEvents() ; + static void unlockAllEvents() ; + static bool eventsLocked() ; protected: virtual void showEvent(QShowEvent *e) ; @@ -30,4 +34,7 @@ class RsAutoUpdatePage: public MainPage private: QTimer *_timer ; + + static bool _locked ; }; + diff --git a/retroshare-gui/src/gui/StartDialog.cpp b/retroshare-gui/src/gui/StartDialog.cpp index 4f68f328c..3a953a110 100644 --- a/retroshare-gui/src/gui/StartDialog.cpp +++ b/retroshare-gui/src/gui/StartDialog.cpp @@ -211,7 +211,7 @@ void StartDialog::notSecureWarning() { if(ui.autologin_checkbox->isChecked()){ QMessageBox::StandardButton sb = QMessageBox::warning ( NULL, tr("Insecure"), - tr("Auto-Login is not Secure \n It can be disabled in General Settings"), + tr("Auto Login is not so much secure:\n - Your SSL certificate will be stored unprotected. \n - Your PGP key will however not be stored.\nThis choice be reverted in settings."), QMessageBox::Ok); } diff --git a/retroshare-gui/src/gui/bwgraph/bwgraph.cpp b/retroshare-gui/src/gui/bwgraph/bwgraph.cpp index 402d32304..b7a659061 100644 --- a/retroshare-gui/src/gui/bwgraph/bwgraph.cpp +++ b/retroshare-gui/src/gui/bwgraph/bwgraph.cpp @@ -24,6 +24,7 @@ #include #include #include "bwgraph.h" +#include #include "rsiface/rsiface.h" #include @@ -119,6 +120,9 @@ BandwidthGraph::timerEvent( QTimerEvent * ) void BandwidthGraph::updategraphstatus( ) { + if(RsAutoUpdatePage::eventsLocked()) + return ; + /* set users/friends/network */ float downKb = 0; float upKb = 0; diff --git a/retroshare-gui/src/gui/notifyqt.cpp b/retroshare-gui/src/gui/notifyqt.cpp index 65f3e6351..f3a2a012d 100644 --- a/retroshare-gui/src/gui/notifyqt.cpp +++ b/retroshare-gui/src/gui/notifyqt.cpp @@ -42,13 +42,18 @@ void NotifyQt::notifyOwnAvatarChanged() emit ownAvatarChanged() ; } -std::string NotifyQt::askForPassword(const std::string& key_details) +std::string NotifyQt::askForPassword(const std::string& key_details,bool prev_is_bad) { - return QInputDialog::getText(NULL, tr("GPG key passphrase"), - tr("Please enter the password to unlock the following GPG key:\n") + QString::fromStdString(key_details), - QLineEdit::Password, - NULL, NULL).toStdString(); + RsAutoUpdatePage::lockAllEvents() ; + + std::string res = QInputDialog::getText(NULL, tr("GPG key passphrase"), + (prev_is_bad?tr("Wrong password !\n\n"):QString()) + + tr("Please enter the password to unlock the following GPG key:\n") + QString::fromStdString(key_details), QLineEdit::Password, NULL, NULL).toStdString(); + + RsAutoUpdatePage::unlockAllEvents() ; + + return res ; } void NotifyQt::notifyOwnStatusMessageChanged() diff --git a/retroshare-gui/src/gui/notifyqt.h b/retroshare-gui/src/gui/notifyqt.h index dcd0fd795..2723afb63 100644 --- a/retroshare-gui/src/gui/notifyqt.h +++ b/retroshare-gui/src/gui/notifyqt.h @@ -41,7 +41,7 @@ class NotifyQt: public QObject, public NotifyBase virtual void notifyOwnAvatarChanged() ; virtual void notifyOwnStatusMessageChanged() ; - virtual std::string askForPassword(const std::string& key_details) ; + virtual std::string askForPassword(const std::string& key_details,bool prev_is_bad) ; signals: // It's beneficial to send info to the GUI using signals, because signals are thread-safe