From 74c01423f07214d568d417a42021db9837e22cb1 Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 11 Jun 2015 20:31:52 +0000 Subject: [PATCH] improved login system: do not re-ask for passphrase when user clicks cancel. Removed warning stating that maybe passphrase is wrong git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@8415 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- TODO.txt | 2 +- libretroshare/src/pgp/pgphandler.cc | 27 +++++++++--- libretroshare/src/pgp/pgphandler.h | 2 +- libretroshare/src/pqi/authgpg.cc | 4 +- libretroshare/src/pqi/p3notify.cc | 4 +- libretroshare/src/pqi/p3notify.h | 2 +- libretroshare/src/retroshare/rsnotify.h | 2 +- openpgpsdk/src/openpgpsdk/readerwriter.c | 54 +++++++++++++++--------- openpgpsdk/src/openpgpsdk/types.h | 5 +-- retroshare-gui/src/gui/notifyqt.cpp | 14 ++++-- retroshare-gui/src/gui/notifyqt.h | 2 +- retroshare-gui/src/rshare.cpp | 9 ++-- retroshare-nogui/src/notifytxt.cc | 3 +- retroshare-nogui/src/notifytxt.h | 2 +- 14 files changed, 87 insertions(+), 45 deletions(-) diff --git a/TODO.txt b/TODO.txt index 13a2b7727..cac9fb613 100644 --- a/TODO.txt +++ b/TODO.txt @@ -26,7 +26,7 @@ GUI [ ] "Wrong IP" security items shouldn't show up when the IP reported by friend is whitelisted. - [ ] at login, when cancel is pressed, the system keeps asking for the passwd. It shouldn't, and directly go back to the list of locations. + [X] at login, when cancel is pressed, the system keeps asking for the passwd. It shouldn't, and directly go back to the list of locations. 0000 [ ] merge the various help systems. there's 3 of them: (1) help buttons on most tabs that pop a flat panel with some info; (2) help wizard accessible from the "!" button in friends details->Trust; (3) 'getting started tab' diff --git a/libretroshare/src/pgp/pgphandler.cc b/libretroshare/src/pgp/pgphandler.cc index 294be072d..415256aee 100644 --- a/libretroshare/src/pgp/pgphandler.cc +++ b/libretroshare/src/pgp/pgphandler.cc @@ -64,8 +64,13 @@ ops_parse_cb_return_t cb_get_passphrase(const ops_parser_content_t *content_,ops uid_hint = std::string((const char *)cbinfo->cryptinfo.keydata->uids[0].user_id) ; uid_hint += "(" + RsPgpId(cbinfo->cryptinfo.keydata->key_id).toStdString()+")" ; - passwd = PGPHandler::passphraseCallback()(NULL,uid_hint.c_str(),NULL,prev_was_bad) ; - *(content->secret_key_passphrase.passphrase)= (char *)ops_mallocz(passwd.length()+1) ; + bool cancelled = false ; + passwd = PGPHandler::passphraseCallback()(NULL,uid_hint.c_str(),NULL,prev_was_bad,&cancelled) ; + + if(cancelled) + *(unsigned char *)cbinfo->arg = 1; + + *(content->secret_key_passphrase.passphrase)= (char *)ops_mallocz(passwd.length()+1) ; memcpy(*(content->secret_key_passphrase.passphrase),passwd.c_str(),passwd.length()) ; return OPS_KEEP_MEMORY; } @@ -1307,7 +1312,8 @@ bool PGPHandler::SignDataBin(const RsPgpId& id,const void *data, const uint32_t PGPFingerprintType fp(f.fingerprint) ; #endif - std::string passphrase = _passphrase_callback(NULL,uid_hint.c_str(),"Please enter passwd for encrypting your key : ",false) ; + bool cancelled =false; + std::string passphrase = _passphrase_callback(NULL,uid_hint.c_str(),"Please enter passwd for encrypting your key : ",false,&cancelled) ; ops_secret_key_t *secret_key = ops_decrypt_secret_key_from_data(key,passphrase.c_str()) ; @@ -1316,6 +1322,11 @@ bool PGPHandler::SignDataBin(const RsPgpId& id,const void *data, const uint32_t std::cerr << "Key decryption went wrong. Wrong passwd?" << std::endl; return false ; } + if(cancelled) + { + std::cerr << "Key entering cancelled" << std::endl; + return false ; + } // then do the signature. @@ -1387,11 +1398,17 @@ bool PGPHandler::privateSignCertificate(const RsPgpId& ownId,const RsPgpId& id_o return false ; } - std::string passphrase = _passphrase_callback(NULL,RsPgpId(skey->key_id).toStdString().c_str(),"Please enter passwd for encrypting your key : ",false) ; + bool cancelled = false; + std::string passphrase = _passphrase_callback(NULL,RsPgpId(skey->key_id).toStdString().c_str(),"Please enter passwd for encrypting your key : ",false,&cancelled) ; ops_secret_key_t *secret_key = ops_decrypt_secret_key_from_data(skey,passphrase.c_str()) ; - if(!secret_key) + if(cancelled) + { + std::cerr << "Key cancelled by used." << std::endl; + return false ; + } + if(!secret_key) { std::cerr << "Key decryption went wrong. Wrong passwd?" << std::endl; return false ; diff --git a/libretroshare/src/pgp/pgphandler.h b/libretroshare/src/pgp/pgphandler.h index 304efc6be..d596f0bcb 100644 --- a/libretroshare/src/pgp/pgphandler.h +++ b/libretroshare/src/pgp/pgphandler.h @@ -16,7 +16,7 @@ extern "C" { #include } -typedef std::string (*PassphraseCallback)(void *data, const char *uid_hint, const char *passphrase_info, int prev_was_bad) ; +typedef std::string (*PassphraseCallback)(void *data, const char *uid_hint, const char *passphrase_info, int prev_was_bad,bool *cancelled) ; class PGPCertificateInfo { diff --git a/libretroshare/src/pqi/authgpg.cc b/libretroshare/src/pqi/authgpg.cc index b7e876796..5c686f9c9 100644 --- a/libretroshare/src/pqi/authgpg.cc +++ b/libretroshare/src/pqi/authgpg.cc @@ -86,14 +86,14 @@ bool AuthGPG::encryptTextToFile(const std::string& text,const std::string& outfi // return PGPHandler::encryptTextToString(RsPgpId(pgp_id),text,outstr) ; // } -std::string pgp_pwd_callback(void * /*hook*/, const char *uid_hint, const char * /*passphrase_info*/, int prev_was_bad) +std::string pgp_pwd_callback(void * /*hook*/, const char *uid_hint, const char * /*passphrase_info*/, int prev_was_bad,bool *cancelled) { #define GPG_DEBUG2 #ifdef GPG_DEBUG2 fprintf(stderr, "pgp_pwd_callback() called.\n"); #endif std::string password; - RsServer::notify()->askForPassword(uid_hint, prev_was_bad, password) ; + RsServer::notify()->askForPassword(uid_hint, prev_was_bad, password,cancelled) ; return password ; } diff --git a/libretroshare/src/pqi/p3notify.cc b/libretroshare/src/pqi/p3notify.cc index b8e8bee1e..fce9a80ed 100644 --- a/libretroshare/src/pqi/p3notify.cc +++ b/libretroshare/src/pqi/p3notify.cc @@ -247,10 +247,10 @@ void p3Notify::notifyDownloadComplete (const std::string& fileHash ) void p3Notify::notifyDownloadCompleteCount (uint32_t count ) { FOR_ALL_NOTIFY_CLIENTS (*it)->notifyDownloadCompleteCount (count) ; } void p3Notify::notifyHistoryChanged (uint32_t msgId , int type) { FOR_ALL_NOTIFY_CLIENTS (*it)->notifyHistoryChanged (msgId,type) ; } -bool p3Notify::askForPassword (const std::string& key_details , bool prev_is_bad , std::string& password) +bool p3Notify::askForPassword (const std::string& key_details , bool prev_is_bad , std::string& password,bool *cancelled) { FOR_ALL_NOTIFY_CLIENTS - if( (*it)->askForPassword(key_details,prev_is_bad,password)) + if( (*it)->askForPassword(key_details,prev_is_bad,password,*cancelled)) return true ; return false ; diff --git a/libretroshare/src/pqi/p3notify.h b/libretroshare/src/pqi/p3notify.h index c426eec75..c9c469dc7 100644 --- a/libretroshare/src/pqi/p3notify.h +++ b/libretroshare/src/pqi/p3notify.h @@ -123,7 +123,7 @@ class p3Notify: public RsNotify void notifyDownloadCompleteCount (uint32_t /* count */) ; void notifyHistoryChanged (uint32_t /* msgId */, int /* type */) ; - bool askForPassword (const std::string& /* key_details */, bool /* prev_is_bad */, std::string& /* password */ ) ; + bool askForPassword (const std::string& /* key_details */, bool /* prev_is_bad */, std::string&, bool *cancelled /* password */ ) ; bool askForPluginConfirmation (const std::string& /* plugin_filename */, const std::string& /* plugin_file_hash */) ; private: diff --git a/libretroshare/src/retroshare/rsnotify.h b/libretroshare/src/retroshare/rsnotify.h index ef2ad0ddd..65c16fdc1 100644 --- a/libretroshare/src/retroshare/rsnotify.h +++ b/libretroshare/src/retroshare/rsnotify.h @@ -234,7 +234,7 @@ class NotifyClient virtual void notifyDownloadCompleteCount (uint32_t /* count */) {} virtual void notifyHistoryChanged (uint32_t /* msgId */, int /* type */) {} - virtual bool askForPassword (const std::string& /* key_details */, bool /* prev_is_bad */, std::string& /* password */ ) { return false ;} + virtual bool askForPassword (const std::string& /* key_details */, bool /* prev_is_bad */, std::string& /* password */,bool& /* cancelled */ ) { return false ;} virtual bool askForPluginConfirmation (const std::string& /* plugin_filename */, const std::string& /* plugin_file_hash */) { return false ;} }; diff --git a/openpgpsdk/src/openpgpsdk/readerwriter.c b/openpgpsdk/src/openpgpsdk/readerwriter.c index a15bb1161..a80fede82 100644 --- a/openpgpsdk/src/openpgpsdk/readerwriter.c +++ b/openpgpsdk/src/openpgpsdk/readerwriter.c @@ -405,28 +405,44 @@ callback_cmd_get_secret_key(const ops_parser_content_t *content_,ops_parse_cb_in /* now get the key from the data */ secret=ops_get_secret_key_from_data(cbinfo->cryptinfo.keydata); int tag_to_use = OPS_PARSER_CMD_GET_SK_PASSPHRASE ; - int nbtries = 0 ; + int nbtries = 0 ; while( (!secret) && nbtries++ < 3) - { - if (!cbinfo->cryptinfo.passphrase) - { - memset(&pc,'\0',sizeof pc); - pc.content.secret_key_passphrase.passphrase=&cbinfo->cryptinfo.passphrase; - CB(cbinfo,tag_to_use,&pc); - if (!cbinfo->cryptinfo.passphrase) - { - fprintf(stderr,"can't get passphrase\n"); - return 0 ; // ASSERT(0); - } - } - /* then it must be encrypted */ - secret=ops_decrypt_secret_key_from_data(cbinfo->cryptinfo.keydata,cbinfo->cryptinfo.passphrase); + { + if (!cbinfo->cryptinfo.passphrase) + { + cbinfo->arg = malloc(sizeof(unsigned char)) ; + *(unsigned char *)cbinfo->arg = 0 ; - free(cbinfo->cryptinfo.passphrase) ; - cbinfo->cryptinfo.passphrase = NULL ; - tag_to_use = OPS_PARSER_CMD_GET_SK_PASSPHRASE_PREV_WAS_BAD ; - } + memset(&pc,'\0',sizeof pc); + pc.content.secret_key_passphrase.passphrase=&cbinfo->cryptinfo.passphrase; + CB(cbinfo,tag_to_use,&pc); + + if(*(unsigned char*)(cbinfo->arg) == 1) + { + fprintf(stderr,"passphrase cancelled\n"); + free(cbinfo->arg) ; + cbinfo->arg=NULL ; + return 0 ; // ASSERT(0); + } + if (!cbinfo->cryptinfo.passphrase) + { + free(cbinfo->arg) ; + cbinfo->arg=NULL ; + fprintf(stderr,"can't get passphrase\n"); + return 0 ; // ASSERT(0); + } + free(cbinfo->arg) ; + cbinfo->arg=NULL ; + } + /* then it must be encrypted */ + secret=ops_decrypt_secret_key_from_data(cbinfo->cryptinfo.keydata,cbinfo->cryptinfo.passphrase); + + free(cbinfo->cryptinfo.passphrase) ; + cbinfo->cryptinfo.passphrase = NULL ; + + tag_to_use = OPS_PARSER_CMD_GET_SK_PASSPHRASE_PREV_WAS_BAD ; + } if(!secret) return 0 ; diff --git a/openpgpsdk/src/openpgpsdk/types.h b/openpgpsdk/src/openpgpsdk/types.h index 4fea73343..50d4107a0 100644 --- a/openpgpsdk/src/openpgpsdk/types.h +++ b/openpgpsdk/src/openpgpsdk/types.h @@ -150,11 +150,10 @@ enum ops_content_tag_t OPS_PTAG_CT_ENCRYPTED_PK_SESSION_KEY=0x300+15, /* commands to the callback */ - OPS_PARSER_CMD_GET_SK_PASSPHRASE =0x400, - OPS_PARSER_CMD_GET_SECRET_KEY =0x400+1, + OPS_PARSER_CMD_GET_SK_PASSPHRASE =0x400, + OPS_PARSER_CMD_GET_SECRET_KEY =0x400+1, OPS_PARSER_CMD_GET_SK_PASSPHRASE_PREV_WAS_BAD =0x400+2, - /* Errors */ OPS_PARSER_ERROR =0x500, /*!< Internal Use: Parser Error */ OPS_PARSER_ERRCODE =0x500+1, /*! < Internal Use: Parser Error with errcode returned */ diff --git a/retroshare-gui/src/gui/notifyqt.cpp b/retroshare-gui/src/gui/notifyqt.cpp index b24dd469f..c807f7876 100644 --- a/retroshare-gui/src/gui/notifyqt.cpp +++ b/retroshare-gui/src/gui/notifyqt.cpp @@ -244,7 +244,7 @@ void NotifyQt::handleSignatureEvent() -bool NotifyQt::askForPassword(const std::string& key_details, bool prev_is_bad, std::string& password) +bool NotifyQt::askForPassword(const std::string& key_details, bool prev_is_bad, std::string& password,bool& cancelled) { RsAutoUpdatePage::lockAllEvents() ; @@ -256,12 +256,20 @@ bool NotifyQt::askForPassword(const std::string& key_details, bool prev_is_bad, int ret = dialog.exec(); + cancelled = false ; + RsAutoUpdatePage::unlockAllEvents() ; - if (ret == QDialog::Accepted) { + if (ret == QDialog::Rejected) { + password.clear() ; + cancelled = true ; + return true ; + } + + if (ret == QDialog::Accepted) { password = dialog.textValue().toUtf8().constData(); return true; - } + } return false; } diff --git a/retroshare-gui/src/gui/notifyqt.h b/retroshare-gui/src/gui/notifyqt.h index d8ed513e5..c081ef2cc 100644 --- a/retroshare-gui/src/gui/notifyqt.h +++ b/retroshare-gui/src/gui/notifyqt.h @@ -70,7 +70,7 @@ class NotifyQt: public QObject, public NotifyClient virtual void notifyDiscInfoChanged() ; virtual void notifyDownloadComplete(const std::string& fileHash); virtual void notifyDownloadCompleteCount(uint32_t count); - virtual bool askForPassword(const std::string& key_details, bool prev_is_bad, std::string& password); + virtual bool askForPassword(const std::string& key_details, bool prev_is_bad, std::string& password, bool &cancelled); virtual bool askForPluginConfirmation(const std::string& plugin_filename, const std::string& plugin_file_hash); // Queues the signature event so that it canhappen in the main GUI thread (to ask for passwd). diff --git a/retroshare-gui/src/rshare.cpp b/retroshare-gui/src/rshare.cpp index 05c14ff83..8950a49b3 100644 --- a/retroshare-gui/src/rshare.cpp +++ b/retroshare-gui/src/rshare.cpp @@ -650,10 +650,11 @@ bool Rshare::loadCertificate(const RsPeerId &accountId, bool autoLogin) QObject::tr("An unexpected error occurred when Retroshare " "tried to acquire the single instance lock\n Lock file:\n") + QString::fromUtf8(lockFile.c_str())); - return false; - case 3: QMessageBox::critical( 0, - QObject::tr("Login Failure"), - QObject::tr("Maybe password is wrong") ); + return false; + case 3: +// case 3: QMessageBox::critical( 0, +// QObject::tr("Login Failure"), +// QObject::tr("Maybe password is wrong") ); return false; default: std::cerr << "Rshare::loadCertificate() unexpected switch value " << retVal << std::endl; return false; diff --git a/retroshare-nogui/src/notifytxt.cc b/retroshare-nogui/src/notifytxt.cc index c9097a8a1..07aa53463 100644 --- a/retroshare-nogui/src/notifytxt.cc +++ b/retroshare-nogui/src/notifytxt.cc @@ -101,11 +101,12 @@ bool NotifyTxt::askForPluginConfirmation(const std::string& plugin_file_name, co return a == 'y' ; } -bool NotifyTxt::askForPassword(const std::string& question, bool prev_is_bad, std::string& password) +bool NotifyTxt::askForPassword(const std::string& question, bool prev_is_bad, std::string& password,bool& cancel) { std::string question1="Please enter your PGP password for key:\n " + question + " :"; char *passwd = getpass(question1.c_str()) ; password = passwd; + cancel = false ; return !password.empty(); } diff --git a/retroshare-nogui/src/notifytxt.h b/retroshare-nogui/src/notifytxt.h index 1f0310ad2..72a6f5771 100644 --- a/retroshare-nogui/src/notifytxt.h +++ b/retroshare-nogui/src/notifytxt.h @@ -41,7 +41,7 @@ class NotifyTxt: public NotifyClient virtual void notifyListChange(int list, int type); virtual void notifyErrorMsg(int list, int sev, std::string msg); virtual void notifyChat(); - virtual bool askForPassword(const std::string& question, bool prev_is_bad, std::string& password); + virtual bool askForPassword(const std::string& question, bool prev_is_bad, std::string& password,bool& cancel); virtual bool askForPluginConfirmation(const std::string& plugin_file, const std::string& plugin_hash); virtual void notifyTurtleSearchResult(uint32_t search_id,const std::list& found_files);