Key collision robustness, refs #108 and fixes #107

Conflicts:

	java/code/src/org/keyczar/Keyczar.java
	java/code/src/org/keyczar/UnversionedVerifier.java
	java/code/src/org/keyczar/Verifier.java

java/code/src/org/keyczar/Crypter.java

@@ -29,6 +29,7 @@
 import org.keyczar.util.Base64Coder;
 
 import java.nio.ByteBuffer;
+import java.util.Collection;
 
 /**
  * Crypters may both encrypt and decrypt data using sets of symmetric or private
@@ -78,7 +79,7 @@ public Crypter(String fileLocation) throws KeyczarException {
   public byte[] decrypt(byte[] input) throws KeyczarException {
     ByteBuffer output = ByteBuffer.allocate(input.length);
     decrypt(ByteBuffer.wrap(input), output);
-    output.reset();
+    output.rewind();
     byte[] outputBytes = new byte[output.remaining()];
     output.get(outputBytes);
     return outputBytes;
@@ -107,57 +108,91 @@ public void decrypt(ByteBuffer input, ByteBuffer output)
 
     byte[] hash = new byte[KEY_HASH_SIZE];
     inputCopy.get(hash);
-    KeyczarKey key = getKey(hash);
-    if (key == null) {
+    Collection<KeyczarKey> keys  = getKey(hash);
+    if (keys == null) {
       throw new KeyNotFoundException(hash);
     }
 
     // The input to decrypt is now positioned at the start of the ciphertext
     inputCopy.mark();
-
-    DecryptingStream cryptStream = (DecryptingStream) key.getStream();
-
-    VerifyingStream verifyStream = cryptStream.getVerifyingStream();
-    if (inputCopy.remaining() < verifyStream.digestSize()) {
-      throw new ShortCiphertextException(inputCopy.remaining());
+    int inputLimit = inputCopy.limit();
+    KeyczarException error = null;
+
+    boolean collision = keys.size() > 1;
+
+    for (KeyczarKey key : keys) {
+      error = null;
+
+      ByteBuffer tempBuffer = output;
+      if (collision) {
+        tempBuffer = ByteBuffer.allocate(output.capacity());
+      }
+
+      DecryptingStream cryptStream = (DecryptingStream) key.getStream();
+
+      try {
+        VerifyingStream verifyStream = cryptStream.getVerifyingStream();
+        if (inputCopy.remaining() < verifyStream.digestSize()) {
+          throw new ShortCiphertextException(inputCopy.remaining());
+        }
+
+        // Slice off the signature into another buffer
+        inputCopy.position(inputCopy.limit() - verifyStream.digestSize());
+        ByteBuffer signature = inputCopy.slice();
+
+        // Reset the position of the input to start of the ciphertext
+        inputCopy.reset();
+        inputCopy.limit(inputCopy.limit() - verifyStream.digestSize());
+
+        // Initialize the crypt stream. This may read an IV if any.
+        cryptStream.initDecrypt(inputCopy);
+
+        // Verify the header and IV if any
+        ByteBuffer headerAndIvToVerify = input.asReadOnlyBuffer();
+        headerAndIvToVerify.limit(inputCopy.position());
+        verifyStream.initVerify();
+        verifyStream.updateVerify(headerAndIvToVerify);
+
+        tempBuffer.mark();
+        // This will process large input in chunks, rather than all at once. This
+        // avoids making two passes through memory.
+        while (inputCopy.remaining() > DECRYPT_CHUNK_SIZE) {
+          ByteBuffer ciphertextChunk = inputCopy.slice();
+          ciphertextChunk.limit(DECRYPT_CHUNK_SIZE);
+          cryptStream.updateDecrypt(ciphertextChunk, output);
+          ciphertextChunk.rewind();
+          verifyStream.updateVerify(ciphertextChunk);
+          inputCopy.position(inputCopy.position() + DECRYPT_CHUNK_SIZE);
+        }
+        int lastBlock = inputCopy.position();
+        verifyStream.updateVerify(inputCopy);
+        if (!verifyStream.verify(signature)) {
+          throw new InvalidSignatureException();
+        }
+        inputCopy.position(lastBlock);
+        cryptStream.doFinalDecrypt(inputCopy, tempBuffer);
+        tempBuffer.limit(tempBuffer.position());
+        if (collision) {
+          //Success copy to final output buffer
+          tempBuffer.rewind();
+          output.put(tempBuffer);
+          output.limit(output.position());
+        }
+        return;
+      } catch (KeyczarException e) {
+        LOG.debug(e.getMessage(), e);
+        error = e;
+      } catch (RuntimeException e) {
+        LOG.debug(e.getMessage(), e);
+        error = new InvalidSignatureException();
+      } finally {
+        inputCopy.reset();
+        inputCopy.limit(inputLimit);
+      }
     }
-
-    // Slice off the signature into another buffer
-    inputCopy.position(inputCopy.limit() - verifyStream.digestSize());
-    ByteBuffer signature = inputCopy.slice();
-
-    // Reset the position of the input to start of the ciphertext
-    inputCopy.reset();
-    inputCopy.limit(inputCopy.limit() - verifyStream.digestSize());
-
-    // Initialize the crypt stream. This may read an IV if any.
-    cryptStream.initDecrypt(inputCopy);
-
-    // Verify the header and IV if any
-    ByteBuffer headerAndIvToVerify = input.asReadOnlyBuffer();
-    headerAndIvToVerify.limit(inputCopy.position());
-    verifyStream.initVerify();
-    verifyStream.updateVerify(headerAndIvToVerify);
-
-    output.mark();
-    // This will process large input in chunks, rather than all at once. This
-    // avoids making two passes through memory.
-    while (inputCopy.remaining() > DECRYPT_CHUNK_SIZE) {
-      ByteBuffer ciphertextChunk = inputCopy.slice();
-      ciphertextChunk.limit(DECRYPT_CHUNK_SIZE);
-      cryptStream.updateDecrypt(ciphertextChunk, output);
-      ciphertextChunk.rewind();
-      verifyStream.updateVerify(ciphertextChunk);
-      inputCopy.position(inputCopy.position() + DECRYPT_CHUNK_SIZE);
-    }
-    inputCopy.mark();
-    verifyStream.updateVerify(inputCopy);
-    if (!verifyStream.verify(signature)) {
-      throw new InvalidSignatureException();
+    if (error != null) {
+      throw error;
     }
-    inputCopy.reset();
-    cryptStream.doFinalDecrypt(inputCopy, output);
-    output.limit(output.position());
   }
 
   /**

java/code/src/org/keyczar/Keyczar.java

@@ -24,6 +24,8 @@
 import org.keyczar.interfaces.KeyczarReader;
 import org.keyczar.util.Util;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 
 /**
@@ -44,8 +46,8 @@ abstract class Keyczar {
   KeyVersion primaryVersion;
   final HashMap<KeyVersion, KeyczarKey> versionMap =
     new HashMap<KeyVersion, KeyczarKey>();
-  final HashMap<KeyHash, KeyczarKey> hashMap =
-    new HashMap<KeyHash, KeyczarKey>(); // keep track of used hash identifiers
+  final HashMap<KeyHash, ArrayList<KeyczarKey>> hashMap =
+    new HashMap<KeyHash, ArrayList<KeyczarKey>>(); // keep track of used hash identifiers
 
   private class KeyHash {
     private byte[] data;
@@ -96,10 +98,19 @@ public Keyczar(KeyczarReader reader) throws KeyczarException {
       }
       String keyString = reader.getKey(version.getVersionNumber());
       KeyczarKey key = kmd.getType().getBuilder().read(keyString);
-      hashMap.put(new KeyHash(key.hash()), key);
+      addKeyHashMap(key.hash(), key);
       versionMap.put(version, key);
     }
   }
+
+  private void addKeyHashMap(byte[] hash, KeyczarKey key) {
+    KeyHash kHash = new KeyHash(hash);
+    if (hashMap.get(kHash) == null) {
+      hashMap.put(kHash, new ArrayList<KeyczarKey>());
+    }
+    hashMap.get(kHash).add(key);
+  }
+
 
   /**
    * Instantiates a new Keyczar object with a KeyczarFileReader instantiated
@@ -125,7 +136,7 @@ public String toString() {
    * @param key KeyczarKey
    */
   void addKey(KeyVersion version, KeyczarKey key) {
-    hashMap.put(new KeyHash(key.hash()), key);
+    addKeyHashMap(key.hash(), key);
     versionMap.put(version, key);
     kmd.addVersion(version);
   }
