Merge pull request #2182 from open-keychain/usb-refactor

Usb refactoring

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java

@@ -17,153 +17,258 @@
 
 package org.sufficientlysecure.keychain.securitytoken.usb;
 
-import android.hardware.usb.UsbDeviceConnection;
-import android.hardware.usb.UsbEndpoint;
-import android.os.SystemClock;
-import android.support.annotation.NonNull;
-
-import org.bouncycastle.util.Arrays;
-import org.bouncycastle.util.encoders.Hex;
 
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 
+import android.hardware.usb.UsbDeviceConnection;
+import android.hardware.usb.UsbEndpoint;
+import android.os.SystemClock;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+import android.support.annotation.WorkerThread;
+import android.util.Log;
+
+import com.google.auto.value.AutoValue;
+import org.bouncycastle.util.Arrays;
+import org.bouncycastle.util.encoders.Hex;
+import org.sufficientlysecure.keychain.Constants;
+
 public class CcidTransceiver {
-    private static final int TIMEOUT = 20 * 1000; // 20s
+    private static final int CCID_HEADER_LENGTH = 10;
 
-    private byte mCounter;
+    private static final int MESSAGE_TYPE_RDR_TO_PC_DATA_BLOCK = 0x80;
-    private UsbDeviceConnection mConnection;
+    private static final int MESSAGE_TYPE_PC_TO_RDR_ICC_POWER_ON = 0x62;
-    private UsbEndpoint mBulkIn;
+    private static final int MESSAGE_TYPE_PC_TO_RDR_XFR_BLOCK = 0x6f;
-    private UsbEndpoint mBulkOut;
 
-    public CcidTransceiver(final UsbDeviceConnection connection, final UsbEndpoint bulkIn,
+    private static final int COMMAND_STATUS_SUCCESS = 0;
-                           final UsbEndpoint bulkOut) {
+    private static final int COMMAND_STATUS_TIME_EXTENSION_RQUESTED = 2;
 
-        mConnection = connection;
+    private static final int SLOT_NUMBER = 0x00;
-        mBulkIn = bulkIn;
-        mBulkOut = bulkOut;
-    }
 
-    public byte[] receiveRaw() throws UsbTransportException {
+    private static final int ICC_STATUS_SUCCESS = 0;
-        byte[] bytes;
-        do {
-            bytes = receive();
-        } while (isDataBlockNotReady(bytes));
 
-        checkDataBlockResponse(bytes);
+    private static final int DEVICE_COMMUNICATE_TIMEOUT_MILLIS = 5000;
+    private static final int DEVICE_SKIP_TIMEOUT_MILLIS = 100;
 
-        return Arrays.copyOfRange(bytes, 10, bytes.length);
+
+    private final UsbDeviceConnection usbConnection;
+    private final UsbEndpoint usbBulkIn;
+    private final UsbEndpoint usbBulkOut;
+    private final byte[] inputBuffer;
+
+    private byte currentSequenceNumber;
+
+
+    CcidTransceiver(UsbDeviceConnection connection, UsbEndpoint bulkIn, UsbEndpoint bulkOut) {
+        usbConnection = connection;
+        usbBulkIn = bulkIn;
+        usbBulkOut = bulkOut;
+
+        inputBuffer = new byte[usbBulkIn.getMaxPacketSize()];
     }
 
     /**
      * Power of ICC
      * Spec: 6.1.1 PC_to_RDR_IccPowerOn
-     *
-     * @throws UsbTransportException
      */
     @NonNull
-    public byte[] iccPowerOn() throws UsbTransportException {
+    @WorkerThread
+    public synchronized CcidDataBlock iccPowerOn() throws UsbTransportException {
+        long startTime = SystemClock.elapsedRealtime();
+
+        skipAvailableInput();
+
+        byte sequenceNumber = currentSequenceNumber++;
         final byte[] iccPowerCommand = {
-                0x62,
+                MESSAGE_TYPE_PC_TO_RDR_ICC_POWER_ON,
                 0x00, 0x00, 0x00, 0x00,
-                0x00,
+                SLOT_NUMBER,
-                mCounter++,
+                sequenceNumber,
-                0x00,
+                0x00, // voltage select = auto
-                0x00, 0x00
+                0x00, 0x00 // reserved for future use
         };
 
-        sendRaw(iccPowerCommand);
+        sendRaw(iccPowerCommand, 0, iccPowerCommand.length);
+        CcidDataBlock response = receiveDataBlock(sequenceNumber);
 
-        long startTime = System.currentTimeMillis();
+        long elapsedTime = SystemClock.elapsedRealtime() - startTime;
-        byte[] atr = null;
-        while (true) {
-            try {
-                atr = receiveRaw();
-                break;
-            } catch (Exception e) {
-                // Try more startTime
-                if (System.currentTimeMillis() - startTime > TIMEOUT) {
-                    break;
-                }
-            }
-            SystemClock.sleep(100);
-        }
 
-        if (atr == null) {
+        Log.d(Constants.TAG, "Usb transport connected  T1/TPDU, took " + elapsedTime + "ms, ATR=" +
-            throw new UsbTransportException("Couldn't power up Security Token");
+                Hex.toHexString(response.getData()));
-        }
 
-        return atr;
+        return response;
     }
 
     /**
      * Transmits XfrBlock
      * 6.1.4 PC_to_RDR_XfrBlock
+     *
      * @param payload payload to transmit
-     * @throws UsbTransportException
      */
-    public void sendXfrBlock(byte[] payload) throws UsbTransportException {
+    @WorkerThread
-        int l = payload.length;
+    public synchronized CcidDataBlock sendXfrBlock(byte[] payload) throws UsbTransportException {
-        byte[] data = Arrays.concatenate(new byte[]{
+        long startTime = SystemClock.elapsedRealtime();
-                        0x6f,
-                        (byte) l, (byte) (l >> 8), (byte) (l >> 16), (byte) (l >> 24),
-                        0x00,
-                        mCounter++,
-                        0x00,
-                        0x00, 0x00},
-                payload);
 
-        int send = 0;
+        int l = payload.length;
-        while (send < data.length) {
+        byte sequenceNumber = currentSequenceNumber++;
-            final int len = Math.min(mBulkIn.getMaxPacketSize(), data.length - send);
+        byte[] headerData = {
-            sendRaw(Arrays.copyOfRange(data, send, send + len));
+                MESSAGE_TYPE_PC_TO_RDR_XFR_BLOCK,
-            send += len;
+                (byte) l, (byte) (l >> 8), (byte) (l >> 16), (byte) (l >> 24),
+                SLOT_NUMBER,
+                sequenceNumber,
+                0x00, // block waiting time
+                0x00, 0x00 // level parameters
+        };
+        byte[] data = Arrays.concatenate(headerData, payload);
+
+        int sentBytes = 0;
+        while (sentBytes < data.length) {
+            int bytesToSend = Math.min(usbBulkIn.getMaxPacketSize(), data.length - sentBytes);
+            sendRaw(data, sentBytes, bytesToSend);
+            sentBytes += bytesToSend;
         }
+
+        CcidDataBlock ccidDataBlock = receiveDataBlock(sequenceNumber);
+
+        long elapsedTime = SystemClock.elapsedRealtime() - startTime;
+        Log.d(Constants.TAG, "USB XferBlock call took " + elapsedTime + "ms");
+
+        return ccidDataBlock;
     }
 
-    public byte[] receive() throws UsbTransportException {
+    private void skipAvailableInput() {
-        byte[] buffer = new byte[mBulkIn.getMaxPacketSize()];
+        int ignoredBytes;
-        byte[] result = null;
-        int readBytes = 0, totalBytes = 0;
-
         do {
-            int res = mConnection.bulkTransfer(mBulkIn, buffer, buffer.length, TIMEOUT);
+            ignoredBytes = usbConnection.bulkTransfer(
-            if (res < 0) {
+                    usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_SKIP_TIMEOUT_MILLIS);
-                throw new UsbTransportException("USB error - failed to receive response " + res);
+            if (ignoredBytes > 0) {
+                Log.e(Constants.TAG,
+                        "Skipped " + ignoredBytes + " bytes: " + Hex.toHexString(inputBuffer, 0, ignoredBytes));
             }
-            if (result == null) {
+        } while (ignoredBytes > 0);
-                if (res < 10) {
+    }
-                    throw new UsbTransportException("USB-CCID error - failed to receive CCID header");
-                }
-                totalBytes = ByteBuffer.wrap(buffer, 1, 4).order(ByteOrder.LITTLE_ENDIAN).asIntBuffer().get() + 10;
-                result = new byte[totalBytes];
-            }
-            System.arraycopy(buffer, 0, result, readBytes, res);
-            readBytes += res;
-        } while (readBytes < totalBytes);
 
+    private CcidDataBlock receiveDataBlock(byte expectedSequenceNumber) throws UsbTransportException {
+        CcidDataBlock response;
+        do {
+            response = receiveDataBlockImmediate(expectedSequenceNumber);
+        } while (response.isStatusTimeoutExtensionRequest());
+
+        if (!response.isStatusSuccess()) {
+            throw new UsbTransportException("USB-CCID error: " + response);
+        }
+
+        return response;
+    }
+
+    private CcidDataBlock receiveDataBlockImmediate(byte expectedSequenceNumber) throws UsbTransportException {
+        int readBytes = usbConnection.bulkTransfer(usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS);
+        if (readBytes < CCID_HEADER_LENGTH) {
+            throw new UsbTransportException("USB-CCID error - failed to receive CCID header");
+        }
+        if (inputBuffer[0] != (byte) MESSAGE_TYPE_RDR_TO_PC_DATA_BLOCK) {
+            if (expectedSequenceNumber != inputBuffer[6]) {
+                throw new UsbTransportException("USB-CCID error - bad CCID header, type " + inputBuffer[0] + " (expected " +
+                        MESSAGE_TYPE_RDR_TO_PC_DATA_BLOCK + "), sequence number " + inputBuffer[6] + " (expected " +
+                        expectedSequenceNumber + ")");
+            }
+
+            throw new UsbTransportException("USB-CCID error - bad CCID header type " + inputBuffer[0]);
+        }
+
+        CcidDataBlock result = CcidDataBlock.parseHeaderFromBytes(inputBuffer);
+
+        if (expectedSequenceNumber != result.getSeq()) {
+            throw new UsbTransportException("USB-CCID error - expected sequence number " +
+                    expectedSequenceNumber + ", got " + result);
+        }
+
+        byte[] dataBuffer = new byte[result.getDataLength()];
+        int bufferedBytes = readBytes - CCID_HEADER_LENGTH;
+        System.arraycopy(inputBuffer, CCID_HEADER_LENGTH, dataBuffer, 0, bufferedBytes);
+
+        while (bufferedBytes < dataBuffer.length) {
+            readBytes = usbConnection.bulkTransfer(usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS);
+            if (readBytes < 0) {
+                throw new UsbTransportException("USB error - failed reading response data! Header: " + result);
+            }
+            System.arraycopy(inputBuffer, 0, dataBuffer, bufferedBytes, readBytes);
+            bufferedBytes += readBytes;
+        }
+
+        result = result.withData(dataBuffer);
         return result;
     }
 
-    private void sendRaw(final byte[] data) throws UsbTransportException {
+    private void sendRaw(byte[] data, int offset, int length) throws UsbTransportException {
-        final int tr1 = mConnection.bulkTransfer(mBulkOut, data, data.length, TIMEOUT);
+        int tr1;
-        if (tr1 != data.length) {
+        if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.JELLY_BEAN_MR2) {
-            throw new UsbTransportException("USB error - failed to transmit data " + tr1);
+            tr1 = usbConnection.bulkTransfer(usbBulkOut, data, offset, length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS);
+        } else {
+            byte[] dataToSend = Arrays.copyOfRange(data, offset, offset+length);
+            tr1 = usbConnection.bulkTransfer(usbBulkOut, dataToSend, dataToSend.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS);
+        }
+
+        if (tr1 != length) {
+            throw new UsbTransportException("USB error - failed to transmit data (" + tr1 + "/" + length + ")");
         }
     }
 
-    private static byte getStatus(byte[] bytes) {
+    /** Corresponds to 6.2.1 RDR_to_PC_DataBlock. */
-        return (byte) ((bytes[7] >> 6) & 0x03);
+    @AutoValue
-    }
+    public abstract static class CcidDataBlock {
+        public abstract int getDataLength();
+        public abstract byte getSlot();
+        public abstract byte getSeq();
+        public abstract byte getStatus();
+        public abstract byte getError();
+        public abstract byte getChainParameter();
+        @Nullable
+        public abstract byte[] getData();
 
-    private void checkDataBlockResponse(byte[] bytes) throws UsbTransportException {
+        static CcidDataBlock parseHeaderFromBytes(byte[] headerBytes) {
-        final byte status = getStatus(bytes);
+            ByteBuffer buf = ByteBuffer.wrap(headerBytes);
-        if (status != 0) {
+            buf.order(ByteOrder.LITTLE_ENDIAN);
-            throw new UsbTransportException("USB-CCID error - status " + status + " error code: " + Hex.toHexString(bytes, 8, 1));
+
+            byte type = buf.get();
+            if (type != (byte) MESSAGE_TYPE_RDR_TO_PC_DATA_BLOCK) {
+                throw new IllegalArgumentException("Header has incorrect type value!");
+            }
+            int dwLength = buf.getInt();
+            byte bSlot = buf.get();
+            byte bSeq = buf.get();
+            byte bStatus = buf.get();
+            byte bError = buf.get();
+            byte bChainParameter = buf.get();
+
+            return new AutoValue_CcidTransceiver_CcidDataBlock(
+                    dwLength, bSlot, bSeq, bStatus, bError, bChainParameter, null);
         }
-    }
 
-    private static boolean isDataBlockNotReady(byte[] bytes) {
+        CcidDataBlock withData(byte[] data) {
-        return getStatus(bytes) == 2;
+            if (getData() != null) {
+                throw new IllegalStateException("Cannot add data to this class twice!");
+            }
+
+            return new AutoValue_CcidTransceiver_CcidDataBlock(
+                    getDataLength(), getSlot(), getSeq(), getStatus(), getError(), getChainParameter(), data);
+        }
+
+        byte getIccStatus() {
+            return (byte) (getStatus() & 0x03);
+        }
+
+        byte getCommandStatus() {
+            return (byte) ((getStatus() >> 6) & 0x03);
+        }
+
+        boolean isStatusTimeoutExtensionRequest() {
+            return getCommandStatus() == COMMAND_STATUS_TIME_EXTENSION_RQUESTED;
+        }
+
+        boolean isStatusSuccess() {
+            return getIccStatus() == ICC_STATUS_SUCCESS && getCommandStatus() == COMMAND_STATUS_SUCCESS;
+        }
     }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransportProtocol.java

@@ -20,5 +20,6 @@ package org.sufficientlysecure.keychain.securitytoken.usb;
 import android.support.annotation.NonNull;
 
 public interface CcidTransportProtocol {
+    void connect(@NonNull CcidTransceiver transceiver) throws UsbTransportException;
     byte[] transceive(@NonNull byte[] apdu) throws UsbTransportException;
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UsbTransport.java

@@ -31,6 +31,7 @@ import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.securitytoken.Transport;
 import javax.smartcardio.CommandAPDU;
 import javax.smartcardio.ResponseAPDU;
+import org.sufficientlysecure.keychain.securitytoken.usb.tpdu.T1ShortApduProtocol;
 import org.sufficientlysecure.keychain.securitytoken.usb.tpdu.T1TpduProtocol;
 import org.sufficientlysecure.keychain.util.Log;
 
@@ -54,20 +55,154 @@ public class UsbTransport implements Transport {
     private static final int MASK_EXTENDED_APDU = 0x40000;
 
 
-    private final UsbManager mUsbManager;
+    private final UsbDevice usbDevice;
-    private final UsbDevice mUsbDevice;
+    private final UsbManager usbManager;
-    private UsbInterface mUsbInterface;
+
-    private UsbEndpoint mBulkIn;
+    private UsbDeviceConnection usbConnection;
-    private UsbEndpoint mBulkOut;
+    private UsbInterface usbInterface;
-    private UsbDeviceConnection mConnection;
+    private CcidTransportProtocol ccidTransportProtocol;
-    private CcidTransceiver mTransceiver;
-    private CcidTransportProtocol mProtocol;
 
     public UsbTransport(UsbDevice usbDevice, UsbManager usbManager) {
-        mUsbDevice = usbDevice;
+        this.usbDevice = usbDevice;
-        mUsbManager = usbManager;
+        this.usbManager = usbManager;
     }
 
+    @Override
+    public void release() {
+        if (usbConnection != null) {
+            usbConnection.releaseInterface(usbInterface);
+            usbConnection.close();
+            usbConnection = null;
+        }
+
+        Log.d(Constants.TAG, "Usb transport disconnected");
+    }
+
+    /**
+     * Check if device is was connected to and still is connected
+     * @return true if device is connected
+     */
+    @Override
+    public boolean isConnected() {
+        return usbConnection != null && usbManager.getDeviceList().containsValue(usbDevice) &&
+                usbConnection.getSerial() != null;
+    }
+
+    /**
+     * Check if Transport supports persistent connections e.g connections which can
+     * handle multiple operations in one session
+     * @return true if transport supports persistent connections
+     */
+    @Override
+    public boolean isPersistentConnectionAllowed() {
+        return true;
+    }
+
+    /**
+     * Connect to OTG device
+     */
+    @Override
+    public void connect() throws IOException {
+        usbInterface = getSmartCardInterface(usbDevice);
+        if (usbInterface == null) {
+            // Shouldn't happen as we whitelist only class 11 devices
+            throw new UsbTransportException("USB error - device doesn't have class 11 interface");
+        }
+
+        final Pair<UsbEndpoint, UsbEndpoint> ioEndpoints = getIoEndpoints(usbInterface);
+        UsbEndpoint usbBulkIn = ioEndpoints.first;
+        UsbEndpoint usbBulkOut = ioEndpoints.second;
+
+        if (usbBulkIn == null || usbBulkOut == null) {
+            throw new UsbTransportException("USB error - invalid class 11 interface");
+        }
+
+        usbConnection = usbManager.openDevice(usbDevice);
+        if (usbConnection == null) {
+            throw new UsbTransportException("USB error - failed to connect to device");
+        }
+
+        if (!usbConnection.claimInterface(usbInterface, true)) {
+            throw new UsbTransportException("USB error - failed to claim interface");
+        }
+
+        byte[] rawDescriptors = usbConnection.getRawDescriptors();
+        ccidTransportProtocol = getCcidTransportProtocolForRawDescriptors(rawDescriptors);
+
+        CcidTransceiver transceiver = new CcidTransceiver(usbConnection, usbBulkIn, usbBulkOut);
+        ccidTransportProtocol.connect(transceiver);
+    }
+
+    private CcidTransportProtocol getCcidTransportProtocolForRawDescriptors(byte[] desc) throws UsbTransportException {
+        int dwProtocols = 0, dwFeatures = 0;
+        boolean hasCcidDescriptor = false;
+
+        ByteBuffer byteBuffer = ByteBuffer.wrap(desc).order(ByteOrder.LITTLE_ENDIAN);
+
+        while (byteBuffer.hasRemaining()) {
+            byteBuffer.mark();
+            byte len = byteBuffer.get(), type = byteBuffer.get();
+
+            if (type == 0x21 && len == 0x36) {
+                byteBuffer.reset();
+
+                byteBuffer.position(byteBuffer.position() + PROTOCOLS_OFFSET);
+                dwProtocols = byteBuffer.getInt();
+
+                byteBuffer.reset();
+
+                byteBuffer.position(byteBuffer.position() + FEATURES_OFFSET);
+                dwFeatures = byteBuffer.getInt();
+                hasCcidDescriptor = true;
+                break;
+            } else {
+                byteBuffer.position(byteBuffer.position() + len - 2);
+            }
+        }
+
+        if (!hasCcidDescriptor) {
+            throw new UsbTransportException("CCID descriptor not found");
+        }
+
+        if ((dwProtocols & MASK_T1_PROTO) == 0) {
+            throw new UsbTransportException("T=0 protocol is not supported");
+        }
+
+        if ((dwFeatures & MASK_TPDU) != 0) {
+            return new T1TpduProtocol();
+        } else if (((dwFeatures & MASK_SHORT_APDU) != 0) || ((dwFeatures & MASK_EXTENDED_APDU) != 0)) {
+            return new T1ShortApduProtocol();
+        } else {
+            throw new UsbTransportException("Character level exchange is not supported");
+        }
+    }
+
+    /**
+     * Transmit and receive data
+     * @param data data to transmit
+     * @return received data
+     */
+    @Override
+    public ResponseAPDU transceive(CommandAPDU data) throws UsbTransportException {
+        return new ResponseAPDU(ccidTransportProtocol.transceive(data.getBytes()));
+    }
+
+    @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 usbDevice != null ? usbDevice.equals(that.usbDevice) : that.usbDevice == null;
+    }
+
+    @Override
+    public int hashCode() {
+        return usbDevice != null ? usbDevice.hashCode() : 0;
+    }
+
+
     /**
      * Get first class 11 (Chip/Smartcard) interface of the device
      *
@@ -108,149 +243,4 @@ public class UsbTransport implements Transport {
         }
         return new Pair<>(bulkIn, bulkOut);
     }
-
-    /**
-     * Release interface and disconnect
-     */
-    @Override
-    public void release() {
-        if (mConnection != null) {
-            mConnection.releaseInterface(mUsbInterface);
-            mConnection.close();
-            mConnection = null;
-        }
-
-        Log.d(Constants.TAG, "Usb transport disconnected");
-    }
-
-    /**
-     * Check if device is was connected to and still is connected
-     * @return true if device is connected
-     */
-    @Override
-    public boolean isConnected() {
-        return mConnection != null && mUsbManager.getDeviceList().containsValue(mUsbDevice) &&
-                mConnection.getSerial() != null;
-    }
-
-    /**
-     * Check if Transport supports persistent connections e.g connections which can
-     * handle multiple operations in one session
-     * @return true if transport supports persistent connections
-     */
-    @Override
-    public boolean isPersistentConnectionAllowed() {
-        return true;
-    }
-
-    /**
-     * Connect to OTG device
-     * @throws IOException
-     */
-    @Override
-    public void connect() throws IOException {
-        mUsbInterface = getSmartCardInterface(mUsbDevice);
-        if (mUsbInterface == null) {
-            // Shouldn't happen as we whitelist only class 11 devices
-            throw new UsbTransportException("USB error - device doesn't have class 11 interface");
-        }
-
-        final Pair<UsbEndpoint, UsbEndpoint> ioEndpoints = getIoEndpoints(mUsbInterface);
-        mBulkIn = ioEndpoints.first;
-        mBulkOut = ioEndpoints.second;
-
-        if (mBulkIn == null || mBulkOut == null) {
-            throw new UsbTransportException("USB error - invalid class 11 interface");
-        }
-
-        mConnection = mUsbManager.openDevice(mUsbDevice);
-        if (mConnection == null) {
-            throw new UsbTransportException("USB error - failed to connect to device");
-        }
-
-        if (!mConnection.claimInterface(mUsbInterface, true)) {
-            throw new UsbTransportException("USB error - failed to claim interface");
-        }
-
-        mTransceiver = new CcidTransceiver(mConnection, mBulkIn, mBulkOut);
-
-
-
-        configureProtocol();
-    }
-
-    private void configureProtocol() throws UsbTransportException {
-        byte[] desc = mConnection.getRawDescriptors();
-        int dwProtocols = 0, dwFeatures = 0;
-        boolean hasCcidDescriptor = false;
-
-        ByteBuffer byteBuffer = ByteBuffer.wrap(desc).order(ByteOrder.LITTLE_ENDIAN);
-
-        while (byteBuffer.hasRemaining()) {
-            byteBuffer.mark();
-            byte len = byteBuffer.get(), type = byteBuffer.get();
-
-            if (type == 0x21 && len == 0x36) {
-                byteBuffer.reset();
-
-                byteBuffer.position(byteBuffer.position() + PROTOCOLS_OFFSET);
-                dwProtocols = byteBuffer.getInt();
-
-                byteBuffer.reset();
-
-                byteBuffer.position(byteBuffer.position() + FEATURES_OFFSET);
-                dwFeatures = byteBuffer.getInt();
-                hasCcidDescriptor = true;
-                break;
-            } else {
-                byteBuffer.position(byteBuffer.position() + len - 2);
-            }
-        }
-
-        if (!hasCcidDescriptor) {
-            throw new UsbTransportException("CCID descriptor not found");
-        }
-
-        if ((dwProtocols & MASK_T1_PROTO) == 0) {
-            throw new UsbTransportException("T=0 protocol is not supported");
-        }
-
-        if ((dwFeatures & MASK_TPDU) != 0) {
-            mProtocol = new T1TpduProtocol(mTransceiver);
-        } else if (((dwFeatures & MASK_SHORT_APDU) != 0) || ((dwFeatures & MASK_EXTENDED_APDU) != 0)) {
-            mProtocol = new T1ShortApduProtocol(mTransceiver);
-        } else {
-            throw new UsbTransportException("Character level exchange is not supported");
-        }
-    }
-
-    /**
-     * Transmit and receive data
-     * @param data data to transmit
-     * @return received data
-     * @throws UsbTransportException
-     */
-    @Override
-    public ResponseAPDU transceive(CommandAPDU data) throws UsbTransportException {
-        return new ResponseAPDU(mProtocol.transceive(data.getBytes()));
-    }
-
-    @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;
-    }
-
-    public UsbDevice getUsbDevice() {
-        return mUsbDevice;
-    }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/Block.java

@@ -21,79 +21,100 @@ import org.bouncycastle.util.Arrays;
 import org.bouncycastle.util.encoders.Hex;
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 
-public class Block {
+class Block {
-    protected static final int MAX_PAYLOAD_LEN = 254;
+    private static final int MAX_PAYLOAD_LEN = 254;
-    protected static final int OFFSET_NAD = 0;
+    private static final int OFFSET_NAD = 0;
-    protected static final int OFFSET_PCB = 1;
+    static final int OFFSET_PCB = 1;
-    protected static final int OFFSET_LEN = 2;
+    private static final int OFFSET_LEN = 2;
-    protected static final int OFFSET_DATA = 3;
+    private static final int OFFSET_DATA = 3;
 
-    protected byte[] mData;
+    private final byte[] blockData;
-    protected BlockChecksumType mChecksumType;
+    private final BlockChecksumAlgorithm checksumType;
 
-    public Block(BlockChecksumType checksumType, byte[] data) throws UsbTransportException {
+    Block(BlockChecksumAlgorithm checksumType, byte[] data) throws UsbTransportException {
-        this.mChecksumType = checksumType;
+        this.checksumType = checksumType;
-        this.mData = data;
+        this.blockData = data;
 
-        int checksumOffset = this.mData.length - mChecksumType.getLength();
+        int checksumOffset = blockData.length - checksumType.getLength();
-        byte[] checksum = mChecksumType.computeChecksum(data, 0, checksumOffset);
+        byte[] checksum = checksumType.computeChecksum(data, 0, checksumOffset);
         if (!Arrays.areEqual(checksum, getEdc())) {
             throw new UsbTransportException("TPDU CRC doesn't match");
         }
     }
 
-    protected Block(BlockChecksumType checksumType, byte nad, byte pcb, byte[] apdu)
+    /*
+    protected Block(BlockChecksumType checksumType, byte nad, byte pcb, byte[] apdu, int offset, int length)
             throws UsbTransportException {
-        this.mChecksumType = checksumType;
+        apdu = Arrays.copyOfRange(apdu, offset, offset + length);
+
+        this.checksumType = checksumType;
         if (apdu.length > MAX_PAYLOAD_LEN) {
             throw new UsbTransportException("APDU is too long; should be split");
         }
-        this.mData = Arrays.concatenate(
+        blockData = Arrays.concatenate(
                 new byte[]{nad, pcb, (byte) apdu.length},
                 apdu,
-                new byte[mChecksumType.getLength()]);
+                new byte[checksumType.getLength()]);
 
-        int checksumOffset = this.mData.length - mChecksumType.getLength();
+        int checksumOffset = blockData.length - checksumType.getLength();
-        byte[] checksum = mChecksumType.computeChecksum(this.mData, 0, checksumOffset);
+        byte[] checksum = checksumType.computeChecksum(blockData, 0, checksumOffset);
 
-        System.arraycopy(checksum, 0, this.mData, checksumOffset, mChecksumType.getLength());
+        System.arraycopy(checksum, 0, blockData, checksumOffset, checksumType.getLength());
     }
+    */
 
-    protected Block(Block baseBlock) {
+//    /*
-        this.mChecksumType = baseBlock.getChecksumType();
+    Block(BlockChecksumAlgorithm checksumType, byte nad, byte pcb, byte[] apdu, int offset, int length)
-        this.mData = baseBlock.getRawData();
+            throws UsbTransportException {
+        this.checksumType = checksumType;
+        if (length > MAX_PAYLOAD_LEN) {
+            throw new IllegalArgumentException("Payload too long! " + length + " > " + MAX_PAYLOAD_LEN);
+        }
+
+        int lengthWithoutChecksum = length + 3;
+        int checksumLength = this.checksumType.getLength();
+
+        blockData = new byte[lengthWithoutChecksum + checksumLength];
+        blockData[0] = nad;
+        blockData[1] = pcb;
+        blockData[2] = (byte) length;
+        System.arraycopy(apdu, offset, blockData, 3, length);
+
+        byte[] checksum = this.checksumType.computeChecksum(blockData, 0, lengthWithoutChecksum);
+        System.arraycopy(checksum, 0, blockData, lengthWithoutChecksum, checksumLength);
     }
 
     public byte getNad() {
-        return mData[OFFSET_NAD];
+        return blockData[OFFSET_NAD];
     }
 
     public byte getPcb() {
-        return mData[OFFSET_PCB];
+        return blockData[OFFSET_PCB];
     }
 
     public byte getLen() {
-        return mData[OFFSET_LEN];
+        return blockData[OFFSET_LEN];
     }
 
     public byte[] getEdc() {
-        return Arrays.copyOfRange(mData, mData.length - mChecksumType.getLength(), mData.length);
+        return Arrays.copyOfRange(blockData, blockData.length - checksumType.getLength(), blockData.length);
     }
 
-    public BlockChecksumType getChecksumType() {
+    public BlockChecksumAlgorithm getChecksumType() {
-        return mChecksumType;
+        return checksumType;
     }
 
     public byte[] getApdu() {
-        return Arrays.copyOfRange(mData, OFFSET_DATA, mData.length - mChecksumType.getLength());
+        return Arrays.copyOfRange(blockData, OFFSET_DATA, blockData.length - checksumType.getLength());
     }
 
     public byte[] getRawData() {
-        return mData;
+        return blockData;
     }
 
     @Override
     public String toString() {
-        return Hex.toHexString(mData);
+        return Hex.toHexString(blockData);
     }
+
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/BlockChecksumAlgorithm.java

@@ -19,12 +19,12 @@ package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
 
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 
-public enum BlockChecksumType {
+enum BlockChecksumAlgorithm {
     LRC(1), CRC(2);
 
     private int mLength;
 
-    BlockChecksumType(int length) {
+    BlockChecksumAlgorithm(int length) {
         mLength = length;
     }
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/FrameType.java

@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2016 Nikita Mikhailov <nikita.s.mikhailov@gmail.com>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
-
-import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
-
-public enum FrameType {
-    I_BLOCK(0b00000000, 0b10000000, 6, true), // Information
-    R_BLOCK(0b10000000, 0b11000000, 4, false), // Receipt ack
-    S_BLOCK(0b11000000, 0b11000000, -1, false); // System
-
-    private byte mValue;
-    private byte mMask;
-    private int mSequenceBit;
-    private boolean mChainingSupported;
-
-    FrameType(int value, int mask, int sequenceBit, boolean chaining) {
-        // Accept ints just to avoid cast in creation
-        this.mValue = (byte) value;
-        this.mMask = (byte) mask;
-        this.mSequenceBit = sequenceBit;
-        this.mChainingSupported = chaining;
-    }
-
-    public static FrameType fromPCB(byte pcb) throws UsbTransportException {
-        for (final FrameType frameType : values()) {
-            if ((frameType.mMask & pcb) == frameType.mValue) {
-                return frameType;
-            }
-        }
-        throw new UsbTransportException("Invalid PCB byte");
-    }
-
-    public int getSequenceBit() {
-        return mSequenceBit;
-    }
-
-    public boolean isChainingSupported() {
-        return mChainingSupported;
-    }
-}

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/IBlock.java

@@ -17,31 +17,38 @@
 
 package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
 
+
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 
-public class IBlock extends Block {
+
-    public static final byte MASK_RBLOCK = (byte) 0b10000000;
+class IBlock extends Block {
-    public static final byte MASK_VALUE_RBLOCK = (byte) 0b00000000;
+    static final byte MASK_IBLOCK = (byte) 0b10000000;
+    static final byte MASK_VALUE_IBLOCK = (byte) 0b00000000;
 
     private static final byte BIT_SEQUENCE = 6;
     private static final byte BIT_CHAINING = 5;
 
-    public IBlock(final Block baseBlock) {
+    IBlock(BlockChecksumAlgorithm checksumType, byte[] data) throws UsbTransportException {
-        super(baseBlock);
+        super(checksumType, data);
+
+        if ((getPcb() & MASK_IBLOCK) != MASK_VALUE_IBLOCK) {
+            throw new IllegalArgumentException("Data contained incorrect block type!");
+        }
     }
 
-    public IBlock(BlockChecksumType checksumType, byte nad, byte sequence, boolean chaining,
+    IBlock(BlockChecksumAlgorithm checksumType, byte nad, byte sequence, boolean chaining, byte[] apdu, int offset,
-                  byte[] apdu) throws UsbTransportException {
+            int length)
+            throws UsbTransportException {
         super(checksumType, nad,
                 (byte) (((sequence & 1) << BIT_SEQUENCE) | (chaining ? 1 << BIT_CHAINING : 0)),
-                apdu);
+                apdu, offset, length);
     }
 
-    public byte getSequence() {
+    byte getSequence() {
         return (byte) ((getPcb() >> BIT_SEQUENCE) & 1);
     }
 
-    public boolean getChaining() {
+    boolean getChaining() {
         return ((getPcb() >> BIT_CHAINING) & 1) != 0;
     }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/RBlock.java

@@ -21,29 +21,34 @@ import android.support.annotation.NonNull;
 
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 
-public class RBlock extends Block {
+class RBlock extends Block {
-    public static final byte MASK_RBLOCK = (byte) 0b11000000;
+    static final byte MASK_RBLOCK = (byte) 0b11000000;
-    public static final byte MASK_VALUE_RBLOCK = (byte) 0b10000000;
+    static final byte MASK_VALUE_RBLOCK = (byte) 0b10000000;
 
     private static final byte BIT_SEQUENCE = 4;
 
-    public RBlock(Block baseBlock) throws UsbTransportException {
+    RBlock(BlockChecksumAlgorithm checksumType, byte[] data) throws UsbTransportException {
-        super(baseBlock);
+        super(checksumType, data);
+
+        if ((getPcb() & MASK_RBLOCK) != MASK_VALUE_RBLOCK) {
+            throw new IllegalArgumentException("Data contained incorrect block type!");
+        }
+
         if (getApdu().length != 0) {
             throw new UsbTransportException("Data in R-block");
         }
     }
 
-    public RBlock(BlockChecksumType checksumType, byte nad, byte sequence)
+    RBlock(BlockChecksumAlgorithm checksumType, byte nad, byte sequence)
             throws UsbTransportException {
-        super(checksumType, nad, (byte) (MASK_VALUE_RBLOCK | ((sequence & 1) << BIT_SEQUENCE)), new byte[0]);
+        super(checksumType, nad, (byte) (MASK_VALUE_RBLOCK | ((sequence & 1) << BIT_SEQUENCE)), new byte[0], 0, 0);
     }
 
     public RError getError() throws UsbTransportException {
         return RError.from(getPcb());
     }
 
-    public enum RError {
+    enum RError {
         NO_ERROR(0), EDC_ERROR(1), OTHER_ERROR(2);
 
         private byte mLowBits;

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/SBlock.java

@@ -17,11 +17,19 @@
 
 package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
 
-public class SBlock extends Block {
-    public static final byte MASK_SBLOCK = (byte) 0b11000000;
-    public static final byte MASK_VALUE_SBLOCK = (byte) 0b11000000;
 
-    public SBlock(Block baseBlock) {
+import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
-        super(baseBlock);
+
+
+class SBlock extends Block {
+    static final byte MASK_SBLOCK = (byte) 0b11000000;
+    static final byte MASK_VALUE_SBLOCK = (byte) 0b11000000;
+
+    SBlock(BlockChecksumAlgorithm checksumType, byte[] data) throws UsbTransportException {
+        super(checksumType, data);
+
+        if ((getPcb() & MASK_SBLOCK) != MASK_VALUE_SBLOCK) {
+            throw new IllegalArgumentException("Data contained incorrect block type!");
+        }
     }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1ShortApduProtocol.java

@@ -15,27 +15,27 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-package org.sufficientlysecure.keychain.securitytoken.usb;
+package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
+
 
 import android.support.annotation.NonNull;
 
-import org.bouncycastle.util.encoders.Hex;
+import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransceiver;
-import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransceiver.CcidDataBlock;
-import org.sufficientlysecure.keychain.util.Log;
+import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransportProtocol;
+import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 
 public class T1ShortApduProtocol implements CcidTransportProtocol {
-    private CcidTransceiver mTransceiver;
+    private CcidTransceiver ccidTransceiver;
 
-    public T1ShortApduProtocol(CcidTransceiver transceiver) throws UsbTransportException {
+    public void connect(@NonNull CcidTransceiver transceiver) throws UsbTransportException {
-        mTransceiver = transceiver;
+        ccidTransceiver = transceiver;
-
+        ccidTransceiver.iccPowerOn();
-        byte[] atr = mTransceiver.iccPowerOn();
-        Log.d(Constants.TAG, "Usb transport connected T1/Short APDU, ATR=" + Hex.toHexString(atr));
     }
 
     @Override
     public byte[] transceive(@NonNull final byte[] apdu) throws UsbTransportException {
-        mTransceiver.sendXfrBlock(apdu);
+        CcidDataBlock response = ccidTransceiver.sendXfrBlock(apdu);
-        return mTransceiver.receiveRaw();
+        return response.getData();
     }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1TpduBlockFactory.java

@@ -0,0 +1,36 @@
+package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
+
+
+import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
+
+
+class T1TpduBlockFactory {
+    private BlockChecksumAlgorithm checksumType;
+
+    T1TpduBlockFactory(BlockChecksumAlgorithm checksumType) {
+        this.checksumType = checksumType;
+    }
+
+    Block fromBytes(byte[] data) throws UsbTransportException {
+        byte pcbByte = data[Block.OFFSET_PCB];
+
+        if ((pcbByte & IBlock.MASK_IBLOCK) == IBlock.MASK_VALUE_IBLOCK) {
+            return new IBlock(checksumType, data);
+        } else if ((pcbByte & SBlock.MASK_SBLOCK) == SBlock.MASK_VALUE_SBLOCK) {
+            return new SBlock(checksumType, data);
+        } else if ((pcbByte & RBlock.MASK_RBLOCK) == RBlock.MASK_VALUE_RBLOCK) {
+            return new RBlock(checksumType, data);
+        }
+
+        throw new UsbTransportException("TPDU Unknown block type");
+    }
+
+    IBlock newIBlock(byte sequence, boolean chaining, byte[] apdu, int offset, int length)
+            throws UsbTransportException {
+        return new IBlock(checksumType, (byte) 0, sequence, chaining, apdu, offset, length);
+    }
+
+    RBlock createAckRBlock(byte receivedSeqNum) throws UsbTransportException {
+        return new RBlock(checksumType, (byte) 0, (byte) (receivedSeqNum + 1));
+    }
+}

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1TpduProtocol.java

@@ -17,128 +17,114 @@
 
 package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
 
+
 import android.support.annotation.NonNull;
 
 import org.bouncycastle.util.Arrays;
-import org.bouncycastle.util.encoders.Hex;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransceiver;
-import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
+import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransceiver.CcidDataBlock;
 import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransportProtocol;
+import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 import org.sufficientlysecure.keychain.util.Log;
 
 public class T1TpduProtocol implements CcidTransportProtocol {
     private final static int MAX_FRAME_LEN = 254;
 
-    private byte mCounter = 0;
-    private CcidTransceiver mTransceiver;
-    private BlockChecksumType mChecksumType;
 
-    public T1TpduProtocol(final CcidTransceiver transceiver) throws UsbTransportException {
+    private CcidTransceiver ccidTransceiver;
-        mTransceiver = transceiver;
+    private T1TpduBlockFactory blockFactory;
+
+    private byte sequenceCounter = 0;
+
+
+    public void connect(@NonNull CcidTransceiver ccidTransceiver) throws UsbTransportException {
+        if (this.ccidTransceiver != null) {
+            throw new IllegalStateException("Protocol already connected!");
+        }
+        this.ccidTransceiver = ccidTransceiver;
+
+        this.ccidTransceiver.iccPowerOn();
 
-        // Connect
-        byte[] atr = mTransceiver.iccPowerOn();
-        Log.d(Constants.TAG, "Usb transport connected  T1/TPDU, ATR=" + Hex.toHexString(atr));
         // TODO: set checksum from atr
-        mChecksumType = BlockChecksumType.LRC;
+        blockFactory = new T1TpduBlockFactory(BlockChecksumAlgorithm.LRC);
 
-        // PPS all auto
+        performPpsExchange();
-        pps();
     }
 
-    protected void pps() throws UsbTransportException {
+    private void performPpsExchange() throws UsbTransportException {
-        byte[] pps = new byte[]{(byte) 0xFF, 1, (byte) (0xFF ^ 1)};
+        byte[] pps = { (byte) 0xFF, 1, (byte) (0xFF ^ 1) };
 
-        mTransceiver.sendXfrBlock(pps);
+        CcidDataBlock response = ccidTransceiver.sendXfrBlock(pps);
 
-        byte[] ppsResponse = mTransceiver.receiveRaw();
+        if (!Arrays.areEqual(pps, response.getData())) {
-
+            throw new UsbTransportException("Protocol and parameters (PPS) negotiation failed!");
-        Log.d(Constants.TAG, "PPS response " + Hex.toHexString(ppsResponse));
+        }
     }
 
     public byte[] transceive(@NonNull byte[] apdu) throws UsbTransportException {
-        int start = 0;
+        if (this.ccidTransceiver == null) {
+            throw new IllegalStateException("Protocol not connected!");
+        }
 
         if (apdu.length == 0) {
             throw new UsbTransportException("Cant transcive zero-length apdu(tpdu)");
         }
 
-        Block responseBlock = null;
+        IBlock responseBlock = sendChainedData(apdu);
-        while (apdu.length - start > 0) {
+        return receiveChainedResponse(responseBlock);
-            boolean hasMore = start + MAX_FRAME_LEN < apdu.length;
+    }
-            int len = Math.min(MAX_FRAME_LEN, apdu.length - start);
 
-            // Send next frame
+    private IBlock sendChainedData(@NonNull byte[] apdu) throws UsbTransportException {
-            Block block = newIBlock(mCounter++, hasMore, Arrays.copyOfRange(apdu, start, start + len));
+        int sentLength = 0;
+        while (sentLength < apdu.length) {
+            boolean hasMore = sentLength + MAX_FRAME_LEN < apdu.length;
+            int len = Math.min(MAX_FRAME_LEN, apdu.length - sentLength);
 
-            mTransceiver.sendXfrBlock(block.getRawData());
+            Block sendBlock = blockFactory.newIBlock(sequenceCounter++, hasMore, apdu, sentLength, len);
+            CcidDataBlock response = ccidTransceiver.sendXfrBlock(sendBlock.getRawData());
+            Block responseBlock = blockFactory.fromBytes(response.getData());
 
-            // Receive I or R block
+            sentLength += len;
-            responseBlock = getBlockFromResponse(mTransceiver.receiveRaw());
-
-            start += len;
 
             if (responseBlock instanceof SBlock) {
-                Log.d(Constants.TAG, "S-Block received " + responseBlock.toString());
+                Log.d(Constants.TAG, "S-Block received " + responseBlock);
                 // just ignore
             } else if (responseBlock instanceof RBlock) {
-                Log.d(Constants.TAG, "R-Block received " + responseBlock.toString());
+                Log.d(Constants.TAG, "R-Block received " + responseBlock);
                 if (((RBlock) responseBlock).getError() != RBlock.RError.NO_ERROR) {
-                    throw new UsbTransportException("R-Block reports error "
+                    throw new UsbTransportException("R-Block reports error " + ((RBlock) responseBlock).getError());
-                            + ((RBlock) responseBlock).getError());
                 }
             } else {  // I block
-                if (start != apdu.length) {
+                if (sentLength != apdu.length) {
                     throw new UsbTransportException("T1 frame response underflow");
                 }
-                break;
+                return (IBlock) responseBlock;
             }
         }
 
-        // Receive
+        throw new UsbTransportException("Invalid tpdu sequence state");
-        if (responseBlock == null || !(responseBlock instanceof IBlock))
+    }
-            throw new UsbTransportException("Invalid tpdu sequence state");
 
-        byte[] responseApdu = responseBlock.getApdu();
+    private byte[] receiveChainedResponse(IBlock responseIBlock) throws UsbTransportException {
+        byte[] responseApdu = responseIBlock.getApdu();
 
-        while (((IBlock) responseBlock).getChaining()) {
+        while (responseIBlock.getChaining()) {
-            Block ackBlock = newRBlock((byte) (((IBlock) responseBlock).getSequence() + 1));
+            byte receivedSeqNum = responseIBlock.getSequence();
-            mTransceiver.sendXfrBlock(ackBlock.getRawData());
 
-            responseBlock = getBlockFromResponse(mTransceiver.receiveRaw());
+            Block ackBlock = blockFactory.createAckRBlock(receivedSeqNum);
+            CcidDataBlock response = ccidTransceiver.sendXfrBlock(ackBlock.getRawData());
+            Block responseBlock = blockFactory.fromBytes(response.getData());
 
-            if (responseBlock instanceof IBlock) {
+            if (!(responseBlock instanceof IBlock)) {
-                responseApdu = Arrays.concatenate(responseApdu, responseBlock.getApdu());
+                Log.e(Constants.TAG, "Invalid response block received " + responseBlock);
-            } else {
-                Log.d(Constants.TAG, "Response block received " + responseBlock.toString());
                 throw new UsbTransportException("Response: invalid state - invalid block received");
             }
+
+            responseIBlock = (IBlock) responseBlock;
+            responseApdu = Arrays.concatenate(responseApdu, responseBlock.getApdu());
         }
 
         return responseApdu;
     }
-
-    // Factory methods
-    public Block getBlockFromResponse(byte[] data) throws UsbTransportException {
-        final Block baseBlock = new Block(mChecksumType, data);
-
-        if ((baseBlock.getPcb() & IBlock.MASK_RBLOCK) == IBlock.MASK_VALUE_RBLOCK) {
-            return new IBlock(baseBlock);
-        } else if ((baseBlock.getPcb() & SBlock.MASK_SBLOCK) == SBlock.MASK_VALUE_SBLOCK) {
-            return new SBlock(baseBlock);
-        } else if ((baseBlock.getPcb() & RBlock.MASK_RBLOCK) == RBlock.MASK_VALUE_RBLOCK) {
-            return new RBlock(baseBlock);
-        }
-
-        throw new UsbTransportException("TPDU Unknown block type");
-    }
-
-    public IBlock newIBlock(byte sequence, boolean chaining, byte[] apdu) throws UsbTransportException {
-        return new IBlock(mChecksumType, (byte) 0, sequence, chaining, apdu);
-    }
-
-    public RBlock newRBlock(byte sequence) throws UsbTransportException {
-        return new RBlock(mChecksumType, (byte) 0, sequence);
-    }
 }

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

@@ -112,21 +112,26 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
         onSecurityTokenError(error);
     }
 
-    public void tagDiscovered(final Tag tag) {
+    public void tagDiscovered(Tag tag) {
         // Actual NFC operations are executed in doInBackground to not block the UI thread
-        if (!mTagHandlingEnabled)
+        if (!mTagHandlingEnabled) {
             return;
+        }
 
-        securityTokenDiscovered(new NfcTransport(tag));
+        NfcTransport nfcTransport = new NfcTransport(tag);
+        securityTokenDiscovered(nfcTransport);
     }
 
-    public void usbDeviceDiscovered(final UsbDevice usbDevice) {
+    public void usbDeviceDiscovered(UsbDevice usbDevice) {
         // Actual USB operations are executed in doInBackground to not block the UI thread
-        if (!mTagHandlingEnabled)
+        if (!mTagHandlingEnabled) {
             return;
+        }
 
         UsbManager usbManager = (UsbManager) getSystemService(Context.USB_SERVICE);
-        securityTokenDiscovered(new UsbTransport(usbDevice, usbManager));
+
+        UsbTransport usbTransport = new UsbTransport(usbDevice, usbManager);
+        securityTokenDiscovered(usbTransport);
     }
 
     public void securityTokenDiscovered(final Transport transport) {