some T=1 refactoring

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

@@ -22,78 +22,99 @@ import org.bouncycastle.util.encoders.Hex;
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 
 public 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 BlockChecksumType checksumType;
 
-    public Block(BlockChecksumType checksumType, byte[] data) throws UsbTransportException {
+    Block(BlockChecksumType 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());
+    }
+    */
+
+//    /*
+    Block(BlockChecksumType checksumType, byte nad, byte pcb, byte[] apdu, int offset, int length)
+            throws UsbTransportException {
+        this.checksumType = checksumType;
+        if (length > MAX_PAYLOAD_LEN) {
+            throw new IllegalArgumentException("Payload too long! " + length + " > " + MAX_PAYLOAD_LEN);
         }
 
-    protected Block(Block baseBlock) {
+        int lengthWithoutChecksum = length + 3;
-        this.mChecksumType = baseBlock.getChecksumType();
+        int checksumLength = this.checksumType.getLength();
-        this.mData = baseBlock.getRawData();
+
+        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() {
-        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/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(BlockChecksumType 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(BlockChecksumType 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(BlockChecksumType 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(BlockChecksumType 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(BlockChecksumType 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/T1TpduBlockFactory.java

@@ -0,0 +1,36 @@
+package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
+
+
+import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
+
+
+class T1TpduBlockFactory {
+    private BlockChecksumType checksumType;
+
+    T1TpduBlockFactory(BlockChecksumType 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,15 +17,15 @@
 
 package org.sufficientlysecure.keychain.securitytoken.usb.tpdu;
 
+
 import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
 
 import org.bouncycastle.util.Arrays;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransceiver;
 import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransceiver.CcidDataBlock;
-import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 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 {
@@ -33,9 +33,9 @@ public class T1TpduProtocol implements CcidTransportProtocol {
 
 
     private CcidTransceiver ccidTransceiver;
-    private BlockChecksumType checksumType;
+    private T1TpduBlockFactory blockFactory;
 
-    private byte mCounter = 0;
+    private byte sequenceCounter = 0;
 
 
     public void connect(@NonNull CcidTransceiver ccidTransceiver) throws UsbTransportException {
@@ -44,22 +44,22 @@ public class T1TpduProtocol implements CcidTransportProtocol {
         }
         this.ccidTransceiver = ccidTransceiver;
 
-        // Connect
+        this.ccidTransceiver.iccPowerOn();
-        CcidDataBlock response = this.ccidTransceiver.iccPowerOn();
 
         // TODO: set checksum from atr
-        checksumType = BlockChecksumType.LRC;
+        blockFactory = new T1TpduBlockFactory(BlockChecksumType.LRC);
-
-        // PPS all auto
-        pps();
 
+        performPpsExchange();
     }
 
-    private 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) };
 
         CcidDataBlock response = ccidTransceiver.sendXfrBlock(pps);
-        Log.d(Constants.TAG, "PPS response " + response);
+
+        if (!Arrays.areEqual(pps, response.getData())) {
+            throw new UsbTransportException("Protocol and parameters (PPS) negotiation failed!");
+        }
     }
 
     public byte[] transceive(@NonNull byte[] apdu) throws UsbTransportException {
@@ -67,87 +67,64 @@ public class T1TpduProtocol implements CcidTransportProtocol {
             throw new IllegalStateException("Protocol not connected!");
         }
 
-        int start = 0;
-
         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);
 
-            CcidDataBlock response = ccidTransceiver.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(response);
-
-            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
-        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();
+
+            Block ackBlock = blockFactory.createAckRBlock(receivedSeqNum);
             CcidDataBlock response = ccidTransceiver.sendXfrBlock(ackBlock.getRawData());
+            Block responseBlock = blockFactory.fromBytes(response.getData());
 
-            responseBlock = getBlockFromResponse(response);
+            if (!(responseBlock instanceof IBlock)) {
-
+                Log.e(Constants.TAG, "Invalid response block received " + responseBlock);
-            if (responseBlock instanceof IBlock) {
-                responseApdu = Arrays.concatenate(responseApdu, responseBlock.getApdu());
-            } 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
-    private Block getBlockFromResponse(CcidDataBlock dataBlock) throws UsbTransportException {
-        final Block baseBlock = new Block(checksumType, dataBlock.getData());
-
-        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");
-    }
-
-    private IBlock newIBlock(byte sequence, boolean chaining, byte[] apdu) throws UsbTransportException {
-        return new IBlock(checksumType, (byte) 0, sequence, chaining, apdu);
-    }
-
-    private RBlock newRBlock(byte sequence) throws UsbTransportException {
-        return new RBlock(checksumType, (byte) 0, sequence);
-    }
 }