@@ -137,7 +148,7 @@ KeyczarKey getPrimaryKey() {
     return versionMap.get(primaryVersion);
   }
 
-  KeyczarKey getKey(byte[] hash) {
+  Collection<KeyczarKey> getKey(byte[] hash) {
     return hashMap.get(new KeyHash(hash));
   }
 

java/code/src/org/keyczar/UnversionedVerifier.java

@@ -93,10 +93,8 @@ public boolean verify(byte[] data, byte[] signature) throws KeyczarException {
    * @param data The data to verify the signature on
    * @param signature The signature to verify
    * @return Whether this is a valid signature
-   * @throws KeyczarException If the signature is malformed or a JCE error occurs.
    */
-  public boolean verify(ByteBuffer data, ByteBuffer signature)
-      throws KeyczarException {
+  public boolean verify(ByteBuffer data, ByteBuffer signature) {
     for (KeyczarKey key : versionMap.values()) {
       if (verify(data, signature, key)) {
         return true;
@@ -105,13 +103,17 @@ public boolean verify(ByteBuffer data, ByteBuffer signature)
     return false;
   }
 
-  private boolean verify(ByteBuffer data, ByteBuffer signature, KeyczarKey key)
-      throws KeyczarException {
-    VerifyingStream stream = (VerifyingStream) key.getStream();
-    stream.initVerify();
-    stream.updateVerify(data.duplicate());
-    boolean foundValidSignature = stream.verify(signature.duplicate());
-    return foundValidSignature;
+  private boolean verify(ByteBuffer data, ByteBuffer signature, KeyczarKey key) {
+    try {
+      VerifyingStream stream = (VerifyingStream) key.getStream();
+      stream.initVerify();
+      stream.updateVerify(data.duplicate());
+      return stream.verify(signature.duplicate());
+    } catch (KeyczarException e) {
+      return false;
+    } catch (RuntimeException e) {
+      return false;
+    }
   }
 
   /**
@@ -137,4 +139,4 @@ boolean isAcceptablePurpose(KeyPurpose purpose) {
     return (purpose == KeyPurpose.VERIFY ||
             purpose == KeyPurpose.SIGN_AND_VERIFY);
   }
-}
+}

java/code/src/org/keyczar/Verifier.java

@@ -117,26 +117,34 @@ boolean verify(ByteBuffer data, ByteBuffer hidden,
     }
 
     byte[] hash = checkFormatAndGetHash(signature);
-    KeyczarKey key = getVerifyingKey(hash);
+    Iterable<KeyczarKey> keys = getVerifyingKey(hash);
 
-    if (key == null) {
+    if (keys == null) {
       throw new KeyNotFoundException(hash);
     }
-
-    // TODO(normandl): replace following with rawVerify().
-    VerifyingStream stream = (VerifyingStream) key.getStream();
-    stream.initVerify();
+
+    data.mark();
     if (hidden != null) {
-      stream.updateVerify(hidden);
+      hidden.mark();
     }
-
-    stream.updateVerify(data);
-
-    // The signed data is terminated with the current Keyczar format
-    stream.updateVerify(ByteBuffer.wrap(FORMAT_BYTES));
-
-    boolean result = stream.verify(signature);
-    return result;
+    signature.mark();
+    for (KeyczarKey key : keys) {
+      try {
+        if (rawVerify(key,data, hidden, signature)) {
+          return true;
+        }
+      } catch (KeyczarException e) {
+        //Continue Checking keys in case of collision
+      } catch (RuntimeException e) {
+        //Unfortunately Java crypto apis can throw runtime exceptions
+      }
+      data.reset();
+      if (hidden != null) {
+        hidden.reset();
+      }
+      signature.reset();
+    }
+    return false;
   }
 
 
@@ -195,7 +203,6 @@ public boolean attachedVerify(final byte[] signedBlob,
     ByteBuffer sigBuffer = ByteBuffer.wrap(signedBlob);
     // assume I need to decode here as well.
     byte[] hash = checkFormatAndGetHash(sigBuffer);
-    KeyczarKey key = getVerifyingKey(hash);
 
     // we have stripped the format and hash, now just get the blob and
     // raw signature
@@ -210,10 +217,24 @@ public boolean attachedVerify(final byte[] signedBlob,
     // [blob | hidden.length | hidden | format] or [blob | 0 | format]
     byte[] hiddenPlusLength = Util.fromInt(0);
     if (hidden.length > 0) {
-    	hiddenPlusLength = Util.lenPrefix(hidden);
+      hiddenPlusLength = Util.lenPrefix(hidden);
+    }
+
+    Iterable<KeyczarKey> keys = getVerifyingKey(hash);
+    for (KeyczarKey key : keys) {
+      try {
+        if (rawVerify(key, ByteBuffer.wrap(blob), ByteBuffer.wrap(hiddenPlusLength), 
+            ByteBuffer.wrap(signature))) {
+          return true;
+        }
+      } catch (KeyczarException e) {
+            //continue checking keys incase of collision
+      } catch (RuntimeException e) {
+            //unfortunately java crypto apis can throw a runtime exception
+      } 
     }
-    return rawVerify(key, ByteBuffer.wrap(blob),
-      ByteBuffer.wrap(hiddenPlusLength), ByteBuffer.wrap(signature));
+
+    return false;
   }
 
   /**
@@ -278,8 +299,8 @@ private byte[] checkFormatAndGetHash(ByteBuffer signature)
     return hash;
   }
 
-  private KeyczarKey getVerifyingKey(byte[] hash) throws KeyNotFoundException {
-    KeyczarKey key = getKey(hash);
+  private Iterable<KeyczarKey> getVerifyingKey(byte[] hash) throws KeyNotFoundException {
+    Iterable<KeyczarKey> key = getKey(hash);
 
     if (key == null) {
       throw new KeyNotFoundException(hash);

java/code/testdata/key-collision/aes/1

@@ -0,0 +1 @@
+{"mode":"CBC","aesKeyString":"wcGN7p7j_muIu86LDoAdmw","hmacKey":{"hmacKeyString":"iRJQ5ofhCWwCp0WBSIXwxdaidf-WOqdRHMif6150PCA","size":256},"size":128}

java/code/testdata/key-collision/aes/1.out

@@ -0,0 +1 @@
+AKHGfTNGchk4xtw28PRtgcNHSBa1sy9aTXDcomTzui7i9_56Ws9mYQYc35j79sbF4XLJXtCVIm94Ur1itL-1rJwSaCLweayKzw

java/code/testdata/key-collision/aes/2

@@ -0,0 +1 @@
+{"mode":"CBC","aesKeyString":"gBaxAD2aVfYQFc_dGyPc-Q","hmacKey":{"hmacKeyString":"ITt6UOcOqp28iGVo40QCKj9FhRazTvc9csb17qqOO0E","size":256},"size":128}

java/code/testdata/key-collision/aes/2.out

@@ -0,0 +1 @@
+AKHGfTNFRJZKAOzy8cofi3B7C2KJ3ARPXICvOIq-EHkaDJDIYAqNlMgk7znvoYEh2yiGAeV9C1Q1jgivKHN_aUsoqNO0w56dAg

java/code/testdata/key-collision/aes/meta

@@ -0,0 +1 @@
+{"name":"Test","purpose":"DECRYPT_AND_ENCRYPT","type":"AES","encrypted":false,"versions":[{"versionNumber":1,"exportable":false,"status":"ACTIVE"},{"versionNumber":2,"exportable":false,"status":"PRIMARY"}]}

java/code/testdata/key-collision/dsa/1

@@ -0,0 +1 @@
+{"publicKey":{"p":"AI6k1w2pdVBtB15tjNWv87wTJgwyqXxRtQEYL1m4CapIr-U6LVT1A1ekgPFP9Ftb7th4BQ_mBH4ZPywJ_oR2kG66ePF-jYlgnw6gYtLC9UuQ7o6rEdNSpC6c_W5MQr2mzkjq81JfHAu2ZRFvDwITuBX6BAK6_PFEs9cRjWdOQpjp","q":"AME9UCwVrc9TnLJNgMWXjNJXTSh3","g":"Pt_xKE1QFQbqN-bb8nHuMSoyyg5IRimmiJFD0sKqFxSjRvcZUwfsF1un62E_Bbvmzzb0HJA5OtdkkwkV8Fvnga7_U2XzGyu4m92njCYBJ70GuLagkUgUQKPKDvNq_W_Z70Nup5KN5k62gdLBKtgZfes_ahTWb_fpOwQmazqHOVY","y":"X-_Zff6_T31fLOxcKjLZ9VMoI5SAZ8kuYgUEaXhVgJhQD4IkMiPGvxofr3nCTGhBkkrEdwX7MfzKE-Bo99f4z0DTU3D7g94_Nci3xQ9oo5cq3NajSH0i0LQFg1fkY65H160RyMg6OhpSDxU3GAvU5XyxRHydrypr2KJIZXHsxx8","size":1024},"x":"Ir907TaDdjF0_eV1DwffaIWyk6Y","size":1024}

java/code/testdata/key-collision/dsa/1.attached

@@ -0,0 +1 @@
+AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAuAhUAioxiTw-nS5mZSr2_4_ypN4mbXHACFQCwqVKuvKvdnVsznQnyk5KbHx9-kQ

java/code/testdata/key-collision/dsa/1.out

@@ -0,0 +1 @@
+AP2pfRwwLAIUTpbMjHGAFJ81zGKFI9fbONckXroCFAXQaMD3qXQ5oe5APGal7wvcSSCz

java/code/testdata/key-collision/dsa/1.secret.attached

@@ -0,0 +1 @@
+AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAsAhRlEVcAyc3Ep5MA8uRJFAMt9oATywIUU8XCn5sj7lpRAJUXc9uxlDU5nE0

java/code/testdata/key-collision/dsa/1.timeout

@@ -0,0 +1 @@
+AP2pfRwAAAE7vSi1oDAsAhQUKQvYH7PJoq42lhBAK3qEk6RD3wIUCQd_4Xqw9u59UknfKg2O5xg3qe0

java/code/testdata/key-collision/dsa/2

@@ -0,0 +1 @@
+{"publicKey":{"p":"AKSQoTOhq_B6L0jJuvx_GurShx0wS_ql_aUFD49DI_ANt38wxNqaGksP228k4yp-7ExKKZsy_Kwf-Ky_iQpypZFGU0QVvED2gohicIk3rAZAW_I3gFqGdYlCCWTtzNIS3uvA_imrR1o9J3ln7vW6WMIfuZ7j_pkBz3I5ClVOhmR3","q":"AMFOUsxxEAnDr8QlQPMg7MMi8yHv","g":"KzFQ61prEBg9Dj4cCNVuPWk3zNoJnL3Qjt5_EJnQXhV2j2X762S6H9Fp02sQihKxgSsrdcm2Ue7OyIEA06GyZqNL3TkrUCiPFNPfjplkS8w_GlYbW67gKqzdbXZFh76je3jMremtiriOHL7LRDfgmxf39Ozart5Vguh98zniBd8","y":"CDV3PDcrRwYDDTb2FnQntf-OLekF6kMm21pJeGCFKdh5iWzBU-7FwJkVbfFbQcjPymLpzEDT5y4GK3cqHmevg0heAgztPmlvGpB4piNP5l5sTDJBgDCnm7qJ_BWKJUxgdI6jnh0EyPrx0pEFJZVoAzqBUUDKXj7EiLfswRnq_wI","size":1024},"x":"D1I-0k7mlNMRcN1L1tXoKSYD80E","size":1024}

java/code/testdata/key-collision/dsa/2.attached

@@ -0,0 +1 @@
+AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAsAhQGKuj9kDwo9NXc_xmw0-8NDJhB1AIUcRoGhAEqCTIl3jstTN5YAACEwtU

java/code/testdata/key-collision/dsa/2.out

@@ -0,0 +1 @@
+AP2pfRwwLQIUebyB04kb72X0Ak9b73H88-4ODuUCFQCFq2geOmT2tz3LPwqCMFwLjG9E4w

java/code/testdata/key-collision/dsa/2.secret.attached

@@ -0,0 +1 @@
+AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAsAhQSmnjBtMyPDjNPybMm0BrPY8CyYAIUKxNsoCO2Sx5aE3iAQEbqgTa2Vt0

java/code/testdata/key-collision/dsa/2.timeout

@@ -0,0 +1 @@
+AP2pfRwAAAE7vSi1oDAtAhUAwO9Fw4LF9AnbRFkxgF7afpoFERYCFFGrJF6Df6gWRf1eSrdYM683fKF1