improved error handling

git-svn-id: http://svn.code.sf.net/p/retroshare/code/branches/v0.5-OpenPGP@5289 b45a01b8-16f6-495d-af2f-9b41ad6348cc

libretroshare/src/pgp/pgphandler.cc

@@ -527,15 +527,17 @@ bool PGPHandler::exportGPGKeyPair(const std::string& filename,const PGPIdType& e
 	return true ;
 }
 
-bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& imported_key_id)
+bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& imported_key_id,std::string& import_error)
 {
+	import_error = "" ;
+
 	// 1 - Test for file existance
 	//
 	FILE *ftest = fopen(filename.c_str(),"r") ;
 
 	if(ftest == NULL)
 	{
-		std::cerr << "Cannot open file " << filename << " for read. Please check access permissions." << std::endl;
+		import_error = "Cannot open file " + filename + " for read. Please check access permissions." ;
 		return false ;
 	}
 
@@ -547,12 +549,14 @@ bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& importe
 
 	if(ops_false == ops_keyring_read_from_file(tmp_keyring, ops_true, filename.c_str()))
 	{
-		std::cerr << "PGPHandler::readKeyRing(): cannot read key file. File corrupted?" << std::endl ;
+		import_error = "PGPHandler::readKeyRing(): cannot read key file. File corrupted?" ;
 		return false ;
 	}
 	if(tmp_keyring->nkeys != 2)
 	{
-		std::cerr << "PGPHandler::importKeyPair(): file does not contain a valid keypair." << std::endl ;
+		import_error = "PGPHandler::importKeyPair(): file does not contain a valid keypair." ;
+		if(tmp_keyring->nkeys > 2)
+			import_error += "\nMake sure that your key is a RSA key (DSA is not yet supported) and does not contain subkeys (not supported yet).";
 		return false ;
 	}
 
@@ -567,6 +571,7 @@ bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& importe
 		seckey = &tmp_keyring->keys[0] ;
 	else
 	{
+		import_error = "Unrecognised key type in key file for key #0. Giving up." ;
 		std::cerr << "Unrecognised key type " << tmp_keyring->keys[0].type << " in key file for key #0. Giving up." << std::endl;
 		return false ;
 	}
@@ -576,18 +581,24 @@ bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& importe
 		seckey = &tmp_keyring->keys[1] ;
 	else
 	{
+		import_error = "Unrecognised key type in key file for key #1. Giving up." ;
 		std::cerr << "Unrecognised key type " << tmp_keyring->keys[1].type << " in key file for key #1. Giving up." << std::endl;
 		return false ;
 	}
 
 	if(pubkey == NULL || seckey == NULL || pubkey == seckey)
 	{
-		std::cerr << "File does not contain a public and a private key. Sorry." << std::endl;
+		import_error = "File does not contain a public and a private key. Sorry." ;
 		return false ;
 	}
 	if(memcmp(pubkey->fingerprint.fingerprint,seckey->fingerprint.fingerprint,KEY_FINGERPRINT_SIZE) != 0)
 	{
-		std::cerr << "Public and private keys do nt have the same fingerprint. Sorry!" << std::endl;
+		import_error = "Public and private keys do nt have the same fingerprint. Sorry!" ;
+		return false ;
+	}
+	if(pubkey->key.pkey.version != 4)
+	{
+		import_error = "Public key is not version 4. Rejected!" ;
 		return false ;
 	}
 
@@ -602,7 +613,7 @@ bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& importe
 
 	if( (!ops_validate_key_signatures(result, const_cast<ops_keydata_t*>(pubkey), &dummy_keyring, cb_get_passphrase))  || result->valid_count != 1 || result->invalid_count > 0)
 	{
-		std::cerr << "Cannot validate self signature for the imported key. Sorry." << std::endl;
+		import_error = "Cannot validate self signature for the imported key. Sorry." ;
 		return false ;
 	}
 	ops_validate_result_free(result);
@@ -622,13 +633,13 @@ bool PGPHandler::importGPGKeyPair(const std::string& filename,PGPIdType& importe
 
 		if(!ops_write_transferable_secret_key_from_packet_data(seckey,ops_false,cinfo))
 		{
-			std::cerr << "(EE) Cannot encode secret key to disk!! Disk full? Out of disk quota?" << std::endl;
+			import_error = "(EE) Cannot encode secret key to disk!! Disk full? Out of disk quota?" ;
 			return false ;
 		}
 		ops_teardown_file_write(cinfo,fd) ;
 	}
 	else
-		std::cerr << "Private key already exists! Not importing it again." << std::endl;
+		import_error = "Private key already exists! Not importing it again." ;
 
 	if(addOrMergeKey(_pubring,_public_keyring_map,pubkey))
 		_pubring_changed = true ;

libretroshare/src/pgp/pgphandler.h

@@ -72,7 +72,7 @@ class PGPHandler
 		bool getGPGFilteredList(std::list<PGPIdType>& list,bool (*filter)(const PGPCertificateInfo&) = NULL) const ;
 		bool haveSecretKey(const PGPIdType& id) const ;
 
-		bool importGPGKeyPair(const std::string& filename,PGPIdType& imported_id) ;
+		bool importGPGKeyPair(const std::string& filename,PGPIdType& imported_id,std::string& import_error) ;
 		bool exportGPGKeyPair(const std::string& filename,const PGPIdType& exported_id) const ;
 
 		bool availableGPGCertificatesWithPrivateKeys(std::list<PGPIdType>& ids);

libretroshare/src/pqi/authgpg.cc

@@ -304,11 +304,11 @@ bool AuthGPG::exportProfile(const std::string& fname,const std::string& exported
 	return PGPHandler::exportGPGKeyPair(fname,PGPIdType(exported_id)) ;
 }
 
-bool AuthGPG::importProfile(const std::string& fname,std::string& imported_id)
+bool AuthGPG::importProfile(const std::string& fname,std::string& imported_id,std::string& import_error)
 {
 	PGPIdType id ;
 
-	if(PGPHandler::importGPGKeyPair(fname,id))
+	if(PGPHandler::importGPGKeyPair(fname,id,import_error))
 	{
 		imported_id = id.toStdString() ;
 		return true ;

libretroshare/src/pqi/authgpg.h

@@ -172,7 +172,7 @@ class AuthGPG: public p3Config, public RsThread, public PGPHandler
 		virtual bool	getGPGValidList(std::list<std::string> &ids);
 		virtual bool	getGPGAcceptedList(std::list<std::string> &ids);
 		virtual bool	getGPGSignedList(std::list<std::string> &ids);
-		virtual bool   importProfile(const std::string& filename,std::string& gpg_id) ;
+		virtual bool   importProfile(const std::string& filename,std::string& gpg_id,std::string& import_error) ;
 		virtual bool   exportProfile(const std::string& filename,const std::string& gpg_id) ;
 
 		/*********************************************************************************/

libretroshare/src/retroshare/rsinit.h

@@ -81,7 +81,7 @@ class RsInit
 		static bool	ValidateCertificate(std::string &userName) ;
 
 		static bool exportIdentity(const std::string& fname,const std::string& pgp_id) ;
-		static bool importIdentity(const std::string& fname,std::string& imported_pgp_id) ;
+		static bool importIdentity(const std::string& fname,std::string& imported_pgp_id,std::string& import_error) ;
 
 		/*!
 		 *  Generating GPGme Account

libretroshare/src/rsserver/rsinit.cc

@@ -713,9 +713,9 @@ bool RsInit::exportIdentity(const std::string& fname,const std::string& id)
 	return AuthGPG::getAuthGPG()->exportProfile(fname,id);
 }
 
-bool RsInit::importIdentity(const std::string& fname,std::string& id)
+bool RsInit::importIdentity(const std::string& fname,std::string& id,std::string& import_error)
 {
-	return AuthGPG::getAuthGPG()->importProfile(fname,id);
+	return AuthGPG::getAuthGPG()->importProfile(fname,id,import_error);
 }
 
 bool RsInit::copyGnuPGKeyrings()

openpgpsdk/src/openpgpsdk/create.c

@@ -116,42 +116,54 @@ ops_boolean_t ops_write_user_id(const unsigned char *user_id,
 \ingroup Core_MPI
 */
 static unsigned mpi_length(const BIGNUM *bn)
-    {
+{
-    return 2+(BN_num_bits(bn)+7)/8;
+	return 2+(BN_num_bits(bn)+7)/8;
-    }
+}
 
 static unsigned public_key_length(const ops_public_key_t *key)
-    {
+{
-    switch(key->algorithm)
+	switch(key->algorithm)
 	{
-    case OPS_PKA_RSA:
+		case OPS_PKA_RSA: return mpi_length(key->key.rsa.n)
-	return mpi_length(key->key.rsa.n)+mpi_length(key->key.rsa.e);
+										+mpi_length(key->key.rsa.e);
 
-    default:
+		case OPS_PKA_DSA: return mpi_length(key->key.dsa.p)
-	fprintf(stderr,"Bad algorithm type in key: %d\n",key->algorithm) ;
+										+mpi_length(key->key.dsa.q)
-	assert(!"unknown key algorithm");
+										+mpi_length(key->key.dsa.g)
+										+mpi_length(key->key.dsa.y);
+		case OPS_PKA_ELGAMAL:
+								return mpi_length(key->key.elgamal.p)
+										+mpi_length(key->key.elgamal.g)
+										+mpi_length(key->key.elgamal.y) ;
+
+		default:
+			fprintf(stderr,"Bad algorithm type in key: %d\n",key->algorithm) ;
+			assert(!"unknown key algorithm");
 	}
-    /* not reached */
+	/* not reached */
-    return 0;
+	return 0;
-    }
+}
 
 static unsigned secret_key_length(const ops_secret_key_t *key)
-    {
+{
-    int l;
+	int l;
 
-    switch(key->public_key.algorithm)
+	switch(key->public_key.algorithm)
 	{
-    case OPS_PKA_RSA:
+		case OPS_PKA_RSA: l=mpi_length(key->key.rsa.d)
-	l=mpi_length(key->key.rsa.d)+mpi_length(key->key.rsa.p)
+								 +mpi_length(key->key.rsa.p)
-	    +mpi_length(key->key.rsa.q)+mpi_length(key->key.rsa.u);
+								 +mpi_length(key->key.rsa.q)
-	break;
+								 +mpi_length(key->key.rsa.u);
 
-    default:
+		case OPS_PKA_DSA: l=mpi_length(key->key.dsa.x);
-	assert(!"unknown key algorithm");
+			break;
+
+		default:
+			assert(!"unknown key algorithm");
 	}
 
-    return l+public_key_length(&key->public_key);
+	return l+public_key_length(&key->public_key);
-    }
+}
 
 /** 
  * \ingroup Core_Create
@@ -174,45 +186,44 @@ void ops_fast_create_rsa_public_key(ops_public_key_t *key,time_t time,
  * for verification - the writer doesn't allow them, though */
 static ops_boolean_t write_public_key_body(const ops_public_key_t *key,
 					   ops_create_info_t *info)
-    {
+{
-    if(!(ops_write_scalar(key->version,1,info)
+	if(!(ops_write_scalar(key->version,1,info) && ops_write_scalar(key->creation_time,4,info)))
-	 && ops_write_scalar(key->creation_time,4,info)))
+		return ops_false;
-	return ops_false;
 
-    if(key->version != 4 && !ops_write_scalar(key->days_valid,2,info))
+	if(key->version != 4 && !ops_write_scalar(key->days_valid,2,info))
-	return ops_false;
+		return ops_false;
 
-    if(!ops_write_scalar(key->algorithm,1,info))
+	if(!ops_write_scalar(key->algorithm,1,info))
-	return ops_false;
+		return ops_false;
 
-    switch(key->algorithm)
+	switch(key->algorithm)
 	{
-    case OPS_PKA_DSA:
+		case OPS_PKA_DSA:
-	return ops_write_mpi(key->key.dsa.p,info)
+			return ops_write_mpi(key->key.dsa.p,info)
-	    && ops_write_mpi(key->key.dsa.q,info)
+				&& ops_write_mpi(key->key.dsa.q,info)
-	    && ops_write_mpi(key->key.dsa.g,info)
+				&& ops_write_mpi(key->key.dsa.g,info)
-	    && ops_write_mpi(key->key.dsa.y,info);
+				&& ops_write_mpi(key->key.dsa.y,info);
 
-    case OPS_PKA_RSA:
+		case OPS_PKA_RSA:
-    case OPS_PKA_RSA_ENCRYPT_ONLY:
+		case OPS_PKA_RSA_ENCRYPT_ONLY:
-    case OPS_PKA_RSA_SIGN_ONLY:
+		case OPS_PKA_RSA_SIGN_ONLY:
-	return ops_write_mpi(key->key.rsa.n,info)
+			return ops_write_mpi(key->key.rsa.n,info)
-	    && ops_write_mpi(key->key.rsa.e,info);
+				&& ops_write_mpi(key->key.rsa.e,info);
 
-    case OPS_PKA_ELGAMAL:
+		case OPS_PKA_ELGAMAL:
-	return ops_write_mpi(key->key.elgamal.p,info)
+			return ops_write_mpi(key->key.elgamal.p,info)
-	    && ops_write_mpi(key->key.elgamal.g,info)
+				&& ops_write_mpi(key->key.elgamal.g,info)
-	    && ops_write_mpi(key->key.elgamal.y,info);
+				&& ops_write_mpi(key->key.elgamal.y,info);
 
-    default:
+		default:
-	fprintf(stderr, "Unknown algorithm %d\n", key->algorithm);
+			fprintf(stderr, "Unknown algorithm %d\n", key->algorithm);
-	assert(0);
+			assert(0);
-	break;
+			break;
 	}
 
-    /* not reached */
+	/* not reached */
-    return ops_false;
+	return ops_false;
-    }
+}
 
 /* Note that we support v3 keys here because they're needed for
  * for verification - the writer doesn't allow them, though */
@@ -372,8 +383,8 @@ static ops_boolean_t write_secret_key_body(const ops_secret_key_t *key,
 
 	switch(key->public_key.algorithm)
 	{
-		//    case OPS_PKA_DSA:
+		case OPS_PKA_DSA:
-		//	return ops_write_mpi(key->key.dsa.x,info);
+			return ops_write_mpi(key->key.dsa.x,info);
 
 		case OPS_PKA_RSA:
 		case OPS_PKA_RSA_ENCRYPT_ONLY:
@@ -632,19 +643,19 @@ ops_boolean_t ops_write_transferable_secret_key(const ops_keydata_t *keydata, co
  */
 ops_boolean_t ops_write_struct_public_key(const ops_public_key_t *key,
 					  ops_create_info_t *info)
-    {
+{
-    assert(key->version == 4);
+	assert(key->version == 4);
 
-    if(key->algorithm != OPS_PKA_RSA)
+	if(key->algorithm != OPS_PKA_RSA)
-	 {
+	{
-		 fprintf(stderr,"\nUnhandled key algorithm %d\n",key->algorithm);
+		fprintf(stderr,"\nUnhandled key algorithm %d\n",key->algorithm);
-		 return ops_false ;
+		return ops_false ;
-	 }
+	}
 
-    return ops_write_ptag(OPS_PTAG_CT_PUBLIC_KEY,info)
+	return ops_write_ptag(OPS_PTAG_CT_PUBLIC_KEY,info)
-	&& ops_write_length(1+4+1+public_key_length(key),info)
+ 		 && ops_write_length(1+4+1+public_key_length(key),info)
-	&& write_public_key_body(key,info);
+		 && write_public_key_body(key,info);
-    }
+}
 
 /**
  * \ingroup Core_WritePackets

openpgpsdk/src/openpgpsdk/keyring.c

@@ -426,14 +426,14 @@ const unsigned char* ops_get_user_id(const ops_keydata_t *key, unsigned index)
 */
 
 ops_boolean_t ops_is_key_supported(const ops_keydata_t *keydata)
-    {
+{
-    if(keydata->type == OPS_PTAG_CT_PUBLIC_KEY)
+	if(keydata->type == OPS_PTAG_CT_PUBLIC_KEY)
 	{
-        if(keydata->key.pkey.algorithm == OPS_PKA_RSA)
+		if(keydata->key.pkey.algorithm == OPS_PKA_RSA)
-            return ops_true;
+			return ops_true;
-        }
+	}
-    return ops_false;
+	return ops_false;
-    }
+}
 
 
 /** 
@@ -1046,17 +1046,17 @@ ops_boolean_t ops_write_keyring_to_file(const ops_keyring_t *keyring,ops_boolean
 
 	int i;
 	for(i=0;i<keyring->nkeys;++i)
-		if(keyring->keys[i].key.pkey.algorithm  == OPS_PKA_RSA)
+//		if(keyring->keys[i].key.pkey.algorithm == OPS_PKA_RSA)
 			if(write_all_packets)
 				ops_write_transferable_public_key_from_packet_data(&keyring->keys[i],armoured,info) ;
 			else
 				ops_write_transferable_public_key(&keyring->keys[i],armoured,info) ;
-		else
+//		else
-		{
+//		{
-			fprintf(stdout, "ops_write_keyring: not writing key. Algorithm not handled: ") ;
+//			fprintf(stdout, "ops_write_keyring: not writing key. Algorithm not handled: ") ;
-			ops_print_public_keydata(&keyring->keys[i]);
+//			ops_print_public_keydata(&keyring->keys[i]);
-			fprintf(stdout, "\n") ;
+//			fprintf(stdout, "\n") ;
-		}
+//		}
 
 	ops_writer_close(info);
 	ops_create_info_delete(info);

retroshare-gui/src/gui/GenCertDialog.cpp

@@ -160,10 +160,11 @@ void GenCertDialog::importIdentity()
 		return ;
 
 	std::string gpg_id ;
+	std::string err_string ;
 
-	if(!RsInit::importIdentity(fname.toStdString(),gpg_id))
+	if(!RsInit::importIdentity(fname.toStdString(),gpg_id,err_string))
 	{
-		QMessageBox::information(this,tr("Identity not loaded"),tr("Your identity was not loaded properly. \nCheck that it is a valid GPG key pair.")) ;
+		QMessageBox::information(this,tr("Identity not loaded"),tr("Your identity was not loaded properly:")+" \n    "+QString::fromStdString(err_string)) ;
 		return ;
 	}
 

retroshare-gui/src/gui/GenCertDialog.ui

@@ -689,7 +689,7 @@ border: 1px solid #CCCCCC;}</string>
            <item row="0" column="1">
             <widget class="QComboBox" name="genPGPuser">
              <property name="toolTip">
-              <string>Your profile is associated to a GPG key</string>
+              <string>Your profile is associated to a GPG key. RetroShare currently ignores DSA keys.</string>
              </property>
             </widget>
            </item>

retroshare-gui/src/main.cpp

@@ -86,7 +86,7 @@ int main(int argc, char *argv[])
 
 		QMessageBox msgBox;
 		msgBox.setText(QObject::tr("This version of RetroShare is using OpenPGP-SDK. As a side effect, it's not using the system shared PGP keyring, but has it's own keyring shared by all RetroShare instances. <br><br>You do not appear to have such a keyring, although GPG keys are mentionned by existing RetroShare accounts, probably because you just changed to this new version of the software."));
-		msgBox.setInformativeText(QObject::tr("Choose between:<br><ul><li>Ok to copy the existing keyring from gnupg (safest bet), or </li><li>Discard to start fresh with an empty keyring (you will be asked to create a new PGP key to work with RetroShare). </li><li>Cancel to quit and forge a keyring by yourself (needs some PGP skills)</li></ul>"));
+		msgBox.setInformativeText(QObject::tr("Choose between:<br><ul><li>Ok to copy the existing keyring from gnupg (safest bet), or </li><li>Close without saving to start fresh with an empty keyring (you will be asked to create a new PGP key to work with RetroShare). </li><li>Cancel to quit and forge a keyring by yourself (needs some PGP skills)</li></ul>"));
 		msgBox.setStandardButtons(QMessageBox::Ok | QMessageBox::Discard | QMessageBox::Cancel);
 		msgBox.setDefaultButton(QMessageBox::Ok);
 		msgBox.setWindowIcon(QIcon(":/images/rstray3.png"));