[bugfixes]

	- Bugfix in secure channel APDU encryption (missing offset)
	- Bugfix in AES CBC mode where doFinal must be calle in place
	of Update because of potential buffering yielding in bad
	result + check in AES when inputbuffer == outputffer and
	overlapping buffers (see Javacard API)
	- Use non overlapping buffer when encrypting data in
	get_key

[enhancements]
	- Better handling of native versus non-native HMAC + explicit warnings
	to the end user
	- Remove transaction when constructing persistent objects since some
	Javacards do not support this well
	- Remove DEBUG mode in the WooKey class since it is not necessary
	in production

src/wookey/auth/WooKeyAuth.java

@@ -14,6 +14,9 @@ public class WooKeyAuth extends Applet implements ExtendedLength
  /* Instructions specific to the AUTH applet */
  public static final byte TOKEN_INS_GET_KEY = (byte) 0x10;
 
+ /* Variable handling initialization */
+ private boolean init_done = false;
+
  public static void install(byte[] bArray,
  short bOffset, byte bLength)
  {
@@ -59,9 +62,9 @@ private void get_key(APDU apdu, byte ins){
  /* Also send SHA-256(ESSIV master key) */
  md.reset();
  md.doFinal(Keys.MasterSecretKey, (short) 0, (short) 32, W.data, (short) 32);
- W.schannel.pin_encrypt_sensitive_data(W.data, W.data, (short) 0, (short) 0, (short) 64); 
+ W.schannel.pin_encrypt_sensitive_data(W.data, W.data, (short) 0, (short) 64, (short) 64);
  /* Now send the encrypted APDU */
- W.schannel.send_encrypted_apdu(apdu, W.data, (short) 0, (short) 64, (byte) 0x90, (byte) 0x00);
+ W.schannel.send_encrypted_apdu(apdu, W.data, (short) 64, (short) 64, (byte) 0x90, (byte) 0x00);
  return;
  }
  }
@@ -81,13 +84,19 @@ public void process(APDU apdu)
  return;
  }
 
