From f4edb170704e5143ed410a3e147d73777e7dec6b Mon Sep 17 00:00:00 2001 From: "Antoine FERRON [BitLogiK]" Date: Fri, 5 Feb 2021 14:34:02 +0100 Subject: [PATCH 1/2] Code cleanup double ;, and unnecessary castings --- src/main/java/toys/BlindOracleApplet.java | 2 +- src/main/java/toys/Crypto.java | 2 +- src/main/java/toys/MemoryCardApplet.java | 2 +- src/main/java/toys/PinCode.java | 4 ++-- src/main/java/toys/SecureChannel.java | 8 ++++---- src/main/java/toys/TeapotApplet.java | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/toys/BlindOracleApplet.java b/src/main/java/toys/BlindOracleApplet.java index 1f8c2e8..76a752f 100644 --- a/src/main/java/toys/BlindOracleApplet.java +++ b/src/main/java/toys/BlindOracleApplet.java @@ -88,7 +88,7 @@ public static void install(byte[] bArray, short bOffset, byte bLength){ // the JCRE calls the Applet.install method. if(bArray!=null && bArray.length > 0){ // the line below works on the card, but not in the simulator - new BlindOracleApplet().register(bArray, (short) (bOffset + 1), bArray[bOffset]);; + new BlindOracleApplet().register(bArray, (short) (bOffset + 1), bArray[bOffset]); }else{ // keep the simulator happy and register without arguments new BlindOracleApplet().register(); diff --git a/src/main/java/toys/Crypto.java b/src/main/java/toys/Crypto.java index 0720560..84077bc 100644 --- a/src/main/java/toys/Crypto.java +++ b/src/main/java/toys/Crypto.java @@ -97,6 +97,6 @@ static public short pbkdf2(byte[] pass, short pOff, short pLen, } // get our memory back heap.free(len); - return (short)hash.getLength(); + return hash.getLength(); } } \ No newline at end of file diff --git a/src/main/java/toys/MemoryCardApplet.java b/src/main/java/toys/MemoryCardApplet.java index 2083190..7c3e998 100644 --- a/src/main/java/toys/MemoryCardApplet.java +++ b/src/main/java/toys/MemoryCardApplet.java @@ -29,7 +29,7 @@ public static void install(byte[] bArray, short bOffset, byte bLength){ // the JCRE calls the Applet.install method. if(bArray!=null && bArray.length > 0){ // the line below works on the card, but not in the simulator - new MemoryCardApplet().register(bArray, (short) (bOffset + 1), bArray[bOffset]);; + new MemoryCardApplet().register(bArray, (short) (bOffset + 1), bArray[bOffset]); }else{ // keep the simulator happy and register without arguments new MemoryCardApplet().register(); diff --git a/src/main/java/toys/PinCode.java b/src/main/java/toys/PinCode.java index 3c052d0..9c48b4f 100644 --- a/src/main/java/toys/PinCode.java +++ b/src/main/java/toys/PinCode.java @@ -32,7 +32,7 @@ public PinCode(byte maxCounter, byte maxLen){ } public boolean check(byte[] buf, short off, byte len){ Crypto.hmacSha256.init(secret, (short)0, (short)secret.length); - short lenHmac = Crypto.hmacSha256.doFinal(buf, off, (short)len, hmacResult, (short)0); + short lenHmac = Crypto.hmacSha256.doFinal(buf, off, len, hmacResult, (short)0); boolean result = super.check(hmacResult, (short)0, (byte)lenHmac); // empty array Util.arrayFillNonAtomic(hmacResult, (short)0, (short)hmacResult.length, (byte)0x00); @@ -40,7 +40,7 @@ public boolean check(byte[] buf, short off, byte len){ } public void update(byte[] buf, short off, byte len){ Crypto.hmacSha256.init(secret, (short)0, (short)secret.length); - short lenHmac = Crypto.hmacSha256.doFinal(buf, off, (short)len, hmacResult, (short)0); + short lenHmac = Crypto.hmacSha256.doFinal(buf, off, len, hmacResult, (short)0); super.update(hmacResult, (short)0, (byte)lenHmac); // empty array Util.arrayFillNonAtomic(hmacResult, (short)0, (short)hmacResult.length, (byte)0x00); diff --git a/src/main/java/toys/SecureChannel.java b/src/main/java/toys/SecureChannel.java index ef2c7ff..758f1df 100644 --- a/src/main/java/toys/SecureChannel.java +++ b/src/main/java/toys/SecureChannel.java @@ -132,7 +132,7 @@ public short openChannelSS(byte[] hostPubkey, short hostPubkeyOff, // add card nonce Crypto.random.generateData(cardNonce, cardNonceOff, LENGTH_NONCE); Crypto.sha256.doFinal(cardNonce, cardNonceOff, LENGTH_NONCE, heap.buffer, off); - openChannel(heap.buffer, off, (short)Crypto.sha256.getLength()); + openChannel(heap.buffer, off, Crypto.sha256.getLength()); heap.free(len); return LENGTH_NONCE; } @@ -163,7 +163,7 @@ public short openChannelES(byte[] hostPubkey, short hostPubkeyOff, // add card nonce Crypto.random.generateData(cardNonce, cardNonceOff, LENGTH_NONCE); Crypto.sha256.doFinal(cardNonce, cardNonceOff, LENGTH_NONCE, heap.buffer, off); - openChannel(heap.buffer, off, (short)Crypto.sha256.getLength()); + openChannel(heap.buffer, off, Crypto.sha256.getLength()); heap.free(len); return LENGTH_NONCE; } @@ -194,7 +194,7 @@ public short openChannelEE( // calculating shared secret Crypto.sha256.reset(); Crypto.sha256.doFinal(heap.buffer, off, ecdhLen, heap.buffer, off); - openChannel(heap.buffer, off, (short)Crypto.sha256.getLength()); + openChannel(heap.buffer, off, Crypto.sha256.getLength()); heap.free(len); // pubkey is just ECDH of private key with G return Secp256k1.getPublicKey( ephemeralPrivateKey, false, @@ -284,7 +284,7 @@ public short authenticateData(byte[] data, short dataOffset, short dataLen, public short signData(byte[] data, short dataOffset, short dataLen, byte[] out, short outOffset) { - short len = (short)Crypto.sha256.getLength(); + short len = Crypto.sha256.getLength(); short off = heap.allocate(len); Crypto.sha256.reset(); diff --git a/src/main/java/toys/TeapotApplet.java b/src/main/java/toys/TeapotApplet.java index d39fe08..df9d7a2 100644 --- a/src/main/java/toys/TeapotApplet.java +++ b/src/main/java/toys/TeapotApplet.java @@ -116,7 +116,7 @@ protected void StoreData(APDU apdu){ ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); } // copy content of the buffer to the data array - data.put(buf, (short)ISO7816.OFFSET_CDATA, len); + data.put(buf, ISO7816.OFFSET_CDATA, len); // send back data stored in flash SendData(apdu); } From 92bf47a1c518adf536351f2fb332a373a2af8dce Mon Sep 17 00:00:00 2001 From: "Antoine FERRON [BitLogiK]" Date: Fri, 5 Feb 2021 14:34:57 +0100 Subject: [PATCH 2/2] Remove duplicate code and enforce non-break --- src/main/java/toys/SingleUseKeyApplet.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/toys/SingleUseKeyApplet.java b/src/main/java/toys/SingleUseKeyApplet.java index 9ea0e50..e449314 100644 --- a/src/main/java/toys/SingleUseKeyApplet.java +++ b/src/main/java/toys/SingleUseKeyApplet.java @@ -54,6 +54,7 @@ public SingleUseKeyApplet(){ // ok, if you want to use it without secure communication // - you should be able to, even though it might be an issue with MITM // if you don't - comment out this function + @SuppressWarnings("fallthrough") protected short processPlainMessage(byte[] buf, short len){ // ugly copy-paste for now switch (buf[ISO7816.OFFSET_INS]){ @@ -61,9 +62,6 @@ protected short processPlainMessage(byte[] buf, short len){ switch (buf[ISO7816.OFFSET_P1]){ case SUBCMD_SINGLE_USE_KEY_GENERATE: generateRandomKey(); - // no need to break - we return compressed pubkey - // but compiler complains, so - return Secp256k1.serialize((ECPublicKey)singleUseKeyPair.getPublic(), true, buf, OFFSET_PLAIN_PAYLOAD); case SUBCMD_SINGLE_USE_KEY_GET_PUBKEY: // serialize pubkey in compressed form return Secp256k1.serialize((ECPublicKey)singleUseKeyPair.getPublic(), true, buf, OFFSET_PLAIN_PAYLOAD); @@ -89,6 +87,7 @@ protected short processSecureMessage(byte[] buf, short len){ } return 0; } + @SuppressWarnings("fallthrough") protected short processSingleUseKeyCommand(byte[] buf, short len){ if(isLocked()){ ISOException.throwIt(ERR_CARD_LOCKED); @@ -98,10 +97,6 @@ protected short processSingleUseKeyCommand(byte[] buf, short len){ switch (subcmd){ case SUBCMD_SINGLE_USE_KEY_GENERATE: generateRandomKey(); - // no need to break - we return compressed pubkey - // but compiler complains, so - lenOut += Secp256k1.serialize((ECPublicKey)singleUseKeyPair.getPublic(), true, buf, OFFSET_SECURE_PAYLOAD); - return lenOut; case SUBCMD_SINGLE_USE_KEY_GET_PUBKEY: // serialize pubkey in compressed form lenOut += Secp256k1.serialize((ECPublicKey)singleUseKeyPair.getPublic(), true, buf, OFFSET_SECURE_PAYLOAD);