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 5fa4f93e9..1ffbb72f7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java @@ -28,7 +28,9 @@ import org.bouncycastle.util.encoders.Hex; public class OpenPgpCommandApduFactory { - private static final int MAX_APDU_NC = 255; + // The spec allows 255, but for compatibility with non-compliant tokens we use 254 here + // See https://github.com/open-keychain/open-keychain/issues/2049 + private static final int MAX_APDU_NC = 254; private static final int MAX_APDU_NC_EXT = 65535; private static final int MAX_APDU_NE = 256; @@ -134,8 +136,7 @@ public class OpenPgpCommandApduFactory { @NonNull public CommandApdu createDecipherCommand(byte[] data) { - return CommandApdu.create(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data, - MAX_APDU_NE_EXT); + return CommandApdu.create(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data); } @NonNull diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOp.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOp.java index 3a2a8307d..4fde33d14 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOp.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOp.java @@ -25,6 +25,7 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; import javax.crypto.Cipher; import javax.crypto.NoSuchPaddingException; @@ -43,7 +44,6 @@ import org.sufficientlysecure.keychain.securitytoken.CardException; import org.sufficientlysecure.keychain.securitytoken.CommandApdu; import org.sufficientlysecure.keychain.securitytoken.ECKeyFormat; import org.sufficientlysecure.keychain.securitytoken.KeyFormat; -import org.sufficientlysecure.keychain.securitytoken.KeyType; import org.sufficientlysecure.keychain.securitytoken.ResponseApdu; import org.sufficientlysecure.keychain.securitytoken.SecurityTokenConnection; @@ -84,14 +84,7 @@ public class PsoDecryptTokenOp { } private byte[] decryptSessionKeyRsa(byte[] encryptedSessionKeyMpi) throws IOException { - int mpiLength = getMpiLength(encryptedSessionKeyMpi); - if (mpiLength != encryptedSessionKeyMpi.length - 2) { - throw new IOException("Malformed RSA session key!"); - } - - byte[] psoDecipherPayload = new byte[mpiLength + 1]; - psoDecipherPayload[0] = (byte) 0x00; // RSA Padding Indicator Byte - System.arraycopy(encryptedSessionKeyMpi, 2, psoDecipherPayload, 1, mpiLength); + byte[] psoDecipherPayload = getRsaOperationPayload(encryptedSessionKeyMpi); CommandApdu command = connection.getCommandFactory().createDecipherCommand(psoDecipherPayload); ResponseApdu response = connection.communicate(command); @@ -103,6 +96,19 @@ public class PsoDecryptTokenOp { return response.getData(); } + @VisibleForTesting + public byte[] getRsaOperationPayload(byte[] encryptedSessionKeyMpi) throws IOException { + int mpiLength = getMpiLength(encryptedSessionKeyMpi); + if (mpiLength != encryptedSessionKeyMpi.length - 2) { + throw new IOException("Malformed RSA session key!"); + } + + byte[] psoDecipherPayload = new byte[mpiLength + 1]; + psoDecipherPayload[0] = 0x00; // RSA Padding Indicator Byte + System.arraycopy(encryptedSessionKeyMpi, 2, psoDecipherPayload, 1, mpiLength); + return psoDecipherPayload; + } + private byte[] decryptSessionKeyEcdh(byte[] encryptedSessionKeyMpi, ECKeyFormat eckf, CanonicalizedPublicKey publicKey) throws IOException { int mpiLength = getMpiLength(encryptedSessionKeyMpi); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionCompatTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionCompatTest.java index fc368dc00..c5289f32e 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionCompatTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionCompatTest.java @@ -19,7 +19,6 @@ package org.sufficientlysecure.keychain.securitytoken; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import org.bouncycastle.util.encoders.Hex; @@ -28,8 +27,10 @@ import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.sufficientlysecure.keychain.KeychainTestRunner; +import org.sufficientlysecure.keychain.securitytoken.operations.PsoDecryptTokenOp; import static junit.framework.Assert.assertEquals; +import static org.mockito.Mockito.mock; @RunWith(KeychainTestRunner.class) @@ -52,7 +53,7 @@ public class SecurityTokenConnectionCompatTest { */ @Test - public void testPrePostEquals() { + public void testPrePostEquals() throws Exception { List preApdus = decryptPre_ee8cd38(); List postApdus = decryptNow(); @@ -80,11 +81,9 @@ public class SecurityTokenConnectionCompatTest { return apduData; } - public List decryptNow() { - int mpiLength = ((((encryptedSessionKey[0] & 0xff) << 8) + (encryptedSessionKey[1] & 0xff)) + 7) / 8; - byte[] psoDecipherPayload = new byte[mpiLength + 1]; - psoDecipherPayload[0] = (byte) 0x00; - System.arraycopy(encryptedSessionKey, 2, psoDecipherPayload, 1, mpiLength); + public List decryptNow() throws Exception { + PsoDecryptTokenOp psoDecryptTokenOp = PsoDecryptTokenOp.create(mock(SecurityTokenConnection.class)); + byte[] psoDecipherPayload = psoDecryptTokenOp.getRsaOperationPayload(encryptedSessionKey); CommandApdu command = openPgpCommandApduFactory.createDecipherCommand(psoDecipherPayload); List chainedApdus = openPgpCommandApduFactory.createChainedApdus(command);