From e8f72718e9fc0a8651e2f5e60c3441fa10e2ff9b Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 9 Oct 2017 01:53:57 +0200 Subject: [PATCH 01/13] rename SecurityTokenHelper to SecurityTokenConnection --- .../securitytoken/SCP11bSecureMessaging.java | 20 ++++++------ ...lper.java => SecurityTokenConnection.java} | 8 ++--- .../keychain/ui/CreateKeyActivity.java | 2 +- .../CreateSecurityTokenAlgorithmFragment.java | 4 +-- .../ui/CreateSecurityTokenPinFragment.java | 5 ++- ...curityTokenChangePinOperationActivity.java | 10 +++--- .../ui/SecurityTokenOperationActivity.java | 29 +++++++++-------- .../ui/base/BaseSecurityTokenActivity.java | 32 +++++++++---------- 8 files changed, 55 insertions(+), 55 deletions(-) rename OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/{SecurityTokenHelper.java => SecurityTokenConnection.java} (99%) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index 7953a02f0..fd619ccd0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -275,7 +275,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } - public static void establish(final SecurityTokenHelper t, final Context ctx) + public static void establish(final SecurityTokenConnection t, final Context ctx) throws SecureMessagingException, IOException { CommandAPDU cmd; @@ -286,9 +286,9 @@ class SCP11bSecureMessaging implements SecureMessaging { // retrieving key algorithm cmd = new CommandAPDU(0, (byte)0xCA, (byte)0x00, - OPENPGP_SECURE_MESSAGING_KEY_ATTRIBUTES_TAG, SecurityTokenHelper.MAX_APDU_NE_EXT); + OPENPGP_SECURE_MESSAGING_KEY_ATTRIBUTES_TAG, SecurityTokenConnection.MAX_APDU_NE_EXT); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenHelper.APDU_SW_SUCCESS) { + if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging key attributes"); } tlvs = Iso7816TLV.readList(resp.getData(), true); @@ -320,12 +320,12 @@ class SCP11bSecureMessaging implements SecureMessaging { cmd = new CommandAPDU(0, (byte) 0xA5, (byte) 0x03, (byte) 0x04, new byte[]{(byte) 0x60, (byte) 0x04, (byte) 0x5C, (byte) 0x02, (byte) 0x7F, (byte) 0x21}); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenHelper.APDU_SW_SUCCESS) { + if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to select secure messaging certificate"); } - cmd = new CommandAPDU(0, (byte) 0xCA, (byte) 0x7F, (byte) 0x21, SecurityTokenHelper.MAX_APDU_NE_EXT); + cmd = new CommandAPDU(0, (byte) 0xCA, (byte) 0x7F, (byte) 0x21, SecurityTokenConnection.MAX_APDU_NE_EXT); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenHelper.APDU_SW_SUCCESS) { + if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging certificate"); } @@ -334,9 +334,9 @@ class SCP11bSecureMessaging implements SecureMessaging { } else { // retrieving public key cmd = new CommandAPDU(0, (byte) 0x47, (byte) 0x81, (byte) 0x00, - OPENPGP_SECURE_MESSAGING_KEY_CRT, SecurityTokenHelper.MAX_APDU_NE_EXT); + OPENPGP_SECURE_MESSAGING_KEY_CRT, SecurityTokenConnection.MAX_APDU_NE_EXT); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenHelper.APDU_SW_SUCCESS) { + if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging public key"); } tlvs = Iso7816TLV.readList(resp.getData(), true); @@ -396,9 +396,9 @@ class SCP11bSecureMessaging implements SecureMessaging { // internal authenticate cmd = new CommandAPDU(0, (byte)0x88, (byte)0x01, (byte)0x0, pkout.toByteArray(), - SecurityTokenHelper.MAX_APDU_NE_EXT); + SecurityTokenConnection.MAX_APDU_NE_EXT); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenHelper.APDU_SW_SUCCESS) { + if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to initiate internal authenticate"); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java similarity index 99% rename from OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenHelper.java rename to OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 980c28888..6a71a98f6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -70,7 +70,7 @@ import java.security.interfaces.RSAPrivateCrtKey; * devices. * For the full specs, see http://g10code.com/docs/openpgp-card-2.0.pdf */ -public class SecurityTokenHelper { +public class SecurityTokenConnection { private static final int MAX_APDU_NC = 255; private static final int MAX_APDU_NC_EXT = 65535; @@ -100,7 +100,7 @@ public class SecurityTokenHelper { private boolean mPw1ValidatedForDecrypt; // Mode 82 does other things; consider renaming? private boolean mPw3Validated; - private SecurityTokenHelper() { + private SecurityTokenConnection() { } public static double parseOpenPgpVersion(final byte[] aid) { @@ -109,7 +109,7 @@ public class SecurityTokenHelper { return aid[6] + minv; } - public static SecurityTokenHelper getInstance() { + public static SecurityTokenConnection getInstance() { return LazyHolder.SECURITY_TOKEN_HELPER; } @@ -1007,6 +1007,6 @@ public class SecurityTokenHelper { } private static class LazyHolder { - private static final SecurityTokenHelper SECURITY_TOKEN_HELPER = new SecurityTokenHelper(); + private static final SecurityTokenConnection SECURITY_TOKEN_HELPER = new SecurityTokenConnection(); } } 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 5ba56c67f..51d158710 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java @@ -139,7 +139,7 @@ public class CreateKeyActivity extends BaseSecurityTokenActivity { return; } - tokenInfo = mSecurityTokenHelper.getTokenInfo(); + tokenInfo = mSecurityTokenConnection.getTokenInfo(); } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenAlgorithmFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenAlgorithmFragment.java index 0c3985eff..1efd96455 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenAlgorithmFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenAlgorithmFragment.java @@ -32,7 +32,7 @@ import android.widget.TextView; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.securitytoken.KeyFormat; -import org.sufficientlysecure.keychain.securitytoken.SecurityTokenHelper; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.ui.CreateKeyActivity.FragAction; import org.sufficientlysecure.keychain.util.Choice; @@ -100,7 +100,7 @@ public class CreateSecurityTokenAlgorithmFragment extends Fragment { choices.add(new Choice<>(SupportedKeyType.RSA_4096, getResources().getString( R.string.rsa_4096), getResources().getString(R.string.rsa_4096_description_html))); - final double version = SecurityTokenHelper.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid()); + final double version = SecurityTokenConnection.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid()); if (version >= 3.0) { choices.add(new Choice<>(SupportedKeyType.ECC_P256, getResources().getString( diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenPinFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenPinFragment.java index 911cddf9e..a6bcb92a9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenPinFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateSecurityTokenPinFragment.java @@ -17,7 +17,6 @@ package org.sufficientlysecure.keychain.ui; -import android.app.Activity; import android.content.Context; import android.os.AsyncTask; import android.os.Bundle; @@ -31,7 +30,7 @@ import android.widget.TextView; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; -import org.sufficientlysecure.keychain.securitytoken.SecurityTokenHelper; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.ui.CreateKeyActivity.FragAction; import org.sufficientlysecure.keychain.util.Passphrase; @@ -206,7 +205,7 @@ public class CreateSecurityTokenPinFragment extends Fragment { mCreateKeyActivity.mSecurityTokenPin = new Passphrase(mPin.getText().toString()); - final double version = SecurityTokenHelper.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid()); + final double version = SecurityTokenConnection.parseOpenPgpVersion(mCreateKeyActivity.tokenInfo.getAid()); Fragment frag; if (version >= 3.0) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java index c38f667de..95ac39143 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java @@ -139,10 +139,10 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc @Override protected void doSecurityTokenInBackground() throws IOException { - mSecurityTokenHelper.setAdminPin(new Passphrase(changePinInput.getAdminPin())); - mSecurityTokenHelper.resetPin(changePinInput.getNewPin()); + mSecurityTokenConnection.setAdminPin(new Passphrase(changePinInput.getAdminPin())); + mSecurityTokenConnection.resetPin(changePinInput.getNewPin()); - resultTokenInfo = mSecurityTokenHelper.getTokenInfo(); + resultTokenInfo = mSecurityTokenConnection.getTokenInfo(); } @Override @@ -156,11 +156,11 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE); - if (mSecurityTokenHelper.isPersistentConnectionAllowed()) { + if (mSecurityTokenConnection.isPersistentConnectionAllowed()) { // Just close finish(); } else { - mSecurityTokenHelper.clearSecureMessaging(); + mSecurityTokenConnection.clearSecureMessaging(); new AsyncTask() { @Override protected Void doInBackground(Void... params) { 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 a29615405..1cd2ecd8f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java @@ -190,7 +190,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { switch (mRequiredInput.mType) { case SECURITY_TOKEN_DECRYPT: { long tokenKeyId = KeyFormattingUtils.getKeyIdFromFingerprint( - mSecurityTokenHelper.getKeyFingerprint(KeyType.ENCRYPT)); + mSecurityTokenConnection.getKeyFingerprint(KeyType.ENCRYPT)); if (tokenKeyId != mRequiredInput.getSubKeyId()) { throw new IOException(getString(R.string.error_wrong_security_token)); @@ -208,14 +208,15 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { for (int i = 0; i < mRequiredInput.mInputData.length; i++) { byte[] encryptedSessionKey = mRequiredInput.mInputData[i]; - byte[] decryptedSessionKey = mSecurityTokenHelper.decryptSessionKey(encryptedSessionKey, publicKeyRing.getPublicKey(tokenKeyId)); + byte[] decryptedSessionKey = mSecurityTokenConnection + .decryptSessionKey(encryptedSessionKey, publicKeyRing.getPublicKey(tokenKeyId)); mInputParcel = mInputParcel.withCryptoData(encryptedSessionKey, decryptedSessionKey); } break; } case SECURITY_TOKEN_SIGN: { long tokenKeyId = KeyFormattingUtils.getKeyIdFromFingerprint( - mSecurityTokenHelper.getKeyFingerprint(KeyType.SIGN)); + mSecurityTokenConnection.getKeyFingerprint(KeyType.SIGN)); if (tokenKeyId != mRequiredInput.getSubKeyId()) { throw new IOException(getString(R.string.error_wrong_security_token)); @@ -226,15 +227,15 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { for (int i = 0; i < mRequiredInput.mInputData.length; i++) { byte[] hash = mRequiredInput.mInputData[i]; int algo = mRequiredInput.mSignAlgos[i]; - byte[] signedHash = mSecurityTokenHelper.calculateSignature(hash, algo); + byte[] signedHash = mSecurityTokenConnection.calculateSignature(hash, algo); mInputParcel = mInputParcel.withCryptoData(hash, signedHash); } break; } case SECURITY_TOKEN_MOVE_KEY_TO_CARD: { // TODO: assume PIN and Admin PIN to be default for this operation - mSecurityTokenHelper.setPin(new Passphrase("123456")); - mSecurityTokenHelper.setAdminPin(new Passphrase("12345678")); + mSecurityTokenConnection.setPin(new Passphrase("123456")); + mSecurityTokenConnection.setAdminPin(new Passphrase("12345678")); KeyRepository keyRepository = KeyRepository.create(this); @@ -256,7 +257,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { long subkeyId = buf.getLong(); CanonicalizedSecretKey key = secretKeyRing.getSecretKey(subkeyId); - byte[] tokenSerialNumber = Arrays.copyOf(mSecurityTokenHelper.getAid(), 16); + byte[] tokenSerialNumber = Arrays.copyOf(mSecurityTokenConnection.getAid(), 16); Passphrase passphrase; try { @@ -266,21 +267,21 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { throw new IOException("Unable to get cached passphrase!"); } - mSecurityTokenHelper.changeKey(key, passphrase); + mSecurityTokenConnection.changeKey(key, passphrase); // TODO: Is this really used anywhere? mInputParcel = mInputParcel.withCryptoData(subkeyBytes, tokenSerialNumber); } // change PINs afterwards - mSecurityTokenHelper.modifyPin(0x81, newPin); - mSecurityTokenHelper.modifyPin(0x83, newAdminPin); + mSecurityTokenConnection.modifyPin(0x81, newPin); + mSecurityTokenConnection.modifyPin(0x83, newAdminPin); break; } case SECURITY_TOKEN_RESET_CARD: { - mSecurityTokenHelper.resetAndWipeToken(); - mResultTokenInfo = mSecurityTokenHelper.getTokenInfo(); + mSecurityTokenConnection.resetAndWipeToken(); + mResultTokenInfo = mSecurityTokenConnection.getTokenInfo(); break; } @@ -300,11 +301,11 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE); - if (mSecurityTokenHelper.isPersistentConnectionAllowed()) { + if (mSecurityTokenConnection.isPersistentConnectionAllowed()) { // Just close finish(); } else { - mSecurityTokenHelper.clearSecureMessaging(); + mSecurityTokenConnection.clearSecureMessaging(); new AsyncTask() { @Override protected Void doInBackground(Void... params) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java index 99e82353f..413573084 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java @@ -43,7 +43,7 @@ import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.securitytoken.CardException; import org.sufficientlysecure.keychain.securitytoken.NfcTransport; -import org.sufficientlysecure.keychain.securitytoken.SecurityTokenHelper; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo; import org.sufficientlysecure.keychain.securitytoken.Transport; import org.sufficientlysecure.keychain.securitytoken.UsbConnectionDispatcher; @@ -68,7 +68,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity private static final String FIDESMO_APP_PACKAGE = "com.fidesmo.sec.android"; - protected SecurityTokenHelper mSecurityTokenHelper = SecurityTokenHelper.getInstance(); + protected SecurityTokenConnection mSecurityTokenConnection = SecurityTokenConnection.getInstance(); protected TagDispatcher mNfcTagDispatcher; protected UsbConnectionDispatcher mUsbDispatcher; private boolean mTagHandlingEnabled; @@ -85,7 +85,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity * Override to implement SecurityToken operations (background thread) */ protected void doSecurityTokenInBackground() throws IOException { - tokenInfo = mSecurityTokenHelper.getTokenInfo(); + tokenInfo = mSecurityTokenConnection.getTokenInfo(); Log.d(Constants.TAG, "Security Token: " + tokenInfo); } @@ -250,7 +250,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity SecurityTokenInfo tokeninfo = null; try { - tokeninfo = mSecurityTokenHelper.getTokenInfo(); + tokeninfo = mSecurityTokenConnection.getTokenInfo(); } catch (IOException e2) { // don't care } @@ -271,7 +271,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity case 0x6982: { SecurityTokenInfo tokeninfo = null; try { - tokeninfo = mSecurityTokenHelper.getTokenInfo(); + tokeninfo = mSecurityTokenConnection.getTokenInfo(); } catch (IOException e2) { // don't care } @@ -325,7 +325,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity } // 6A82 app not installed on security token! case 0x6A82: { - if (mSecurityTokenHelper.isFidesmoToken()) { + if (mSecurityTokenConnection.isFidesmoToken()) { // Check if the Fidesmo app is installed if (isAndroidAppInstalled(FIDESMO_APP_PACKAGE)) { promptFidesmoPgpInstall(); @@ -396,7 +396,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity Passphrase passphrase = PassphraseCacheService.getCachedPassphrase(this, requiredInput.getMasterKeyId(), requiredInput.getSubKeyId()); if (passphrase != null) { - mSecurityTokenHelper.setPin(passphrase); + mSecurityTokenConnection.setPin(passphrase); return; } @@ -421,7 +421,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity return; } CryptoInputParcel input = data.getParcelableExtra(PassphraseDialogActivity.RESULT_CRYPTO_INPUT); - mSecurityTokenHelper.setPin(input.getPassphrase()); + mSecurityTokenConnection.setPin(input.getPassphrase()); break; } default: @@ -431,17 +431,17 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity protected void handleSecurityToken(Transport transport, Context ctx) throws IOException { // Don't reconnect if device was already connected - if (!(mSecurityTokenHelper.isPersistentConnectionAllowed() - && mSecurityTokenHelper.isConnected() - && mSecurityTokenHelper.getTransport().equals(transport))) { - mSecurityTokenHelper.setTransport(transport); - mSecurityTokenHelper.connectToDevice(ctx); + if (!(mSecurityTokenConnection.isPersistentConnectionAllowed() + && mSecurityTokenConnection.isConnected() + && mSecurityTokenConnection.getTransport().equals(transport))) { + mSecurityTokenConnection.setTransport(transport); + mSecurityTokenConnection.connectToDevice(ctx); } doSecurityTokenInBackground(); } public boolean isSecurityTokenConnected() { - return mSecurityTokenHelper.isConnected(); + return mSecurityTokenConnection.isConnected(); } public static class IsoDepNotSupportedException extends IOException { @@ -500,8 +500,8 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity mUsbDispatcher.onStart(); } - public SecurityTokenHelper getSecurityTokenHelper() { - return mSecurityTokenHelper; + public SecurityTokenConnection getSecurityTokenHelper() { + return mSecurityTokenConnection; } /** From 46b69d45c4c195eb21921f0e52d1d58b2a39567c Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 9 Oct 2017 02:46:52 +0200 Subject: [PATCH 02/13] explicitly pass around instance of SecurityTokenConnection --- .../SecurityTokenConnection.java | 161 +++++++++--------- .../keychain/ui/CreateKeyActivity.java | 7 +- ...curityTokenChangePinOperationActivity.java | 17 +- .../ui/SecurityTokenOperationActivity.java | 35 ++-- .../keychain/ui/ShowNfcSweetspotActivity.java | 3 +- .../ui/base/BaseSecurityTokenActivity.java | 52 +++--- .../keychain/ui/keyview/ViewKeyActivity.java | 5 +- 7 files changed, 132 insertions(+), 148 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 6a71a98f6..f26534e5b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -87,30 +87,34 @@ public class SecurityTokenConnection { private static final byte[] BLANK_FINGERPRINT = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + private static SecurityTokenConnection sCachedInstance; + private final JcaKeyFingerprintCalculator fingerprintCalculator = new JcaKeyFingerprintCalculator(); - private Transport mTransport; + @NonNull + private final Transport mTransport; + @NonNull + private final Passphrase mPin; + private CardCapabilities mCardCapabilities; private OpenPgpCapabilities mOpenPgpCapabilities; private SecureMessaging mSecureMessaging; - private Passphrase mPin; - private Passphrase mAdminPin; private boolean mPw1ValidatedForSignature; private boolean mPw1ValidatedForDecrypt; // Mode 82 does other things; consider renaming? private boolean mPw3Validated; - private SecurityTokenConnection() { + public static SecurityTokenConnection getInstanceForTransport(Transport transport, Passphrase pin) { + if (sCachedInstance == null || !sCachedInstance.isPersistentConnectionAllowed() || + !sCachedInstance.isConnected() || !sCachedInstance.mTransport.equals(transport)) { + sCachedInstance = new SecurityTokenConnection(transport, pin); + } + return sCachedInstance; } - public static double parseOpenPgpVersion(final byte[] aid) { - float minv = aid[7]; - while (minv > 0) minv /= 10.0; - return aid[6] + minv; - } - - public static SecurityTokenConnection getInstance() { - return LazyHolder.SECURITY_TOKEN_HELPER; + private SecurityTokenConnection(@NonNull Transport transport, @NonNull Passphrase pin) { + this.mTransport = transport; + this.mPin = pin; } private String getHolderName(byte[] name) { @@ -126,23 +130,7 @@ public class SecurityTokenConnection { } } - public Passphrase getPin() { - return mPin; - } - - public void setPin(final Passphrase pin) { - this.mPin = pin; - } - - public Passphrase getAdminPin() { - return mAdminPin; - } - - public void setAdminPin(final Passphrase adminPin) { - this.mAdminPin = adminPin; - } - - public void changeKey(CanonicalizedSecretKey secretKey, Passphrase passphrase) throws IOException { + public void changeKey(CanonicalizedSecretKey secretKey, Passphrase passphrase, Passphrase adminPin) throws IOException { long keyGenerationTimestamp = secretKey.getCreationTime().getTime() / 1000; byte[] timestampBytes = ByteBuffer.allocate(4).putInt((int) keyGenerationTimestamp).array(); KeyType keyType = KeyType.from(secretKey); @@ -160,9 +148,9 @@ public class SecurityTokenConnection { keyType.toString())); } - putKey(keyType, secretKey, passphrase); - putData(keyType.getFingerprintObjectId(), secretKey.getFingerprint()); - putData(keyType.getTimestampObjectId(), timestampBytes); + putKey(keyType, secretKey, passphrase, adminPin); + putData(adminPin, keyType.getFingerprintObjectId(), secretKey.getFingerprint()); + putData(adminPin, keyType.getTimestampObjectId(), timestampBytes); } private boolean isSlotEmpty(KeyType keyType) throws IOException { @@ -179,12 +167,18 @@ public class SecurityTokenConnection { return java.util.Arrays.equals(getKeyFingerprint(keyType), fingerprint); } + public void connectIfNecessary(Context context) throws IOException { + if (isConnected()) { + return; + } + + connectToDevice(context); + } + /** * Connect to device and select pgp applet - * - * @throws IOException */ - public void connectToDevice(final Context ctx) throws IOException { + private void connectToDevice(Context context) throws IOException { // Connect on transport layer mCardCapabilities = new CardCapabilities(); @@ -208,7 +202,7 @@ public class SecurityTokenConnection { if (mOpenPgpCapabilities.isHasSCP11bSM()) { try { - SCP11bSecureMessaging.establish(this, ctx); + SCP11bSecureMessaging.establish(this, context); } catch (SecureMessagingException e) { mSecureMessaging = null; Log.e(Constants.TAG, "failed to establish secure messaging", e); @@ -217,9 +211,9 @@ public class SecurityTokenConnection { } - public void resetPin(String newPinStr) throws IOException { + public void resetPin(Passphrase adminPin, String newPinStr) throws IOException { if (!mPw3Validated) { - verifyPin(0x83); // (Verify PW1 with mode 82 for decryption) + verifyAdminPin(adminPin); } byte[] newPin = newPinStr.getBytes(); @@ -246,7 +240,7 @@ public class SecurityTokenConnection { * @param pw For PW1, this is 0x81. For PW3 (Admin PIN), mode is 0x83. * @param newPin The new PW1 or PW3. */ - public void modifyPin(int pw, byte[] newPin) throws IOException { + public void modifyPin(int pw, byte[] newPin, Passphrase adminPin) throws IOException { final int MAX_PW1_LENGTH_INDEX = 1; final int MAX_PW3_LENGTH_INDEX = 3; @@ -266,7 +260,10 @@ public class SecurityTokenConnection { byte[] pin; if (pw == 0x83) { - pin = mAdminPin.toStringUnsafe().getBytes(); + if (adminPin == null) { + throw new IllegalArgumentException("Changing the admin pin requires admin pin argument!"); + } + pin = adminPin.toStringUnsafe().getBytes(); } else { pin = mPin.toStringUnsafe().getBytes(); } @@ -422,28 +419,30 @@ public class SecurityTokenConnection { * For PW3 (Admin PIN), mode is 0x83. */ private void verifyPin(int mode) throws IOException { - if (mPin != null || mode == 0x83) { + byte[] pin = mPin.toStringUnsafe().getBytes(); - byte[] pin; - if (mode == 0x83) { - pin = mAdminPin.toStringUnsafe().getBytes(); - } else { - pin = mPin.toStringUnsafe().getBytes(); - } - - ResponseAPDU response = tryPin(mode, pin);// login - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Bad PIN!", response.getSW()); - } - - if (mode == 0x81) { - mPw1ValidatedForSignature = true; - } else if (mode == 0x82) { - mPw1ValidatedForDecrypt = true; - } else if (mode == 0x83) { - mPw3Validated = true; - } + ResponseAPDU response = tryPin(mode, pin);// login + if (response.getSW() != APDU_SW_SUCCESS) { + throw new CardException("Bad PIN!", response.getSW()); } + + if (mode == 0x81) { + mPw1ValidatedForSignature = true; + } else if (mode == 0x82) { + mPw1ValidatedForDecrypt = true; + } + } + + /** + * Verifies the user's PW1 or PW3 with the appropriate mode. + */ + private void verifyAdminPin(Passphrase adminPin) throws IOException { + ResponseAPDU response = tryPin(0x83, adminPin.toStringUnsafe().getBytes()); + if (response.getSW() != APDU_SW_SUCCESS) { + throw new CardException("Bad PIN!", response.getSW()); + } + + mPw3Validated = true; } /** @@ -454,16 +453,17 @@ public class SecurityTokenConnection { * @param dataObject The data object to be stored. * @param data The data to store in the object */ - private void putData(int dataObject, byte[] data) throws IOException { + private void putData(Passphrase adminPin, int dataObject, byte[] data) throws IOException { if (data.length > 254) { throw new IOException("Cannot PUT DATA with length > 254"); } + // TODO use admin pin regardless, if we have it? if (dataObject == 0x0101 || dataObject == 0x0103) { if (!mPw1ValidatedForDecrypt) { verifyPin(0x82); // (Verify PW1 for non-signing operations) } } else if (!mPw3Validated) { - verifyPin(0x83); // (Verify PW3) + verifyAdminPin(adminPin); } CommandAPDU command = new CommandAPDU(0x00, 0xDA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); @@ -475,7 +475,7 @@ public class SecurityTokenConnection { } - private void setKeyAttributes(final KeyType slot, final CanonicalizedSecretKey secretKey) + private void setKeyAttributes(Passphrase adminPin, final KeyType slot, final CanonicalizedSecretKey secretKey) throws IOException { if (mOpenPgpCapabilities.isAttributesChangable()) { @@ -493,7 +493,7 @@ public class SecurityTokenConnection { try { - putData(tag, SecurityTokenUtils.attributesFromSecretKey(slot, secretKey)); + putData(adminPin, tag, SecurityTokenUtils.attributesFromSecretKey(slot, secretKey)); mOpenPgpCapabilities.updateWithData(getData(0x00, tag)); @@ -512,14 +512,14 @@ public class SecurityTokenConnection { * 0xB8: Decipherment Key * 0xA4: Authentication Key */ - private void putKey(KeyType slot, CanonicalizedSecretKey secretKey, Passphrase passphrase) + private void putKey(KeyType slot, CanonicalizedSecretKey secretKey, Passphrase passphrase, Passphrase adminPin) throws IOException { RSAPrivateCrtKey crtSecretKey; ECPrivateKey ecSecretKey; ECPublicKey ecPublicKey; if (!mPw3Validated) { - verifyPin(0x83); // (Verify PW3 with mode 83) + verifyAdminPin(adminPin); } // Now we're ready to communicate with the token. @@ -528,7 +528,7 @@ public class SecurityTokenConnection { try { secretKey.unlock(passphrase); - setKeyAttributes(slot, secretKey); + setKeyAttributes(adminPin, slot, secretKey); switch (mOpenPgpCapabilities.getFormatForKeyType(slot).keyFormatType()) { case RSAKeyFormatType: @@ -836,15 +836,6 @@ public class SecurityTokenConnection { return lastResponse; } - public Transport getTransport() { - return mTransport; - } - - public void setTransport(Transport mTransport) { - clearSecureMessaging(); - this.mTransport = mTransport; - } - public boolean isFidesmoToken() { if (isConnected()) { // Check if we can still talk to the card try { @@ -873,13 +864,13 @@ public class SecurityTokenConnection { * @return the public key data objects, in TLV format. For RSA this will be the public modulus * (0x81) and exponent (0x82). These may come out of order; proper TLV parsing is required. */ - public byte[] generateKey(int slot) throws IOException { + public byte[] generateKey(Passphrase adminPin, int slot) throws IOException { if (slot != 0xB6 && slot != 0xB8 && slot != 0xA4) { throw new IOException("Invalid key slot"); } if (!mPw3Validated) { - verifyPin(0x83); // (Verify PW3 with mode 83) + verifyAdminPin(adminPin); } CommandAPDU apdu = new CommandAPDU(0x00, 0x47, 0x80, 0x00, new byte[]{(byte) slot, 0x00}, MAX_APDU_NE_EXT); @@ -962,14 +953,12 @@ public class SecurityTokenConnection { } public boolean isPersistentConnectionAllowed() { - return mTransport != null && - mTransport.isPersistentConnectionAllowed() && - (mSecureMessaging == null || - !mSecureMessaging.isEstablished()); + return mTransport.isPersistentConnectionAllowed() && + (mSecureMessaging == null || !mSecureMessaging.isEstablished()); } public boolean isConnected() { - return mTransport != null && mTransport.isConnected(); + return mTransport.isConnected(); } public void clearSecureMessaging() { @@ -1006,7 +995,9 @@ public class SecurityTokenConnection { return SecurityTokenInfo.create(fingerprints, aid, userId, url, pwInfo[4], pwInfo[6]); } - private static class LazyHolder { - private static final SecurityTokenConnection SECURITY_TOKEN_HELPER = new SecurityTokenConnection(); + public static double parseOpenPgpVersion(final byte[] aid) { + float minv = aid[7]; + while (minv > 0) minv /= 10.0; + return aid[6] + minv; } } 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 51d158710..fcb947016 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java @@ -32,6 +32,7 @@ import android.support.v4.app.TaskStackBuilder; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.securitytoken.KeyFormat; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo; import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity; import org.sufficientlysecure.keychain.ui.token.ManageSecurityTokenFragment; @@ -133,17 +134,17 @@ public class CreateKeyActivity extends BaseSecurityTokenActivity { } @Override - protected void doSecurityTokenInBackground() throws IOException { + protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException { if (mCurrentFragment instanceof SecurityTokenListenerFragment) { ((SecurityTokenListenerFragment) mCurrentFragment).doSecurityTokenInBackground(); return; } - tokenInfo = mSecurityTokenConnection.getTokenInfo(); + tokenInfo = stConnection.getTokenInfo(); } @Override - protected void onSecurityTokenPostExecute() { + protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) { handleTokenInfo(tokenInfo); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java index 95ac39143..1c03b00e7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java @@ -32,6 +32,7 @@ import android.widget.ViewAnimator; import nordpol.android.NfcGuideView; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo; import org.sufficientlysecure.keychain.service.input.SecurityTokenChangePinParcel; import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity; @@ -138,15 +139,15 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc } @Override - protected void doSecurityTokenInBackground() throws IOException { - mSecurityTokenConnection.setAdminPin(new Passphrase(changePinInput.getAdminPin())); - mSecurityTokenConnection.resetPin(changePinInput.getNewPin()); + protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException { + Passphrase adminPin = new Passphrase(changePinInput.getAdminPin()); + stConnection.resetPin(adminPin, changePinInput.getNewPin()); - resultTokenInfo = mSecurityTokenConnection.getTokenInfo(); + resultTokenInfo = stConnection.getTokenInfo(); } @Override - protected final void onSecurityTokenPostExecute() { + protected final void onSecurityTokenPostExecute(final SecurityTokenConnection stConnection) { Intent result = new Intent(); result.putExtra(RESULT_TOKEN_INFO, resultTokenInfo); setResult(RESULT_OK, result); @@ -156,17 +157,17 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE); - if (mSecurityTokenConnection.isPersistentConnectionAllowed()) { + if (stConnection.isPersistentConnectionAllowed()) { // Just close finish(); } else { - mSecurityTokenConnection.clearSecureMessaging(); + stConnection.clearSecureMessaging(); new AsyncTask() { @Override protected Void doInBackground(Void... params) { // check all 200ms if Security Token has been taken away while (true) { - if (isSecurityTokenConnected()) { + if (stConnection.isConnected()) { try { Thread.sleep(200); } catch (InterruptedException ignored) { 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 1cd2ecd8f..4a3c60256 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java @@ -44,6 +44,7 @@ import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; import org.sufficientlysecure.keychain.provider.KeyRepository; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.securitytoken.KeyType; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo; import org.sufficientlysecure.keychain.service.PassphraseCacheService; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; @@ -185,12 +186,12 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { } @Override - protected void doSecurityTokenInBackground() throws IOException { + protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException { switch (mRequiredInput.mType) { case SECURITY_TOKEN_DECRYPT: { long tokenKeyId = KeyFormattingUtils.getKeyIdFromFingerprint( - mSecurityTokenConnection.getKeyFingerprint(KeyType.ENCRYPT)); + stConnection.getKeyFingerprint(KeyType.ENCRYPT)); if (tokenKeyId != mRequiredInput.getSubKeyId()) { throw new IOException(getString(R.string.error_wrong_security_token)); @@ -208,7 +209,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { for (int i = 0; i < mRequiredInput.mInputData.length; i++) { byte[] encryptedSessionKey = mRequiredInput.mInputData[i]; - byte[] decryptedSessionKey = mSecurityTokenConnection + byte[] decryptedSessionKey = stConnection .decryptSessionKey(encryptedSessionKey, publicKeyRing.getPublicKey(tokenKeyId)); mInputParcel = mInputParcel.withCryptoData(encryptedSessionKey, decryptedSessionKey); } @@ -216,7 +217,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { } case SECURITY_TOKEN_SIGN: { long tokenKeyId = KeyFormattingUtils.getKeyIdFromFingerprint( - mSecurityTokenConnection.getKeyFingerprint(KeyType.SIGN)); + stConnection.getKeyFingerprint(KeyType.SIGN)); if (tokenKeyId != mRequiredInput.getSubKeyId()) { throw new IOException(getString(R.string.error_wrong_security_token)); @@ -227,15 +228,13 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { for (int i = 0; i < mRequiredInput.mInputData.length; i++) { byte[] hash = mRequiredInput.mInputData[i]; int algo = mRequiredInput.mSignAlgos[i]; - byte[] signedHash = mSecurityTokenConnection.calculateSignature(hash, algo); + byte[] signedHash = stConnection.calculateSignature(hash, algo); mInputParcel = mInputParcel.withCryptoData(hash, signedHash); } break; } case SECURITY_TOKEN_MOVE_KEY_TO_CARD: { - // TODO: assume PIN and Admin PIN to be default for this operation - mSecurityTokenConnection.setPin(new Passphrase("123456")); - mSecurityTokenConnection.setAdminPin(new Passphrase("12345678")); + Passphrase adminPin = new Passphrase("12345678"); KeyRepository keyRepository = KeyRepository.create(this); @@ -257,7 +256,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { long subkeyId = buf.getLong(); CanonicalizedSecretKey key = secretKeyRing.getSecretKey(subkeyId); - byte[] tokenSerialNumber = Arrays.copyOf(mSecurityTokenConnection.getAid(), 16); + byte[] tokenSerialNumber = Arrays.copyOf(stConnection.getAid(), 16); Passphrase passphrase; try { @@ -267,21 +266,21 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { throw new IOException("Unable to get cached passphrase!"); } - mSecurityTokenConnection.changeKey(key, passphrase); + stConnection.changeKey(key, passphrase, adminPin); // TODO: Is this really used anywhere? mInputParcel = mInputParcel.withCryptoData(subkeyBytes, tokenSerialNumber); } // change PINs afterwards - mSecurityTokenConnection.modifyPin(0x81, newPin); - mSecurityTokenConnection.modifyPin(0x83, newAdminPin); + stConnection.modifyPin(0x81, newPin, null); + stConnection.modifyPin(0x83, newAdminPin, adminPin); break; } case SECURITY_TOKEN_RESET_CARD: { - mSecurityTokenConnection.resetAndWipeToken(); - mResultTokenInfo = mSecurityTokenConnection.getTokenInfo(); + stConnection.resetAndWipeToken(); + mResultTokenInfo = stConnection.getTokenInfo(); break; } @@ -293,7 +292,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { } @Override - protected final void onSecurityTokenPostExecute() { + protected final void onSecurityTokenPostExecute(final SecurityTokenConnection stConnection) { handleResult(mInputParcel); // show finish @@ -301,17 +300,17 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { nfcGuideView.setCurrentStatus(NfcGuideView.NfcGuideViewStatus.DONE); - if (mSecurityTokenConnection.isPersistentConnectionAllowed()) { + if (stConnection.isPersistentConnectionAllowed()) { // Just close finish(); } else { - mSecurityTokenConnection.clearSecureMessaging(); + stConnection.clearSecureMessaging(); new AsyncTask() { @Override protected Void doInBackground(Void... params) { // check all 200ms if Security Token has been taken away while (true) { - if (isSecurityTokenConnected()) { + if (stConnection.isConnected()) { try { Thread.sleep(200); } catch (InterruptedException ignored) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ShowNfcSweetspotActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ShowNfcSweetspotActivity.java index 1a3f4cef3..1893d1f78 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ShowNfcSweetspotActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ShowNfcSweetspotActivity.java @@ -15,6 +15,7 @@ import android.view.animation.DecelerateInterpolator; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.securitytoken.NfcSweetspotData; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity; @@ -88,7 +89,7 @@ public class ShowNfcSweetspotActivity extends BaseSecurityTokenActivity { } @Override - protected void onSecurityTokenPostExecute() { + protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) { Intent result = new Intent(); result.putExtra(EXTRA_TOKEN_INFO, tokenInfo); setResult(Activity.RESULT_OK, result); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java index 413573084..26470a9f6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java @@ -68,12 +68,12 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity private static final String FIDESMO_APP_PACKAGE = "com.fidesmo.sec.android"; - protected SecurityTokenConnection mSecurityTokenConnection = SecurityTokenConnection.getInstance(); protected TagDispatcher mNfcTagDispatcher; protected UsbConnectionDispatcher mUsbDispatcher; private boolean mTagHandlingEnabled; protected SecurityTokenInfo tokenInfo; + private Passphrase mCachedPin; /** * Override to change UI before SecurityToken handling (UI thread) @@ -84,15 +84,15 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity /** * Override to implement SecurityToken operations (background thread) */ - protected void doSecurityTokenInBackground() throws IOException { - tokenInfo = mSecurityTokenConnection.getTokenInfo(); + protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException { + tokenInfo = stConnection.getTokenInfo(); Log.d(Constants.TAG, "Security Token: " + tokenInfo); } /** * Override to handle result of SecurityToken operations (UI thread) */ - protected void onSecurityTokenPostExecute() { + protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) { Intent intent = new Intent(this, CreateKeyActivity.class); intent.putExtra(CreateKeyActivity.EXTRA_SECURITY_TOKEN_INFO, tokenInfo); startActivity(intent); @@ -138,6 +138,10 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity // Actual Security Token operations are executed in doInBackground to not block the UI thread if (!mTagHandlingEnabled) return; + + final SecurityTokenConnection stConnection = + SecurityTokenConnection.getInstanceForTransport(transport, mCachedPin); + new AsyncTask() { @Override protected void onPreExecute() { @@ -148,7 +152,9 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity @Override protected IOException doInBackground(Void... params) { try { - handleSecurityToken(transport, BaseSecurityTokenActivity.this); + stConnection.connectIfNecessary(getBaseContext()); + + handleSecurityToken(stConnection); } catch (IOException e) { return e; } @@ -161,11 +167,11 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity super.onPostExecute(exception); if (exception != null) { - handleSecurityTokenError(exception); + handleSecurityTokenError(stConnection, exception); return; } - onSecurityTokenPostExecute(); + onSecurityTokenPostExecute(stConnection); } }.execute(); } @@ -223,7 +229,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity mNfcTagDispatcher.interceptIntent(intent); } - private void handleSecurityTokenError(IOException e) { + private void handleSecurityTokenError(SecurityTokenConnection stConnection, IOException e) { if (e instanceof TagLostException) { onSecurityTokenError(getString(R.string.security_token_error_tag_lost)); @@ -250,7 +256,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity SecurityTokenInfo tokeninfo = null; try { - tokeninfo = mSecurityTokenConnection.getTokenInfo(); + tokeninfo = stConnection.getTokenInfo(); } catch (IOException e2) { // don't care } @@ -271,7 +277,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity case 0x6982: { SecurityTokenInfo tokeninfo = null; try { - tokeninfo = mSecurityTokenConnection.getTokenInfo(); + tokeninfo = stConnection.getTokenInfo(); } catch (IOException e2) { // don't care } @@ -325,7 +331,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity } // 6A82 app not installed on security token! case 0x6A82: { - if (mSecurityTokenConnection.isFidesmoToken()) { + if (stConnection.isFidesmoToken()) { // Check if the Fidesmo app is installed if (isAndroidAppInstalled(FIDESMO_APP_PACKAGE)) { promptFidesmoPgpInstall(); @@ -391,12 +397,11 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity } protected void obtainSecurityTokenPin(RequiredInputParcel requiredInput) { - try { Passphrase passphrase = PassphraseCacheService.getCachedPassphrase(this, requiredInput.getMasterKeyId(), requiredInput.getSubKeyId()); if (passphrase != null) { - mSecurityTokenConnection.setPin(passphrase); + mCachedPin = passphrase; return; } @@ -421,7 +426,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity return; } CryptoInputParcel input = data.getParcelableExtra(PassphraseDialogActivity.RESULT_CRYPTO_INPUT); - mSecurityTokenConnection.setPin(input.getPassphrase()); + mCachedPin = input.getPassphrase(); break; } default: @@ -429,19 +434,8 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity } } - protected void handleSecurityToken(Transport transport, Context ctx) throws IOException { - // Don't reconnect if device was already connected - if (!(mSecurityTokenConnection.isPersistentConnectionAllowed() - && mSecurityTokenConnection.isConnected() - && mSecurityTokenConnection.getTransport().equals(transport))) { - mSecurityTokenConnection.setTransport(transport); - mSecurityTokenConnection.connectToDevice(ctx); - } - doSecurityTokenInBackground(); - } - - public boolean isSecurityTokenConnected() { - return mSecurityTokenConnection.isConnected(); + protected void handleSecurityToken(SecurityTokenConnection stConnection) throws IOException { + doSecurityTokenInBackground(stConnection); } public static class IsoDepNotSupportedException extends IOException { @@ -500,10 +494,6 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity mUsbDispatcher.onStart(); } - public SecurityTokenConnection getSecurityTokenHelper() { - return mSecurityTokenConnection; - } - /** * Run Security Token routines if last used token is connected and supports * persistent connections diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyActivity.java index 91e9916ed..0e9df8c4a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/ViewKeyActivity.java @@ -79,6 +79,7 @@ import org.sufficientlysecure.keychain.provider.KeyRepository; import org.sufficientlysecure.keychain.provider.KeyRepository.NotFoundException; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; +import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; import org.sufficientlysecure.keychain.service.ChangeUnlockParcel; import org.sufficientlysecure.keychain.service.ImportKeyringParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; @@ -619,8 +620,8 @@ public class ViewKeyActivity extends BaseSecurityTokenActivity implements } @Override - protected void onSecurityTokenPostExecute() { - super.onSecurityTokenPostExecute(); + protected void onSecurityTokenPostExecute(SecurityTokenConnection stConnection) { + super.onSecurityTokenPostExecute(stConnection); finish(); } From 8e9a62070d36e4b01c616683fb693a64fe48c008 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 23 Oct 2017 18:05:15 +0200 Subject: [PATCH 03/13] extract creation of CommandAPDUs into factory --- .../securitytoken/CommandAPDUFactory.java | 140 ++++++++++++++++++ .../securitytoken/SCP11bSecureMessaging.java | 16 +- .../SecurityTokenConnection.java | 78 ++++------ 3 files changed, 178 insertions(+), 56 deletions(-) create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java new file mode 100644 index 000000000..ce78d7e41 --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java @@ -0,0 +1,140 @@ +package org.sufficientlysecure.keychain.securitytoken; + + +import java.io.ByteArrayOutputStream; +import java.util.ArrayList; +import java.util.List; + +import android.support.annotation.NonNull; + +import javax.smartcardio.CommandAPDU; +import org.bouncycastle.util.Arrays; +import org.bouncycastle.util.encoders.Hex; + + +class CommandAPDUFactory { + private static final int MAX_APDU_NC = 255; + private static final int MAX_APDU_NC_EXT = 65535; + + private static final int MAX_APDU_NE = 256; + private static final int MAX_APDU_NE_EXT = 65536; + + final int CLA = 0x00; + + final int MASK_CLA_CHAINING = 1 << 4; + + @NonNull + CommandAPDU createPutKeyCommand(byte[] keyBytes) { + return new CommandAPDU(CLA, 0xDB, 0x3F, 0xFF, keyBytes); + } + + @NonNull + CommandAPDU createComputeDigitalSignatureCommand(byte[] data) { + return new CommandAPDU(CLA, 0x2A, 0x9E, 0x9A, data, MAX_APDU_NE_EXT); + } + + + @NonNull + CommandAPDU createPutDataCommand(int dataObject, byte[] data) { + return new CommandAPDU(CLA, 0xDA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); + } + + @NonNull + CommandAPDU createDecipherCommand(byte[] data) { + return new CommandAPDU(CLA, 0x2A, 0x80, 0x86, data, MAX_APDU_NE_EXT); + } + + @NonNull + CommandAPDU createChangeReferenceDataCommand(int pw, byte[] newPin, byte[] pin) { + return new CommandAPDU(CLA, 0x24, 0x00, pw, Arrays.concatenate(pin, newPin)); + } + + @NonNull + CommandAPDU createResetRetryCounter(byte[] newPin) { + return new CommandAPDU(CLA, 0x2C, 0x02, 0x81, newPin); + } + + @NonNull + CommandAPDU createGetDataCommand(int p1, int p2) { + return new CommandAPDU(CLA, 0xCA, p1, p2, MAX_APDU_NE_EXT); + } + + @NonNull + CommandAPDU createGenerateKeyCommand(int slot) { + return new CommandAPDU(CLA, 0x47, 0x80, 0x00, new byte[] { (byte) slot, 0x00 }, MAX_APDU_NE_EXT); + } + + @NonNull + CommandAPDU createGetResponseCommand(int lastResponseSw2) { + return new CommandAPDU(CLA, 0xC0, 0x00, 0x00, lastResponseSw2); + } + + + @NonNull + CommandAPDU createVerifyCommand(int mode, byte[] pin) { + return new CommandAPDU(CLA, 0x20, 0x00, mode, pin); + } + + @NonNull + CommandAPDU createSelectFileCommand(String fileAid) { + return new CommandAPDU(CLA, 0xA4, 0x04, 0x00, Hex.decode(fileAid)); + } + + @NonNull + CommandAPDU createReactivate2Command() { + return new CommandAPDU(CLA, 0x44, 0x00, 0x00); + } + + @NonNull + CommandAPDU createReactivate1Command() { + return new CommandAPDU(CLA, 0xE6, 0x00, 0x00); + } + + @NonNull + CommandAPDU createInternalAuthenticateCommand(ByteArrayOutputStream pkout) { + return new CommandAPDU(0, 0x88, 0x01, 0x0, pkout.toByteArray(), MAX_APDU_NE_EXT); + } + + @NonNull + CommandAPDU createRetrievePublicKeyCommand(byte[] msgCert) { + return new CommandAPDU(0, 0x47, 0x81, 0x00, msgCert, MAX_APDU_NE_EXT); + } + + @NonNull + CommandAPDU createRetrieveCertificateCommand() { + return new CommandAPDU(0, 0xA5, 0x03, 0x04, + new byte[]{0x60, 0x04, 0x5C, 0x02, 0x7F, 0x21}); + } + + @NonNull + CommandAPDU createShortApdu(CommandAPDU apdu) { + int ne = Math.min(apdu.getNe(), MAX_APDU_NE); + return new CommandAPDU(apdu.getCLA(), apdu.getINS(), apdu.getP1(), apdu.getP2(), apdu.getData(), ne); + } + + @NonNull + List createChainedApdus(CommandAPDU apdu) { + ArrayList result = new ArrayList<>(); + + int offset = 0; + byte[] data = apdu.getData(); + int ne = Math.min(apdu.getNe(), MAX_APDU_NE); + while (offset < data.length) { + int curLen = Math.min(MAX_APDU_NC, data.length - offset); + boolean last = offset + curLen >= data.length; + int cla = apdu.getCLA() + (last ? 0 : MASK_CLA_CHAINING); + + CommandAPDU cmd = + new CommandAPDU(cla, apdu.getINS(), apdu.getP1(), apdu.getP2(), data, offset, curLen, ne); + result.add(cmd); + + offset += curLen; + } + + return result; + } + + boolean isSuitableForShortApdu(CommandAPDU apdu) { + return apdu.getData().length <= MAX_APDU_NC; + } +} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index fd619ccd0..0f0aa9521 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -275,7 +275,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } - public static void establish(final SecurityTokenConnection t, final Context ctx) + public static void establish(final SecurityTokenConnection t, final Context ctx, CommandAPDUFactory commandFactory) throws SecureMessagingException, IOException { CommandAPDU cmd; @@ -285,8 +285,7 @@ class SCP11bSecureMessaging implements SecureMessaging { t.clearSecureMessaging(); // retrieving key algorithm - cmd = new CommandAPDU(0, (byte)0xCA, (byte)0x00, - OPENPGP_SECURE_MESSAGING_KEY_ATTRIBUTES_TAG, SecurityTokenConnection.MAX_APDU_NE_EXT); + cmd = commandFactory.createGetDataCommand(0x00, OPENPGP_SECURE_MESSAGING_KEY_ATTRIBUTES_TAG); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging key attributes"); @@ -317,13 +316,12 @@ class SCP11bSecureMessaging implements SecureMessaging { if (prefs != null && prefs.getExperimentalSmartPGPAuthoritiesEnable()) { // retrieving certificate - cmd = new CommandAPDU(0, (byte) 0xA5, (byte) 0x03, (byte) 0x04, - new byte[]{(byte) 0x60, (byte) 0x04, (byte) 0x5C, (byte) 0x02, (byte) 0x7F, (byte) 0x21}); + cmd = commandFactory.createRetrieveCertificateCommand(); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to select secure messaging certificate"); } - cmd = new CommandAPDU(0, (byte) 0xCA, (byte) 0x7F, (byte) 0x21, SecurityTokenConnection.MAX_APDU_NE_EXT); + cmd = commandFactory.createGetDataCommand(0x7F, 0x21); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging certificate"); @@ -333,8 +331,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } else { // retrieving public key - cmd = new CommandAPDU(0, (byte) 0x47, (byte) 0x81, (byte) 0x00, - OPENPGP_SECURE_MESSAGING_KEY_CRT, SecurityTokenConnection.MAX_APDU_NE_EXT); + cmd = commandFactory.createRetrievePublicKeyCommand(OPENPGP_SECURE_MESSAGING_KEY_CRT); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging public key"); @@ -395,8 +392,7 @@ class SCP11bSecureMessaging implements SecureMessaging { pkout = bout; // internal authenticate - cmd = new CommandAPDU(0, (byte)0x88, (byte)0x01, (byte)0x0, pkout.toByteArray(), - SecurityTokenConnection.MAX_APDU_NE_EXT); + cmd = commandFactory.createInternalAuthenticateCommand(pkout); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to initiate internal authenticate"); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index f26534e5b..1d95c2e9f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -64,6 +64,8 @@ import java.security.NoSuchAlgorithmException; import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; import java.security.interfaces.RSAPrivateCrtKey; +import java.util.List; + /** * This class provides a communication interface to OpenPGP applications on ISO SmartCard compliant @@ -71,17 +73,9 @@ import java.security.interfaces.RSAPrivateCrtKey; * For the full specs, see http://g10code.com/docs/openpgp-card-2.0.pdf */ public class SecurityTokenConnection { - private static final int MAX_APDU_NC = 255; - private static final int MAX_APDU_NC_EXT = 65535; - - private static final int MAX_APDU_NE = 256; - static final int MAX_APDU_NE_EXT = 65536; - static final int APDU_SW_SUCCESS = 0x9000; private static final int APDU_SW1_RESPONSE_AVAILABLE = 0x61; - private static final int MASK_CLA_CHAINING = 1 << 4; - // Fidesmo constants private static final String FIDESMO_APPS_AID_PREFIX = "A000000617"; @@ -95,6 +89,7 @@ public class SecurityTokenConnection { private final Transport mTransport; @NonNull private final Passphrase mPin; + private final CommandAPDUFactory commandFactory; private CardCapabilities mCardCapabilities; private OpenPgpCapabilities mOpenPgpCapabilities; @@ -115,6 +110,8 @@ public class SecurityTokenConnection { private SecurityTokenConnection(@NonNull Transport transport, @NonNull Passphrase pin) { this.mTransport = transport; this.mPin = pin; + + commandFactory = new CommandAPDUFactory(); } private String getHolderName(byte[] name) { @@ -186,7 +183,7 @@ public class SecurityTokenConnection { // Connect on smartcard layer // Command APDU (page 51) for SELECT FILE command (page 29) - CommandAPDU select = new CommandAPDU(0x00, 0xA4, 0x04, 0x00, Hex.decode("D27600012401")); + CommandAPDU select = commandFactory.createSelectFileCommand("D27600012401"); ResponseAPDU response = communicate(select); // activate connection if (response.getSW() != APDU_SW_SUCCESS) { @@ -202,7 +199,7 @@ public class SecurityTokenConnection { if (mOpenPgpCapabilities.isHasSCP11bSM()) { try { - SCP11bSecureMessaging.establish(this, context); + SCP11bSecureMessaging.establish(this, context, commandFactory); } catch (SecureMessagingException e) { mSecureMessaging = null; Log.e(Constants.TAG, "failed to establish secure messaging", e); @@ -225,7 +222,7 @@ public class SecurityTokenConnection { } // Command APDU for RESET RETRY COUNTER command (page 33) - CommandAPDU changePin = new CommandAPDU(0x00, 0x2C, 0x02, 0x81, newPin); + CommandAPDU changePin = commandFactory.createResetRetryCounter(newPin); ResponseAPDU response = communicate(changePin); if (response.getSW() != APDU_SW_SUCCESS) { @@ -269,7 +266,7 @@ public class SecurityTokenConnection { } // Command APDU for CHANGE REFERENCE DATA command (page 32) - CommandAPDU changePin = new CommandAPDU(0x00, 0x24, 0x00, pw, Arrays.concatenate(pin, newPin)); + CommandAPDU changePin = commandFactory.createChangeReferenceDataCommand(pw, newPin, pin); ResponseAPDU response = communicate(changePin); if (response.getSW() != APDU_SW_SUCCESS) { @@ -349,7 +346,7 @@ public class SecurityTokenConnection { throw new CardException("Unknown encryption key type!"); } - CommandAPDU command = new CommandAPDU(0x00, 0x2A, 0x80, 0x86, data, MAX_APDU_NE_EXT); + CommandAPDU command = commandFactory.createDecipherCommand(data); ResponseAPDU response = communicate(command); if (response.getSW() != APDU_SW_SUCCESS) { @@ -466,7 +463,7 @@ public class SecurityTokenConnection { verifyAdminPin(adminPin); } - CommandAPDU command = new CommandAPDU(0x00, 0xDA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); + CommandAPDU command = commandFactory.createPutDataCommand(dataObject, data); ResponseAPDU response = communicate(command); // put data if (response.getSW() != APDU_SW_SUCCESS) { @@ -474,7 +471,6 @@ public class SecurityTokenConnection { } } - private void setKeyAttributes(Passphrase adminPin, final KeyType slot, final CanonicalizedSecretKey secretKey) throws IOException { @@ -566,7 +562,7 @@ public class SecurityTokenConnection { throw new IOException(e.getMessage()); } - CommandAPDU apdu = new CommandAPDU(0x00, 0xDB, 0x3F, 0xFF, keyBytes); + CommandAPDU apdu = commandFactory.createPutKeyCommand(keyBytes); ResponseAPDU response = communicate(apdu); if (response.getSW() != APDU_SW_SUCCESS) { @@ -581,7 +577,7 @@ public class SecurityTokenConnection { * @return The fingerprints of all subkeys in a contiguous byte array. */ public byte[] getFingerprints() throws IOException { - CommandAPDU apdu = new CommandAPDU(0x00, 0xCA, 0x00, 0x6E, MAX_APDU_NE_EXT); + CommandAPDU apdu = commandFactory.createGetDataCommand(0x00, 0x6E); ResponseAPDU response = communicate(apdu); if (response.getSW() != APDU_SW_SUCCESS) { @@ -629,7 +625,7 @@ public class SecurityTokenConnection { } private byte[] getData(int p1, int p2) throws IOException { - ResponseAPDU response = communicate(new CommandAPDU(0x00, 0xCA, p1, p2, MAX_APDU_NE_EXT)); + ResponseAPDU response = communicate(commandFactory.createGetDataCommand(p1, p2)); if (response.getSW() != APDU_SW_SUCCESS) { throw new CardException("Failed to get pw status bytes", response.getSW()); } @@ -711,7 +707,7 @@ public class SecurityTokenConnection { } // Command APDU for PERFORM SECURITY OPERATION: COMPUTE DIGITAL SIGNATURE (page 37) - CommandAPDU command = new CommandAPDU(0x00, 0x2A, 0x9E, 0x9A, data, MAX_APDU_NE_EXT); + CommandAPDU command = commandFactory.createComputeDigitalSignatureCommand(data); ResponseAPDU response = communicate(command); if (response.getSW() != APDU_SW_SUCCESS) { @@ -756,7 +752,6 @@ public class SecurityTokenConnection { return signature; } - /** * Transceives APDU * Splits extended APDU into short APDUs and chains them if necessary @@ -776,45 +771,36 @@ public class SecurityTokenConnection { } } - ByteArrayOutputStream result = new ByteArrayOutputStream(); - ResponseAPDU lastResponse = null; // Transmit if (mCardCapabilities.hasExtended()) { lastResponse = mTransport.transceive(apdu); - } else if (apdu.getData().length <= MAX_APDU_NC) { - int ne = Math.min(apdu.getNe(), MAX_APDU_NE); - lastResponse = mTransport.transceive(new CommandAPDU(apdu.getCLA(), apdu.getINS(), - apdu.getP1(), apdu.getP2(), apdu.getData(), ne)); - } else if (apdu.getData().length > MAX_APDU_NC && mCardCapabilities.hasChaining()) { - int offset = 0; - byte[] data = apdu.getData(); - int ne = Math.min(apdu.getNe(), MAX_APDU_NE); - while (offset < data.length) { - int curLen = Math.min(MAX_APDU_NC, data.length - offset); - boolean last = offset + curLen >= data.length; - int cla = apdu.getCLA() + (last ? 0 : MASK_CLA_CHAINING); + } else if (commandFactory.isSuitableForShortApdu(apdu)) { + CommandAPDU shortApdu = commandFactory.createShortApdu(apdu); + lastResponse = mTransport.transceive(shortApdu); + } else if (mCardCapabilities.hasChaining()) { + List chainedApdus = commandFactory.createChainedApdus(apdu); + for (int i = 0, totalCommands = chainedApdus.size(); i < totalCommands; i++) { + CommandAPDU chainedApdu = chainedApdus.get(i); + lastResponse = mTransport.transceive(chainedApdu); - lastResponse = mTransport.transceive(new CommandAPDU(cla, apdu.getINS(), apdu.getP1(), - apdu.getP2(), data, offset, curLen, ne)); - - if (!last && lastResponse.getSW() != APDU_SW_SUCCESS) { + boolean isLastCommand = i < totalCommands - 1; + if (isLastCommand && lastResponse.getSW() != APDU_SW_SUCCESS) { throw new UsbTransportException("Failed to chain apdu (last SW: " + lastResponse.getSW() + ")"); } - - offset += curLen; } } if (lastResponse == null) { throw new UsbTransportException("Can't transmit command"); } + ByteArrayOutputStream result = new ByteArrayOutputStream(); result.write(lastResponse.getData()); // Receive while (lastResponse.getSW1() == APDU_SW1_RESPONSE_AVAILABLE) { // GET RESPONSE ISO/IEC 7816-4 par.7.6.1 - CommandAPDU getResponse = new CommandAPDU(0x00, 0xC0, 0x00, 0x00, lastResponse.getSW2()); + CommandAPDU getResponse = commandFactory.createGetResponseCommand(lastResponse.getSW2()); lastResponse = mTransport.transceive(getResponse); result.write(lastResponse.getData()); } @@ -841,7 +827,7 @@ public class SecurityTokenConnection { try { // By trying to select any apps that have the Fidesmo AID prefix we can // see if it is a Fidesmo device or not - CommandAPDU apdu = new CommandAPDU(0x00, 0xA4, 0x04, 0x00, Hex.decode(FIDESMO_APPS_AID_PREFIX)); + CommandAPDU apdu = commandFactory.createSelectFileCommand(FIDESMO_APPS_AID_PREFIX); return communicate(apdu).getSW() == APDU_SW_SUCCESS; } catch (IOException e) { Log.e(Constants.TAG, "Card communication failed!", e); @@ -873,7 +859,7 @@ public class SecurityTokenConnection { verifyAdminPin(adminPin); } - CommandAPDU apdu = new CommandAPDU(0x00, 0x47, 0x80, 0x00, new byte[]{(byte) slot, 0x00}, MAX_APDU_NE_EXT); + CommandAPDU apdu = commandFactory.createGenerateKeyCommand(slot); ResponseAPDU response = communicate(apdu); if (response.getSW() != APDU_SW_SUCCESS) { @@ -885,7 +871,7 @@ public class SecurityTokenConnection { private ResponseAPDU tryPin(int mode, byte[] pin) throws IOException { // Command APDU for VERIFY command (page 32) - return communicate(new CommandAPDU(0x00, 0x20, 0x00, mode, pin)); + return communicate(commandFactory.createVerifyCommand(mode, pin)); } /** @@ -918,8 +904,8 @@ public class SecurityTokenConnection { // reactivate token! // NOTE: keep the order here! First execute _both_ reactivate commands. Before checking _both_ responses // If a token is in a bad state and reactivate1 fails, it could still be reactivated with reactivate2 - CommandAPDU reactivate1 = new CommandAPDU(0x00, 0xE6, 0x00, 0x00); - CommandAPDU reactivate2 = new CommandAPDU(0x00, 0x44, 0x00, 0x00); + CommandAPDU reactivate1 = commandFactory.createReactivate1Command(); + CommandAPDU reactivate2 = commandFactory.createReactivate2Command(); ResponseAPDU response1 = communicate(reactivate1); ResponseAPDU response2 = communicate(reactivate2); if (response1.getSW() != APDU_SW_SUCCESS) { From b7723c1a4a4ea3b2367289a2a2b3c9b73daa9dde Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 15:58:21 +0200 Subject: [PATCH 04/13] replace magic constants in APDU factory --- .../securitytoken/CommandAPDUFactory.java | 156 ++++++++++++++---- .../securitytoken/SCP11bSecureMessaging.java | 33 ++-- .../SecurityTokenConnection.java | 111 +++++++------ .../ui/SecurityTokenOperationActivity.java | 4 +- 4 files changed, 200 insertions(+), 104 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java index ce78d7e41..583cf4376 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java @@ -1,7 +1,6 @@ package org.sufficientlysecure.keychain.securitytoken; -import java.io.ByteArrayOutputStream; import java.util.ArrayList; import java.util.List; @@ -19,91 +18,178 @@ class CommandAPDUFactory { private static final int MAX_APDU_NE = 256; private static final int MAX_APDU_NE_EXT = 65536; - final int CLA = 0x00; + private static final int CLA = 0x00; + private static final int MASK_CLA_CHAINING = 1 << 4; - final int MASK_CLA_CHAINING = 1 << 4; + private static final int INS_SELECT_FILE = 0xA4; + private static final int P1_SELECT_FILE = 0x04; + private static final byte[] AID_SELECT_FILE_OPENPGP = Hex.decode("D27600012401"); + + private static final int INS_ACTIVATE_FILE = 0x44; + private static final int INS_TERMINATE_DF = 0xE6; + private static final int INS_GET_RESPONSE = 0xC0; + + private static final int INS_INTERNAL_AUTHENTICATE = 0x88; + private static final int P1_INTERNAL_AUTH_SECURE_MESSAGING = 0x01; + + private static final int INS_VERIFY = 0x20; + private static final int P2_VERIFY_PW1_SIGN = 0x81; + private static final int P2_VERIFY_PW1_OTHER = 0x82; + private static final int P2_VERIFY_PW3 = 0x83; + + private static final int INS_CHANGE_REFERENCE_DATA = 0x24; + private static final int P2_CHANGE_REFERENCE_DATA_PW1 = 0x81; + private static final int P2_CHANGE_REFERENCE_DATA_PW3 = 0x83; + + private static final int INS_RESET_RETRY_COUNTER = 0x2C; + private static final int P1_RESET_RETRY_COUNTER_NEW_PW = 0x02; + private static final int P2_RESET_RETRY_COUNTER = 0x81; + + private static final int INS_PERFORM_SECURITY_OPERATION = 0x2A; + private static final int P1_PSO_DECIPHER = 0x80; + private static final int P1_PSO_COMPUTE_DIGITAL_SIGNATURE = 0x9E; + private static final int P2_PSO_DECIPHER = 0x86; + private static final int P2_PSO_COMPUTE_DIGITAL_SIGNATURE = 0x9A; + + private static final int INS_SELECT_DATA = 0xA5; + private static final int P1_SELECT_DATA_FOURTH = 0x03; + private static final int P2_SELECT_DATA = 0x04; + private static final byte[] CP_SELECT_DATA_CARD_HOLDER_CERT = Hex.decode("60045C027F21"); + + private static final int INS_GET_DATA = 0xCA; + private static final int P1_GET_DATA_CARD_HOLDER_CERT = 0x7F; + private static final int P2_GET_DATA_CARD_HOLDER_CERT = 0x21; + + private static final int INS_PUT_DATA = 0xDA; + + private static final int INS_PUT_DATA_ODD = 0xDB; + private static final int P1_PUT_DATA_ODD_KEY = 0x3F; + private static final int P2_PUT_DATA_ODD_KEY = 0xFF; + + private static final int INS_GENERATE_ASYMMETRIC_KEY_PAIR = 0x47; + private static final int P1_GAKP_GENERATE = 0x80; + private static final int P1_GAKP_READ_PUBKEY_TEMPLATE = 0x81; + private static final byte[] CRT_GAKP_SECURE_MESSAGING = Hex.decode("A600"); + + private static final int P1_EMPTY = 0x00; + private static final int P2_EMPTY = 0x00; + + @NonNull + CommandAPDU createPutDataCommand(int dataObject, byte[] data) { + return new CommandAPDU(CLA, INS_PUT_DATA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); + } @NonNull CommandAPDU createPutKeyCommand(byte[] keyBytes) { - return new CommandAPDU(CLA, 0xDB, 0x3F, 0xFF, keyBytes); + // the odd PUT DATA INS is for compliance with ISO 7816-8. This is used only to put key data on the card + return new CommandAPDU(CLA, INS_PUT_DATA_ODD, P1_PUT_DATA_ODD_KEY, P2_PUT_DATA_ODD_KEY, keyBytes); } @NonNull CommandAPDU createComputeDigitalSignatureCommand(byte[] data) { - return new CommandAPDU(CLA, 0x2A, 0x9E, 0x9A, data, MAX_APDU_NE_EXT); - } - - - @NonNull - CommandAPDU createPutDataCommand(int dataObject, byte[] data) { - return new CommandAPDU(CLA, 0xDA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); + return new CommandAPDU(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_COMPUTE_DIGITAL_SIGNATURE, + P2_PSO_COMPUTE_DIGITAL_SIGNATURE, data, MAX_APDU_NE_EXT); } @NonNull CommandAPDU createDecipherCommand(byte[] data) { - return new CommandAPDU(CLA, 0x2A, 0x80, 0x86, data, MAX_APDU_NE_EXT); + return new CommandAPDU(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data, + MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createChangeReferenceDataCommand(int pw, byte[] newPin, byte[] pin) { - return new CommandAPDU(CLA, 0x24, 0x00, pw, Arrays.concatenate(pin, newPin)); + CommandAPDU createChangePw1Command(byte[] pin, byte[] newPin) { + return new CommandAPDU(CLA, INS_CHANGE_REFERENCE_DATA, P1_EMPTY, + P2_CHANGE_REFERENCE_DATA_PW1, Arrays.concatenate(pin, newPin)); } @NonNull - CommandAPDU createResetRetryCounter(byte[] newPin) { - return new CommandAPDU(CLA, 0x2C, 0x02, 0x81, newPin); + CommandAPDU createChangePw3Command(byte[] adminPin, byte[] newAdminPin) { + return new CommandAPDU(CLA, INS_CHANGE_REFERENCE_DATA, P1_EMPTY, + P2_CHANGE_REFERENCE_DATA_PW3, Arrays.concatenate(adminPin, newAdminPin)); + } + + @NonNull + CommandAPDU createResetPw1Command(byte[] newPin) { + return new CommandAPDU(CLA, INS_RESET_RETRY_COUNTER, P1_RESET_RETRY_COUNTER_NEW_PW, + P2_RESET_RETRY_COUNTER, newPin); } @NonNull CommandAPDU createGetDataCommand(int p1, int p2) { - return new CommandAPDU(CLA, 0xCA, p1, p2, MAX_APDU_NE_EXT); - } - - @NonNull - CommandAPDU createGenerateKeyCommand(int slot) { - return new CommandAPDU(CLA, 0x47, 0x80, 0x00, new byte[] { (byte) slot, 0x00 }, MAX_APDU_NE_EXT); + return new CommandAPDU(CLA, INS_GET_DATA, p1, p2, MAX_APDU_NE_EXT); } @NonNull CommandAPDU createGetResponseCommand(int lastResponseSw2) { - return new CommandAPDU(CLA, 0xC0, 0x00, 0x00, lastResponseSw2); + return new CommandAPDU(CLA, INS_GET_RESPONSE, P1_EMPTY, P2_EMPTY, lastResponseSw2); } + @NonNull + CommandAPDU createVerifyPw1ForSignatureCommand(byte[] pin) { + return new CommandAPDU(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW1_SIGN, pin); + } @NonNull - CommandAPDU createVerifyCommand(int mode, byte[] pin) { - return new CommandAPDU(CLA, 0x20, 0x00, mode, pin); + CommandAPDU createVerifyPw1ForOtherCommand(byte[] pin) { + return new CommandAPDU(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW1_OTHER, pin); + } + + @NonNull + CommandAPDU createVerifyPw3Command(byte[] pin) { + return new CommandAPDU(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW3, pin); + } + + @NonNull + CommandAPDU createSelectFileOpenPgpCommand() { + return new CommandAPDU(CLA, INS_SELECT_FILE, P1_SELECT_FILE, P2_EMPTY, AID_SELECT_FILE_OPENPGP); } @NonNull CommandAPDU createSelectFileCommand(String fileAid) { - return new CommandAPDU(CLA, 0xA4, 0x04, 0x00, Hex.decode(fileAid)); + return new CommandAPDU(CLA, INS_SELECT_FILE, P1_SELECT_FILE, P2_EMPTY, Hex.decode(fileAid)); } @NonNull CommandAPDU createReactivate2Command() { - return new CommandAPDU(CLA, 0x44, 0x00, 0x00); + return new CommandAPDU(CLA, INS_ACTIVATE_FILE, P1_EMPTY, P2_EMPTY); } @NonNull CommandAPDU createReactivate1Command() { - return new CommandAPDU(CLA, 0xE6, 0x00, 0x00); + return new CommandAPDU(CLA, INS_TERMINATE_DF, P1_EMPTY, P2_EMPTY); } @NonNull - CommandAPDU createInternalAuthenticateCommand(ByteArrayOutputStream pkout) { - return new CommandAPDU(0, 0x88, 0x01, 0x0, pkout.toByteArray(), MAX_APDU_NE_EXT); + CommandAPDU createInternalAuthForSecureMessagingCommand(byte[] authData) { + return new CommandAPDU(CLA, INS_INTERNAL_AUTHENTICATE, P1_INTERNAL_AUTH_SECURE_MESSAGING, P2_EMPTY, authData, + MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createRetrievePublicKeyCommand(byte[] msgCert) { - return new CommandAPDU(0, 0x47, 0x81, 0x00, msgCert, MAX_APDU_NE_EXT); + CommandAPDU createGenerateKeyCommand(int slot) { + return new CommandAPDU(CLA, INS_GENERATE_ASYMMETRIC_KEY_PAIR, + P1_GAKP_GENERATE, P2_EMPTY, new byte[] { (byte) slot, 0x00 }, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createRetrieveCertificateCommand() { - return new CommandAPDU(0, 0xA5, 0x03, 0x04, - new byte[]{0x60, 0x04, 0x5C, 0x02, 0x7F, 0x21}); + CommandAPDU createRetrieveSecureMessagingPublicKeyCommand() { + // see https://github.com/ANSSI-FR/SmartPGP/blob/master/secure_messaging/smartpgp_sm.pdf + return new CommandAPDU(CLA, INS_GENERATE_ASYMMETRIC_KEY_PAIR, P1_GAKP_READ_PUBKEY_TEMPLATE, P2_EMPTY, + CRT_GAKP_SECURE_MESSAGING, MAX_APDU_NE_EXT); + } + + @NonNull + CommandAPDU createSelectSecureMessagingCertificateCommand() { + // see https://github.com/ANSSI-FR/SmartPGP/blob/master/secure_messaging/smartpgp_sm.pdf + // this command selects the fourth occurence of data tag 7F21 + return new CommandAPDU(CLA, INS_SELECT_DATA, P1_SELECT_DATA_FOURTH, P2_SELECT_DATA, + CP_SELECT_DATA_CARD_HOLDER_CERT); + } + + @NonNull + CommandAPDU createGetDataCardHolderCertCommand() { + return createGetDataCommand(P1_GET_DATA_CARD_HOLDER_CERT, P2_GET_DATA_CARD_HOLDER_CERT); } @NonNull diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index 0f0aa9521..15a76616b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -17,17 +17,6 @@ package org.sufficientlysecure.keychain.securitytoken; -import android.content.Context; -import android.support.annotation.NonNull; - -import org.bouncycastle.asn1.nist.NISTNamedCurves; -import org.bouncycastle.asn1.x9.ECNamedCurveTable; -import org.bouncycastle.asn1.x9.X9ECParameters; -import org.bouncycastle.math.ec.ECCurve; -import org.bouncycastle.math.ec.ECPoint; -import org.bouncycastle.util.Arrays; -import org.sufficientlysecure.keychain.ui.SettingsSmartPGPAuthoritiesActivity; -import org.sufficientlysecure.keychain.util.Preferences; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -65,6 +54,9 @@ import java.security.spec.InvalidKeySpecException; import java.security.spec.InvalidParameterSpecException; import java.util.ArrayList; +import android.content.Context; +import android.support.annotation.NonNull; + import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; @@ -76,12 +68,19 @@ import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; import javax.smartcardio.CommandAPDU; import javax.smartcardio.ResponseAPDU; +import org.bouncycastle.asn1.nist.NISTNamedCurves; +import org.bouncycastle.asn1.x9.ECNamedCurveTable; +import org.bouncycastle.asn1.x9.X9ECParameters; +import org.bouncycastle.math.ec.ECCurve; +import org.bouncycastle.math.ec.ECPoint; +import org.bouncycastle.util.Arrays; +import org.sufficientlysecure.keychain.ui.SettingsSmartPGPAuthoritiesActivity; +import org.sufficientlysecure.keychain.util.Preferences; class SCP11bSecureMessaging implements SecureMessaging { private static final byte OPENPGP_SECURE_MESSAGING_CLA_MASK = (byte)0x04; - private static final byte[] OPENPGP_SECURE_MESSAGING_KEY_CRT = new byte[] { (byte)0xA6, (byte)0 }; private static final byte OPENPGP_SECURE_MESSAGING_KEY_ATTRIBUTES_TAG = (byte)0xD4; private static final int AES_BLOCK_SIZE = 128 / 8; @@ -316,12 +315,12 @@ class SCP11bSecureMessaging implements SecureMessaging { if (prefs != null && prefs.getExperimentalSmartPGPAuthoritiesEnable()) { // retrieving certificate - cmd = commandFactory.createRetrieveCertificateCommand(); + cmd = commandFactory.createSelectSecureMessagingCertificateCommand(); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to select secure messaging certificate"); } - cmd = commandFactory.createGetDataCommand(0x7F, 0x21); + cmd = commandFactory.createGetDataCardHolderCertCommand(); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging certificate"); @@ -330,8 +329,7 @@ class SCP11bSecureMessaging implements SecureMessaging { pkcard = verifyCertificate(ctx, eckf, resp.getData()); } else { - // retrieving public key - cmd = commandFactory.createRetrievePublicKeyCommand(OPENPGP_SECURE_MESSAGING_KEY_CRT); + cmd = commandFactory.createRetrieveSecureMessagingPublicKeyCommand(); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to retrieve secure messaging public key"); @@ -391,8 +389,7 @@ class SCP11bSecureMessaging implements SecureMessaging { pkout.writeTo(bout); pkout = bout; - // internal authenticate - cmd = commandFactory.createInternalAuthenticateCommand(pkout); + cmd = commandFactory.createInternalAuthForSecureMessagingCommand(pkout.toByteArray()); resp = t.communicate(cmd); if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { throw new SecureMessagingException("failed to initiate internal authenticate"); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 1d95c2e9f..6e2aadc86 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -183,7 +183,7 @@ public class SecurityTokenConnection { // Connect on smartcard layer // Command APDU (page 51) for SELECT FILE command (page 29) - CommandAPDU select = commandFactory.createSelectFileCommand("D27600012401"); + CommandAPDU select = commandFactory.createSelectFileOpenPgpCommand(); ResponseAPDU response = communicate(select); // activate connection if (response.getSW() != APDU_SW_SUCCESS) { @@ -222,7 +222,7 @@ public class SecurityTokenConnection { } // Command APDU for RESET RETRY COUNTER command (page 33) - CommandAPDU changePin = commandFactory.createResetRetryCounter(newPin); + CommandAPDU changePin = commandFactory.createResetPw1Command(newPin); ResponseAPDU response = communicate(changePin); if (response.getSW() != APDU_SW_SUCCESS) { @@ -231,42 +231,48 @@ public class SecurityTokenConnection { } /** - * Modifies the user's PW1 or PW3. Before sending, the new PIN will be validated for + * Modifies the user's PW3. Before sending, the new PIN will be validated for * conformance to the token's requirements for key length. * - * @param pw For PW1, this is 0x81. For PW3 (Admin PIN), mode is 0x83. - * @param newPin The new PW1 or PW3. + * @param newAdminPin The new PW3. */ - public void modifyPin(int pw, byte[] newPin, Passphrase adminPin) throws IOException { - final int MAX_PW1_LENGTH_INDEX = 1; + public void modifyPw3Pin(byte[] newAdminPin, Passphrase adminPin) throws IOException { final int MAX_PW3_LENGTH_INDEX = 3; byte[] pwStatusBytes = getPwStatusBytes(); - if (pw == 0x81) { - if (newPin.length < 6 || newPin.length > pwStatusBytes[MAX_PW1_LENGTH_INDEX]) { - throw new IOException("Invalid PIN length"); - } - } else if (pw == 0x83) { - if (newPin.length < 8 || newPin.length > pwStatusBytes[MAX_PW3_LENGTH_INDEX]) { - throw new IOException("Invalid PIN length"); - } - } else { - throw new IOException("Invalid PW index for modify PIN operation"); + if (newAdminPin.length < 8 || newAdminPin.length > pwStatusBytes[MAX_PW3_LENGTH_INDEX]) { + throw new IOException("Invalid PIN length"); } - byte[] pin; - if (pw == 0x83) { - if (adminPin == null) { - throw new IllegalArgumentException("Changing the admin pin requires admin pin argument!"); - } - pin = adminPin.toStringUnsafe().getBytes(); - } else { - pin = mPin.toStringUnsafe().getBytes(); + byte[] pin = adminPin.toStringUnsafe().getBytes(); + + CommandAPDU changePin = commandFactory.createChangePw3Command(pin, newAdminPin); + ResponseAPDU response = communicate(changePin); + + if (response.getSW() != APDU_SW_SUCCESS) { + throw new CardException("Failed to change PIN", response.getSW()); + } + } + + /** + * Modifies the user's PW1. Before sending, the new PIN will be validated for + * conformance to the token's requirements for key length. + * + * @param newPin The new PW1. + */ + public void modifyPw1Pin(byte[] newPin) throws IOException { + final int MAX_PW1_LENGTH_INDEX = 1; + + byte[] pwStatusBytes = getPwStatusBytes(); + + if (newPin.length < 6 || newPin.length > pwStatusBytes[MAX_PW1_LENGTH_INDEX]) { + throw new IOException("Invalid PIN length"); } - // Command APDU for CHANGE REFERENCE DATA command (page 32) - CommandAPDU changePin = commandFactory.createChangeReferenceDataCommand(pw, newPin, pin); + byte[] pin = mPin.toStringUnsafe().getBytes(); + + CommandAPDU changePin = commandFactory.createChangePw1Command(pin, newPin); ResponseAPDU response = communicate(changePin); if (response.getSW() != APDU_SW_SUCCESS) { @@ -287,7 +293,7 @@ public class SecurityTokenConnection { final KeyFormat kf = mOpenPgpCapabilities.getFormatForKeyType(KeyType.ENCRYPT); if (!mPw1ValidatedForDecrypt) { - verifyPin(0x82); // (Verify PW1 with mode 82 for decryption) + verifyPinForOther(); } byte[] data; @@ -410,31 +416,41 @@ public class SecurityTokenConnection { } /** - * Verifies the user's PW1 or PW3 with the appropriate mode. - * - * @param mode For PW1, this is 0x81 for signing, 0x82 for everything else. - * For PW3 (Admin PIN), mode is 0x83. + * Verifies the user's PW1 with the appropriate mode. */ - private void verifyPin(int mode) throws IOException { + private void verifyPinForSignature() throws IOException { byte[] pin = mPin.toStringUnsafe().getBytes(); - ResponseAPDU response = tryPin(mode, pin);// login + ResponseAPDU response = communicate(commandFactory.createVerifyPw1ForSignatureCommand(pin)); if (response.getSW() != APDU_SW_SUCCESS) { throw new CardException("Bad PIN!", response.getSW()); } - if (mode == 0x81) { - mPw1ValidatedForSignature = true; - } else if (mode == 0x82) { - mPw1ValidatedForDecrypt = true; + mPw1ValidatedForSignature = true; + } + + /** + * Verifies the user's PW1 with the appropriate mode. + */ + private void verifyPinForOther() throws IOException { + byte[] pin = mPin.toStringUnsafe().getBytes(); + + // Command APDU for VERIFY command (page 32) + ResponseAPDU response = communicate(commandFactory.createVerifyPw1ForOtherCommand(pin)); + if (response.getSW() != APDU_SW_SUCCESS) { + throw new CardException("Bad PIN!", response.getSW()); } + + mPw1ValidatedForDecrypt = true; } /** * Verifies the user's PW1 or PW3 with the appropriate mode. */ private void verifyAdminPin(Passphrase adminPin) throws IOException { - ResponseAPDU response = tryPin(0x83, adminPin.toStringUnsafe().getBytes()); + // Command APDU for VERIFY command (page 32) + ResponseAPDU response = + communicate(commandFactory.createVerifyPw3Command(adminPin.toStringUnsafe().getBytes())); if (response.getSW() != APDU_SW_SUCCESS) { throw new CardException("Bad PIN!", response.getSW()); } @@ -457,7 +473,7 @@ public class SecurityTokenConnection { // TODO use admin pin regardless, if we have it? if (dataObject == 0x0101 || dataObject == 0x0103) { if (!mPw1ValidatedForDecrypt) { - verifyPin(0x82); // (Verify PW1 for non-signing operations) + verifyPinForOther(); } } else if (!mPw3Validated) { verifyAdminPin(adminPin); @@ -640,7 +656,7 @@ public class SecurityTokenConnection { */ public byte[] calculateSignature(byte[] hash, int hashAlgo) throws IOException { if (!mPw1ValidatedForSignature) { - verifyPin(0x81); // (Verify PW1 with mode 81 for signing) + verifyPinForSignature(); } byte[] dsi; @@ -869,11 +885,6 @@ public class SecurityTokenConnection { return response.getData(); } - private ResponseAPDU tryPin(int mode, byte[] pin) throws IOException { - // Command APDU for VERIFY command (page 32) - return communicate(commandFactory.createVerifyCommand(mode, pin)); - } - /** * Resets security token, which deletes all keys and data objects. * This works by entering a wrong PIN and then Admin PIN 4 times respectively. @@ -883,8 +894,9 @@ public class SecurityTokenConnection { // try wrong PIN 4 times until counter goes to C0 byte[] pin = "XXXXXX".getBytes(); for (int i = 0; i <= 4; i++) { - ResponseAPDU response = tryPin(0x81, pin); - if (response.getSW() == APDU_SW_SUCCESS) { // Should NOT accept! + // Command APDU for VERIFY command (page 32) + ResponseAPDU response = communicate(commandFactory.createVerifyPw1ForSignatureCommand(pin)); + if (response.getSW() == APDU_SW_SUCCESS) { throw new CardException("Should never happen, XXXXXX has been accepted!", response.getSW()); } } @@ -892,7 +904,8 @@ public class SecurityTokenConnection { // try wrong Admin PIN 4 times until counter goes to C0 byte[] adminPin = "XXXXXXXX".getBytes(); for (int i = 0; i <= 4; i++) { - ResponseAPDU response = tryPin(0x83, adminPin); + // Command APDU for VERIFY command (page 32) + ResponseAPDU response = communicate(commandFactory.createVerifyPw3Command(adminPin)); if (response.getSW() == APDU_SW_SUCCESS) { // Should NOT accept! throw new CardException("Should never happen, XXXXXXXX has been accepted", response.getSW()); } 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 4a3c60256..40d609842 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java @@ -273,8 +273,8 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { } // change PINs afterwards - stConnection.modifyPin(0x81, newPin, null); - stConnection.modifyPin(0x83, newAdminPin, adminPin); + stConnection.modifyPw1Pin(newPin); + stConnection.modifyPw3Pin(newAdminPin, adminPin); break; } From 9b292a4c707cdf6dc8e5b2982a0f62d59f6f0eb5 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 15:59:33 +0200 Subject: [PATCH 05/13] rename CommandAPDUFactory -> OpenPgpCommandApduFactory --- ...CommandAPDUFactory.java => OpenPgpCommandApduFactory.java} | 2 +- .../keychain/securitytoken/SCP11bSecureMessaging.java | 2 +- .../keychain/securitytoken/SecurityTokenConnection.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/{CommandAPDUFactory.java => OpenPgpCommandApduFactory.java} (99%) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java similarity index 99% rename from OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java rename to OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java index 583cf4376..2b57dd3f4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandAPDUFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java @@ -11,7 +11,7 @@ import org.bouncycastle.util.Arrays; import org.bouncycastle.util.encoders.Hex; -class CommandAPDUFactory { +class OpenPgpCommandApduFactory { private static final int MAX_APDU_NC = 255; private static final int MAX_APDU_NC_EXT = 65535; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index 15a76616b..c021ad662 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -274,7 +274,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } - public static void establish(final SecurityTokenConnection t, final Context ctx, CommandAPDUFactory commandFactory) + public static void establish(final SecurityTokenConnection t, final Context ctx, OpenPgpCommandApduFactory commandFactory) throws SecureMessagingException, IOException { CommandAPDU cmd; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 6e2aadc86..7a0ccbb77 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -89,7 +89,7 @@ public class SecurityTokenConnection { private final Transport mTransport; @NonNull private final Passphrase mPin; - private final CommandAPDUFactory commandFactory; + private final OpenPgpCommandApduFactory commandFactory; private CardCapabilities mCardCapabilities; private OpenPgpCapabilities mOpenPgpCapabilities; @@ -111,7 +111,7 @@ public class SecurityTokenConnection { this.mTransport = transport; this.mPin = pin; - commandFactory = new CommandAPDUFactory(); + commandFactory = new OpenPgpCommandApduFactory(); } private String getHolderName(byte[] name) { From e8103d83767f2309b120f776bcb25a61b16b17a6 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 16:40:37 +0200 Subject: [PATCH 06/13] use reset instead of modify for changing pw1 --- .../OpenPgpCommandApduFactory.java | 6 ---- .../SecurityTokenConnection.java | 29 +------------------ ...curityTokenChangePinOperationActivity.java | 2 +- .../ui/SecurityTokenOperationActivity.java | 2 +- 4 files changed, 3 insertions(+), 36 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java index 2b57dd3f4..dfee53265 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java @@ -97,12 +97,6 @@ class OpenPgpCommandApduFactory { MAX_APDU_NE_EXT); } - @NonNull - CommandAPDU createChangePw1Command(byte[] pin, byte[] newPin) { - return new CommandAPDU(CLA, INS_CHANGE_REFERENCE_DATA, P1_EMPTY, - P2_CHANGE_REFERENCE_DATA_PW1, Arrays.concatenate(pin, newPin)); - } - @NonNull CommandAPDU createChangePw3Command(byte[] adminPin, byte[] newAdminPin) { return new CommandAPDU(CLA, INS_CHANGE_REFERENCE_DATA, P1_EMPTY, diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 7a0ccbb77..dde5bc9ea 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -208,13 +208,11 @@ public class SecurityTokenConnection { } - public void resetPin(Passphrase adminPin, String newPinStr) throws IOException { + public void resetPin(byte[] newPin, Passphrase adminPin) throws IOException { if (!mPw3Validated) { verifyAdminPin(adminPin); } - byte[] newPin = newPinStr.getBytes(); - final int MAX_PW1_LENGTH_INDEX = 1; byte[] pwStatusBytes = getPwStatusBytes(); if (newPin.length < 6 || newPin.length > pwStatusBytes[MAX_PW1_LENGTH_INDEX]) { @@ -255,31 +253,6 @@ public class SecurityTokenConnection { } } - /** - * Modifies the user's PW1. Before sending, the new PIN will be validated for - * conformance to the token's requirements for key length. - * - * @param newPin The new PW1. - */ - public void modifyPw1Pin(byte[] newPin) throws IOException { - final int MAX_PW1_LENGTH_INDEX = 1; - - byte[] pwStatusBytes = getPwStatusBytes(); - - if (newPin.length < 6 || newPin.length > pwStatusBytes[MAX_PW1_LENGTH_INDEX]) { - throw new IOException("Invalid PIN length"); - } - - byte[] pin = mPin.toStringUnsafe().getBytes(); - - CommandAPDU changePin = commandFactory.createChangePw1Command(pin, newPin); - ResponseAPDU response = communicate(changePin); - - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to change PIN", response.getSW()); - } - } - /** * Call DECIPHER command * diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java index 1c03b00e7..36cf8bdf5 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenChangePinOperationActivity.java @@ -141,7 +141,7 @@ public class SecurityTokenChangePinOperationActivity extends BaseSecurityTokenAc @Override protected void doSecurityTokenInBackground(SecurityTokenConnection stConnection) throws IOException { Passphrase adminPin = new Passphrase(changePinInput.getAdminPin()); - stConnection.resetPin(adminPin, changePinInput.getNewPin()); + stConnection.resetPin(changePinInput.getNewPin().getBytes(), adminPin); resultTokenInfo = stConnection.getTokenInfo(); } 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 40d609842..8450f81fa 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SecurityTokenOperationActivity.java @@ -273,7 +273,7 @@ public class SecurityTokenOperationActivity extends BaseSecurityTokenActivity { } // change PINs afterwards - stConnection.modifyPw1Pin(newPin); + stConnection.resetPin(newPin, adminPin); stConnection.modifyPw3Pin(newAdminPin, adminPin); break; From a4af2f7f5c4ec9c4d4fe8fe6a251d50b75379cf4 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 16:57:12 +0200 Subject: [PATCH 07/13] rewrite ResponseApdu --- .../java/javax/smartcardio/CommandAPDU.java | 5 +- .../java/javax/smartcardio/ResponseAPDU.java | 184 ------------------ .../keychain/securitytoken/NfcTransport.java | 5 +- .../keychain/securitytoken/ResponseApdu.java | 62 ++++++ .../securitytoken/SCP11bSecureMessaging.java | 36 ++-- .../securitytoken/SecureMessaging.java | 6 +- .../SecurityTokenConnection.java | 122 ++++++------ .../keychain/securitytoken/Transport.java | 3 +- .../securitytoken/usb/UsbTransport.java | 6 +- 9 files changed, 151 insertions(+), 278 deletions(-) delete mode 100644 OpenKeychain/src/main/java/javax/smartcardio/ResponseAPDU.java create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java diff --git a/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java b/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java index 936d450d0..30e8331cd 100644 --- a/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java +++ b/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java @@ -29,6 +29,9 @@ import java.util.Arrays; import java.nio.ByteBuffer; +import org.sufficientlysecure.keychain.securitytoken.ResponseApdu; + + /** * A command APDU following the structure defined in ISO/IEC 7816-4. * It consists of a four byte header and a conditional body of variable length. @@ -55,7 +58,7 @@ import java.nio.ByteBuffer; *

Instances of this class are immutable. Where data is passed in or out * via byte arrays, defensive cloning is performed. * - * @see ResponseAPDU + * @see ResponseApdu * * @since 1.6 * @author Andreas Sterbenz diff --git a/OpenKeychain/src/main/java/javax/smartcardio/ResponseAPDU.java b/OpenKeychain/src/main/java/javax/smartcardio/ResponseAPDU.java deleted file mode 100644 index b135a2e82..000000000 --- a/OpenKeychain/src/main/java/javax/smartcardio/ResponseAPDU.java +++ /dev/null @@ -1,184 +0,0 @@ -/* - * Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code 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 - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package javax.smartcardio; - -import java.util.Arrays; - -/** - * A response APDU as defined in ISO/IEC 7816-4. It consists of a conditional - * body and a two byte trailer. - * This class does not attempt to verify that the APDU encodes a semantically - * valid response. - * - *

Instances of this class are immutable. Where data is passed in or out - * via byte arrays, defensive cloning is performed. - * - * @see CommandAPDU - * - * @since 1.6 - * @author Andreas Sterbenz - * @author JSR 268 Expert Group - */ -public final class ResponseAPDU implements java.io.Serializable { - - private static final long serialVersionUID = 6962744978375594225L; - - /** @serial */ - private byte[] apdu; - - /** - * Constructs a ResponseAPDU from a byte array containing the complete - * APDU contents (conditional body and trailed). - * - *

Note that the byte array is cloned to protect against subsequent - * modification. - * - * @param apdu the complete response APDU - * - * @throws NullPointerException if apdu is null - * @throws IllegalArgumentException if apdu.length is less than 2 - */ - public ResponseAPDU(byte[] apdu) { - apdu = apdu.clone(); - check(apdu); - this.apdu = apdu; - } - - private static void check(byte[] apdu) { - if (apdu.length < 2) { - throw new IllegalArgumentException("apdu must be at least 2 bytes long"); - } - } - - /** - * Returns the number of data bytes in the response body (Nr) or 0 if this - * APDU has no body. This call is equivalent to - * getData().length. - * - * @return the number of data bytes in the response body or 0 if this APDU - * has no body. - */ - public int getNr() { - return apdu.length - 2; - } - - /** - * Returns a copy of the data bytes in the response body. If this APDU as - * no body, this method returns a byte array with a length of zero. - * - * @return a copy of the data bytes in the response body or the empty - * byte array if this APDU has no body. - */ - public byte[] getData() { - byte[] data = new byte[apdu.length - 2]; - System.arraycopy(apdu, 0, data, 0, data.length); - return data; - } - - /** - * Returns the value of the status byte SW1 as a value between 0 and 255. - * - * @return the value of the status byte SW1 as a value between 0 and 255. - */ - public int getSW1() { - return apdu[apdu.length - 2] & 0xff; - } - - /** - * Returns the value of the status byte SW2 as a value between 0 and 255. - * - * @return the value of the status byte SW2 as a value between 0 and 255. - */ - public int getSW2() { - return apdu[apdu.length - 1] & 0xff; - } - - /** - * Returns the value of the status bytes SW1 and SW2 as a single - * status word SW. - * It is defined as - * (getSW1() << 8) | getSW2(). - * - * @return the value of the status word SW. - */ - public int getSW() { - return (getSW1() << 8) | getSW2(); - } - - /** - * Returns a copy of the bytes in this APDU. - * - * @return a copy of the bytes in this APDU. - */ - public byte[] getBytes() { - return apdu.clone(); - } - - /** - * Returns a string representation of this response APDU. - * - * @return a String representation of this response APDU. - */ - public String toString() { - return "ResponseAPDU: " + apdu.length + " bytes, SW=" - + Integer.toHexString(getSW()); - } - - /** - * Compares the specified object with this response APDU for equality. - * Returns true if the given object is also a ResponseAPDU and its bytes are - * identical to the bytes in this ResponseAPDU. - * - * @param obj the object to be compared for equality with this response APDU - * @return true if the specified object is equal to this response APDU - */ - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj instanceof ResponseAPDU == false) { - return false; - } - ResponseAPDU other = (ResponseAPDU)obj; - return Arrays.equals(this.apdu, other.apdu); - } - - /** - * Returns the hash code value for this response APDU. - * - * @return the hash code value for this response APDU. - */ - public int hashCode() { - return Arrays.hashCode(apdu); - } - - private void readObject(java.io.ObjectInputStream in) - throws java.io.IOException, ClassNotFoundException { - apdu = (byte[])in.readUnshared(); - check(apdu); - } - -} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java index 1485754ef..ac1c653ed 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java @@ -20,7 +20,6 @@ package org.sufficientlysecure.keychain.securitytoken; import android.nfc.Tag; import javax.smartcardio.CommandAPDU; -import javax.smartcardio.ResponseAPDU; import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity; import java.io.IOException; @@ -45,8 +44,8 @@ public class NfcTransport implements Transport { * @throws IOException */ @Override - public ResponseAPDU transceive(final CommandAPDU data) throws IOException { - return new ResponseAPDU(mIsoCard.transceive(data.getBytes())); + public ResponseApdu transceive(final CommandAPDU data) throws IOException { + return ResponseApdu.fromBytes(mIsoCard.transceive(data.getBytes())); } /** diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java new file mode 100644 index 000000000..0ace27b71 --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2016 Vincent Breitmoser + * + * 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 . + */ + +package org.sufficientlysecure.keychain.securitytoken; + +import java.util.Arrays; + +import com.google.auto.value.AutoValue; + + +/** A response APDU as defined in ISO/IEC 7816-4. */ +@AutoValue +public abstract class ResponseApdu { + private static final int APDU_SW_SUCCESS = 0x9000; + + public abstract byte[] getData(); + public abstract int getSw1(); + public abstract int getSw2(); + + public static ResponseApdu fromBytes(byte[] apdu) { + if (apdu.length < 2) { + throw new IllegalArgumentException("Response apdu must be 2 bytes or larger!"); + } + byte[] data = Arrays.copyOfRange(apdu, 0, apdu.length - 2); + int sw1 = apdu[apdu.length -2] & 0xff; + int sw2 = apdu[apdu.length -1] & 0xff; + return new AutoValue_ResponseApdu(data, sw1, sw2); + } + + public int getSw() { + return (getSw1() << 8) | getSw2(); + } + + public boolean isSuccess() { + return getSw() == APDU_SW_SUCCESS; + } + + public byte[] toBytes() { + byte[] data = getData(); + byte[] bytes = new byte[data.length + 2]; + System.arraycopy(data, 0, bytes, 0, data.length); + + bytes[bytes.length -2] = (byte) getSw1(); + bytes[bytes.length -1] = (byte) getSw2(); + + return bytes; + } +} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index c021ad662..a135caae9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -67,7 +67,6 @@ import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; import javax.smartcardio.CommandAPDU; -import javax.smartcardio.ResponseAPDU; import org.bouncycastle.asn1.nist.NISTNamedCurves; import org.bouncycastle.asn1.x9.ECNamedCurveTable; import org.bouncycastle.asn1.x9.X9ECParameters; @@ -278,7 +277,7 @@ class SCP11bSecureMessaging implements SecureMessaging { throws SecureMessagingException, IOException { CommandAPDU cmd; - ResponseAPDU resp; + ResponseApdu resp; Iso7816TLV[] tlvs; t.clearSecureMessaging(); @@ -286,7 +285,7 @@ class SCP11bSecureMessaging implements SecureMessaging { // retrieving key algorithm cmd = commandFactory.createGetDataCommand(0x00, OPENPGP_SECURE_MESSAGING_KEY_ATTRIBUTES_TAG); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { + if (!resp.isSuccess()) { throw new SecureMessagingException("failed to retrieve secure messaging key attributes"); } tlvs = Iso7816TLV.readList(resp.getData(), true); @@ -317,12 +316,12 @@ class SCP11bSecureMessaging implements SecureMessaging { // retrieving certificate cmd = commandFactory.createSelectSecureMessagingCertificateCommand(); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { + if (!resp.isSuccess()) { throw new SecureMessagingException("failed to select secure messaging certificate"); } cmd = commandFactory.createGetDataCardHolderCertCommand(); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { + if (!resp.isSuccess()) { throw new SecureMessagingException("failed to retrieve secure messaging certificate"); } @@ -331,7 +330,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } else { cmd = commandFactory.createRetrieveSecureMessagingPublicKeyCommand(); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { + if (!resp.isSuccess()) { throw new SecureMessagingException("failed to retrieve secure messaging public key"); } tlvs = Iso7816TLV.readList(resp.getData(), true); @@ -391,7 +390,7 @@ class SCP11bSecureMessaging implements SecureMessaging { cmd = commandFactory.createInternalAuthForSecureMessagingCommand(pkout.toByteArray()); resp = t.communicate(cmd); - if (resp.getSW() != SecurityTokenConnection.APDU_SW_SUCCESS) { + if (!resp.isSuccess()) { throw new SecureMessagingException("failed to initiate internal authenticate"); } @@ -605,7 +604,7 @@ class SCP11bSecureMessaging implements SecureMessaging { @Override - public ResponseAPDU verifyAndDecrypt(ResponseAPDU apdu) + public ResponseApdu verifyAndDecrypt(ResponseApdu apdu) throws SecureMessagingException { if (!isEstablished()) { @@ -614,10 +613,9 @@ class SCP11bSecureMessaging implements SecureMessaging { byte[] data = apdu.getData(); - if ((data.length == 0) && - (apdu.getSW() != 0x9000) && - (apdu.getSW1() != 0x62) && - (apdu.getSW1() != 0x63)) { + if ((data.length == 0) && !apdu.isSuccess() && + (apdu.getSw1() != 0x62) && + (apdu.getSw1() != 0x63)) { return apdu; } @@ -634,8 +632,8 @@ class SCP11bSecureMessaging implements SecureMessaging { if ((data.length - SCP11_MAC_LENGTH) > 0) { mac.update(data, 0, data.length - SCP11_MAC_LENGTH); } - mac.update((byte) apdu.getSW1()); - mac.update((byte) apdu.getSW2()); + mac.update((byte) apdu.getSw1()); + mac.update((byte) apdu.getSw2()); final byte[] sig = mac.doFinal(); @@ -675,19 +673,19 @@ class SCP11bSecureMessaging implements SecureMessaging { final byte[] datasw = new byte[i + 2]; System.arraycopy(data, 0, datasw, 0, i); - datasw[datasw.length - 2] = (byte) apdu.getSW1(); - datasw[datasw.length - 1] = (byte) apdu.getSW2(); + datasw[datasw.length - 2] = (byte) apdu.getSw1(); + datasw[datasw.length - 1] = (byte) apdu.getSw2(); Arrays.fill(data, (byte) 0); data = datasw; } else { data = new byte[2]; - data[0] = (byte) apdu.getSW1(); - data[1] = (byte) apdu.getSW2(); + data[0] = (byte) apdu.getSw1(); + data[1] = (byte) apdu.getSw2(); } - apdu = new ResponseAPDU(data); + apdu = ResponseApdu.fromBytes(data); return apdu; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java index 4107a3e18..a43219b2a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java @@ -17,10 +17,8 @@ package org.sufficientlysecure.keychain.securitytoken; -import java.io.IOException; - import javax.smartcardio.CommandAPDU; -import javax.smartcardio.ResponseAPDU; + public interface SecureMessaging { @@ -30,5 +28,5 @@ public interface SecureMessaging { CommandAPDU encryptAndSign(CommandAPDU apdu) throws SecureMessagingException; - ResponseAPDU verifyAndDecrypt(ResponseAPDU apdu) throws SecureMessagingException; + ResponseApdu verifyAndDecrypt(ResponseApdu apdu) throws SecureMessagingException; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index dde5bc9ea..9501697e3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -47,7 +47,6 @@ import javax.crypto.Cipher; import javax.crypto.NoSuchPaddingException; import javax.crypto.spec.SecretKeySpec; import javax.smartcardio.CommandAPDU; -import javax.smartcardio.ResponseAPDU; import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException; import org.sufficientlysecure.keychain.util.Log; @@ -73,7 +72,6 @@ import java.util.List; * For the full specs, see http://g10code.com/docs/openpgp-card-2.0.pdf */ public class SecurityTokenConnection { - static final int APDU_SW_SUCCESS = 0x9000; private static final int APDU_SW1_RESPONSE_AVAILABLE = 0x61; // Fidesmo constants @@ -184,10 +182,10 @@ public class SecurityTokenConnection { // Connect on smartcard layer // Command APDU (page 51) for SELECT FILE command (page 29) CommandAPDU select = commandFactory.createSelectFileOpenPgpCommand(); - ResponseAPDU response = communicate(select); // activate connection + ResponseApdu response = communicate(select); // activate connection - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Initialization failed!", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Initialization failed!", response.getSw()); } mOpenPgpCapabilities = new OpenPgpCapabilities(getData(0x00, 0x6E)); @@ -221,10 +219,10 @@ public class SecurityTokenConnection { // Command APDU for RESET RETRY COUNTER command (page 33) CommandAPDU changePin = commandFactory.createResetPw1Command(newPin); - ResponseAPDU response = communicate(changePin); + ResponseApdu response = communicate(changePin); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to change PIN", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Failed to change PIN", response.getSw()); } } @@ -246,10 +244,10 @@ public class SecurityTokenConnection { byte[] pin = adminPin.toStringUnsafe().getBytes(); CommandAPDU changePin = commandFactory.createChangePw3Command(pin, newAdminPin); - ResponseAPDU response = communicate(changePin); + ResponseApdu response = communicate(changePin); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to change PIN", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Failed to change PIN", response.getSw()); } } @@ -326,10 +324,10 @@ public class SecurityTokenConnection { } CommandAPDU command = commandFactory.createDecipherCommand(data); - ResponseAPDU response = communicate(command); + ResponseApdu response = communicate(command); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Deciphering with Security token failed on receive", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Deciphering with Security token failed on receive", response.getSw()); } switch (mOpenPgpCapabilities.getFormatForKeyType(KeyType.ENCRYPT).keyFormatType()) { @@ -394,9 +392,9 @@ public class SecurityTokenConnection { private void verifyPinForSignature() throws IOException { byte[] pin = mPin.toStringUnsafe().getBytes(); - ResponseAPDU response = communicate(commandFactory.createVerifyPw1ForSignatureCommand(pin)); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Bad PIN!", response.getSW()); + ResponseApdu response = communicate(commandFactory.createVerifyPw1ForSignatureCommand(pin)); + if (!response.isSuccess()) { + throw new CardException("Bad PIN!", response.getSw()); } mPw1ValidatedForSignature = true; @@ -409,9 +407,9 @@ public class SecurityTokenConnection { byte[] pin = mPin.toStringUnsafe().getBytes(); // Command APDU for VERIFY command (page 32) - ResponseAPDU response = communicate(commandFactory.createVerifyPw1ForOtherCommand(pin)); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Bad PIN!", response.getSW()); + ResponseApdu response = communicate(commandFactory.createVerifyPw1ForOtherCommand(pin)); + if (!response.isSuccess()) { + throw new CardException("Bad PIN!", response.getSw()); } mPw1ValidatedForDecrypt = true; @@ -422,10 +420,10 @@ public class SecurityTokenConnection { */ private void verifyAdminPin(Passphrase adminPin) throws IOException { // Command APDU for VERIFY command (page 32) - ResponseAPDU response = + ResponseApdu response = communicate(commandFactory.createVerifyPw3Command(adminPin.toStringUnsafe().getBytes())); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Bad PIN!", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Bad PIN!", response.getSw()); } mPw3Validated = true; @@ -453,10 +451,10 @@ public class SecurityTokenConnection { } CommandAPDU command = commandFactory.createPutDataCommand(dataObject, data); - ResponseAPDU response = communicate(command); // put data + ResponseApdu response = communicate(command); // put data - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to put data.", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Failed to put data.", response.getSw()); } } @@ -552,10 +550,10 @@ public class SecurityTokenConnection { } CommandAPDU apdu = commandFactory.createPutKeyCommand(keyBytes); - ResponseAPDU response = communicate(apdu); + ResponseApdu response = communicate(apdu); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Key export to Security Token failed", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Key export to Security Token failed", response.getSw()); } } @@ -567,10 +565,10 @@ public class SecurityTokenConnection { */ public byte[] getFingerprints() throws IOException { CommandAPDU apdu = commandFactory.createGetDataCommand(0x00, 0x6E); - ResponseAPDU response = communicate(apdu); + ResponseApdu response = communicate(apdu); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to get fingerprints", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Failed to get fingerprints", response.getSw()); } Iso7816TLV[] tlvList = Iso7816TLV.readList(response.getData(), true); @@ -614,9 +612,9 @@ public class SecurityTokenConnection { } private byte[] getData(int p1, int p2) throws IOException { - ResponseAPDU response = communicate(commandFactory.createGetDataCommand(p1, p2)); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to get pw status bytes", response.getSW()); + ResponseApdu response = communicate(commandFactory.createGetDataCommand(p1, p2)); + if (!response.isSuccess()) { + throw new CardException("Failed to get pw status bytes", response.getSw()); } return response.getData(); } @@ -697,10 +695,10 @@ public class SecurityTokenConnection { // Command APDU for PERFORM SECURITY OPERATION: COMPUTE DIGITAL SIGNATURE (page 37) CommandAPDU command = commandFactory.createComputeDigitalSignatureCommand(data); - ResponseAPDU response = communicate(command); + ResponseApdu response = communicate(command); - if (response.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Failed to sign", response.getSW()); + if (!response.isSuccess()) { + throw new CardException("Failed to sign", response.getSw()); } if (!mOpenPgpCapabilities.isPw1ValidForMultipleSignatures()) { @@ -750,7 +748,7 @@ public class SecurityTokenConnection { * @return response from the card * @throws IOException */ - ResponseAPDU communicate(CommandAPDU apdu) throws IOException { + ResponseApdu communicate(CommandAPDU apdu) throws IOException { if ((mSecureMessaging != null) && mSecureMessaging.isEstablished()) { try { apdu = mSecureMessaging.encryptAndSign(apdu); @@ -760,7 +758,7 @@ public class SecurityTokenConnection { } } - ResponseAPDU lastResponse = null; + ResponseApdu lastResponse = null; // Transmit if (mCardCapabilities.hasExtended()) { lastResponse = mTransport.transceive(apdu); @@ -774,8 +772,8 @@ public class SecurityTokenConnection { lastResponse = mTransport.transceive(chainedApdu); boolean isLastCommand = i < totalCommands - 1; - if (isLastCommand && lastResponse.getSW() != APDU_SW_SUCCESS) { - throw new UsbTransportException("Failed to chain apdu (last SW: " + lastResponse.getSW() + ")"); + if (isLastCommand && !lastResponse.isSuccess()) { + throw new UsbTransportException("Failed to chain apdu (last SW: " + lastResponse.getSw() + ")"); } } } @@ -787,17 +785,17 @@ public class SecurityTokenConnection { result.write(lastResponse.getData()); // Receive - while (lastResponse.getSW1() == APDU_SW1_RESPONSE_AVAILABLE) { + while (lastResponse.getSw1() == APDU_SW1_RESPONSE_AVAILABLE) { // GET RESPONSE ISO/IEC 7816-4 par.7.6.1 - CommandAPDU getResponse = commandFactory.createGetResponseCommand(lastResponse.getSW2()); + CommandAPDU getResponse = commandFactory.createGetResponseCommand(lastResponse.getSw2()); lastResponse = mTransport.transceive(getResponse); result.write(lastResponse.getData()); } - result.write(lastResponse.getSW1()); - result.write(lastResponse.getSW2()); + result.write(lastResponse.getSw1()); + result.write(lastResponse.getSw2()); - lastResponse = new ResponseAPDU(result.toByteArray()); + lastResponse = ResponseApdu.fromBytes(result.toByteArray()); if ((mSecureMessaging != null) && mSecureMessaging.isEstablished()) { try { @@ -817,7 +815,7 @@ public class SecurityTokenConnection { // By trying to select any apps that have the Fidesmo AID prefix we can // see if it is a Fidesmo device or not CommandAPDU apdu = commandFactory.createSelectFileCommand(FIDESMO_APPS_AID_PREFIX); - return communicate(apdu).getSW() == APDU_SW_SUCCESS; + return communicate(apdu).isSuccess(); } catch (IOException e) { Log.e(Constants.TAG, "Card communication failed!", e); } @@ -849,9 +847,9 @@ public class SecurityTokenConnection { } CommandAPDU apdu = commandFactory.createGenerateKeyCommand(slot); - ResponseAPDU response = communicate(apdu); + ResponseApdu response = communicate(apdu); - if (response.getSW() != APDU_SW_SUCCESS) { + if (!response.isSuccess()) { throw new IOException("On-card key generation failed"); } @@ -868,9 +866,9 @@ public class SecurityTokenConnection { byte[] pin = "XXXXXX".getBytes(); for (int i = 0; i <= 4; i++) { // Command APDU for VERIFY command (page 32) - ResponseAPDU response = communicate(commandFactory.createVerifyPw1ForSignatureCommand(pin)); - if (response.getSW() == APDU_SW_SUCCESS) { - throw new CardException("Should never happen, XXXXXX has been accepted!", response.getSW()); + ResponseApdu response = communicate(commandFactory.createVerifyPw1ForSignatureCommand(pin)); + if (response.isSuccess()) { + throw new CardException("Should never happen, XXXXXX has been accepted!", response.getSw()); } } @@ -878,9 +876,9 @@ public class SecurityTokenConnection { byte[] adminPin = "XXXXXXXX".getBytes(); for (int i = 0; i <= 4; i++) { // Command APDU for VERIFY command (page 32) - ResponseAPDU response = communicate(commandFactory.createVerifyPw3Command(adminPin)); - if (response.getSW() == APDU_SW_SUCCESS) { // Should NOT accept! - throw new CardException("Should never happen, XXXXXXXX has been accepted", response.getSW()); + ResponseApdu response = communicate(commandFactory.createVerifyPw3Command(adminPin)); + if (response.isSuccess()) { // Should NOT accept! + throw new CardException("Should never happen, XXXXXXXX has been accepted", response.getSw()); } } @@ -892,13 +890,13 @@ public class SecurityTokenConnection { // If a token is in a bad state and reactivate1 fails, it could still be reactivated with reactivate2 CommandAPDU reactivate1 = commandFactory.createReactivate1Command(); CommandAPDU reactivate2 = commandFactory.createReactivate2Command(); - ResponseAPDU response1 = communicate(reactivate1); - ResponseAPDU response2 = communicate(reactivate2); - if (response1.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Reactivating failed!", response1.getSW()); + ResponseApdu response1 = communicate(reactivate1); + ResponseApdu response2 = communicate(reactivate2); + if (!response1.isSuccess()) { + throw new CardException("Reactivating failed!", response1.getSw()); } - if (response2.getSW() != APDU_SW_SUCCESS) { - throw new CardException("Reactivating failed!", response2.getSW()); + if (!response2.isSuccess()) { + throw new CardException("Reactivating failed!", response2.getSw()); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java index 00fb4db07..c336b4264 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java @@ -18,7 +18,6 @@ package org.sufficientlysecure.keychain.securitytoken; import javax.smartcardio.CommandAPDU; -import javax.smartcardio.ResponseAPDU; import java.io.IOException; @@ -32,7 +31,7 @@ public interface Transport { * @return received data * @throws IOException */ - ResponseAPDU transceive(CommandAPDU data) throws IOException; + ResponseApdu transceive(CommandAPDU data) throws IOException; /** * Disconnect and release connection 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 cd6412560..a37bff476 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 @@ -30,7 +30,7 @@ import android.util.Pair; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.securitytoken.Transport; import javax.smartcardio.CommandAPDU; -import javax.smartcardio.ResponseAPDU; +import org.sufficientlysecure.keychain.securitytoken.ResponseApdu; import org.sufficientlysecure.keychain.securitytoken.usb.tpdu.T1ShortApduProtocol; import org.sufficientlysecure.keychain.securitytoken.usb.tpdu.T1TpduProtocol; import org.sufficientlysecure.keychain.util.Log; @@ -183,8 +183,8 @@ public class UsbTransport implements Transport { * @return received data */ @Override - public ResponseAPDU transceive(CommandAPDU data) throws UsbTransportException { - return new ResponseAPDU(ccidTransportProtocol.transceive(data.getBytes())); + public ResponseApdu transceive(CommandAPDU data) throws UsbTransportException { + return ResponseApdu.fromBytes(ccidTransportProtocol.transceive(data.getBytes())); } @Override From c89aab88022aea20d0aa7163986936f264ce8134 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 17:04:44 +0200 Subject: [PATCH 08/13] reduce visibility where unnecessary --- .../securitytoken/CardCapabilities.java | 3 +- .../keychain/securitytoken/KeyFormat.java | 8 ++--- .../securitytoken/OpenPgpCapabilities.java | 29 ++++++++++--------- .../keychain/securitytoken/ResponseApdu.java | 1 + .../securitytoken/SCP11bSecureMessaging.java | 4 +-- .../securitytoken/SecurityTokenUtils.java | 18 ++++++------ 6 files changed, 33 insertions(+), 30 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CardCapabilities.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CardCapabilities.java index d6600ab01..dc8a65e6d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CardCapabilities.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CardCapabilities.java @@ -21,7 +21,8 @@ import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException; import java.nio.ByteBuffer; -public class CardCapabilities { +@SuppressWarnings("WeakerAccess") +class CardCapabilities { private static final int MASK_CHAINING = 1 << 7; private static final int MASK_EXTENDED = 1 << 6; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java index 9d612517b..aea814cc4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/KeyFormat.java @@ -25,18 +25,18 @@ import org.sufficientlysecure.keychain.ui.CreateSecurityTokenAlgorithmFragment; public abstract class KeyFormat { - public enum KeyFormatType { + enum KeyFormatType { RSAKeyFormatType, ECKeyFormatType - }; + } private final KeyFormatType mKeyFormatType; - public KeyFormat(final KeyFormatType keyFormatType) { + KeyFormat(final KeyFormatType keyFormatType) { mKeyFormatType = keyFormatType; } - public final KeyFormatType keyFormatType() { + final KeyFormatType keyFormatType() { return mKeyFormatType; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java index 082ae90ec..2edc03c0d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java @@ -21,7 +21,8 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; -public class OpenPgpCapabilities { +@SuppressWarnings("unused") // just expose all included data +class OpenPgpCapabilities { private final static int MASK_SM = 1 << 7; private final static int MASK_KEY_IMPORT = 1 << 5; private final static int MASK_ATTRIBUTES_CHANGABLE = 1 << 2; @@ -40,12 +41,12 @@ public class OpenPgpCapabilities { private Map mKeyFormats; - public OpenPgpCapabilities(byte[] data) throws IOException { + OpenPgpCapabilities(byte[] data) throws IOException { mKeyFormats = new HashMap<>(); updateWithData(data); } - public void updateWithData(byte[] data) throws IOException { + void updateWithData(byte[] data) throws IOException { Iso7816TLV[] tlvs = Iso7816TLV.readList(data, true); if (tlvs.length == 1 && tlvs[0].mT == 0x6E) { tlvs = ((Iso7816TLV.Iso7816CompositeTLV) tlvs[0]).mSubs; @@ -114,47 +115,47 @@ public class OpenPgpCapabilities { mMaxRspLen = (v[8] << 8) + v[9]; } - public boolean isPw1ValidForMultipleSignatures() { + boolean isPw1ValidForMultipleSignatures() { return mPw1ValidForMultipleSignatures; } - public byte[] getAid() { + byte[] getAid() { return mAid; } - public byte[] getHistoricalBytes() { + byte[] getHistoricalBytes() { return mHistoricalBytes; } - public boolean isHasSM() { + boolean isHasSM() { return mHasSM; } - public boolean isAttributesChangable() { + boolean isAttributesChangable() { return mAttriburesChangable; } - public boolean isHasKeyImport() { + boolean isHasKeyImport() { return mHasKeyImport; } - public boolean isHasAESSM() { + boolean isHasAESSM() { return isHasSM() && ((mSMType == 1) || (mSMType == 2)); } - public boolean isHasSCP11bSM() { + boolean isHasSCP11bSM() { return isHasSM() && (mSMType == 3); } - public int getMaxCmdLen() { + int getMaxCmdLen() { return mMaxCmdLen; } - public int getMaxRspLen() { + int getMaxRspLen() { return mMaxRspLen; } - public KeyFormat getFormatForKeyType(KeyType keyType) { + KeyFormat getFormatForKeyType(KeyType keyType) { return mKeyFormats.get(keyType); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java index 0ace27b71..b3eb85743 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/ResponseApdu.java @@ -24,6 +24,7 @@ import com.google.auto.value.AutoValue; /** A response APDU as defined in ISO/IEC 7816-4. */ @AutoValue +@SuppressWarnings("WeakerAccess") public abstract class ResponseApdu { private static final int APDU_SW_SUCCESS = 0x9000; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index a135caae9..fb7b3e5ec 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -150,7 +150,7 @@ class SCP11bSecureMessaging implements SecureMessaging { && (mMacChaining != null); } - private static final ECParameterSpec getAlgorithmParameterSpec(final ECKeyFormat kf) + private static ECParameterSpec getAlgorithmParameterSpec(final ECKeyFormat kf) throws NoSuchProviderException, NoSuchAlgorithmException, InvalidParameterSpecException { final AlgorithmParameters algoParams = AlgorithmParameters.getInstance(SCP11B_KEY_AGREEMENT_KEY_ALGO, PROVIDER); @@ -273,7 +273,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } - public static void establish(final SecurityTokenConnection t, final Context ctx, OpenPgpCommandApduFactory commandFactory) + static void establish(final SecurityTokenConnection t, final Context ctx, OpenPgpCommandApduFactory commandFactory) throws SecureMessagingException, IOException { CommandAPDU cmd; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenUtils.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenUtils.java index d8077c615..31df10576 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenUtils.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenUtils.java @@ -33,8 +33,8 @@ import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; import java.security.interfaces.RSAPrivateCrtKey; -public class SecurityTokenUtils { - public static byte[] attributesFromSecretKey(final KeyType slot, final CanonicalizedSecretKey secretKey) throws IOException, PgpGeneralException { +class SecurityTokenUtils { + static byte[] attributesFromSecretKey(final KeyType slot, final CanonicalizedSecretKey secretKey) throws IOException, PgpGeneralException { if (secretKey.isRSA()) { final int mModulusLength = secretKey.getBitStrength(); final int mExponentLength = secretKey.getSecurityTokenRSASecretKey().getPublicExponent().bitLength(); @@ -46,7 +46,7 @@ public class SecurityTokenUtils { attrs[i++] = (byte) (mModulusLength & 0xff); attrs[i++] = (byte) ((mExponentLength >> 8) & 0xff); attrs[i++] = (byte) (mExponentLength & 0xff); - attrs[i++] = RSAKeyFormat.RSAAlgorithmFormat.CRT_WITH_MODULUS.getValue(); + attrs[i] = RSAKeyFormat.RSAAlgorithmFormat.CRT_WITH_MODULUS.getValue(); return attrs; } else if (secretKey.isEC()) { @@ -70,8 +70,8 @@ public class SecurityTokenUtils { } - public static byte[] createRSAPrivKeyTemplate(RSAPrivateCrtKey secretKey, KeyType slot, - RSAKeyFormat format) throws IOException { + static byte[] createRSAPrivKeyTemplate(RSAPrivateCrtKey secretKey, KeyType slot, + RSAKeyFormat format) throws IOException { ByteArrayOutputStream stream = new ByteArrayOutputStream(), template = new ByteArrayOutputStream(), data = new ByteArrayOutputStream(), @@ -138,8 +138,8 @@ public class SecurityTokenUtils { return res.toByteArray(); } - public static byte[] createECPrivKeyTemplate(ECPrivateKey secretKey, ECPublicKey publicKey, KeyType slot, - ECKeyFormat format) throws IOException { + static byte[] createECPrivKeyTemplate(ECPrivateKey secretKey, ECPublicKey publicKey, KeyType slot, + ECKeyFormat format) throws IOException { ByteArrayOutputStream stream = new ByteArrayOutputStream(), template = new ByteArrayOutputStream(), data = new ByteArrayOutputStream(), @@ -184,7 +184,7 @@ public class SecurityTokenUtils { return res.toByteArray(); } - public static byte[] encodeLength(int len) { + static byte[] encodeLength(int len) { if (len < 0) { throw new IllegalArgumentException("length is negative"); } else if (len >= 16777216) { @@ -214,7 +214,7 @@ public class SecurityTokenUtils { return res; } - public static void writeBits(ByteArrayOutputStream stream, BigInteger value, int width) { + static void writeBits(ByteArrayOutputStream stream, BigInteger value, int width) { if (value.signum() == -1) { throw new IllegalArgumentException("value is negative"); } else if (width <= 0) { From 911d2a1c96dd7e874dc19152e50c81aca8c24a50 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 18:22:04 +0200 Subject: [PATCH 09/13] rewrite CommandApdu --- .../java/javax/smartcardio/CommandAPDU.java | 608 ------------------ .../keychain/securitytoken/CommandApdu.java | 245 +++++++ .../keychain/securitytoken/NfcTransport.java | 5 +- .../OpenPgpCommandApduFactory.java | 93 ++- .../securitytoken/SCP11bSecureMessaging.java | 7 +- .../securitytoken/SecureMessaging.java | 4 +- .../SecurityTokenConnection.java | 35 +- .../keychain/securitytoken/Transport.java | 4 +- .../securitytoken/usb/UsbTransport.java | 6 +- 9 files changed, 318 insertions(+), 689 deletions(-) delete mode 100644 OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandApdu.java diff --git a/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java b/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java deleted file mode 100644 index 30e8331cd..000000000 --- a/OpenKeychain/src/main/java/javax/smartcardio/CommandAPDU.java +++ /dev/null @@ -1,608 +0,0 @@ -/* - * Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code 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 - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package javax.smartcardio; - -import java.util.Arrays; - -import java.nio.ByteBuffer; - -import org.sufficientlysecure.keychain.securitytoken.ResponseApdu; - - -/** - * A command APDU following the structure defined in ISO/IEC 7816-4. - * It consists of a four byte header and a conditional body of variable length. - * This class does not attempt to verify that the APDU encodes a semantically - * valid command. - * - *

Note that when the expected length of the response APDU is specified - * in the {@linkplain #CommandAPDU(int,int,int,int,int) constructors}, - * the actual length (Ne) must be specified, not its - * encoded form (Le). Similarly, {@linkplain #getNe} returns the actual - * value Ne. In other words, a value of 0 means "no data in the response APDU" - * rather than "maximum length." - * - *

This class supports both the short and extended forms of length - * encoding for Ne and Nc. However, note that not all terminals and Smart Cards - * are capable of accepting APDUs that use the extended form. - * - *

For the header bytes CLA, INS, P1, and P2 the Java type int - * is used to represent the 8 bit unsigned values. In the constructors, only - * the 8 lowest bits of the int value specified by the application - * are significant. The accessor methods always return the byte as an unsigned - * value between 0 and 255. - * - *

Instances of this class are immutable. Where data is passed in or out - * via byte arrays, defensive cloning is performed. - * - * @see ResponseApdu - * - * @since 1.6 - * @author Andreas Sterbenz - * @author JSR 268 Expert Group - */ -public final class CommandAPDU implements java.io.Serializable { - - private static final long serialVersionUID = 398698301286670877L; - - private static final int MAX_APDU_SIZE = 65544; - - /** @serial */ - private byte[] apdu; - - // value of nc - private transient int nc; - - // value of ne - private transient int ne; - - // index of start of data within the apdu array - private transient int dataOffset; - - /** - * Constructs a CommandAPDU from a byte array containing the complete - * APDU contents (header and body). - * - *

Note that the apdu bytes are copied to protect against - * subsequent modification. - * - * @param apdu the complete command APDU - * - * @throws NullPointerException if apdu is null - * @throws IllegalArgumentException if apdu does not contain a valid - * command APDU - */ - public CommandAPDU(byte[] apdu) { - this.apdu = apdu.clone(); - parse(); - } - - /** - * Constructs a CommandAPDU from a byte array containing the complete - * APDU contents (header and body). The APDU starts at the index - * apduOffset in the byte array and is apduLength - * bytes long. - * - *

Note that the apdu bytes are copied to protect against - * subsequent modification. - * - * @param apdu the complete command APDU - * @param apduOffset the offset in the byte array at which the apdu - * data begins - * @param apduLength the length of the APDU - * - * @throws NullPointerException if apdu is null - * @throws IllegalArgumentException if apduOffset or apduLength are - * negative or if apduOffset + apduLength are greater than apdu.length, - * or if the specified bytes are not a valid APDU - */ - public CommandAPDU(byte[] apdu, int apduOffset, int apduLength) { - checkArrayBounds(apdu, apduOffset, apduLength); - this.apdu = new byte[apduLength]; - System.arraycopy(apdu, apduOffset, this.apdu, 0, apduLength); - parse(); - } - - private void checkArrayBounds(byte[] b, int ofs, int len) { - if ((ofs < 0) || (len < 0)) { - throw new IllegalArgumentException - ("Offset and length must not be negative"); - } - if (b == null) { - if ((ofs != 0) && (len != 0)) { - throw new IllegalArgumentException - ("offset and length must be 0 if array is null"); - } - } else { - if (ofs > b.length - len) { - throw new IllegalArgumentException - ("Offset plus length exceed array size"); - } - } - } - - /** - * Creates a CommandAPDU from the ByteBuffer containing the complete APDU - * contents (header and body). - * The buffer's position must be set to the start of the APDU, - * its limit to the end of the APDU. Upon return, the buffer's - * position is equal to its limit; its limit remains unchanged. - * - *

Note that the data in the ByteBuffer is copied to protect against - * subsequent modification. - * - * @param apdu the ByteBuffer containing the complete APDU - * - * @throws NullPointerException if apdu is null - * @throws IllegalArgumentException if apdu does not contain a valid - * command APDU - */ - public CommandAPDU(ByteBuffer apdu) { - this.apdu = new byte[apdu.remaining()]; - apdu.get(this.apdu); - parse(); - } - - /** - * Constructs a CommandAPDU from the four header bytes. This is case 1 - * in ISO 7816, no command body. - * - * @param cla the class byte CLA - * @param ins the instruction byte INS - * @param p1 the parameter byte P1 - * @param p2 the parameter byte P2 - */ - public CommandAPDU(int cla, int ins, int p1, int p2) { - this(cla, ins, p1, p2, null, 0, 0, 0); - } - - /** - * Constructs a CommandAPDU from the four header bytes and the expected - * response data length. This is case 2 in ISO 7816, empty command data - * field with Ne specified. If Ne is 0, the APDU is encoded as ISO 7816 - * case 1. - * - * @param cla the class byte CLA - * @param ins the instruction byte INS - * @param p1 the parameter byte P1 - * @param p2 the parameter byte P2 - * @param ne the maximum number of expected data bytes in a response APDU - * - * @throws IllegalArgumentException if ne is negative or greater than - * 65536 - */ - public CommandAPDU(int cla, int ins, int p1, int p2, int ne) { - this(cla, ins, p1, p2, null, 0, 0, ne); - } - - /** - * Constructs a CommandAPDU from the four header bytes and command data. - * This is case 3 in ISO 7816, command data present and Ne absent. The - * value Nc is taken as data.length. If data is null or - * its length is 0, the APDU is encoded as ISO 7816 case 1. - * - *

Note that the data bytes are copied to protect against - * subsequent modification. - * - * @param cla the class byte CLA - * @param ins the instruction byte INS - * @param p1 the parameter byte P1 - * @param p2 the parameter byte P2 - * @param data the byte array containing the data bytes of the command body - * - * @throws IllegalArgumentException if data.length is greater than 65535 - */ - public CommandAPDU(int cla, int ins, int p1, int p2, byte[] data) { - this(cla, ins, p1, p2, data, 0, arrayLength(data), 0); - } - - /** - * Constructs a CommandAPDU from the four header bytes and command data. - * This is case 3 in ISO 7816, command data present and Ne absent. The - * value Nc is taken as dataLength. If dataLength - * is 0, the APDU is encoded as ISO 7816 case 1. - * - *

Note that the data bytes are copied to protect against - * subsequent modification. - * - * @param cla the class byte CLA - * @param ins the instruction byte INS - * @param p1 the parameter byte P1 - * @param p2 the parameter byte P2 - * @param data the byte array containing the data bytes of the command body - * @param dataOffset the offset in the byte array at which the data - * bytes of the command body begin - * @param dataLength the number of the data bytes in the command body - * - * @throws NullPointerException if data is null and dataLength is not 0 - * @throws IllegalArgumentException if dataOffset or dataLength are - * negative or if dataOffset + dataLength are greater than data.length - * or if dataLength is greater than 65535 - */ - public CommandAPDU(int cla, int ins, int p1, int p2, byte[] data, - int dataOffset, int dataLength) { - this(cla, ins, p1, p2, data, dataOffset, dataLength, 0); - } - - /** - * Constructs a CommandAPDU from the four header bytes, command data, - * and expected response data length. This is case 4 in ISO 7816, - * command data and Ne present. The value Nc is taken as data.length - * if data is non-null and as 0 otherwise. If Ne or Nc - * are zero, the APDU is encoded as case 1, 2, or 3 per ISO 7816. - * - *

Note that the data bytes are copied to protect against - * subsequent modification. - * - * @param cla the class byte CLA - * @param ins the instruction byte INS - * @param p1 the parameter byte P1 - * @param p2 the parameter byte P2 - * @param data the byte array containing the data bytes of the command body - * @param ne the maximum number of expected data bytes in a response APDU - * - * @throws IllegalArgumentException if data.length is greater than 65535 - * or if ne is negative or greater than 65536 - */ - public CommandAPDU(int cla, int ins, int p1, int p2, byte[] data, int ne) { - this(cla, ins, p1, p2, data, 0, arrayLength(data), ne); - } - - private static int arrayLength(byte[] b) { - return (b != null) ? b.length : 0; - } - - /** - * Command APDU encoding options: - * - * case 1: |CLA|INS|P1 |P2 | len = 4 - * case 2s: |CLA|INS|P1 |P2 |LE | len = 5 - * case 3s: |CLA|INS|P1 |P2 |LC |...BODY...| len = 6..260 - * case 4s: |CLA|INS|P1 |P2 |LC |...BODY...|LE | len = 7..261 - * case 2e: |CLA|INS|P1 |P2 |00 |LE1|LE2| len = 7 - * case 3e: |CLA|INS|P1 |P2 |00 |LC1|LC2|...BODY...| len = 8..65542 - * case 4e: |CLA|INS|P1 |P2 |00 |LC1|LC2|...BODY...|LE1|LE2| len =10..65544 - * - * LE, LE1, LE2 may be 0x00. - * LC must not be 0x00 and LC1|LC2 must not be 0x00|0x00 - */ - private void parse() { - if (apdu.length < 4) { - throw new IllegalArgumentException("apdu must be at least 4 bytes long"); - } - if (apdu.length == 4) { - // case 1 - return; - } - int l1 = apdu[4] & 0xff; - if (apdu.length == 5) { - // case 2s - this.ne = (l1 == 0) ? 256 : l1; - return; - } - if (l1 != 0) { - if (apdu.length == 4 + 1 + l1) { - // case 3s - this.nc = l1; - this.dataOffset = 5; - return; - } else if (apdu.length == 4 + 2 + l1) { - // case 4s - this.nc = l1; - this.dataOffset = 5; - int l2 = apdu[apdu.length - 1] & 0xff; - this.ne = (l2 == 0) ? 256 : l2; - return; - } else { - throw new IllegalArgumentException - ("Invalid APDU: length=" + apdu.length + ", b1=" + l1); - } - } - if (apdu.length < 7) { - throw new IllegalArgumentException - ("Invalid APDU: length=" + apdu.length + ", b1=" + l1); - } - int l2 = ((apdu[5] & 0xff) << 8) | (apdu[6] & 0xff); - if (apdu.length == 7) { - // case 2e - this.ne = (l2 == 0) ? 65536 : l2; - return; - } - if (l2 == 0) { - throw new IllegalArgumentException("Invalid APDU: length=" - + apdu.length + ", b1=" + l1 + ", b2||b3=" + l2); - } - if (apdu.length == 4 + 3 + l2) { - // case 3e - this.nc = l2; - this.dataOffset = 7; - return; - } else if (apdu.length == 4 + 5 + l2) { - // case 4e - this.nc = l2; - this.dataOffset = 7; - int leOfs = apdu.length - 2; - int l3 = ((apdu[leOfs] & 0xff) << 8) | (apdu[leOfs + 1] & 0xff); - this.ne = (l3 == 0) ? 65536 : l3; - } else { - throw new IllegalArgumentException("Invalid APDU: length=" - + apdu.length + ", b1=" + l1 + ", b2||b3=" + l2); - } - } - - /** - * Constructs a CommandAPDU from the four header bytes, command data, - * and expected response data length. This is case 4 in ISO 7816, - * command data and Le present. The value Nc is taken as - * dataLength. - * If Ne or Nc - * are zero, the APDU is encoded as case 1, 2, or 3 per ISO 7816. - * - *

Note that the data bytes are copied to protect against - * subsequent modification. - * - * @param cla the class byte CLA - * @param ins the instruction byte INS - * @param p1 the parameter byte P1 - * @param p2 the parameter byte P2 - * @param data the byte array containing the data bytes of the command body - * @param dataOffset the offset in the byte array at which the data - * bytes of the command body begin - * @param dataLength the number of the data bytes in the command body - * @param ne the maximum number of expected data bytes in a response APDU - * - * @throws NullPointerException if data is null and dataLength is not 0 - * @throws IllegalArgumentException if dataOffset or dataLength are - * negative or if dataOffset + dataLength are greater than data.length, - * or if ne is negative or greater than 65536, - * or if dataLength is greater than 65535 - */ - public CommandAPDU(int cla, int ins, int p1, int p2, byte[] data, - int dataOffset, int dataLength, int ne) { - checkArrayBounds(data, dataOffset, dataLength); - if (dataLength > 65535) { - throw new IllegalArgumentException("dataLength is too large"); - } - if (ne < 0) { - throw new IllegalArgumentException("ne must not be negative"); - } - if (ne > 65536) { - throw new IllegalArgumentException("ne is too large"); - } - this.ne = ne; - this.nc = dataLength; - if (dataLength == 0) { - if (ne == 0) { - // case 1 - this.apdu = new byte[4]; - setHeader(cla, ins, p1, p2); - } else { - // case 2s or 2e - if (ne <= 256) { - // case 2s - // 256 is encoded as 0x00 - byte len = (ne != 256) ? (byte)ne : 0; - this.apdu = new byte[5]; - setHeader(cla, ins, p1, p2); - this.apdu[4] = len; - } else { - // case 2e - byte l1, l2; - // 65536 is encoded as 0x00 0x00 - if (ne == 65536) { - l1 = 0; - l2 = 0; - } else { - l1 = (byte)(ne >> 8); - l2 = (byte)ne; - } - this.apdu = new byte[7]; - setHeader(cla, ins, p1, p2); - this.apdu[5] = l1; - this.apdu[6] = l2; - } - } - } else { - if (ne == 0) { - // case 3s or 3e - if (dataLength <= 255) { - // case 3s - apdu = new byte[4 + 1 + dataLength]; - setHeader(cla, ins, p1, p2); - apdu[4] = (byte)dataLength; - this.dataOffset = 5; - System.arraycopy(data, dataOffset, apdu, 5, dataLength); - } else { - // case 3e - apdu = new byte[4 + 3 + dataLength]; - setHeader(cla, ins, p1, p2); - apdu[4] = 0; - apdu[5] = (byte)(dataLength >> 8); - apdu[6] = (byte)dataLength; - this.dataOffset = 7; - System.arraycopy(data, dataOffset, apdu, 7, dataLength); - } - } else { - // case 4s or 4e - if ((dataLength <= 255) && (ne <= 256)) { - // case 4s - apdu = new byte[4 + 2 + dataLength]; - setHeader(cla, ins, p1, p2); - apdu[4] = (byte)dataLength; - this.dataOffset = 5; - System.arraycopy(data, dataOffset, apdu, 5, dataLength); - apdu[apdu.length - 1] = (ne != 256) ? (byte)ne : 0; - } else { - // case 4e - apdu = new byte[4 + 5 + dataLength]; - setHeader(cla, ins, p1, p2); - apdu[4] = 0; - apdu[5] = (byte)(dataLength >> 8); - apdu[6] = (byte)dataLength; - this.dataOffset = 7; - System.arraycopy(data, dataOffset, apdu, 7, dataLength); - if (ne != 65536) { - int leOfs = apdu.length - 2; - apdu[leOfs] = (byte)(ne >> 8); - apdu[leOfs + 1] = (byte)ne; - } // else le == 65536: no need to fill in, encoded as 0 - } - } - } - } - - private void setHeader(int cla, int ins, int p1, int p2) { - apdu[0] = (byte)cla; - apdu[1] = (byte)ins; - apdu[2] = (byte)p1; - apdu[3] = (byte)p2; - } - - /** - * Returns the value of the class byte CLA. - * - * @return the value of the class byte CLA. - */ - public int getCLA() { - return apdu[0] & 0xff; - } - - /** - * Returns the value of the instruction byte INS. - * - * @return the value of the instruction byte INS. - */ - public int getINS() { - return apdu[1] & 0xff; - } - - /** - * Returns the value of the parameter byte P1. - * - * @return the value of the parameter byte P1. - */ - public int getP1() { - return apdu[2] & 0xff; - } - - /** - * Returns the value of the parameter byte P2. - * - * @return the value of the parameter byte P2. - */ - public int getP2() { - return apdu[3] & 0xff; - } - - /** - * Returns the number of data bytes in the command body (Nc) or 0 if this - * APDU has no body. This call is equivalent to - * getData().length. - * - * @return the number of data bytes in the command body or 0 if this APDU - * has no body. - */ - public int getNc() { - return nc; - } - - /** - * Returns a copy of the data bytes in the command body. If this APDU as - * no body, this method returns a byte array with length zero. - * - * @return a copy of the data bytes in the command body or the empty - * byte array if this APDU has no body. - */ - public byte[] getData() { - byte[] data = new byte[nc]; - System.arraycopy(apdu, dataOffset, data, 0, nc); - return data; - } - - /** - * Returns the maximum number of expected data bytes in a response - * APDU (Ne). - * - * @return the maximum number of expected data bytes in a response APDU. - */ - public int getNe() { - return ne; - } - - /** - * Returns a copy of the bytes in this APDU. - * - * @return a copy of the bytes in this APDU. - */ - public byte[] getBytes() { - return apdu.clone(); - } - - /** - * Returns a string representation of this command APDU. - * - * @return a String representation of this command APDU. - */ - public String toString() { - return "CommmandAPDU: " + apdu.length + " bytes, nc=" + nc + ", ne=" + ne; - } - - /** - * Compares the specified object with this command APDU for equality. - * Returns true if the given object is also a CommandAPDU and its bytes are - * identical to the bytes in this CommandAPDU. - * - * @param obj the object to be compared for equality with this command APDU - * @return true if the specified object is equal to this command APDU - */ - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj instanceof CommandAPDU == false) { - return false; - } - CommandAPDU other = (CommandAPDU)obj; - return Arrays.equals(this.apdu, other.apdu); - } - - /** - * Returns the hash code value for this command APDU. - * - * @return the hash code value for this command APDU. - */ - public int hashCode() { - return Arrays.hashCode(apdu); - } - - private void readObject(java.io.ObjectInputStream in) - throws java.io.IOException, ClassNotFoundException { - apdu = (byte[])in.readUnshared(); - // initialize transient fields - parse(); - } - -} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandApdu.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandApdu.java new file mode 100644 index 000000000..9a6d91613 --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/CommandApdu.java @@ -0,0 +1,245 @@ +/* + * Copyright (C) 2016 Vincent Breitmoser + * + * 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 . + */ + +package org.sufficientlysecure.keychain.securitytoken; + + +import java.util.Arrays; + +import com.google.auto.value.AutoValue; + + +/** + * A command APDU following the structure defined in ISO/IEC 7816-4. + * It consists of a four byte header and a conditional body of variable length. + */ +@AutoValue +public abstract class CommandApdu { + public abstract int getCLA(); + public abstract int getINS(); + public abstract int getP1(); + public abstract int getP2(); + public abstract byte[] getData(); + public abstract int getNe(); + + public static CommandApdu create(byte[] apdu, int apduOffset, int apduLength) { + return fromBytes(Arrays.copyOfRange(apdu, apduOffset, apduOffset + apduLength)); + } + + public static CommandApdu create(int cla, int ins, int p1, int p2) { + return create(cla, ins, p1, p2, null, 0); + } + + public static CommandApdu create(int cla, int ins, int p1, int p2, int ne) { + return create(cla, ins, p1, p2, null, ne); + } + + public static CommandApdu create(int cla, int ins, int p1, int p2, byte[] data) { + return create(cla, ins, p1, p2, data, 0); + } + + public static CommandApdu create(int cla, int ins, int p1, int p2, byte[] data, int dataOffset, int dataLength) { + if (data != null) { + data = Arrays.copyOfRange(data, dataOffset, dataOffset + dataLength); + } + return create(cla, ins, p1, p2, data, 0); + } + + public static CommandApdu create(int cla, int ins, int p1, int p2, byte[] data, int dataOffset, int dataLength, + int ne) { + if (data != null) { + data = Arrays.copyOfRange(data, dataOffset, dataOffset + dataLength); + } + return create(cla, ins, p1, p2, data, ne); + } + + public static CommandApdu create(int cla, int ins, int p1, int p2, byte[] data, int ne) { + if (ne < 0) { + throw new IllegalArgumentException("ne must not be negative"); + } + if (ne > 65536) { + throw new IllegalArgumentException("ne is too large"); + } + + if (data == null) { + data = new byte[0]; + } + return new AutoValue_CommandApdu(cla, ins, p1, p2, data, ne); + } + + public static CommandApdu fromBytes(byte[] apdu, int offset, int length) { + return fromBytes(Arrays.copyOfRange(apdu, offset, offset + length)); + } + + /** + * Command APDU encoding options: + *

+ * case 1: |CLA|INS|P1 |P2 | len = 4 + * case 2s: |CLA|INS|P1 |P2 |LE | len = 5 + * case 3s: |CLA|INS|P1 |P2 |LC |...BODY...| len = 6..260 + * case 4s: |CLA|INS|P1 |P2 |LC |...BODY...|LE | len = 7..261 + * case 2e: |CLA|INS|P1 |P2 |00 |LE1|LE2| len = 7 + * case 3e: |CLA|INS|P1 |P2 |00 |LC1|LC2|...BODY...| len = 8..65542 + * case 4e: |CLA|INS|P1 |P2 |00 |LC1|LC2|...BODY...|LE1|LE2| len =10..65544 + *

+ * LE, LE1, LE2 may be 0x00. + * LC must not be 0x00 and LC1|LC2 must not be 0x00|0x00 + */ + public static CommandApdu fromBytes(byte[] apdu) { + if (apdu.length < 4) { + throw new IllegalArgumentException("apdu must be at least 4 bytes long"); + } + + int cla = apdu[0] & 0xff; + int ins = apdu[1] & 0xff; + int p1 = apdu[2] & 0xff; + int p2 = apdu[3] & 0xff; + final Integer dataOffset; + final Integer dataLength; + final int ne; + + if (apdu.length == 4) { + // case 1 + dataOffset = null; + dataLength = null; + ne = 0; + } else if (apdu.length == 5) { + // case 2s + dataOffset = null; + dataLength = null; + ne = (apdu[4] == 0) ? 256 : (apdu[4] & 0xff); + } else if (apdu[4] != 0) { + dataOffset = 5; + dataLength = apdu[4] & 0xff; + + if (apdu.length == 4 + 1 + dataLength) { + // case 3s + ne = 0; + } else { + // case 4s + int l2 = apdu[apdu.length - 1] & 0xff; + ne = (l2 == 0) ? 256 : l2; + } + } else { + int l2 = ((apdu[5] & 0xff) << 8) | (apdu[6] & 0xff); + if (apdu.length == 7) { + // case 2e + dataOffset = null; + dataLength = null; + ne = (l2 == 0) ? 65536 : l2; + } else { + dataOffset = 7; + dataLength = l2; + + if (apdu.length == 4 + 3 + l2) { + // case 3e + ne = 0; + } else { + // case 4e + int leOfs = apdu.length - 2; + int le = ((apdu[leOfs] & 0xff) << 8) | (apdu[leOfs + 1] & 0xff); + ne = (le == 0) ? 65536 : le; + } + } + } + + byte[] data; + if (dataOffset != null) { + data = Arrays.copyOfRange(apdu, dataOffset, dataOffset + dataLength); + } else { + data = new byte[0]; + } + + return new AutoValue_CommandApdu(cla, ins, p1, p2, data, ne); + } + + public byte[] toBytes() { + final byte[] apdu; + + byte[] data = getData(); + int ne = getNe(); + if (data.length == 0) { + if (ne == 0) { + // case 1 + apdu = new byte[4]; + } else { + // case 2s or 2e + if (ne <= 256) { + // case 2s + apdu = new byte[5]; + apdu[4] = (ne != 256) ? (byte) ne : 0; + } else { + // case 2e + apdu = new byte[7]; + if (ne != 65536) { + apdu[5] = (byte) (ne >> 8); + apdu[6] = (byte) ne; + } else { + apdu[5] = 0; + apdu[6] = 0; + } + } + } + } else { + if (ne == 0) { + // case 3s or 3e + if (data.length <= 255) { + // case 3s + apdu = new byte[4 + 1 + data.length]; + apdu[4] = (byte) data.length; + System.arraycopy(data, 0, apdu, 5, data.length); + } else { + // case 3e + apdu = new byte[4 + 3 + data.length]; + apdu[4] = 0; + apdu[5] = (byte) (data.length >> 8); + apdu[6] = (byte) data.length; + System.arraycopy(data, 0, apdu, 7, data.length); + } + } else { + if (data.length <= 255 && ne <= 256) { + // case 4s + apdu = new byte[4 + 2 + data.length]; + apdu[4] = (byte) data.length; + System.arraycopy(data, 0, apdu, 5, data.length); + apdu[apdu.length - 1] = (ne != 256) ? (byte) ne : 0; + } else { + // case 4e + apdu = new byte[4 + 5 + data.length]; + apdu[4] = 0; + apdu[5] = (byte) (data.length >> 8); + apdu[6] = (byte) data.length; + System.arraycopy(data, 0, apdu, 7, data.length); + if (ne != 65536) { + apdu[apdu.length - 2] = (byte) (ne >> 8); + apdu[apdu.length - 1] = (byte) ne; + } else { + apdu[apdu.length - 2] = 0; + apdu[apdu.length - 1] = 0; + } + } + } + } + + apdu[0] = (byte) getCLA(); + apdu[1] = (byte) getINS(); + apdu[2] = (byte) getP1(); + apdu[3] = (byte) getP2(); + + return apdu; + } +} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java index ac1c653ed..f004379bd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java @@ -19,7 +19,6 @@ package org.sufficientlysecure.keychain.securitytoken; import android.nfc.Tag; -import javax.smartcardio.CommandAPDU; import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity; import java.io.IOException; @@ -44,8 +43,8 @@ public class NfcTransport implements Transport { * @throws IOException */ @Override - public ResponseApdu transceive(final CommandAPDU data) throws IOException { - return ResponseApdu.fromBytes(mIsoCard.transceive(data.getBytes())); + public ResponseApdu transceive(final CommandApdu data) throws IOException { + return ResponseApdu.fromBytes(mIsoCard.transceive(data.toBytes())); } /** diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java index dfee53265..6d6b7e812 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java @@ -6,7 +6,6 @@ import java.util.List; import android.support.annotation.NonNull; -import javax.smartcardio.CommandAPDU; import org.bouncycastle.util.Arrays; import org.bouncycastle.util.encoders.Hex; @@ -75,126 +74,126 @@ class OpenPgpCommandApduFactory { private static final int P2_EMPTY = 0x00; @NonNull - CommandAPDU createPutDataCommand(int dataObject, byte[] data) { - return new CommandAPDU(CLA, INS_PUT_DATA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); + CommandApdu createPutDataCommand(int dataObject, byte[] data) { + return CommandApdu.create(CLA, INS_PUT_DATA, (dataObject & 0xFF00) >> 8, dataObject & 0xFF, data); } @NonNull - CommandAPDU createPutKeyCommand(byte[] keyBytes) { + CommandApdu createPutKeyCommand(byte[] keyBytes) { // the odd PUT DATA INS is for compliance with ISO 7816-8. This is used only to put key data on the card - return new CommandAPDU(CLA, INS_PUT_DATA_ODD, P1_PUT_DATA_ODD_KEY, P2_PUT_DATA_ODD_KEY, keyBytes); + return CommandApdu.create(CLA, INS_PUT_DATA_ODD, P1_PUT_DATA_ODD_KEY, P2_PUT_DATA_ODD_KEY, keyBytes); } @NonNull - CommandAPDU createComputeDigitalSignatureCommand(byte[] data) { - return new CommandAPDU(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_COMPUTE_DIGITAL_SIGNATURE, + CommandApdu createComputeDigitalSignatureCommand(byte[] data) { + return CommandApdu.create(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_COMPUTE_DIGITAL_SIGNATURE, P2_PSO_COMPUTE_DIGITAL_SIGNATURE, data, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createDecipherCommand(byte[] data) { - return new CommandAPDU(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data, + CommandApdu createDecipherCommand(byte[] data) { + return CommandApdu.create(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createChangePw3Command(byte[] adminPin, byte[] newAdminPin) { - return new CommandAPDU(CLA, INS_CHANGE_REFERENCE_DATA, P1_EMPTY, + CommandApdu createChangePw3Command(byte[] adminPin, byte[] newAdminPin) { + return CommandApdu.create(CLA, INS_CHANGE_REFERENCE_DATA, P1_EMPTY, P2_CHANGE_REFERENCE_DATA_PW3, Arrays.concatenate(adminPin, newAdminPin)); } @NonNull - CommandAPDU createResetPw1Command(byte[] newPin) { - return new CommandAPDU(CLA, INS_RESET_RETRY_COUNTER, P1_RESET_RETRY_COUNTER_NEW_PW, + CommandApdu createResetPw1Command(byte[] newPin) { + return CommandApdu.create(CLA, INS_RESET_RETRY_COUNTER, P1_RESET_RETRY_COUNTER_NEW_PW, P2_RESET_RETRY_COUNTER, newPin); } @NonNull - CommandAPDU createGetDataCommand(int p1, int p2) { - return new CommandAPDU(CLA, INS_GET_DATA, p1, p2, MAX_APDU_NE_EXT); + CommandApdu createGetDataCommand(int p1, int p2) { + return CommandApdu.create(CLA, INS_GET_DATA, p1, p2, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createGetResponseCommand(int lastResponseSw2) { - return new CommandAPDU(CLA, INS_GET_RESPONSE, P1_EMPTY, P2_EMPTY, lastResponseSw2); + CommandApdu createGetResponseCommand(int lastResponseSw2) { + return CommandApdu.create(CLA, INS_GET_RESPONSE, P1_EMPTY, P2_EMPTY, lastResponseSw2); } @NonNull - CommandAPDU createVerifyPw1ForSignatureCommand(byte[] pin) { - return new CommandAPDU(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW1_SIGN, pin); + CommandApdu createVerifyPw1ForSignatureCommand(byte[] pin) { + return CommandApdu.create(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW1_SIGN, pin); } @NonNull - CommandAPDU createVerifyPw1ForOtherCommand(byte[] pin) { - return new CommandAPDU(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW1_OTHER, pin); + CommandApdu createVerifyPw1ForOtherCommand(byte[] pin) { + return CommandApdu.create(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW1_OTHER, pin); } @NonNull - CommandAPDU createVerifyPw3Command(byte[] pin) { - return new CommandAPDU(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW3, pin); + CommandApdu createVerifyPw3Command(byte[] pin) { + return CommandApdu.create(CLA, INS_VERIFY, P1_EMPTY, P2_VERIFY_PW3, pin); } @NonNull - CommandAPDU createSelectFileOpenPgpCommand() { - return new CommandAPDU(CLA, INS_SELECT_FILE, P1_SELECT_FILE, P2_EMPTY, AID_SELECT_FILE_OPENPGP); + CommandApdu createSelectFileOpenPgpCommand() { + return CommandApdu.create(CLA, INS_SELECT_FILE, P1_SELECT_FILE, P2_EMPTY, AID_SELECT_FILE_OPENPGP); } @NonNull - CommandAPDU createSelectFileCommand(String fileAid) { - return new CommandAPDU(CLA, INS_SELECT_FILE, P1_SELECT_FILE, P2_EMPTY, Hex.decode(fileAid)); + CommandApdu createSelectFileCommand(String fileAid) { + return CommandApdu.create(CLA, INS_SELECT_FILE, P1_SELECT_FILE, P2_EMPTY, Hex.decode(fileAid)); } @NonNull - CommandAPDU createReactivate2Command() { - return new CommandAPDU(CLA, INS_ACTIVATE_FILE, P1_EMPTY, P2_EMPTY); + CommandApdu createReactivate2Command() { + return CommandApdu.create(CLA, INS_ACTIVATE_FILE, P1_EMPTY, P2_EMPTY); } @NonNull - CommandAPDU createReactivate1Command() { - return new CommandAPDU(CLA, INS_TERMINATE_DF, P1_EMPTY, P2_EMPTY); + CommandApdu createReactivate1Command() { + return CommandApdu.create(CLA, INS_TERMINATE_DF, P1_EMPTY, P2_EMPTY); } @NonNull - CommandAPDU createInternalAuthForSecureMessagingCommand(byte[] authData) { - return new CommandAPDU(CLA, INS_INTERNAL_AUTHENTICATE, P1_INTERNAL_AUTH_SECURE_MESSAGING, P2_EMPTY, authData, + CommandApdu createInternalAuthForSecureMessagingCommand(byte[] authData) { + return CommandApdu.create(CLA, INS_INTERNAL_AUTHENTICATE, P1_INTERNAL_AUTH_SECURE_MESSAGING, P2_EMPTY, authData, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createGenerateKeyCommand(int slot) { - return new CommandAPDU(CLA, INS_GENERATE_ASYMMETRIC_KEY_PAIR, + CommandApdu createGenerateKeyCommand(int slot) { + return CommandApdu.create(CLA, INS_GENERATE_ASYMMETRIC_KEY_PAIR, P1_GAKP_GENERATE, P2_EMPTY, new byte[] { (byte) slot, 0x00 }, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createRetrieveSecureMessagingPublicKeyCommand() { + CommandApdu createRetrieveSecureMessagingPublicKeyCommand() { // see https://github.com/ANSSI-FR/SmartPGP/blob/master/secure_messaging/smartpgp_sm.pdf - return new CommandAPDU(CLA, INS_GENERATE_ASYMMETRIC_KEY_PAIR, P1_GAKP_READ_PUBKEY_TEMPLATE, P2_EMPTY, + return CommandApdu.create(CLA, INS_GENERATE_ASYMMETRIC_KEY_PAIR, P1_GAKP_READ_PUBKEY_TEMPLATE, P2_EMPTY, CRT_GAKP_SECURE_MESSAGING, MAX_APDU_NE_EXT); } @NonNull - CommandAPDU createSelectSecureMessagingCertificateCommand() { + CommandApdu createSelectSecureMessagingCertificateCommand() { // see https://github.com/ANSSI-FR/SmartPGP/blob/master/secure_messaging/smartpgp_sm.pdf // this command selects the fourth occurence of data tag 7F21 - return new CommandAPDU(CLA, INS_SELECT_DATA, P1_SELECT_DATA_FOURTH, P2_SELECT_DATA, + return CommandApdu.create(CLA, INS_SELECT_DATA, P1_SELECT_DATA_FOURTH, P2_SELECT_DATA, CP_SELECT_DATA_CARD_HOLDER_CERT); } @NonNull - CommandAPDU createGetDataCardHolderCertCommand() { + CommandApdu createGetDataCardHolderCertCommand() { return createGetDataCommand(P1_GET_DATA_CARD_HOLDER_CERT, P2_GET_DATA_CARD_HOLDER_CERT); } @NonNull - CommandAPDU createShortApdu(CommandAPDU apdu) { + CommandApdu createShortApdu(CommandApdu apdu) { int ne = Math.min(apdu.getNe(), MAX_APDU_NE); - return new CommandAPDU(apdu.getCLA(), apdu.getINS(), apdu.getP1(), apdu.getP2(), apdu.getData(), ne); + return CommandApdu.create(apdu.getCLA(), apdu.getINS(), apdu.getP1(), apdu.getP2(), apdu.getData(), ne); } @NonNull - List createChainedApdus(CommandAPDU apdu) { - ArrayList result = new ArrayList<>(); + List createChainedApdus(CommandApdu apdu) { + ArrayList result = new ArrayList<>(); int offset = 0; byte[] data = apdu.getData(); @@ -204,8 +203,8 @@ class OpenPgpCommandApduFactory { boolean last = offset + curLen >= data.length; int cla = apdu.getCLA() + (last ? 0 : MASK_CLA_CHAINING); - CommandAPDU cmd = - new CommandAPDU(cla, apdu.getINS(), apdu.getP1(), apdu.getP2(), data, offset, curLen, ne); + CommandApdu cmd = + CommandApdu.create(cla, apdu.getINS(), apdu.getP1(), apdu.getP2(), data, offset, curLen, ne); result.add(cmd); offset += curLen; @@ -214,7 +213,7 @@ class OpenPgpCommandApduFactory { return result; } - boolean isSuitableForShortApdu(CommandAPDU apdu) { + boolean isSuitableForShortApdu(CommandApdu apdu) { return apdu.getData().length <= MAX_APDU_NC; } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java index fb7b3e5ec..eb4101907 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SCP11bSecureMessaging.java @@ -66,7 +66,6 @@ import javax.crypto.NoSuchPaddingException; import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; -import javax.smartcardio.CommandAPDU; import org.bouncycastle.asn1.nist.NISTNamedCurves; import org.bouncycastle.asn1.x9.ECNamedCurveTable; import org.bouncycastle.asn1.x9.X9ECParameters; @@ -276,7 +275,7 @@ class SCP11bSecureMessaging implements SecureMessaging { static void establish(final SecurityTokenConnection t, final Context ctx, OpenPgpCommandApduFactory commandFactory) throws SecureMessagingException, IOException { - CommandAPDU cmd; + CommandApdu cmd; ResponseApdu resp; Iso7816TLV[] tlvs; @@ -501,7 +500,7 @@ class SCP11bSecureMessaging implements SecureMessaging { @Override - public CommandAPDU encryptAndSign(CommandAPDU apdu) + public CommandApdu encryptAndSign(CommandApdu apdu) throws SecureMessagingException { if (!isEstablished()) { @@ -579,7 +578,7 @@ class SCP11bSecureMessaging implements SecureMessaging { } odata[ooff++] = (byte) 0; - apdu = new CommandAPDU(odata, 0, ooff); + apdu = CommandApdu.fromBytes(odata, 0, ooff); Arrays.fill(odata, (byte)0); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java index a43219b2a..11f53d138 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecureMessaging.java @@ -17,8 +17,6 @@ package org.sufficientlysecure.keychain.securitytoken; -import javax.smartcardio.CommandAPDU; - public interface SecureMessaging { @@ -26,7 +24,7 @@ public interface SecureMessaging { boolean isEstablished(); - CommandAPDU encryptAndSign(CommandAPDU apdu) throws SecureMessagingException; + CommandApdu encryptAndSign(CommandApdu apdu) throws SecureMessagingException; ResponseApdu verifyAndDecrypt(ResponseApdu apdu) throws SecureMessagingException; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 9501697e3..6b659c5f6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -46,7 +46,6 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import javax.crypto.Cipher; import javax.crypto.NoSuchPaddingException; import javax.crypto.spec.SecretKeySpec; -import javax.smartcardio.CommandAPDU; import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException; import org.sufficientlysecure.keychain.util.Log; @@ -181,7 +180,7 @@ public class SecurityTokenConnection { // Connect on smartcard layer // Command APDU (page 51) for SELECT FILE command (page 29) - CommandAPDU select = commandFactory.createSelectFileOpenPgpCommand(); + CommandApdu select = commandFactory.createSelectFileOpenPgpCommand(); ResponseApdu response = communicate(select); // activate connection if (!response.isSuccess()) { @@ -218,7 +217,7 @@ public class SecurityTokenConnection { } // Command APDU for RESET RETRY COUNTER command (page 33) - CommandAPDU changePin = commandFactory.createResetPw1Command(newPin); + CommandApdu changePin = commandFactory.createResetPw1Command(newPin); ResponseApdu response = communicate(changePin); if (!response.isSuccess()) { @@ -243,7 +242,7 @@ public class SecurityTokenConnection { byte[] pin = adminPin.toStringUnsafe().getBytes(); - CommandAPDU changePin = commandFactory.createChangePw3Command(pin, newAdminPin); + CommandApdu changePin = commandFactory.createChangePw3Command(pin, newAdminPin); ResponseApdu response = communicate(changePin); if (!response.isSuccess()) { @@ -323,7 +322,7 @@ public class SecurityTokenConnection { throw new CardException("Unknown encryption key type!"); } - CommandAPDU command = commandFactory.createDecipherCommand(data); + CommandApdu command = commandFactory.createDecipherCommand(data); ResponseApdu response = communicate(command); if (!response.isSuccess()) { @@ -450,7 +449,7 @@ public class SecurityTokenConnection { verifyAdminPin(adminPin); } - CommandAPDU command = commandFactory.createPutDataCommand(dataObject, data); + CommandApdu command = commandFactory.createPutDataCommand(dataObject, data); ResponseApdu response = communicate(command); // put data if (!response.isSuccess()) { @@ -549,7 +548,7 @@ public class SecurityTokenConnection { throw new IOException(e.getMessage()); } - CommandAPDU apdu = commandFactory.createPutKeyCommand(keyBytes); + CommandApdu apdu = commandFactory.createPutKeyCommand(keyBytes); ResponseApdu response = communicate(apdu); if (!response.isSuccess()) { @@ -564,7 +563,7 @@ public class SecurityTokenConnection { * @return The fingerprints of all subkeys in a contiguous byte array. */ public byte[] getFingerprints() throws IOException { - CommandAPDU apdu = commandFactory.createGetDataCommand(0x00, 0x6E); + CommandApdu apdu = commandFactory.createGetDataCommand(0x00, 0x6E); ResponseApdu response = communicate(apdu); if (!response.isSuccess()) { @@ -694,7 +693,7 @@ public class SecurityTokenConnection { } // Command APDU for PERFORM SECURITY OPERATION: COMPUTE DIGITAL SIGNATURE (page 37) - CommandAPDU command = commandFactory.createComputeDigitalSignatureCommand(data); + CommandApdu command = commandFactory.createComputeDigitalSignatureCommand(data); ResponseApdu response = communicate(command); if (!response.isSuccess()) { @@ -748,7 +747,7 @@ public class SecurityTokenConnection { * @return response from the card * @throws IOException */ - ResponseApdu communicate(CommandAPDU apdu) throws IOException { + ResponseApdu communicate(CommandApdu apdu) throws IOException { if ((mSecureMessaging != null) && mSecureMessaging.isEstablished()) { try { apdu = mSecureMessaging.encryptAndSign(apdu); @@ -763,12 +762,12 @@ public class SecurityTokenConnection { if (mCardCapabilities.hasExtended()) { lastResponse = mTransport.transceive(apdu); } else if (commandFactory.isSuitableForShortApdu(apdu)) { - CommandAPDU shortApdu = commandFactory.createShortApdu(apdu); + CommandApdu shortApdu = commandFactory.createShortApdu(apdu); lastResponse = mTransport.transceive(shortApdu); } else if (mCardCapabilities.hasChaining()) { - List chainedApdus = commandFactory.createChainedApdus(apdu); + List chainedApdus = commandFactory.createChainedApdus(apdu); for (int i = 0, totalCommands = chainedApdus.size(); i < totalCommands; i++) { - CommandAPDU chainedApdu = chainedApdus.get(i); + CommandApdu chainedApdu = chainedApdus.get(i); lastResponse = mTransport.transceive(chainedApdu); boolean isLastCommand = i < totalCommands - 1; @@ -787,7 +786,7 @@ public class SecurityTokenConnection { // Receive while (lastResponse.getSw1() == APDU_SW1_RESPONSE_AVAILABLE) { // GET RESPONSE ISO/IEC 7816-4 par.7.6.1 - CommandAPDU getResponse = commandFactory.createGetResponseCommand(lastResponse.getSw2()); + CommandApdu getResponse = commandFactory.createGetResponseCommand(lastResponse.getSw2()); lastResponse = mTransport.transceive(getResponse); result.write(lastResponse.getData()); } @@ -814,7 +813,7 @@ public class SecurityTokenConnection { try { // By trying to select any apps that have the Fidesmo AID prefix we can // see if it is a Fidesmo device or not - CommandAPDU apdu = commandFactory.createSelectFileCommand(FIDESMO_APPS_AID_PREFIX); + CommandApdu apdu = commandFactory.createSelectFileCommand(FIDESMO_APPS_AID_PREFIX); return communicate(apdu).isSuccess(); } catch (IOException e) { Log.e(Constants.TAG, "Card communication failed!", e); @@ -846,7 +845,7 @@ public class SecurityTokenConnection { verifyAdminPin(adminPin); } - CommandAPDU apdu = commandFactory.createGenerateKeyCommand(slot); + CommandApdu apdu = commandFactory.createGenerateKeyCommand(slot); ResponseApdu response = communicate(apdu); if (!response.isSuccess()) { @@ -888,8 +887,8 @@ public class SecurityTokenConnection { // reactivate token! // NOTE: keep the order here! First execute _both_ reactivate commands. Before checking _both_ responses // If a token is in a bad state and reactivate1 fails, it could still be reactivated with reactivate2 - CommandAPDU reactivate1 = commandFactory.createReactivate1Command(); - CommandAPDU reactivate2 = commandFactory.createReactivate2Command(); + CommandApdu reactivate1 = commandFactory.createReactivate1Command(); + CommandApdu reactivate2 = commandFactory.createReactivate2Command(); ResponseApdu response1 = communicate(reactivate1); ResponseApdu response2 = communicate(reactivate2); if (!response1.isSuccess()) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java index c336b4264..2d82842dc 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/Transport.java @@ -17,8 +17,6 @@ package org.sufficientlysecure.keychain.securitytoken; -import javax.smartcardio.CommandAPDU; - import java.io.IOException; /** @@ -31,7 +29,7 @@ public interface Transport { * @return received data * @throws IOException */ - ResponseApdu transceive(CommandAPDU data) throws IOException; + ResponseApdu transceive(CommandApdu data) throws IOException; /** * Disconnect and release connection 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 a37bff476..d02c06f2d 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 @@ -29,7 +29,7 @@ import android.util.Pair; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.securitytoken.Transport; -import javax.smartcardio.CommandAPDU; +import org.sufficientlysecure.keychain.securitytoken.CommandApdu; import org.sufficientlysecure.keychain.securitytoken.ResponseApdu; import org.sufficientlysecure.keychain.securitytoken.usb.tpdu.T1ShortApduProtocol; import org.sufficientlysecure.keychain.securitytoken.usb.tpdu.T1TpduProtocol; @@ -183,8 +183,8 @@ public class UsbTransport implements Transport { * @return received data */ @Override - public ResponseApdu transceive(CommandAPDU data) throws UsbTransportException { - return ResponseApdu.fromBytes(ccidTransportProtocol.transceive(data.getBytes())); + public ResponseApdu transceive(CommandApdu data) throws UsbTransportException { + return ResponseApdu.fromBytes(ccidTransportProtocol.transceive(data.toBytes())); } @Override From edaa629f4699ca5b5bdca731c215f9feb4666dca Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Oct 2017 18:42:41 +0200 Subject: [PATCH 10/13] add unit tests for CommandApdu --- .../securitytoken/CommandApduTest.java | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/CommandApduTest.java diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/CommandApduTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/CommandApduTest.java new file mode 100644 index 000000000..d77dfa24c --- /dev/null +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/CommandApduTest.java @@ -0,0 +1,102 @@ +package org.sufficientlysecure.keychain.securitytoken; + + +import org.bouncycastle.util.encoders.Hex; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.sufficientlysecure.keychain.KeychainTestRunner; + +import static junit.framework.Assert.assertEquals; +import static org.junit.Assert.assertArrayEquals; + + +@SuppressWarnings("WeakerAccess") +@RunWith(KeychainTestRunner.class) +public class CommandApduTest { + static final byte[] DATA_LONG = new byte[500]; + static final byte[] DATA_SHORT = { 1, 2, 3 }; + static final int CLA = 1; + static final int INS = 2; + static final int P1 = 3; + static final int P2 = 4; + static final int NE_SHORT = 5; + static final int NE_LONG = 500; + static final int NE_SPECIAL = 256; + + @Test + public void testCase1() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase2s() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, NE_SHORT); + + assertEquals(5, commandApdu.toBytes().length); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase2e() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, NE_LONG); + + assertEquals(7, commandApdu.toBytes().length); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase2e_specialNe() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, NE_SPECIAL); + + assertEquals(5, commandApdu.toBytes().length); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase3s() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, DATA_SHORT); + + assertEquals(4 + 1 + DATA_SHORT.length, commandApdu.toBytes().length); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase3e() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, DATA_LONG); + + assertEquals(4 + 3 + DATA_LONG.length, commandApdu.toBytes().length); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase4s() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, DATA_SHORT, 5); + + assertArrayEquals(Hex.decode("010203040301020305"), commandApdu.toBytes()); + + assertParsesCorrectly(commandApdu); + } + + @Test + public void testCase4e() throws Exception { + CommandApdu commandApdu = CommandApdu.create(CLA, INS, P1, P2, DATA_LONG, 5); + + assertEquals(4 + 5 + DATA_LONG.length, commandApdu.toBytes().length); + + assertParsesCorrectly(commandApdu); + } + + private void assertParsesCorrectly(CommandApdu commandApdu) { + byte[] bytes = commandApdu.toBytes(); + CommandApdu parsedCommandApdu = CommandApdu.fromBytes(bytes); + assertEquals(commandApdu, parsedCommandApdu); + } +} \ No newline at end of file From c295a6815f341bdd6ee8f0b6a3800f88cf91615e Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 23 Oct 2017 20:06:52 +0200 Subject: [PATCH 11/13] write nfc communication to debug output --- .../keychain/securitytoken/NfcTransport.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java index f004379bd..808b92410 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/NfcTransport.java @@ -18,7 +18,10 @@ package org.sufficientlysecure.keychain.securitytoken; import android.nfc.Tag; +import android.util.Log; +import org.bouncycastle.util.encoders.Hex; +import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity; import java.io.IOException; @@ -44,7 +47,17 @@ public class NfcTransport implements Transport { */ @Override public ResponseApdu transceive(final CommandApdu data) throws IOException { - return ResponseApdu.fromBytes(mIsoCard.transceive(data.toBytes())); + byte[] rawCommand = data.toBytes(); + if (Constants.DEBUG) { + Log.d(Constants.TAG, "nfc out: " + Hex.toHexString(rawCommand)); + } + + byte[] rawResponse = mIsoCard.transceive(rawCommand); + if (Constants.DEBUG) { + Log.d(Constants.TAG, "nfc in: " + Hex.toHexString(rawResponse)); + } + + return ResponseApdu.fromBytes(rawResponse); } /** From 2812f07d342bbd6700fa33d13e01b60ae73a76ce Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 23 Oct 2017 20:07:42 +0200 Subject: [PATCH 12/13] add rudimentary unit test for SecurityTokenConnection --- .../SecurityTokenConnection.java | 21 +++- .../ui/base/BaseSecurityTokenActivity.java | 2 + .../SecurityTokenConnectionTest.java | 114 ++++++++++++++++++ 3 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 6b659c5f6..2d2a2a27e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -23,6 +23,7 @@ package org.sufficientlysecure.keychain.securitytoken; import android.content.Context; import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; import org.bouncycastle.asn1.ASN1Encodable; import org.bouncycastle.asn1.ASN1Integer; @@ -99,16 +100,18 @@ public class SecurityTokenConnection { public static SecurityTokenConnection getInstanceForTransport(Transport transport, Passphrase pin) { if (sCachedInstance == null || !sCachedInstance.isPersistentConnectionAllowed() || !sCachedInstance.isConnected() || !sCachedInstance.mTransport.equals(transport)) { - sCachedInstance = new SecurityTokenConnection(transport, pin); + sCachedInstance = new SecurityTokenConnection(transport, pin, new OpenPgpCommandApduFactory()); } return sCachedInstance; } - private SecurityTokenConnection(@NonNull Transport transport, @NonNull Passphrase pin) { + @VisibleForTesting + SecurityTokenConnection(@NonNull Transport transport, @NonNull Passphrase pin, + OpenPgpCommandApduFactory commandFactory) { this.mTransport = transport; this.mPin = pin; - commandFactory = new OpenPgpCommandApduFactory(); + this.commandFactory = commandFactory; } private String getHolderName(byte[] name) { @@ -172,7 +175,8 @@ public class SecurityTokenConnection { /** * Connect to device and select pgp applet */ - private void connectToDevice(Context context) throws IOException { + @VisibleForTesting + void connectToDevice(Context context) throws IOException { // Connect on transport layer mCardCapabilities = new CardCapabilities(); @@ -187,8 +191,8 @@ public class SecurityTokenConnection { throw new CardException("Initialization failed!", response.getSw()); } - mOpenPgpCapabilities = new OpenPgpCapabilities(getData(0x00, 0x6E)); - mCardCapabilities = new CardCapabilities(mOpenPgpCapabilities.getHistoricalBytes()); + OpenPgpCapabilities openPgpCapabilities = new OpenPgpCapabilities(getData(0x00, 0x6E)); + setConnectionCapabilities(openPgpCapabilities); mPw1ValidatedForSignature = false; mPw1ValidatedForDecrypt = false; @@ -202,7 +206,12 @@ public class SecurityTokenConnection { Log.e(Constants.TAG, "failed to establish secure messaging", e); } } + } + @VisibleForTesting + void setConnectionCapabilities(OpenPgpCapabilities openPgpCapabilities) throws IOException { + this.mOpenPgpCapabilities = openPgpCapabilities; + this.mCardCapabilities = new CardCapabilities(openPgpCapabilities.getHistoricalBytes()); } public void resetPin(byte[] newPin, Passphrase adminPin) throws IOException { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java index 26470a9f6..bfb17a3a3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java @@ -266,6 +266,8 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity return; } + Log.d(Constants.TAG, "security token exception", e); + // Otherwise, all status codes are fixed values. switch (status) { diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java new file mode 100644 index 000000000..873536609 --- /dev/null +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java @@ -0,0 +1,114 @@ +package org.sufficientlysecure.keychain.securitytoken; + + +import java.io.IOException; + +import org.bouncycastle.util.encoders.Hex; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InOrder; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.shadows.ShadowLog; +import org.sufficientlysecure.keychain.KeychainTestRunner; +import org.sufficientlysecure.keychain.util.Passphrase; + +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + + +@RunWith(KeychainTestRunner.class) +public class SecurityTokenConnectionTest { + + @Before + public void setUp() throws Exception { + ShadowLog.stream = System.out; + } + + @Test + public void test_connectToDevice() throws Exception { + Transport transport = mock(Transport.class); + SecurityTokenConnection securityTokenConnection = + new SecurityTokenConnection(transport, new Passphrase("123456"), new OpenPgpCommandApduFactory()); + + String[] dialog = { "00a4040006d27600012401", "9000", + "00ca006e00", + "6e81de4f10d27600012401020000060364311500005f520f0073000080000000000000000000007381b7c00af" + + "00000ff04c000ff00ffc106010800001103c206010800001103c306010800001103c407007f7f7f03030" + + "3c53c4ec5fee25c4e89654d58cad8492510a89d3c3d8468da7b24e15bfc624c6a792794f15b7599915f7" + + "03aab55ed25424d60b17026b7b06c6ad4b9be30a3c63c000000000000000000000000000000000000000" + + "000000000000000000000000000000000000000000000000000000000000000000000000000000000cd0" + + "c59cd0f2a59cd0af059cd0c959000" + }; + expect(transport, dialog); + + + securityTokenConnection.connectToDevice(RuntimeEnvironment.application); + + + verify(transport).connect(); + verifyDialog(transport, dialog); + } + + @Test + public void test_getTokenInfo() throws Exception { + Transport transport = mock(Transport.class); + SecurityTokenConnection securityTokenConnection = + new SecurityTokenConnection(transport, new Passphrase("123456"), new OpenPgpCommandApduFactory()); + OpenPgpCapabilities openPgpCapabilities = new OpenPgpCapabilities( + Hex.decode( + "6e81de4f10d27600012401020000060364311500005f520f0073000080000000000000000000007381b7c00af" + + "00000ff04c000ff00ffc106010800001103c206010800001103c306010800001103c407007f7f7f03" + + "0303c53c4ec5fee25c4e89654d58cad8492510a89d3c3d8468da7b24e15bfc624c6a792794f15b759" + + "9915f703aab55ed25424d60b17026b7b06c6ad4b9be30a3c63c000000000000000000000000000000" + + "000000000000000000000000000000000000000000000000000000000000000000000000000000000" + + "000000000cd0c59cd0f2a59cd0af059cd0c95" + )); + securityTokenConnection.setConnectionCapabilities(openPgpCapabilities); + + String[] dialog = { + "00ca006e00", + "6e81de4f10d27600012401020000060364311500005f520f0073000080000000000000000000007381b7c00af" + + "00000ff04c000ff00ffc106010800001103c206010800001103c306010800001103c407007f7f7f03030" + + "3c53c4ec5fee25c4e89654d58cad8492510a89d3c3d8468da7b24e15bfc624c6a792794f15b7599915f7" + + "03aab55ed25424d60b17026b7b06c6ad4b9be30a3c63c000000000000000000000000000000000000000" + + "000000000000000000000000000000000000000000000000000000000000000000000000000000000cd0" + + "c59cd0f2a59cd0af059cd0c959000", + "00ca004f00", + "d27600012401020000060364311500009000", + "00ca006500", + "65095b005f2d005f3501399000", + "00ca5f5000", + "9000", + "00ca00c400", + "007f7f7f0303039000" + }; + expect(transport, dialog); + + + securityTokenConnection.getTokenInfo(); + + + verifyDialog(transport, dialog); + } + + private void expect(Transport transport, String... dialog) throws IOException { + for (int i = 0; i < dialog.length; i += 2) { + CommandApdu command = CommandApdu.fromBytes(Hex.decode(dialog[i])); + ResponseApdu response = ResponseApdu.fromBytes(Hex.decode(dialog[i + 1])); + when(transport.transceive(eq(command))).thenReturn(response); + } + } + + private void verifyDialog(Transport transport, String... dialog) throws IOException { + InOrder inOrder = inOrder(transport); + for (int i = 0; i < dialog.length; i += 2) { + CommandApdu command = CommandApdu.fromBytes(Hex.decode(dialog[i])); + inOrder.verify(transport).transceive(eq(command)); + } + inOrder.verifyNoMoreInteractions(); + } +} \ No newline at end of file From 7eb37a89d80518db42e1203427d2ff57f6211683 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 23 Oct 2017 20:30:49 +0200 Subject: [PATCH 13/13] reduce number of token roundtrips used to obtain SecurityTokenInfo --- .../securitytoken/OpenPgpCapabilities.java | 29 ++++++++++++++----- .../SecurityTokenConnection.java | 28 ++---------------- .../SecurityTokenConnectionTest.java | 11 ------- 3 files changed, 25 insertions(+), 43 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java index 2edc03c0d..9e9fbb285 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCapabilities.java @@ -27,7 +27,6 @@ class OpenPgpCapabilities { private final static int MASK_KEY_IMPORT = 1 << 5; private final static int MASK_ATTRIBUTES_CHANGABLE = 1 << 2; - private boolean mPw1ValidForMultipleSignatures; private byte[] mAid; private byte[] mHistoricalBytes; @@ -40,6 +39,8 @@ class OpenPgpCapabilities { private int mMaxRspLen; private Map mKeyFormats; + private byte[] mFingerprints; + private byte[] mPwStatusBytes; OpenPgpCapabilities(byte[] data) throws IOException { mKeyFormats = new HashMap<>(); @@ -76,7 +77,10 @@ class OpenPgpCapabilities { mKeyFormats.put(KeyType.AUTH, KeyFormat.fromBytes(tlv.mV)); break; case 0xC4: - mPw1ValidForMultipleSignatures = tlv.mV[0] == 1; + mPwStatusBytes = tlv.mV; + break; + case 0xC5: + mFingerprints = tlv.mV; break; } } @@ -98,7 +102,10 @@ class OpenPgpCapabilities { mKeyFormats.put(KeyType.AUTH, KeyFormat.fromBytes(tlv.mV)); break; case 0xC4: - mPw1ValidForMultipleSignatures = tlv.mV[0] == 1; + mPwStatusBytes = tlv.mV; + break; + case 0xC5: + mFingerprints = tlv.mV; break; } } @@ -115,14 +122,18 @@ class OpenPgpCapabilities { mMaxRspLen = (v[8] << 8) + v[9]; } - boolean isPw1ValidForMultipleSignatures() { - return mPw1ValidForMultipleSignatures; - } - byte[] getAid() { return mAid; } + byte[] getPwStatusBytes() { + return mPwStatusBytes; + } + + boolean isPw1ValidForMultipleSignatures() { + return mPwStatusBytes[0] == 1; + } + byte[] getHistoricalBytes() { return mHistoricalBytes; } @@ -158,4 +169,8 @@ class OpenPgpCapabilities { KeyFormat getFormatForKeyType(KeyType keyType) { return mKeyFormats.get(keyType); } + + public byte[] getFingerprints() { + return mFingerprints; + } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java index 2d2a2a27e..2a52f7586 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java @@ -572,29 +572,7 @@ public class SecurityTokenConnection { * @return The fingerprints of all subkeys in a contiguous byte array. */ public byte[] getFingerprints() throws IOException { - CommandApdu apdu = commandFactory.createGetDataCommand(0x00, 0x6E); - ResponseApdu response = communicate(apdu); - - if (!response.isSuccess()) { - throw new CardException("Failed to get fingerprints", response.getSw()); - } - - Iso7816TLV[] tlvList = Iso7816TLV.readList(response.getData(), true); - Iso7816TLV fingerPrintTlv = null; - - for (Iso7816TLV tlv : tlvList) { - Log.d(Constants.TAG, "nfcGetFingerprints() Iso7816TLV tlv data:\n" + tlv.prettyPrint()); - - Iso7816TLV matchingTlv = Iso7816TLV.findRecursive(tlv, 0xc5); - if (matchingTlv != null) { - fingerPrintTlv = matchingTlv; - } - } - - if (fingerPrintTlv == null) { - return null; - } - return fingerPrintTlv.mV; + return mOpenPgpCapabilities.getFingerprints(); } /** @@ -603,11 +581,11 @@ public class SecurityTokenConnection { * @return Seven bytes in fixed format, plus 0x9000 status word at the end. */ private byte[] getPwStatusBytes() throws IOException { - return getData(0x00, 0xC4); + return mOpenPgpCapabilities.getPwStatusBytes(); } public byte[] getAid() throws IOException { - return getData(0x00, 0x4F); + return mOpenPgpCapabilities.getAid(); } public String getUrl() throws IOException { diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java index 873536609..76ceb5b47 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionTest.java @@ -70,21 +70,10 @@ public class SecurityTokenConnectionTest { securityTokenConnection.setConnectionCapabilities(openPgpCapabilities); String[] dialog = { - "00ca006e00", - "6e81de4f10d27600012401020000060364311500005f520f0073000080000000000000000000007381b7c00af" + - "00000ff04c000ff00ffc106010800001103c206010800001103c306010800001103c407007f7f7f03030" + - "3c53c4ec5fee25c4e89654d58cad8492510a89d3c3d8468da7b24e15bfc624c6a792794f15b7599915f7" + - "03aab55ed25424d60b17026b7b06c6ad4b9be30a3c63c000000000000000000000000000000000000000" + - "000000000000000000000000000000000000000000000000000000000000000000000000000000000cd0" + - "c59cd0f2a59cd0af059cd0c959000", - "00ca004f00", - "d27600012401020000060364311500009000", "00ca006500", "65095b005f2d005f3501399000", "00ca5f5000", "9000", - "00ca00c400", - "007f7f7f0303039000" }; expect(transport, dialog);