Change PIN and Admin PIN after move to key operation

7 files changed, 73 insertions, 47 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java
index 36ab62cb4..1db93d2c0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java
@@ -58,8 +58,8 @@ public class CreateKeyActivity extends BaseNfcActivity {
     Passphrase mPassphrase;
     boolean mFirstTime;
     boolean mCreateYubiKey;
-    String mYubiKeyPin;
-    String mYubiKeyAdminPin;
+    Passphrase mYubiKeyPin;
+    Passphrase mYubiKeyAdminPin;
 
     Fragment mCurrentFragment;
 
@@ -93,8 +93,8 @@ public class CreateKeyActivity extends BaseNfcActivity {
             mPassphrase = savedInstanceState.getParcelable(EXTRA_PASSPHRASE);
             mFirstTime = savedInstanceState.getBoolean(EXTRA_FIRST_TIME);
             mCreateYubiKey = savedInstanceState.getBoolean(EXTRA_CREATE_YUBI_KEY);
-            mYubiKeyPin = savedInstanceState.getString(EXTRA_YUBI_KEY_PIN);
-            mYubiKeyAdminPin = savedInstanceState.getString(EXTRA_YUBI_KEY_ADMIN_PIN);
+            mYubiKeyPin = savedInstanceState.getParcelable(EXTRA_YUBI_KEY_PIN);
+            mYubiKeyAdminPin = savedInstanceState.getParcelable(EXTRA_YUBI_KEY_ADMIN_PIN);
 
             mCurrentFragment = getSupportFragmentManager().findFragmentByTag(FRAGMENT_TAG);
         } else {
@@ -200,8 +200,8 @@ public class CreateKeyActivity extends BaseNfcActivity {
         outState.putParcelable(EXTRA_PASSPHRASE, mPassphrase);
         outState.putBoolean(EXTRA_FIRST_TIME, mFirstTime);
         outState.putBoolean(EXTRA_CREATE_YUBI_KEY, mCreateYubiKey);
-        outState.putString(EXTRA_YUBI_KEY_PIN, mYubiKeyPin);
-        outState.putString(EXTRA_YUBI_KEY_ADMIN_PIN, mYubiKeyAdminPin);
+        outState.putParcelable(EXTRA_YUBI_KEY_PIN, mYubiKeyPin);
+        outState.putParcelable(EXTRA_YUBI_KEY_ADMIN_PIN, mYubiKeyAdminPin);
     }
 
     @Override
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
index 8c7abb874..94bb68f7e 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
@@ -267,10 +267,11 @@ public class CreateKeyFinalFragment extends Fragment {
     }
 
     private void moveToCard(final EditKeyResult saveKeyResult) {
-        CachedPublicKeyRing key = (new ProviderHelper(getActivity()))
-                .getCachedPublicKeyRing(saveKeyResult.mMasterKeyId);
+        final CreateKeyActivity createKeyActivity = (CreateKeyActivity) getActivity();
 
         final SaveKeyringParcel changeKeyringParcel;
+        CachedPublicKeyRing key = (new ProviderHelper(getActivity()))
+                .getCachedPublicKeyRing(saveKeyResult.mMasterKeyId);
         try {
             changeKeyringParcel = new SaveKeyringParcel(key.getMasterKeyId(), key.getFingerprint());
         } catch (PgpKeyNotFoundException e) {
@@ -278,6 +279,7 @@ public class CreateKeyFinalFragment extends Fragment {
             return;
         }
 
+        // define subkeys that should be moved to the card
         Cursor cursor = getActivity().getContentResolver().query(
                 KeychainContract.Keys.buildKeysUri(changeKeyringParcel.mMasterKeyId),
                 new String[]{KeychainContract.Keys.KEY_ID,}, null, null, null
@@ -293,6 +295,10 @@ public class CreateKeyFinalFragment extends Fragment {
             }
         }
 
+        // define new PIN and Admin PIN for the card
+        changeKeyringParcel.mCardPin = createKeyActivity.mYubiKeyPin;
+        changeKeyringParcel.mCardAdminPin = createKeyActivity.mYubiKeyAdminPin;
+
         CryptoOperationHelper.Callback<SaveKeyringParcel, EditKeyResult> callback
                 = new CryptoOperationHelper.Callback<SaveKeyringParcel, EditKeyResult>() {
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyPassphraseFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyPassphraseFragment.java
index 3379e0a6d..6de5e71b3 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyPassphraseFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyPassphraseFragment.java
@@ -107,8 +107,8 @@ public class CreateKeyPassphraseFragment extends Fragment {
         // initial values
         // TODO: using String here is unsafe...
         if (mCreateKeyActivity.mPassphrase != null) {
-            mPassphraseEdit.setText(new String(mCreateKeyActivity.mPassphrase.getCharArray()));
-            mPassphraseEditAgain.setText(new String(mCreateKeyActivity.mPassphrase.getCharArray()));
+            mPassphraseEdit.setText(mCreateKeyActivity.mPassphrase.toStringUnsafe());
+            mPassphraseEditAgain.setText(mCreateKeyActivity.mPassphrase.toStringUnsafe());
         }
 
         mPassphraseEdit.requestFocus();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinFragment.java
index 8744762fe..a793b31f2 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinFragment.java
@@ -29,6 +29,7 @@ import android.widget.TextView;
 
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.ui.CreateKeyActivity.FragAction;
+import org.sufficientlysecure.keychain.util.Passphrase;
 
 import java.security.SecureRandom;
 
@@ -63,30 +64,38 @@ public class CreateYubiKeyPinFragment extends Fragment {
         mNextButton = view.findViewById(R.id.create_key_next_button);
 
         if (mCreateKeyActivity.mYubiKeyPin == null) {
-            new AsyncTask<Void, Void, Pair<String, String>>() {
+            new AsyncTask<Void, Void, Pair<Passphrase, Passphrase>>() {
                 @Override
-                protected Pair<String, String> doInBackground(Void... unused) {
+                protected Pair<Passphrase, Passphrase> doInBackground(Void... unused) {
                     SecureRandom secureRandom = new SecureRandom();
                     // min = 6, we choose 6
-                    String pin = "" + secureRandom.nextInt(999999);
+                    String pin = "" + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9);
                     // min = 8, we choose 10, but 6 are equals the PIN
-                    String adminPin = pin + secureRandom.nextInt(9999);
+                    String adminPin = pin + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9)
+                            + secureRandom.nextInt(9);
 
-                    return new Pair<>(pin, adminPin);
+                    return new Pair<>(new Passphrase(pin), new Passphrase(adminPin));
                 }
 
                 @Override
-                protected void onPostExecute(Pair<String, String> pair) {
+                protected void onPostExecute(Pair<Passphrase, Passphrase> pair) {
                     mCreateKeyActivity.mYubiKeyPin = pair.first;
                     mCreateKeyActivity.mYubiKeyAdminPin = pair.second;
 
-                    mPin.setText(mCreateKeyActivity.mYubiKeyPin);
-                    mAdminPin.setText(mCreateKeyActivity.mYubiKeyAdminPin);
+                    mPin.setText(mCreateKeyActivity.mYubiKeyPin.toStringUnsafe());
+                    mAdminPin.setText(mCreateKeyActivity.mYubiKeyAdminPin.toStringUnsafe());
                 }
             }.execute();
         } else {
-            mPin.setText(mCreateKeyActivity.mYubiKeyPin);
-            mAdminPin.setText(mCreateKeyActivity.mYubiKeyAdminPin);
+            mPin.setText(mCreateKeyActivity.mYubiKeyPin.toStringUnsafe());
+            mAdminPin.setText(mCreateKeyActivity.mYubiKeyAdminPin.toStringUnsafe());
         }
 
         mBackButton.setOnClickListener(new View.OnClickListener() {
@@ -114,9 +123,6 @@ public class CreateYubiKeyPinFragment extends Fragment {
 
 
     private void nextClicked() {
-        // save state
-//            mCreateKeyActivity.mPassphrase = new Passphrase(mPassphraseEdit);
-
         CreateYubiKeyPinRepeatFragment frag = CreateYubiKeyPinRepeatFragment.newInstance();
         mCreateKeyActivity.loadFragment(frag, FragAction.TO_RIGHT);
     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinRepeatFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinRepeatFragment.java
index dc437577a..2e752e609 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinRepeatFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyPinRepeatFragment.java
@@ -124,9 +124,9 @@ public class CreateYubiKeyPinRepeatFragment extends Fragment {
 
     private void nextClicked() {
         if (isEditTextNotEmpty(getActivity(), mPin)
-                && checkPin(getActivity(), mPin, mCreateKeyActivity.mYubiKeyPin)
+                && checkPin(getActivity(), mPin, mCreateKeyActivity.mYubiKeyPin.toStringUnsafe())
                 && isEditTextNotEmpty(getActivity(), mAdminPin)
-                && checkPin(getActivity(), mAdminPin, mCreateKeyActivity.mYubiKeyAdminPin)) {
+                && checkPin(getActivity(), mAdminPin, mCreateKeyActivity.mYubiKeyAdminPin.toStringUnsafe())) {
 
             CreateKeyFinalFragment frag = CreateKeyFinalFragment.newInstance();
             hideKeyboard();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
index addfb6a23..8a455bcec 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
@@ -80,16 +80,16 @@ public class NfcOperationActivity extends BaseNfcActivity {
 
         switch (mRequiredInput.mType) {
             case NFC_DECRYPT: {
-                for (int i = 0; i < mRequiredInput.mInputHashes.length; i++) {
-                    byte[] hash = mRequiredInput.mInputHashes[i];
-                    byte[] decryptedSessionKey = nfcDecryptSessionKey(hash);
-                    inputParcel.addCryptoData(hash, decryptedSessionKey);
+                for (int i = 0; i < mRequiredInput.mInputData.length; i++) {
+                    byte[] encryptedSessionKey = mRequiredInput.mInputData[i];
+                    byte[] decryptedSessionKey = nfcDecryptSessionKey(encryptedSessionKey);
+                    inputParcel.addCryptoData(encryptedSessionKey, decryptedSessionKey);
                 }
                 break;
             }
             case NFC_SIGN: {
-                for (int i = 0; i < mRequiredInput.mInputHashes.length; i++) {
-                    byte[] hash = mRequiredInput.mInputHashes[i];
+                for (int i = 0; i < mRequiredInput.mInputData.length; i++) {
+                    byte[] hash = mRequiredInput.mInputData[i];
                     int algo = mRequiredInput.mSignAlgos[i];
                     byte[] signedHash = nfcCalculateSignature(hash, algo);
                     inputParcel.addCryptoData(hash, signedHash);
@@ -97,6 +97,10 @@ public class NfcOperationActivity extends BaseNfcActivity {
                 break;
             }
             case NFC_MOVE_KEY_TO_CARD: {
+                // TODO: assume PIN and Admin PIN to be default for this operation
+                mPin = new Passphrase("123456");
+                mAdminPin = new Passphrase("12345678");
+
                 ProviderHelper providerHelper = new ProviderHelper(this);
                 CanonicalizedSecretKeyRing secretKeyRing;
                 try {
@@ -107,8 +111,11 @@ public class NfcOperationActivity extends BaseNfcActivity {
                     throw new IOException("Couldn't find subkey for key to card operation.");
                 }
 
-                for (int i = 0; i < mRequiredInput.mInputHashes.length; i++) {
-                    byte[] subkeyBytes = mRequiredInput.mInputHashes[i];
+                byte[] newPin = mRequiredInput.mInputData[0];
+                byte[] newAdminPin = mRequiredInput.mInputData[1];
+
+                for (int i = 2; i < mRequiredInput.mInputData.length; i++) {
+                    byte[] subkeyBytes = mRequiredInput.mInputData[i];
                     ByteBuffer buf = ByteBuffer.wrap(subkeyBytes);
                     long subkeyId = buf.getLong();
 
@@ -155,8 +162,18 @@ public class NfcOperationActivity extends BaseNfcActivity {
                         throw new IOException("Inappropriate key flags for smart card key.");
                     }
 
+                    // TODO: Is this really needed?
                     inputParcel.addCryptoData(subkeyBytes, cardSerialNumber);
                 }
+
+                // change PINs afterwards
+                nfcModifyPIN(0x81, newPin);
+                nfcModifyPIN(0x83, newAdminPin);
+
+                break;
+            }
+            default: {
+                throw new AssertionError("Unhandled mRequiredInput.mType");
             }
         }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java
index bede16b2a..ba8dd3b55 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java
@@ -179,8 +179,10 @@ public abstract class BaseNfcActivity extends BaseActivity {
                 Notify.create(this, getString(R.string.error_nfc_unknown), Style.WARN).show();
                 break;
             }
-            default:
+            default: {
                 Notify.create(this, getString(R.string.error_nfc, e.getMessage()), Style.WARN).show();
+                break;
+            }
         }
 
     }
@@ -311,9 +313,6 @@ public abstract class BaseNfcActivity extends BaseActivity {
         mPw1ValidatedForDecrypt = false;
         mPw3Validated = false;
 
-        // TODO: Handle non-default Admin PIN
-        mAdminPin = new Passphrase("12345678");
-
         onNfcPerform();
 
         mIsoDep.close();
@@ -569,12 +568,12 @@ public abstract class BaseNfcActivity extends BaseActivity {
      */
     public void nfcVerifyPIN(int mode) throws IOException {
         if (mPin != null || mode == 0x83) {
-            byte[] pin;
 
+            byte[] pin;
             if (mode == 0x83) {
-                pin = new String(mAdminPin.getCharArray()).getBytes();
+                pin = mAdminPin.toStringUnsafe().getBytes();
             } else {
-                pin = new String(mPin.getCharArray()).getBytes();
+                pin = mPin.toStringUnsafe().getBytes();
             }
 
             // SW1/2 0x9000 is the generic "ok" response, which we expect most of the time.
@@ -611,12 +610,11 @@ public abstract class BaseNfcActivity extends BaseActivity {
      * @param pw For PW1, this is 0x81. For PW3 (Admin PIN), mode is 0x83.
      * @param newPinString The new PW1 or PW3.
      */
-    public void nfcModifyPIN(int pw, String newPinString) throws IOException {
+    public void nfcModifyPIN(int pw, byte[] newPin) throws IOException {
         final int MAX_PW1_LENGTH_INDEX = 1;
         final int MAX_PW3_LENGTH_INDEX = 3;
 
         byte[] pwStatusBytes = nfcGetPwStatusBytes();
-        byte[] newPin = newPinString.getBytes();
 
         if (pw == 0x81) {
             if (newPin.length < 6 || newPin.length > pwStatusBytes[MAX_PW1_LENGTH_INDEX]) {
@@ -631,11 +629,10 @@ public abstract class BaseNfcActivity extends BaseActivity {
         }
 
         byte[] pin;
-
         if (pw == 0x83) {
-            pin = new String(mAdminPin.getCharArray()).getBytes();
+            pin = mAdminPin.toStringUnsafe().getBytes();
         } else {
-            pin = new String(mPin.getCharArray()).getBytes();
+            pin = mPin.toStringUnsafe().getBytes();
         }
 
         // Command APDU for CHANGE REFERENCE DATA command (page 32)
@@ -700,7 +697,7 @@ public abstract class BaseNfcActivity extends BaseActivity {
             throw new IOException("Invalid key slot");
         }
 
-        RSAPrivateCrtKey crtSecretKey = null;
+        RSAPrivateCrtKey crtSecretKey;
         try {
             secretKey.unlock(passphrase);
             crtSecretKey = secretKey.getCrtSecretKey();
@@ -719,7 +716,7 @@ public abstract class BaseNfcActivity extends BaseActivity {
         }
 
         if (!mPw3Validated) {
-            nfcVerifyPIN(0x83); // (Verify PW1 with mode 83)
+            nfcVerifyPIN(0x83); // (Verify PW3 with mode 83)
         }
 
         byte[] header= Hex.decode(