Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Commit c2cbcdd

Browse files
jbtulejbtule
committed
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
1 parent da0154d commit c2cbcdd

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+409
-82
lines changed

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

+80-45
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.keyczar.util.Base64Coder;
3030

3131
import java.nio.ByteBuffer;
32+
import java.util.Collection;
3233

3334
/**
3435
* Crypters may both encrypt and decrypt data using sets of symmetric or private
@@ -78,7 +79,7 @@ public Crypter(String fileLocation) throws KeyczarException {
7879
public byte[] decrypt(byte[] input) throws KeyczarException {
7980
ByteBuffer output = ByteBuffer.allocate(input.length);
8081
decrypt(ByteBuffer.wrap(input), output);
81-
output.reset();
82+
output.rewind();
8283
byte[] outputBytes = new byte[output.remaining()];
8384
output.get(outputBytes);
8485
return outputBytes;
@@ -107,57 +108,91 @@ public void decrypt(ByteBuffer input, ByteBuffer output)
107108

108109
byte[] hash = new byte[KEY_HASH_SIZE];
109110
inputCopy.get(hash);
110-
KeyczarKey key = getKey(hash);
111-
if (key == null) {
111+
Collection<KeyczarKey> keys = getKey(hash);
112+
if (keys == null) {
112113
throw new KeyNotFoundException(hash);
113114
}
114115

115116
// The input to decrypt is now positioned at the start of the ciphertext
116117
inputCopy.mark();
117-
118-
DecryptingStream cryptStream = (DecryptingStream) key.getStream();
119-
120-
VerifyingStream verifyStream = cryptStream.getVerifyingStream();
121-
if (inputCopy.remaining() < verifyStream.digestSize()) {
122-
throw new ShortCiphertextException(inputCopy.remaining());
118+
int inputLimit = inputCopy.limit();
119+
KeyczarException error = null;
120+
121+
boolean collision = keys.size() > 1;
122+
123+
for (KeyczarKey key : keys) {
124+
error = null;
125+
126+
ByteBuffer tempBuffer = output;
127+
if (collision) {
128+
tempBuffer = ByteBuffer.allocate(output.capacity());
129+
}
130+
131+
DecryptingStream cryptStream = (DecryptingStream) key.getStream();
132+
133+
try {
134+
VerifyingStream verifyStream = cryptStream.getVerifyingStream();
135+
if (inputCopy.remaining() < verifyStream.digestSize()) {
136+
throw new ShortCiphertextException(inputCopy.remaining());
137+
}
138+
139+
// Slice off the signature into another buffer
140+
inputCopy.position(inputCopy.limit() - verifyStream.digestSize());
141+
ByteBuffer signature = inputCopy.slice();
142+
143+
// Reset the position of the input to start of the ciphertext
144+
inputCopy.reset();
145+
inputCopy.limit(inputCopy.limit() - verifyStream.digestSize());
146+
147+
// Initialize the crypt stream. This may read an IV if any.
148+
cryptStream.initDecrypt(inputCopy);
149+
150+
// Verify the header and IV if any
151+
ByteBuffer headerAndIvToVerify = input.asReadOnlyBuffer();
152+
headerAndIvToVerify.limit(inputCopy.position());
153+
verifyStream.initVerify();
154+
verifyStream.updateVerify(headerAndIvToVerify);
155+
156+
tempBuffer.mark();
157+
// This will process large input in chunks, rather than all at once. This
158+
// avoids making two passes through memory.
159+
while (inputCopy.remaining() > DECRYPT_CHUNK_SIZE) {
160+
ByteBuffer ciphertextChunk = inputCopy.slice();
161+
ciphertextChunk.limit(DECRYPT_CHUNK_SIZE);
162+
cryptStream.updateDecrypt(ciphertextChunk, output);
163+
ciphertextChunk.rewind();
164+
verifyStream.updateVerify(ciphertextChunk);
165+
inputCopy.position(inputCopy.position() + DECRYPT_CHUNK_SIZE);
166+
}
167+
int lastBlock = inputCopy.position();
168+
verifyStream.updateVerify(inputCopy);
169+
if (!verifyStream.verify(signature)) {
170+
throw new InvalidSignatureException();
171+
}
172+
inputCopy.position(lastBlock);
173+
cryptStream.doFinalDecrypt(inputCopy, tempBuffer);
174+
tempBuffer.limit(tempBuffer.position());
175+
if (collision) {
176+
//Success copy to final output buffer
177+
tempBuffer.rewind();
178+
output.put(tempBuffer);
179+
output.limit(output.position());
180+
}
181+
return;
182+
} catch (KeyczarException e) {
183+
LOG.debug(e.getMessage(), e);
184+
error = e;
185+
} catch (RuntimeException e) {
186+
LOG.debug(e.getMessage(), e);
187+
error = new InvalidSignatureException();
188+
} finally {
189+
inputCopy.reset();
190+
inputCopy.limit(inputLimit);
191+
}
123192
}
124-
125-
// Slice off the signature into another buffer
126-
inputCopy.position(inputCopy.limit() - verifyStream.digestSize());
127-
ByteBuffer signature = inputCopy.slice();
128-
129-
// Reset the position of the input to start of the ciphertext
130-
inputCopy.reset();
131-
inputCopy.limit(inputCopy.limit() - verifyStream.digestSize());
132-
133-
// Initialize the crypt stream. This may read an IV if any.
134-
cryptStream.initDecrypt(inputCopy);
135-
136-
// Verify the header and IV if any
137-
ByteBuffer headerAndIvToVerify = input.asReadOnlyBuffer();
138-
headerAndIvToVerify.limit(inputCopy.position());
139-
verifyStream.initVerify();
140-
verifyStream.updateVerify(headerAndIvToVerify);
141-
142-
output.mark();
143-
// This will process large input in chunks, rather than all at once. This
144-
// avoids making two passes through memory.
145-
while (inputCopy.remaining() > DECRYPT_CHUNK_SIZE) {
146-
ByteBuffer ciphertextChunk = inputCopy.slice();
147-
ciphertextChunk.limit(DECRYPT_CHUNK_SIZE);
148-
cryptStream.updateDecrypt(ciphertextChunk, output);
149-
ciphertextChunk.rewind();
150-
verifyStream.updateVerify(ciphertextChunk);
151-
inputCopy.position(inputCopy.position() + DECRYPT_CHUNK_SIZE);
152-
}
153-
inputCopy.mark();
154-
verifyStream.updateVerify(inputCopy);
155-
if (!verifyStream.verify(signature)) {
156-
throw new InvalidSignatureException();
193+
if (error != null) {
194+
throw error;
157195
}
158-
inputCopy.reset();
159-
cryptStream.doFinalDecrypt(inputCopy, output);
160-
output.limit(output.position());
161196
}
162197

163198
/**

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

+16-5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.keyczar.interfaces.KeyczarReader;
2525
import org.keyczar.util.Util;
2626

27+
import java.util.ArrayList;
28+
import java.util.Collection;
2729
import java.util.HashMap;
2830

2931
/**
@@ -44,8 +46,8 @@ abstract class Keyczar {
4446
KeyVersion primaryVersion;
4547
final HashMap<KeyVersion, KeyczarKey> versionMap =
4648
new HashMap<KeyVersion, KeyczarKey>();
47-
final HashMap<KeyHash, KeyczarKey> hashMap =
48-
new HashMap<KeyHash, KeyczarKey>(); // keep track of used hash identifiers
49+
final HashMap<KeyHash, ArrayList<KeyczarKey>> hashMap =
50+
new HashMap<KeyHash, ArrayList<KeyczarKey>>(); // keep track of used hash identifiers
4951

5052
private class KeyHash {
5153
private byte[] data;
@@ -96,10 +98,19 @@ public Keyczar(KeyczarReader reader) throws KeyczarException {
9698
}
9799
String keyString = reader.getKey(version.getVersionNumber());
98100
KeyczarKey key = kmd.getType().getBuilder().read(keyString);
99-
hashMap.put(new KeyHash(key.hash()), key);
101+
addKeyHashMap(key.hash(), key);
100102
versionMap.put(version, key);
101103
}
102104
}
105+
106+
private void addKeyHashMap(byte[] hash, KeyczarKey key) {
107+
KeyHash kHash = new KeyHash(hash);
108+
if (hashMap.get(kHash) == null) {
109+
hashMap.put(kHash, new ArrayList<KeyczarKey>());
110+
}
111+
hashMap.get(kHash).add(key);
112+
}
113+
103114

104115
/**
105116
* Instantiates a new Keyczar object with a KeyczarFileReader instantiated
@@ -125,7 +136,7 @@ public String toString() {
125136
* @param key KeyczarKey
126137
*/
127138
void addKey(KeyVersion version, KeyczarKey key) {
128-
hashMap.put(new KeyHash(key.hash()), key);
139+
addKeyHashMap(key.hash(), key);
129140
versionMap.put(version, key);
130141
kmd.addVersion(version);
131142
}
@@ -137,7 +148,7 @@ KeyczarKey getPrimaryKey() {
137148
return versionMap.get(primaryVersion);
138149
}
139150

