From 64b4c9fad83476d0c7417c8fb7408258ba990695 Mon Sep 17 00:00:00 2001 From: Stuart Longland Date: Sat, 15 Jul 2023 12:39:35 +1000 Subject: [PATCH 1/6] SecurityTokenInfo: Add NITROKEY_3 to TokenType enum --- .../keychain/securitytoken/SecurityTokenInfo.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenInfo.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenInfo.java index 4320db59b..dad5a9b4b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenInfo.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenInfo.java @@ -117,7 +117,7 @@ public abstract class SecurityTokenInfo implements Parcelable { public enum TokenType { YUBIKEY_NEO, YUBIKEY_4_5, FIDESMO, NITROKEY_PRO, NITROKEY_STORAGE, NITROKEY_START_OLD, - NITROKEY_START_1_25_AND_NEWER, GNUK_OLD, GNUK_1_25_AND_NEWER, LEDGER_NANO_S, SECALOT, UNKNOWN + NITROKEY_START_1_25_AND_NEWER, NITROKEY_3, GNUK_OLD, GNUK_1_25_AND_NEWER, LEDGER_NANO_S, SECALOT, UNKNOWN } public static final Set SUPPORTED_USB_TOKENS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( @@ -127,6 +127,7 @@ public abstract class SecurityTokenInfo implements Parcelable { TokenType.NITROKEY_STORAGE, TokenType.NITROKEY_START_OLD, TokenType.NITROKEY_START_1_25_AND_NEWER, + TokenType.NITROKEY_3, TokenType.GNUK_OLD, TokenType.GNUK_1_25_AND_NEWER, TokenType.LEDGER_NANO_S, @@ -139,6 +140,7 @@ public abstract class SecurityTokenInfo implements Parcelable { TokenType.NITROKEY_PRO, TokenType.NITROKEY_STORAGE, TokenType.NITROKEY_START_1_25_AND_NEWER, + TokenType.NITROKEY_3, TokenType.GNUK_1_25_AND_NEWER, TokenType.SECALOT ))); From 99fed989add03de120723b0d734a315add6fb313 Mon Sep 17 00:00:00 2001 From: Stuart Longland Date: Sat, 15 Jul 2023 12:40:40 +1000 Subject: [PATCH 2/6] UsbTransport: Recognise NitroKey 3 family USB ID https://raw.githubusercontent.com/Nitrokey/libnitrokey/master/data/41-nitrokey.rules lists the following USB VID/PID: 20a0:42b2: Nitrokey 3A Mini/3A NFC/3C NFC --- .../keychain/securitytoken/usb/UsbTransport.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UsbTransport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UsbTransport.java index 18766bce5..63ec009e0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UsbTransport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UsbTransport.java @@ -66,6 +66,7 @@ public class UsbTransport implements Transport { private static final int PRODUCT_NITROKEY_PRO = 16648; private static final int PRODUCT_NITROKEY_START = 16913; private static final int PRODUCT_NITROKEY_STORAGE = 16649; + private static final int PRODUCT_NITROKEY_3 = 17074; private static final int VENDOR_FSIJ = 9035; private static final int VENDOR_LEDGER = 11415; @@ -245,6 +246,8 @@ public class UsbTransport implements Transport { return versionGreaterEquals125 ? TokenType.NITROKEY_START_1_25_AND_NEWER : TokenType.NITROKEY_START_OLD; case PRODUCT_NITROKEY_STORAGE: return TokenType.NITROKEY_STORAGE; + case PRODUCT_NITROKEY_3: + return TokenType.NITROKEY_3; } break; } From 1ee192f0984ac11d69ad0505646836276469cece Mon Sep 17 00:00:00 2001 From: Stuart Longland Date: Mon, 17 Jul 2023 22:08:25 +1000 Subject: [PATCH 3/6] CcidTransceiver: Expose Level Param The `PC_TO_RDR_XFR_BLOCK` structure has a fields which is used in multi-block transfers: - `levelParam`: Indicates if APDU begins or ends in this command Some constants for `levelParam` are defined. --- .../securitytoken/usb/CcidTransceiver.java | 73 ++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java index 518532514..d14c102f3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java @@ -47,6 +47,53 @@ public class CcidTransceiver { private static final int COMMAND_STATUS_SUCCESS = 0; private static final int COMMAND_STATUS_TIME_EXTENSION_RQUESTED = 2; + /** + * Level Parameter: APDU is a single command. + * + * "the command APDU begins and ends with this command" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices rev 1.0 + * § 6.1.1.3 + */ + public static final short LEVEL_PARAM_START_SINGLE_CMD_APDU = 0x0000; + + /** + * Level Parameter: First APDU in a multi-command APDU. + * + * "the command APDU begins with this command, and continue in the + * next PC_to_RDR_XfrBlock" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices rev 1.0 + * § 6.1.1.3 + */ + public static final short LEVEL_PARAM_START_MULTI_CMD_APDU = 0x0001; + + /** + * Level Parameter: Final APDU in a multi-command APDU. + * + * "this abData field continues a command APDU and ends the command APDU" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices rev 1.0 + * § 6.1.1.3 + */ + public static final short LEVEL_PARAM_END_MULTI_CMD_APDU = 0x0002; + + /** + * Level Parameter: Next command in a multi-command APDU. + * + * "the abData field continues a command APDU and another block is to follow" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices rev 1.0 + * § 6.1.1.3 + */ + public static final short LEVEL_PARAM_CONTINUE_MULTI_CMD_APDU = 0x0003; + + /** + * Level Parameter: Request the device continue sending APDU. + * + * "empty abData field, continuation of response APDU is expected in the next + * RDR_to_PC_DataBlock" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices rev 1.0 + * § 6.1.1.3 + */ + public static final short LEVEL_PARAM_CONTINUE_RESPONSE = 0x0010; + private static final int SLOT_NUMBER = 0x00; private static final int ICC_STATUS_SUCCESS = 0; @@ -152,6 +199,27 @@ public class CcidTransceiver { */ @WorkerThread public synchronized CcidDataBlock sendXfrBlock(byte[] payload) throws UsbTransportException { + return sendXfrBlock(payload, LEVEL_PARAM_START_SINGLE_CMD_APDU); + } + + /** + * Receives a continued XfrBlock. Should be called when a multiblock response is indicated + * 6.1.4 PC_to_RDR_XfrBlock + */ + @WorkerThread + public synchronized CcidDataBlock receiveContinuedResponse() throws UsbTransportException { + return sendXfrBlock(new byte[0], LEVEL_PARAM_CONTINUE_RESPONSE); + } + + /** + * Transmits XfrBlock + * 6.1.4 PC_to_RDR_XfrBlock + * + * @param payload payload to transmit + * @param levelParam Level parameter + */ + @WorkerThread + public synchronized CcidDataBlock sendXfrBlock(byte[] payload, short levelParam) throws UsbTransportException { long startTime = SystemClock.elapsedRealtime(); int l = payload.length; @@ -161,8 +229,9 @@ public class CcidTransceiver { (byte) l, (byte) (l >> 8), (byte) (l >> 16), (byte) (l >> 24), SLOT_NUMBER, sequenceNumber, - 0x00, // block waiting time - 0x00, 0x00 // level parameters + (byte) 0x00, // block waiting time + (byte)(levelParam & 0x00ff), + (byte)(levelParam >> 8) }; byte[] data = Arrays.concatenate(headerData, payload); From 19cd6967c0f39d1e855adf47656d03492210d5f8 Mon Sep 17 00:00:00 2001 From: Stuart Longland Date: Mon, 17 Jul 2023 22:18:02 +1000 Subject: [PATCH 4/6] T1ShortApduProtocol: Implement CCID chaining Code credit: @sosthene-nitrokey https://github.com/open-keychain/open-keychain/pull/2842#issuecomment-1637598171 --- .../usb/tpdu/T1ShortApduProtocol.java | 50 ++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1ShortApduProtocol.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1ShortApduProtocol.java index 41f659f26..853c59c75 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1ShortApduProtocol.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/tpdu/T1ShortApduProtocol.java @@ -17,6 +17,8 @@ package org.sufficientlysecure.keychain.securitytoken.usb.tpdu; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import androidx.annotation.NonNull; @@ -26,6 +28,22 @@ import org.sufficientlysecure.keychain.securitytoken.usb.CcidTransportProtocol; import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException; public class T1ShortApduProtocol implements CcidTransportProtocol { + /** + * Chain Parameter: Start of multi-command APDU response. + * + * "The response APDU begins with this command and is to continue" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices v1.0 § 6.1.1.3 + */ + public static final byte CHAIN_PARAM_APDU_MULTIBLOCK_START = 1; + + /** + * Chain Parameter: Continued multi-command APDU response with more data. + * + * "This abData field continues the response APDU and another block is to follow" + * -- DWG Smart-Card USB Integrated Circuit(s) Card Devices v1.0 § 6.1.1.3 + */ + public static final byte CHAIN_PARAM_APDU_MULTIBLOCK_MORE = 3; + private CcidTransceiver ccidTransceiver; public void connect(@NonNull CcidTransceiver transceiver) throws UsbTransportException { @@ -35,7 +53,35 @@ public class T1ShortApduProtocol implements CcidTransportProtocol { @Override public byte[] transceive(@NonNull final byte[] apdu) throws UsbTransportException { - CcidDataBlock response = ccidTransceiver.sendXfrBlock(apdu); - return response.getData(); + CcidDataBlock initialResponse = ccidTransceiver.sendXfrBlock(apdu); + + if (initialResponse.getChainParameter() != CHAIN_PARAM_APDU_MULTIBLOCK_START) { + return initialResponse.getData(); + } + + /* + * Handle multi-block responses in accordance with DWG Smart-Card USB Integrated Circut(s) + * Card Devices v1.0 § 6.1.1. If we receive a response with a chain parameter indicating + * more data is to come, then instruct the device to continue and append the response to our + * output buffer. + */ + + try { + ByteArrayOutputStream output = new ByteArrayOutputStream(); + output.write(initialResponse.getData()); + + CcidDataBlock continuedResponse; + do { + continuedResponse = ccidTransceiver.receiveContinuedResponse(); + output.write(continuedResponse.getData()); + } while(continuedResponse.getChainParameter() == CHAIN_PARAM_APDU_MULTIBLOCK_MORE); + + return output.toByteArray(); + } catch (UsbTransportException e) { + // rethrow as-is + throw e; + } catch (IOException e) { + throw new UsbTransportException("Failed to write block to temporary buffer", e); + } } } From fec3f85fad35866d89bc2b1878018ee57899a908 Mon Sep 17 00:00:00 2001 From: Stuart Longland Date: Sun, 23 Jul 2023 20:34:41 +1000 Subject: [PATCH 5/6] CcidTransceiver: Poke device again if we get zero-sized packet. --- .../securitytoken/usb/CcidTransceiver.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java index d14c102f3..188fcfce5 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java @@ -275,7 +275,20 @@ public class CcidTransceiver { } private CcidDataBlock receiveDataBlockImmediate(byte expectedSequenceNumber) throws UsbTransportException { - int readBytes = usbConnection.bulkTransfer(usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS); + /* + * Some USB CCID devices (notably NitroKey 3) may time-out and need a subsequent poke to + * carry on communications. No particular reason why the number 3 was chosen. If we get a + * zero-sized reply (or a time-out), we try again. Clamped retries prevent an infinite loop + * if things really turn sour. + */ + int attempts = 3; + Timber.d("Receive data block immediate seq=%d", expectedSequenceNumber); + int readBytes; + do { + readBytes = usbConnection.bulkTransfer(usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS); + Timber.d("Received " + readBytes + " bytes: " + toHexString(inputBuffer)); + } while ((readBytes <= 0) && (attempts-- > 0)); + if (readBytes < CCID_HEADER_LENGTH) { throw new UsbTransportException("USB-CCID error - failed to receive CCID header"); } From 0fcd77d61d9f67f61498abb3728879a8b9b847a1 Mon Sep 17 00:00:00 2001 From: Stuart Longland Date: Sat, 5 Aug 2023 10:18:00 +1000 Subject: [PATCH 6/6] CcidTransceiver: Adjust line lengths 100 characters with exceptions made for CLI commands, URIs and imports. Some of this is existing code, but for the sake of consistency, we'll wrap all at 100 characters. Function arguments: when this happens, if we put the closing `)` on its own line (like `}` in code blocks), it visually stands out better for indicating where the function call ends. --- .../securitytoken/usb/CcidTransceiver.java | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java index 188fcfce5..fd6139f48 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/CcidTransceiver.java @@ -219,7 +219,8 @@ public class CcidTransceiver { * @param levelParam Level parameter */ @WorkerThread - public synchronized CcidDataBlock sendXfrBlock(byte[] payload, short levelParam) throws UsbTransportException { + private synchronized CcidDataBlock sendXfrBlock(byte[] payload, short levelParam) + throws UsbTransportException { long startTime = SystemClock.elapsedRealtime(); int l = payload.length; @@ -256,12 +257,16 @@ public class CcidTransceiver { ignoredBytes = usbConnection.bulkTransfer( usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_SKIP_TIMEOUT_MILLIS); if (ignoredBytes > 0) { - Timber.e("Skipped " + ignoredBytes + " bytes: " + toHexString(inputBuffer, 0, ignoredBytes)); + Timber.e( + "Skipped " + ignoredBytes + " bytes: " + + toHexString(inputBuffer, 0, ignoredBytes) + ); } } while (ignoredBytes > 0); } - private CcidDataBlock receiveDataBlock(byte expectedSequenceNumber) throws UsbTransportException { + private CcidDataBlock receiveDataBlock(byte expectedSequenceNumber) + throws UsbTransportException { CcidDataBlock response; do { response = receiveDataBlockImmediate(expectedSequenceNumber); @@ -274,7 +279,8 @@ public class CcidTransceiver { return response; } - private CcidDataBlock receiveDataBlockImmediate(byte expectedSequenceNumber) throws UsbTransportException { + private CcidDataBlock receiveDataBlockImmediate(byte expectedSequenceNumber) + throws UsbTransportException { /* * Some USB CCID devices (notably NitroKey 3) may time-out and need a subsequent poke to * carry on communications. No particular reason why the number 3 was chosen. If we get a @@ -285,7 +291,9 @@ public class CcidTransceiver { Timber.d("Receive data block immediate seq=%d", expectedSequenceNumber); int readBytes; do { - readBytes = usbConnection.bulkTransfer(usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS); + readBytes = usbConnection.bulkTransfer( + usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS + ); Timber.d("Received " + readBytes + " bytes: " + toHexString(inputBuffer)); } while ((readBytes <= 0) && (attempts-- > 0)); @@ -294,12 +302,17 @@ public class CcidTransceiver { } 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] + " (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]); + throw new UsbTransportException( + "USB-CCID error - bad CCID header type " + inputBuffer[0] + ); } CcidDataBlock result = CcidDataBlock.parseHeaderFromBytes(inputBuffer); @@ -313,9 +326,13 @@ public class CcidTransceiver { System.arraycopy(inputBuffer, CCID_HEADER_LENGTH, dataBuffer, 0, bufferedBytes); while (bufferedBytes < dataBuffer.length) { - readBytes = usbConnection.bulkTransfer(usbBulkIn, inputBuffer, inputBuffer.length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS); + 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); + throw new UsbTransportException( + "USB error - failed reading response data! Header: " + result + ); } System.arraycopy(inputBuffer, 0, dataBuffer, bufferedBytes, readBytes); bufferedBytes += readBytes; @@ -329,14 +346,20 @@ public class CcidTransceiver { private void sendRaw(byte[] data, int offset, int length) throws UsbTransportException { int tr1; if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.JELLY_BEAN_MR2) { - tr1 = usbConnection.bulkTransfer(usbBulkOut, data, offset, length, DEVICE_COMMUNICATE_TIMEOUT_MILLIS); + 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); + 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 + ")"); + throw new UsbTransportException( + "USB error - failed to transmit data (" + tr1 + "/" + length + ")" + ); } } @@ -382,7 +405,9 @@ public class CcidTransceiver { } return new AutoValue_CcidTransceiver_CcidDataBlock( - getDataLength(), getSlot(), getSeq(), getStatus(), getError(), getChainParameter(), data); + getDataLength(), getSlot(), getSeq(), getStatus(), getError(), + getChainParameter(), data + ); } byte getIccStatus() { @@ -398,7 +423,8 @@ public class CcidTransceiver { } boolean isStatusSuccess() { - return getIccStatus() == ICC_STATUS_SUCCESS && getCommandStatus() == COMMAND_STATUS_SUCCESS; + return getIccStatus() == ICC_STATUS_SUCCESS + && getCommandStatus() == COMMAND_STATUS_SUCCESS; } } }