From bca030f0695c92294c5c2de993a6062a999a5ecc Mon Sep 17 00:00:00 2001 From: Michele Balistreri Date: Mon, 23 Dec 2019 17:33:49 +0100 Subject: [PATCH 1/2] reduce NVM writes --- .../java/im/status/keycard/KeycardApplet.java | 1 - .../java/im/status/keycard/SecureChannel.java | 15 --------------- 2 files changed, 16 deletions(-) diff --git a/src/main/java/im/status/keycard/KeycardApplet.java b/src/main/java/im/status/keycard/KeycardApplet.java index 89303a2..f89aee7 100644 --- a/src/main/java/im/status/keycard/KeycardApplet.java +++ b/src/main/java/im/status/keycard/KeycardApplet.java @@ -374,7 +374,6 @@ private void selectApplet(APDU apdu) { pin.reset(); puk.reset(); secureChannel.reset(); - secureChannel.updateSecureChannelCounter(); byte[] apduBuffer = apdu.getBuffer(); diff --git a/src/main/java/im/status/keycard/SecureChannel.java b/src/main/java/im/status/keycard/SecureChannel.java index b278ce3..b70751b 100644 --- a/src/main/java/im/status/keycard/SecureChannel.java +++ b/src/main/java/im/status/keycard/SecureChannel.java @@ -13,7 +13,6 @@ public class SecureChannel { public static final short PAIRING_KEY_LENGTH = SC_SECRET_LENGTH + 1; public static final short SC_BLOCK_SIZE = Crypto.AES_BLOCK_SIZE; public static final short SC_OUT_OFFSET = ISO7816.OFFSET_CDATA + (SC_BLOCK_SIZE * 2); - public static final short SC_COUNTER_MAX = 100; public static final byte INS_OPEN_SECURE_CHANNEL = 0x10; public static final byte INS_MUTUALLY_AUTHENTICATE = 0x11; @@ -33,8 +32,6 @@ public class SecureChannel { private byte[] secret; private byte[] pairingSecret; - private short scCounter; - /* * To avoid overhead, the pairing keys are stored in a plain byte array as sequences of 33-bytes elements. The first * byte is 0 if the slot is free and 1 if used. The following 32 bytes are the actual key data. @@ -391,18 +388,6 @@ public byte getRemainingPairingSlots() { return remainingSlots; } - /** - * Called before sending the public key to the client, gives a chance to change keys if needed. - */ - public void updateSecureChannelCounter() { - if (scCounter < SC_COUNTER_MAX) { - scCounter++; - } else { - scKeypair.genKeyPair(); - scCounter = 0; - } - } - /** * Resets the Secure Channel, invalidating the current session. If no session is opened, this does nothing. */ From 1d16a82117bb587a14594d19593659dfda72e7a5 Mon Sep 17 00:00:00 2001 From: Michele Balistreri Date: Thu, 23 Jan 2020 19:05:49 +0300 Subject: [PATCH 2/2] JCOP4 workaround (defer initialization) --- .../im/status/keycard/build/InstallTask.java | 6 +++--- src/main/java/im/status/keycard/CashApplet.java | 2 +- .../java/im/status/keycard/KeycardApplet.java | 8 +++++--- src/main/java/im/status/keycard/SECP256k1.java | 4 +--- .../java/im/status/keycard/SecureChannel.java | 5 +++-- src/test/java/im/status/keycard/KeycardTest.java | 16 ++-------------- 6 files changed, 15 insertions(+), 26 deletions(-) diff --git a/buildSrc/src/main/java/im/status/keycard/build/InstallTask.java b/buildSrc/src/main/java/im/status/keycard/build/InstallTask.java index 4486627..9e8c2ad 100644 --- a/buildSrc/src/main/java/im/status/keycard/build/InstallTask.java +++ b/buildSrc/src/main/java/im/status/keycard/build/InstallTask.java @@ -64,11 +64,11 @@ public void blockLoaded(int loadedBlock, int blockCount) { } }); logger.info("Installing the Keycard Applet"); - cmdSet.installKeycardApplet(); + cmdSet.installKeycardApplet().checkOK(); logger.info("Installing the NDEF Applet"); - cmdSet.installNDEFApplet(new byte[0]); + cmdSet.installNDEFApplet(new byte[0]).checkOK(); logger.info("Installing the Cash Applet"); - cmdSet.installCashApplet(); + cmdSet.installCashApplet().checkOK(); } catch (IOException e) { throw new GradleException("I/O error", e); } catch (APDUException e) { diff --git a/src/main/java/im/status/keycard/CashApplet.java b/src/main/java/im/status/keycard/CashApplet.java index 1067d3a..dab799d 100644 --- a/src/main/java/im/status/keycard/CashApplet.java +++ b/src/main/java/im/status/keycard/CashApplet.java @@ -40,7 +40,7 @@ public static void install(byte[] bArray, short bOffset, byte bLength) { */ public CashApplet(byte[] bArray, short bOffset, byte bLength) { crypto = new Crypto(); - secp256k1 = new SECP256k1(crypto); + secp256k1 = new SECP256k1(); keypair = new KeyPair(KeyPair.ALG_EC_FP, SECP256k1.SECP256K1_KEY_SIZE); publicKey = (ECPublicKey) keypair.getPublic(); diff --git a/src/main/java/im/status/keycard/KeycardApplet.java b/src/main/java/im/status/keycard/KeycardApplet.java index f89aee7..cc71842 100644 --- a/src/main/java/im/status/keycard/KeycardApplet.java +++ b/src/main/java/im/status/keycard/KeycardApplet.java @@ -167,8 +167,7 @@ public static void install(byte[] bArray, short bOffset, byte bLength) { */ public KeycardApplet(byte[] bArray, short bOffset, byte bLength) { crypto = new Crypto(); - secp256k1 = new SECP256k1(crypto); - secureChannel = new SecureChannel(PAIRING_MAX_CLIENT_COUNT, crypto, secp256k1); + secp256k1 = new SECP256k1(); uid = new byte[UID_LENGTH]; crypto.random.generateData(uid, (short) 0, UID_LENGTH); @@ -199,7 +198,7 @@ public KeycardApplet(byte[] bArray, short bOffset, byte bLength) { derivationOutput = JCSystem.makeTransientByteArray((short) (Crypto.KEY_SECRET_SIZE + CHAIN_CODE_SIZE), JCSystem.CLEAR_ON_RESET); - data = new byte[MAX_DATA_LENGTH + 1]; + data = new byte[(short)(MAX_DATA_LENGTH + 1)]; register(bArray, (short) (bOffset + 1), bArray[bOffset]); } @@ -214,6 +213,9 @@ public KeycardApplet(byte[] bArray, short bOffset, byte bLength) { public void process(APDU apdu) throws ISOException { // If we have no PIN it means we still have to initialize the applet. if (pin == null) { + if (secureChannel == null) { + secureChannel = new SecureChannel(PAIRING_MAX_CLIENT_COUNT, crypto, secp256k1); + } processInit(apdu); return; } diff --git a/src/main/java/im/status/keycard/SECP256k1.java b/src/main/java/im/status/keycard/SECP256k1.java index ad33bba..5f53de7 100644 --- a/src/main/java/im/status/keycard/SECP256k1.java +++ b/src/main/java/im/status/keycard/SECP256k1.java @@ -54,14 +54,12 @@ public class SECP256k1 { private KeyAgreement ecPointMultiplier; - private Crypto crypto; ECPrivateKey tmpECPrivateKey; /** * Allocates objects needed by this class. Must be invoked during the applet installation exactly 1 time. */ - SECP256k1(Crypto crypto) { - this.crypto = crypto; + SECP256k1() { this.ecPointMultiplier = KeyAgreement.getInstance(ALG_EC_SVDP_DH_PLAIN_XY, false); this.tmpECPrivateKey = (ECPrivateKey) KeyBuilder.buildKey(KeyBuilder.TYPE_EC_FP_PRIVATE, SECP256K1_KEY_SIZE, false); setCurveParameters(tmpECPrivateKey); diff --git a/src/main/java/im/status/keycard/SecureChannel.java b/src/main/java/im/status/keycard/SecureChannel.java index b70751b..a77e6a7 100644 --- a/src/main/java/im/status/keycard/SecureChannel.java +++ b/src/main/java/im/status/keycard/SecureChannel.java @@ -56,13 +56,14 @@ public SecureChannel(byte pairingLimit, Crypto crypto, SECP256k1 secp256k1) { scEncKey = (AESKey) KeyBuilder.buildKey(KeyBuilder.TYPE_AES_TRANSIENT_DESELECT, KeyBuilder.LENGTH_AES_256, false); scMacKey = (AESKey) KeyBuilder.buildKey(KeyBuilder.TYPE_AES_TRANSIENT_DESELECT, KeyBuilder.LENGTH_AES_256, false); + secret = JCSystem.makeTransientByteArray((short)(SC_SECRET_LENGTH * 2), JCSystem.CLEAR_ON_DESELECT); + pairingKeys = new byte[(short)(PAIRING_KEY_LENGTH * pairingLimit)]; + scKeypair = new KeyPair(KeyPair.ALG_EC_FP, SC_KEY_LENGTH); secp256k1.setCurveParameters((ECKey) scKeypair.getPrivate()); secp256k1.setCurveParameters((ECKey) scKeypair.getPublic()); scKeypair.genKeyPair(); - secret = JCSystem.makeTransientByteArray((short)(SC_SECRET_LENGTH * 2), JCSystem.CLEAR_ON_DESELECT); - pairingKeys = new byte[(short)(PAIRING_KEY_LENGTH * pairingLimit)]; remainingSlots = pairingLimit; } diff --git a/src/test/java/im/status/keycard/KeycardTest.java b/src/test/java/im/status/keycard/KeycardTest.java index 540d4c8..b1a66ba 100644 --- a/src/test/java/im/status/keycard/KeycardTest.java +++ b/src/test/java/im/status/keycard/KeycardTest.java @@ -289,19 +289,7 @@ void openSecureChannelTest() throws Exception { response = cmdSet.getStatus(KeycardApplet.GET_STATUS_P1_APPLICATION); assertEquals(0x9000, response.getSw()); - // Verify that the keys are changed correctly. Since we do not know the internal counter we just iterate until that - // happens for a maximum of SC_COUNTER_MAX times - byte[] initialKey = new ApplicationInfo(cmdSet.select().getData()).getSecureChannelPubKey(); - for (int i = 0; i < SecureChannel.SC_COUNTER_MAX; i++) { - byte[] otherKey = new ApplicationInfo(cmdSet.select().getData()).getSecureChannelPubKey(); - - if (!Arrays.equals(initialKey, otherKey)) { - secureChannel.generateSecret(otherKey); - cmdSet.autoOpenSecureChannel(); - break; - } - } } @Test @@ -384,7 +372,7 @@ void pairTest() throws Exception { cmdSet.openSecureChannel(secureChannel.getPairingIndex(), secureChannel.getPublicKey()); // Pair multiple indexes - for (int i = 1; i < 5; i++) { + for (int i = 1; i < KeycardApplet.PAIRING_MAX_CLIENT_COUNT; i++) { cmdSet.autoPair(sharedSecret); assertEquals(i, secureChannel.getPairingIndex()); cmdSet.autoOpenSecureChannel(); @@ -403,7 +391,7 @@ void pairTest() throws Exception { assertEquals(0x9000, response.getSw()); } - for (byte i = 0; i < 4; i++) { + for (byte i = 0; i < (KeycardApplet.PAIRING_MAX_CLIENT_COUNT - 1); i++) { response = cmdSet.unpair(i); assertEquals(0x9000, response.getSw()); }