140-
KeyczarKey getKey(byte[] hash) {
151+
Collection<KeyczarKey> getKey(byte[] hash) {
141152
return hashMap.get(new KeyHash(hash));
142153
}
143154

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

+13-11
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,8 @@ public boolean verify(byte[] data, byte[] signature) throws KeyczarException {
9393
* @param data The data to verify the signature on
9494
* @param signature The signature to verify
9595
* @return Whether this is a valid signature
96-
* @throws KeyczarException If the signature is malformed or a JCE error occurs.
9796
*/
98-
public boolean verify(ByteBuffer data, ByteBuffer signature)
99-
throws KeyczarException {
97+
public boolean verify(ByteBuffer data, ByteBuffer signature) {
10098
for (KeyczarKey key : versionMap.values()) {
10199
if (verify(data, signature, key)) {
102100
return true;
@@ -105,13 +103,17 @@ public boolean verify(ByteBuffer data, ByteBuffer signature)
105103
return false;
106104
}
107105

108-
private boolean verify(ByteBuffer data, ByteBuffer signature, KeyczarKey key)
109-
throws KeyczarException {
110-
VerifyingStream stream = (VerifyingStream) key.getStream();
111-
stream.initVerify();
112-
stream.updateVerify(data.duplicate());
113-
boolean foundValidSignature = stream.verify(signature.duplicate());
114-
return foundValidSignature;
106+
private boolean verify(ByteBuffer data, ByteBuffer signature, KeyczarKey key) {
107+
try {
108+
VerifyingStream stream = (VerifyingStream) key.getStream();
109+
stream.initVerify();
110+
stream.updateVerify(data.duplicate());
111+
return stream.verify(signature.duplicate());
112+
} catch (KeyczarException e) {
113+
return false;
114+
} catch (RuntimeException e) {
115+
return false;
116+
}
115117
}
116118

117119
/**
@@ -137,4 +139,4 @@ boolean isAcceptablePurpose(KeyPurpose purpose) {
137139
return (purpose == KeyPurpose.VERIFY ||
138140
purpose == KeyPurpose.SIGN_AND_VERIFY);
139141
}
140-
}
142+
}

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

+42-21
Original file line numberDiff line numberDiff line change
@@ -117,26 +117,34 @@ boolean verify(ByteBuffer data, ByteBuffer hidden,
117117
}
118118

119119
byte[] hash = checkFormatAndGetHash(signature);
120-
KeyczarKey key = getVerifyingKey(hash);
120+
Iterable<KeyczarKey> keys = getVerifyingKey(hash);
121121

122-
if (key == null) {
122+
if (keys == null) {
123123
throw new KeyNotFoundException(hash);
124124
}
125-
126-
// TODO(normandl): replace following with rawVerify().
127-
VerifyingStream stream = (VerifyingStream) key.getStream();
128-
stream.initVerify();
125+
126+
data.mark();
129127
if (hidden != null) {
130-
stream.updateVerify(hidden);
128+
hidden.mark();
131129
}
132-
133-
stream.updateVerify(data);
134-
135-
// The signed data is terminated with the current Keyczar format
136-
stream.updateVerify(ByteBuffer.wrap(FORMAT_BYTES));
137-
138-
boolean result = stream.verify(signature);
139-
return result;
130+
signature.mark();
131+
for (KeyczarKey key : keys) {
132+
try {
133+
if (rawVerify(key,data, hidden, signature)) {
134+
return true;
135+
}
136+
} catch (KeyczarException e) {
137+
//Continue Checking keys in case of collision
138+
} catch (RuntimeException e) {
139+
//Unfortunately Java crypto apis can throw runtime exceptions
140+
}
141+
data.reset();
142+
if (hidden != null) {
143+
hidden.reset();
144+
}
145+
signature.reset();
146+
}
147+
return false;
140148
}
141149

142150

@@ -195,7 +203,6 @@ public boolean attachedVerify(final byte[] signedBlob,
195203
ByteBuffer sigBuffer = ByteBuffer.wrap(signedBlob);
196204
// assume I need to decode here as well.
197205
byte[] hash = checkFormatAndGetHash(sigBuffer);
198-
KeyczarKey key = getVerifyingKey(hash);
199206

200207
// we have stripped the format and hash, now just get the blob and
201208
// raw signature
@@ -210,10 +217,24 @@ public boolean attachedVerify(final byte[] signedBlob,
210217
// [blob | hidden.length | hidden | format] or [blob | 0 | format]
211218
byte[] hiddenPlusLength = Util.fromInt(0);
212219
if (hidden.length > 0) {
213-
hiddenPlusLength = Util.lenPrefix(hidden);
220+
hiddenPlusLength = Util.lenPrefix(hidden);
221+
}
222+
223+
Iterable<KeyczarKey> keys = getVerifyingKey(hash);
224+
for (KeyczarKey key : keys) {
225+
try {
226+
if (rawVerify(key, ByteBuffer.wrap(blob), ByteBuffer.wrap(hiddenPlusLength),
227+
ByteBuffer.wrap(signature))) {
228+
return true;
229+
}
230+
} catch (KeyczarException e) {
231+
//continue checking keys incase of collision
232+
} catch (RuntimeException e) {
233+
//unfortunately java crypto apis can throw a runtime exception
234+
}
214235
}
215-
return rawVerify(key, ByteBuffer.wrap(blob),
216-
ByteBuffer.wrap(hiddenPlusLength), ByteBuffer.wrap(signature));
236+
237+
return false;
217238
}
218239

219240
/**
@@ -278,8 +299,8 @@ private byte[] checkFormatAndGetHash(ByteBuffer signature)
278299
return hash;
279300
}
280301

281-
private KeyczarKey getVerifyingKey(byte[] hash) throws KeyNotFoundException {
282-
KeyczarKey key = getKey(hash);
302+
private Iterable<KeyczarKey> getVerifyingKey(byte[] hash) throws KeyNotFoundException {
303+
Iterable<KeyczarKey> key = getKey(hash);
283304

284305
if (key == null) {
285306
throw new KeyNotFoundException(hash);

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

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

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AKHGfTNGchk4xtw28PRtgcNHSBa1sy9aTXDcomTzui7i9_56Ws9mYQYc35j79sbF4XLJXtCVIm94Ur1itL-1rJwSaCLweayKzw

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

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

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AKHGfTNFRJZKAOzy8cofi3B7C2KJ3ARPXICvOIq-EHkaDJDIYAqNlMgk7znvoYEh2yiGAeV9C1Q1jgivKHN_aUsoqNO0w56dAg

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAuAhUAioxiTw-nS5mZSr2_4_ypN4mbXHACFQCwqVKuvKvdnVsznQnyk5KbHx9-kQ

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwwLAIUTpbMjHGAFJ81zGKFI9fbONckXroCFAXQaMD3qXQ5oe5APGal7wvcSSCz

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAsAhRlEVcAyc3Ep5MA8uRJFAMt9oATywIUU8XCn5sj7lpRAJUXc9uxlDU5nE0

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwAAAE7vSi1oDAsAhQUKQvYH7PJoq42lhBAK3qEk6RD3wIUCQd_4Xqw9u59UknfKg2O5xg3qe0

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAsAhQGKuj9kDwo9NXc_xmw0-8NDJhB1AIUcRoGhAEqCTIl3jstTN5YAACEwtU

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwwLQIUebyB04kb72X0Ak9b73H88-4ODuUCFQCFq2geOmT2tz3LPwqCMFwLjG9E4w

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwAAAAWVGhpcyBpcyBzb21lIHRlc3QgZGF0YTAsAhQSmnjBtMyPDjNPybMm0BrPY8CyYAIUKxNsoCO2Sx5aE3iAQEbqgTa2Vt0

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AP2pfRwAAAE7vSi1oDAtAhUAwO9Fw4LF9AnbRFkxgF7afpoFERYCFFGrJF6Df6gWRf1eSrdYM683fKF1

0 commit comments

Comments
 (0)