From 52c8824969b365cfaa085e3cc1ad7c4a4a2fd2ad Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 22 Mar 2018 17:09:16 +0100 Subject: [PATCH 1/2] token: send expected result size as Le This is a different take on 1c8cc99c681a435254a1326949da33a88a532b57, sending the expected result size. It's not what the spec says, but it's what GnuPG does, so it should achieve good compatibility. --- .../keychain/securitytoken/OpenPgpCommandApduFactory.java | 5 +++-- .../securitytoken/operations/PsoDecryptTokenOp.java | 6 ++++-- .../securitytoken/SecurityTokenConnectionCompatTest.java | 2 +- .../securitytoken/operations/PsoDecryptTokenOpTest.java | 5 ++++- 4 files changed, 12 insertions(+), 6 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 1ffbb72f7..f5070db70 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java @@ -135,8 +135,9 @@ public class OpenPgpCommandApduFactory { } @NonNull - public CommandApdu createDecipherCommand(byte[] data) { - return CommandApdu.create(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data); + public CommandApdu createDecipherCommand(byte[] data, int expectedLength) { + return CommandApdu.create(CLA, INS_PERFORM_SECURITY_OPERATION, P1_PSO_DECIPHER, P2_PSO_DECIPHER, data, + expectedLength); } @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 f5b534142..ceb54cfcb 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 @@ -85,9 +85,10 @@ public class PsoDecryptTokenOp { } private byte[] decryptSessionKeyRsa(byte[] encryptedSessionKeyMpi) throws IOException { + int mpiLength = getMpiLength(encryptedSessionKeyMpi); byte[] psoDecipherPayload = getRsaOperationPayload(encryptedSessionKeyMpi); - CommandApdu command = connection.getCommandFactory().createDecipherCommand(psoDecipherPayload); + CommandApdu command = connection.getCommandFactory().createDecipherCommand(psoDecipherPayload, mpiLength); ResponseApdu response = connection.communicate(command); if (!response.isSuccess()) { @@ -139,7 +140,8 @@ public class PsoDecryptTokenOp { } psoDecipherPayload = Arrays.concatenate(Hex.decode("A6"), dataLen, psoDecipherPayload); - CommandApdu command = connection.getCommandFactory().createDecipherCommand(psoDecipherPayload); + CommandApdu command = connection.getCommandFactory().createDecipherCommand( + psoDecipherPayload, encryptedPoint.length); ResponseApdu response = connection.communicate(command); if (!response.isSuccess()) { 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 c5289f32e..b0cdb7a46 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionCompatTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnectionCompatTest.java @@ -85,7 +85,7 @@ public class SecurityTokenConnectionCompatTest { PsoDecryptTokenOp psoDecryptTokenOp = PsoDecryptTokenOp.create(mock(SecurityTokenConnection.class)); byte[] psoDecipherPayload = psoDecryptTokenOp.getRsaOperationPayload(encryptedSessionKey); - CommandApdu command = openPgpCommandApduFactory.createDecipherCommand(psoDecipherPayload); + CommandApdu command = openPgpCommandApduFactory.createDecipherCommand(psoDecipherPayload, encryptedSessionKey.length); List chainedApdus = openPgpCommandApduFactory.createChainedApdus(command); List apduData = new ArrayList<>(); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOpTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOpTest.java index 4911996d6..4ffd42fae 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOpTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/securitytoken/operations/PsoDecryptTokenOpTest.java @@ -32,6 +32,7 @@ import org.sufficientlysecure.keychain.securitytoken.operations.PsoDecryptTokenO import static org.junit.Assert.*; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -45,6 +46,8 @@ public class PsoDecryptTokenOpTest { "fb9d8cbcb34f28d0b968b6e09eda0e1d3ab6b251eb09f9fb9d9abfeaf9010001733b9015e9e4b6c9df61bbc76041f439d1" + "273e41f5d0e8414a2b8d6d4c7e86f30b94cfba308b38de53d694a8ca15382301ace806c8237641b03525b3e3e8cbb017e2" + "51265229bcbb0da5d5aeb4eafbad9779"); + private static final int RSA_ENC_MODULUS = 256; + private SecurityTokenConnection securityTokenConnection; private OpenPgpCommandApduFactory commandFactory; private PsoDecryptTokenOp useCase; @@ -75,7 +78,7 @@ public class PsoDecryptTokenOpTest { ResponseApdu dummyResponseApdu = ResponseApdu.fromBytes(Hex.decode("010203049000")); - when(commandFactory.createDecipherCommand(any(byte[].class))).thenReturn(dummyCommandApdu); + when(commandFactory.createDecipherCommand(any(byte[].class), eq(RSA_ENC_MODULUS))).thenReturn(dummyCommandApdu); when(securityTokenConnection.communicate(dummyCommandApdu)).thenReturn(dummyResponseApdu); byte[] response = useCase.verifyAndDecryptSessionKey(RSA_ENC_SESSIONKEY_MPI, null); From 9a86d45bc160cfa1111d3198b58f53fa820e72e9 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sat, 24 Mar 2018 14:14:30 +0100 Subject: [PATCH 2/2] token: send Ne only on last in a chain --- .../securitytoken/OpenPgpCommandApduFactory.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 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 f5070db70..b31748de1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/OpenPgpCommandApduFactory.java @@ -221,14 +221,18 @@ public class OpenPgpCommandApduFactory { 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 = - CommandApdu.create(cla, apdu.getINS(), apdu.getP1(), apdu.getP2(), data, offset, curLen, ne); + CommandApdu cmd; + if (last) { + int ne = Math.min(apdu.getNe(), MAX_APDU_NE); + cmd = CommandApdu.create(cla, apdu.getINS(), apdu.getP1(), apdu.getP2(), data, offset, curLen, ne); + } else { + cmd = CommandApdu.create(cla, apdu.getINS(), apdu.getP1(), apdu.getP2(), data, offset, curLen); + } result.add(cmd); offset += curLen;