The KDF was using CTR mode unsafely.

The data to be encrypted should go in the IV, with a blank
plaintext, so that the ciphertext is equal to the keystream.

Putting the data in the plaintext would have led to different keys
derived from the same source consisting of the same keystream XORed
with different guessable plaintexts. That would have been bad.
This commit is contained in:
akwizgran
2011-11-17 09:01:59 +00:00
parent a144884ecd
commit 13ebd369e2

View File

@@ -36,14 +36,17 @@ class CryptoComponentImpl implements CryptoComponent {
private static final String KEY_DERIVATION_ALGO = "AES/CTR/NoPadding"; private static final String KEY_DERIVATION_ALGO = "AES/CTR/NoPadding";
private static final int KEY_DERIVATION_IV_BYTES = 16; // 128 bits private static final int KEY_DERIVATION_IV_BYTES = 16; // 128 bits
// Labels for key derivation, null-terminated
private static final byte[] FRAME = { 'F', 'R', 'A', 'M', 'E', 0 };
private static final byte[] IV = { 'I', 'V', 0 };
private static final byte[] MAC = { 'M', 'A', 'C', 0 };
private static final byte[] NEXT = { 'N', 'E', 'X', 'T', 0 };
// Context strings for key derivation // Context strings for key derivation
private static final byte[] FRAME_I = { 'F', 'R', 'A', 'M', 'E', '_', 'I' }; private static final byte[] INITIATOR = { 'I' };
private static final byte[] FRAME_R = { 'F', 'R', 'A', 'M', 'E', '_', 'R' }; private static final byte[] RESPONDER = { 'R' };
private static final byte[] IV_I = { 'I', 'V', '_', 'I' }; // Blank plaintext for key derivation
private static final byte[] IV_R = { 'I', 'V', '_', 'R' }; private static final byte[] KEY_DERIVATION_INPUT =
private static final byte[] MAC_I = { 'M', 'A', 'C', '_', 'I' }; new byte[SECRET_KEY_BYTES];
private static final byte[] MAC_R = { 'M', 'A', 'C', '_', 'R' };
private static final byte[] NEXT = { 'N', 'E', 'X', 'T' };
private final KeyParser keyParser; private final KeyParser keyParser;
private final KeyPairGenerator keyPairGenerator; private final KeyPairGenerator keyPairGenerator;
@@ -62,47 +65,46 @@ class CryptoComponentImpl implements CryptoComponent {
} }
public ErasableKey deriveFrameKey(byte[] secret, boolean initiator) { public ErasableKey deriveFrameKey(byte[] secret, boolean initiator) {
if(initiator) return deriveKey(secret, FRAME_I); if(initiator) return deriveKey(secret, FRAME, INITIATOR);
else return deriveKey(secret, FRAME_R); else return deriveKey(secret, FRAME, RESPONDER);
} }
public ErasableKey deriveIvKey(byte[] secret, boolean initiator) { public ErasableKey deriveIvKey(byte[] secret, boolean initiator) {
if(initiator) return deriveKey(secret, IV_I); if(initiator) return deriveKey(secret, IV, INITIATOR);
else return deriveKey(secret, IV_R); else return deriveKey(secret, IV, RESPONDER);
} }
public ErasableKey deriveMacKey(byte[] secret, boolean initiator) { public ErasableKey deriveMacKey(byte[] secret, boolean initiator) {
if(initiator) return deriveKey(secret, MAC_I); if(initiator) return deriveKey(secret, MAC, INITIATOR);
else return deriveKey(secret, MAC_R); else return deriveKey(secret, MAC, RESPONDER);
} }
private ErasableKey deriveKey(byte[] secret, byte[] context) { private ErasableKey deriveKey(byte[] secret, byte[] label, byte[] context) {
byte[] key = counterModeKdf(secret, context); byte[] key = counterModeKdf(secret, label, context);
return new ErasableKeyImpl(key, SECRET_KEY_ALGO); return new ErasableKeyImpl(key, SECRET_KEY_ALGO);
} }
// Key derivation function based on a block cipher in CTR mode - see // Key derivation function based on a block cipher in CTR mode - see
// NIST SP 800-108, section 5.1 // NIST SP 800-108, section 5.1
private byte[] counterModeKdf(byte[] secret, byte[] context) { private byte[] counterModeKdf(byte[] secret, byte[] label, byte[] context) {
// The secret must be usable as a key // The secret must be usable as a key
if(secret.length != SECRET_KEY_BYTES) if(secret.length != SECRET_KEY_BYTES)
throw new IllegalArgumentException(); throw new IllegalArgumentException();
ErasableKey key = new ErasableKeyImpl(secret, SECRET_KEY_ALGO); ErasableKey key = new ErasableKeyImpl(secret, SECRET_KEY_ALGO);
// The context must leave two bytes free for the length // The label and context must leave a byte free for the counter
if(context.length + 2 > SECRET_KEY_BYTES) if(label.length + context.length + 1 > KEY_DERIVATION_IV_BYTES)
throw new IllegalArgumentException(); throw new IllegalArgumentException();
byte[] input = new byte[SECRET_KEY_BYTES]; // The IV starts with the null-terminated label
// The input starts with the length of the context as a big-endian int16 byte[] ivBytes = new byte[KEY_DERIVATION_IV_BYTES];
ByteUtils.writeUint16(context.length, input, 0); System.arraycopy(label, 0, ivBytes, 0, label.length);
// The remaining bytes of the input are the context // Next comes the context, leaving the last byte free for the counter
System.arraycopy(context, 0, input, 2, context.length); System.arraycopy(context, 0, ivBytes, label.length, context.length);
// Initialise the counter to zero assert ivBytes[ivBytes.length - 1] == 0;
byte[] zero = new byte[KEY_DERIVATION_IV_BYTES]; IvParameterSpec iv = new IvParameterSpec(ivBytes);
IvParameterSpec iv = new IvParameterSpec(zero);
try { try {
Cipher cipher = Cipher.getInstance(KEY_DERIVATION_ALGO, PROVIDER); Cipher cipher = Cipher.getInstance(KEY_DERIVATION_ALGO, PROVIDER);
cipher.init(Cipher.ENCRYPT_MODE, key, iv); cipher.init(Cipher.ENCRYPT_MODE, key, iv);
byte[] output = cipher.doFinal(input); byte[] output = cipher.doFinal(KEY_DERIVATION_INPUT);
assert output.length == SECRET_KEY_BYTES; assert output.length == SECRET_KEY_BYTES;
return output; return output;
} catch(GeneralSecurityException e) { } catch(GeneralSecurityException e) {
@@ -115,11 +117,10 @@ class CryptoComponentImpl implements CryptoComponent {
throw new IllegalArgumentException(); throw new IllegalArgumentException();
if(connection < 0 || connection > ByteUtils.MAX_32_BIT_UNSIGNED) if(connection < 0 || connection > ByteUtils.MAX_32_BIT_UNSIGNED)
throw new IllegalArgumentException(); throw new IllegalArgumentException();
byte[] context = new byte[NEXT.length + 6]; byte[] context = new byte[6];
System.arraycopy(NEXT, 0, context, 0, NEXT.length); ByteUtils.writeUint16(index, context, 0);
ByteUtils.writeUint16(index, context, NEXT.length); ByteUtils.writeUint32(connection, context, 2);
ByteUtils.writeUint32(connection, context, NEXT.length + 2); return counterModeKdf(secret, NEXT, context);
return counterModeKdf(secret, context);
} }
public KeyPair generateKeyPair() { public KeyPair generateKeyPair() {