From 28b9068ae03b6c17b247a1d2fdf9b20d8a11a274 Mon Sep 17 00:00:00 2001 From: Joey Castillo Date: Wed, 13 May 2015 06:01:42 -0400 Subject: [PATCH] Adding keytocard flag to SubkeyChange: UI sets this flag to initiate keytocard; operation unsets it and fills in dummyDivert to finish it. --- .../keychain/pgp/PgpKeyOperation.java | 36 +++++++++---------- .../keychain/service/SaveKeyringParcel.java | 19 ++++++++-- .../keychain/ui/EditKeyFragment.java | 18 +++++----- .../keychain/ui/adapter/SubkeysAdapter.java | 4 +-- 4 files changed, 45 insertions(+), 32 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 93c479d5e..b6e978f48 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -412,7 +412,7 @@ public class PgpKeyOperation { } for(SaveKeyringParcel.SubkeyChange change : saveParcel.mChangeSubKeys) { - if (change.mDummyDivert != null && change.mDummyDivert.length == 0) { + if (change.mMoveKeyToCard) { // If this is a keytocard operation, see if it was completed: look for a hash // matching the given subkey ID in cryptoData. byte[] subKeyId = new byte[8]; @@ -421,6 +421,7 @@ public class PgpKeyOperation { byte[] serialNumber = cryptoInput.getCryptoData().get(buf); if (serialNumber != null) { + change.mMoveKeyToCard = false; change.mDummyDivert = serialNumber; } } @@ -776,28 +777,27 @@ public class PgpKeyOperation { // no really, it is. this operation irrevocably removes the private key data from the key sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey()); sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey); - } else if (change.mDummyDivert != null) { - if (change.mDummyDivert.length == 0) { - // If serial number is 0 length, we're moving the key to a card. - if (checkSmartCardCompatibility(sKey, log, indent + 1)) { - log.add(LogType.MSG_MF_KEYTOCARD_START, indent + 1, - KeyFormattingUtils.convertKeyIdToHex(change.mKeyId)); - nfcKeyToCardOps.addSubkey(change.mKeyId); - } else { - return new PgpEditKeyResult(EditKeyResult.RESULT_ERROR, log, null); - } - } else if (change.mDummyDivert.length == 16) { - // If serial number is 16 bytes long, we're associating the key with a card. - log.add(LogType.MSG_MF_KEYTOCARD_FINISH, indent + 1, - KeyFormattingUtils.convertKeyIdToHex(change.mKeyId), - Hex.toHexString(change.mDummyDivert, 8, 6)); - sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey(), change.mDummyDivert); - sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey); + } else if (change.mMoveKeyToCard) { + if (checkSmartCardCompatibility(sKey, log, indent + 1)) { + log.add(LogType.MSG_MF_KEYTOCARD_START, indent + 1, + KeyFormattingUtils.convertKeyIdToHex(change.mKeyId)); + nfcKeyToCardOps.addSubkey(change.mKeyId); } else { + // Appropriate log message already set by checkSmartCardCompatibility + return new PgpEditKeyResult(EditKeyResult.RESULT_ERROR, log, null); + } + } else if (change.mDummyDivert != null) { + // NOTE: Does this code get executed? Or always handled in internalRestricted? + if (change.mDummyDivert.length != 16) { log.add(LogType.MSG_MF_ERROR_DIVERT_SERIAL, indent + 1, KeyFormattingUtils.convertKeyIdToHex(change.mKeyId)); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } + log.add(LogType.MSG_MF_KEYTOCARD_FINISH, indent + 1, + KeyFormattingUtils.convertKeyIdToHex(change.mKeyId), + Hex.toHexString(change.mDummyDivert, 8, 6)); + sKey = PGPSecretKey.constructGnuDummyKey(sKey.getPublicKey(), change.mDummyDivert); + sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java index 6064c28fb..dd6697f21 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java @@ -95,8 +95,8 @@ public class SaveKeyringParcel implements Parcelable { } for (SubkeyChange change : mChangeSubKeys) { - if (change.mRecertify || change.mFlags != null || change.mExpiry != null || - (change.mDummyDivert != null && change.mDummyDivert.length == 0)) { + if (change.mRecertify || change.mFlags != null || change.mExpiry != null + || change.mMoveKeyToCard) { return false; } } @@ -143,6 +143,8 @@ public class SaveKeyringParcel implements Parcelable { public boolean mRecertify; // if this flag is true, the subkey should be changed to a stripped key public boolean mDummyStrip; + // if this flag is true, the subkey should be moved to a card + public boolean mMoveKeyToCard; // if this is non-null, the subkey will be changed to a divert-to-card // key for the given serial number public byte[] mDummyDivert; @@ -174,12 +176,25 @@ public class SaveKeyringParcel implements Parcelable { mDummyDivert = dummyDivert; } + public SubkeyChange(long keyId, boolean dummyStrip, boolean moveKeyToCard) { + this(keyId, null, null); + + // these flags are mutually exclusive! + if (dummyStrip && moveKeyToCard) { + throw new AssertionError( + "cannot set strip and keytocard flags at the same time - this is a bug!"); + } + mDummyStrip = dummyStrip; + mMoveKeyToCard = moveKeyToCard; + } + @Override public String toString() { String out = "mKeyId: " + mKeyId + ", "; out += "mFlags: " + mFlags + ", "; out += "mExpiry: " + mExpiry + ", "; out += "mDummyStrip: " + mDummyStrip + ", "; + out += "mMoveKeyToCard: " + mMoveKeyToCard + ", "; out += "mDummyDivert: [" + (mDummyDivert == null ? 0 : mDummyDivert.length) + " bytes]"; return out; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java index bc2fbff76..acc0c32b8 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java @@ -429,14 +429,14 @@ public class EditKeyFragment extends CryptoOperationFragment implements SubkeyChange change = mSaveKeyringParcel.getSubkeyChange(keyId); if (change == null) { - mSaveKeyringParcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null)); + mSaveKeyringParcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, false)); break; } // toggle change.mDummyStrip = !change.mDummyStrip; - if (change.mDummyStrip && change.mDummyDivert != null) { + if (change.mDummyStrip && change.mMoveKeyToCard) { // User had chosen to divert key, but now wants to strip it instead. - change.mDummyDivert = null; + change.mMoveKeyToCard = false; } break; } @@ -455,17 +455,15 @@ public class EditKeyFragment extends CryptoOperationFragment implements change = mSaveKeyringParcel.getSubkeyChange(keyId); if (change == null) { mSaveKeyringParcel.mChangeSubKeys.add( - new SubkeyChange(keyId, false, null) + new SubkeyChange(keyId, false, true) ); - change = mSaveKeyringParcel.getSubkeyChange(keyId); + break; } // toggle - if (change.mDummyDivert == null) { - change.mDummyDivert = new byte[0]; - // If user had chosen to strip key, we cancel that action now. + change.mMoveKeyToCard = !change.mMoveKeyToCard; + if (change.mMoveKeyToCard && change.mDummyStrip) { + // User had chosen to strip key, but now wants to divert it. change.mDummyStrip = false; - } else { - change.mDummyDivert = null; } break; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/SubkeysAdapter.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/SubkeysAdapter.java index dfa5a39fb..87539ea05 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/SubkeysAdapter.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/SubkeysAdapter.java @@ -179,7 +179,7 @@ public class SubkeysAdapter extends CursorAdapter { ? mSaveKeyringParcel.getSubkeyChange(keyId) : null; - if (change != null && (change.mDummyStrip || change.mDummyDivert != null)) { + if (change != null && (change.mDummyStrip || change.mMoveKeyToCard)) { if (change.mDummyStrip) { algorithmStr.append(", "); final SpannableString boldStripped = new SpannableString( @@ -188,7 +188,7 @@ public class SubkeysAdapter extends CursorAdapter { boldStripped.setSpan(new StyleSpan(Typeface.BOLD), 0, boldStripped.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE); algorithmStr.append(boldStripped); } - if (change.mDummyDivert != null) { + if (change.mMoveKeyToCard) { algorithmStr.append(", "); final SpannableString boldDivert = new SpannableString( context.getString(R.string.key_divert)