OTG: Refactor persistent connections, naming

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/NfcTransport.java

@@ -1,18 +1,22 @@
 package org.sufficientlysecure.keychain.smartcard;
 
+import android.nfc.Tag;
+
+import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenNfcActivity;
+
 import java.io.IOException;
 
 import nordpol.IsoCard;
+import nordpol.android.AndroidCard;
 
 public class NfcTransport implements Transport {
     // timeout is set to 100 seconds to avoid cancellation during calculation
     private static final int TIMEOUT = 100 * 1000;
-    private final IsoCard mIsoCard;
+    private final Tag mTag;
+    private IsoCard mIsoCard;
 
-    public NfcTransport(final IsoCard isoDep) throws IOException {
+    public NfcTransport(Tag tag) {
-        this.mIsoCard = isoDep;
+        this.mTag = tag;
-        mIsoCard.setTimeout(TIMEOUT);
-        mIsoCard.connect();
     }
 
     @Override
@@ -26,11 +30,52 @@ public class NfcTransport implements Transport {
 
     @Override
     public boolean isConnected() {
-        return mIsoCard.isConnected();
+        return mIsoCard != null && mIsoCard.isConnected();
     }
 
     @Override
     public boolean allowPersistentConnection() {
         return false;
     }
+
+    /**
+     * Handle NFC communication and return a result.
+     * <p/>
+     * On general communication, see also
+     * http://www.cardwerk.com/smartcards/smartcard_standard_ISO7816-4_annex-a.aspx
+     * <p/>
+     * References to pages are generally related to the OpenPGP Application
+     * on ISO SmartCard Systems specification.
+     */
+    @Override
+    public void connect() throws IOException {
+        mIsoCard = AndroidCard.get(mTag);
+        if (mIsoCard == null) {
+            throw new BaseSecurityTokenNfcActivity.IsoDepNotSupportedException("Tag does not support ISO-DEP (ISO 14443-4)");
+        }
+
+        mIsoCard.setTimeout(TIMEOUT);
+        mIsoCard.connect();
+    }
+
+    @Override
+    public boolean equals(final Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        final NfcTransport that = (NfcTransport) o;
+
+        if (mTag != null ? !mTag.equals(that.mTag) : that.mTag != null) return false;
+        if (mIsoCard != null ? !mIsoCard.equals(that.mIsoCard) : that.mIsoCard != null)
+            return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = mTag != null ? mTag.hashCode() : 0;
+        result = 31 * result + (mIsoCard != null ? mIsoCard.hashCode() : 0);
+        return result;
+    }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/SmartcardDevice.java

@@ -112,6 +112,8 @@ public class SmartcardDevice {
 
     // METHOD UPDATED OK
     public void connectToDevice() throws IOException {
+        mTransport.connect();
+
         // SW1/2 0x9000 is the generic "ok" response, which we expect most of the time.
         // See specification, page 51
         String accepted = "9000";
@@ -722,7 +724,7 @@ public class SmartcardDevice {
         this.mTransport = mTransport;
     }
 
-    public boolean allowPersistentConnection() {
+    public boolean isPersistentConnectionAllowed() {
         return mTransport != null && mTransport.allowPersistentConnection();
     }
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/Transport.java

@@ -10,4 +10,6 @@ public interface Transport {
     boolean isConnected();
 
     boolean allowPersistentConnection();
+
+    void connect() throws IOException;
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbConnectionManager.java

@@ -51,12 +51,4 @@ public class UsbConnectionManager {
     public void onStop() {
         mActivity.unregisterReceiver(mUsbReceiver);
     }
-
-    public void rescanDevices() {
-        final SmartcardDevice smartcardDevice = SmartcardDevice.getInstance();
-        if (smartcardDevice.isConnected()
-                && (smartcardDevice.getTransport() instanceof UsbTransport)) {
-            mListener.usbDeviceDiscovered(((UsbTransport) smartcardDevice.getTransport()).getUsbDevice());
-        }
-    }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbTransport.java

@@ -15,6 +15,7 @@ import org.bouncycastle.util.encoders.Hex;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.util.Log;
 
+import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 
@@ -24,30 +25,15 @@ public class UsbTransport implements Transport {
 
     private final UsbManager mUsbManager;
     private final UsbDevice mUsbDevice;
-    private final UsbInterface mUsbInterface;
+    private UsbInterface mUsbInterface;
-    private final UsbEndpoint mBulkIn;
+    private UsbEndpoint mBulkIn;
-    private final UsbEndpoint mBulkOut;
+    private UsbEndpoint mBulkOut;
-    private final UsbDeviceConnection mConnection;
+    private UsbDeviceConnection mConnection;
     private byte mCounter = 0;
 
-    public UsbTransport(final UsbDevice usbDevice, final UsbManager usbManager) throws TransportIoException {
+    public UsbTransport(final UsbDevice usbDevice, final UsbManager usbManager) {
         mUsbDevice = usbDevice;
         mUsbManager = usbManager;
-
-        mUsbInterface = getSmartCardInterface(mUsbDevice);
-        // throw if mUsbInterface == null
-        final Pair<UsbEndpoint, UsbEndpoint> ioEndpoints = getIoEndpoints(mUsbInterface);
-        mBulkIn = ioEndpoints.first;
-        mBulkOut = ioEndpoints.second;
-        // throw if any endpoint is null
-
-        mConnection = mUsbManager.openDevice(mUsbDevice);
-        // throw if connection is null
-        mConnection.claimInterface(mUsbInterface, true);
-        // check result
-
-        powerOn();
-        Log.d(Constants.TAG, "Usb transport connected");
     }
 
     private void powerOff() throws TransportIoException {
@@ -120,7 +106,7 @@ public class UsbTransport implements Transport {
     @Override
     public boolean isConnected() {
         // TODO: redo
-        return mUsbManager.getDeviceList().containsValue(mUsbDevice);
+        return mConnection != null && mUsbManager.getDeviceList().containsValue(mUsbDevice);
     }
 
     @Override
@@ -128,6 +114,24 @@ public class UsbTransport implements Transport {
         return true;
     }
 
+    @Override
+    public void connect() throws IOException {
+        mUsbInterface = getSmartCardInterface(mUsbDevice);
+        // throw if mUsbInterface == null
+        final Pair<UsbEndpoint, UsbEndpoint> ioEndpoints = getIoEndpoints(mUsbInterface);
+        mBulkIn = ioEndpoints.first;
+        mBulkOut = ioEndpoints.second;
+        // throw if any endpoint is null
+
+        mConnection = mUsbManager.openDevice(mUsbDevice);
+        // throw if connection is null
+        mConnection.claimInterface(mUsbInterface, true);
+        // check result
+
+        powerOn();
+        Log.d(Constants.TAG, "Usb transport connected");
+    }
+
     @Override
     public byte[] sendAndReceive(byte[] data) throws TransportIoException {
         send(data);
@@ -208,4 +212,19 @@ public class UsbTransport implements Transport {
     public UsbDevice getUsbDevice() {
         return mUsbDevice;
     }
+
+    @Override
+    public boolean equals(final Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        final UsbTransport that = (UsbTransport) o;
+
+        return mUsbDevice != null ? mUsbDevice.equals(that.mUsbDevice) : that.mUsbDevice == null;
+    }
+
+    @Override
+    public int hashCode() {
+        return mUsbDevice != null ? mUsbDevice.hashCode() : 0;
+    }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java

@@ -155,7 +155,7 @@ public class CreateKeyActivity extends BaseSecurityTokenNfcActivity {
     }
 
     @Override
-    protected void onNfcPostExecute() {
+    protected void onSmartcardPostExecute() {
         if (mCurrentFragment instanceof NfcListenerFragment) {
             ((NfcListenerFragment) mCurrentFragment).onNfcPostExecute();
             return;

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java

@@ -142,8 +142,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenNfcActivity
             obtainSecurityTokenPin(mRequiredInput);
             checkPinAvailability();
         } else {
-            // No need for pin, rescan USB devices
+            checkDeviceConnection();
-            mUsbDispatcher.rescanDevices();
         }
     }
 
@@ -160,8 +159,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenNfcActivity
             Passphrase passphrase = PassphraseCacheService.getCachedPassphrase(this,
                     mRequiredInput.getMasterKeyId(), mRequiredInput.getSubKeyId());
             if (passphrase != null) {
-                // Rescan USB devices
+                checkDeviceConnection();
-                mUsbDispatcher.rescanDevices();
             }
         } catch (PassphraseCacheService.KeyNotFoundException e) {
             throw new AssertionError(
@@ -175,7 +173,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenNfcActivity
     }
 
     @Override
-    public void onNfcPreExecute() {
+    public void onSmartcardPreExecute() {
         // start with indeterminate progress
         vAnimator.setDisplayedChild(1);
         nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.TRANSFERRING);
@@ -293,7 +291,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenNfcActivity
     }
 
     @Override
-    protected final void onNfcPostExecute() {
+    protected final void onSmartcardPostExecute() {
         handleResult(mInputParcel);
 
         // show finish
@@ -301,7 +299,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenNfcActivity
 
         nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE);
 
-        if (mSmartcardDevice.allowPersistentConnection()) {
+        if (mSmartcardDevice.isPersistentConnectionAllowed()) {
             // Just close
             finish();
         } else {

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java

@@ -655,7 +655,7 @@ public class ViewKeyActivity extends BaseSecurityTokenNfcActivity implements
     }
 
     @Override
-    protected void onNfcPostExecute() {
+    protected void onSmartcardPostExecute() {
 
         long tokenId = KeyFormattingUtils.getKeyIdFromFingerprint(mNfcFingerprints);
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenNfcActivity.java

@@ -43,6 +43,7 @@ import org.sufficientlysecure.keychain.service.input.RequiredInputParcel;
 import org.sufficientlysecure.keychain.smartcard.NfcTransport;
 import org.sufficientlysecure.keychain.smartcard.OnDiscoveredUsbDeviceListener;
 import org.sufficientlysecure.keychain.smartcard.SmartcardDevice;
+import org.sufficientlysecure.keychain.smartcard.Transport;
 import org.sufficientlysecure.keychain.smartcard.UsbConnectionManager;
 import org.sufficientlysecure.keychain.smartcard.UsbTransport;
 import org.sufficientlysecure.keychain.ui.CreateKeyActivity;
@@ -83,7 +84,7 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
     /**
      * Override to change UI before NFC handling (UI thread)
      */
-    protected void onNfcPreExecute() {
+    protected void onSmartcardPreExecute() {
     }
 
     /**
@@ -98,7 +99,7 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
     /**
      * Override to handle result of NFC operations (UI thread)
      */
-    protected void onNfcPostExecute() {
+    protected void onSmartcardPostExecute() {
 
         final long subKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(mNfcFingerprints);
 
@@ -140,54 +141,34 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
         // Actual NFC operations are executed in doInBackground to not block the UI thread
         if (!mTagHandlingEnabled)
             return;
-        new AsyncTask<Void, Void, IOException>() {
-            @Override
-            protected void onPreExecute() {
-                super.onPreExecute();
-                onNfcPreExecute();
-            }
 
-            @Override
+        smartcardDiscovered(new NfcTransport(tag));
-            protected IOException doInBackground(Void... params) {
-                try {
-                    handleTagDiscovered(tag);
-                } catch (IOException e) {
-                    return e;
-                }
-
-                return null;
-            }
-
-            @Override
-            protected void onPostExecute(IOException exception) {
-                super.onPostExecute(exception);
-
-                if (exception != null) {
-                    handleNfcError(exception);
-                    return;
-                }
-
-                onNfcPostExecute();
-            }
-        }.execute();
     }
 
-
     public void usbDeviceDiscovered(final UsbDevice device) {
         // Actual USB operations are executed in doInBackground to not block the UI thread
+        if (!mTagHandlingEnabled)
+            return;
+
+        UsbManager usbManager = (UsbManager) getSystemService(USB_SERVICE);
+        smartcardDiscovered(new UsbTransport(device, usbManager));
+    }
+
+    public void smartcardDiscovered(final Transport transport) {
+        // Actual Smartcard operations are executed in doInBackground to not block the UI thread
         if (!mTagHandlingEnabled)
             return;
         new AsyncTask<Void, Void, IOException>() {
             @Override
             protected void onPreExecute() {
                 super.onPreExecute();
-                onNfcPreExecute();
+                onSmartcardPreExecute();
             }
 
             @Override
             protected IOException doInBackground(Void... params) {
                 try {
-                    handleUsbDevice(device);
+                    handleSmartcard(transport);
                 } catch (IOException e) {
                     return e;
                 }
@@ -200,11 +181,11 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
                 super.onPostExecute(exception);
 
                 if (exception != null) {
-                    handleNfcError(exception);
+                    handleSmartcardError(exception);
                     return;
                 }
 
-                onNfcPostExecute();
+                onSmartcardPostExecute();
             }
         }.execute();
     }
@@ -256,7 +237,7 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
         mTagDispatcher.interceptIntent(intent);
     }
 
-    private void handleNfcError(IOException e) {
+    private void handleSmartcardError(IOException e) {
 
         if (e instanceof TagLostException) {
             onNfcError(getString(R.string.security_token_error_tag_lost));
@@ -422,42 +403,11 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
         }
     }
 
-    /**
+    protected void handleSmartcard(Transport transport) throws IOException {
-     * Handle NFC communication and return a result.
-     * <p/>
-     * This method is called by onNewIntent above upon discovery of an NFC tag.
-     * It handles initialization and login to the application, subsequently
-     * calls either nfcCalculateSignature() or nfcDecryptSessionKey(), then
-     * finishes the activity with an appropriate result.
-     * <p/>
-     * On general communication, see also
-     * http://www.cardwerk.com/smartcards/smartcard_standard_ISO7816-4_annex-a.aspx
-     * <p/>
-     * References to pages are generally related to the OpenPGP Application
-     * on ISO SmartCard Systems specification.
-     */
-    protected void handleTagDiscovered(Tag tag) throws IOException {
-
-        // Connect to the detected tag, setting a couple of settings
-        IsoCard isoCard = AndroidCard.get(tag);
-        if (isoCard == null) {
-            throw new IsoDepNotSupportedException("Tag does not support ISO-DEP (ISO 14443-4)");
-        }
-
-        mSmartcardDevice.setTransport(new NfcTransport(isoCard));
-        mSmartcardDevice.connectToDevice();
-
-        doNfcInBackground();
-    }
-
-    protected void handleUsbDevice(UsbDevice device) throws IOException {
         // Don't reconnect if device was already connected
-        if (!mSmartcardDevice.isConnected()
+        if (!(mSmartcardDevice.isConnected()
-                || !(mSmartcardDevice.getTransport() instanceof UsbTransport)
+                && mSmartcardDevice.getTransport().equals(transport))) {
-                || !((UsbTransport) mSmartcardDevice.getTransport()).getUsbDevice().equals(device)) {
+            mSmartcardDevice.setTransport(transport);
-            UsbManager usbManager = (UsbManager) getSystemService(USB_SERVICE);
-
-            mSmartcardDevice.setTransport(new UsbTransport(device, usbManager));
             mSmartcardDevice.connectToDevice();
         }
         doNfcInBackground();
@@ -467,7 +417,7 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
         return mSmartcardDevice.isConnected();
     }
 
-    public class IsoDepNotSupportedException extends IOException {
+    public static class IsoDepNotSupportedException extends IOException {
 
         public IsoDepNotSupportedException(String detailMessage) {
             super(detailMessage);
@@ -540,4 +490,10 @@ public abstract class BaseSecurityTokenNfcActivity extends BaseActivity
     public SmartcardDevice getSmartcardDevice() {
         return mSmartcardDevice;
     }
+
+    protected void checkDeviceConnection() {
+        if (mSmartcardDevice.isConnected() && mSmartcardDevice.isPersistentConnectionAllowed()) {
+            this.smartcardDiscovered(mSmartcardDevice.getTransport());
+        }
+    }
 }