From f10512d7873246e6fc1df85665440edfa7deba84 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Wed, 16 Nov 2011 16:22:35 +0000
Subject: [PATCH] Erase keys after using them. (Copies created by ciphers, etc,
may still exist.)
---
api/net/sf/briar/api/crypto/ErasableKey.java | 3 +
.../net/sf/briar/crypto/ErasableKeyImpl.java | 4 +
.../transport/ConnectionDecrypterImpl.java | 105 +++++++++++-------
.../transport/ConnectionEncrypterImpl.java | 52 ++++++---
.../ConnectionReaderFactoryImpl.java | 1 +
.../briar/transport/ConnectionReaderImpl.java | 1 +
.../transport/ConnectionRecogniserImpl.java | 1 +
.../ConnectionWriterFactoryImpl.java | 1 +
.../briar/transport/ConnectionWriterImpl.java | 1 +
.../briar/transport/FrameReadWriteTest.java | 8 +-
10 files changed, 115 insertions(+), 62 deletions(-)
diff --git a/api/net/sf/briar/api/crypto/ErasableKey.java b/api/net/sf/briar/api/crypto/ErasableKey.java
index 2a46356c95..3503898ede 100644
--- a/api/net/sf/briar/api/crypto/ErasableKey.java
+++ b/api/net/sf/briar/api/crypto/ErasableKey.java
@@ -4,6 +4,9 @@ import javax.crypto.SecretKey;
public interface ErasableKey extends SecretKey {
+ /** Returns a copy of the key. */
+ ErasableKey copy();
+
/** Erases the key from memory. */
void erase();
}
diff --git a/components/net/sf/briar/crypto/ErasableKeyImpl.java b/components/net/sf/briar/crypto/ErasableKeyImpl.java
index 595485f3bb..5267e394d5 100644
--- a/components/net/sf/briar/crypto/ErasableKeyImpl.java
+++ b/components/net/sf/briar/crypto/ErasableKeyImpl.java
@@ -33,6 +33,10 @@ class ErasableKeyImpl implements ErasableKey {
return "RAW";
}
+ public ErasableKey copy() {
+ return new ErasableKeyImpl(getEncoded(), algorithm);
+ }
+
public void erase() {
if(erased) throw new IllegalStateException();
ByteUtils.erase(key);
diff --git a/components/net/sf/briar/transport/ConnectionDecrypterImpl.java b/components/net/sf/briar/transport/ConnectionDecrypterImpl.java
index 3f6f426baa..0232dd4e71 100644
--- a/components/net/sf/briar/transport/ConnectionDecrypterImpl.java
+++ b/components/net/sf/briar/transport/ConnectionDecrypterImpl.java
@@ -43,45 +43,58 @@ implements ConnectionDecrypter {
}
public void readMac(byte[] mac) throws IOException {
- if(betweenFrames) throw new IllegalStateException();
- // If we have any plaintext in the buffer, copy it into the MAC
- System.arraycopy(buf, bufOff, mac, 0, bufLen);
- // Read the remainder of the MAC
- int offset = bufLen;
- while(offset < mac.length) {
- int read = in.read(mac, offset, mac.length - offset);
- if(read == -1) break;
- offset += read;
- }
- if(offset < mac.length) throw new EOFException(); // Unexpected EOF
- // Decrypt the remainder of the MAC
try {
- int length = mac.length - bufLen;
- int i = frameCipher.doFinal(mac, bufLen, length, mac, bufLen);
- if(i < length) throw new RuntimeException();
- } catch(BadPaddingException badCipher) {
- throw new RuntimeException(badCipher);
- } catch(IllegalBlockSizeException badCipher) {
- throw new RuntimeException(badCipher);
- } catch(ShortBufferException badCipher) {
- throw new RuntimeException(badCipher);
+ if(betweenFrames) throw new IllegalStateException();
+ // If we have any plaintext in the buffer, copy it into the MAC
+ System.arraycopy(buf, bufOff, mac, 0, bufLen);
+ // Read the remainder of the MAC
+ int offset = bufLen;
+ while(offset < mac.length) {
+ int read = in.read(mac, offset, mac.length - offset);
+ if(read == -1) break;
+ offset += read;
+ }
+ if(offset < mac.length) throw new EOFException(); // Unexpected EOF
+ // Decrypt the remainder of the MAC
+ try {
+ int length = mac.length - bufLen;
+ int i = frameCipher.doFinal(mac, bufLen, length, mac, bufLen);
+ if(i < length) throw new RuntimeException();
+ } catch(BadPaddingException badCipher) {
+ throw new RuntimeException(badCipher);
+ } catch(IllegalBlockSizeException badCipher) {
+ throw new RuntimeException(badCipher);
+ } catch(ShortBufferException badCipher) {
+ throw new RuntimeException(badCipher);
+ }
+ bufOff = bufLen = 0;
+ betweenFrames = true;
+ } catch(IOException e) {
+ frameKey.erase();
+ throw e;
}
- bufOff = bufLen = 0;
- betweenFrames = true;
}
@Override
public int read() throws IOException {
- if(betweenFrames) initialiseCipher();
- if(bufLen == 0) {
- if(!readBlock()) return -1;
- bufOff = 0;
- bufLen = buf.length;
+ try {
+ if(betweenFrames) initialiseCipher();
+ if(bufLen == 0) {
+ if(!readBlock()) {
+ frameKey.erase();
+ return -1;
+ }
+ bufOff = 0;
+ bufLen = buf.length;
+ }
+ int i = buf[bufOff];
+ bufOff++;
+ bufLen--;
+ return i < 0 ? i + 256 : i;
+ } catch(IOException e) {
+ frameKey.erase();
+ throw e;
}
- int i = buf[bufOff];
- bufOff++;
- bufLen--;
- return i < 0 ? i + 256 : i;
}
@Override
@@ -91,17 +104,25 @@ implements ConnectionDecrypter {
@Override
public int read(byte[] b, int off, int len) throws IOException {
- if(betweenFrames) initialiseCipher();
- if(bufLen == 0) {
- if(!readBlock()) return -1;
- bufOff = 0;
- bufLen = buf.length;
+ try {
+ if(betweenFrames) initialiseCipher();
+ if(bufLen == 0) {
+ if(!readBlock()) {
+ frameKey.erase();
+ return -1;
+ }
+ bufOff = 0;
+ bufLen = buf.length;
+ }
+ int length = Math.min(len, bufLen);
+ System.arraycopy(buf, bufOff, b, off, length);
+ bufOff += length;
+ bufLen -= length;
+ return length;
+ } catch(IOException e) {
+ frameKey.erase();
+ throw e;
}
- int length = Math.min(len, bufLen);
- System.arraycopy(buf, bufOff, b, off, length);
- bufOff += length;
- bufLen -= length;
- return length;
}
// Although we're using CTR mode, which doesn't require full blocks of
diff --git a/components/net/sf/briar/transport/ConnectionEncrypterImpl.java b/components/net/sf/briar/transport/ConnectionEncrypterImpl.java
index 5494b84bc1..79066fac1c 100644
--- a/components/net/sf/briar/transport/ConnectionEncrypterImpl.java
+++ b/components/net/sf/briar/transport/ConnectionEncrypterImpl.java
@@ -46,6 +46,7 @@ implements ConnectionEncrypter {
}
if(encryptedIv.length != IV_LENGTH)
throw new IllegalArgumentException();
+ ivKey.erase();
}
public OutputStream getOutputStream() {
@@ -53,16 +54,21 @@ implements ConnectionEncrypter {
}
public void writeMac(byte[] mac) throws IOException {
- if(!ivWritten || betweenFrames) throw new IllegalStateException();
try {
- out.write(frameCipher.doFinal(mac));
- } catch(BadPaddingException badCipher) {
- throw new RuntimeException(badCipher);
- } catch(IllegalBlockSizeException badCipher) {
- throw new RuntimeException(badCipher);
+ if(!ivWritten || betweenFrames) throw new IllegalStateException();
+ try {
+ out.write(frameCipher.doFinal(mac));
+ } catch(BadPaddingException badCipher) {
+ throw new RuntimeException(badCipher);
+ } catch(IllegalBlockSizeException badCipher) {
+ throw new RuntimeException(badCipher);
+ }
+ capacity -= mac.length;
+ betweenFrames = true;
+ } catch(IOException e) {
+ frameKey.erase();
+ throw e;
}
- capacity -= mac.length;
- betweenFrames = true;
}
public long getRemainingCapacity() {
@@ -71,11 +77,16 @@ implements ConnectionEncrypter {
@Override
public void write(int b) throws IOException {
- if(!ivWritten) writeIv();
- if(betweenFrames) initialiseCipher();
- byte[] ciphertext = frameCipher.update(new byte[] {(byte) b});
- if(ciphertext != null) out.write(ciphertext);
- capacity--;
+ try {
+ if(!ivWritten) writeIv();
+ if(betweenFrames) initialiseCipher();
+ byte[] ciphertext = frameCipher.update(new byte[] {(byte) b});
+ if(ciphertext != null) out.write(ciphertext);
+ capacity--;
+ } catch(IOException e) {
+ frameKey.erase();
+ throw e;
+ }
}
@Override
@@ -85,11 +96,16 @@ implements ConnectionEncrypter {
@Override
public void write(byte[] b, int off, int len) throws IOException {
- if(!ivWritten) writeIv();
- if(betweenFrames) initialiseCipher();
- byte[] ciphertext = frameCipher.update(b, off, len);
- if(ciphertext != null) out.write(ciphertext);
- capacity -= len;
+ try {
+ if(!ivWritten) writeIv();
+ if(betweenFrames) initialiseCipher();
+ byte[] ciphertext = frameCipher.update(b, off, len);
+ if(ciphertext != null) out.write(ciphertext);
+ capacity -= len;
+ } catch(IOException e) {
+ frameKey.erase();
+ throw e;
+ }
}
private void writeIv() throws IOException {
diff --git a/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java b/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java
index ffc95ca0c0..0109c7f469 100644
--- a/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java
+++ b/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java
@@ -42,6 +42,7 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory {
} catch(InvalidKeyException badKey) {
throw new IllegalArgumentException(badKey);
}
+ ivKey.erase();
// Validate the IV
if(!IvEncoder.validateIv(iv, true, ctx))
throw new IllegalArgumentException();
diff --git a/components/net/sf/briar/transport/ConnectionReaderImpl.java b/components/net/sf/briar/transport/ConnectionReaderImpl.java
index c637458f14..28b37389c3 100644
--- a/components/net/sf/briar/transport/ConnectionReaderImpl.java
+++ b/components/net/sf/briar/transport/ConnectionReaderImpl.java
@@ -40,6 +40,7 @@ implements ConnectionReader {
} catch(InvalidKeyException e) {
throw new IllegalArgumentException(e);
}
+ macKey.erase();
maxPayloadLength = MAX_FRAME_LENGTH - 4 - mac.getMacLength();
header = new byte[4];
payload = new byte[maxPayloadLength];
diff --git a/components/net/sf/briar/transport/ConnectionRecogniserImpl.java b/components/net/sf/briar/transport/ConnectionRecogniserImpl.java
index fb5e59eff1..b4da6dc02e 100644
--- a/components/net/sf/briar/transport/ConnectionRecogniserImpl.java
+++ b/components/net/sf/briar/transport/ConnectionRecogniserImpl.java
@@ -92,6 +92,7 @@ DatabaseListener {
byte[] secret = e.getValue();
ErasableKey ivKey = crypto.deriveIvKey(secret, true);
Bytes iv = new Bytes(encryptIv(i, unseen, ivKey));
+ ivKey.erase();
expected.put(iv, new ConnectionContextImpl(c, i, unseen, secret));
}
}
diff --git a/components/net/sf/briar/transport/ConnectionWriterFactoryImpl.java b/components/net/sf/briar/transport/ConnectionWriterFactoryImpl.java
index 0b91a80d77..35b55f47c4 100644
--- a/components/net/sf/briar/transport/ConnectionWriterFactoryImpl.java
+++ b/components/net/sf/briar/transport/ConnectionWriterFactoryImpl.java
@@ -47,6 +47,7 @@ class ConnectionWriterFactoryImpl implements ConnectionWriterFactory {
} catch(InvalidKeyException badKey) {
throw new RuntimeException(badKey);
}
+ ivKey.erase();
// Validate the IV
if(!IvEncoder.validateIv(iv, true, ctx))
throw new IllegalArgumentException();
diff --git a/components/net/sf/briar/transport/ConnectionWriterImpl.java b/components/net/sf/briar/transport/ConnectionWriterImpl.java
index e66c373bc8..61359e6152 100644
--- a/components/net/sf/briar/transport/ConnectionWriterImpl.java
+++ b/components/net/sf/briar/transport/ConnectionWriterImpl.java
@@ -41,6 +41,7 @@ implements ConnectionWriter {
} catch(InvalidKeyException badKey) {
throw new IllegalArgumentException(badKey);
}
+ macKey.erase();
maxPayloadLength = MAX_FRAME_LENGTH - 4 - mac.getMacLength();
buf = new ByteArrayOutputStream(maxPayloadLength);
header = new byte[4];
diff --git a/test/net/sf/briar/transport/FrameReadWriteTest.java b/test/net/sf/briar/transport/FrameReadWriteTest.java
index 07b00d9b98..b7fc4f51d8 100644
--- a/test/net/sf/briar/transport/FrameReadWriteTest.java
+++ b/test/net/sf/briar/transport/FrameReadWriteTest.java
@@ -74,12 +74,16 @@ public class FrameReadWriteTest extends TestCase {
random.nextBytes(frame);
byte[] frame1 = new byte[321];
random.nextBytes(frame1);
+ // Copy the keys - the copies will be erased
+ ErasableKey frameCopy = frameKey.copy();
+ ErasableKey ivCopy = ivKey.copy();
+ ErasableKey macCopy = macKey.copy();
// Write the frames
ByteArrayOutputStream out = new ByteArrayOutputStream();
ConnectionEncrypter encrypter = new ConnectionEncrypterImpl(out,
- Long.MAX_VALUE, iv, ivCipher, frameCipher, ivKey, frameKey);
+ Long.MAX_VALUE, iv, ivCipher, frameCipher, ivCopy, frameCopy);
ConnectionWriter writer = new ConnectionWriterImpl(encrypter, mac,
- macKey);
+ macCopy);
OutputStream out1 = writer.getOutputStream();
out1.write(frame);
out1.flush();
--
GitLab