introduce new ChangeUnlockParcel packet for extended passphrase changing capabilities

This commit is contained in:
Vincent Breitmoser 2014-12-29 23:12:11 +01:00
parent bf4762ef6f
commit 576e6fd0cc
10 changed files with 189 additions and 81 deletions

View File

@ -68,7 +68,7 @@ public class PgpEncryptDecryptTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
parcel.mAddUserIds.add("bloom");
parcel.mNewPassphrase = mKeyPhrase1;
parcel.mNewUnlock = mKeyPhrase1;
EditKeyResult result = op.createSecretKeyRing(parcel);
Assert.assertTrue("initial test key creation must succeed", result.success());
@ -86,7 +86,7 @@ public class PgpEncryptDecryptTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
parcel.mAddUserIds.add("belle");
parcel.mNewPassphrase = mKeyPhrase2;
parcel.mNewUnlock = mKeyPhrase2;
EditKeyResult result = op.createSecretKeyRing(parcel);
Assert.assertTrue("initial test key creation must succeed", result.success());

View File

@ -91,7 +91,7 @@ public class PgpKeyOperationTest {
parcel.mAddUserIds.add("twi");
parcel.mAddUserIds.add("pink");
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
PgpKeyOperation op = new PgpKeyOperation(null);
EditKeyResult result = op.createSecretKeyRing(parcel);
@ -127,7 +127,7 @@ public class PgpKeyOperationTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, new Random().nextInt(256)+255, null, KeyFlags.CERTIFY_OTHER, 0L));
parcel.mAddUserIds.add("shy");
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
assertFailure("creating ring with < 512 bytes keysize should fail", parcel,
LogType.MSG_CR_ERROR_KEYSIZE_512);
@ -138,7 +138,7 @@ public class PgpKeyOperationTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.ELGAMAL, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
parcel.mAddUserIds.add("shy");
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
assertFailure("creating ring with ElGamal master key should fail", parcel,
LogType.MSG_CR_ERROR_FLAGS_ELGAMAL);
@ -149,7 +149,7 @@ public class PgpKeyOperationTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, null));
parcel.mAddUserIds.add("lotus");
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
assertFailure("creating master key with null expiry should fail", parcel,
LogType.MSG_CR_ERROR_NULL_EXPIRY);
@ -160,7 +160,7 @@ public class PgpKeyOperationTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
parcel.mAddUserIds.add("shy");
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
assertFailure("creating ring with non-certifying master key should fail", parcel,
LogType.MSG_CR_ERROR_NO_CERTIFY);
@ -170,7 +170,7 @@ public class PgpKeyOperationTest {
parcel.reset();
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
assertFailure("creating ring without user ids should fail", parcel,
LogType.MSG_CR_ERROR_NO_USER_ID);
@ -179,7 +179,7 @@ public class PgpKeyOperationTest {
{
parcel.reset();
parcel.mAddUserIds.add("shy");
parcel.mNewPassphrase = passphrase;
parcel.mNewUnlock = passphrase;
assertFailure("creating ring with no master key should fail", parcel,
LogType.MSG_CR_ERROR_NO_MASTER);
@ -910,7 +910,7 @@ public class PgpKeyOperationTest {
public void testPassphraseChange() throws Exception {
// change passphrase to empty
parcel.mNewPassphrase = "";
parcel.mNewUnlock = "";
UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
@ -923,7 +923,7 @@ public class PgpKeyOperationTest {
// modify keyring, change to non-empty passphrase
String otherPassphrase = TestingUtils.genPassphrase(true);
parcel.mNewPassphrase = otherPassphrase;
parcel.mNewUnlock = otherPassphrase;
modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, "");
Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
@ -948,7 +948,7 @@ public class PgpKeyOperationTest {
PacketTags.SECRET_SUBKEY, sKeyNoPassphrase.tag);
String otherPassphrase2 = TestingUtils.genPassphrase(true);
parcel.mNewPassphrase = otherPassphrase2;
parcel.mNewUnlock = otherPassphrase2;
{
// if we replace a secret key with one without passphrase
modified = KeyringTestingHelper.removePacket(modified, sKeyNoPassphrase.position);

View File

@ -104,7 +104,7 @@ public class UncachedKeyringCanonicalizeTest {
parcel.mAddUserIds.add("twi");
parcel.mAddUserIds.add("pink");
// passphrase is tested in PgpKeyOperationTest, just use empty here
parcel.mNewPassphrase = "";
parcel.mNewUnlock = "";
PgpKeyOperation op = new PgpKeyOperation(null);
EditKeyResult result = op.createSecretKeyRing(parcel);

View File

@ -96,7 +96,7 @@ public class UncachedKeyringMergeTest {
parcel.mAddUserIds.add("twi");
parcel.mAddUserIds.add("pink");
// passphrase is tested in PgpKeyOperationTest, just use empty here
parcel.mNewPassphrase = "";
parcel.mNewUnlock = "";
PgpKeyOperation op = new PgpKeyOperation(null);
OperationResult.OperationLog log = new OperationResult.OperationLog();
@ -112,7 +112,7 @@ public class UncachedKeyringMergeTest {
parcel.mAddUserIds.add("shy");
// passphrase is tested in PgpKeyOperationTest, just use empty here
parcel.mNewPassphrase = "";
parcel.mNewUnlock = "";
PgpKeyOperation op = new PgpKeyOperation(null);
OperationResult.OperationLog log = new OperationResult.OperationLog();

View File

@ -57,7 +57,7 @@ public class UncachedKeyringTest {
parcel.mAddUserIds.add("twi");
parcel.mAddUserIds.add("pink");
// passphrase is tested in PgpKeyOperationTest, just use empty here
parcel.mNewPassphrase = "";
parcel.mNewUnlock = "";
PgpKeyOperation op = new PgpKeyOperation(null);
EditKeyResult result = op.createSecretKeyRing(parcel);

View File

@ -52,6 +52,7 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult.Operat
import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Curve;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
@ -871,11 +872,63 @@ public class PgpKeyOperation {
}
// 6. If requested, change passphrase
if (saveParcel.mNewPassphrase != null) {
if (saveParcel.mNewUnlock != null) {
progress(R.string.progress_modify_passphrase, 90);
log.add(LogType.MSG_MF_PASSPHRASE, indent);
indent += 1;
sKR = applyNewUnlock(sKR, masterPublicKey, passphrase, saveParcel.mNewUnlock, log, indent);
if (sKR == null) {
// The error has been logged above, just return a bad state
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
}
indent -= 1;
}
} catch (IOException e) {
Log.e(Constants.TAG, "encountered IOException while modifying key", e);
log.add(LogType.MSG_MF_ERROR_ENCODE, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
} catch (PGPException e) {
Log.e(Constants.TAG, "encountered pgp error while modifying key", e);
log.add(LogType.MSG_MF_ERROR_PGP, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
} catch (SignatureException e) {
Log.e(Constants.TAG, "encountered SignatureException while modifying key", e);
log.add(LogType.MSG_MF_ERROR_SIG, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
}
progress(R.string.progress_done, 100);
log.add(LogType.MSG_MF_SUCCESS, indent);
return new EditKeyResult(OperationResult.RESULT_OK, log, new UncachedKeyRing(sKR));
}
private static PGPSecretKeyRing applyNewUnlock(
PGPSecretKeyRing sKR,
PGPPublicKey masterPublicKey,
String passphrase,
ChangeUnlockParcel newUnlock,
OperationLog log, int indent) throws PGPException {
if (newUnlock.mNewPassphrase != null) {
return applyNewPassphrase(sKR, masterPublicKey, passphrase, newUnlock.mNewPassphrase, log, indent);
}
throw new UnsupportedOperationException("PIN passphrases not yet implemented!");
}
private static PGPSecretKeyRing applyNewPassphrase(
PGPSecretKeyRing sKR,
PGPPublicKey masterPublicKey,
String passphrase,
String newPassphrase,
OperationLog log, int indent) throws PGPException {
PGPDigestCalculator encryptorHashCalc = new JcaPGPDigestCalculatorProviderBuilder().build()
.get(SECRET_KEY_ENCRYPTOR_HASH_ALGO);
PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
@ -884,7 +937,7 @@ public class PgpKeyOperation {
PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder(
SECRET_KEY_ENCRYPTOR_SYMMETRIC_ALGO, encryptorHashCalc, SECRET_KEY_ENCRYPTOR_S2K_COUNT)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
saveParcel.mNewPassphrase.toCharArray());
newPassphrase.toCharArray());
// noinspection unchecked
for (PGPSecretKey sKey : new IterableIterator<PGPSecretKey>(sKR.getSecretKeys())) {
@ -902,7 +955,7 @@ public class PgpKeyOperation {
// if this is the master key, error!
if (sKey.getKeyID() == masterPublicKey.getKeyID()) {
log.add(LogType.MSG_MF_ERROR_PASSPHRASE_MASTER, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
return null;
}
// being in here means decrypt failed, likely due to a bad passphrase try
@ -930,26 +983,7 @@ public class PgpKeyOperation {
}
indent -= 1;
}
} catch (IOException e) {
Log.e(Constants.TAG, "encountered IOException while modifying key", e);
log.add(LogType.MSG_MF_ERROR_ENCODE, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
} catch (PGPException e) {
Log.e(Constants.TAG, "encountered pgp error while modifying key", e);
log.add(LogType.MSG_MF_ERROR_PGP, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
} catch (SignatureException e) {
Log.e(Constants.TAG, "encountered SignatureException while modifying key", e);
log.add(LogType.MSG_MF_ERROR_SIG, indent+1);
return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
}
progress(R.string.progress_done, 100);
log.add(LogType.MSG_MF_SUCCESS, indent);
return new EditKeyResult(OperationResult.RESULT_OK, log, new UncachedKeyRing(sKR));
return sKR;
}

View File

@ -399,9 +399,14 @@ public class KeychainIntentService extends IntentService implements Progressable
}
// cache new passphrase
if (saveParcel.mNewPassphrase != null) {
PassphraseCacheService.addCachedPassphrase(this, ring.getMasterKeyId(), ring.getMasterKeyId(),
saveParcel.mNewPassphrase, ring.getPublicKey().getPrimaryUserIdWithFallback());
if (saveParcel.mNewUnlock != null) {
PassphraseCacheService.addCachedPassphrase(this,
ring.getMasterKeyId(),
ring.getMasterKeyId(),
saveParcel.mNewUnlock.mNewPassphrase != null
? saveParcel.mNewUnlock.mNewPassphrase
: saveParcel.mNewUnlock.mNewPin,
ring.getPublicKey().getPrimaryUserIdWithFallback());
}
setProgress(R.string.progress_done, 100, 100);

View File

@ -46,7 +46,7 @@ public class SaveKeyringParcel implements Parcelable {
// the key fingerprint, for safety. MUST be null for a new key.
public byte[] mFingerprint;
public String mNewPassphrase;
public ChangeUnlockParcel mNewUnlock;
public ArrayList<String> mAddUserIds;
public ArrayList<SubkeyAdd> mAddSubKeys;
@ -69,7 +69,7 @@ public class SaveKeyringParcel implements Parcelable {
}
public void reset() {
mNewPassphrase = null;
mNewUnlock = null;
mAddUserIds = new ArrayList<String>();
mAddSubKeys = new ArrayList<SubkeyAdd>();
mChangePrimaryUserId = null;
@ -159,7 +159,7 @@ public class SaveKeyringParcel implements Parcelable {
mMasterKeyId = source.readInt() != 0 ? source.readLong() : null;
mFingerprint = source.createByteArray();
mNewPassphrase = source.readString();
mNewUnlock = source.readParcelable(getClass().getClassLoader());
mAddUserIds = source.createStringArrayList();
mAddSubKeys = (ArrayList<SubkeyAdd>) source.readSerializable();
@ -180,7 +180,8 @@ public class SaveKeyringParcel implements Parcelable {
}
destination.writeByteArray(mFingerprint);
destination.writeString(mNewPassphrase);
// yes, null values are ok for parcelables
destination.writeParcelable(mNewUnlock, 0);
destination.writeStringList(mAddUserIds);
destination.writeSerializable(mAddSubKeys);
@ -211,7 +212,7 @@ public class SaveKeyringParcel implements Parcelable {
@Override
public String toString() {
String out = "mMasterKeyId: " + mMasterKeyId + "\n";
out += "mNewPassphrase: " + mNewPassphrase + "\n";
out += "mNewUnlock: " + mNewUnlock + "\n";
out += "mAddUserIds: " + mAddUserIds + "\n";
out += "mAddSubKeys: " + mAddSubKeys + "\n";
out += "mChangeSubKeys: " + mChangeSubKeys + "\n";
@ -238,4 +239,64 @@ public class SaveKeyringParcel implements Parcelable {
// BRAINPOOL_P256, BRAINPOOL_P384, BRAINPOOL_P512
}
/** This subclass contains information on how the passphrase should be changed.
*
* If no changes are to be made, this class should NOT be used!
*
* At this point, there must be *exactly one* non-null value here, which specifies the type
* of unlocking mechanism to use.
*
*/
public static class ChangeUnlockParcel implements Parcelable {
// The new passphrase to use
public final String mNewPassphrase;
// A new pin to use. Must only contain [0-9]+
public final String mNewPin;
public ChangeUnlockParcel(String newPassphrase, String newPin) {
if (newPassphrase == null && newPin == null) {
throw new RuntimeException("Cannot set both passphrase and pin. THIS IS A BUG!");
}
if (newPin != null && !newPin.matches("[0-9]+")) {
throw new RuntimeException("Pin must be numeric digits only. THIS IS A BUG!");
}
mNewPassphrase = newPassphrase;
mNewPin = newPin;
}
public ChangeUnlockParcel(Parcel source) {
mNewPassphrase = source.readString();
mNewPin = source.readString();
}
@Override
public void writeToParcel(Parcel destination, int flags) {
destination.writeString(mNewPassphrase);
destination.writeString(mNewPin);
}
@Override
public int describeContents() {
return 0;
}
public static final Creator<ChangeUnlockParcel> CREATOR = new Creator<ChangeUnlockParcel>() {
public ChangeUnlockParcel createFromParcel(final Parcel source) {
return new ChangeUnlockParcel(source);
}
public ChangeUnlockParcel[] newArray(final int size) {
return new ChangeUnlockParcel[size];
}
};
public String toString() {
return mNewPassphrase != null
? ("passphrase (" + mNewPassphrase + ")")
: ("pin (" + mNewPin + ")");
}
}
}

View File

@ -34,6 +34,7 @@ import android.widget.TextView;
import org.spongycastle.bcpg.sig.KeyFlags;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.R;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
import org.sufficientlysecure.keychain.util.Preferences;
import org.sufficientlysecure.keychain.pgp.KeyRing;
import org.sufficientlysecure.keychain.provider.KeychainContract;
@ -165,7 +166,9 @@ public class CreateKeyFinalFragment extends Fragment {
String userId = KeyRing.createUserId(mName, mEmail, null);
mSaveKeyringParcel.mAddUserIds.add(userId);
mSaveKeyringParcel.mChangePrimaryUserId = userId;
mSaveKeyringParcel.mNewPassphrase = mPassphrase;
mSaveKeyringParcel.mNewUnlock = mPassphrase != null
? new ChangeUnlockParcel(mPassphrase, null)
: null;
}
}

View File

@ -53,6 +53,7 @@ import org.sufficientlysecure.keychain.service.KeychainIntentService;
import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler;
import org.sufficientlysecure.keychain.service.PassphraseCacheService;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter;
import org.sufficientlysecure.keychain.ui.adapter.SubkeysAddedAdapter;
import org.sufficientlysecure.keychain.ui.adapter.UserIdsAdapter;
@ -189,7 +190,9 @@ public class EditKeyFragment extends LoaderFragment implements
private void loadSaveKeyringParcel(SaveKeyringParcel saveKeyringParcel) {
mSaveKeyringParcel = saveKeyringParcel;
mPrimaryUserId = saveKeyringParcel.mChangePrimaryUserId;
mCurrentPassphrase = saveKeyringParcel.mNewPassphrase;
if (saveKeyringParcel.mNewUnlock != null) {
mCurrentPassphrase = saveKeyringParcel.mNewUnlock.mNewPassphrase;
}
mUserIdsAddedAdapter = new UserIdsAddedAdapter(getActivity(), mSaveKeyringParcel.mAddUserIds, true);
mUserIdsAddedList.setAdapter(mUserIdsAddedAdapter);
@ -387,8 +390,10 @@ public class EditKeyFragment extends LoaderFragment implements
Bundle data = message.getData();
// cache new returned passphrase!
mSaveKeyringParcel.mNewPassphrase = data
.getString(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE);
mSaveKeyringParcel.mNewUnlock = new ChangeUnlockParcel(
data.getString(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE),
null
);
}
}
};