- if(W == null){
+ if((W == null) || (init_done == false)){
+ init_done = false;
  W = new WooKey(Keys.UserPin, Keys.PetPin, Keys.OurPrivKeyBuf, Keys.OurPubKeyBuf, Keys.WooKeyPubKeyBuf, Keys.LibECCparams, Keys.PetName, Keys.PetNameLength, Keys.max_pin_tries, Keys.max_secure_channel_tries);
+ init_done = true;
  }
 
- if(buffer[ISO7816.OFFSET_CLA] != (byte)0x00){
+ if(init_done == false){
  ISOException.throwIt((short) 0x6660);
  }
+
+ if(buffer[ISO7816.OFFSET_CLA] != (byte)0x00){
+ ISOException.throwIt((short) 0x6661);
+ }
 
  /* Begin to handle the common APDUs */
  if(W.common_apdu_process(apdu) == true){

src/wookey/common/Aes.java

@@ -217,6 +217,13 @@ private void increment_iv(){
  }
 
  public short aes(byte[] input, short inputoffset, short inputlen, byte[] output, short outputoffset){
+ /* If input and output are the same and size is block aligned (which is always the case here since we do not handle padding),
+ * they should not overlap ... See the Javacard API documentation for Cipher.update */
+ if(input == output){
+ if((inputoffset < outputoffset) && (outputoffset < (short)(inputoffset + inputlen))){
+ CryptoException.throwIt(CryptoException.ILLEGAL_USE);
+ }
+ }
  try{
  if(cipherAES == null){
  CryptoException.throwIt(CryptoException.UNINITIALIZED_KEY);
@@ -231,8 +238,11 @@ public short aes(byte[] input, short inputoffset, short inputlen, byte[] output,
  if(inputlen % AES_BLOCK_SIZE != 0){
  CryptoException.throwIt(CryptoException.ILLEGAL_VALUE);
  }
- /* Note: we use the update method for CBC to keep this feature at the lower level */
- return cipherAES.update(input, inputoffset, inputlen, output, outputoffset);
+ /* !!NOTE: we would want to use the update method for CBC to keep the CBC context at the lower level,
+ * however the Cipher.update method is buffered and doFinal must be invoked otherwise bad results
+ * can be still buffered ... (see the Javacard API documentation)
+ */
+ return cipherAES.doFinal(input, inputoffset, inputlen, output, outputoffset);
  case CTR:
  if(USE_AES_CTR_ALT_IMPLEMENTATION == false){
  /* NOTE: this seems to be sub-optimal way of performing AES-CTR for big data chunks, since

src/wookey/common/Hmac.java

@@ -7,8 +7,14 @@
  * The software implementation optionally uses masking to try limiting some leaks.
  */
 public class Hmac {
+ /* !!WARNING: using native HMAC can provoke unexpected errors on
+ * cards that do not retur an error when getting an instance, but
+ * raise an exception when using the instance ... Turn to 'false'
+ * to fall back to software HMAC if this happens.
+ */
+ private static final boolean TRY_USE_NATIVE_HMAC = true;
  /* If we have native HMAC support, use it */
- private boolean use_native_hmac = true;
+ private boolean use_native_hmac = false;
  Signature hmac_instance = null;
  HMACKey hmac_key = null;
  /* The message digest instances */
@@ -21,6 +27,11 @@ public class Hmac {
  private byte[] dgst_i;
  private byte digest = 0;
  /* For random masking */
+ /* !!WARNING: the ipad/opad masking can be hard on the eeprom initialization, and some
+ * Javacards won't be compatible with this. Turn to false if you have an error at
+ * first applet selection when eeprom is initialized by the applet ...
+ * (masking has been tested to be working on NXP J3D081 smart cards)
+ */
  private static final boolean USE_HMAC_MASKING = true;
  private RandomData random = null;
  private byte[] orig_ipad_masks = null;
@@ -29,33 +40,53 @@ public class Hmac {
  private byte[] opad_masks = null;
 
  protected Hmac(byte digest_type){
- /* Check native HMAC support on the card
- * NOTE: HMAC implementation is unfortunately not present on the tested cards ...
- */
- use_native_hmac = true;
- try {
- switch(digest_type){
- case MessageDigest.ALG_SHA_224:
- CryptoException.throwIt(CryptoException.NO_SUCH_ALGORITHM);
- break;
- case MessageDigest.ALG_SHA_256:
- hmac_instance = Signature.getInstance(Signature.ALG_HMAC_SHA_256, false);
- hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC, KeyBuilder.LENGTH_HMAC_SHA_256_BLOCK_64, false);
- break;
- case MessageDigest.ALG_SHA_384:
- hmac_instance = Signature.getInstance(Signature.ALG_HMAC_SHA_384, false);
- hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC, KeyBuilder.LENGTH_HMAC_SHA_384_BLOCK_128, false);
- break;
- case MessageDigest.ALG_SHA_512:
- hmac_instance = Signature.getInstance(Signature.ALG_HMAC_SHA_512, false);
- hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC, KeyBuilder.LENGTH_HMAC_SHA_512_BLOCK_128, false);
- break;
+ if(TRY_USE_NATIVE_HMAC == true){
+ /* Check native HMAC support on the card
+ * NOTE: HMAC implementation is unfortunately not present on the tested cards so far ...
+ * but we prepare its usage on compatible cards!
+ */
+ use_native_hmac = true;
+ try {
+ switch(digest_type){
+ case MessageDigest.ALG_SHA_224:
+ CryptoException.throwIt(CryptoException.NO_SUCH_ALGORITHM);
+ break;
+ case MessageDigest.ALG_SHA_256:
+ try {
+ hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC_TRANSIENT_DESELECT, KeyBuilder.LENGTH_HMAC_SHA_256_BLOCK_64, true);
+ }
+ catch(CryptoException e){
+ hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC_TRANSIENT_DESELECT, KeyBuilder.LENGTH_HMAC_SHA_256_BLOCK_64, false);
+ }
+ hmac_instance = Signature.getInstance(Signature.ALG_HMAC_SHA_256, false);
+ break;
+ case MessageDigest.ALG_SHA_384:
+ try {
+ hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC_TRANSIENT_DESELECT, KeyBuilder.LENGTH_HMAC_SHA_384_BLOCK_128, true);
+ }
+ catch(CryptoException e){
+ hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC_TRANSIENT_DESELECT, KeyBuilder.LENGTH_HMAC_SHA_384_BLOCK_128, false);
+ }
+ hmac_instance = Signature.getInstance(Signature.ALG_HMAC_SHA_384, false);
+ break;
+ case MessageDigest.ALG_SHA_512:
+ try {
+ hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC_TRANSIENT_DESELECT, KeyBuilder.LENGTH_HMAC_SHA_512_BLOCK_128, true);
+ }
+ catch(CryptoException e){
+ hmac_key = (HMACKey) KeyBuilder.buildKey(KeyBuilder.TYPE_HMAC_TRANSIENT_DESELECT, KeyBuilder.LENGTH_HMAC_SHA_512_BLOCK_128, false);
+ }
+ hmac_instance = Signature.getInstance(Signature.ALG_HMAC_SHA_512, false);
+ break;
+ }
+ }
+ catch(CryptoException exception){
+ /* If we got an exception, this means that the card does not have a
+ * native HMAC support. Fallback to the software one!
+ */
+ use_native_hmac = false;
  }
  }
- catch(CryptoException exception){
- use_native_hmac = false;
- }
-
  if(use_native_hmac == false){ 
  if((ipad != null) || (opad != null) || (md_i != null) || (md_o != null)){
  CryptoException.throwIt(CryptoException.INVALID_INIT);

src/wookey/common/SecureChannel.java

@@ -52,8 +52,6 @@ protected SecureChannel(byte[] default_pin, byte[] OurPrivKeyBuf, byte[] OurPubK
  public void initialize_eeprom(byte[] default_pin, byte[] OurPrivKeyBuf, byte[] OurPubKeyBuf, byte[] WooKeyPubKeyBuf, byte[] LibECCparams){
  try {
  /* Initialize our long term variables */
- /* We do this in a transaction to be safer ... */
- JCSystem.beginTransaction();
 
  /* Initialize all the crypto algorithms contexts */
  hmac_ctx = new Hmac(MessageDigest.ALG_SHA_256);
@@ -79,15 +77,9 @@ public void initialize_eeprom(byte[] default_pin, byte[] OurPrivKeyBuf, byte[] O
  ec_context.initialize_EC_key_pair_context(null, false, WooKeyPubKeyBuf, WooKeyKeyPairWrapper);
  WooKeyKeyPair = WooKeyKeyPairWrapper.kp;
  WooKeyPubKey = WooKeyKeyPairWrapper.PubKey;
-
- /* Commit our transaction */
- JCSystem.commitTransaction();
  }
  catch(CryptoException exception)
  {
- /* Abort our transaction */
- JCSystem.abortTransaction();
-
  switch(exception.getReason()){
  case CryptoException.ILLEGAL_USE:
  ISOException.throwIt((short) 0x6BD0);
@@ -111,8 +103,6 @@ public void initialize_eeprom(byte[] default_pin, byte[] OurPrivKeyBuf, byte[] O
  }
  catch(Exception e)
  {
- /* Abort our transaction */
- JCSystem.abortTransaction();
  ISOException.throwIt((short) 0x6BD6);
  }
 
@@ -352,7 +342,7 @@ public void send_encrypted_apdu(APDU apdu, byte[] indata, short indataoffset, sh
  hmac_ctx.hmac_update(tmp, (short) 1, (short) 1);
  if(indatalen > 0){
  aes_ctr_ctx.aes_init(AES_key, IV, Aes.DECRYPT);
- aes_ctr_ctx.aes(indata, (short) 0, indatalen, working_buffer, (short) 0);
+ aes_ctr_ctx.aes(indata, indataoffset, indatalen, working_buffer, (short) 0);
  /* Increment the IV by as many blocks as necessary */
  add_iv((short) (indatalen / Aes.AES_BLOCK_SIZE));
  tmp[0] = (byte)indatalen;

src/wookey/common/WooKey.java

@@ -3,7 +3,10 @@
 
 public class WooKey
 {
- private static final boolean DEBUG_MODE = true;
+ /* NOTE: the debug mode activates echo tests for the secure channel.
+ * Should be removed in production.
+ */
+ private static final boolean DEBUG_MODE = false;
 
  /* The secure channel instance */
  public SecureChannel schannel;

src/wookey/dfu/WooKeyDFU.java

@@ -40,6 +40,8 @@ public class WooKeyDFU extends Applet implements ExtendedLength
  /* Useful tmp buffer */
  private static byte[] tmp = null;
 
+ /* Variable handling initialization */
+ private boolean init_done = false;
 
  public static void install(byte[] bArray,
  short bOffset, byte bLength)
@@ -244,17 +246,24 @@ public void process(APDU apdu)
  return;
  }
 
- if(W == null){
+ if((W == null) || (init_done == false)){
+ init_done = false;
  W = new WooKey(Keys.UserPin, Keys.PetPin, Keys.OurPrivKeyBuf, Keys.OurPubKeyBuf, Keys.WooKeyPubKeyBuf, Keys.LibECCparams, Keys.PetName, Keys.PetNameLength, Keys.max_pin_tries, Keys.max_secure_channel_tries);
  /* Get the shared crypto contexts with the secure channel layer. This is done to **save memory**. */
  /* HMAC context */
  hmac_ctx = W.schannel.hmac_ctx;
  /* AES context */
  aes_cbc_ctx = W.schannel.aes_cbc_ctx;
+ init_done = true;
+
  }
 
+ if(init_done == false){
+ ISOException.throwIt((short) 0x6660);
+ }
+
  if(buffer[ISO7816.OFFSET_CLA] != (byte)0x00){
- ISOException.throwIt((short) 0x6660);
+ ISOException.throwIt((short) 0x6661);
  }
 
  /* Begin to handle the common APDUs */

src/wookey/sig/WooKeySIG.java

@@ -54,6 +54,9 @@ public class WooKeySIG extends Applet implements ExtendedLength
  public static final byte TOKEN_INS_VERIFY_FIRMWARE = (byte) 0x33;
  public static final byte TOKEN_INS_GET_SIG_TYPE = (byte) 0x34;
 
+ /* Variable handling initialization */
+ private boolean init_done = false;
+
  public static void install(byte[] bArray,
  short bOffset, byte bLength)
  {
@@ -354,7 +357,8 @@ public void process(APDU apdu)
  return;
  }
 
- if(W == null){
+ if((W == null) || (init_done == false)){
+ init_done = false;
  /* Instantiate WooKey common class */
  W = new WooKey(Keys.UserPin, Keys.PetPin, Keys.OurPrivKeyBuf, Keys.OurPubKeyBuf, Keys.WooKeyPubKeyBuf, Keys.LibECCparams, Keys.PetName, Keys.PetNameLength, Keys.max_pin_tries, Keys.max_secure_channel_tries);
  /* Import the firmware signature keys once and for all */
@@ -371,10 +375,15 @@ public void process(APDU apdu)
  hmac_ctx = W.schannel.hmac_ctx;
  /* AES context */
  aes_cbc_ctx = W.schannel.aes_cbc_ctx;
+ init_done = true;
  }
 
+ if(init_done == false){
+ ISOException.throwIt((short) 0x6660);
+ }
+
  if(buffer[ISO7816.OFFSET_CLA] != (byte)0x00){
- ISOException.throwIt((short) 0x6660);
+ ISOException.throwIt((short) 0x6661);
  }
 
  /* Begin to handle the common APDUs */