From 4d9ce8e95b4604f753aa5f49fe2c243dc73a13a9 Mon Sep 17 00:00:00 2001 From: Nikita Mikhailov Date: Sat, 9 Apr 2016 13:13:02 +0600 Subject: OTG: Refactor persistent connections, naming --- .../keychain/smartcard/NfcTransport.java | 57 ++++++++++-- .../keychain/smartcard/SmartcardDevice.java | 4 +- .../keychain/smartcard/Transport.java | 2 + .../keychain/smartcard/UsbConnectionManager.java | 8 -- .../keychain/smartcard/UsbTransport.java | 61 +++++++----- .../keychain/ui/CreateKeyActivity.java | 2 +- .../ui/SecurityTokenOperationActivity.java | 12 +-- .../keychain/ui/ViewKeyActivity.java | 2 +- .../ui/base/BaseSecurityTokenNfcActivity.java | 102 ++++++--------------- 9 files changed, 132 insertions(+), 118 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/NfcTransport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/NfcTransport.java index d56f5b5bf..e47ba5360 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/NfcTransport.java +++ b/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 { - this.mIsoCard = isoDep; - mIsoCard.setTimeout(TIMEOUT); - mIsoCard.connect(); + public NfcTransport(Tag tag) { + this.mTag = tag; } @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. + *

+ * On general communication, see also + * http://www.cardwerk.com/smartcards/smartcard_standard_ISO7816-4_annex-a.aspx + *

+ * 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; + } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/SmartcardDevice.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/SmartcardDevice.java index 4420c0c88..286a38d1f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/SmartcardDevice.java +++ b/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(); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/Transport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/Transport.java index 9b0ad2998..5252a95d0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/Transport.java +++ b/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; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbConnectionManager.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbConnectionManager.java index e6fb5b04d..8a6971fe6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbConnectionManager.java +++ b/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()); - } - } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbTransport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbTransport.java index 2d435ccbe..2a21c10dd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/smartcard/UsbTransport.java +++ b/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 final UsbEndpoint mBulkIn; - private final UsbEndpoint mBulkOut; - private final UsbDeviceConnection mConnection; + private UsbInterface mUsbInterface; + private UsbEndpoint mBulkIn; + private UsbEndpoint mBulkOut; + 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 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 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; + } } 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 268dbad02..07d5be821 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java +++ b/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; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java index 5f0093678..884f33365 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java +++ b/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 - mUsbDispatcher.rescanDevices(); + checkDeviceConnection(); } } @@ -160,8 +159,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenNfcActivity Passphrase passphrase = PassphraseCacheService.getCachedPassphrase(this, mRequiredInput.getMasterKeyId(), mRequiredInput.getSubKeyId()); if (passphrase != null) { - // Rescan USB devices - mUsbDispatcher.rescanDevices(); + checkDeviceConnection(); } } 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 { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java index 8ed2db9b7..5bf81f1aa 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java +++ b/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); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenNfcActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenNfcActivity.java index ecec98aaf..5e1592346 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenNfcActivity.java +++ b/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() { - @Override - protected void onPreExecute() { - super.onPreExecute(); - onNfcPreExecute(); - } - - @Override - 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(); + smartcardDiscovered(new NfcTransport(tag)); } - 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() { @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 } } - /** - * Handle NFC communication and return a result. - *

- * 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. - *

- * On general communication, see also - * http://www.cardwerk.com/smartcards/smartcard_standard_ISO7816-4_annex-a.aspx - *

- * 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 { + protected void handleSmartcard(Transport transport) throws IOException { // Don't reconnect if device was already connected - if (!mSmartcardDevice.isConnected() - || !(mSmartcardDevice.getTransport() instanceof UsbTransport) - || !((UsbTransport) mSmartcardDevice.getTransport()).getUsbDevice().equals(device)) { - UsbManager usbManager = (UsbManager) getSystemService(USB_SERVICE); - - mSmartcardDevice.setTransport(new UsbTransport(device, usbManager)); + if (!(mSmartcardDevice.isConnected() + && mSmartcardDevice.getTransport().equals(transport))) { + mSmartcardDevice.setTransport(transport); 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()); + } + } } -- cgit v1.2.3