diff options
Diffstat (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java')
| -rw-r--r-- | OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java | 267 |
1 files changed, 175 insertions, 92 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 9a08290e4..69244ca14 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -65,10 +65,8 @@ import java.security.NoSuchProviderException; import java.security.SecureRandom; import java.security.SignatureException; import java.util.Arrays; -import java.util.Calendar; import java.util.Date; import java.util.Iterator; -import java.util.TimeZone; /** * This class is the single place where ALL operations that actually modify a PGP public or secret @@ -104,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; @@ -143,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; } } @@ -157,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; } } @@ -166,20 +168,36 @@ public class PgpKeyOperation { try { - log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_KEYID, indent); + log.add(LogLevel.START, LogType.MSG_CR, indent); indent += 1; updateProgress(R.string.progress_building_key, 0, 100); - if (saveParcel.addSubKeys == null || saveParcel.addSubKeys.isEmpty()) { + if (saveParcel.mAddSubKeys.isEmpty()) { log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_NO_MASTER, indent); return null; } - SubkeyAdd add = saveParcel.addSubKeys.remove(0); - PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize); + 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; + } 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; + } + + PGPKeyPair keyPair = createKey(add.mAlgorithm, add.mKeysize, log, indent); + + // return null if this failed (an error will already have been logged by createKey) + if (keyPair == null) { + return null; } // define hashing and signing algos @@ -195,17 +213,15 @@ public class PgpKeyOperation { PGPSecretKeyRing sKR = new PGPSecretKeyRing( masterSecretKey.getEncoded(), new JcaKeyFingerprintCalculator()); - return internal(sKR, masterSecretKey, saveParcel, "", log, indent); + 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; } } @@ -257,11 +273,16 @@ public class PgpKeyOperation { return null; } - return internal(sKR, masterSecretKey, saveParcel, passphrase, log, indent); + // read masterKeyFlags, and use the same as before. + // since this is the master key, this contains at least CERTIFY_OTHER + int masterKeyFlags = readKeyFlags(masterSecretKey.getPublicKey()) | KeyFlags.CERTIFY_OTHER; + + return internal(sKR, masterSecretKey, masterKeyFlags, saveParcel, passphrase, log, indent); } private UncachedKeyRing internal(PGPSecretKeyRing sKR, PGPSecretKey masterSecretKey, + int masterKeyFlags, SaveKeyringParcel saveParcel, String passphrase, OperationLog log, int indent) { @@ -289,18 +310,24 @@ public class PgpKeyOperation { PGPPublicKey modifiedPublicKey = masterPublicKey; // 2a. Add certificates for new user ids - for (String userId : saveParcel.addUserIds) { + for (String userId : saveParcel.mAddUserIds) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent); + if (userId.equals("")) { + log.add(LogLevel.ERROR, LogType.MSG_MF_UID_ERROR_EMPTY, indent+1); + return null; + } + // this operation supersedes all previous binding and revocation certificates, // so remove those to retain assertions from canonicalization for later operations @SuppressWarnings("unchecked") Iterator<PGPSignature> it = modifiedPublicKey.getSignaturesForID(userId); if (it != null) { for (PGPSignature cert : new IterableIterator<PGPSignature>(it)) { - // if it's not a self cert, never mind if (cert.getKeyID() != masterPublicKey.getKeyID()) { - continue; + // foreign certificate?! error error error + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent); + return null; } if (cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION || cert.getSignatureType() == PGPSignature.NO_CERTIFICATION @@ -314,27 +341,31 @@ public class PgpKeyOperation { } // if it's supposed to be primary, we can do that here as well - boolean isPrimary = saveParcel.changePrimaryUserId != null - && userId.equals(saveParcel.changePrimaryUserId); + boolean isPrimary = saveParcel.mChangePrimaryUserId != null + && userId.equals(saveParcel.mChangePrimaryUserId); // generate and add new certificate PGPSignature cert = generateUserIdSignature(masterPrivateKey, - masterPublicKey, userId, isPrimary); - modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); + masterPublicKey, userId, isPrimary, masterKeyFlags); + modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert); } // 2b. Add revocations for revoked user ids - for (String userId : saveParcel.revokeUserIds) { + for (String userId : saveParcel.mRevokeUserIds) { log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent); - // a duplicate revocatin will be removed during canonicalization, so no need to + // a duplicate revocation will be removed during canonicalization, so no need to // take care of that here. PGPSignature cert = generateRevocationSignature(masterPrivateKey, masterPublicKey, userId); - modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); + modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, userId, cert); } // 3. If primary user id changed, generate new certificates for both old and new - if (saveParcel.changePrimaryUserId != null) { + if (saveParcel.mChangePrimaryUserId != null) { + + // keep track if we actually changed one + boolean ok = false; log.add(LogLevel.INFO, LogType.MSG_MF_UID_PRIMARY, indent); + indent += 1; // we work on the modifiedPublicKey here, to respect new or newly revoked uids // noinspection unchecked @@ -343,10 +374,11 @@ public class PgpKeyOperation { PGPSignature currentCert = null; // noinspection unchecked for (PGPSignature cert : new IterableIterator<PGPSignature>( - masterPublicKey.getSignaturesForID(userId))) { - // if it's not a self cert, never mind + modifiedPublicKey.getSignaturesForID(userId))) { if (cert.getKeyID() != masterPublicKey.getKeyID()) { - continue; + // foreign certificate?! error error error + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_INTEGRITY, indent); + return null; } // we know from canonicalization that if there is any revocation here, it // is valid and not superseded by a newer certification. @@ -373,7 +405,7 @@ public class PgpKeyOperation { // we definitely should not update certifications of revoked keys, so just leave it. if (isRevoked) { // revoked user ids cannot be primary! - if (userId.equals(saveParcel.changePrimaryUserId)) { + if (userId.equals(saveParcel.mChangePrimaryUserId)) { log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_REVOKED_PRIMARY, indent); return null; } @@ -383,14 +415,16 @@ public class PgpKeyOperation { // if this is~ the/a primary user id if (currentCert.hasSubpackets() && currentCert.getHashedSubPackets().isPrimaryUserID()) { // if it's the one we want, just leave it as is - if (userId.equals(saveParcel.changePrimaryUserId)) { + if (userId.equals(saveParcel.mChangePrimaryUserId)) { + ok = true; continue; } // otherwise, generate new non-primary certification + log.add(LogLevel.DEBUG, LogType.MSG_MF_PRIMARY_REPLACE_OLD, indent); modifiedPublicKey = PGPPublicKey.removeCertification( modifiedPublicKey, userId, currentCert); PGPSignature newCert = generateUserIdSignature( - masterPrivateKey, masterPublicKey, userId, false); + masterPrivateKey, masterPublicKey, userId, false, masterKeyFlags); modifiedPublicKey = PGPPublicKey.addCertification( modifiedPublicKey, userId, newCert); continue; @@ -399,20 +433,28 @@ public class PgpKeyOperation { // if we are here, this is not currently a primary user id // if it should be - if (userId.equals(saveParcel.changePrimaryUserId)) { + if (userId.equals(saveParcel.mChangePrimaryUserId)) { // add shiny new primary user id certificate + log.add(LogLevel.DEBUG, LogType.MSG_MF_PRIMARY_NEW, indent); modifiedPublicKey = PGPPublicKey.removeCertification( modifiedPublicKey, userId, currentCert); PGPSignature newCert = generateUserIdSignature( - masterPrivateKey, masterPublicKey, userId, true); + masterPrivateKey, masterPublicKey, userId, true, masterKeyFlags); modifiedPublicKey = PGPPublicKey.addCertification( modifiedPublicKey, userId, newCert); + ok = true; } // user id is not primary and is not supposed to be - nothing to do here. } + indent -= 1; + + if (!ok) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NOEXIST_PRIMARY, indent); + return null; + } } // Update the secret key ring @@ -423,9 +465,16 @@ public class PgpKeyOperation { } // 4a. For each subkey change, generate new subkey binding certificate - for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) { + for (SaveKeyringParcel.SubkeyChange change : saveParcel.mChangeSubKeys) { log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE, indent, PgpKeyHelper.convertKeyIdToHex(change.mKeyId)); + + // TODO allow changes in master key? this implies generating new user id certs... + if (change.mKeyId == masterPublicKey.getKeyID()) { + Log.e(Constants.TAG, "changing the master key not supported"); + return null; + } + PGPSecretKey sKey = sKR.getSecretKey(change.mKeyId); if (sKey == null) { log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_MISSING, @@ -434,22 +483,45 @@ public class PgpKeyOperation { } PGPPublicKey pKey = sKey.getPublicKey(); - if (change.mExpiry != null && new Date(change.mExpiry).before(new Date())) { + // expiry must not be in the past + if (change.mExpiry != null && new Date(change.mExpiry*1000).before(new Date())) { log.add(LogLevel.ERROR, LogType.MSG_MF_SUBKEY_PAST_EXPIRY, indent + 1, PgpKeyHelper.convertKeyIdToHex(change.mKeyId)); return null; } - // generate and add new signature. we can be sloppy here and just leave the old one, - // it will be removed during canonicalization + // keep old flags, or replace with new ones + int flags = change.mFlags == null ? readKeyFlags(pKey) : change.mFlags; + long expiry; + if (change.mExpiry == null) { + long valid = pKey.getValidSeconds(); + expiry = valid == 0 + ? 0 + : pKey.getCreationTime().getTime() / 1000 + pKey.getValidSeconds(); + } else { + expiry = change.mExpiry; + } + + // drop all old signatures, they will be superseded by the new one + //noinspection unchecked + for (PGPSignature sig : new IterableIterator<PGPSignature>(pKey.getSignatures())) { + // special case: if there is a revocation, don't use expiry from before + if (change.mExpiry == null + && sig.getSignatureType() == PGPSignature.SUBKEY_REVOCATION) { + expiry = 0; + } + pKey = PGPPublicKey.removeCertification(pKey, sig); + } + + // generate and add new signature PGPSignature sig = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, - sKey, pKey, change.mFlags, change.mExpiry, passphrase); + sKey, pKey, flags, expiry, passphrase); pKey = PGPPublicKey.addCertification(pKey, sig); sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); } // 4b. For each subkey revocation, generate new subkey revocation certificate - for (long revocation : saveParcel.revokeSubKeys) { + for (long revocation : saveParcel.mRevokeSubKeys) { log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_REVOKE, indent, PgpKeyHelper.convertKeyIdToHex(revocation)); PGPSecretKey sKey = sKR.getSecretKey(revocation); @@ -468,52 +540,51 @@ public class PgpKeyOperation { } // 5. Generate and add new subkeys - for (SaveKeyringParcel.SubkeyAdd add : saveParcel.addSubKeys) { - try { + for (SaveKeyringParcel.SubkeyAdd add : saveParcel.mAddSubKeys) { - if (add.mExpiry != null && new Date(add.mExpiry).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); - 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 - if (saveParcel.newPassphrase != null) { + if (saveParcel.mNewPassphrase != null) { log.add(LogLevel.INFO, LogType.MSG_MF_PASSPHRASE, indent); PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build() .get(HashAlgorithmTags.SHA1); @@ -523,7 +594,7 @@ public class PgpKeyOperation { PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder( PGPEncryptedData.CAST5, sha1Calc) .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build( - saveParcel.newPassphrase.toCharArray()); + saveParcel.mNewPassphrase.toCharArray()); sKR = PGPSecretKeyRing.copyWithNewPassword(sKR, keyDecryptor, keyEncryptorNew); } @@ -533,6 +604,7 @@ public class PgpKeyOperation { log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_ENCODE, indent+1); return null; } catch (PGPException e) { + Log.e(Constants.TAG, "encountered pgp error while modifying key", e); log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_PGP, indent+1); return null; } catch (SignatureException e) { @@ -546,7 +618,7 @@ public class PgpKeyOperation { } private static PGPSignature generateUserIdSignature( - PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, String userId, boolean primary) + PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, String userId, boolean primary, int flags) throws IOException, PGPException, SignatureException { PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder( pKey.getAlgorithm(), PGPUtil.SHA1) @@ -558,6 +630,7 @@ public class PgpKeyOperation { subHashedPacketsGen.setPreferredHashAlgorithms(true, PREFERRED_HASH_ALGORITHMS); subHashedPacketsGen.setPreferredCompressionAlgorithms(true, PREFERRED_COMPRESSION_ALGORITHMS); subHashedPacketsGen.setPrimaryUserID(false, primary); + subHashedPacketsGen.setKeyFlags(false, flags); sGen.setHashedSubpackets(subHashedPacketsGen.generate()); sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey); return sGen.generateCertification(userId, pKey); @@ -599,7 +672,7 @@ public class PgpKeyOperation { private static PGPSignature generateSubkeyBindingSignature( PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey, - PGPSecretKey sKey, PGPPublicKey pKey, int flags, Long expiry, String passphrase) + PGPSecretKey sKey, PGPPublicKey pKey, int flags, long expiry, String passphrase) throws IOException, PGPException, SignatureException { PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder() .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build( @@ -611,7 +684,7 @@ public class PgpKeyOperation { private static PGPSignature generateSubkeyBindingSignature( PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey, - PGPPrivateKey subPrivateKey, PGPPublicKey pKey, int flags, Long expiry) + PGPPrivateKey subPrivateKey, PGPPublicKey pKey, int flags, long expiry) throws IOException, PGPException, SignatureException { // date for signing @@ -640,17 +713,9 @@ public class PgpKeyOperation { hashedPacketsGen.setKeyFlags(false, flags); } - if (expiry != null) { - Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC")); - creationDate.setTime(pKey.getCreationTime()); - - // (Just making sure there's no programming error here, this MUST have been checked above!) - if (new Date(expiry).before(todayDate)) { - throw new RuntimeException("Bad subkey creation date, this is a bug!"); - } - hashedPacketsGen.setKeyExpirationTime(false, expiry - creationDate.getTimeInMillis()); - } else { - hashedPacketsGen.setKeyExpirationTime(false, 0); + if (expiry > 0) { + long creationTime = pKey.getCreationTime().getTime() / 1000; + hashedPacketsGen.setKeyExpirationTime(false, expiry - creationTime); } PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder( @@ -665,4 +730,22 @@ public class PgpKeyOperation { } + /** Returns all flags valid for this key. + * + * This method does not do any validity checks on the signature, so it should not be used on + * a non-canonicalized key! + * + */ + private static int readKeyFlags(PGPPublicKey key) { + int flags = 0; + //noinspection unchecked + for(PGPSignature sig : new IterableIterator<PGPSignature>(key.getSignatures())) { + if (!sig.hasSubpackets()) { + continue; + } + flags |= sig.getHashedSubPackets().getKeyFlags(); + } + return flags; + } + } |