diff options
3 files changed, 66 insertions, 44 deletions
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index 8dbc2208c..3e7e9d98e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -102,11 +102,12 @@ public class PgpKeyOperation { } /** Creates new secret key. */ - private PGPKeyPair createKey(int algorithmChoice, int keySize) throws PgpGeneralMsgIdException { + private PGPKeyPair createKey(int algorithmChoice, int keySize, OperationLog log, int indent) { try { if (keySize < 512) { - throw new PgpGeneralMsgIdException(R.string.error_key_size_minimum512bit); + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_KEYSIZE_512, indent); + return null; } int algorithm; @@ -141,7 +142,8 @@ public class PgpKeyOperation { } default: { - throw new PgpGeneralMsgIdException(R.string.error_unknown_algorithm_choice); + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_UNKNOWN_ALGO, indent); + return null; } } @@ -155,7 +157,9 @@ public class PgpKeyOperation { } catch(InvalidAlgorithmParameterException e) { throw new RuntimeException(e); } catch(PGPException e) { - throw new PgpGeneralMsgIdException(R.string.msg_mf_error_pgp, e); + Log.e(Constants.TAG, "internal pgp error", e); + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_INTERNAL_PGP, indent); + return null; } } @@ -168,21 +172,32 @@ public class PgpKeyOperation { indent += 1; updateProgress(R.string.progress_building_key, 0, 100); - if (saveParcel.mAddSubKeys == null || saveParcel.mAddSubKeys.isEmpty()) { + if (saveParcel.mAddSubKeys.isEmpty()) { log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_NO_MASTER, indent); return null; } + if (saveParcel.mAddUserIds.isEmpty()) { + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_NO_USER_ID, indent); + return null; + } + SubkeyAdd add = saveParcel.mAddSubKeys.remove(0); if ((add.mFlags & KeyFlags.CERTIFY_OTHER) != KeyFlags.CERTIFY_OTHER) { log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_NO_CERTIFY, indent); return null; } - PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize); + PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize, log, indent); if (add.mAlgorithm == Constants.choice.algorithm.elgamal) { - throw new PgpGeneralMsgIdException(R.string.error_master_key_must_not_be_el_gamal); + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_MASTER_ELGAMAL, indent); + return null; + } + + // return null if this failed (it will already have been logged by createKey) + if (keyPair == null) { + return null; } // define hashing and signing algos @@ -201,14 +216,12 @@ public class PgpKeyOperation { return internal(sKR, masterSecretKey, add.mFlags, saveParcel, "", log, indent); } catch (PGPException e) { + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_INTERNAL_PGP, indent); Log.e(Constants.TAG, "pgp error encoding key", e); return null; } catch (IOException e) { Log.e(Constants.TAG, "io error encoding key", e); return null; - } catch (PgpGeneralMsgIdException e) { - Log.e(Constants.TAG, "pgp msg id error", e); - return null; } } @@ -521,47 +534,46 @@ public class PgpKeyOperation { // 5. Generate and add new subkeys for (SaveKeyringParcel.SubkeyAdd add : saveParcel.mAddSubKeys) { - try { - - if (add.mExpiry != null && new Date(add.mExpiry*1000).before(new Date())) { - log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY, indent +1); - return null; - } - - log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_NEW, indent); - // generate a new secret key (privkey only for now) - PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize); - - // add subkey binding signature (making this a sub rather than master key) - PGPPublicKey pKey = keyPair.getPublicKey(); - PGPSignature cert = generateSubkeyBindingSignature( - masterPublicKey, masterPrivateKey, keyPair.getPrivateKey(), pKey, - add.mFlags, add.mExpiry == null ? 0 : add.mExpiry); - pKey = PGPPublicKey.addSubkeyBindingCertification(pKey, cert); + if (add.mExpiry != null && new Date(add.mExpiry*1000).before(new Date())) { + log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY, indent +1); + return null; + } - PGPSecretKey sKey; { - // define hashing and signing algos - PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder() - .build().get(HashAlgorithmTags.SHA1); + log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_NEW, indent); - // Build key encrypter and decrypter based on passphrase - PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder( - PGPEncryptedData.CAST5, sha1Calc) - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray()); + // generate a new secret key (privkey only for now) + PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize, log, indent); + if(keyPair == null) { + return null; + } - sKey = new PGPSecretKey(keyPair.getPrivateKey(), pKey, - sha1Calc, false, keyEncryptor); - } + // add subkey binding signature (making this a sub rather than master key) + PGPPublicKey pKey = keyPair.getPublicKey(); + PGPSignature cert = generateSubkeyBindingSignature( + masterPublicKey, masterPrivateKey, keyPair.getPrivateKey(), pKey, + add.mFlags, add.mExpiry == null ? 0 : add.mExpiry); + pKey = PGPPublicKey.addSubkeyBindingCertification(pKey, cert); + + PGPSecretKey sKey; { + // define hashing and signing algos + PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder() + .build().get(HashAlgorithmTags.SHA1); + + // Build key encrypter and decrypter based on passphrase + PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder( + PGPEncryptedData.CAST5, sha1Calc) + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray()); + + sKey = new PGPSecretKey(keyPair.getPrivateKey(), pKey, + sha1Calc, false, keyEncryptor); + } - log.add(LogLevel.DEBUG, LogType.MSG_MF_SUBKEY_NEW_ID, - indent+1, PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID())); + log.add(LogLevel.DEBUG, LogType.MSG_MF_SUBKEY_NEW_ID, + indent+1, PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID())); - sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey); + sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey); - } catch (PgpGeneralMsgIdException e) { - return null; - } } // 6. If requested, change passphrase diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index c160da483..67386da06 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -246,7 +246,12 @@ public class OperationResultParcel implements Parcelable { // secret key create MSG_CR (R.string.msg_cr), MSG_CR_ERROR_NO_MASTER (R.string.msg_cr_error_no_master), + MSG_CR_ERROR_NO_USER_ID (R.string.msg_cr_error_no_user_id), MSG_CR_ERROR_NO_CERTIFY (R.string.msg_cr_error_no_certify), + MSG_CR_ERROR_KEYSIZE_512 (R.string.msg_cr_error_keysize_512), + MSG_CR_ERROR_UNKNOWN_ALGO (R.string.msg_cr_error_unknown_algo), + MSG_CR_ERROR_INTERNAL_PGP (R.string.msg_cr_error_internal_pgp), + MSG_CR_ERROR_MASTER_ELGAMAL (R.string.msg_cr_error_master_elgamal), // secret key modify MSG_MF (R.string.msg_mr), diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index d8e102b04..5efe51aa9 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -640,7 +640,12 @@ <!-- createSecretKeyRing --> <string name="msg_cr">Generating new master key</string> <string name="msg_cr_error_no_master">No master key options specified!</string> + <string name="msg_cr_error_no_user_id">Keyrings must be created with at least one user id!</string> <string name="msg_cr_error_no_certify">Master key must have certify flag!</string> + <string name="msg_cr_error_keysize_512">Key size must be greater or equal 512!</string> + <string name="msg_cr_error_internal_pgp">Internal PGP error!</string> + <string name="msg_cr_error_unknown_algo">Bad algorithm choice!</string> + <string name="msg_cr_error_master_elgamal">Master key must not be of type ElGamal!</string> <!-- modifySecretKeyRing --> <string name="msg_mr">Modifying keyring %s</string> |