Merge pull request #2186 from open-keychain/apdu-refactor

Refactor OpenPGP applet communication code
2017-10-26 12:26:55 +02:00
parent 07243bcefe 7eb37a89d8
commit 60b970b060
25 changed files with 1094 additions and 1198 deletions
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java
@@ -32,6 +32,7 @@ import android.support.v4.app.TaskStackBuilder;

 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.securitytoken.KeyFormat;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo;
 import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity;
 import org.sufficientlysecure.keychain.ui.token.ManageSecurityTokenFragment;
@@ -133,17 +134,17 @@ public class CreateKeyActivity extends BaseSecurityTokenActivity {
    }

    @Override
-    protected void doSecurityTokenInBackground() throws IOException {
+    protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException {
        if (mCurrentFragment instanceof SecurityTokenListenerFragment) {
            ((SecurityTokenListenerFragment) mCurrentFragment).doSecurityTokenInBackground();
            return;
        }

-        tokenInfo = mSecurityTokenHelper.getTokenInfo();
+        tokenInfo = stConnection.getTokenInfo();
    }

    @Override
-    protected void onSecurityTokenPostExecute() {
+    protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) {
        handleTokenInfo(tokenInfo);
    }

--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenAlgorithmFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenAlgorithmFragment.java
@@ -32,7 +32,7 @@ import android.widget.TextView;

 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.securitytoken.KeyFormat;
-import org.sufficientlysecure.keychain.securitytoken.SecurityTokenHelper;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.ui.CreateKeyActivity.FragAction;
 import org.sufficientlysecure.keychain.util.Choice;

@@ -100,7 +100,7 @@ public class CreateSecurityTokenAlgorithmFragment extends Fragment {
        choices.add(new Choice<>(SupportedKeyType.RSA_4096, getResources().getString(
                R.string.rsa_4096), getResources().getString(R.string.rsa_4096_description_html)));

-        final double version = SecurityTokenHelper.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid());
+        final double version = SecurityTokenConnection.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid());

        if (version >= 3.0) {
            choices.add(new Choice<>(SupportedKeyType.ECC_P256, getResources().getString(
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenPinFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenPinFragment.java
@@ -17,7 +17,6 @@

 package org.sufficientlysecure.keychain.ui;

-import android.app.Activity;
 import android.content.Context;
 import android.os.AsyncTask;
 import android.os.Bundle;
@@ -31,7 +30,7 @@ import android.widget.TextView;

 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
-import org.sufficientlysecure.keychain.securitytoken.SecurityTokenHelper;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.ui.CreateKeyActivity.FragAction;
 import org.sufficientlysecure.keychain.util.Passphrase;

@@ -206,7 +205,7 @@ public class CreateSecurityTokenPinFragment extends Fragment {

            mCreateKeyActivity.mSecurityTokenPin = new Passphrase(mPin.getText().toString());

-            final double version = SecurityTokenHelper.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid());
+            final double version = SecurityTokenConnection.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid());

            Fragment frag;
            if (version >= 3.0) {
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java
@@ -32,6 +32,7 @@ import android.widget.ViewAnimator;
 import nordpol.android.NfcGuideView;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo;
 import org.sufficientlysecure.keychain.service.input.SecurityTokenChangePinParcel;
 import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity;
@@ -138,15 +139,15 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc
    }

    @Override
-    protected void doSecurityTokenInBackground() throws IOException {
-        mSecurityTokenHelper.setAdminPin(new Passphrase(changePinInput.getAdminPin()));
-        mSecurityTokenHelper.resetPin(changePinInput.getNewPin());
+    protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException {
+        Passphrase adminPin = new Passphrase(changePinInput.getAdminPin());
+        stConnection.resetPin(changePinInput.getNewPin().getBytes(), adminPin);

-        resultTokenInfo = mSecurityTokenHelper.getTokenInfo();
+        resultTokenInfo = stConnection.getTokenInfo();
    }

    @Override
-    protected final void onSecurityTokenPostExecute() {
+    protected final void onSecurityTokenPostExecute(final SecurityTokenConnection stConnection) {
        Intent result = new Intent();
        result.putExtra(RESULT_TOKEN_INFO, resultTokenInfo);
        setResult(RESULT_OK, result);
@@ -156,17 +157,17 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc

        nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE);

-        if (mSecurityTokenHelper.isPersistentConnectionAllowed()) {
+        if (stConnection.isPersistentConnectionAllowed()) {
            // Just close
            finish();
        } else {
-            mSecurityTokenHelper.clearSecureMessaging();
+            stConnection.clearSecureMessaging();
            new AsyncTask<Void, Void, Void>() {
                @Override
                protected Void doInBackground(Void... params) {
                    // check all 200ms if Security Token has been taken away
                    while (true) {
-                        if (isSecurityTokenConnected()) {
+                        if (stConnection.isConnected()) {
                            try {
                                Thread.sleep(200);
                            } catch (InterruptedException ignored) {
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java
@@ -44,6 +44,7 @@ import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.provider.KeyRepository;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.securitytoken.KeyType;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
@@ -185,12 +186,12 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {
    }

    @Override
-    protected void doSecurityTokenInBackground() throws IOException {
+    protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException {

        switch (mRequiredInput.mType) {
            case SECURITY_TOKEN_DECRYPT: {
                long tokenKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(
-                        mSecurityTokenHelper.getKeyFingerprint(KeyType.ENCRYPT));
+                        stConnection.getKeyFingerprint(KeyType.ENCRYPT));

                if (tokenKeyId != mRequiredInput.getSubKeyId()) {
                    throw new IOException(getString(R.string.error_wrong_security_token));
@@ -208,14 +209,15 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {

                for (int i = 0; i < mRequiredInput.mInputData.length; i++) {
                    byte[] encryptedSessionKey = mRequiredInput.mInputData[i];
-                    byte[] decryptedSessionKey = mSecurityTokenHelper.decryptSessionKey(encryptedSessionKey, publicKeyRing.getPublicKey(tokenKeyId));
+                    byte[] decryptedSessionKey = stConnection
+                            .decryptSessionKey(encryptedSessionKey, publicKeyRing.getPublicKey(tokenKeyId));
                    mInputParcel = mInputParcel.withCryptoData(encryptedSessionKey, decryptedSessionKey);
                }
                break;
            }
            case SECURITY_TOKEN_SIGN: {
                long tokenKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(
-                        mSecurityTokenHelper.getKeyFingerprint(KeyType.SIGN));
+                        stConnection.getKeyFingerprint(KeyType.SIGN));

                if (tokenKeyId != mRequiredInput.getSubKeyId()) {
                    throw new IOException(getString(R.string.error_wrong_security_token));
@@ -226,15 +228,13 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {
                for (int i = 0; i < mRequiredInput.mInputData.length; i++) {
                    byte[] hash = mRequiredInput.mInputData[i];
                    int algo = mRequiredInput.mSignAlgos[i];
-                    byte[] signedHash = mSecurityTokenHelper.calculateSignature(hash, algo);
+                    byte[] signedHash = stConnection.calculateSignature(hash, algo);
                    mInputParcel = mInputParcel.withCryptoData(hash, signedHash);
                }
                break;
            }
            case SECURITY_TOKEN_MOVE_KEY_TO_CARD: {
-                // TODO: assume PIN and Admin PIN to be default for this operation
-                mSecurityTokenHelper.setPin(new Passphrase("123456"));
-                mSecurityTokenHelper.setAdminPin(new Passphrase("12345678"));
+                Passphrase adminPin = new Passphrase("12345678");

                KeyRepository keyRepository =
                        KeyRepository.create(this);
@@ -256,7 +256,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {
                    long subkeyId = buf.getLong();

                    CanonicalizedSecretKey key = secretKeyRing.getSecretKey(subkeyId);
-                    byte[] tokenSerialNumber = Arrays.copyOf(mSecurityTokenHelper.getAid(), 16);
+                    byte[] tokenSerialNumber = Arrays.copyOf(stConnection.getAid(), 16);

                    Passphrase passphrase;
                    try {
@@ -266,21 +266,21 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {
                        throw new IOException("Unable to get cached passphrase!");
                    }

-                    mSecurityTokenHelper.changeKey(key, passphrase);
+                    stConnection.changeKey(key, passphrase, adminPin);

                    // TODO: Is this really used anywhere?
                    mInputParcel = mInputParcel.withCryptoData(subkeyBytes, tokenSerialNumber);
                }

                // change PINs afterwards
-                mSecurityTokenHelper.modifyPin(0x81, newPin);
-                mSecurityTokenHelper.modifyPin(0x83, newAdminPin);
+                stConnection.resetPin(newPin, adminPin);
+                stConnection.modifyPw3Pin(newAdminPin, adminPin);

                break;
            }
            case SECURITY_TOKEN_RESET_CARD: {
-                mSecurityTokenHelper.resetAndWipeToken();
-                mResultTokenInfo = mSecurityTokenHelper.getTokenInfo();
+                stConnection.resetAndWipeToken();
+                mResultTokenInfo = stConnection.getTokenInfo();

                break;
            }
@@ -292,7 +292,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {
    }

    @Override
-    protected final void onSecurityTokenPostExecute() {
+    protected final void onSecurityTokenPostExecute(final SecurityTokenConnection stConnection) {
        handleResult(mInputParcel);

        // show finish
@@ -300,17 +300,17 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity {

        nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE);

-        if (mSecurityTokenHelper.isPersistentConnectionAllowed()) {
+        if (stConnection.isPersistentConnectionAllowed()) {
            // Just close
            finish();
        } else {
-            mSecurityTokenHelper.clearSecureMessaging();
+            stConnection.clearSecureMessaging();
            new AsyncTask<Void, Void, Void>() {
                @Override
                protected Void doInBackground(Void... params) {
                    // check all 200ms if Security Token has been taken away
                    while (true) {
-                        if (isSecurityTokenConnected()) {
+                        if (stConnection.isConnected()) {
                            try {
                                Thread.sleep(200);
                            } catch (InterruptedException ignored) {
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ShowNfcSweetspotActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ShowNfcSweetspotActivity.java
@@ -15,6 +15,7 @@ import android.view.animation.DecelerateInterpolator;

 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.securitytoken.NfcSweetspotData;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity;


@@ -88,7 +89,7 @@ public class ShowNfcSweetspotActivity extends BaseSecurityTokenActivity {
    }

    @Override
-    protected void onSecurityTokenPostExecute() {
+    protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) {
        Intent result = new Intent();
        result.putExtra(EXTRA_TOKEN_INFO, tokenInfo);
        setResult(Activity.RESULT_OK, result);
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java
@@ -43,7 +43,7 @@ import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.securitytoken.CardException;
 import org.sufficientlysecure.keychain.securitytoken.NfcTransport;
-import org.sufficientlysecure.keychain.securitytoken.SecurityTokenHelper;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo;
 import org.sufficientlysecure.keychain.securitytoken.Transport;
 import org.sufficientlysecure.keychain.securitytoken.UsbConnectionDispatcher;
@@ -68,12 +68,12 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity

    private static final String FIDESMO_APP_PACKAGE = "com.fidesmo.sec.android";

-    protected SecurityTokenHelper mSecurityTokenHelper = SecurityTokenHelper.getInstance();
    protected TagDispatcher mNfcTagDispatcher;
    protected UsbConnectionDispatcher mUsbDispatcher;
    private boolean mTagHandlingEnabled;

    protected SecurityTokenInfo tokenInfo;
+    private Passphrase mCachedPin;

    /**
     * Override to change UI before SecurityToken handling (UI thread)
@@ -84,15 +84,15 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
    /**
     * Override to implement SecurityToken operations (background thread)
     */
-    protected void doSecurityTokenInBackground() throws IOException {
-        tokenInfo = mSecurityTokenHelper.getTokenInfo();
+    protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException {
+        tokenInfo = stConnection.getTokenInfo();
        Log.d(Constants.TAG, "Security Token: " + tokenInfo);
    }

    /**
     * Override to handle result of SecurityToken operations (UI thread)
     */
-    protected void onSecurityTokenPostExecute() {
+    protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) {
        Intent intent = new Intent(this, CreateKeyActivity.class);
        intent.putExtra(CreateKeyActivity.EXTRA_SECURITY_TOKEN_INFO, tokenInfo);
        startActivity(intent);
@@ -138,6 +138,10 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
        // Actual Security Token operations are executed in doInBackground to not block the UI thread
        if (!mTagHandlingEnabled)
            return;
+
+        final SecurityTokenConnection stConnection =
+                SecurityTokenConnection.getInstanceForTransport(transport, mCachedPin);
+
        new AsyncTask<Void, Void, IOException>() {
            @Override
            protected void onPreExecute() {
@@ -148,7 +152,9 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
            @Override
            protected IOException doInBackground(Void... params) {
                try {
-                    handleSecurityToken(transport, BaseSecurityTokenActivity.this);
+                    stConnection.connectIfNecessary(getBaseContext());
+
+                    handleSecurityToken(stConnection);
                } catch (IOException e) {
                    return e;
                }
@@ -161,11 +167,11 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
                super.onPostExecute(exception);

                if (exception != null) {
-                    handleSecurityTokenError(exception);
+                    handleSecurityTokenError(stConnection, exception);
                    return;
                }

-                onSecurityTokenPostExecute();
+                onSecurityTokenPostExecute(stConnection);
            }
        }.execute();
    }
@@ -223,7 +229,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
        mNfcTagDispatcher.interceptIntent(intent);
    }

-    private void handleSecurityTokenError(IOException e) {
+    private void handleSecurityTokenError(SecurityTokenConnection stConnection, IOException e) {

        if (e instanceof TagLostException) {
            onSecurityTokenError(getString(R.string.security_token_error_tag_lost));
@@ -250,7 +256,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity

            SecurityTokenInfo tokeninfo = null;
            try {
-                tokeninfo = mSecurityTokenHelper.getTokenInfo();
+                tokeninfo = stConnection.getTokenInfo();
            } catch (IOException e2) {
                // don't care
            }
@@ -260,6 +266,8 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
            return;
        }

+        Log.d(Constants.TAG, "security token exception", e);
+
        // Otherwise, all status codes are fixed values.
        switch (status) {

@@ -271,7 +279,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
            case 0x6982: {
                SecurityTokenInfo tokeninfo = null;
                try {
-                    tokeninfo = mSecurityTokenHelper.getTokenInfo();
+                    tokeninfo = stConnection.getTokenInfo();
                } catch (IOException e2) {
                    // don't care
                }
@@ -325,7 +333,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
            }
            // 6A82 app not installed on security token!
            case 0x6A82: {
-                if (mSecurityTokenHelper.isFidesmoToken()) {
+                if (stConnection.isFidesmoToken()) {
                    // Check if the Fidesmo app is installed
                    if (isAndroidAppInstalled(FIDESMO_APP_PACKAGE)) {
                        promptFidesmoPgpInstall();
@@ -391,12 +399,11 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
    }

    protected void obtainSecurityTokenPin(RequiredInputParcel requiredInput) {
-
        try {
            Passphrase passphrase = PassphraseCacheService.getCachedPassphrase(this,
                    requiredInput.getMasterKeyId(), requiredInput.getSubKeyId());
            if (passphrase != null) {
-                mSecurityTokenHelper.setPin(passphrase);
+                mCachedPin = passphrase;
                return;
            }

@@ -421,7 +428,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
                    return;
                }
                CryptoInputParcel input = data.getParcelableExtra(PassphraseDialogActivity.RESULT_CRYPTO_INPUT);
-                mSecurityTokenHelper.setPin(input.getPassphrase());
+                mCachedPin = input.getPassphrase();
                break;
            }
            default:
@@ -429,19 +436,8 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
        }
    }

-    protected void handleSecurityToken(Transport transport, Context ctx) throws IOException {
-        // Don't reconnect if device was already connected
-        if (!(mSecurityTokenHelper.isPersistentConnectionAllowed()
-                && mSecurityTokenHelper.isConnected()
-                && mSecurityTokenHelper.getTransport().equals(transport))) {
-            mSecurityTokenHelper.setTransport(transport);
-            mSecurityTokenHelper.connectToDevice(ctx);
-        }
-        doSecurityTokenInBackground();
-    }
-
-    public boolean isSecurityTokenConnected() {
-        return mSecurityTokenHelper.isConnected();
+    protected void handleSecurityToken(SecurityTokenConnection stConnection) throws IOException {
+        doSecurityTokenInBackground(stConnection);
    }

    public static class IsoDepNotSupportedException extends IOException {
@@ -500,10 +496,6 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
        mUsbDispatcher.onStart();
    }

-    public SecurityTokenHelper getSecurityTokenHelper() {
-        return mSecurityTokenHelper;
-    }
-
    /**
     * Run Security Token routines if last used token is connected and supports
     * persistent connections
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyActivity.java
@@ -79,6 +79,7 @@ import org.sufficientlysecure.keychain.provider.KeyRepository;
 import org.sufficientlysecure.keychain.provider.KeyRepository.NotFoundException;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
+import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection;
 import org.sufficientlysecure.keychain.service.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.service.ImportKeyringParcel;
 import org.sufficientlysecure.keychain.service.input.RequiredInputParcel;
@@ -619,8 +620,8 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements
    }

    @Override
-    protected void onSecurityTokenPostExecute() {
-        super.onSecurityTokenPostExecute();
+    protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) {
+        super.onSecurityTokenPostExecute(stConnection);
        finish();
    }